Resolve ${user.home} and other JVM system properties in MCP server env, headers, and cwd — previously only OS env vars were expanded, causing the filesystem MCP server to fail on Windows where $HOME isn't set.
Temporarily restore McpClientManager.java to its pre-#60 state so the
contributor's PR can squash-merge cleanly with their authorship preserved.
The args-expansion follow-up will land as a separate commit right after.
When a user-installed skill (e.g. RedisOps) was bound to an agent, the
model frequently called the skill name directly as a tool, hit
"Tool not found: RedisOps", and either gave up or fell back to shell
guessing. Two compounding causes:
1. The system prompt block injected by SkillRuntimeService listed each
skill as `- **RedisOps** — desc`, which is the same format used for
tool catalogs and primed the model to call the names directly. The
"how to use" instructions referenced `read_skill_file` /
`run_skill_script` — names that don't exist in the tool registry,
so even a compliant LLM couldn't follow them.
2. ToolExecutionExecutor's `callback == null` branches returned a bare
"Tool not found: <name>" string. The model had no recovery signal
and no hint that the name it called was actually a skill.
Fix is two-layered:
- Prompt rewrite (SkillRuntimeService.buildSkillPromptEnhancement): lead
with an explicit warning that skills are NOT directly callable, use the
correct camelCase tool names (readSkillFile / runSkillScript), include
a concrete worked example anchored to the first enabled skill, and
render the listing as a markdown table so it stops looking like a
callable tool list. listAvailableSkills tool description and output
follow the same pattern.
- Runtime safety net (ToolExecutionExecutor): when toolCallbackMap.get
misses, check if the requested name (case-insensitive) matches an
active skill. If so, return a precise hint telling the LLM the right
invocation pattern instead of the bare error. Wired through both the
main execute path and the pre-approved replay path. SkillRuntimeService
is attached via a setter from AgentGraphBuilder so the executor's many
legacy constructors stay untouched, and it's nullable so isolated
tests still work.
Adds 5 unit tests covering: skill match -> hint, case-insensitive match,
no-match -> bare error, no SkillRuntimeService wired -> bare error,
pre-approved replay path -> hint.
Reported and reproduced by @pipima9950-glitch in issue #46.
Three layers landed together because they share the same routing /
lifecycle plumbing:
1. Cron output unification
- New CronConversationResolver routes web-origin jobs to the per-workspace
tasks_<wsId> conversation; IM-bound jobs go to the channel session
conversation when one exists (matched by senderId then targetId);
legacy cron_<id> remains as the fallback.
- CronJobLifecycleService inserts a system-role header divider when a
run starts so users browsing the unified tasks_<wsId> view can tell
which job started a run. BaseAgent.sanitizeForLlm filters these
headers so they never reach the model.
- WorkspaceService seeds tasks_<wsId> on workspace creation; V65
migration backfills existing workspaces.
- DeliveryConfig gains a userId field so IM session lookup can match
by senderId (replyToken-based targetId is not stable across runs).
- ConversationVO recognizes tasks_/cron_ underscore prefix as cron
source. MessageList renders the system header as a labeled divider.
- ChatConsole pins tasks_* conversations and tracks per-conversation
read state so new cron output gets a visible unread dot.
2. Reminder task type
- New task_type='reminder' in CronJobEntity + service validation.
- CronJobRunner short-circuits 'reminder' jobs: hands trigger_message
to finishRunAndPublish verbatim, no LLM call. Fixes a regression
where reminders were rephrased into echoed wrappers.
- New create_reminder tool alongside create_cron_job, with descriptions
tightened so the model picks the right one (verbatim push vs LLM
query that needs computation).
- CronJobs.vue gets a third radio option + dedicated reminder field.
3. In-flight progress placeholder
- Cron uses non-streaming chat()/execute(); tool-heavy ReAct loops
can run 1-5 minutes between start and finish with no visible
state, looking hung.
- New GET /api/v1/cron-jobs/active-runs returns runs in status=running
for a conversation. ChatConsole polls it on the existing 4s tick
(and on conversation switch) and shows a spinner bar with elapsed
time. When run count drops to zero, it refetches messages so the
assistant bubble appears within ~1s of finish.
Chat attachments with non-ASCII filenames (e.g. Chinese) get sanitized
at upload time — `人人有虾.docx` is stored as `1777391026594_____.docx`.
Tools then receive only the original filename via '[Attachment] foo.docx'
and fail with 'file not found'.
- renderMessageContent now appends the actual server-side path so any
tool the LLM picks (read_file / extract_document_text /
detect_file_type) gets a path that resolves directly.
- New ChatUploadResolver helper performs basename-suffix matching inside
the conversation's chat-upload directory; ReadFileTool, DocumentExtractTool
and FileTypeDetectorTool fall through to it when the literal path does
not exist (defense in depth for cases where the LLM ignores the path
hint).
Refs https://github.com/matevip/mateclaw/issues/29
User-reported field issues + a deeper code audit revealed multiple
overlapping bugs in the prior cron-channel delivery change. This fixes
all six.
#1 — Concurrency race on ToolExecutionExecutor (root cause of 'sometimes
succeeds, sometimes fails' tool calls). The volatile instance fields
currentRequesterId / currentWorkspaceBasePath / currentChatOrigin
were shared by every conversation routed through the same per-agent
executor; one user mid-build-loop while another's execute()
overwrote the field would cross-contaminate the captured values into
PreparedToolCall. Fix: kill the instance fields, thread
origin/requester/workspace as method params straight into
PreparedToolCall snapshot. Comment pins the rule so it cannot regress.
#2 — CHAT_ORIGIN missing from KeyStrategyFactory (latent timebomb,
masked by spring-ai-alibaba-graph-core's non-filtering builder path).
Without an addStrategy registration, multi-node state merges in long
ReAct / Plan-Execute loops drop the key, ActionNode reads
ChatOrigin.EMPTY, and the cron persists with channel_id=NULL. Also
caught 4 more keys that were latently unregistered:
WORKSPACE_BASE_PATH, STOP_REQUESTED, RETURN_DIRECT_TRIGGERED,
DIRECT_TOOL_OUTPUTS. All five now registered in both ReAct and
Plan-Execute factories.
#3 — CronJobs UI didn't surface channel binding. CronJobDTO carried
channelId / deliveryConfig but the list page never rendered them.
Added: (a) 'channel' column on list page, (b) channel + targetId
rows in the detail modal, (c) backend batch-loads channel names via
ChannelMapper.selectBatchIds so the column shows the human-readable
name, (d) i18n keys (zh + en), (e) channelName field on TS CronJob
type.
#4a — DingTalk targetId expiry. ChannelChatOriginFactory.resolveTargetId
used to prefer ChannelMessage.replyToken which for DingTalk encodes
a sessionWebhook URL that expires ~90 minutes after the inbound
message. Cron persisted with that webhook then dies with 401/403 and
marks NOT_DELIVERED forever. Fix: prefer the stable chatId, fall
back to senderId — both work indefinitely via DingTalk's Robot API.
#4b — Scheduler pool exhaustion under long LLM. CronJobService's
ThreadPoolTaskScheduler ran with poolSize=4 AND the LLM call lived
on the scheduler thread. Four concurrent crons saturated the pool
and the 5th silently missed its tick. Fix: keep scheduler tiny (it
just fires triggers) and offload runAgent to a dedicated
virtual-thread executor (cron-execute-* threads). LLM workload is
I/O-bound — virtual threads scale to thousands at trivial cost.
#5 — Minor latent bugs:
- AbstractCronResultDelivery.claimRun used .in(... 'NONE','PENDING',null),
but SQL IN never matches NULL. Rewrote as IS NULL OR IN
(NONE,PENDING) so legacy pre-V57 rows can still claim.
- CronDeliveryListener.onCompletedRaw was an empty @EventListener
with a wrong-headed comment about test fallbackExecution. Removed.
- CronJobTool.resolveAgentId silently returned 1L when origin
lacked an agentId — would silently bind to whatever agent #1
happens to be. Replaced with explicit error so wiring bugs surface
immediately instead of producing scheduled-but-never-runs crons.
State-key registration guard. New StateKeyRegistrationCoverageTest
scans MateClawStateKeys via reflection and parses
AgentGraphBuilder.java to extract every
.addStrategy(MateClawStateKeys.X, ...). Asserts every non-_NODE
constant appears in at least one factory. Caught the 4 unregistered
keys above on first run; will catch any future 'forgot to register'
regression.
Tests: 33 unit/arch tests + 27 regression in touched areas — all green.
Vue typecheck clean.
Refs: #25, #16
Replaces the prior ThreadLocal context plumbing with explicit Spring AI
ToolContext threading carried by an immutable ChatOrigin value object,
so a cron created from inside WeChat (or any IM channel) delivers its
results back to the originating channel.
Architecture
- ChatOrigin / ChannelTarget value objects + per-entry-point factories
(ChannelChatOriginFactory in vip.mate.channel, CronChatOriginFactory
in vip.mate.cron — symmetric, no cyclic deps).
- LocaleAwareToolCallback now forwards call(String, ToolContext) and
getToolMetadata so the decorator chain cannot silently drop the origin.
- AgentService 6-method overhaul + ChatOriginHolder bridge into
StateGraph buildInitialState which writes CHAT_ORIGIN; ActionNode +
StepExecutionNode forward it to ToolExecutionExecutor.
- ToolExecutionExecutor builds ToolContext per call; 8/8 tools migrated
(CronJobTool, WorkspacePathGuard, Video/Image/Browser/ReadFile/Music,
DelegateAgentTool with parent-origin inheritance).
- CronJobRunner + CronJobLifecycleService 3-segment REQUIRES_NEW model
(T1 startRun / no-tx runAgent / T2 finishRunAndPublish); ArchUnit
pins CronJobRunner as @Transactional-free.
- CronResultDelivery Strategy + AbstractCronResultDelivery Template
with SQL CAS idempotency on mate_cron_job_run.delivery_status —
replaces the prior process-local Caffeine TTL, cluster-safe.
- CronJobCompletedEvent + @Async @TransactionalEventListener(AFTER_COMMIT);
cronDeliveryExecutor (core=2, max=4, queue=1000, AbortPolicy + audit).
- CronRunStaleCleanup @Scheduled(5min) sweeps PENDING-15min and
status='running'-30min in one query each.
- CronJobRunner.wrapWithDeliveryGuard prepends a system note for
channel-bound crons to suppress hallucinated 'install CLI to send
WeChat' suggestions.
- ApprovalWorkflowService Memento: persist ChatOrigin snapshot on
create, restore on replay so cross-restart approvals keep channel
binding; ChannelMessageRouter + ChatController web-replay both prefer
the Memento and fall back to fresh-build.
- ChannelManager.sendToChannel 4-arg DeliveryOptions overload;
ChannelAdapter#proactiveSend default 4-arg pass-through; Slack
overrides for thread_ts and Telegram overrides for message_thread_id.
- CronJobs UI: read-only 'last delivery' badge driven by
CronJobMapper.selectListWithDeliveryStatus subquery.
Schema migrations V57/V58/V59 (V56 was already taken by an unrelated
provider migration — Flyway processes versions in order regardless of
gaps):
- V57: mate_cron_job_run delivery_status / target / error + composite
index (delivery_status, started_at) covering the cleanup sweep.
- V58: mate_cron_job channel_id (indexed) + delivery_config TEXT (JSON
via MyBatis Plus JacksonTypeHandler).
- V59: mate_tool_approval chat_origin TEXT (Memento).
All idempotent in both H2 (IF NOT EXISTS) and MySQL (INFORMATION_SCHEMA
guard + PREPARE).
ArchUnit guards (test scope, archunit-junit5 1.3.0):
- every concrete vip.mate.* ToolCallback must override
call(String, ToolContext) — pins the decorator-forward fix.
- CronJobRunner must NOT carry @Transactional on the class or any
method — pins the 3-segment lifecycle rule.
Tests: 32 new unit tests + 21 regression tests in touched areas, all
53 green:
- ChatOriginTest (6) — value-object invariants + JSON round-trip.
- LocaleAwareToolCallbackToolContextTest (2) — decorator forward.
- DeliveryConfigTest (4) — Jackson round-trip + forward-compat.
- ToolCallbackToolContextForwardArchTest (2) — both ArchUnit guards.
- CronJobRunnerDeliveryGuardTest (3) — channel-cron prefix injection.
- AbstractCronResultDeliveryTest (4) — claim CAS + concurrent CAS.
- ChannelCronResultDeliveryTest (6) — supports / doDeliver / errors.
- ApprovalReplayContinuityTest (5) — Memento round-trip + corrupt
payload fallback + unknown-field tolerance.
Refs: #25, #16
Issue #24: tools selected in the agent binding UI had no effect at runtime.
mate_tool.name stores the Java class name (e.g. "BrowserUseTool") and was
written into mate_agent_tool.tool_name, but AgentToolSet.withAllowedToolsOnly
matched by the @Tool function name (e.g. "browser_use") — so every binding
was silently filtered out.
Fix: AgentToolSet builds an alias index per ToolCallback indexed by every
equivalent identifier — function name, Spring bean name, and Java class
simple name. withAllowedToolsOnly / withDeniedToolsFiltered / excluding
all accept any of these aliases, mirroring how Spring's BeanFactory accepts
bean names + aliases.
ToolRegistry.getEnabledToolSet now threads a bean→beanName resolver into
the new AgentToolSet.fromCallbacks(...) overload. Existing two-arg callers
keep working; tests pass without changes.
Zero data migration: stale mate_agent_tool rows that previously had no
effect now resolve correctly via the class-name alias.
