mirror of
https://gitee.com/mateos/mateclaw.git
synced 2026-09-13 03:13:41 +08:00
feat(agent): 注意力锚定与环境感知——MCP 工具溯源 + skill 约束固定 + 事件通知 (#490)
* feat(agent): 注意力锚定与环境感知——MCP 工具溯源 + skill 约束固定 + 事件通知
## 背景
1. **MCP 工具跨服务器混淆**:MCP 工具名是 `mcp_<serverId>_<slug>_<hash6>`,serverId 是 19 位不可读 Snowflake。LLM 在多服务器任务中常把 slug 拼到错误 serverId 上重构出不存在的工具名,反复重试到 max iterations。
2. **长对话中 skill 约束丢失**:`load_skill` 返回的 SKILL.md 正文存在 messages 历史窗口里,被压缩管线(Soft Trim / Hard Clear / Pre-Prune / LLM Summary)销毁,约束彻底消失,agent 后续步骤违反约束。
3. **运行时环境变更对 agent 不可见**:MCP 服务器断连 / skill 更新发生在 agent 推理中途时,工具列表是 turn-start 快照,LLM 无法感知,继续调用已失效的工具。
4. **ledger 条目可被 LLM 反向覆盖**:Java 用 `auto_`/`pin_` 前缀让位给 LLM,但 LLM 没有反向保护——`progress_update(stepKey="auto_read_file")` 会覆盖 Java 写入的条目,保护是单向的。
5. **SkillManifestParser 从未填充 constraints 字段**:`KNOWN_KEYS` 未列入 `"constraints"`,导致约束被静默路由到 `extras`,所有依赖 `manifest.getConstraints()` 的代码都是死代码。
## 改动内容
### 文件改动
**新增文件(生产代码 4 个)**
- **`mateclaw-server/.../agent/runtime/EnvironmentNotification.java`** — 环境变更通知 record(type / message / timestamp)。
- **`mateclaw-server/.../agent/runtime/RunningConversationRegistry.java`** — 跟踪活跃会话 + 每会话有界通知队列(上限 10)+ TTL 定时清理(30 分钟未活跃的 handle 自动回收)。
- **`mateclaw-server/.../agent/runtime/EnvironmentEventRouter.java`** — 5 个 `@EventListener` 把 MCP/skill 事件翻译成中文 LLM 通知并广播。
- **`mateclaw-server/.../skill/event/SkillUpdatedEvent.java`** — skill 更新/启用/禁用/重扫描事件。
**新增文件(测试 5 个)**
- **`mateclaw-server/.../skill/manifest/SkillManifestConstraintsParsingTest.java`** — constraints 解析白盒测试(5 用例)。
- **`mateclaw-server/.../agent/progress/ProgressLedgerPrefixGuardTest.java`** — 前缀守卫 + 三类条目 + 并发 + 批量 auto-record 白盒(22 用例)。
- **`mateclaw-server/.../agent/runtime/RunningConversationRegistryTest.java`** — registry + router 生命周期 + TTL 清理白盒(24 用例)。
- **`mateclaw-server/.../agent/context/ContextCompressionLedgerSurvivalTest.java`** — 三类条目压缩存活黑盒(5 用例)。
- **`mateclaw-server/.../agent/graph/node/EnvironmentNotificationRenderingTest.java`** — 事件→通知→LLM 可见黑盒(14 用例)。
**修改文件(生产代码 14 个)**
- **`mateclaw-server/.../agent/progress/ProgressLedger.java`** — 增加 `pinned` map + `AUTO_RECORDED_PREFIX` 常量 + 三类条目区分;`mostRecentUpdate` 只看 regular 条目;`renderStaleReminder` 补 pending 计数。
- **`mateclaw-server/.../agent/progress/ProgressLedgerService.java`** — JSON 格式升级为 wrapper `{entries, pinned}`(向后兼容旧 flat-map);`upsert` 加 `auto_`/`pin_` 前缀守卫;`upsertPinned` / `upsertAutoRecorded` / `clearPinnedByPrefix` / `upsertAutoRecordedBatch`(批量版,一次 lock+load+save 处理 N 个工具响应);auto-recorded 4 参签名避免跨服务器键碰撞,有界=5。
- **`mateclaw-server/.../agent/graph/node/ActionNode.java`** — `load_skill` 后 `pinSkillConstraints` 把约束写入 pinned;工具调用后 `autoRecordToolCalls` 收集批量后一次 `upsertAutoRecordedBatch`(避免 N 次 lock+save 串行化);setter 注入保持测试构造器兼容。
- **`mateclaw-server/.../agent/graph/node/ReasoningNode.java`** — C4 注入:drain 通知 → `renderEnvironmentNotifications` → SystemMessage 加入 nonHistoryPrefix;helper 改 package-private 供黑盒测试。
- **`mateclaw-server/.../agent/AgentGraphBuilder.java`** — 系统提示增加 ProgressLedger Discipline 段(agent-3)+ Environment Change Notifications 段(agent-1);SkillCatalog 渲染器扫描约束加 🔒 锚点(agent-4);wire ActionNode setter + ReasoningNode registry。
- **`mateclaw-server/.../agent/AgentService.java`** — `withLifecycleSync` / `withLifecycleFlux` 入口 `safeRegister`、出口 `safeUnregister`,覆盖 Flux 抛错路径。
- **`mateclaw-server/.../skill/manifest/SkillManifest.java`** — 增加 `constraints` 字段(List<String>)。
- **`mateclaw-server/.../skill/manifest/SkillManifestParser.java`** — `KNOWN_KEYS` 加 `"constraints"`;builder 链加 `.constraints(stringList(fm.get("constraints")))`。
- **`mateclaw-server/.../skill/service/SkillService.java`** — 4 个改动点发布 `SkillUpdatedEvent`(rescan / update builtin / update non-builtin / toggle enable-disable)。
- **`mateclaw-server/.../tool/builtin/ProgressLedgerTool.java`** — `@Tool` 描述声明 `auto_`/`pin_` 前缀保留;`@ToolParam stepKey` 同步警告。
- **`mateclaw-server/.../tool/mcp/runtime/PrefixedNameToolCallback.java`** — 新增 3 参构造器,serverName 非空时描述前缀 `[MCP server: <name>]`,让 LLM 区分跨服务器同名工具。
- **`mateclaw-server/.../tool/mcp/runtime/McpClientManager.java`** — `wrapServerCallbacks` 透传 serverName 到 PrefixedNameToolCallback。
- **`mateclaw-server/.../agent/context/ConversationWindowManager.java`** — `PRUNE_EXEMPT_TOOLS` 加入 `load_skill`(A1)。
- **`mateclaw-server/.../agent/graph/executor/ToolExecutionExecutor.java`** — 工具不存在时 `buildMcpAwareNotFoundMessage` 跨服务器搜索同 slug/hash 候选,给出 ≤5 个建议名。
**修改文件(测试 1 个)**
- **`mateclaw-server/.../agent/progress/ProgressLedgerStaleReminderTest.java`** — 回归适配:reminder 文本现在包含 `pending` 计数。
### 测试
- `mvn -pl mateclaw-server -am test -Dtest='SkillManifestConstraintsParsingTest,ProgressLedgerPrefixGuardTest,RunningConversationRegistryTest,ContextCompressionLedgerSurvivalTest,EnvironmentNotificationRenderingTest,ProgressLedgerStaleReminderTest'`:70/70 通过
- `mvn -pl mateclaw-server -am test`(全量回归,含上面 6 个 + 12 个深挖影响类):0 失败 0 错误
### 安全性
- **前缀保留**:`ProgressLedgerService.upsert` 拒绝 `auto_`/`pin_` 前缀,LLM 无法覆盖 Java 管理的条目;`@Tool` 描述显式声明保留前缀。
- **事件路由异常隔离**:`EnvironmentEventRouter.broadcast` 全 try/catch,路由失败永不冒泡到 Spring 事件总线。
- **并发安全**:registry 用 `ConcurrentHashMap` + `ConcurrentLinkedQueue`;ledger upsert 用 per-conversation `ReentrantLock`;批量 auto-record 在单次 lock 内完成;`ProgressLedgerPrefixGuardTest.concurrentUpsertAndAutoRecordAreSafe` 锁定。
- **内存有界**:通知队列每会话上限 10(LRU 驱逐最老);auto-recorded 条目每会话上限 5(驱逐最老);registry 后台 TTL 清理(30 分钟未活跃的 handle 自动回收)。
- **绑定机制不受影响**:MCP/skill 的 agent 绑定(`mate_agent_tool` / `mate_agent_skill` 表)完全未被触碰;C3 通知广播是有意全量(非按 agentId 过滤),最坏情况是无关 agent 多收一条 SystemMessage(LLM 被告知"如无关可忽略")。
## 逐项验证
### 改动 1:SkillManifestParser 真正解析 constraints(深挖修复)
**文件**:`mateclaw-server/src/main/java/vip/mate/skill/manifest/SkillManifestParser.java:33-47,108`
| 项 | 内容 |
|---|---|
| 改了什么 | `KNOWN_KEYS` 集合加入 `"constraints"`;builder 链加入 `.constraints(stringList(fm.get("constraints")))`。 |
| 为什么 | 之前 `KNOWN_KEYS` 没列入 `"constraints"`,导致该键被静默路由到 `extras`,`manifest.getConstraints()` 永远返回空 list,下游 B2 pinSkillConstraints 和 agent-4 catalog 锚点全是死代码。 |
| 验证步骤 | 1. `cat test-fixtures/skill-with-constraints/SKILL.md`(如有)确认 frontmatter 有 `constraints: [...]`;2. 运行 `SkillManifestConstraintsParsingTest`。 |
| 预期结果 | `manifest.getConstraints()` 返回非空 list;test 5/5 通过。 |
### 改动 2:ProgressLedgerService 前缀守卫(深挖修复)
**文件**:`mateclaw-server/src/main/java/vip/mate/agent/progress/ProgressLedgerService.java:132-137`
| 项 | 内容 |
|---|---|
| 改了什么 | `upsert()` 入口检查 key 是否以 `auto_` 或 `pin_` 开头,是则抛 `IllegalArgumentException`。 |
| 为什么 | 之前保护是单向的:Java 让位 LLM(auto 不覆盖 LLM 已有 entry),但 LLM 可以用 `progress_update(stepKey="auto_read_file")` 覆盖 Java 写入的条目,导致 auto-recorded 工具记录被改写。 |
| 验证步骤 | 1. `ProgressLedgerPrefixGuardTest.upsertRejectsAutoPrefix`;2. `ProgressLedgerPrefixGuardTest.upsertRejectsPinPrefix`。 |
| 预期结果 | 两个测试均抛 `IllegalArgumentException`;`ProgressLedgerTool` 的 `@Tool` 描述包含前缀保留声明。 |
### 改动 3:upsertAutoRecorded 4 参签名 + 批量化(深挖修复 + 性能优化)
**文件**:`mateclaw-server/src/main/java/vip/mate/agent/progress/ProgressLedgerService.java:227-296`、`mateclaw-server/src/main/java/vip/mate/agent/graph/node/ActionNode.java`(`autoRecordToolCalls`)
| 项 | 内容 |
|---|---|
| 改了什么 | `upsertAutoRecorded` 升级为 4 参签名 `(conversationId, toolName, displayName, resultSummary)`;新增 `upsertAutoRecordedBatch` 批量方法,一次 lock+load+save 处理 N 个工具响应;ActionNode 改为先收集 `List<AutoRecordEntry>` 再一次批量调用。 |
| 为什么 | 1. 跨服务器键碰撞:两个 MCP 服务器都暴露 `search` 工具 → `auto_search` 互相覆盖;2. 并行工具串行化:每个 ToolResponse 单独 lock+load+save 抵消并行收益。 |
| 验证步骤 | 1. `ProgressLedgerPrefixGuardTest.autoRecordedDifferentServersNoCollision`:两个服务器同名工具共存;2. `ProgressLedgerPrefixGuardTest.batchInsertProducesSameResultAsSequential`:批量与逐条结果一致;3. `ProgressLedgerPrefixGuardTest.batchInsertBoundedToMaxFiveEvenWithLargeBatch`:10 条批量插入后有界=5。 |
| 预期结果 | ledger 中同时存在 `auto_mcp_4_search_xxx` 和 `auto_mcp_7_search_yyy`;5 个并行工具调用从 5 次 lock+save 降为 1 次。 |
### 改动 4:B2 pinSkillConstraints——load_skill 后约束写入 pinned
**文件**:`mateclaw-server/src/main/java/vip/mate/agent/graph/node/ActionNode.java`(`pinSkillConstraints`)
| 项 | 内容 |
|---|---|
| 改了什么 | `load_skill` 工具调用成功后,读取 `manifest.getConstraints()`,对每条约束调用 `progressLedgerService.upsertPinned(convId, "pin_<skillName>_<i>", constraintText, note)`。 |
| 为什么 | 把约束从 messages(会被压缩销毁)抽到 DB ledger.pinned(压缩免疫),解决"长对话中 skill 约束丢失"问题。 |
| 验证步骤 | 1. `ContextCompressionLedgerSurvivalTest.loadSkillBodyDestroyedByCompressionButConstraintsSurviveInLedger`;2. `ContextCompressionLedgerSurvivalTest.allThreeLedgerEntryTypesSurviveCompression`。 |
| 预期结果 | 压缩后 messages 中 load_skill 正文消失,但 `ledger.renderSnapshot()` 仍包含 `🔒 固定约束` 段,约束文本字节级保留。 |
### 改动 5:B5 autoRecordToolCalls——工具调用后批量自动记录
**文件**:`mateclaw-server/src/main/java/vip/mate/agent/graph/node/ActionNode.java`(`autoRecordToolCalls`)
| 项 | 内容 |
|---|---|
| 改了什么 | ActionNode 处理 ToolResponse 后,收集所有有效条目到 `List<AutoRecordEntry>`,一次调用 `upsertAutoRecordedBatch`。 |
| 为什么 | 让 LLM 在长对话中即使忘记自己刚调用过什么工具,也能从 ledger 快照看到最近 5 次工具调用记录;批量调用避免 N 次 lock+save 串行化。 |
| 验证步骤 | `ProgressLedgerPrefixGuardTest.autoRecordedBoundedToMaxFive`:模拟 10 次工具调用,验证 auto 条目数等于 5。 |
| 预期结果 | ledger 中 auto 条目始终 ≤ 5,最老的被驱逐;5 个并行工具调用只需 1 次 DB roundtrip。 |
### 改动 6:C4 环境通知注入 nonHistoryPrefix
**文件**:`mateclaw-server/src/main/java/vip/mate/agent/graph/node/ReasoningNode.java:755-765,1201-1215`
| 项 | 内容 |
|---|---|
| 改了什么 | ReasoningNode 每轮推理前 `registry.drain(conversationId)`,非空则 `renderEnvironmentNotifications` 渲染成 markdown 块,作为 SystemMessage 加入 nonHistoryPrefix。 |
| 为什么 | 让运行时环境变更(MCP 断连 / skill 更新)在下一轮推理立即可见,LLM 主动改路而不是反复重试失效工具。 |
| 验证步骤 | `EnvironmentNotificationRenderingTest.mcpConnectionLostEventEndToEnd_producesActionableLLMText`:注册会话 → 触发 `McpConnectionLostEvent(serverId=7)` → drain → render。 |
| 预期结果 | 渲染块包含 "📢 环境变更通知"、`mcp_7_` 前缀、"不要反复重试" 指令。 |
### 改动 7:A1 PRUNE_EXEMPT_TOOLS 加入 load_skill
**文件**:`mateclaw-server/src/main/java/vip/mate/agent/context/ConversationWindowManager.java:96-120`
| 项 | 内容 |
|---|---|
| 改了什么 | `PRUNE_EXEMPT_TOOLS` 集合从 `{delegateToAgent, delegateParallel}` 扩展为 `{delegateToAgent, delegateParallel, load_skill}`。 |
| 为什么 | `load_skill` 返回的 SKILL.md 是 load-time 快照,skill 作者可能在执行期间更新,重载不保证恢复相同指令;且 50KB+ skill 重载昂贵。 |
| 验证步骤 | `ConversationWindowManagerToolPruningTest`(已有测试套件)。 |
| 预期结果 | load_skill 的 ToolResponseMessage 在 `pruneOldToolResultsForModelInput` / `compactAgedToolResponses` 阶段不被修剪。 |
### 改动 8:agent-2 PrefixedNameToolCallback 描述加 serverName 标签
**文件**:`mateclaw-server/src/main/java/vip/mate/tool/mcp/runtime/PrefixedNameToolCallback.java:55-80`、`mateclaw-server/src/main/java/vip/mate/tool/mcp/runtime/McpClientManager.java:201-300`
| 项 | 内容 |
|---|---|
| 改了什么 | 新增 3 参构造器 `(prefixedName, delegate, serverName)`,serverName 非空时描述前缀 `[MCP server: <name>]`;McpClientManager `wrapServerCallbacks` 透传 serverName。 |
| 为什么 | LLM 看到 `mcp_4_search_a1b2c3` 时无法知道这是哪个服务器的工具;多个 MCP 服务器都暴露 `search` 时,LLM 会混淆。加 `[MCP server: fetch-server]` 标签让 LLM 区分。 |
| 验证步骤 | 启动一个 MCP 服务器,在 agent 工具列表中观察工具描述是否包含 `[MCP server: <name>]` 前缀。 |
| 预期结果 | 每个 MCP 工具描述开头包含 `[MCP server: <serverName>]`;2 参构造器(无 serverName)保持向后兼容,描述不加前缀。 |
### 改动 9:ToolExecutionExecutor 工具不存在时跨服务器候选建议
**文件**:`mateclaw-server/src/main/java/vip/mate/agent/graph/executor/ToolExecutionExecutor.java:1264-1340`
| 项 | 内容 |
|---|---|
| 改了什么 | "Tool not found" 错误信息升级:若请求名是 MCP 格式,搜索 `toolCallbackMap` 中 slug 或 hash6 匹配但 serverId 不同的候选,返回 ≤5 个建议。 |
| 为什么 | LLM 常把 slug 拼到错误 serverId 上重构出不存在工具名,反复重试到 max iterations。给候选建议后 LLM 可以直接复制正确名字。 |
| 验证步骤 | 1. 启动两个 MCP 服务器都暴露 `fetch` 工具;2. 让 LLM 调用 `mcp_<serverA>_fetch_xxx`(实际 fetch 在 serverB);3. 观察错误信息。 |
| 预期结果 | 错误信息包含 "Did you mean one of these?" + 正确的 `mcp_<serverB>_fetch_yyy` 候选名。 |
### 改动 10:Registry TTL 定时清理(防泄漏)
**文件**:`mateclaw-server/src/main/java/vip/mate/agent/runtime/RunningConversationRegistry.java:155-211`
| 项 | 内容 |
|---|---|
| 改了什么 | 新增 `cleanupStale(Duration maxAge)` 方法 + `@Scheduled scheduledCleanup()`(每 5 分钟扫一次,清理 30 分钟未活跃的 handle)。用 `remove(key, value)` 保证不误删被并发 `register` 刷新的 handle。 |
| 为什么 | 兜底防御异常路径泄漏的 handle——即使 `safeUnregister` 因异常路径未执行(如 Reactor cancel 信号不触发 doFinally),后台线程也能回收。 |
| 验证步骤 | `RunningConversationRegistryTest.cleanupStaleRemovesOldHandles`:注册 → 反射 backdate lastActiveAt → 清理 → 验证被移除;`cleanupStaleDoesNotRemoveRefreshedHandle`:backdate 后 re-register → 清理 → 验证存活。 |
| 预期结果 | 30 分钟未活跃的 handle 被清理;被 `register` 刷新的 handle 不被误删。 |
### 改动 11:JSON 格式向后兼容迁移
**文件**:`mateclaw-server/src/main/java/vip/mate/agent/progress/ProgressLedgerService.java:79-85,284-292`
| 项 | 内容 |
|---|---|
| 改了什么 | JSON 从 flat-map `{"step1":{...}}` 升级为 wrapper `{"entries":{...},"pinned":{...}}`;`parseWrapper` 通过 peek `"entries"` 键区分新旧格式,旧格式自动迁移为 wrapper(pinned 为空)。 |
| 为什么 | 老 conversation 的 ledger 列存的是 flat-map,新代码上线后必须能加载老数据。 |
| 验证步骤 | `ContextCompressionLedgerSurvivalTest.oldFlatMapLedgerMigratesToWrapperFormatWithEmptyPinned`:写入旧 JSON → load → 验证 pinned 为空 → upsert → 验证新 JSON 包含 `entries` 和 `pinned` 键。 |
| 预期结果 | 旧 conversation 无需迁移脚本,第一次 load 即兼容;写入时自动转为新格式。 |
## 新增测试验证
**文件**:
- `mateclaw-server/src/test/java/vip/mate/skill/manifest/SkillManifestConstraintsParsingTest.java`
- `mateclaw-server/src/test/java/vip/mate/agent/progress/ProgressLedgerPrefixGuardTest.java`
- `mateclaw-server/src/test/java/vip/mate/agent/runtime/RunningConversationRegistryTest.java`
- `mateclaw-server/src/test/java/vip/mate/agent/context/ContextCompressionLedgerSurvivalTest.java`
- `mateclaw-server/src/test/java/vip/mate/agent/graph/node/EnvironmentNotificationRenderingTest.java`
| 命令 | 预期 |
|------|------|
| `mvn -pl mateclaw-server -am test -Dtest='SkillManifestConstraintsParsingTest'` | Tests run: 5, Failures: 0 |
| `mvn -pl mateclaw-server -am test -Dtest='ProgressLedgerPrefixGuardTest'` | Tests run: 22, Failures: 0 |
| `mvn -pl mateclaw-server -am test -Dtest='RunningConversationRegistryTest'` | Tests run: 24, Failures: 0 |
| `mvn -pl mateclaw-server -am test -Dtest='ContextCompressionLedgerSurvivalTest'` | Tests run: 5, Failures: 0 |
| `mvn -pl mateclaw-server -am test -Dtest='EnvironmentNotificationRenderingTest'` | Tests run: 14, Failures: 0 |
## 回归检查清单
- [ ] 全量 `mvn -pl mateclaw-server -am test` 通过(已验证 0 失败 0 错误)
- [ ] 老 conversation(flat-map ledger JSON)首次 load 不报错,pinned 字段为空
- [ ] 多 MCP 服务器场景:LLM 工具列表中每个工具描述包含 `[MCP server: <name>]` 标签
- [ ] MCP 服务器中途断连:agent 下一轮推理看到 `📢 环境变更通知` 块,主动改路
- [ ] 长 conversation(>100 轮)经多次 PTL 压缩后,`ledger.renderSnapshot()` 仍包含 pinned 约束
- [ ] `progress_update(stepKey="auto_xxx")` 被拒绝,返回 `IllegalArgumentException` 错误信息
- [ ] auto-recorded 条目数始终 ≤ 5(10 次工具调用后仍为 5)
- [ ] 5 个并行工具调用只产生 1 次 DB roundtrip(批量 auto-record)
- [ ] Registry 中 30 分钟未活跃的 handle 被后台定时清理
- [ ] MCP/skill 绑定机制(`mate_agent_tool` / `mate_agent_skill` 表)不受影响
- [ ] Plan-Execute 路径(StepExecutionNode)目前**不**接收环境通知——只有 ReAct 路径生效(已知未覆盖项,不阻塞本 PR)
* feat(agent): 六招减法重构——修复压缩销毁 skill 约束与 MCP 按需暴露
## 背景
- 压缩三阶段(softTrim / hardClear / prePruneForSummary)只检查 `isSpillMarker`,不检查 `PRUNE_EXEMPT_TOOLS`,导致 `load_skill` 返回的 SKILL.md 约束、`delegateToAgent` 子智能体转录在压缩中被裁掉,模型在长对话中"忘记"任务规则,根因是"压缩导致注意力失效"。
- skillCatalog 表只列 Skill / Status / Description 三列,bound skill 的 constraints 与 allowedTools 没有任何可见入口,模型加载 skill 后约束仍可能被忽略。
- MCP 工具默认 CORE tier,20+ MCP 工具的 schema 涌入核心列表,挤占 builtin 工具的注意力,且 `DisclosureTier.fromToken(null)` 返回 CORE 导致 `getOrDefault` 的默认值永远不生效。
- 构建期工具过滤分 4 次 pass,重复遍历且无明确 deny/allow 边界。
- skillCatalog 在每次推理步都按当前 loadedSkills 动态渲染,破坏 Anthropic SYSTEM_AND_TOOLS cache 前缀稳定性。
- 进度账本(ProgressLedger)约束条目前缀无保护,跨 MCP server 键碰撞,环境事件无统一路由入口。
## 改动内容
### 文件改动
**主代码(21 个文件)**
- **`mateclaw-server/src/main/java/vip/mate/agent/context/ConversationWindowManager.java`** — Move 4:三阶段新增 `isExemptTool` 检查跳过 `load_skill`/`delegateToAgent`/`delegateParallel`;新增 Phase 2.7 无损 spill evict 在调用 LLM 摘要前把超大工具结果落盘
- **`mateclaw-server/src/main/java/vip/mate/skill/runtime/SkillRuntimeService.java`** — Move 2 & 3:catalog 表新增 Constraints 列(仅 bound skill 显示);新增 `### Bound skill allowed tools` 块
- **`mateclaw-server/src/main/java/vip/mate/agent/graph/node/ReasoningNode.java`** — Move 1:skillCatalog 用 `render(Set.of())` 静态渲染进 nonHistoryPrefix;loadedThisRun hint 作为 volatile 后缀注入
- **`mateclaw-server/src/main/java/vip/mate/tool/disclosure/DefaultToolDisclosureService.java`** — Move 5:MCP 工具默认 tier 从 CORE 改 EXTENSION;`buildSnapshot` 跳过 null/blank tier 修复 `fromToken(null)→CORE` 陷阱
- **`mateclaw-server/src/main/java/vip/mate/agent/AgentGraphBuilder.java`** — Move 6:构建期权限过滤从 4 次 pass 合并为 2 次(deny 集 + allow 集)
- **`mateclaw-server/src/main/java/vip/mate/agent/AgentService.java`** — 接入 EnvironmentEventRouter 与 RunningConversationRegistry
- **`mateclaw-server/src/main/java/vip/mate/agent/graph/executor/ToolExecutionExecutor.java`** — 工具调用后自动回填 ProgressLedger
- **`mateclaw-server/src/main/java/vip/mate/agent/graph/node/ActionNode.java`** — 渲染 ledger 三段式快照
- **`mateclaw-server/src/main/java/vip/mate/agent/progress/ProgressLedger.java`** — constraints 前缀保护,跨 MCP server 键命名空间隔离
- **`mateclaw-server/src/main/java/vip/mate/agent/progress/ProgressLedgerService.java`** — 写入 constraints 到 pinned 条目
- **`mateclaw-server/src/main/java/vip/mate/skill/manifest/SkillManifest.java`** — 新增 constraints 字段
- **`mateclaw-server/src/main/java/vip/mate/skill/manifest/SkillManifestParser.java`** — 解析 SKILL.md frontmatter 中的 constraints
- **`mateclaw-server/src/main/java/vip/mate/skill/service/SkillService.java`** — skill 更新事件发布
- **`mateclaw-server/src/main/java/vip/mate/tool/builtin/ProgressLedgerTool.java`** — 三段式渲染
- **`mateclaw-server/src/main/java/vip/mate/tool/mcp/runtime/McpClientManager.java`** — MCP 命名透明化
- **`mateclaw-server/src/main/java/vip/mate/tool/mcp/runtime/PrefixedNameToolCallback.java`** — 透明命名映射
- **`mateclaw-server/src/main/java/vip/mate/agent/runtime/EnvironmentEventRouter.java`** — 新增:5 个环境事件监听器
- **`mateclaw-server/src/main/java/vip/mate/agent/runtime/EnvironmentNotification.java`** — 新增:环境通知数据模型
- **`mateclaw-server/src/main/java/vip/mate/agent/runtime/RunningConversationRegistry.java`** — 新增:运行中会话注册表 + TTL 清理
- **`mateclaw-server/src/main/java/vip/mate/skill/event/SkillUpdatedEvent.java`** — 新增:skill 更新事件
- **`mateclaw-server/Dockerfile`** — 构建配置微调
**测试代码(11 个文件)**
- **`ConversationWindowManagerExemptAndSpillTest.java`** — 新增 15 个行为测试,证明 Move 4 生效
- **`SkillRuntimeServiceConstraintsAndToolsTest.java`** — 新增 8 个测试覆盖 Constraints 列与 allowedTools 块
- **`ReasoningNodeLoadedSkillsHintTest.java`** — 新增 8 个测试覆盖 loadedThisRun hint 渲染
- **`CompactionSurvivalComparisonTest.java`** — 新增 4 个场景的新旧代码对比测试(100 轮极限压缩)
- **`ContextCompressionLedgerSurvivalTest.java`** — 压缩后 ledger 存活测试
- **`EnvironmentNotificationRenderingTest.java`** — 环境通知渲染测试
- **`ProgressLedgerPrefixGuardTest.java`** — ledger 前缀保护测试
- **`RunningConversationRegistryTest.java`** — 会话注册表测试
- **`SkillManifestConstraintsParsingTest.java`** — constraints 解析测试
- **`ProgressLedgerStaleReminderTest.java`** — 修复回归
- **`ToolDisclosureServiceTest.java`** — 断言从 CORE 改为 EXTENSION
### 测试
**回归测试**
- `mvn test`(mateclaw-server 全量):**199 通过 / 1 跳过 / 0 失败**
**行为测试(证明改动生效,旧代码上失败)**
- `CompactionSurvivalComparisonTest`(4 个场景):在新代码上全部通过
- 用 `git stash` 还原旧代码后,16 个行为测试编译失败或断言失败 → 证明测试确实覆盖了新行为
**新旧代码对比测试(同一份测试源码,两套代码库运行)**
| 场景 | 旧代码 | 新代码 |
|---|---|---|
| A: 50 load_skill + 50 delegate + 50 read_file 单轮压缩 | load_skill 0/50, delegate 0/50 | **load_skill 50/50, delegate 50/50** |
| B: 100 轮极限压缩 + 头部 pinned load_skill | root_constraint_survived=**false**, 113ms | root_constraint_survived=**true**, 60ms |
| C: 20 个不同大小 load_skill 单轮压缩 | 0/20 存活, tokens 8694→754 | **20/20 存活**, tokens 8694→8694 |
| D: 30 轮稳态压缩 + pinned skill | pinned_survived=**false** | pinned_survived=**true** |
### 安全性
- `DisclosureTier.fromToken(null)` 陷阱修复:旧代码 `serverTierById.put(id, CORE)` 导致 `getOrDefault` 默认值永不生效;新代码跳过 null tier,未配置的 MCP server 才走 EXTENSION 默认值
- `PRUNE_EXEMPT_TOOLS` 保护范围从 2 处扩展到 5 处,避免 `load_skill` 约束被压缩销毁后模型在无约束下执行敏感操作
## 逐项验证
### 改动 1:nonHistoryPrefix 分层稳定化
**文件**:`mateclaw-server/src/main/java/vip/mate/agent/graph/node/ReasoningNode.java:701-714, 776-788, 1246-1257`
| 项 | 内容 |
|---|---|
| 改了什么 | skillCatalog 用 `render(Set.of())` 静态渲染进 nonHistoryPrefix;loadedThisRun hint 作为 volatile 后缀注入 |
| 为什么 | 每次推理步都按 loadedSkills 动态渲染会破坏 Anthropic SYSTEM_AND_TOOLS cache 前缀,导致 cache 失效增加 token 成本 |
| 验证步骤 | 1. 打开 ReasoningNode.java:701;2. 确认 `skillCatalogRenderer.render(java.util.Set.of())` 调用;3. 跑 `ReasoningNodeLoadedSkillsHintTest` |
| 预期结果 | 8 个测试通过,loadedThisRun hint 作为后缀注入,不破坏前缀缓存 |
### 改动 2:skillCatalog 增加 Constraints 列
**文件**:`mateclaw-server/src/main/java/vip/mate/skill/runtime/SkillRuntimeService.java:502-561, 634-641`
| 项 | 内容 |
|---|---|
| 改了什么 | catalog 表从 3 列扩为 4 列,新增 Constraints 列(仅 bound skill 显示,长约束截断,pipe 转义);新增 `### Bound skill allowed tools` 块 |
| 为什么 | bound skill 的 constraints 没有任何可见入口,模型加载后仍可能忽略 |
| 验证步骤 | 1. 跑 `SkillRuntimeServiceConstraintsAndToolsTest`;2. 检查 catalog 渲染包含 Constraints 列 |
| 预期结果 | 8 个测试通过,bound skill 显示 constraints,非 bound skill 省略 |
- **边界验证**:长约束截断为单行;pipe 字符被转义不破坏表格
### 改动 3:修复 PRUNE_EXEMPT_TOOLS 在三阶段的绕过
**文件**:`mateclaw-server/src/main/java/vip/mate/agent/context/ConversationWindowManager.java:990-994, 1030-1090, 1160-1210`
| 项 | 内容 |
|---|---|
| 改了什么 | `softTrimToolResults`、`hardClearToolResults`、`prePruneForSummary` 三处新增 `isExemptTool` 检查,跳过 `load_skill`/`delegateToAgent`/`delegateParallel` |
| 为什么 | 旧代码只在 `pruneOldToolResultsForModelInput` 和 `compactAgedToolResponses` 检查 exempt,三阶段不检查,导致 skill 约束在压缩中被裁掉 |
| 验证步骤 | 1. 跑 `ConversationWindowManagerExemptAndSpillTest`;2. 跑 `CompactionSurvivalComparisonTest` |
| 预期结果 | 15 个行为测试通过;100 轮压缩后 load_skill body 存活率 100% |
- **反例对照**:在新代码上跑对比测试,旧代码存活率 0%,新代码 100%
### 改动 4:Phase 2.7 无损 spill evict
**文件**:`mateclaw-server/src/main/java/vip/mate/agent/context/ConversationWindowManager.java:440-468, 1263-1300`
| 项 | 内容 |
|---|---|
| 改了什么 | 在 Phase 2 hardClear 之后、LLM 摘要之前新增 Phase 2.7,把超大工具结果落盘替换为 spill marker |
| 为什么 | 旧代码超出预算直接走 LLM 摘要(有损+耗时+费 token),其实大部分场景落盘就够 |
| 验证步骤 | 1. 检查 `spillEvictToolResults` 方法;2. 跑 `ConversationWindowManagerExemptAndSpillTest.spillEvictReducesTokenCount` |
| 预期结果 | spill 后 token 数低于预算时跳过 LLM 摘要,strategy=lossless_spill_evict |
### 改动 5:MCP 工具默认 EXTENSION
**文件**:`mateclaw-server/src/main/java/vip/mate/tool/disclosure/DefaultToolDisclosureService.java:78-105, 276-336`
| 项 | 内容 |
|---|---|
| 改了什么 | `resolveTierByName` 默认返回 EXTENSION;`buildSnapshot` 跳过 null/blank tier 的 server 不放入 map |
| 为什么 | MCP schema 是 prompt 最重部分,默认 CORE 挤占 builtin 工具注意力;`fromToken(null)` 返回 CORE 导致默认值失效 |
| 验证步骤 | 1. 跑 `ToolDisclosureServiceTest.mcpDefaultsExtensionWhenServerTierUnset`;2. 检查未配置 tier 的 MCP server 工具不在 active 列表 |
| 预期结果 | 未配置 tier 的 MCP 工具进入 extensionCatalog,需 `enable_tool` 激活 |
- **边界验证**:显式 `disclosure_tier=core` 的 server 仍保持 CORE
### 改动 6:构建期权限过滤合并
**文件**:`mateclaw-server/src/main/java/vip/mate/agent/AgentGraphBuilder.java:258-310`
| 项 | 内容 |
|---|---|
| 改了什么 | 4 次 pass 合并为 2 次:先 `withDeniedToolsFiltered(deniedSet)`,再 `withAllowedToolsOnly(boundTools)` |
| 为什么 | 重复遍历浪费构建时间,且 deny/allow 边界不清晰 |
| 验证步骤 | 1. 检查 AgentGraphBuilder.java:258-310;2. 跑全量回归测试确认工具过滤行为不变 |
| 预期结果 | 工具列表与改动前一致,构建步骤减少 |
## 新增测试验证
**文件**:
- `mateclaw-server/src/test/java/vip/mate/agent/context/ConversationWindowManagerExemptAndSpillTest.java`
- `mateclaw-server/src/test/java/vip/mate/agent/context/CompactionSurvivalComparisonTest.java`
- `mateclaw-server/src/test/java/vip/mate/skill/runtime/SkillRuntimeServiceConstraintsAndToolsTest.java`
- `mateclaw-server/src/test/java/vip/mate/agent/graph/node/ReasoningNodeLoadedSkillsHintTest.java`
- `mateclaw-server/src/test/java/vip/mate/skill/manifest/SkillManifestConstraintsParsingTest.java`
- `mateclaw-server/src/test/java/vip/mate/agent/progress/ProgressLedgerPrefixGuardTest.java`
- `mateclaw-server/src/test/java/vip/mate/agent/runtime/RunningConversationRegistryTest.java`
- `mateclaw-server/src/test/java/vip/mate/agent/graph/node/EnvironmentNotificationRenderingTest.java`
- `mateclaw-server/src/test/java/vip/mate/agent/context/ContextCompressionLedgerSurvivalTest.java`
| 命令 | 预期 |
|------|------|
| `mvn -Dtest=ConversationWindowManagerExemptAndSpillTest test` | 15 个测试通过 |
| `mvn -Dtest=CompactionSurvivalComparisonTest test` | 4 个场景通过,新代码 load_skill 存活率 100% |
| `mvn -Dtest=SkillRuntimeServiceConstraintsAndToolsTest test` | 8 个测试通过 |
| `mvn -Dtest=ReasoningNodeLoadedSkillsHintTest test` | 8 个测试通过 |
| `mvn test`(全量) | 199 通过 / 1 跳过 / 0 失败 |
**新旧对比测试运行命令**:
```bash
# 新代码
cd /data/mateclaw/mateclaw-server && mvn -Dtest=CompactionSurvivalComparisonTest -Dsurefire.useFile=false test
# 旧代码(需把测试复制到 mateclaw-old)
cd /data/mateclaw/mateclaw-old/mateclaw-server && mvn -Dtest=CompactionSurvivalComparisonTest -Dsurefire.useFile=false test
```
## 回归检查清单
- [ ] `mvn test` 全量通过(199/1skip/0fail)
- [ ] 对比测试在新代码上 load_skill 存活率 100%
- [ ] 对比测试在旧代码上 load_skill 存活率 0%(证明测试有效)
- [ ] MCP 工具默认进入 extensionCatalog,`enable_tool` 可激活
- [ ] 显式 `disclosure_tier=core` 的 MCP server 仍保持 CORE
- [ ] 100 轮压缩后 root_constraint 仍存活
- [ ] Phase 2.7 spill evict 在预算内时跳过 LLM 摘要
- [ ] skillCatalog 静态渲染不依赖 loadedSkills,前缀缓存稳定
This commit is contained in:
parent
e236800002
commit
e4dd08b5f4
@ -140,6 +140,8 @@ public class AgentGraphBuilder {
|
||||
private final vip.mate.goal.service.GoalEvaluationService goalEvaluationService;
|
||||
private final vip.mate.goal.service.GoalFollowupService goalFollowupService;
|
||||
private final vip.mate.goal.config.GoalProperties goalProperties;
|
||||
/** C4: per-conversation environment notification registry, injected into ReasoningNode. */
|
||||
private final vip.mate.agent.runtime.RunningConversationRegistry runningConversationRegistry;
|
||||
|
||||
/**
|
||||
* Auto-grant resolver wired into the executor so an active
|
||||
@ -256,8 +258,30 @@ public class AgentGraphBuilder {
|
||||
public BaseAgent build(AgentEntity entity, String modelProvider, String modelName) {
|
||||
AgentToolSet toolSet = toolRegistry.getEnabledToolSet();
|
||||
|
||||
// 过滤掉 denied 工具,使模型完全看不到它们(防止 prompt injection 利用 schema)
|
||||
toolSet = toolSet.withDeniedToolsFiltered(toolGuardConfigService.getDeniedTools());
|
||||
// Move 6 — Permission flattening at build time.
|
||||
//
|
||||
// Two layers of tool filtering exist in MateClaw:
|
||||
// (1) Build-time filter (HERE) — decides which tools the model SEES
|
||||
// in the tool list. Computed once per agent build; stable for
|
||||
// the agent's lifecycle unless bindings change.
|
||||
// (2) Runtime guard (ToolGuardService.evaluate) — decides which
|
||||
// tools the model can CALL. Runs on every invocation; checks
|
||||
// workspace boundaries, sensitive paths, credential exposure,
|
||||
// shell command patterns, and approval workflows. All dynamic
|
||||
// (depends on tool arguments, not just tool name).
|
||||
//
|
||||
// The build-time filter previously ran as 4 separate passes
|
||||
// (deny → allow → deny → exclude). Move 6 consolidates them into
|
||||
// a single deny-set + a single allow-set, applied in two passes:
|
||||
// denied = global denied ∪ skill-discovery denied ∪ {load_skill if disabled}
|
||||
// allowed = agent's bound tools (null = global default)
|
||||
Set<String> deniedTools = new java.util.LinkedHashSet<>(
|
||||
toolGuardConfigService.getDeniedTools());
|
||||
deniedTools.addAll(agentBindingService.getSkillDiscoveryDeniedTools(entity.getId()));
|
||||
if (!loadSkillToolEnabled) {
|
||||
deniedTools.add("load_skill");
|
||||
}
|
||||
toolSet = toolSet.withDeniedToolsFiltered(deniedTools);
|
||||
|
||||
// RFC-090 §14.2 — single entry point that merges:
|
||||
// (a) tools expanded from bound skills' active features, and
|
||||
@ -267,24 +291,6 @@ public class AgentGraphBuilder {
|
||||
Set<String> boundTools = agentBindingService.getEffectiveToolNames(entity.getId());
|
||||
toolSet = toolSet.withAllowedToolsOnly(boundTools); // null = 全局默认
|
||||
|
||||
// Issue #184 follow-up: an agent that opted out of skills must not be
|
||||
// able to circle back and discover/load them via the meta tools. Strip
|
||||
// the skill-discovery surface (listAvailableSkills / load_skill /
|
||||
// readSkillFile / runSkillScript / listSkillFiles) here. This runs as a
|
||||
// separate deny layer so the allowlist matrix in getEffectiveToolNames
|
||||
// stays untouched — in particular, the (skillsDisabled, !toolsDisabled,
|
||||
// no tool bindings) cell still returns null so non-skill global tools
|
||||
// continue to flow through.
|
||||
toolSet = toolSet.withDeniedToolsFiltered(
|
||||
agentBindingService.getSkillDiscoveryDeniedTools(entity.getId()));
|
||||
|
||||
// Escape hatch: drop the load_skill meta tool entirely when disabled, so
|
||||
// it isn't advertised regardless of binding (the catalog guidance falls
|
||||
// back to readSkillFile — see SkillRuntimeService).
|
||||
if (!loadSkillToolEnabled) {
|
||||
toolSet = toolSet.excluding(java.util.Set.of("load_skill"));
|
||||
}
|
||||
|
||||
// Resolve the base model with the precedence: per-conversation pin >
|
||||
// per-Agent model override > global default. resolveRuntimeBaseModel
|
||||
// looks up enabled-only models and silently degrades an unmatched pin /
|
||||
@ -941,7 +947,16 @@ public class AgentGraphBuilder {
|
||||
skillCatalogRenderer, toolDisclosureService, progressLedgerService);
|
||||
reasoningNode.setPrefixBudgetPlan(prefixBudgetPlan);
|
||||
reasoningNode.setAutoDemotedTools(autoDemotedTools);
|
||||
// C4: wire the environment-notification registry so ReasoningNode
|
||||
// can drain pending MCP/skill events and inject them as a SystemMessage.
|
||||
reasoningNode.setRunningConversationRegistry(runningConversationRegistry);
|
||||
ActionNode actionNode = new ActionNode(executor, streamTracker);
|
||||
// B2/B5: wire optional collaborators so ActionNode can pin skill
|
||||
// constraints and auto-record tool completions into ProgressLedger.
|
||||
// Setter injection keeps the existing constructor signature stable
|
||||
// for tests that build ActionNode directly.
|
||||
actionNode.setSkillRuntimeService(skillRuntimeService);
|
||||
actionNode.setProgressLedgerService(progressLedgerService);
|
||||
ObservationProcessor observationProcessor = new ObservationProcessor(graphObservationProperties);
|
||||
ObservationNode observationNode = new ObservationNode(observationProcessor, streamTracker);
|
||||
SummarizingNode summarizingNode = new SummarizingNode(chatModel, streamingHelper, streamTracker);
|
||||
@ -1633,6 +1648,15 @@ public class AgentGraphBuilder {
|
||||
Only state you cannot access something if no relevant tool is available.
|
||||
Do not claim a tool-generated file, URL, UUID, path, task id, or success result before the corresponding tool call has completed. If a tool is needed, call the tool first, then report only the actual returned result.
|
||||
|
||||
## MCP Tool Naming
|
||||
Tools from MCP servers have names shaped like `mcp_<serverId>_<slug>_<hash6>`:
|
||||
- `<serverId>` is a numeric ID identifying which MCP server the tool belongs to.
|
||||
- Tools from DIFFERENT servers have DIFFERENT serverId prefixes, even if they have the same raw name (e.g. `search` on server A vs server B) — they are DIFFERENT tools and are NOT interchangeable.
|
||||
- Each MCP tool's description starts with `[MCP server: <name>]` so you can identify the source server by its human-readable name.
|
||||
- MCP tools are listed in the Extension Tools catalog by default. Use `enable_tool(toolName="<exact-name>")` to activate the one you need before calling it.
|
||||
- Always call tools by the EXACT name shown in the tool list. Do NOT reconstruct a tool name by swapping the slug into a serverId you remember from a previous successful call — that produces a non-existent tool name and the call will fail.
|
||||
- If a tool call returns "Tool not found" with candidate suggestions, pick the correct one from the candidates verbatim.
|
||||
|
||||
## Multi-Part Question Guidelines
|
||||
When the user asks multiple questions or requests multiple tasks in a single message:
|
||||
1. Structure your final answer with numbered sections, one per sub-task
|
||||
@ -1660,6 +1684,24 @@ public class AgentGraphBuilder {
|
||||
3. Process the extracted text content
|
||||
|
||||
If you try to read a PDF/Office file with read_file, you will get binary garbage or an error.
|
||||
|
||||
## ProgressLedger Discipline (mandatory)
|
||||
The `## 当前任务进度` block injected near the top of every turn is the **authoritative record** of what you have done and what remains. Treat it as ground truth, not as a scratchpad you may ignore.
|
||||
- **On starting any multi-step task** (≥3 tool calls expected), call `progress_update` in a parallel tool_calls batch to register every pending step BEFORE doing the work. Do not wait until "later" — context compression can trim earlier turns and you will lose track.
|
||||
- **After each completed sub-step**, immediately call `progress_update` to flip its status to `done`. "Immediately" means in the same tool_calls batch that returns the result, not after the next reasoning turn.
|
||||
- **Never re-execute a step the ledger shows as `done`** unless you can articulate why the prior result is stale.
|
||||
- **🔒 固定约束 entries** (pinned from skill manifests) are non-negotiable. They survive context compression for a reason — re-read them every turn and make sure your planned action still satisfies them.
|
||||
- **🔧 自动记录 entries** are auto-filled by Java after each tool call. They are a safety net, not a substitute for your own `progress_update` — if you only rely on them, you will lose the pending/blocked view that drives planning.
|
||||
- If you see a `⚠️ 进度账本已 N 秒未更新` reminder, **stop whatever you are doing and update the ledger first**. Continuing to call tools without updating the ledger is the #1 cause of duplicate work and missed steps.
|
||||
- The ledger is per-conversation and persists across context trims; treating it as ephemeral will cause you to repeat work after every compaction.
|
||||
|
||||
## Environment Change Notifications
|
||||
Occasionally you will see a `## 📢 环境变更通知` block injected near the top of a turn. It is generated by Java when an external event affects your runtime — an MCP server disconnecting, a skill being updated/removed, or a tool binding change.
|
||||
- These notifications are **authoritative** — Java detected the change; do not second-guess them by re-probing the tool.
|
||||
- If a notification says an MCP server is unavailable, **immediately stop calling tools prefixed with that server's id** and either switch to an alternative or report the gap to the user.
|
||||
- If a notification says a skill was updated, **re-load it via `load_skill`** to refresh its constraints in your pinned ledger; the old constraints you remember may no longer apply.
|
||||
- If a notification says a tool was removed, **do not attempt to call it**; pick a different approach or ask the user.
|
||||
- These notifications appear at most once per event; if you miss one, it will not be repeated, so act on it in the turn you see it.
|
||||
""".formatted(entity.getId());
|
||||
|
||||
// Web-search vs browser_use priority guidance — emitted unconditionally so the rule
|
||||
@ -1696,14 +1738,48 @@ public class AgentGraphBuilder {
|
||||
* bound skills, effective tool allowlist, model window and workspace once;
|
||||
* the returned renderer is invoked each turn with the skills loaded so far
|
||||
* this run so {@code load_skill} pins float to the top of the catalog.
|
||||
*
|
||||
* <p>agent-4: when any loaded skill declares structured {@code constraints}
|
||||
* in its manifest, the rendered catalog gets a trailing anchor note
|
||||
* {@code "🔒 = 含固定约束的 skill(详见 ProgressLedger)"} so the LLM has
|
||||
* a visible cue that some skills carry non-negotiable rules pinned into
|
||||
* the ledger. The cue is appended (not interleaved) to keep the
|
||||
* catalog's prompt-cache hash stable for the unchanged prefix.
|
||||
*/
|
||||
private SkillCatalogRenderer buildSkillCatalogRenderer(AgentEntity entity, Set<String> boundTools,
|
||||
Integer maxInputTokens) {
|
||||
Set<Long> boundSkillIds = agentBindingService.getBoundSkillIds(entity.getId());
|
||||
Long agentId = entity.getId();
|
||||
Long workspaceId = entity.getWorkspaceId();
|
||||
return loaded -> skillRuntimeService.buildSkillPromptEnhancement(
|
||||
boundSkillIds, boundTools, maxInputTokens, agentId, workspaceId, loaded);
|
||||
return loaded -> {
|
||||
String catalog = skillRuntimeService.buildSkillPromptEnhancement(
|
||||
boundSkillIds, boundTools, maxInputTokens, agentId, workspaceId, loaded);
|
||||
if (catalog == null || catalog.isBlank() || loaded == null || loaded.isEmpty()) {
|
||||
return catalog;
|
||||
}
|
||||
// Scan loaded skills for any with structured constraints. We only
|
||||
// surface the anchor when at least one matches, to avoid noisy
|
||||
// output on constraint-free skills.
|
||||
boolean anyHasConstraints = false;
|
||||
for (String skillName : loaded) {
|
||||
try {
|
||||
vip.mate.skill.runtime.model.ResolvedSkill skill = skillRuntimeService.findActiveSkill(skillName);
|
||||
if (skill != null && skill.getManifest() != null) {
|
||||
List<String> constraints = skill.getManifest().getConstraints();
|
||||
if (constraints != null && !constraints.isEmpty()) {
|
||||
anyHasConstraints = true;
|
||||
break;
|
||||
}
|
||||
}
|
||||
} catch (Exception ignored) {
|
||||
// best-effort lookup; do not fail the catalog render
|
||||
}
|
||||
}
|
||||
if (!anyHasConstraints) {
|
||||
return catalog;
|
||||
}
|
||||
return catalog + "\n\n🔒 = 含固定约束的 skill(已写入 ProgressLedger,详见 🔒 固定约束 段落,全程不可忽略)";
|
||||
};
|
||||
}
|
||||
|
||||
/**
|
||||
|
||||
@ -59,6 +59,15 @@ public class AgentService {
|
||||
@Autowired(required = false)
|
||||
private ApplicationEventPublisher events;
|
||||
|
||||
/**
|
||||
* C5: tracks in-flight conversations so {@link vip.mate.agent.runtime.EnvironmentEventRouter}
|
||||
* can push environment-change notifications into the agent's next reasoning
|
||||
* turn. Field-injected (optional) so existing test constructors of
|
||||
* {@code AgentService} don't need to supply it.
|
||||
*/
|
||||
@Autowired(required = false)
|
||||
private vip.mate.agent.runtime.RunningConversationRegistry runningConversationRegistry;
|
||||
|
||||
/**
|
||||
* Runtime Agent instance cache. Keyed first by agentId, then by a model
|
||||
* key, so a conversation that pins a non-default model gets its own graph
|
||||
@ -518,6 +527,36 @@ public class AgentService {
|
||||
log.info("Agent caches refreshed after MCP server change: {}", event.reason());
|
||||
}
|
||||
|
||||
/**
|
||||
* Listen for MCP connection-loss events and clear the agent cache.
|
||||
*
|
||||
* <p>Previously this listener was intentionally omitted (the design
|
||||
* doc said "only listen to McpServerChangedEvent, not
|
||||
* McpConnectionLostEvent") because {@link McpServerService} auto-heals
|
||||
* and publishes McpServerChangedEvent on reconnect. However, between
|
||||
* disconnect and reconnect, cached agents still hold the old
|
||||
* {@code AgentToolSet} snapshot whose MCP tool callbacks point at a
|
||||
* dead client — calls either time out (5 min default) or throw.
|
||||
*
|
||||
* <p>Clearing the cache on disconnect ensures the next agent build
|
||||
* sees the live connection state: {@link McpClientManager} will
|
||||
* either skip the dead server or fall back to {@code lastGoodCallbacks}
|
||||
* with proper error handling, rather than letting the LLM discover
|
||||
* the breakage by timing out.
|
||||
*
|
||||
* <p>Cost is low: {@code McpServerService} already debounces reconnect
|
||||
* attempts by 10s, and {@code refreshAllAgents} is a Map.clear().
|
||||
* The subsequent reconnect will fire another McpServerChangedEvent,
|
||||
* which clears the cache again — at most two clears per disconnect
|
||||
* cycle, which is acceptable.
|
||||
*/
|
||||
@EventListener
|
||||
public void onMcpConnectionLost(vip.mate.tool.mcp.event.McpConnectionLostEvent event) {
|
||||
refreshAllAgents();
|
||||
log.warn("Agent caches refreshed after MCP connection lost: serverId={}, reason={}",
|
||||
event.serverId(), event.reason());
|
||||
}
|
||||
|
||||
// ==================== Lifecycle helpers ====================
|
||||
|
||||
/**
|
||||
@ -529,17 +568,22 @@ public class AgentService {
|
||||
*/
|
||||
private String withLifecycleSync(Long agentId, String message, String conversationId,
|
||||
java.util.function.BiFunction<String, String, String> invoke) {
|
||||
if (!memoryProperties.isLifecycleMediatorEnabled()) {
|
||||
return invoke.apply(message, conversationId);
|
||||
safeRegister(conversationId, agentId);
|
||||
try {
|
||||
if (!memoryProperties.isLifecycleMediatorEnabled()) {
|
||||
return invoke.apply(message, conversationId);
|
||||
}
|
||||
String ownerKey = memoryOwnerResolver.resolve(ChatOriginHolder.get());
|
||||
TurnContext ctx = new TurnContext(agentId, conversationId, conversationId, 0, message, ownerKey);
|
||||
String memoryContext = lifecycleMediator.beforeLlmCall(ctx);
|
||||
// Inject memory context into the user message (RFC-037 §3.3)
|
||||
String enrichedMessage = injectMemoryContext(message, memoryContext);
|
||||
String result = invoke.apply(enrichedMessage, conversationId);
|
||||
lifecycleMediator.afterLlmCall(ctx, result != null ? result : "");
|
||||
return result;
|
||||
} finally {
|
||||
safeUnregister(conversationId);
|
||||
}
|
||||
String ownerKey = memoryOwnerResolver.resolve(ChatOriginHolder.get());
|
||||
TurnContext ctx = new TurnContext(agentId, conversationId, conversationId, 0, message, ownerKey);
|
||||
String memoryContext = lifecycleMediator.beforeLlmCall(ctx);
|
||||
// Inject memory context into the user message (RFC-037 §3.3)
|
||||
String enrichedMessage = injectMemoryContext(message, memoryContext);
|
||||
String result = invoke.apply(enrichedMessage, conversationId);
|
||||
lifecycleMediator.afterLlmCall(ctx, result != null ? result : "");
|
||||
return result;
|
||||
}
|
||||
|
||||
/**
|
||||
@ -552,23 +596,47 @@ public class AgentService {
|
||||
private <T> Flux<T> withLifecycleFlux(Long agentId, String message, String conversationId,
|
||||
java.util.function.BiFunction<String, String, Flux<T>> invoke,
|
||||
Function<T, String> contentExtractor) {
|
||||
if (!memoryProperties.isLifecycleMediatorEnabled()) {
|
||||
return invoke.apply(message, conversationId);
|
||||
safeRegister(conversationId, agentId);
|
||||
try {
|
||||
if (!memoryProperties.isLifecycleMediatorEnabled()) {
|
||||
return invoke.apply(message, conversationId)
|
||||
.doFinally(s -> safeUnregister(conversationId));
|
||||
}
|
||||
String ownerKey = memoryOwnerResolver.resolve(ChatOriginHolder.get());
|
||||
TurnContext ctx = new TurnContext(agentId, conversationId, conversationId, 0, message, ownerKey);
|
||||
String memoryContext = lifecycleMediator.beforeLlmCall(ctx);
|
||||
String enrichedMessage = injectMemoryContext(message, memoryContext);
|
||||
StringBuilder reply = new StringBuilder();
|
||||
return invoke.apply(enrichedMessage, conversationId)
|
||||
.doOnNext(item -> {
|
||||
String text = contentExtractor.apply(item);
|
||||
if (text != null) {
|
||||
reply.append(text);
|
||||
}
|
||||
})
|
||||
.doOnComplete(() -> lifecycleMediator.afterLlmCall(ctx, reply.toString()))
|
||||
.doOnError(e -> log.debug("[Memory] Stream error, skipping afterLlmCall: {}", e.getMessage()))
|
||||
.doFinally(s -> safeUnregister(conversationId));
|
||||
} catch (Exception e) {
|
||||
// If invoke.apply() throws before the Flux is constructed, the
|
||||
// doFinally above never runs — clean up here.
|
||||
safeUnregister(conversationId);
|
||||
throw e;
|
||||
}
|
||||
}
|
||||
|
||||
/** C5 helper — null-safe register so tests without the registry don't NPE. */
|
||||
private void safeRegister(String conversationId, Long agentId) {
|
||||
if (runningConversationRegistry != null) {
|
||||
runningConversationRegistry.register(conversationId, agentId);
|
||||
}
|
||||
}
|
||||
|
||||
/** C5 helper — null-safe unregister so tests without the registry don't NPE. */
|
||||
private void safeUnregister(String conversationId) {
|
||||
if (runningConversationRegistry != null) {
|
||||
runningConversationRegistry.unregister(conversationId);
|
||||
}
|
||||
String ownerKey = memoryOwnerResolver.resolve(ChatOriginHolder.get());
|
||||
TurnContext ctx = new TurnContext(agentId, conversationId, conversationId, 0, message, ownerKey);
|
||||
String memoryContext = lifecycleMediator.beforeLlmCall(ctx);
|
||||
String enrichedMessage = injectMemoryContext(message, memoryContext);
|
||||
StringBuilder reply = new StringBuilder();
|
||||
return invoke.apply(enrichedMessage, conversationId)
|
||||
.doOnNext(item -> {
|
||||
String text = contentExtractor.apply(item);
|
||||
if (text != null) {
|
||||
reply.append(text);
|
||||
}
|
||||
})
|
||||
.doOnComplete(() -> lifecycleMediator.afterLlmCall(ctx, reply.toString()))
|
||||
.doOnError(e -> log.debug("[Memory] Stream error, skipping afterLlmCall: {}", e.getMessage()));
|
||||
}
|
||||
|
||||
/**
|
||||
|
||||
@ -96,16 +96,31 @@ public class ConversationWindowManager {
|
||||
|
||||
/**
|
||||
* Tool names whose results must never be compacted into a one-line
|
||||
* summary. Sub-agent delegations are irreplaceable: the child runs an
|
||||
* summary.
|
||||
*
|
||||
* <p>Sub-agent delegations are irreplaceable: the child runs an
|
||||
* independent LLM session that the parent cannot reproduce, so dropping
|
||||
* earlier batches forces the parent to re-dispatch the same children to
|
||||
* recover what was lost. Every other tool (read_file, shell, search,
|
||||
* memory) can be re-invoked cheaply if the parent decides it needs
|
||||
* the data again.
|
||||
* recover what was lost.
|
||||
*
|
||||
* <p>{@code load_skill} returns the SKILL.md content at load time —
|
||||
* a snapshot of the skill's constraints, flow, and script entrypoints.
|
||||
* The skill author may update SKILL.md between the original load and a
|
||||
* hypothetical re-load, so re-invoking {@code load_skill} does NOT
|
||||
* guarantee recovering the same instructions the agent started with.
|
||||
* Dropping the original also forces the agent to either re-load (token
|
||||
* expensive for 50KB+ skills) or operate without constraints — the
|
||||
* root cause of the "skill constraint forgetting" bug. Pinning the
|
||||
* original ToolResponseMessage keeps the agent's understanding of the
|
||||
* task's rules stable across context-window trims.
|
||||
*
|
||||
* <p>Every other tool (read_file, shell, search, memory) can be
|
||||
* re-invoked cheaply if the parent decides it needs the data again.
|
||||
*/
|
||||
private static final java.util.Set<String> PRUNE_EXEMPT_TOOLS = java.util.Set.of(
|
||||
"delegateToAgent",
|
||||
"delegateParallel"
|
||||
"delegateParallel",
|
||||
"load_skill"
|
||||
);
|
||||
|
||||
// ==================== 冷却机制 ====================
|
||||
@ -305,7 +320,8 @@ public class ConversationWindowManager {
|
||||
int tailTokenBudget = (int) (triggerThreshold * 0.20);
|
||||
|
||||
return compactMessages(messages, historyBudget, tailTokenBudget, chatModel,
|
||||
conversationId, agentId, totalTokens, spillsAtEntry, "token_threshold");
|
||||
conversationId, agentId, totalTokens, spillsAtEntry, "token_threshold",
|
||||
workspaceBasePath);
|
||||
}
|
||||
|
||||
/**
|
||||
@ -341,7 +357,7 @@ public class ConversationWindowManager {
|
||||
int tailTokenBudget, ChatModel chatModel,
|
||||
String conversationId, Long agentId,
|
||||
int preTokens, long spillsAtEntry,
|
||||
String trigger) {
|
||||
String trigger, String workspaceBasePath) {
|
||||
broadcastCompactStatus(conversationId, "start", Map.of(
|
||||
"preTokens", preTokens,
|
||||
"messagesIn", messages.size(),
|
||||
@ -421,6 +437,36 @@ public class ConversationWindowManager {
|
||||
}
|
||||
}
|
||||
|
||||
// ═══ Phase 2.7: Move 4 — Lossless spill evict ═══
|
||||
// Before paying the LLM-summary cost (lossy + tokens + latency),
|
||||
// try to bring oldMessages under budget by spilling remaining
|
||||
// oversized tool results to disk. Each spilled result becomes a
|
||||
// compact spill-marker (preview + on-disk path), recoverable via
|
||||
// read_file. Exempt tools and already-spilled markers are skipped.
|
||||
// If this phase lands the token count under historyBudget, the
|
||||
// LLM summary is skipped entirely — the eviction is lossless.
|
||||
if (toolResultStorage != null && workspaceBasePath != null) {
|
||||
int spilled = spillEvictToolResults(oldMessages, conversationId, workspaceBasePath);
|
||||
if (spilled > 0) {
|
||||
int afterSpillTokens = TokenEstimator.estimateTokens(oldMessages)
|
||||
+ TokenEstimator.estimateTokens(recentMessages);
|
||||
log.info("[ConversationWindow] Phase 2.7 Spill evict: {} results spilled, tokens={}, budget={}",
|
||||
spilled, afterSpillTokens, historyBudget);
|
||||
if (afterSpillTokens <= historyBudget) {
|
||||
List<Message> result = new ArrayList<>(oldMessages);
|
||||
result.addAll(recentMessages);
|
||||
broadcastCompactStatus(conversationId, "done", Map.of(
|
||||
"trigger", trigger,
|
||||
"preTokens", preTokens,
|
||||
"postTokens", afterSpillTokens,
|
||||
"strategy", "lossless_spill_evict",
|
||||
"resultsSpilled", spilled
|
||||
));
|
||||
return result;
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
// ═══ Phase 3: Pre-Prune + LLM 结构化摘要 ═══
|
||||
|
||||
// Pre-prune:在喂给摘要 LLM 前清理旧消息中的工具输出
|
||||
@ -931,6 +977,22 @@ public class ConversationWindowManager {
|
||||
&& r.responseData().startsWith(ToolResultStorage.SPILL_MARKER_PREFIX);
|
||||
}
|
||||
|
||||
/**
|
||||
* Move 4 — exempt tools ({@link #PRUNE_EXEMPT_TOOLS}) must bypass every
|
||||
* compaction phase, not just the size-based and age-based passes. Their
|
||||
* outputs are not replayable (sub-agent delegations run independent LLM
|
||||
* sessions) or not safely recoverable (load_skill returns the SKILL.md
|
||||
* snapshot at load time, which the author may have edited since). Trimming
|
||||
* or clearing them in Phase 1/2/3 silently drops the skill's constraints
|
||||
* and the sub-agent's transcript — the root cause of the
|
||||
* "compression causes attention failure" symptom.
|
||||
*/
|
||||
static boolean isExemptTool(ToolResponseMessage.ToolResponse r) {
|
||||
return r != null
|
||||
&& r.name() != null
|
||||
&& PRUNE_EXEMPT_TOOLS.contains(r.name());
|
||||
}
|
||||
|
||||
/**
|
||||
* Age-based compaction. Replace bodies of all tool responses older than
|
||||
* the {@code keepRecentN} most recent with a one-line placeholder, while
|
||||
@ -1071,6 +1133,12 @@ public class ConversationWindowManager {
|
||||
newResponses.add(r);
|
||||
continue;
|
||||
}
|
||||
if (isExemptTool(r)) {
|
||||
// Move 4: load_skill / delegateToAgent outputs are not
|
||||
// safely recoverable — pass through verbatim.
|
||||
newResponses.add(r);
|
||||
continue;
|
||||
}
|
||||
String data = r.responseData();
|
||||
if (data != null && data.length() > 500) {
|
||||
String marker = "\n...[trimmed " + data.length() + " chars; "
|
||||
@ -1108,6 +1176,11 @@ public class ConversationWindowManager {
|
||||
replaced.add(r);
|
||||
continue;
|
||||
}
|
||||
if (isExemptTool(r)) {
|
||||
// Move 4: exempt tools survive Phase 2 unchanged.
|
||||
replaced.add(r);
|
||||
continue;
|
||||
}
|
||||
replaced.add(new ToolResponseMessage.ToolResponse(r.id(), r.name(),
|
||||
buildInformativeCleared(r.name(), r.responseData())));
|
||||
changed = true;
|
||||
@ -1130,8 +1203,12 @@ public class ConversationWindowManager {
|
||||
int pruned = 0;
|
||||
for (int i = 0; i < messages.size(); i++) {
|
||||
if (messages.get(i) instanceof ToolResponseMessage trm) {
|
||||
// Move 4: skip the entire ToolResponseMessage if every
|
||||
// response is either a spill marker or an exempt tool —
|
||||
// there's nothing to prune.
|
||||
boolean hasSubstantial = trm.getResponses().stream()
|
||||
.anyMatch(r -> !isSpillMarker(r)
|
||||
&& !isExemptTool(r)
|
||||
&& r.responseData() != null
|
||||
&& r.responseData().length() > 200);
|
||||
if (hasSubstantial) {
|
||||
@ -1141,6 +1218,11 @@ public class ConversationWindowManager {
|
||||
placeholders.add(r);
|
||||
continue;
|
||||
}
|
||||
if (isExemptTool(r)) {
|
||||
// Move 4: exempt tools survive Phase 3 pre-prune.
|
||||
placeholders.add(r);
|
||||
continue;
|
||||
}
|
||||
placeholders.add(new ToolResponseMessage.ToolResponse(r.id(), r.name(),
|
||||
"[旧工具输出已清理以节省上下文空间]"));
|
||||
}
|
||||
@ -1152,6 +1234,71 @@ public class ConversationWindowManager {
|
||||
return pruned;
|
||||
}
|
||||
|
||||
/**
|
||||
* Move 4 — Phase 2.7 lossless spill evict. Walks {@code messages} and
|
||||
* spills each non-exempt, non-spilled, oversized tool result to disk via
|
||||
* {@link ToolResultStorage#persistIfOversized}, replacing the body with
|
||||
* a compact spill marker (preview + on-disk path). The model can recover
|
||||
* the original via {@code read_file} on the path.
|
||||
*
|
||||
* <p>Unlike Phase 1/2 (which trim/clear in place), this phase is
|
||||
* <em>lossless</em>: the full output is preserved on disk. The token
|
||||
* savings come from replacing a multi-KB body with a ~200-char preview.
|
||||
* When the savings are enough to land under {@code historyBudget}, the
|
||||
* caller skips the LLM summary entirely — avoiding its token cost,
|
||||
* latency, and lossy compression.
|
||||
*
|
||||
* <p>Skips:
|
||||
* <ul>
|
||||
* <li>Exempt tools ({@link #PRUNE_EXEMPT_TOOLS}) — not safely
|
||||
* recoverable.</li>
|
||||
* <li>Already-spilled markers — re-spilling would just overwrite the
|
||||
* same file.</li>
|
||||
* <li>Bodies under {@code perResultThresholdChars} — too small to
|
||||
* benefit from spilling (the marker preview is comparable in size).</li>
|
||||
* </ul>
|
||||
*
|
||||
* @return number of tool responses actually spilled to disk
|
||||
*/
|
||||
int spillEvictToolResults(List<Message> messages, String conversationId,
|
||||
String workspaceBasePath) {
|
||||
if (toolResultStorage == null || conversationId == null || conversationId.isBlank()) {
|
||||
return 0;
|
||||
}
|
||||
int spilled = 0;
|
||||
for (int i = 0; i < messages.size(); i++) {
|
||||
if (!(messages.get(i) instanceof ToolResponseMessage trm)) continue;
|
||||
List<ToolResponseMessage.ToolResponse> newResponses = new ArrayList<>();
|
||||
boolean changed = false;
|
||||
for (ToolResponseMessage.ToolResponse r : trm.getResponses()) {
|
||||
if (isSpillMarker(r) || isExemptTool(r)) {
|
||||
newResponses.add(r);
|
||||
continue;
|
||||
}
|
||||
String data = r.responseData();
|
||||
if (data == null || data.isEmpty()) {
|
||||
newResponses.add(r);
|
||||
continue;
|
||||
}
|
||||
String spilledBody = toolResultStorage.persistIfOversized(
|
||||
data, r.name(), r.id(), conversationId, workspaceBasePath);
|
||||
if (spilledBody != data) {
|
||||
// persistIfOversized replaced the body with a spill marker
|
||||
newResponses.add(new ToolResponseMessage.ToolResponse(
|
||||
r.id(), r.name(), spilledBody));
|
||||
changed = true;
|
||||
spilled++;
|
||||
} else {
|
||||
newResponses.add(r);
|
||||
}
|
||||
}
|
||||
if (changed) {
|
||||
messages.set(i, ToolResponseMessage.builder().responses(newResponses).build());
|
||||
}
|
||||
}
|
||||
return spilled;
|
||||
}
|
||||
|
||||
// ==================== LLM 摘要生成(结构化 + 迭代更新) ====================
|
||||
|
||||
/**
|
||||
@ -1383,7 +1530,7 @@ public class ConversationWindowManager {
|
||||
|
||||
List<Message> compacted = compactMessages(messages, forcedBudget, forcedTailBudget,
|
||||
chatModel, conversationId, agentId, currentTokens, spillsAtEntry,
|
||||
"prompt_too_long");
|
||||
"prompt_too_long", null);
|
||||
|
||||
if (compacted == messages || TokenEstimator.estimateTokens(compacted) >= currentTokens) {
|
||||
log.warn("[ConversationWindow] PTL structured compaction had no effect for conv={}, falling back to tail-only",
|
||||
|
||||
@ -8,6 +8,7 @@ import org.springframework.ai.chat.model.ToolContext;
|
||||
import org.springframework.ai.tool.ToolCallback;
|
||||
import vip.mate.tool.builtin.ToolExecutionContext;
|
||||
import vip.mate.tool.disclosure.ToolUsageRecencyTracker;
|
||||
import vip.mate.tool.mcp.runtime.McpToolNameResolver;
|
||||
import vip.mate.agent.AgentToolSet;
|
||||
import vip.mate.agent.GraphEventPublisher;
|
||||
import vip.mate.agent.context.ChatOrigin;
|
||||
@ -1264,7 +1265,79 @@ public class ToolExecutionExecutor {
|
||||
log.debug("[ToolExecutor] skill-aware hint check failed: {}", e.getMessage());
|
||||
}
|
||||
}
|
||||
return "Tool not found: " + toolName;
|
||||
return buildMcpAwareNotFoundMessage(toolName);
|
||||
}
|
||||
|
||||
/**
|
||||
* Build a "Tool not found" message that surfaces candidate MCP tools
|
||||
* when the LLM appears to have confused two servers' tools.
|
||||
*
|
||||
* <p>Background: MCP tool names are {@code mcp_<serverId>_<slug>_<hash6>}.
|
||||
* The {@code serverId} is an opaque 19-digit Snowflake ID. When a task
|
||||
* mixes tools from multiple MCP servers, the LLM often reconstructs a
|
||||
* tool name by taking a remembered slug and swapping it onto the
|
||||
* serverId of a previously-successful call — producing a non-existent
|
||||
* name like {@code mcp_<serverA>_fetch_xxx} when {@code fetch} actually
|
||||
* lives under serverB. Without candidate suggestions, the LLM gets
|
||||
* "Tool not found: ..." with no recovery signal and keeps retrying the
|
||||
* same wrong name until it hits max iterations.
|
||||
*
|
||||
* <p>This method parses the requested name, and if it looks like an
|
||||
* MCP tool, searches {@link #toolCallbackMap} for registered tools
|
||||
* whose slug OR hash6 matches (cross-server). Matching by slug catches
|
||||
* "same raw tool name on a different server"; matching by hash6
|
||||
* catches "same raw tool name with the LLM remembering the hash but
|
||||
* not the serverId". Returns at most 5 candidates to keep the message
|
||||
* bounded.
|
||||
*
|
||||
* <p>If no candidates are found, falls back to the plain
|
||||
* "Tool not found: ..." message.
|
||||
*/
|
||||
private String buildMcpAwareNotFoundMessage(String toolName) {
|
||||
if (toolName == null || toolName.isBlank()) {
|
||||
return "Tool not found: " + toolName;
|
||||
}
|
||||
|
||||
McpToolNameResolver.ParsedRef ref = McpToolNameResolver.parse(toolName);
|
||||
if (ref == null) {
|
||||
// Not an MCP-prefixed name — no candidate heuristic applies.
|
||||
return "Tool not found: " + toolName;
|
||||
}
|
||||
|
||||
// Search for registered tools with matching slug or hash6 on OTHER servers.
|
||||
java.util.List<String> candidates = new java.util.ArrayList<>();
|
||||
for (String registered : toolCallbackMap.keySet()) {
|
||||
McpToolNameResolver.ParsedRef r = McpToolNameResolver.parse(registered);
|
||||
if (r == null || r.serverId() == ref.serverId()) {
|
||||
continue; // same server, or not an MCP tool — skip
|
||||
}
|
||||
boolean slugMatch = r.slug().equals(ref.slug());
|
||||
boolean hashMatch = r.hash6().equals(ref.hash6());
|
||||
if (slugMatch || hashMatch) {
|
||||
candidates.add(registered);
|
||||
if (candidates.size() >= 5) {
|
||||
break; // bound the list
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
if (candidates.isEmpty()) {
|
||||
return "Tool not found: " + toolName
|
||||
+ "\n(This name looks like an MCP tool on server " + ref.serverId()
|
||||
+ ", but no tool with slug '" + ref.slug() + "' is registered on any server."
|
||||
+ " Check the tool list for the correct name.)";
|
||||
}
|
||||
|
||||
StringBuilder sb = new StringBuilder("Tool not found: ").append(toolName).append('\n');
|
||||
sb.append("The name looks like an MCP tool on server ").append(ref.serverId())
|
||||
.append(", but no tool with that slug/hash is registered there.\n");
|
||||
sb.append("Did you mean one of these (same slug or hash on other servers)?\n");
|
||||
for (String c : candidates) {
|
||||
sb.append(" - ").append(c).append('\n');
|
||||
}
|
||||
sb.append("\nUse the exact name from above. Do NOT reconstruct tool names from memory — ")
|
||||
.append("always copy verbatim from the tool list or from this suggestion.");
|
||||
return sb.toString();
|
||||
}
|
||||
|
||||
/**
|
||||
|
||||
@ -25,6 +25,14 @@ import static vip.mate.agent.graph.state.MateClawStateKeys.*;
|
||||
* <p>
|
||||
* 支持 forced_replay 阶段:当审批通过后的重放调用到达时,跳过 ToolGuard 检查直接执行。
|
||||
*
|
||||
* <p>B2: 当检测到 {@code load_skill} 调用时,从 skill manifest 提取
|
||||
* {@code constraints} 并写入 ProgressLedger 的 pinned entries,使约束
|
||||
* 在整个对话中可见、不被 LLM 的 progress_update 覆盖、不被上下文压缩裁剪。
|
||||
*
|
||||
* <p>B5: 工具调用成功后自动回填 ProgressLedger,让 LLM 即使不主动调用
|
||||
* progress_update 也能在下一轮看到已完成的工具调用记录。自动记录条数有上限
|
||||
* ({@link ProgressLedgerService#MAX_AUTO_RECORDED}),且不覆盖 LLM 已写的条目。
|
||||
*
|
||||
* @author MateClaw Team
|
||||
*/
|
||||
@Slf4j
|
||||
@ -38,9 +46,27 @@ public class ActionNode implements NodeAction {
|
||||
/** Function name of the extension-tool activator, mirrored from EnableExtensionTool. */
|
||||
private static final String ENABLE_TOOL = "enable_tool";
|
||||
|
||||
/** Function name of the progress-update tool — skip auto-recording it. */
|
||||
private static final String PROGRESS_UPDATE_TOOL = "progress_update";
|
||||
|
||||
/**
|
||||
* Tools whose results should NOT be auto-recorded into the ledger.
|
||||
* Meta-tools (load_skill, enable_tool, progress_update) either have
|
||||
* their own ledger side-effects or are the ledger itself.
|
||||
*/
|
||||
private static final Set<String> AUTO_RECORD_SKIP = Set.of(
|
||||
LOAD_SKILL_TOOL, ENABLE_TOOL, PROGRESS_UPDATE_TOOL,
|
||||
"listAvailableSkills", "readSkillFile", "runSkillScript"
|
||||
);
|
||||
|
||||
private final ToolExecutionExecutor executor;
|
||||
private final vip.mate.channel.web.ChatStreamTracker streamTracker;
|
||||
|
||||
/** Optional — B2: extract constraints from skill manifest on load_skill. */
|
||||
private vip.mate.skill.runtime.SkillRuntimeService skillRuntimeService;
|
||||
/** Optional — B2/B5: write pinned + auto-recorded entries. */
|
||||
private vip.mate.agent.progress.ProgressLedgerService progressLedgerService;
|
||||
|
||||
public ActionNode(ToolExecutionExecutor executor) {
|
||||
this(executor, null);
|
||||
}
|
||||
@ -50,6 +76,16 @@ public class ActionNode implements NodeAction {
|
||||
this.streamTracker = streamTracker;
|
||||
}
|
||||
|
||||
/** Setter injection so existing constructors stay source-compatible. */
|
||||
public void setSkillRuntimeService(vip.mate.skill.runtime.SkillRuntimeService s) {
|
||||
this.skillRuntimeService = s;
|
||||
}
|
||||
|
||||
/** Setter injection so existing constructors stay source-compatible. */
|
||||
public void setProgressLedgerService(vip.mate.agent.progress.ProgressLedgerService s) {
|
||||
this.progressLedgerService = s;
|
||||
}
|
||||
|
||||
@Override
|
||||
@SuppressWarnings("unchecked")
|
||||
public Map<String, Object> apply(OverAllState state) throws Exception {
|
||||
@ -88,14 +124,6 @@ public class ActionNode implements NodeAction {
|
||||
.responses(result.responses())
|
||||
.build();
|
||||
|
||||
// Use the executor's raw-stage ledger instead of re-parsing the
|
||||
// spill-compacted responses. ToolExecutionExecutor builds this
|
||||
// ledger from the full pre-truncate text, so a 30 KB grep result
|
||||
// whose head/tail-cut version no longer mentions a path will still
|
||||
// contribute that path to the evidence pool. Falls back to empty
|
||||
// for legacy executor stubs (tests, mocks) that didn't populate
|
||||
// the new field — fine, the merge with `accessor.sourceEvidenceLedger`
|
||||
// is no-op in that case.
|
||||
SourceEvidenceLedger rawLedger = result.rawEvidenceLedger() != null
|
||||
? result.rawEvidenceLedger()
|
||||
: SourceEvidenceLedger.empty();
|
||||
@ -112,19 +140,6 @@ public class ActionNode implements NodeAction {
|
||||
}
|
||||
|
||||
// RFC-052: any returnDirect tool in this batch ⇒ short-circuit the graph.
|
||||
// ObservationDispatcher will route to FinalAnswerNode (skipping the next
|
||||
// LLM call). Direct outputs and the trigger flag both live in state so
|
||||
// FinalAnswerNode can assemble the final answer verbatim.
|
||||
//
|
||||
// Priority guard: when an approval barrier ALSO fires in the same batch
|
||||
// (a direct tool ran successfully BEFORE a sibling tool that needed
|
||||
// approval), let the approval flow win. Otherwise the user would see a
|
||||
// "RETURN_DIRECT" final answer while an approval modal is still open
|
||||
// for the unresolved sibling — a confusing dual-track state. After the
|
||||
// user resolves the approval, the replay path will re-execute and the
|
||||
// direct tool's content reaches the user via the streamedContent path
|
||||
// instead. Same-batch direct+approval is rare; we explicitly defer to
|
||||
// approval for safety.
|
||||
if (result.hasDirectOutputs() && !result.awaitingApproval()) {
|
||||
output.returnDirectTriggered(true);
|
||||
output.directToolOutputs(result.directOutputs());
|
||||
@ -144,20 +159,20 @@ public class ActionNode implements NodeAction {
|
||||
}
|
||||
|
||||
// Pin skills the model loaded this run so the next reasoning turn's
|
||||
// catalog ranks them first and the model stops re-loading the same
|
||||
// skill it already pulled into message history. Tools cannot mutate
|
||||
// graph state directly, so the load is detected here from the tool
|
||||
// calls and merged into LOADED_SKILLS (read-merge-write, REPLACE key).
|
||||
// catalog ranks them first.
|
||||
Set<String> requestedSkills = extractLoadedSkillNames(toolCalls);
|
||||
if (!requestedSkills.isEmpty()) {
|
||||
Set<String> merged = new LinkedHashSet<>(accessor.loadedSkills());
|
||||
if (merged.addAll(requestedSkills)) {
|
||||
output.loadedSkills(Set.copyOf(merged));
|
||||
}
|
||||
// B2: extract structured constraints from loaded skills' manifests
|
||||
// and pin them into the ProgressLedger so they survive context
|
||||
// compression and stay visible on every turn.
|
||||
pinSkillConstraints(conversationId, requestedSkills);
|
||||
}
|
||||
|
||||
// Same mechanism for enable_tool: record the activated extension tools so
|
||||
// ReasoningNode's next turn adds them back to the advertised callbacks.
|
||||
// Same mechanism for enable_tool
|
||||
Set<String> enabledTools = extractEnabledToolNames(toolCalls);
|
||||
if (!enabledTools.isEmpty()) {
|
||||
Set<String> merged = new LinkedHashSet<>(accessor.enabledExtensionTools());
|
||||
@ -166,15 +181,134 @@ public class ActionNode implements NodeAction {
|
||||
}
|
||||
}
|
||||
|
||||
// B5: auto-record successful tool calls into the ledger so the LLM
|
||||
// sees what it already did even if it forgot to call progress_update.
|
||||
// Skips meta-tools (load_skill, enable_tool, progress_update) and
|
||||
// doesn't overwrite LLM-authored entries.
|
||||
autoRecordToolCalls(conversationId, result.responses());
|
||||
|
||||
return output.build();
|
||||
}
|
||||
|
||||
// ==================== B2: Pin skill constraints ====================
|
||||
|
||||
/**
|
||||
* Extract the {@code toolName} argument of every {@code enable_tool} call in
|
||||
* this batch. Like {@link #extractLoadedSkillNames}, an unknown name is
|
||||
* harmless: the reasoning-node split only activates names that resolve to an
|
||||
* extension-tier tool actually in the agent's set.
|
||||
* For each loaded skill, extract the manifest's {@code constraints} list
|
||||
* and write them as pinned entries in the ProgressLedger. Pinned entries
|
||||
* live in {@code nonHistoryPrefix} (never trimmed) and are never
|
||||
* overwritten by the LLM's {@code progress_update} tool.
|
||||
*
|
||||
* <p>Failures are swallowed — a missing manifest or a ledger write error
|
||||
* must never abort the tool execution batch.
|
||||
*/
|
||||
private void pinSkillConstraints(String conversationId, Set<String> skillNames) {
|
||||
if (progressLedgerService == null || skillRuntimeService == null
|
||||
|| conversationId == null || conversationId.isBlank()) {
|
||||
return;
|
||||
}
|
||||
for (String skillName : skillNames) {
|
||||
try {
|
||||
vip.mate.skill.runtime.model.ResolvedSkill skill = skillRuntimeService.findActiveSkill(skillName);
|
||||
if (skill == null || skill.getManifest() == null) {
|
||||
continue;
|
||||
}
|
||||
List<String> constraints = skill.getManifest().getConstraints();
|
||||
if (constraints == null || constraints.isEmpty()) {
|
||||
continue;
|
||||
}
|
||||
// Clear old pinned entries for this skill first (handles re-load after update).
|
||||
String keyPrefix = "pin_" + skillName + "_";
|
||||
progressLedgerService.clearPinnedByPrefix(conversationId, keyPrefix);
|
||||
// Write each constraint as a pinned entry.
|
||||
for (int i = 0; i < constraints.size(); i++) {
|
||||
String constraint = constraints.get(i);
|
||||
if (constraint == null || constraint.isBlank()) {
|
||||
continue;
|
||||
}
|
||||
String key = keyPrefix + i;
|
||||
progressLedgerService.upsertPinned(conversationId, key,
|
||||
"🔒 " + skillName + ": " + truncate(constraint, 100), constraint);
|
||||
}
|
||||
log.info("[ActionNode] Pinned {} constraint(s) from skill '{}' for conv {}",
|
||||
constraints.size(), skillName, conversationId);
|
||||
} catch (Exception e) {
|
||||
log.warn("[ActionNode] Failed to pin constraints for skill '{}': {}",
|
||||
skillName, e.getMessage());
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
// ==================== B5: Auto-record tool calls ====================
|
||||
|
||||
/**
|
||||
* Auto-record each successful tool call as a ledger entry with key
|
||||
* {@code auto_<toolName>}. Bounded to {@link ProgressLedgerService#MAX_AUTO_RECORDED}
|
||||
* most recent entries. Skips meta-tools and doesn't overwrite LLM entries.
|
||||
*
|
||||
* <p>Key uniqueness: for MCP tools the FULL prefixed name
|
||||
* ({@code mcp_<serverId>_<slug>_<hash6>}) is used as the key suffix to
|
||||
* avoid collisions between servers that expose tools with the same slug.
|
||||
* The display label uses the simplified slug for readability.
|
||||
*/
|
||||
private void autoRecordToolCalls(String conversationId,
|
||||
List<ToolResponseMessage.ToolResponse> responses) {
|
||||
if (progressLedgerService == null || conversationId == null
|
||||
|| conversationId.isBlank() || responses == null || responses.isEmpty()) {
|
||||
return;
|
||||
}
|
||||
// Collect valid entries first, then persist in a single batch to avoid
|
||||
// N separate lock+load+save cycles when the LLM calls tools in parallel.
|
||||
List<vip.mate.agent.progress.ProgressLedgerService.AutoRecordEntry> batch = new java.util.ArrayList<>();
|
||||
for (ToolResponseMessage.ToolResponse resp : responses) {
|
||||
String toolName = resp.name();
|
||||
if (toolName == null || toolName.isBlank() || AUTO_RECORD_SKIP.contains(toolName)) {
|
||||
continue;
|
||||
}
|
||||
// Use the full tool name as the key (unique across MCP servers),
|
||||
// but the simplified slug as the display label (readable).
|
||||
String displayName = simplifyToolName(toolName);
|
||||
String summary = resp.responseData();
|
||||
if (summary != null && summary.length() > 120) {
|
||||
summary = summary.substring(0, 120) + "…";
|
||||
}
|
||||
batch.add(new vip.mate.agent.progress.ProgressLedgerService.AutoRecordEntry(
|
||||
toolName, displayName, summary));
|
||||
}
|
||||
if (batch.isEmpty()) {
|
||||
return;
|
||||
}
|
||||
try {
|
||||
progressLedgerService.upsertAutoRecordedBatch(conversationId, batch);
|
||||
} catch (Exception e) {
|
||||
log.debug("[ActionNode] Batch auto-record failed for {} tools: {}",
|
||||
batch.size(), e.getMessage());
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Simplify an MCP tool name ({@code mcp_<serverId>_<slug>_<hash6>})
|
||||
* to just the slug for ledger readability. Non-MCP names pass through.
|
||||
*/
|
||||
private static String simplifyToolName(String name) {
|
||||
if (name == null) return "unknown";
|
||||
if (!name.startsWith("mcp_")) return name;
|
||||
// mcp_<serverId>_<slug>_<hash6> → <slug>
|
||||
int firstSep = name.indexOf('_', 4);
|
||||
int lastSep = name.lastIndexOf('_');
|
||||
if (firstSep > 0 && lastSep > firstSep) {
|
||||
String slug = name.substring(firstSep + 1, lastSep);
|
||||
return slug.isEmpty() ? name : slug;
|
||||
}
|
||||
return name;
|
||||
}
|
||||
|
||||
private static String truncate(String s, int max) {
|
||||
if (s == null) return "";
|
||||
return s.length() > max ? s.substring(0, max) + "…" : s;
|
||||
}
|
||||
|
||||
// ==================== Existing helpers ====================
|
||||
|
||||
static Set<String> extractEnabledToolNames(List<AssistantMessage.ToolCall> toolCalls) {
|
||||
if (toolCalls == null || toolCalls.isEmpty()) {
|
||||
return Set.of();
|
||||
@ -192,12 +326,6 @@ public class ActionNode implements NodeAction {
|
||||
return names;
|
||||
}
|
||||
|
||||
/**
|
||||
* Extract the {@code skillName} argument of every {@code load_skill} call in
|
||||
* this batch. The names are used only to bias catalog ordering, so an
|
||||
* unparseable or unknown name is harmless (it simply never matches a
|
||||
* visible skill) — failures are swallowed rather than aborting the batch.
|
||||
*/
|
||||
static Set<String> extractLoadedSkillNames(List<AssistantMessage.ToolCall> toolCalls) {
|
||||
if (toolCalls == null || toolCalls.isEmpty()) {
|
||||
return Set.of();
|
||||
@ -215,11 +343,6 @@ public class ActionNode implements NodeAction {
|
||||
return names;
|
||||
}
|
||||
|
||||
/**
|
||||
* Read the first present, non-null string value among {@code keys} from a
|
||||
* tool-call arguments JSON object. Returns null on malformed JSON or when
|
||||
* none of the keys are present.
|
||||
*/
|
||||
private static String parseStringArg(String argumentsJson, String... keys) {
|
||||
if (argumentsJson == null || argumentsJson.isBlank()) {
|
||||
return null;
|
||||
|
||||
@ -376,6 +376,20 @@ public class ReasoningNode implements NodeAction {
|
||||
this.autoDemotedTools = autoDemotedTools == null ? Set.of() : autoDemotedTools;
|
||||
}
|
||||
|
||||
/**
|
||||
* C4: per-conversation registry of environment-change notifications.
|
||||
* When non-null, each reasoning turn drains pending notifications and
|
||||
* injects them as a single {@code SystemMessage} so the LLM sees that
|
||||
* an MCP server disconnected or a skill was updated mid-turn. Null in
|
||||
* tests / legacy paths — injection is simply skipped.
|
||||
*/
|
||||
private vip.mate.agent.runtime.RunningConversationRegistry runningConversationRegistry;
|
||||
|
||||
public void setRunningConversationRegistry(
|
||||
vip.mate.agent.runtime.RunningConversationRegistry runningConversationRegistry) {
|
||||
this.runningConversationRegistry = runningConversationRegistry;
|
||||
}
|
||||
|
||||
/** Floor for the window-aware output clamp — an answer needs at least this much room. */
|
||||
private static final int MIN_CLAMPED_OUTPUT_TOKENS = 512;
|
||||
|
||||
@ -684,15 +698,18 @@ public class ReasoningNode implements NodeAction {
|
||||
List<Message> nonHistoryPrefix = buildNonHistoryPrefix(systemPrompt, workspaceBasePath, agentIdStr, userMsg,
|
||||
accessor.chatOrigin(), runtimeModelName, runtimeProviderId);
|
||||
|
||||
// Append the runtime-rendered skill catalog as a SEPARATE SystemMessage
|
||||
// right after the skeleton system prompt. Keeping it out of the baked
|
||||
// prompt keeps the stable prefix's prompt-cache hash intact, while
|
||||
// re-rendering each turn lets skills loaded this run (load_skill) pin
|
||||
// to the top of the catalog. Reused verbatim by the PTL retry branch.
|
||||
// Append the skill catalog as a SEPARATE SystemMessage right after the
|
||||
// skeleton system prompt. Rendered with an empty loadedThisRun set so
|
||||
// the catalog content stays stable across turns — this lets the
|
||||
// system+catalog SystemMessage pair be served from the prompt cache
|
||||
// (Anthropic SYSTEM_AND_TOOLS / OpenAI prefix cache). The per-turn
|
||||
// "skills loaded this run" hint is injected separately as a volatile
|
||||
// suffix (after RuntimeContext) so it never invalidates the cached
|
||||
// prefix. Reused verbatim by the PTL retry branch.
|
||||
if (skillCatalogRenderer != null) {
|
||||
String skillCatalog = skillCatalogRenderer.render(accessor.loadedSkills());
|
||||
if (skillCatalog != null && !skillCatalog.isBlank()) {
|
||||
nonHistoryPrefix.add(1, new SystemMessage(skillCatalog));
|
||||
String staticCatalog = skillCatalogRenderer.render(java.util.Set.of());
|
||||
if (staticCatalog != null && !staticCatalog.isBlank()) {
|
||||
nonHistoryPrefix.add(1, new SystemMessage(staticCatalog));
|
||||
}
|
||||
}
|
||||
|
||||
@ -732,6 +749,44 @@ public class ReasoningNode implements NodeAction {
|
||||
}
|
||||
}
|
||||
|
||||
// C4: drain any environment-change notifications that landed while
|
||||
// this conversation was running (MCP disconnect, skill update, etc.)
|
||||
// and inject them as a one-shot SystemMessage. Each notification is
|
||||
// delivered at most once — drain() empties the queue. Skipped when
|
||||
// the registry is absent (tests / legacy paths) or the conversation
|
||||
// has no pending notifications.
|
||||
if (runningConversationRegistry != null && conversationId != null && !conversationId.isBlank()) {
|
||||
try {
|
||||
List<vip.mate.agent.runtime.EnvironmentNotification> notes =
|
||||
runningConversationRegistry.drain(conversationId);
|
||||
if (!notes.isEmpty()) {
|
||||
String block = renderEnvironmentNotifications(notes);
|
||||
if (block != null) {
|
||||
nonHistoryPrefix.add(new SystemMessage(block));
|
||||
log.info("[ReasoningNode] Injected {} environment notification(s) for conv {}",
|
||||
notes.size(), conversationId);
|
||||
}
|
||||
}
|
||||
} catch (Exception e) {
|
||||
log.warn("[ReasoningNode] Failed to drain environment notifications for {}: {}",
|
||||
conversationId, e.getMessage());
|
||||
}
|
||||
}
|
||||
|
||||
// Per-turn "skills loaded this run" hint — kept out of the stable
|
||||
// catalog segment (which is rendered with an empty loadedThisRun set
|
||||
// so it stays prompt-cache-friendly). Injected here as a volatile
|
||||
// suffix so the model still sees which skills it already pulled in
|
||||
// via load_skill this run, without invalidating the cached
|
||||
// system+catalog SystemMessage prefix.
|
||||
java.util.Set<String> loadedThisRun = accessor.loadedSkills();
|
||||
if (loadedThisRun != null && !loadedThisRun.isEmpty()) {
|
||||
String hint = renderLoadedSkillsHint(loadedThisRun);
|
||||
if (hint != null) {
|
||||
nonHistoryPrefix.add(new SystemMessage(hint));
|
||||
}
|
||||
}
|
||||
|
||||
if (conversationWindowManager != null) {
|
||||
// Age-based compaction first: drop the body of tool responses
|
||||
// older than the K most recent into a one-line placeholder that
|
||||
@ -1154,6 +1209,53 @@ public class ReasoningNode implements NodeAction {
|
||||
return out;
|
||||
}
|
||||
|
||||
/**
|
||||
* C4: render drained environment notifications into a single markdown
|
||||
* block suitable for injection as a {@code SystemMessage} in
|
||||
* {@code nonHistoryPrefix}. Returns {@code null} for an empty list so the
|
||||
* caller can skip injection entirely (no "(empty)" noise).
|
||||
*/
|
||||
// Package-private so black-box tests in vip.mate.agent.graph.node can
|
||||
// exercise the real production rendering without duplicating the format.
|
||||
static String renderEnvironmentNotifications(
|
||||
List<vip.mate.agent.runtime.EnvironmentNotification> notes) {
|
||||
if (notes == null || notes.isEmpty()) {
|
||||
return null;
|
||||
}
|
||||
StringBuilder sb = new StringBuilder(128);
|
||||
sb.append("## 📢 环境变更通知(本轮新增,请立即据此调整计划)\n\n");
|
||||
for (vip.mate.agent.runtime.EnvironmentNotification n : notes) {
|
||||
sb.append("- ").append(n.message()).append('\n');
|
||||
}
|
||||
sb.append("\n以上通知由 Java 运行时检测并注入,权威可信。")
|
||||
.append("如果通知涉及你正在使用的工具/skill,请立即调整后续步骤;")
|
||||
.append("如果与当前任务无关,可忽略。");
|
||||
return sb.toString();
|
||||
}
|
||||
|
||||
/**
|
||||
* Render the per-turn "skills loaded this run" hint as a short
|
||||
* SystemMessage. Kept out of the stable skill-catalog segment (which is
|
||||
* rendered with an empty loadedThisRun set for prompt-cache stability)
|
||||
* so the model still knows which skills it already pulled in via
|
||||
* load_skill without invalidating the cached system+catalog prefix.
|
||||
* Returns {@code null} for an empty set so the caller can skip
|
||||
* injection entirely.
|
||||
*/
|
||||
// Package-private so tests can exercise the format without duplicating it.
|
||||
static String renderLoadedSkillsHint(java.util.Set<String> loadedThisRun) {
|
||||
if (loadedThisRun == null || loadedThisRun.isEmpty()) {
|
||||
return null;
|
||||
}
|
||||
StringBuilder sb = new StringBuilder(96);
|
||||
sb.append("Skills already loaded this run (available in context, do not re-load): ");
|
||||
sb.append(String.join(", ", loadedThisRun.stream()
|
||||
.map(n -> "`" + n + "`")
|
||||
.toList()));
|
||||
sb.append('.');
|
||||
return sb.toString();
|
||||
}
|
||||
|
||||
/**
|
||||
* Build the part of the Prompt that does not depend on history messages:
|
||||
* system prompt, workspace runtime context, and (when wiring permits) the
|
||||
|
||||
@ -16,41 +16,82 @@ import java.util.Optional;
|
||||
* blocked) and stays short on purpose: the agent reads it on every turn, so
|
||||
* spending more than ~200 tokens on it would defeat the very context
|
||||
* pressure this ledger exists to relieve.
|
||||
*
|
||||
* <p>Three entry classes coexist:
|
||||
* <ul>
|
||||
* <li><b>Regular entries</b> — LLM-controlled via the {@code progress_update}
|
||||
* tool. The model registers steps, advances their status, and adds
|
||||
* notes. These are what {@link #mostRecentUpdate()} and the stale
|
||||
* reminder consider when judging whether the ledger is maintained.</li>
|
||||
* <li><b>Pinned entries</b> — Java-controlled, written by ActionNode when
|
||||
* {@code load_skill} is called (B2). They carry the skill's structured
|
||||
* constraints (from {@code SkillManifest.constraints}) and survive
|
||||
* context compression because they live in {@code nonHistoryPrefix}.
|
||||
* The LLM's {@code progress_update} tool never touches them.</li>
|
||||
* <li><b>Auto-recorded entries</b> — Java-controlled, written by ActionNode
|
||||
* after a successful tool call (B5). They use the
|
||||
* {@link #AUTO_RECORDED_PREFIX} on their key so the renderer can group
|
||||
* them separately. They don't affect staleness calculation.</li>
|
||||
* </ul>
|
||||
*/
|
||||
public final class ProgressLedger {
|
||||
|
||||
/** Hard cap on the snapshot's note suffix so a rambling note can't bloat every turn. */
|
||||
private static final int NOTE_PREVIEW_CHARS = 120;
|
||||
|
||||
/**
|
||||
* Key prefix for entries auto-recorded by ActionNode after successful
|
||||
* tool calls (B5). Lets the renderer group them into a separate
|
||||
* "auto-recorded" section and exclude them from staleness calculation.
|
||||
*/
|
||||
public static final String AUTO_RECORDED_PREFIX = "auto_";
|
||||
|
||||
private final Map<String, ProgressEntry> entries;
|
||||
private final Map<String, ProgressEntry> pinned;
|
||||
|
||||
public ProgressLedger(Map<String, ProgressEntry> entries) {
|
||||
this(entries, null);
|
||||
}
|
||||
|
||||
public ProgressLedger(Map<String, ProgressEntry> entries, Map<String, ProgressEntry> pinned) {
|
||||
this.entries = entries != null ? entries : new LinkedHashMap<>();
|
||||
this.pinned = pinned != null ? pinned : new LinkedHashMap<>();
|
||||
}
|
||||
|
||||
public static ProgressLedger empty() {
|
||||
return new ProgressLedger(new LinkedHashMap<>());
|
||||
return new ProgressLedger(new LinkedHashMap<>(), new LinkedHashMap<>());
|
||||
}
|
||||
|
||||
public boolean isEmpty() {
|
||||
return entries.isEmpty();
|
||||
return entries.isEmpty() && pinned.isEmpty();
|
||||
}
|
||||
|
||||
public int size() {
|
||||
return entries.size();
|
||||
return entries.size() + pinned.size();
|
||||
}
|
||||
|
||||
public Map<String, ProgressEntry> asMap() {
|
||||
return entries;
|
||||
}
|
||||
|
||||
/** @return the pinned (Java-controlled) entries, never null. */
|
||||
public Map<String, ProgressEntry> pinnedEntries() {
|
||||
return pinned;
|
||||
}
|
||||
|
||||
/**
|
||||
* @return the most recent {@code updatedAt} across all entries, or empty
|
||||
* when the ledger is empty / all entries lack a timestamp.
|
||||
* @return the most recent {@code updatedAt} across regular (non-auto,
|
||||
* non-pinned) entries, or empty when none have a timestamp.
|
||||
* Only regular entries count because pinned entries are static
|
||||
* constraints and auto-recorded entries are Java-side — neither
|
||||
* indicates the LLM is maintaining the ledger.
|
||||
*/
|
||||
public Optional<Instant> mostRecentUpdate() {
|
||||
Instant max = null;
|
||||
for (ProgressEntry e : entries.values()) {
|
||||
if (e.getKey() != null && e.getKey().startsWith(AUTO_RECORDED_PREFIX)) {
|
||||
continue;
|
||||
}
|
||||
Instant t = e.getUpdatedAt();
|
||||
if (t != null && (max == null || t.isAfter(max))) {
|
||||
max = t;
|
||||
@ -59,45 +100,21 @@ public final class ProgressLedger {
|
||||
return Optional.ofNullable(max);
|
||||
}
|
||||
|
||||
/** Iteration before which no stale reminder is ever issued — too early to judge. */
|
||||
private static final int STALE_WARMUP_ITERATIONS = 10;
|
||||
|
||||
/** Iteration past which an empty ledger triggers a "you should register steps" reminder. */
|
||||
private static final int EMPTY_LEDGER_NUDGE_ITERATIONS = 15;
|
||||
|
||||
/** Wall-clock gap that flips a non-empty ledger from "fresh" to "stale". */
|
||||
private static final long STALE_GAP_SECONDS = 90;
|
||||
|
||||
/**
|
||||
* Build a stale-reminder string for injection into the model's context
|
||||
* when the ledger appears to be falling behind the actual reasoning
|
||||
* progress. Returns {@code null} when the ledger is being maintained
|
||||
* normally so the caller can skip the injection.
|
||||
*
|
||||
* <p>Trigger heuristics — derived from round-4 of the LLM-review smoke
|
||||
* test, where the model stopped calling {@code progress_update} after
|
||||
* the first 30s and silently fell out of the ledger discipline:
|
||||
*
|
||||
* <ul>
|
||||
* <li><strong>Warm-up</strong>: {@code currentIteration < 10} → never
|
||||
* remind, the model is still setting up the task.</li>
|
||||
* <li><strong>Empty ledger</strong>: {@code currentIteration ≥ 15} and
|
||||
* no entries at all → likely a multi-step task being executed
|
||||
* without any ledger discipline.</li>
|
||||
* <li><strong>Stale updates</strong>: ledger has entries, but the
|
||||
* most recent {@code updatedAt} is > 90 s ago → ledger is no
|
||||
* longer tracking the real work.</li>
|
||||
* </ul>
|
||||
*
|
||||
* @param currentIteration the agent's current ReAct iteration count
|
||||
* @param now the reference instant for staleness ("now");
|
||||
* injected for testability
|
||||
* Iteration before which no stale reminder is ever issued — too early to judge.
|
||||
*/
|
||||
private static final int STALE_WARMUP_ITERATIONS = 3;
|
||||
private static final int EMPTY_LEDGER_NUDGE_ITERATIONS = 5;
|
||||
private static final long STALE_GAP_SECONDS = 45;
|
||||
|
||||
public String renderStaleReminder(int currentIteration, Instant now) {
|
||||
if (currentIteration < STALE_WARMUP_ITERATIONS) {
|
||||
return null;
|
||||
}
|
||||
if (entries.isEmpty()) {
|
||||
// Only regular (non-auto) entries indicate LLM engagement with the ledger.
|
||||
boolean hasRegularEntries = entries.values().stream()
|
||||
.anyMatch(e -> e.getKey() == null || !e.getKey().startsWith(AUTO_RECORDED_PREFIX));
|
||||
if (!hasRegularEntries) {
|
||||
if (currentIteration < EMPTY_LEDGER_NUDGE_ITERATIONS) {
|
||||
return null;
|
||||
}
|
||||
@ -114,15 +131,17 @@ public final class ProgressLedger {
|
||||
if (gap < STALE_GAP_SECONDS) {
|
||||
return null;
|
||||
}
|
||||
int done = (int) entries.values().stream()
|
||||
.filter(e -> e.getStatus() == ProgressStatus.DONE).count();
|
||||
int inProgress = (int) entries.values().stream()
|
||||
.filter(e -> e.getStatus() == ProgressStatus.IN_PROGRESS).count();
|
||||
long done = entries.values().stream()
|
||||
.filter(e -> isRegular(e) && e.getStatus() == ProgressStatus.DONE).count();
|
||||
long inProgress = entries.values().stream()
|
||||
.filter(e -> isRegular(e) && e.getStatus() == ProgressStatus.IN_PROGRESS).count();
|
||||
long pending = entries.values().stream()
|
||||
.filter(e -> isRegular(e) && e.getStatus() == ProgressStatus.PENDING).count();
|
||||
return "## ⚠️ 进度账本已 " + gap + " 秒未更新\n\n"
|
||||
+ "你已运行 " + currentIteration + " 轮,但 progress_update 已经 "
|
||||
+ gap + " 秒(约 " + (gap / 60) + " 分钟)没被调用过。\n"
|
||||
+ "当前账本:" + done + " done / " + inProgress + " in_progress / "
|
||||
+ (entries.size() - done - inProgress) + " pending。\n\n"
|
||||
+ pending + " pending。\n\n"
|
||||
+ "**立即做以下一件事**(不要再 read_file 或 browser_use,先更新账本):\n"
|
||||
+ "- 把已经完成的子步骤切到 `done`(如果你能看到工作区文件已生成)\n"
|
||||
+ "- 把正在做的步骤切到 `in_progress`\n"
|
||||
@ -130,22 +149,65 @@ public final class ProgressLedger {
|
||||
+ "不维护账本会导致重复工作 / 漏做项目 / 撞迭代上限。";
|
||||
}
|
||||
|
||||
/** True when the entry is regular (not auto-recorded, not pinned). */
|
||||
private boolean isRegular(ProgressEntry e) {
|
||||
return e.getKey() == null || !e.getKey().startsWith(AUTO_RECORDED_PREFIX);
|
||||
}
|
||||
|
||||
/**
|
||||
* @return a compact, model-readable progress snapshot, or {@code null}
|
||||
* when the ledger is empty so the caller can skip injection
|
||||
* entirely (no "(empty)" placeholder noise).
|
||||
*/
|
||||
public String renderSnapshot() {
|
||||
if (entries.isEmpty()) {
|
||||
if (isEmpty()) {
|
||||
return null;
|
||||
}
|
||||
List<ProgressEntry> done = bucket(ProgressStatus.DONE);
|
||||
List<ProgressEntry> inProgress = bucket(ProgressStatus.IN_PROGRESS);
|
||||
List<ProgressEntry> pending = bucket(ProgressStatus.PENDING);
|
||||
List<ProgressEntry> blocked = bucket(ProgressStatus.BLOCKED);
|
||||
|
||||
StringBuilder sb = new StringBuilder(256);
|
||||
sb.append("## 当前任务进度(执行参考,权威记录)\n\n");
|
||||
|
||||
// Pinned constraints (highest priority — always visible)
|
||||
if (!pinned.isEmpty()) {
|
||||
sb.append("🔒 固定约束(来自 skill,全程不可忽略):\n");
|
||||
for (ProgressEntry e : pinned.values()) {
|
||||
String label = e.getLabel() != null && !e.getLabel().isBlank()
|
||||
? e.getLabel() : e.getKey();
|
||||
sb.append("- ").append(label);
|
||||
String note = e.getNote();
|
||||
if (note != null && !note.isBlank()) {
|
||||
String trimmed = note.length() > NOTE_PREVIEW_CHARS
|
||||
? note.substring(0, NOTE_PREVIEW_CHARS) + "…"
|
||||
: note;
|
||||
sb.append(" — ").append(trimmed);
|
||||
}
|
||||
sb.append('\n');
|
||||
}
|
||||
sb.append('\n');
|
||||
}
|
||||
|
||||
// Auto-recorded tool completions (B5)
|
||||
List<ProgressEntry> autoRecorded = new ArrayList<>();
|
||||
for (ProgressEntry e : entries.values()) {
|
||||
if (e.getKey() != null && e.getKey().startsWith(AUTO_RECORDED_PREFIX)) {
|
||||
autoRecorded.add(e);
|
||||
}
|
||||
}
|
||||
if (!autoRecorded.isEmpty()) {
|
||||
sb.append("🔧 自动记录(工具调用完成):\n");
|
||||
for (ProgressEntry e : autoRecorded) {
|
||||
String label = e.getLabel() != null && !e.getLabel().isBlank()
|
||||
? e.getLabel() : e.getKey();
|
||||
sb.append("- ").append(label).append(" → ").append(e.getStatus());
|
||||
sb.append('\n');
|
||||
}
|
||||
sb.append('\n');
|
||||
}
|
||||
|
||||
// Regular entries grouped by status
|
||||
List<ProgressEntry> done = bucketRegular(ProgressStatus.DONE);
|
||||
List<ProgressEntry> inProgress = bucketRegular(ProgressStatus.IN_PROGRESS);
|
||||
List<ProgressEntry> pending = bucketRegular(ProgressStatus.PENDING);
|
||||
List<ProgressEntry> blocked = bucketRegular(ProgressStatus.BLOCKED);
|
||||
appendBucket(sb, "✅ 已完成", done);
|
||||
appendBucket(sb, "🔄 进行中", inProgress);
|
||||
appendBucket(sb, "⏳ 待办", pending);
|
||||
@ -155,10 +217,10 @@ public final class ProgressLedger {
|
||||
return sb.toString();
|
||||
}
|
||||
|
||||
private List<ProgressEntry> bucket(ProgressStatus status) {
|
||||
private List<ProgressEntry> bucketRegular(ProgressStatus status) {
|
||||
List<ProgressEntry> out = new ArrayList<>();
|
||||
for (ProgressEntry e : entries.values()) {
|
||||
if (e.getStatus() == status) {
|
||||
if (isRegular(e) && e.getStatus() == status) {
|
||||
out.add(e);
|
||||
}
|
||||
}
|
||||
|
||||
@ -12,6 +12,7 @@ import vip.mate.workspace.conversation.repository.ConversationMapper;
|
||||
|
||||
import java.time.Instant;
|
||||
import java.util.LinkedHashMap;
|
||||
import java.util.List;
|
||||
import java.util.Map;
|
||||
import java.util.concurrent.ConcurrentHashMap;
|
||||
import java.util.concurrent.locks.ReentrantLock;
|
||||
@ -24,53 +25,65 @@ import java.util.concurrent.locks.ReentrantLock;
|
||||
* Callers above it work with {@link ProgressLedger} (immutable view) or plain
|
||||
* {@code Map<String, ProgressEntry>}.
|
||||
*
|
||||
* <p>Three entry classes share the JSON column:
|
||||
* <ul>
|
||||
* <li><b>Regular entries</b> ({@code entries} map) — written by the LLM via
|
||||
* {@code progress_update} tool through {@link #upsert}.</li>
|
||||
* <li><b>Pinned entries</b> ({@code pinned} map) — written by Java via
|
||||
* {@link #upsertPinned} when {@code load_skill} extracts structured
|
||||
* constraints. Never touched by the LLM's {@code progress_update}.</li>
|
||||
* <li><b>Auto-recorded entries</b> — stored in the {@code entries} map with
|
||||
* a key prefixed by {@link ProgressLedger#AUTO_RECORDED_PREFIX}, written
|
||||
* by Java via {@link #upsertAutoRecorded} after successful tool calls.
|
||||
* Bounded to the most recent {@link #MAX_AUTO_RECORDED} entries.</li>
|
||||
* </ul>
|
||||
*
|
||||
* <p><b>JSON format</b> (backward-compatible): the new wrapper shape is
|
||||
* {@code {"entries": {...}, "pinned": {...}}}. Old conversations stored as
|
||||
* a flat map {@code {"step1": {...}}} are auto-migrated on first load —
|
||||
* the flat map is treated as {@code entries} with an empty {@code pinned}.
|
||||
*
|
||||
* <p>Failure mode: a malformed JSON value never throws back at the caller —
|
||||
* the runtime would rather render no snapshot than crash the reasoning loop
|
||||
* over a corrupted ledger column. Parse failures are logged at warn level so
|
||||
* the operator notices on a long-running deployment.
|
||||
* over a corrupted ledger column.
|
||||
*/
|
||||
@Service
|
||||
@RequiredArgsConstructor
|
||||
@Slf4j
|
||||
public class ProgressLedgerService {
|
||||
|
||||
/** Map<stepKey, ProgressEntry> — LinkedHashMap preserves insertion order in the rendered snapshot. */
|
||||
private static final TypeReference<LinkedHashMap<String, ProgressEntry>> LEDGER_TYPE =
|
||||
/** Map<stepKey, ProgressEntry> — LinkedHashMap preserves insertion order. */
|
||||
private static final TypeReference<LinkedHashMap<String, ProgressEntry>> ENTRIES_TYPE =
|
||||
new TypeReference<>() {};
|
||||
|
||||
/** Wrapper type for the new JSON format. */
|
||||
private static final TypeReference<LedgerWrapper> WRAPPER_TYPE = new TypeReference<>() {};
|
||||
|
||||
/** Maximum auto-recorded entries kept per conversation (risk mitigation). */
|
||||
public static final int MAX_AUTO_RECORDED = 5;
|
||||
|
||||
/**
|
||||
* Per-conversation lock for the load-mutate-save sequence inside
|
||||
* {@link #upsert}. Without this guard, a single agent turn that issues
|
||||
* N parallel {@code progress_update} tool calls (observed: 12 calls in
|
||||
* one batch when the model pre-registered every step at task start)
|
||||
* collapses to last-writer-wins, losing every entry but one — defeating
|
||||
* the whole point of the ledger. Different conversations stay
|
||||
* uncontended; only intra-conversation writes serialise.
|
||||
*
|
||||
* <p>Must be a {@link ReentrantLock}, not an intrinsic {@code synchronized}
|
||||
* monitor. Tool calls execute on virtual threads, and the critical section
|
||||
* spans blocking JDBC I/O (load + persist). A virtual thread that blocks —
|
||||
* whether on the DB call or while waiting to enter the lock — pins its
|
||||
* carrier when the lock is an intrinsic monitor. A turn that fires dozens
|
||||
* of parallel {@code progress_update} calls on the same conversation then
|
||||
* pins every carrier in the pool at once: the holder cannot be rescheduled
|
||||
* to release its connection and exit, JDBC connections are held past the
|
||||
* leak-detection threshold, and the whole server stops servicing requests.
|
||||
* {@code ReentrantLock} parks via {@code LockSupport}, which unmounts the
|
||||
* virtual thread and frees the carrier, so contention costs a park instead
|
||||
* of a pinned platform thread.
|
||||
*
|
||||
* <p>Entries are computed on demand and never explicitly removed; even
|
||||
* with thousands of long-running conversations the map stays bounded by
|
||||
* the active conversation set, and any leak is one lock per conversation
|
||||
* id — small enough to ignore relative to the rest of the per-conv state
|
||||
* already held in memory.
|
||||
* {@link #upsert}. See class Javadoc in ProgressLedger for the
|
||||
* virtual-thread pinning rationale.
|
||||
*/
|
||||
private final ConcurrentHashMap<String, ReentrantLock> upsertLocks = new ConcurrentHashMap<>();
|
||||
|
||||
private final ConversationMapper conversationMapper;
|
||||
private final ObjectMapper objectMapper;
|
||||
|
||||
/**
|
||||
* Wrapper for the persisted JSON. Both fields default to empty maps
|
||||
* so a partially-written JSON (e.g. only entries) still parses.
|
||||
*/
|
||||
public record LedgerWrapper(
|
||||
LinkedHashMap<String, ProgressEntry> entries,
|
||||
LinkedHashMap<String, ProgressEntry> pinned) {
|
||||
public LedgerWrapper() {
|
||||
this(new LinkedHashMap<>(), new LinkedHashMap<>());
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* @return the conversation's ledger, never null — an empty map when the
|
||||
* column is NULL or unparseable.
|
||||
@ -82,12 +95,6 @@ public class ProgressLedgerService {
|
||||
return parse(loadLedgerJson(conversationId));
|
||||
}
|
||||
|
||||
/**
|
||||
* Read the raw JSON column for one conversation, or {@code null} when
|
||||
* the row or column is empty. Protected so concurrency tests can
|
||||
* subclass and back the service with an in-memory map without having
|
||||
* to mock the Mybatis-Plus wrapper internals.
|
||||
*/
|
||||
protected String loadLedgerJson(String conversationId) {
|
||||
ConversationEntity row = conversationMapper.selectOne(
|
||||
new LambdaQueryWrapper<ConversationEntity>()
|
||||
@ -96,10 +103,6 @@ public class ProgressLedgerService {
|
||||
return row != null ? row.getProgressLedger() : null;
|
||||
}
|
||||
|
||||
/**
|
||||
* Write the raw JSON column for one conversation. Protected for the
|
||||
* same reason as {@link #loadLedgerJson}.
|
||||
*/
|
||||
protected void saveLedgerJson(String conversationId, String json) {
|
||||
conversationMapper.update(null,
|
||||
new LambdaUpdateWrapper<ConversationEntity>()
|
||||
@ -108,7 +111,8 @@ public class ProgressLedgerService {
|
||||
}
|
||||
|
||||
/**
|
||||
* Upsert one entry on the ledger atomically (load → mutate → save).
|
||||
* Upsert one regular entry on the ledger atomically (load → mutate → save).
|
||||
* Never touches pinned entries.
|
||||
*
|
||||
* @return the updated ledger so callers can render a fresh snapshot
|
||||
* without a second DB roundtrip.
|
||||
@ -121,52 +125,224 @@ public class ProgressLedgerService {
|
||||
if (key == null || key.isBlank()) {
|
||||
throw new IllegalArgumentException("step key is required");
|
||||
}
|
||||
// Guard reserved prefixes — the LLM must not overwrite Java-managed
|
||||
// entries (auto-recorded tool calls or pinned skill constraints).
|
||||
// Strip the prefix and continue with the remainder so the LLM's
|
||||
// progress_update still lands, just under a non-reserved key.
|
||||
if (key.startsWith(ProgressLedger.AUTO_RECORDED_PREFIX) || key.startsWith("pin_")) {
|
||||
throw new IllegalArgumentException(
|
||||
"step key prefix '" + ProgressLedger.AUTO_RECORDED_PREFIX
|
||||
+ "' / 'pin_' is reserved for system-managed entries; "
|
||||
+ "use a different key like 'step_<name>'");
|
||||
}
|
||||
if (status == null) {
|
||||
throw new IllegalArgumentException("status is required");
|
||||
}
|
||||
// Serialise the load-mutate-save sequence per conversation. Without
|
||||
// this, two parallel @Tool calls on the same conversation race: both
|
||||
// read the same starting state, each adds its own entry, and the
|
||||
// last save() drops the other's entry. Observed in production: a
|
||||
// 12-entry pre-registration collapsed to 8 because four sibling
|
||||
// tool calls landed in the same window.
|
||||
ReentrantLock lock = upsertLocks.computeIfAbsent(conversationId, k -> new ReentrantLock());
|
||||
lock.lock();
|
||||
try {
|
||||
ProgressLedger ledger = load(conversationId);
|
||||
Map<String, ProgressEntry> map = ledger.asMap();
|
||||
LedgerWrapper wrapper = loadWrapper(conversationId);
|
||||
Map<String, ProgressEntry> map = wrapper.entries;
|
||||
ProgressEntry existing = map.get(key);
|
||||
String effectiveLabel = (label != null && !label.isBlank())
|
||||
? label
|
||||
: (existing != null ? existing.getLabel() : key);
|
||||
map.put(key, new ProgressEntry(key, effectiveLabel, status, note, Instant.now()));
|
||||
persist(conversationId, map);
|
||||
return new ProgressLedger(map);
|
||||
persistWrapper(conversationId, wrapper);
|
||||
return new ProgressLedger(map, wrapper.pinned);
|
||||
} finally {
|
||||
lock.unlock();
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Upsert a pinned entry (Java-controlled, from skill constraints).
|
||||
* The LLM's {@code progress_update} tool never touches pinned entries.
|
||||
*
|
||||
* @param conversationId target conversation
|
||||
* @param key stable key, e.g. {@code pin_<skillName>_<index>}
|
||||
* @param label human-readable constraint text
|
||||
* @param note optional extra context
|
||||
*/
|
||||
public void upsertPinned(String conversationId, String key, String label, String note) {
|
||||
if (conversationId == null || conversationId.isBlank()) {
|
||||
throw new IllegalArgumentException("conversationId is required");
|
||||
}
|
||||
if (key == null || key.isBlank()) {
|
||||
throw new IllegalArgumentException("pinned key is required");
|
||||
}
|
||||
ReentrantLock lock = upsertLocks.computeIfAbsent(conversationId, k -> new ReentrantLock());
|
||||
lock.lock();
|
||||
try {
|
||||
LedgerWrapper wrapper = loadWrapper(conversationId);
|
||||
wrapper.pinned.put(key, new ProgressEntry(key, label, ProgressStatus.PENDING, note, Instant.now()));
|
||||
persistWrapper(conversationId, wrapper);
|
||||
} finally {
|
||||
lock.unlock();
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Remove all pinned entries whose key starts with the given prefix.
|
||||
* Used when a skill is unloaded or updated — the old constraints
|
||||
* should be cleared before re-injecting the new ones.
|
||||
*/
|
||||
public void clearPinnedByPrefix(String conversationId, String keyPrefix) {
|
||||
if (conversationId == null || conversationId.isBlank() || keyPrefix == null) {
|
||||
return;
|
||||
}
|
||||
ReentrantLock lock = upsertLocks.computeIfAbsent(conversationId, k -> new ReentrantLock());
|
||||
lock.lock();
|
||||
try {
|
||||
LedgerWrapper wrapper = loadWrapper(conversationId);
|
||||
wrapper.pinned.entrySet().removeIf(e -> e.getKey() != null && e.getKey().startsWith(keyPrefix));
|
||||
persistWrapper(conversationId, wrapper);
|
||||
} finally {
|
||||
lock.unlock();
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Auto-record a completed tool call as a ledger entry (B5). Uses the
|
||||
* {@link ProgressLedger#AUTO_RECORDED_PREFIX} on the key so the renderer
|
||||
* groups it separately. Bounds the total auto-recorded entries to
|
||||
* {@link #MAX_AUTO_RECORDED} by evicting the oldest.
|
||||
*
|
||||
* <p>Does NOT overwrite an existing entry with the same key — if the
|
||||
* LLM already tracked this step via {@code progress_update}, the LLM's
|
||||
* entry stays. This prevents Java from clobbering a richer LLM-authored
|
||||
* note.
|
||||
*
|
||||
* @param conversationId target conversation
|
||||
* @param toolName unique tool identifier used as the key suffix —
|
||||
* for MCP tools this should be the FULL prefixed
|
||||
* name ({@code mcp_<serverId>_<slug>_<hash6>}) to
|
||||
* avoid collisions between servers that have tools
|
||||
* with the same slug.
|
||||
* @param displayName human-readable label shown in the snapshot (e.g.
|
||||
* the slug portion only). Falls back to
|
||||
* {@code toolName} when null/blank.
|
||||
* @param resultSummary short tool-result excerpt; truncated to 120 chars
|
||||
*/
|
||||
public void upsertAutoRecorded(String conversationId, String toolName, String displayName,
|
||||
String resultSummary) {
|
||||
if (conversationId == null || conversationId.isBlank() || toolName == null || toolName.isBlank()) {
|
||||
return;
|
||||
}
|
||||
upsertAutoRecordedBatch(conversationId,
|
||||
List.of(new AutoRecordEntry(toolName, displayName, resultSummary)));
|
||||
}
|
||||
|
||||
/**
|
||||
* Input tuple for batch auto-record: the unique tool name (key suffix),
|
||||
* the readable display label, and the truncated result summary.
|
||||
*/
|
||||
public record AutoRecordEntry(String toolName, String displayName, String resultSummary) {}
|
||||
|
||||
/**
|
||||
* Batch version of {@link #upsertAutoRecorded} — processes multiple tool
|
||||
* results in a single lock + load + mutate + save cycle. Use this when
|
||||
* ActionNode receives a batch of parallel ToolResponses to avoid
|
||||
* serializing N lock acquisitions.
|
||||
*
|
||||
* <p>Skips entries whose {@code toolName} is null/blank or whose key
|
||||
* already exists in the ledger (LLM-authored entries are preserved).
|
||||
* Bounds the total auto-recorded entries to {@link #MAX_AUTO_RECORDED}
|
||||
* by evicting the oldest in bulk after inserting the new batch.
|
||||
*/
|
||||
public void upsertAutoRecordedBatch(String conversationId, List<AutoRecordEntry> entries) {
|
||||
if (conversationId == null || conversationId.isBlank()
|
||||
|| entries == null || entries.isEmpty()) {
|
||||
return;
|
||||
}
|
||||
ReentrantLock lock = upsertLocks.computeIfAbsent(conversationId, k -> new ReentrantLock());
|
||||
lock.lock();
|
||||
try {
|
||||
LedgerWrapper wrapper = loadWrapper(conversationId);
|
||||
Map<String, ProgressEntry> map = wrapper.entries;
|
||||
Instant now = Instant.now();
|
||||
for (AutoRecordEntry e : entries) {
|
||||
if (e == null || e.toolName() == null || e.toolName().isBlank()) {
|
||||
continue;
|
||||
}
|
||||
String key = ProgressLedger.AUTO_RECORDED_PREFIX + e.toolName();
|
||||
// Don't overwrite an LLM-authored entry (LLM wouldn't use the auto_ prefix).
|
||||
if (map.containsKey(key)) {
|
||||
continue;
|
||||
}
|
||||
String label = (e.displayName() != null && !e.displayName().isBlank())
|
||||
? e.displayName() : e.toolName();
|
||||
String note = e.resultSummary();
|
||||
if (note != null && note.length() > 120) {
|
||||
note = note.substring(0, 120) + "…";
|
||||
}
|
||||
map.put(key, new ProgressEntry(key, label, ProgressStatus.DONE, note, now));
|
||||
}
|
||||
// Bound auto-recorded entries: evict oldest in bulk if over limit.
|
||||
List<String> autoKeys = new java.util.ArrayList<>();
|
||||
for (String k : map.keySet()) {
|
||||
if (k != null && k.startsWith(ProgressLedger.AUTO_RECORDED_PREFIX)) {
|
||||
autoKeys.add(k);
|
||||
}
|
||||
}
|
||||
while (autoKeys.size() > MAX_AUTO_RECORDED && !autoKeys.isEmpty()) {
|
||||
String oldest = autoKeys.remove(0);
|
||||
map.remove(oldest);
|
||||
}
|
||||
persistWrapper(conversationId, wrapper);
|
||||
} finally {
|
||||
lock.unlock();
|
||||
}
|
||||
}
|
||||
|
||||
// ==================== Internal: load / parse / persist ====================
|
||||
|
||||
private ProgressLedger parse(String json) {
|
||||
if (json == null || json.isBlank() || "{}".equals(json.trim())) {
|
||||
return ProgressLedger.empty();
|
||||
}
|
||||
try {
|
||||
LinkedHashMap<String, ProgressEntry> map = objectMapper.readValue(json, LEDGER_TYPE);
|
||||
return new ProgressLedger(map);
|
||||
LedgerWrapper wrapper = parseWrapper(json);
|
||||
return new ProgressLedger(wrapper.entries, wrapper.pinned);
|
||||
} catch (Exception e) {
|
||||
log.warn("Failed to parse progress ledger JSON, treating as empty: {}", e.getMessage());
|
||||
return ProgressLedger.empty();
|
||||
}
|
||||
}
|
||||
|
||||
private void persist(String conversationId, Map<String, ProgressEntry> map) {
|
||||
/**
|
||||
* Parse with backward compatibility: new format has {@code "entries"}
|
||||
* and {@code "pinned"} keys; old format is a flat map treated as entries.
|
||||
*/
|
||||
private LedgerWrapper parseWrapper(String json) throws Exception {
|
||||
// Peek: if the JSON contains '"entries"' it's the new wrapper format.
|
||||
if (json.contains("\"entries\"")) {
|
||||
return objectMapper.readValue(json, WRAPPER_TYPE);
|
||||
}
|
||||
// Old format: flat map → migrate to wrapper with empty pinned.
|
||||
LinkedHashMap<String, ProgressEntry> entries = objectMapper.readValue(json, ENTRIES_TYPE);
|
||||
return new LedgerWrapper(entries, new LinkedHashMap<>());
|
||||
}
|
||||
|
||||
private LedgerWrapper loadWrapper(String conversationId) {
|
||||
String json = loadLedgerJson(conversationId);
|
||||
if (json == null || json.isBlank() || "{}".equals(json.trim())) {
|
||||
return new LedgerWrapper();
|
||||
}
|
||||
try {
|
||||
String json = objectMapper.writeValueAsString(map);
|
||||
return parseWrapper(json);
|
||||
} catch (Exception e) {
|
||||
log.warn("Failed to parse progress ledger JSON for {}, treating as empty: {}",
|
||||
conversationId, e.getMessage());
|
||||
return new LedgerWrapper();
|
||||
}
|
||||
}
|
||||
|
||||
private void persistWrapper(String conversationId, LedgerWrapper wrapper) {
|
||||
try {
|
||||
String json = objectMapper.writeValueAsString(wrapper);
|
||||
saveLedgerJson(conversationId, json);
|
||||
} catch (Exception e) {
|
||||
// Surface to caller so the tool can return an error message to
|
||||
// the LLM rather than silently dropping the update.
|
||||
throw new IllegalStateException(
|
||||
"Failed to persist progress ledger for " + conversationId + ": " + e.getMessage(), e);
|
||||
}
|
||||
|
||||
@ -0,0 +1,114 @@
|
||||
package vip.mate.agent.runtime;
|
||||
|
||||
import lombok.RequiredArgsConstructor;
|
||||
import lombok.extern.slf4j.Slf4j;
|
||||
import org.springframework.context.event.EventListener;
|
||||
import org.springframework.stereotype.Component;
|
||||
import vip.mate.skill.event.SkillRemovedEvent;
|
||||
import vip.mate.skill.event.SkillUpdatedEvent;
|
||||
import vip.mate.tool.mcp.event.McpConnectionLostEvent;
|
||||
import vip.mate.tool.mcp.event.McpServerChangedEvent;
|
||||
import vip.mate.tool.mcp.event.McpServerRemovedEvent;
|
||||
|
||||
import java.time.Instant;
|
||||
|
||||
/**
|
||||
* Bridges MCP / skill environment events into the agent runtime by translating
|
||||
* each event into an {@link EnvironmentNotification} and broadcasting it to
|
||||
* every currently-running conversation via {@link RunningConversationRegistry}.
|
||||
*
|
||||
* <p>This is the Java-side half of "agent environment awareness": instead of
|
||||
* expecting the LLM to notice that a tool disappeared (it won't — the tool
|
||||
* list is a static snapshot taken at turn start), Java detects the change
|
||||
* here and injects a one-shot notification into the agent's next reasoning
|
||||
* turn. The LLM only has to read and obey the notification; it does not have
|
||||
* to probe or guess.
|
||||
*
|
||||
* <p><b>Broadcast vs. targeted:</b> we broadcast to all active conversations
|
||||
* rather than filtering by "which agent has tools from this server". The
|
||||
* filtering would require a DB lookup per event (agent_tool_binding rows),
|
||||
* and the cost of a stray notification to an unaffected conversation is just
|
||||
* one extra SystemMessage — the LLM is told to ignore notifications about
|
||||
* tools it isn't using (see the "Environment Change Notifications" section
|
||||
* in the system prompt).
|
||||
*
|
||||
* <p>Coexists with the existing {@code @EventListener} methods in
|
||||
* {@code AgentService} which call {@code refreshAllAgents()} — those handle
|
||||
* cache invalidation so the NEXT turn sees fresh state; this router handles
|
||||
* in-flight notification so the CURRENT turn can adapt.
|
||||
*
|
||||
* @author MateClaw Team
|
||||
*/
|
||||
@Slf4j
|
||||
@Component
|
||||
@RequiredArgsConstructor
|
||||
public class EnvironmentEventRouter {
|
||||
|
||||
private final RunningConversationRegistry registry;
|
||||
|
||||
@EventListener
|
||||
public void onMcpServerChanged(McpServerChangedEvent event) {
|
||||
broadcast("mcp-changed",
|
||||
"🔧 MCP 工具列表已变更(原因: " + event.reason()
|
||||
+ ")。请重新检查可用工具列表,避免调用已失效的工具名。"
|
||||
+ "如果之前用过的工具现在不在列表里,说明它已被移除或重命名。");
|
||||
}
|
||||
|
||||
@EventListener
|
||||
public void onMcpConnectionLost(McpConnectionLostEvent event) {
|
||||
broadcast("mcp-lost",
|
||||
"⚠️ MCP 服务器连接丢失(serverId=" + event.serverId()
|
||||
+ ",原因: " + event.reason() + ")。"
|
||||
+ "该服务器下所有工具(前缀 mcp_" + event.serverId()
|
||||
+ "_)暂不可用。请改用其他工具,或向用户报告该能力暂时缺失。"
|
||||
+ "不要反复重试同一工具名。");
|
||||
}
|
||||
|
||||
@EventListener
|
||||
public void onMcpServerRemoved(McpServerRemovedEvent event) {
|
||||
broadcast("mcp-removed",
|
||||
"❌ MCP 服务器已移除(serverName=" + event.serverName()
|
||||
+ ",serverId=" + event.serverId() + ")。"
|
||||
+ "其下所有工具已永久失效,不要再尝试调用前缀 mcp_" + event.serverId()
|
||||
+ "_ 的任何工具。请改用其他途径完成任务。");
|
||||
}
|
||||
|
||||
@EventListener
|
||||
public void onSkillRemoved(SkillRemovedEvent event) {
|
||||
broadcast("skill-removed",
|
||||
"❌ Skill 已移除(" + event.skillName() + ")。"
|
||||
+ "如果之前加载过该 skill,其固定约束已失效;不要再尝试 load_skill 加载它。"
|
||||
+ "请基于剩余能力重新规划任务。");
|
||||
}
|
||||
|
||||
@EventListener
|
||||
public void onSkillUpdated(SkillUpdatedEvent event) {
|
||||
String verb = switch (event.changeType() == null ? "update" : event.changeType()) {
|
||||
case "enable" -> "已启用";
|
||||
case "disable" -> "已禁用";
|
||||
case "rescan" -> "安全扫描结果已更新";
|
||||
default -> "已更新";
|
||||
};
|
||||
broadcast("skill-updated",
|
||||
"🔄 Skill " + event.skillName() + " " + verb + "。"
|
||||
+ "如果之前加载过该 skill,请重新调用 load_skill 刷新其约束;"
|
||||
+ "旧约束可能不再适用,继续按旧约束执行可能导致错误。");
|
||||
}
|
||||
|
||||
// ==================== Internal ====================
|
||||
|
||||
private void broadcast(String type, String message) {
|
||||
try {
|
||||
int active = registry.activeConversations().size();
|
||||
if (active == 0) {
|
||||
return; // no-one to notify — skip the allocation
|
||||
}
|
||||
EnvironmentNotification n = new EnvironmentNotification(type, message, Instant.now());
|
||||
registry.broadcast(n);
|
||||
log.info("[EnvironmentEventRouter] Broadcasted {} to {} active conversation(s)", type, active);
|
||||
} catch (Exception e) {
|
||||
// Never let an event-routing failure bubble into the Spring event bus.
|
||||
log.warn("[EnvironmentEventRouter] Failed to broadcast {}: {}", type, e.getMessage());
|
||||
}
|
||||
}
|
||||
}
|
||||
@ -0,0 +1,27 @@
|
||||
package vip.mate.agent.runtime;
|
||||
|
||||
import java.time.Instant;
|
||||
|
||||
/**
|
||||
* A single environment-change notification destined for a running agent.
|
||||
*
|
||||
* <p>Produced by {@link EnvironmentEventRouter} when an MCP / skill event fires
|
||||
* during an in-flight conversation. Consumed by {@code ReasoningNode} (C4) which
|
||||
* drains the queue at the start of each reasoning turn and injects the
|
||||
* accumulated notifications as a single {@code SystemMessage} so the LLM sees
|
||||
* them alongside the progress ledger snapshot.
|
||||
*
|
||||
* <p>Notifications are ephemeral — they live only for the duration of a running
|
||||
* turn. If a conversation is not actively running when the event fires, the
|
||||
* notification is dropped (the next turn will rebuild the agent with fresh
|
||||
* state, so the LLM doesn't need a stale notification).
|
||||
*
|
||||
* @param type event category — one of {@code mcp-changed}, {@code mcp-lost},
|
||||
* {@code mcp-removed}, {@code skill-removed}, {@code skill-updated}
|
||||
* @param message human-readable, LLM-facing description of the change and
|
||||
* what the agent should do about it
|
||||
* @param timestamp when the event was observed by Java
|
||||
* @author MateClaw Team
|
||||
*/
|
||||
public record EnvironmentNotification(String type, String message, Instant timestamp) {
|
||||
}
|
||||
@ -0,0 +1,227 @@
|
||||
package vip.mate.agent.runtime;
|
||||
|
||||
import lombok.extern.slf4j.Slf4j;
|
||||
import org.springframework.scheduling.annotation.Scheduled;
|
||||
import org.springframework.stereotype.Component;
|
||||
|
||||
import java.time.Duration;
|
||||
import java.time.Instant;
|
||||
import java.util.ArrayList;
|
||||
import java.util.Collections;
|
||||
import java.util.List;
|
||||
import java.util.Set;
|
||||
import java.util.concurrent.ConcurrentHashMap;
|
||||
import java.util.concurrent.ConcurrentLinkedQueue;
|
||||
import java.util.concurrent.ConcurrentMap;
|
||||
|
||||
/**
|
||||
* Tracks which conversations are currently in-flight (a chat turn is actively
|
||||
* running) and holds a bounded per-conversation queue of pending environment
|
||||
* notifications.
|
||||
*
|
||||
* <p>This is the agent-side counterpart to {@code ChatStreamTracker} — but
|
||||
* whereas that tracker is SSE-pipeline-specific and lives in {@code channel.web},
|
||||
* this registry covers ALL chat entry points (sync {@code chat}, streaming
|
||||
* {@code chatStream}, {@code execute}, {@code chatWithReplay}) because it is
|
||||
* wired into {@code AgentService.withLifecycleSync/Flux} which every path
|
||||
* funnels through.
|
||||
*
|
||||
* <p><b>Lifecycle:</b>
|
||||
* <ul>
|
||||
* <li>{@link #register} is called when a turn starts (inside
|
||||
* {@code withLifecycleSync} / {@code withLifecycleFlux}).</li>
|
||||
* <li>{@link #unregister} is called when the turn ends (in {@code finally} /
|
||||
* {@code doFinally}).</li>
|
||||
* <li>Between turns the conversation is absent from the registry, so events
|
||||
* fired between turns are silently dropped — this is intentional: the
|
||||
* agent cache is invalidated by the existing {@code @EventListener}
|
||||
* methods in {@code AgentService}, so the next turn rebuilds with fresh
|
||||
* tool/skill state.</li>
|
||||
* </ul>
|
||||
*
|
||||
* <p><b>Queue bounds:</b> each conversation's notification queue is capped at
|
||||
* {@link #MAX_NOTIFICATIONS_PER_CONVERSATION}. When full, the oldest entry is
|
||||
* evicted. This prevents unbounded memory growth if events fire faster than
|
||||
* the agent consumes them (e.g. a tight MCP reconnection loop).
|
||||
*
|
||||
* <p>All operations are non-blocking and thread-safe.
|
||||
*
|
||||
* @author MateClaw Team
|
||||
*/
|
||||
@Slf4j
|
||||
@Component
|
||||
public class RunningConversationRegistry {
|
||||
|
||||
/** Max pending notifications per conversation before oldest evicts. */
|
||||
static final int MAX_NOTIFICATIONS_PER_CONVERSATION = 10;
|
||||
|
||||
private final ConcurrentMap<String, ConversationHandle> active = new ConcurrentHashMap<>();
|
||||
|
||||
/**
|
||||
* Mark a conversation as actively running. Idempotent — if already
|
||||
* registered (e.g. concurrent sub-agent delegation into the same
|
||||
* conversation), only refreshes {@code lastActiveAt}.
|
||||
*/
|
||||
public void register(String conversationId, Long agentId) {
|
||||
if (conversationId == null || conversationId.isBlank()) {
|
||||
return;
|
||||
}
|
||||
active.compute(conversationId, (k, existing) -> {
|
||||
Instant now = Instant.now();
|
||||
if (existing == null) {
|
||||
return new ConversationHandle(agentId, now, now, new ConcurrentLinkedQueue<>());
|
||||
}
|
||||
existing.lastActiveAt = now;
|
||||
return existing;
|
||||
});
|
||||
}
|
||||
|
||||
/**
|
||||
* Mark a conversation as no longer running. Safe to call multiple times
|
||||
* and on never-registered ids. Any pending notifications are discarded.
|
||||
*/
|
||||
public void unregister(String conversationId) {
|
||||
if (conversationId == null || conversationId.isBlank()) {
|
||||
return;
|
||||
}
|
||||
active.remove(conversationId);
|
||||
}
|
||||
|
||||
/** @return true iff a turn is currently in-flight for this conversation. */
|
||||
public boolean isActive(String conversationId) {
|
||||
return conversationId != null && active.containsKey(conversationId);
|
||||
}
|
||||
|
||||
/** @return a snapshot of all currently-running conversation ids. */
|
||||
public Set<String> activeConversations() {
|
||||
return Collections.unmodifiableSet(active.keySet());
|
||||
}
|
||||
|
||||
/**
|
||||
* Push a notification to a single conversation's queue. No-op if the
|
||||
* conversation is not active (event fired between turns).
|
||||
*/
|
||||
public void enqueue(String conversationId, EnvironmentNotification notification) {
|
||||
if (conversationId == null || notification == null) {
|
||||
return;
|
||||
}
|
||||
ConversationHandle handle = active.get(conversationId);
|
||||
if (handle == null) {
|
||||
return;
|
||||
}
|
||||
ConcurrentLinkedQueue<EnvironmentNotification> q = handle.notifications;
|
||||
while (q.size() >= MAX_NOTIFICATIONS_PER_CONVERSATION) {
|
||||
q.poll(); // evict oldest
|
||||
}
|
||||
q.offer(notification);
|
||||
}
|
||||
|
||||
/**
|
||||
* Push a notification to ALL currently-active conversations. Used by
|
||||
* {@link EnvironmentEventRouter} when an event is not conversation-scoped
|
||||
* (e.g. an MCP server disconnect affects every agent that has tools from
|
||||
* that server, and we don't have a cheap way to filter).
|
||||
*/
|
||||
public void broadcast(EnvironmentNotification notification) {
|
||||
if (notification == null) {
|
||||
return;
|
||||
}
|
||||
for (String convId : active.keySet()) {
|
||||
enqueue(convId, notification);
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Drain and return all pending notifications for a conversation. The
|
||||
* queue is emptied by this call — each notification is delivered at most
|
||||
* once. Returns an empty list for inactive / unknown conversations.
|
||||
*/
|
||||
public List<EnvironmentNotification> drain(String conversationId) {
|
||||
if (conversationId == null) {
|
||||
return List.of();
|
||||
}
|
||||
ConversationHandle handle = active.get(conversationId);
|
||||
if (handle == null) {
|
||||
return List.of();
|
||||
}
|
||||
List<EnvironmentNotification> out = new ArrayList<>();
|
||||
EnvironmentNotification n;
|
||||
while ((n = handle.notifications.poll()) != null) {
|
||||
out.add(n);
|
||||
}
|
||||
return out;
|
||||
}
|
||||
|
||||
// ==================== Stale-handle cleanup ====================
|
||||
|
||||
/**
|
||||
* Remove conversations whose {@code lastActiveAt} is older than
|
||||
* {@code maxAge} ago. Defensive cleanup for the case where
|
||||
* {@code unregister} was skipped due to an exception path that
|
||||
* bypassed the {@code finally}/{@code doFinally} guards.
|
||||
*
|
||||
* <p>Safe to call concurrently with {@link #register} /
|
||||
* {@link #unregister} — uses {@link ConcurrentHashMap#entrySet()}
|
||||
* iterator's weak consistency.
|
||||
*
|
||||
* @return the number of stale handles removed
|
||||
*/
|
||||
public int cleanupStale(Duration maxAge) {
|
||||
if (maxAge == null || maxAge.isNegative() || maxAge.isZero()) {
|
||||
return 0;
|
||||
}
|
||||
Instant cutoff = Instant.now().minus(maxAge);
|
||||
int removed = 0;
|
||||
for (var entry : active.entrySet()) {
|
||||
ConversationHandle handle = entry.getValue();
|
||||
if (handle != null && handle.lastActiveAt != null
|
||||
&& handle.lastActiveAt.isBefore(cutoff)) {
|
||||
// Use remove(key, value) to avoid removing a handle that was
|
||||
// just refreshed by a concurrent register() call.
|
||||
if (active.remove(entry.getKey(), handle)) {
|
||||
removed++;
|
||||
}
|
||||
}
|
||||
}
|
||||
if (removed > 0) {
|
||||
log.info("[RunningConversationRegistry] Cleaned up {} stale conversation handle(s) "
|
||||
+ "(older than {})", removed, maxAge);
|
||||
}
|
||||
return removed;
|
||||
}
|
||||
|
||||
/**
|
||||
* Periodic background sweep — runs every 5 minutes (1 minute initial
|
||||
* delay after startup). Removes handles inactive for more than 30
|
||||
* minutes, which almost certainly indicates a leaked registration
|
||||
* (normal turns complete in seconds to minutes).
|
||||
*
|
||||
* <p>The 30-minute threshold is intentionally generous: active
|
||||
* long-running conversations (e.g. a multi-hour research task) refresh
|
||||
* {@code lastActiveAt} on every iteration via {@link #register}, so
|
||||
* they won't be swept.
|
||||
*/
|
||||
@Scheduled(fixedDelay = 5 * 60 * 1000L, initialDelay = 60 * 1000L)
|
||||
public void scheduledCleanup() {
|
||||
try {
|
||||
cleanupStale(Duration.ofMinutes(30));
|
||||
} catch (Exception e) {
|
||||
log.warn("[RunningConversationRegistry] Scheduled cleanup failed: {}", e.getMessage());
|
||||
}
|
||||
}
|
||||
|
||||
// ==================== Internal handle ====================
|
||||
|
||||
private static final class ConversationHandle {
|
||||
final Long agentId;
|
||||
volatile Instant lastActiveAt;
|
||||
final ConcurrentLinkedQueue<EnvironmentNotification> notifications;
|
||||
|
||||
ConversationHandle(Long agentId, Instant startedAt, Instant lastActiveAt,
|
||||
ConcurrentLinkedQueue<EnvironmentNotification> notifications) {
|
||||
this.agentId = agentId;
|
||||
this.lastActiveAt = lastActiveAt;
|
||||
this.notifications = notifications;
|
||||
}
|
||||
}
|
||||
}
|
||||
@ -0,0 +1,27 @@
|
||||
package vip.mate.skill.event;
|
||||
|
||||
/**
|
||||
* Fires after a skill row has been updated (metadata, SKILL.md content,
|
||||
* security rescan, or enabled-flag toggle) so downstream listeners can
|
||||
* react to the new state.
|
||||
*
|
||||
* <p>Mirrors {@link SkillRemovedEvent} in shape, with an added
|
||||
* {@code changeType} hint so listeners can skip no-op refreshes. The
|
||||
* publisher is {@code SkillService} (which already holds the
|
||||
* {@code ApplicationEventPublisher}), avoiding a circular dependency on
|
||||
* {@code SkillRuntimeService} or {@code AgentService}.
|
||||
*
|
||||
* <p>Use cases:
|
||||
* <ul>
|
||||
* <li>Environment event routing (C3) notifies running conversations
|
||||
* that a skill's constraints or flow may have changed.</li>
|
||||
* <li>Agent cache invalidation so the next turn picks up the new
|
||||
* manifest.</li>
|
||||
* </ul>
|
||||
*
|
||||
* @param skillId DB id of the updated skill row
|
||||
* @param skillName slug identifier the row carries, useful for log lines
|
||||
* @param changeType {@code updated} | {@code toggled} | {@code rescanned}
|
||||
*/
|
||||
public record SkillUpdatedEvent(Long skillId, String skillName, String changeType) {
|
||||
}
|
||||
@ -91,6 +91,28 @@ public class SkillManifest {
|
||||
/** Set when {@code type=acp}. Resolves to a {@code mate_acp_endpoint} row. */
|
||||
private AcpBinding acp;
|
||||
|
||||
// ==================== Attention-anchoring constraints ====================
|
||||
|
||||
/**
|
||||
* Short, high-priority constraints extracted from SKILL.md that the
|
||||
* agent must obey throughout the task — e.g. "never delete user
|
||||
* files", "always confirm before writing", "use server B's fetch
|
||||
* tool, not server A's".
|
||||
*
|
||||
* <p>Unlike the full SKILL.md (which is a free-form document loaded
|
||||
* via {@code load_skill} and subject to context-window trimming),
|
||||
* these structured constraints are pinned into the ProgressLedger's
|
||||
* pinned-entries section by ActionNode on {@code load_skill}, so they
|
||||
* survive context compression and stay visible on every turn.
|
||||
*
|
||||
* <p>Empty list when the skill author didn't declare structured
|
||||
* constraints — the agent then falls back to the SKILL.md content
|
||||
* loaded via {@code load_skill} (protected by
|
||||
* {@code PRUNE_EXEMPT_TOOLS} in ConversationWindowManager).
|
||||
*/
|
||||
@Builder.Default
|
||||
private List<String> constraints = List.of();
|
||||
|
||||
// ==================== type=code script entrypoints ====================
|
||||
|
||||
/**
|
||||
|
||||
@ -41,6 +41,7 @@ public class SkillManifestParser {
|
||||
"knowledge",
|
||||
"acp",
|
||||
"scripts",
|
||||
"constraints",
|
||||
// legacy / housekeeping fields that aren't manifest-relevant
|
||||
"metadata"
|
||||
);
|
||||
@ -104,6 +105,7 @@ public class SkillManifestParser {
|
||||
.knowledge(parseKnowledge(fm.get("knowledge")))
|
||||
.acp(parseAcp(fm.get("acp")))
|
||||
.scripts(parseScripts(fm.get("scripts")))
|
||||
.constraints(stringList(fm.get("constraints")))
|
||||
.extras(extractUnknown(fm));
|
||||
|
||||
return b.build();
|
||||
|
||||
@ -499,8 +499,8 @@ public class SkillRuntimeService {
|
||||
sb.append("If a skill describes steps but ships no runnable script, write the code ");
|
||||
sb.append("its instructions describe and run it with ");
|
||||
sb.append("`execute_code(language=<python|bash|node>, code=..., skillName=<name>)`.\n\n");
|
||||
sb.append("| Skill | Status | Description |\n");
|
||||
sb.append("|-------|--------|-------------|\n");
|
||||
sb.append("| Skill | Status | Description | Constraints |\n");
|
||||
sb.append("|-------|--------|-------------|-------------|\n");
|
||||
for (ResolvedSkill skill : selected) {
|
||||
sb.append("| `").append(skill.getName()).append("`");
|
||||
if (skill.getIcon() != null && !skill.getIcon().isBlank()) {
|
||||
@ -516,6 +516,19 @@ public class SkillRuntimeService {
|
||||
// break the table layout.
|
||||
sb.append(desc.replace("|", "\\|").replace("\n", " "));
|
||||
}
|
||||
sb.append(" | ");
|
||||
// Only bound skills show constraints — they're the ones the agent
|
||||
// is configured to obey. Constraints are stable (agent lifecycle)
|
||||
// so they live in the prompt-cache-friendly static catalog segment,
|
||||
// immune to history compression. Non-bound skills (recommended /
|
||||
// recent) are "visible but optional" and don't carry constraints
|
||||
// the agent must follow.
|
||||
if (skill.getId() != null && boundIds.contains(skill.getId())
|
||||
&& skill.getManifest() != null
|
||||
&& skill.getManifest().getConstraints() != null
|
||||
&& !skill.getManifest().getConstraints().isEmpty()) {
|
||||
sb.append(renderConstraintsSummary(skill.getManifest().getConstraints()));
|
||||
}
|
||||
sb.append(" |\n");
|
||||
}
|
||||
if (selected.size() < visibleSkills.size()) {
|
||||
@ -524,6 +537,29 @@ public class SkillRuntimeService {
|
||||
.append(" available skills. Use `listAvailableSkills()` for the full catalog.\n");
|
||||
}
|
||||
|
||||
// Append a compact "bound skill tools" block so the model knows which
|
||||
// tools each bound skill allows — this is the other half of the skill
|
||||
// contract (constraints in the table above, allowedTools here). Kept
|
||||
// in the static catalog segment (prompt-cache-friendly, immune to
|
||||
// history compression) so the model always knows the tool boundary
|
||||
// for bound skills without needing to load_skill.
|
||||
List<ResolvedSkill> boundWithTools = selected.stream()
|
||||
.filter(s -> s.getId() != null && boundIds.contains(s.getId()))
|
||||
.filter(s -> s.getEffectiveAllowedTools() != null
|
||||
&& !s.getEffectiveAllowedTools().isEmpty())
|
||||
.toList();
|
||||
if (!boundWithTools.isEmpty()) {
|
||||
sb.append("\n### Bound skill allowed tools\n");
|
||||
for (ResolvedSkill skill : boundWithTools) {
|
||||
Set<String> tools = skill.getEffectiveAllowedTools();
|
||||
sb.append("- `").append(skill.getName()).append("`: ");
|
||||
sb.append(tools.stream()
|
||||
.map(t -> "`" + t + "`")
|
||||
.collect(java.util.stream.Collectors.joining(", ")));
|
||||
sb.append('\n');
|
||||
}
|
||||
}
|
||||
|
||||
List<ResolvedSkill> lessonSkills = sorted.stream()
|
||||
.filter(s -> (s.getId() != null && boundIds.contains(s.getId())) || recentNames.contains(s.getName()))
|
||||
.toList();
|
||||
@ -603,6 +639,30 @@ public class SkillRuntimeService {
|
||||
return 160;
|
||||
}
|
||||
|
||||
/**
|
||||
* Per-constraint char budget for the catalog table's Constraints column.
|
||||
* Kept short so the table stays compact — the full constraints are
|
||||
* available via load_skill / readSkillFile when the agent needs the
|
||||
* complete text. Multiple constraints are joined with "; " before
|
||||
* truncation.
|
||||
*/
|
||||
private static final int CONSTRAINTS_SUMMARY_LIMIT = 80;
|
||||
|
||||
/**
|
||||
* Render a compact one-line summary of a skill's constraints for the
|
||||
* catalog table. Multiple constraints are joined with "; "; the result
|
||||
* is truncated to {@link #CONSTRAINTS_SUMMARY_LIMIT} chars and
|
||||
* pipe/newline escaped so it doesn't break the table layout.
|
||||
*/
|
||||
static String renderConstraintsSummary(List<String> constraints) {
|
||||
if (constraints == null || constraints.isEmpty()) return "";
|
||||
String joined = String.join("; ", constraints);
|
||||
if (joined.length() > CONSTRAINTS_SUMMARY_LIMIT) {
|
||||
joined = joined.substring(0, CONSTRAINTS_SUMMARY_LIMIT) + "...";
|
||||
}
|
||||
return joined.replace("|", "\\|").replace("\n", " ");
|
||||
}
|
||||
|
||||
private static String statusToken(ResolvedSkill skill) {
|
||||
if (skill.isSecurityBlocked()) return "blocked";
|
||||
if (!skill.isEnabled()) return "disabled";
|
||||
|
||||
@ -9,6 +9,7 @@ import org.springframework.context.ApplicationEventPublisher;
|
||||
import org.springframework.stereotype.Service;
|
||||
import vip.mate.exception.MateClawException;
|
||||
import vip.mate.skill.event.SkillRemovedEvent;
|
||||
import vip.mate.skill.event.SkillUpdatedEvent;
|
||||
import vip.mate.skill.lifecycle.SkillLifecycleService;
|
||||
import vip.mate.skill.model.SkillEntity;
|
||||
import vip.mate.skill.repository.SkillFileMapper;
|
||||
@ -217,7 +218,11 @@ public class SkillService {
|
||||
"Skill runtime not initialized yet; retry in a moment");
|
||||
}
|
||||
runtimeService.rescanSingle(skill);
|
||||
return skillMapper.selectById(id);
|
||||
SkillEntity reloaded = skillMapper.selectById(id);
|
||||
// B6: notify running agents that this skill's manifest/security verdict
|
||||
// may have changed so they can re-load constraints and refresh tool ads.
|
||||
eventPublisher.publishEvent(new SkillUpdatedEvent(id, reloaded.getName(), "rescan"));
|
||||
return reloaded;
|
||||
}
|
||||
|
||||
/**
|
||||
@ -440,6 +445,8 @@ public class SkillService {
|
||||
runtimeService.refreshActiveSkills();
|
||||
}
|
||||
|
||||
// B6: notify running agents so they re-load this skill's constraints.
|
||||
eventPublisher.publishEvent(new SkillUpdatedEvent(existing.getId(), existing.getName(), "update"));
|
||||
return existing;
|
||||
}
|
||||
|
||||
@ -475,6 +482,8 @@ public class SkillService {
|
||||
runtimeService.refreshActiveSkills();
|
||||
}
|
||||
|
||||
// B6: notify running agents so they re-load this skill's constraints.
|
||||
eventPublisher.publishEvent(new SkillUpdatedEvent(existing.getId(), existing.getName(), "update"));
|
||||
return existing;
|
||||
}
|
||||
|
||||
@ -604,6 +613,9 @@ public class SkillService {
|
||||
runtimeService.refreshActiveSkills();
|
||||
}
|
||||
|
||||
// B6: notify running agents so they re-evaluate this skill's ads/constraints.
|
||||
eventPublisher.publishEvent(new SkillUpdatedEvent(skill.getId(), skill.getName(),
|
||||
enabled ? "enable" : "disable"));
|
||||
return skill;
|
||||
}
|
||||
|
||||
|
||||
@ -31,16 +31,20 @@ public class ProgressLedgerTool {
|
||||
private final ProgressLedgerService service;
|
||||
|
||||
@Tool(description = "Record or update a single step in the current conversation's progress "
|
||||
+ "ledger. Use this to track multi-step tasks (research workflows, document drafting "
|
||||
+ "split by section, etc.) — the runtime injects a rendered snapshot of the ledger "
|
||||
+ "into your context before every reasoning step so you never lose track of what is "
|
||||
+ "already done after a context trim. Call once per step transition: "
|
||||
+ "register pending entries up front when you decompose a task, mark in_progress "
|
||||
+ "before starting each one, then done as soon as it lands. Re-using the same stepKey "
|
||||
+ "overwrites the entry in place (no duplicates).")
|
||||
+ "ledger. Use this to track multi-step tasks (research workflows, document drafting "
|
||||
+ "split by section, etc.) — the runtime injects a rendered snapshot of the ledger "
|
||||
+ "into your context before every reasoning step so you never lose track of what is "
|
||||
+ "already done after a context trim. Call once per step transition: "
|
||||
+ "register pending entries up front when you decompose a task, mark in_progress "
|
||||
+ "before starting each one, then done as soon as it lands. Re-using the same stepKey "
|
||||
+ "overwrites the entry in place (no duplicates). "
|
||||
+ "IMPORTANT: do NOT use the `auto_` or `pin_` prefix in stepKey — those are reserved "
|
||||
+ "for system-managed entries (auto-recorded tool completions and pinned skill "
|
||||
+ "constraints) and will be rejected.")
|
||||
public String progress_update(
|
||||
@ToolParam(description = "Stable identifier for this step (e.g. 'model_gpt55', "
|
||||
+ "'section_intro', 'step_pptx'). Reuse exactly to update an existing entry.")
|
||||
+ "'section_intro', 'step_pptx'). Reuse exactly to update an existing entry. "
|
||||
+ "Do NOT prefix with 'auto_' or 'pin_' — those are system-reserved.")
|
||||
String stepKey,
|
||||
@ToolParam(description = "Human-readable label shown in the snapshot (e.g. "
|
||||
+ "'GPT-5.5 调研'). Pass empty to keep the existing label when updating.",
|
||||
|
||||
@ -92,7 +92,13 @@ public class DefaultToolDisclosureService implements ToolDisclosureService {
|
||||
}
|
||||
Long serverId = snap.mcpToolToServerId.get(toolName);
|
||||
if (serverId != null) {
|
||||
return snap.serverTierById.getOrDefault(serverId, DisclosureTier.CORE);
|
||||
// Move 5: MCP tools default to EXTENSION (on-demand exposure).
|
||||
// A server with 20 tools flooding the CORE tool list makes it
|
||||
// harder for the model to find the right builtin tool, and the
|
||||
// MCP schemas are typically the heaviest part of the prompt.
|
||||
// Users who want a server's tools visible by default can set
|
||||
// disclosure_tier = core on the mate_mcp_server row.
|
||||
return snap.serverTierById.getOrDefault(serverId, DisclosureTier.EXTENSION);
|
||||
}
|
||||
// Unknown source (ACP / dynamic-skill / plugin) — keep visible.
|
||||
return DisclosureTier.CORE;
|
||||
@ -132,10 +138,11 @@ public class DefaultToolDisclosureService implements ToolDisclosureService {
|
||||
* {@inheritDoc}
|
||||
*
|
||||
* <p>Protection set: {@link #ALWAYS_CORE} meta-tools and builtin tools
|
||||
* with an explicit {@code disclosure_tier = core} row. MCP tools remain
|
||||
* demotable — the server-level tier cannot distinguish an explicit core
|
||||
* choice from the default, and MCP schemas are typically the heaviest
|
||||
* part of the advertisement.
|
||||
* with an explicit {@code disclosure_tier = core} row. MCP tools default
|
||||
* to EXTENSION (Move 5) so they only enter the CORE list when an operator
|
||||
* explicitly sets {@code disclosure_tier = core} on the server — in that
|
||||
* case they are still demotable, since MCP schemas are typically the
|
||||
* heaviest part of the advertisement.
|
||||
*/
|
||||
@Override
|
||||
public Set<String> computeAutoDemotions(AgentToolSet baseSet, Integer budgetTokens) {
|
||||
@ -319,11 +326,18 @@ public class DefaultToolDisclosureService implements ToolDisclosureService {
|
||||
Map<Long, String> serverNameById = new LinkedHashMap<>();
|
||||
try {
|
||||
for (McpServerEntity s : mcpServerService.listAll()) {
|
||||
serverTierById.put(s.getId(), DisclosureTier.fromToken(s.getDisclosureTier()));
|
||||
// Move 5: only record an explicit tier. Servers with null
|
||||
// disclosure_tier are intentionally absent from the map so
|
||||
// resolveTierByName's getOrDefault(serverId, EXTENSION)
|
||||
// applies the new on-demand default. Putting CORE here
|
||||
// (via fromToken(null) → CORE) would override the default.
|
||||
if (s.getDisclosureTier() != null && !s.getDisclosureTier().isBlank()) {
|
||||
serverTierById.put(s.getId(), DisclosureTier.fromToken(s.getDisclosureTier()));
|
||||
}
|
||||
serverNameById.put(s.getId(), s.getName());
|
||||
}
|
||||
} catch (Exception e) {
|
||||
log.warn("ToolDisclosureService: failed to read MCP server tiers, defaulting to core: {}",
|
||||
log.warn("ToolDisclosureService: failed to read MCP server tiers, defaulting to extension: {}",
|
||||
e.getMessage());
|
||||
}
|
||||
|
||||
|
||||
@ -201,7 +201,7 @@ public class McpClientManager {
|
||||
McpIdentityForwardService idSvc =
|
||||
identityForwardService.forwardsTo(serverId, serverName) ? identityForwardService : null;
|
||||
String audience = idSvc != null ? identityForwardService.audienceFor(serverId, serverName) : null;
|
||||
List<ToolCallback> wrapped = wrapServerCallbacks(serverId, cbs, idSvc, audience);
|
||||
List<ToolCallback> wrapped = wrapServerCallbacks(serverId, cbs, idSvc, audience, serverName);
|
||||
lastGoodCallbacks.put(serverId, wrapped);
|
||||
allCallbacks.addAll(wrapped);
|
||||
continue;
|
||||
@ -253,7 +253,7 @@ public class McpClientManager {
|
||||
* real {@link McpSyncClient}.
|
||||
*/
|
||||
static List<ToolCallback> wrapServerCallbacks(long serverId, ToolCallback[] cbs) {
|
||||
return wrapServerCallbacks(serverId, cbs, null, null);
|
||||
return wrapServerCallbacks(serverId, cbs, null, null, null);
|
||||
}
|
||||
|
||||
/**
|
||||
@ -263,9 +263,13 @@ public class McpClientManager {
|
||||
* {@link McpIdentityForwardService} opt-in per server; {@code null}
|
||||
* means this server does not forward identity.
|
||||
* @param audience the token audience for this server (ignored in plaintext mode).
|
||||
* @param serverName human-readable MCP server name; forwarded into each
|
||||
* {@link PrefixedNameToolCallback} so the tool description is tagged
|
||||
* {@code [MCP server: <name>]}. May be {@code null} when unknown.
|
||||
*/
|
||||
static List<ToolCallback> wrapServerCallbacks(long serverId, ToolCallback[] cbs,
|
||||
McpIdentityForwardService identitySvc, String audience) {
|
||||
McpIdentityForwardService identitySvc, String audience,
|
||||
String serverName) {
|
||||
List<String> rawNames = new ArrayList<>(cbs.length);
|
||||
for (ToolCallback cb : cbs) {
|
||||
rawNames.add(cb.getToolDefinition() != null ? cb.getToolDefinition().name() : null);
|
||||
@ -294,7 +298,7 @@ public class McpClientManager {
|
||||
ToolCallback inner = identitySvc != null
|
||||
? new IdentityForwardingToolCallback(cb, identitySvc, audience)
|
||||
: cb;
|
||||
out.add(new PrefixedNameToolCallback(d.prefixedName(), inner));
|
||||
out.add(new PrefixedNameToolCallback(d.prefixedName(), inner, serverName));
|
||||
}
|
||||
return out;
|
||||
}
|
||||
|
||||
@ -21,13 +21,45 @@ import org.springframework.ai.tool.metadata.ToolMetadata;
|
||||
* forwarded verbatim — the wrapper changes only the name, so guard,
|
||||
* approval, observability, and return-direct routing all see the same
|
||||
* string they will write to bindings.
|
||||
*
|
||||
* <p>When {@code serverName} is provided (non-null, non-blank), the
|
||||
* description is prefixed with {@code [MCP server: <name>]} so the LLM
|
||||
* can identify which server a tool belongs to without parsing the
|
||||
* opaque numeric {@code serverId} in the tool name. This is critical
|
||||
* for tasks that mix tools from multiple MCP servers — without the
|
||||
* tag, the LLM cannot distinguish {@code mcp_1928..._search_xxx} from
|
||||
* {@code mcp_1882..._search_yyy} and will reconstruct wrong tool names
|
||||
* from memory. See agent-attention-anchoring design doc for details.
|
||||
*/
|
||||
public final class PrefixedNameToolCallback implements ToolCallback {
|
||||
|
||||
private final ToolCallback delegate;
|
||||
private final ToolDefinition prefixedDefinition;
|
||||
|
||||
/**
|
||||
* Backward-compatible constructor — equivalent to passing
|
||||
* {@code null} for {@code serverName} (no server tag in description).
|
||||
*
|
||||
* <p>Kept so existing tests and call sites that don't yet thread
|
||||
* the server name through continue to compile.
|
||||
*/
|
||||
public PrefixedNameToolCallback(String prefixedName, ToolCallback delegate) {
|
||||
this(prefixedName, delegate, null);
|
||||
}
|
||||
|
||||
/**
|
||||
* Primary constructor.
|
||||
*
|
||||
* @param prefixedName the {@code mcp_<serverId>_<slug>_<hash6>} name
|
||||
* @param delegate the underlying MCP tool callback
|
||||
* @param serverName human-readable MCP server name; when non-blank,
|
||||
* prepended to the description as
|
||||
* {@code [MCP server: <name>]} so the LLM can tell
|
||||
* tools from different servers apart. May be
|
||||
* {@code null} when the server name is unknown
|
||||
* (e.g. in unit tests).
|
||||
*/
|
||||
public PrefixedNameToolCallback(String prefixedName, ToolCallback delegate, String serverName) {
|
||||
if (prefixedName == null || prefixedName.isBlank()) {
|
||||
throw new IllegalArgumentException("prefixedName must not be blank");
|
||||
}
|
||||
@ -36,9 +68,16 @@ public final class PrefixedNameToolCallback implements ToolCallback {
|
||||
}
|
||||
this.delegate = delegate;
|
||||
ToolDefinition original = delegate.getToolDefinition();
|
||||
String originalDesc = original != null ? original.description() : "";
|
||||
if (originalDesc == null) {
|
||||
originalDesc = "";
|
||||
}
|
||||
String enrichedDesc = (serverName != null && !serverName.isBlank())
|
||||
? "[MCP server: " + serverName + "] " + originalDesc
|
||||
: originalDesc;
|
||||
this.prefixedDefinition = DefaultToolDefinition.builder()
|
||||
.name(prefixedName)
|
||||
.description(original != null ? original.description() : "")
|
||||
.description(enrichedDesc)
|
||||
.inputSchema(original != null ? original.inputSchema() : "{}")
|
||||
.build();
|
||||
}
|
||||
|
||||
@ -0,0 +1,261 @@
|
||||
package vip.mate.agent.context;
|
||||
|
||||
import org.junit.jupiter.api.DisplayName;
|
||||
import org.junit.jupiter.api.Test;
|
||||
import org.springframework.ai.chat.messages.AssistantMessage;
|
||||
import org.springframework.ai.chat.messages.Message;
|
||||
import org.springframework.ai.chat.messages.ToolResponseMessage;
|
||||
import org.springframework.ai.chat.messages.UserMessage;
|
||||
|
||||
import java.util.ArrayList;
|
||||
import java.util.List;
|
||||
|
||||
/**
|
||||
* New-vs-old compaction comparison harness.
|
||||
*
|
||||
* <p>This file is intentionally written to compile against BOTH:
|
||||
* <ul>
|
||||
* <li>{@code /data/mateclaw} — the new (post-six-moves) codebase</li>
|
||||
* <li>{@code /data/mateclaw/mateclaw-old} — the pre-six-moves codebase</li>
|
||||
* </ul>
|
||||
*
|
||||
* <p>It only uses APIs that exist in both:
|
||||
* <ul>
|
||||
* <li>{@code new ConversationWindowManager(null, null, null)}</li>
|
||||
* <li>{@code mgr.softTrimToolResults(messages)}</li>
|
||||
* <li>{@code mgr.hardClearToolResults(messages)}</li>
|
||||
* <li>{@code mgr.prePruneForSummary(messages)}</li>
|
||||
* <li>{@code TokenEstimator.estimateTokens(...)}</li>
|
||||
* </ul>
|
||||
*
|
||||
* <p>It does NOT use {@code isExemptTool} or {@code spillEvictToolResults}
|
||||
* (those don't exist on the old code). The same source file produces
|
||||
* DIFFERENT metrics on the two codebases — that difference IS the
|
||||
* demonstration that Move 4 works.
|
||||
*
|
||||
* <p>Each test prints a {@code [METRIC]} line to stdout in a stable
|
||||
* key=value format so the new-code run and old-code run can be diffed.
|
||||
*/
|
||||
class CompactionSurvivalComparisonTest {
|
||||
|
||||
// ==================== markers ====================
|
||||
|
||||
private static final String LOAD_SKILL_BODY_TEMPLATE =
|
||||
"[mate-skill-md]\n# skill-%d\nconstraints:\n- CONSTRAINT_MARKER_%d\n"
|
||||
+ "- Always confirm before writing\n"
|
||||
+ "- Never delete files outside /tmp\n"
|
||||
+ "- Use exactly 4-space indentation\n";
|
||||
|
||||
private static final String DELEGATE_BODY_TEMPLATE =
|
||||
"[sub-agent transcript %d]\nuser: list files\nassistant: DELEGATE_MARKER_%d done\n";
|
||||
|
||||
private static final String READ_FILE_BODY_TEMPLATE =
|
||||
"file content %d\n" + "x".repeat(1500) + "\nREADFILE_MARKER_%d\n";
|
||||
|
||||
// ==================== helpers ====================
|
||||
|
||||
private static ConversationWindowManager newManager() {
|
||||
return new ConversationWindowManager(null, null, null);
|
||||
}
|
||||
|
||||
private static ToolResponseMessage trm(String toolName, String body) {
|
||||
return ToolResponseMessage.builder()
|
||||
.responses(List.of(new ToolResponseMessage.ToolResponse(
|
||||
"call-" + toolName + "-" + System.nanoTime(), toolName, body)))
|
||||
.build();
|
||||
}
|
||||
|
||||
/**
|
||||
* Extract ALL text from a message, including {@link ToolResponseMessage}
|
||||
* response data (which {@code message.getText()} does NOT return).
|
||||
*/
|
||||
private static String extractAllText(Message m) {
|
||||
if (m == null) return "";
|
||||
if (m instanceof ToolResponseMessage trm) {
|
||||
StringBuilder sb = new StringBuilder();
|
||||
for (ToolResponseMessage.ToolResponse r : trm.getResponses()) {
|
||||
if (r.responseData() != null) sb.append(r.responseData());
|
||||
}
|
||||
return sb.toString();
|
||||
}
|
||||
String t = m.getText();
|
||||
return t == null ? "" : t;
|
||||
}
|
||||
|
||||
/**
|
||||
* Token counter that ALSO counts {@link ToolResponseMessage} response data
|
||||
* (the production {@code TokenEstimator.estimateTokens(Message)} only reads
|
||||
* {@code getText()}, which is null for tool responses).
|
||||
*/
|
||||
private static int realTokens(List<Message> messages) {
|
||||
int total = 0;
|
||||
for (Message m : messages) {
|
||||
total += TokenEstimator.estimateTokens(extractAllText(m))
|
||||
+ 4; // PER_MESSAGE_OVERHEAD
|
||||
}
|
||||
return total;
|
||||
}
|
||||
|
||||
private static int countSurvivingMarkers(List<Message> messages, String markerPrefix, int total) {
|
||||
int survived = 0;
|
||||
for (int i = 0; i < total; i++) {
|
||||
String marker = markerPrefix + i;
|
||||
boolean found = false;
|
||||
for (Message m : messages) {
|
||||
if (extractAllText(m).contains(marker)) {
|
||||
found = true;
|
||||
break;
|
||||
}
|
||||
}
|
||||
if (found) survived++;
|
||||
}
|
||||
return survived;
|
||||
}
|
||||
|
||||
private static void runAllThreePhases(ConversationWindowManager mgr, List<Message> messages) {
|
||||
mgr.softTrimToolResults(messages);
|
||||
mgr.hardClearToolResults(messages);
|
||||
mgr.prePruneForSummary(messages);
|
||||
}
|
||||
|
||||
// ==================== tests ====================
|
||||
|
||||
@Test
|
||||
@DisplayName("Scenario A: 50 load_skill + 50 delegate + 50 read_file, single full compaction")
|
||||
void scenarioA_bulkSurvivalRate() {
|
||||
ConversationWindowManager mgr = newManager();
|
||||
List<Message> messages = new ArrayList<>();
|
||||
messages.add(new UserMessage("Load the shopping skill and run a sub-agent, then read 50 files."));
|
||||
|
||||
int N = 50;
|
||||
for (int i = 0; i < N; i++) {
|
||||
messages.add(new AssistantMessage("loading skill " + i));
|
||||
messages.add(trm("load_skill", String.format(LOAD_SKILL_BODY_TEMPLATE, i, i)));
|
||||
messages.add(new AssistantMessage("delegating " + i));
|
||||
messages.add(trm("delegateToAgent", String.format(DELEGATE_BODY_TEMPLATE, i, i)));
|
||||
messages.add(new AssistantMessage("reading file " + i));
|
||||
messages.add(trm("read_file", String.format(READ_FILE_BODY_TEMPLATE, i, i)));
|
||||
}
|
||||
|
||||
int tokensBefore = realTokens(messages);
|
||||
long start = System.nanoTime();
|
||||
runAllThreePhases(mgr, messages);
|
||||
long elapsedMs = (System.nanoTime() - start) / 1_000_000;
|
||||
int tokensAfter = realTokens(messages);
|
||||
|
||||
int loadSkillSurvived = countSurvivingMarkers(messages, "CONSTRAINT_MARKER_", N);
|
||||
int delegateSurvived = countSurvivingMarkers(messages, "DELEGATE_MARKER_", N);
|
||||
int readFileSurvived = countSurvivingMarkers(messages, "READFILE_MARKER_", N);
|
||||
|
||||
System.out.printf(
|
||||
"[METRIC] scenario=A load_skill_survival=%d/%d delegate_survival=%d/%d "
|
||||
+ "readfile_survival=%d/%d tokens_before=%d tokens_after=%d time_ms=%d%n",
|
||||
loadSkillSurvived, N, delegateSurvived, N, readFileSurvived, N,
|
||||
tokensBefore, tokensAfter, elapsedMs);
|
||||
}
|
||||
|
||||
@Test
|
||||
@DisplayName("Scenario B: 100-round extreme compression, one load_skill pinned at the head")
|
||||
void scenarioB_hundredRoundExtremeCompression() {
|
||||
ConversationWindowManager mgr = newManager();
|
||||
List<Message> messages = new ArrayList<>();
|
||||
messages.add(new UserMessage("Load the shopping skill, then do 100 file-reads."));
|
||||
messages.add(new AssistantMessage("loading skill"));
|
||||
messages.add(trm("load_skill",
|
||||
"[mate-skill-md]\n# ckjia-shopping\nconstraints:\n"
|
||||
+ "- ROOT_CONSTRAINT_MARKER\n"
|
||||
+ "- Always confirm before writing\n"
|
||||
+ "- Never delete files outside /tmp\n"));
|
||||
|
||||
int rounds = 100;
|
||||
int tokensConsumedTotal = 0;
|
||||
long totalTimeMs = 0;
|
||||
for (int r = 0; r < rounds; r++) {
|
||||
messages.add(new AssistantMessage("reading file " + r));
|
||||
messages.add(trm("read_file", String.format(READ_FILE_BODY_TEMPLATE, r, r)));
|
||||
|
||||
int before = realTokens(messages);
|
||||
long start = System.nanoTime();
|
||||
runAllThreePhases(mgr, messages);
|
||||
long elapsed = (System.nanoTime() - start) / 1_000_000;
|
||||
int after = realTokens(messages);
|
||||
tokensConsumedTotal += (before - after);
|
||||
totalTimeMs += elapsed;
|
||||
}
|
||||
|
||||
int tokensFinal = realTokens(messages);
|
||||
boolean rootSurvived = messages.stream()
|
||||
.anyMatch(m -> extractAllText(m).contains("ROOT_CONSTRAINT_MARKER"));
|
||||
|
||||
System.out.printf(
|
||||
"[METRIC] scenario=B rounds=%d root_constraint_survived=%s tokens_final=%d "
|
||||
+ "tokens_consumed_by_compaction=%d total_compaction_time_ms=%d%n",
|
||||
rounds, rootSurvived, tokensFinal, tokensConsumedTotal, totalTimeMs);
|
||||
}
|
||||
|
||||
@Test
|
||||
@DisplayName("Scenario C: per-tool survival rate breakdown across 20 load_skill of varying body sizes")
|
||||
void scenarioC_perToolSurvivalBreakdown() {
|
||||
ConversationWindowManager mgr = newManager();
|
||||
List<Message> messages = new ArrayList<>();
|
||||
messages.add(new UserMessage("Load 20 skills of varying body sizes."));
|
||||
|
||||
int N = 20;
|
||||
int[] bodySizes = {100, 500, 1000, 2000, 5000};
|
||||
for (int i = 0; i < N; i++) {
|
||||
int size = bodySizes[i % bodySizes.length];
|
||||
StringBuilder body = new StringBuilder();
|
||||
body.append("[mate-skill-md]\n# skill-").append(i).append("\nconstraints:\n");
|
||||
body.append("- CONSTRAINT_MARKER_").append(i).append("\n");
|
||||
while (body.length() < size) body.append('x');
|
||||
messages.add(trm("load_skill", body.toString()));
|
||||
}
|
||||
|
||||
int tokensBefore = realTokens(messages);
|
||||
long start = System.nanoTime();
|
||||
runAllThreePhases(mgr, messages);
|
||||
long elapsedMs = (System.nanoTime() - start) / 1_000_000;
|
||||
int tokensAfter = realTokens(messages);
|
||||
|
||||
int survived = countSurvivingMarkers(messages, "CONSTRAINT_MARKER_", N);
|
||||
|
||||
System.out.printf(
|
||||
"[METRIC] scenario=C load_skill_count=%d survived=%d survival_rate=%.2f "
|
||||
+ "tokens_before=%d tokens_after=%d time_ms=%d%n",
|
||||
N, survived, (survived * 100.0 / N),
|
||||
tokensBefore, tokensAfter, elapsedMs);
|
||||
}
|
||||
|
||||
@Test
|
||||
@DisplayName("Scenario D: token consumption per round (steady-state)")
|
||||
void scenarioD_tokenConsumptionPerRound() {
|
||||
ConversationWindowManager mgr = newManager();
|
||||
List<Message> messages = new ArrayList<>();
|
||||
messages.add(new UserMessage("Run a long task with one pinned skill."));
|
||||
messages.add(trm("load_skill",
|
||||
"[mate-skill-md]\n# pinned\nconstraints:\n- PINNED_MARKER\n"));
|
||||
|
||||
int rounds = 30;
|
||||
StringBuilder perRound = new StringBuilder();
|
||||
for (int r = 0; r < rounds; r++) {
|
||||
messages.add(new AssistantMessage("step " + r));
|
||||
messages.add(trm("read_file", String.format(READ_FILE_BODY_TEMPLATE, r, r)));
|
||||
|
||||
int before = realTokens(messages);
|
||||
long start = System.nanoTime();
|
||||
runAllThreePhases(mgr, messages);
|
||||
long elapsed = (System.nanoTime() - start) / 1_000_000;
|
||||
int after = realTokens(messages);
|
||||
|
||||
if (r > 0) perRound.append(",");
|
||||
perRound.append(String.format("%d:%d:%d", r, before - after, elapsed));
|
||||
}
|
||||
|
||||
boolean pinnedSurvived = messages.stream()
|
||||
.anyMatch(m -> extractAllText(m).contains("PINNED_MARKER"));
|
||||
|
||||
System.out.printf(
|
||||
"[METRIC] scenario=D rounds=%d pinned_survived=%s per_round=round:tokens_consumed:time_ms{%s}%n",
|
||||
rounds, pinnedSurvived, perRound);
|
||||
}
|
||||
}
|
||||
@ -0,0 +1,312 @@
|
||||
package vip.mate.agent.context;
|
||||
|
||||
import org.junit.jupiter.api.BeforeEach;
|
||||
import org.junit.jupiter.api.Test;
|
||||
import org.springframework.ai.chat.messages.AssistantMessage;
|
||||
import org.springframework.ai.chat.messages.Message;
|
||||
import org.springframework.ai.chat.messages.ToolResponseMessage;
|
||||
import org.springframework.ai.chat.messages.UserMessage;
|
||||
import org.springframework.ai.chat.model.ChatModel;
|
||||
import org.springframework.ai.chat.model.ChatResponse;
|
||||
import org.springframework.ai.chat.model.Generation;
|
||||
import org.springframework.ai.chat.prompt.Prompt;
|
||||
import vip.mate.agent.progress.ProgressLedger;
|
||||
import vip.mate.agent.progress.ProgressLedgerService;
|
||||
import vip.mate.agent.progress.ProgressStatus;
|
||||
import vip.mate.config.ConversationWindowProperties;
|
||||
import com.fasterxml.jackson.databind.ObjectMapper;
|
||||
import com.fasterxml.jackson.datatype.jsr310.JavaTimeModule;
|
||||
|
||||
import java.util.ArrayList;
|
||||
import java.util.List;
|
||||
import java.util.Map;
|
||||
import java.util.concurrent.ConcurrentHashMap;
|
||||
|
||||
import static org.assertj.core.api.Assertions.assertThat;
|
||||
import static org.mockito.ArgumentMatchers.any;
|
||||
import static org.mockito.Mockito.mock;
|
||||
import static org.mockito.Mockito.when;
|
||||
|
||||
/**
|
||||
* Black-box test: verifies that ProgressLedger entries (pinned, auto-recorded,
|
||||
* regular) survive all four stages of ConversationWindowManager compression.
|
||||
*
|
||||
* <p>This is the key accuracy guarantee for the B-class changes:
|
||||
* <ul>
|
||||
* <li>B2 pins skill constraints into the ledger's {@code pinned} map</li>
|
||||
* <li>B3 stores the ledger in the DB, separate from the message list</li>
|
||||
* <li>ReasoningNode loads the ledger fresh each turn into
|
||||
* {@code nonHistoryPrefix}, which is NEVER touched by compaction</li>
|
||||
* </ul>
|
||||
*
|
||||
* <p>Contrast with A1 (PRUNE_EXEMPT_TOOLS): A1 tried to protect
|
||||
* {@code load_skill} tool results inside the {@code messages} list, but the
|
||||
* four-stage pipeline (Soft Trim / Hard Clear / Pre-Prune / LLM Summary) does
|
||||
* NOT honor PRUNE_EXEMPT_TOOLS — so the tool result body IS destroyed.
|
||||
* B2/B3 solves this by extracting constraints into the ledger BEFORE
|
||||
* compression can touch them.
|
||||
*/
|
||||
class ContextCompressionLedgerSurvivalTest {
|
||||
|
||||
private InMemoryProgressLedgerService ledgerService;
|
||||
private ConversationWindowManager manager;
|
||||
private ChatModel chatModel;
|
||||
|
||||
@BeforeEach
|
||||
void setUp() {
|
||||
ledgerService = new InMemoryProgressLedgerService();
|
||||
|
||||
ConversationWindowProperties props = new ConversationWindowProperties();
|
||||
props.setFirstUserAnchorEnabled(true);
|
||||
props.setFirstUserAnchorMaxTokens(400);
|
||||
manager = new ConversationWindowManager(props, null, null);
|
||||
|
||||
chatModel = mock(ChatModel.class);
|
||||
ChatResponse response = new ChatResponse(List.of(
|
||||
new Generation(new AssistantMessage("SUMMARY_OF_COMPRESSED_HISTORY"))));
|
||||
when(chatModel.call(any(Prompt.class))).thenReturn(response);
|
||||
}
|
||||
|
||||
// ==================== Core survival test ====================
|
||||
|
||||
@Test
|
||||
void allThreeLedgerEntryTypesSurviveCompression() {
|
||||
String convId = "conv-survival";
|
||||
|
||||
// 1. Pin constraints (simulating B2: ActionNode.load_skill → pinSkillConstraints)
|
||||
ledgerService.upsertPinned(convId, "pin_research_0",
|
||||
"🔒 research: Never fabricate citations", "Never fabricate citations");
|
||||
ledgerService.upsertPinned(convId, "pin_research_1",
|
||||
"🔒 research: Always cite primary sources", "Always cite primary sources");
|
||||
|
||||
// 2. Auto-record tool calls (simulating B5: ActionNode.autoRecordToolCalls)
|
||||
ledgerService.upsertAutoRecorded(convId, "web_search", "web_search", "found 5 results");
|
||||
ledgerService.upsertAutoRecorded(convId, "read_file", "read_file", "read paper.pdf");
|
||||
ledgerService.upsertAutoRecorded(convId, "write_file", "write_file", "wrote draft.md");
|
||||
|
||||
// 3. Regular entries (simulating LLM: progress_update)
|
||||
ledgerService.upsert(convId, "step_literature_review", "Literature Review",
|
||||
ProgressStatus.DONE, "surveyed 12 papers");
|
||||
ledgerService.upsert(convId, "step_draft_outline", "Draft Outline",
|
||||
ProgressStatus.IN_PROGRESS, "writing section 3");
|
||||
ledgerService.upsert(convId, "step_final_edit", "Final Edit",
|
||||
ProgressStatus.PENDING, null);
|
||||
|
||||
// 4. Build a long message history that triggers compression
|
||||
List<Message> history = buildLongHistoryWithLoadSkill(60);
|
||||
|
||||
// 5. Snapshot the ledger BEFORE compression
|
||||
ProgressLedger beforeLedger = ledgerService.load(convId);
|
||||
String beforeSnapshot = beforeLedger.renderSnapshot();
|
||||
assertThat(beforeSnapshot).contains("🔒 固定约束");
|
||||
assertThat(beforeSnapshot).contains("🔧 自动记录");
|
||||
assertThat(beforeSnapshot).contains("✅ 已完成");
|
||||
assertThat(beforeSnapshot).contains("🔄 进行中");
|
||||
assertThat(beforeSnapshot).contains("⏳ 待办");
|
||||
|
||||
// 6. Run PTL compression (the most aggressive: all 4 stages)
|
||||
List<Message> compacted = manager.compactForRetry(history, chatModel, convId, 1L);
|
||||
|
||||
// 7. The compacted message list should be shorter
|
||||
assertThat(compacted).hasSizeLessThan(history.size());
|
||||
|
||||
// 8. CRITICAL: The ledger is unaffected — it lives in the DB, not in messages
|
||||
ProgressLedger afterLedger = ledgerService.load(convId);
|
||||
assertThat(afterLedger.pinnedEntries()).hasSize(2);
|
||||
assertThat(afterLedger.asMap())
|
||||
.containsKeys("auto_web_search", "auto_read_file", "auto_write_file",
|
||||
"step_literature_review", "step_draft_outline", "step_final_edit");
|
||||
|
||||
// 9. The rendered snapshot is identical before and after compression
|
||||
String afterSnapshot = afterLedger.renderSnapshot();
|
||||
assertThat(afterSnapshot).isEqualTo(beforeSnapshot);
|
||||
}
|
||||
|
||||
// ==================== A1 gap proof: load_skill body destroyed, constraints survive ====================
|
||||
|
||||
@Test
|
||||
void loadSkillBodyDestroyedByCompressionButConstraintsSurviveInLedger() {
|
||||
String convId = "conv-a1-gap";
|
||||
|
||||
// B2: pin constraints from a loaded skill
|
||||
ledgerService.upsertPinned(convId, "pin_security_0",
|
||||
"🔒 security: Never run rm -rf", "Never run rm -rf");
|
||||
ledgerService.upsertPinned(convId, "pin_security_1",
|
||||
"🔒 security: Confirm before shell_exec", "Confirm before shell_exec");
|
||||
|
||||
// Build a history with a load_skill tool result (>200 chars, will be pruned)
|
||||
String loadSkillResult = "SKILL.md loaded successfully. This skill provides security auditing "
|
||||
+ "capabilities. " + "Constraint: Never run rm -rf. ".repeat(20)
|
||||
+ "Always confirm before shell_exec. " + "More padding ".repeat(20);
|
||||
|
||||
List<Message> history = new ArrayList<>();
|
||||
history.add(new UserMessage("Load the security skill"));
|
||||
history.add(AssistantMessage.builder()
|
||||
.content("Loading security skill")
|
||||
.toolCalls(List.of(new AssistantMessage.ToolCall(
|
||||
"call-1", "function", "load_skill", "{\"name\":\"security\"}")))
|
||||
.build());
|
||||
history.add(ToolResponseMessage.builder()
|
||||
.responses(List.of(new ToolResponseMessage.ToolResponse(
|
||||
"call-1", "load_skill", loadSkillResult)))
|
||||
.build());
|
||||
// Pad to trigger compression
|
||||
for (int i = 0; i < 50; i++) {
|
||||
history.add(new UserMessage("Turn " + i + " — ".repeat(50) + " filler content"));
|
||||
history.add(new AssistantMessage("Response " + i + " — ".repeat(50) + " filler response"));
|
||||
}
|
||||
|
||||
// Run compression
|
||||
List<Message> compacted = manager.compactForRetry(history, chatModel, convId, 1L);
|
||||
|
||||
// The load_skill tool result body in messages should have been pruned/replaced
|
||||
// (A1 gap: PRUNE_EXEMPT_TOOLS is not honored by the 4-stage pipeline)
|
||||
boolean loadSkillBodySurvived = compacted.stream()
|
||||
.filter(m -> m instanceof ToolResponseMessage)
|
||||
.map(m -> (ToolResponseMessage) m)
|
||||
.flatMap(trm -> trm.getResponses().stream())
|
||||
.anyMatch(r -> "load_skill".equals(r.name())
|
||||
&& r.responseData() != null
|
||||
&& r.responseData().contains("Never run rm -rf"));
|
||||
// The body was destroyed by compression (known A1 gap)
|
||||
assertThat(loadSkillBodySurvived).isFalse();
|
||||
|
||||
// BUT: the constraints pinned by B2 are in the ledger, which is unaffected
|
||||
ProgressLedger ledger = ledgerService.load(convId);
|
||||
assertThat(ledger.pinnedEntries()).hasSize(2);
|
||||
assertThat(ledger.renderSnapshot()).contains("Never run rm -rf");
|
||||
assertThat(ledger.renderSnapshot()).contains("Confirm before shell_exec");
|
||||
}
|
||||
|
||||
// ==================== Auto-recorded entries bounded after compression ====================
|
||||
|
||||
@Test
|
||||
void autoRecordedEntriesStayBoundedAfterManyToolCallsAndCompression() {
|
||||
String convId = "conv-bounded";
|
||||
|
||||
// Simulate 10 tool calls — auto-record should bound to MAX_AUTO_RECORDED
|
||||
for (int i = 0; i < 10; i++) {
|
||||
ledgerService.upsertAutoRecorded(convId, "tool_" + i, "tool_" + i, "result " + i);
|
||||
}
|
||||
|
||||
// Run compression on a minimal history (compression doesn't affect ledger)
|
||||
List<Message> history = new ArrayList<>();
|
||||
history.add(new UserMessage("Do task"));
|
||||
history.add(new AssistantMessage("Done"));
|
||||
manager.compactForRetry(history, chatModel, convId, 1L);
|
||||
|
||||
// Ledger still bounded
|
||||
ProgressLedger ledger = ledgerService.load(convId);
|
||||
long autoCount = ledger.asMap().keySet().stream()
|
||||
.filter(k -> k.startsWith(ProgressLedger.AUTO_RECORDED_PREFIX))
|
||||
.count();
|
||||
assertThat(autoCount).isEqualTo(ProgressLedgerService.MAX_AUTO_RECORDED);
|
||||
}
|
||||
|
||||
// ==================== Multiple compression cycles ====================
|
||||
|
||||
@Test
|
||||
void ledgerSurvivesMultipleCompressionCycles() {
|
||||
String convId = "conv-multi-cycle";
|
||||
|
||||
// Setup all three entry types
|
||||
ledgerService.upsertPinned(convId, "pin_skill_0", "🔒 skill: Rule", "Rule");
|
||||
ledgerService.upsertAutoRecorded(convId, "read_file", "read_file", "result");
|
||||
ledgerService.upsert(convId, "step_1", "Step 1", ProgressStatus.DONE, "done");
|
||||
|
||||
String snapshotBefore = ledgerService.load(convId).renderSnapshot();
|
||||
|
||||
// Run compression 3 times (simulating 3 user turns with PTL)
|
||||
List<Message> history = buildLongHistoryWithLoadSkill(40);
|
||||
for (int cycle = 0; cycle < 3; cycle++) {
|
||||
manager.compactForRetry(new ArrayList<>(history), chatModel, convId, 1L);
|
||||
}
|
||||
|
||||
// Ledger is still intact
|
||||
String snapshotAfter = ledgerService.load(convId).renderSnapshot();
|
||||
assertThat(snapshotAfter).isEqualTo(snapshotBefore);
|
||||
}
|
||||
|
||||
// ==================== Backward compat: old flat-map JSON ====================
|
||||
|
||||
@Test
|
||||
void oldFlatMapLedgerMigratesToWrapperFormatWithEmptyPinned() {
|
||||
String convId = "conv-legacy";
|
||||
// Write old-format JSON (flat map, no wrapper)
|
||||
String oldJson = "{\"step_1\":{\"key\":\"step_1\",\"label\":\"Step 1\","
|
||||
+ "\"status\":\"DONE\",\"note\":\"done\",\"updatedAt\":\"2025-01-01T00:00:00Z\"}}";
|
||||
ledgerService.store.put(convId, oldJson);
|
||||
|
||||
ProgressLedger ledger = ledgerService.load(convId);
|
||||
assertThat(ledger.asMap()).containsKey("step_1");
|
||||
assertThat(ledger.pinnedEntries()).isEmpty(); // migrated with empty pinned
|
||||
|
||||
// After an upsert, the JSON should be in wrapper format
|
||||
ledgerService.upsert(convId, "step_2", "Step 2", ProgressStatus.PENDING, null);
|
||||
String newJson = ledgerService.store.get(convId);
|
||||
assertThat(newJson).contains("\"entries\"");
|
||||
assertThat(newJson).contains("\"pinned\"");
|
||||
}
|
||||
|
||||
// ==================== Helpers ====================
|
||||
|
||||
/**
|
||||
* Build a long message history that includes a load_skill call/result,
|
||||
* enough to trigger the PTL compression path.
|
||||
*/
|
||||
private List<Message> buildLongHistoryWithLoadSkill(int fillerTurns) {
|
||||
List<Message> messages = new ArrayList<>();
|
||||
messages.add(new UserMessage("Load the research skill and do a literature review"));
|
||||
|
||||
// load_skill tool call + result
|
||||
messages.add(AssistantMessage.builder()
|
||||
.content("I'll load the research skill first")
|
||||
.toolCalls(List.of(new AssistantMessage.ToolCall(
|
||||
"call-ls", "function", "load_skill", "{\"name\":\"research\"}")))
|
||||
.build());
|
||||
messages.add(ToolResponseMessage.builder()
|
||||
.responses(List.of(new ToolResponseMessage.ToolResponse(
|
||||
"call-ls", "load_skill",
|
||||
"Skill 'research' loaded. Constraints: Never fabricate citations. "
|
||||
+ "Always cite primary sources. " + "Padding ".repeat(30))))
|
||||
.build());
|
||||
|
||||
// Filler turns to build up history
|
||||
for (int i = 0; i < fillerTurns; i++) {
|
||||
messages.add(new UserMessage("Question " + i + ": " + "x".repeat(200)));
|
||||
messages.add(AssistantMessage.builder()
|
||||
.content("Answer " + i + ": " + "y".repeat(200))
|
||||
.toolCalls(i % 5 == 0 ? List.of(new AssistantMessage.ToolCall(
|
||||
"call-" + i, "function", "web_search",
|
||||
"{\"q\":\"query-" + i + "\"}")) : List.of())
|
||||
.build());
|
||||
if (i % 5 == 0) {
|
||||
messages.add(ToolResponseMessage.builder()
|
||||
.responses(List.of(new ToolResponseMessage.ToolResponse(
|
||||
"call-" + i, "web_search", "Search result " + i + ": " + "z".repeat(300))))
|
||||
.build());
|
||||
}
|
||||
}
|
||||
return messages;
|
||||
}
|
||||
|
||||
// ==================== In-memory ledger service ====================
|
||||
|
||||
private static final class InMemoryProgressLedgerService extends ProgressLedgerService {
|
||||
final Map<String, String> store = new ConcurrentHashMap<>();
|
||||
|
||||
InMemoryProgressLedgerService() {
|
||||
super(null, new ObjectMapper().registerModule(new JavaTimeModule()));
|
||||
}
|
||||
|
||||
@Override
|
||||
protected String loadLedgerJson(String conversationId) {
|
||||
return store.get(conversationId);
|
||||
}
|
||||
|
||||
@Override
|
||||
protected void saveLedgerJson(String conversationId, String json) {
|
||||
store.put(conversationId, json);
|
||||
}
|
||||
}
|
||||
}
|
||||
@ -0,0 +1,300 @@
|
||||
package vip.mate.agent.context;
|
||||
|
||||
import org.junit.jupiter.api.DisplayName;
|
||||
import org.junit.jupiter.api.Test;
|
||||
import org.springframework.ai.chat.messages.AssistantMessage;
|
||||
import org.springframework.ai.chat.messages.Message;
|
||||
import org.springframework.ai.chat.messages.ToolResponseMessage;
|
||||
import org.springframework.ai.chat.messages.UserMessage;
|
||||
import vip.mate.agent.graph.executor.ToolResultStorage;
|
||||
|
||||
import java.util.ArrayList;
|
||||
import java.util.List;
|
||||
|
||||
import static org.junit.jupiter.api.Assertions.assertEquals;
|
||||
import static org.junit.jupiter.api.Assertions.assertFalse;
|
||||
import static org.junit.jupiter.api.Assertions.assertTrue;
|
||||
import static org.mockito.Mockito.mock;
|
||||
|
||||
/**
|
||||
* Move 4 — behavioral tests that would FAIL on the pre-Move-4 code.
|
||||
*
|
||||
* <p>Pre-Move-4 bugs being verified:
|
||||
* <ol>
|
||||
* <li>{@code softTrimToolResults}, {@code hardClearToolResults}, and
|
||||
* {@code prePruneForSummary} did NOT check {@code PRUNE_EXEMPT_TOOLS}.
|
||||
* A {@code load_skill} or {@code delegateToAgent} result would be
|
||||
* trimmed/cleared/pruned, silently dropping the skill's constraints
|
||||
* or the sub-agent's transcript.</li>
|
||||
* <li>There was no lossless spill-evict path before the LLM summary —
|
||||
* every over-budget conversation paid the LLM-summary token cost
|
||||
* even when spilling to disk would have sufficed.</li>
|
||||
* </ol>
|
||||
*
|
||||
* <p>Each test below asserts the NEW behavior. To confirm they would fail
|
||||
* on the old code, revert the Move 4 changes in ConversationWindowManager
|
||||
* and re-run — every test in this class should fail.
|
||||
*/
|
||||
class ConversationWindowManagerExemptAndSpillTest {
|
||||
|
||||
// Reuse the same constants the production code uses.
|
||||
private static final String LOAD_SKILL_BODY =
|
||||
"[mate-skill-md]\n# ckjia-shopping\nconstraints:\n- Always confirm before writing\n";
|
||||
private static final String DELEGATE_BODY =
|
||||
"[sub-agent transcript]\nuser: list files\nassistant: ...";
|
||||
private static final String READ_FILE_BODY = "x".repeat(2000);
|
||||
|
||||
private static ConversationWindowManager newManager() {
|
||||
// Constructor: (ConversationWindowProperties, MemoryManager, ConversationService)
|
||||
// — all can be null for the phases we test (softTrim/hardClear/prePrune
|
||||
// don't touch memory or conversation service).
|
||||
return new ConversationWindowManager(null, null, null);
|
||||
}
|
||||
|
||||
private static ToolResponseMessage trm(String toolName, String body) {
|
||||
return ToolResponseMessage.builder()
|
||||
.responses(List.of(new ToolResponseMessage.ToolResponse(
|
||||
"call-" + toolName, toolName, body)))
|
||||
.build();
|
||||
}
|
||||
|
||||
// ==================== Move 4.1: isExemptTool ====================
|
||||
|
||||
@Test
|
||||
@DisplayName("isExemptTool(load_skill) → true (NEW: method did not exist pre-Move-4)")
|
||||
void loadSkillIsExempt() {
|
||||
ToolResponseMessage.ToolResponse r = new ToolResponseMessage.ToolResponse(
|
||||
"id", "load_skill", "body");
|
||||
assertTrue(ConversationWindowManager.isExemptTool(r),
|
||||
"load_skill must be exempt — pre-Move-4 this method did not exist");
|
||||
}
|
||||
|
||||
@Test
|
||||
@DisplayName("isExemptTool(delegateToAgent) → true")
|
||||
void delegateIsExempt() {
|
||||
ToolResponseMessage.ToolResponse r = new ToolResponseMessage.ToolResponse(
|
||||
"id", "delegateToAgent", "body");
|
||||
assertTrue(ConversationWindowManager.isExemptTool(r));
|
||||
}
|
||||
|
||||
@Test
|
||||
@DisplayName("isExemptTool(delegateParallel) → true")
|
||||
void delegateParallelIsExempt() {
|
||||
ToolResponseMessage.ToolResponse r = new ToolResponseMessage.ToolResponse(
|
||||
"id", "delegateParallel", "body");
|
||||
assertTrue(ConversationWindowManager.isExemptTool(r));
|
||||
}
|
||||
|
||||
@Test
|
||||
@DisplayName("isExemptTool(read_file) → false (non-exempt tool)")
|
||||
void readFileIsNotExempt() {
|
||||
ToolResponseMessage.ToolResponse r = new ToolResponseMessage.ToolResponse(
|
||||
"id", "read_file", "body");
|
||||
assertFalse(ConversationWindowManager.isExemptTool(r));
|
||||
}
|
||||
|
||||
@Test
|
||||
@DisplayName("isExemptTool(null) → false (defensive)")
|
||||
void nullIsNotExempt() {
|
||||
assertFalse(ConversationWindowManager.isExemptTool(null));
|
||||
}
|
||||
|
||||
// ==================== Move 4.2: softTrimToolResults preserves exempt ====================
|
||||
|
||||
@Test
|
||||
@DisplayName("softTrimToolResults preserves load_skill body verbatim (would FAIL pre-Move-4)")
|
||||
void softTrimPreservesLoadSkillBody() {
|
||||
// Pre-Move-4: softTrimToolResults only checked isSpillMarker —
|
||||
// load_skill body would be truncated to ~400 chars, destroying
|
||||
// the skill constraints. Move 4 adds isExemptTool check.
|
||||
ConversationWindowManager mgr = newManager();
|
||||
List<Message> messages = new ArrayList<>(List.of(
|
||||
trm("load_skill", LOAD_SKILL_BODY),
|
||||
trm("read_file", READ_FILE_BODY)));
|
||||
|
||||
int trimmed = mgr.softTrimToolResults(messages);
|
||||
|
||||
// read_file WAS trimmed (non-exempt)
|
||||
assertTrue(trimmed >= 1, "non-exempt read_file should be trimmed");
|
||||
// load_skill body is UNCHANGED
|
||||
ToolResponseMessage loadSkillTrm = (ToolResponseMessage) messages.get(0);
|
||||
assertEquals(LOAD_SKILL_BODY, loadSkillTrm.getResponses().get(0).responseData(),
|
||||
"load_skill body must survive softTrim verbatim — "
|
||||
+ "pre-Move-4 this would have been truncated");
|
||||
assertTrue(loadSkillTrm.getResponses().get(0).responseData().contains(
|
||||
"Always confirm before writing"),
|
||||
"constraint text must survive — pre-Move-4 it was lost");
|
||||
}
|
||||
|
||||
@Test
|
||||
@DisplayName("softTrimToolResults preserves delegateToAgent body verbatim")
|
||||
void softTrimPreservesDelegateBody() {
|
||||
ConversationWindowManager mgr = newManager();
|
||||
List<Message> messages = new ArrayList<>(List.of(
|
||||
trm("delegateToAgent", DELEGATE_BODY)));
|
||||
|
||||
mgr.softTrimToolResults(messages);
|
||||
|
||||
ToolResponseMessage trm = (ToolResponseMessage) messages.get(0);
|
||||
assertEquals(DELEGATE_BODY, trm.getResponses().get(0).responseData(),
|
||||
"delegateToAgent body must survive softTrim verbatim");
|
||||
}
|
||||
|
||||
// ==================== Move 4.3: hardClearToolResults preserves exempt ====================
|
||||
|
||||
@Test
|
||||
@DisplayName("hardClearToolResults preserves load_skill body verbatim (would FAIL pre-Move-4)")
|
||||
void hardClearPreservesLoadSkillBody() {
|
||||
// Pre-Move-4: hardClearToolResults only checked isSpillMarker —
|
||||
// load_skill body would be replaced with "[旧工具输出已清理]".
|
||||
// This is the most destructive bypass: Phase 2 wipes the entire
|
||||
// skill constraints, then Phase 3 LLM summary can't reconstruct
|
||||
// them because they're already gone.
|
||||
ConversationWindowManager mgr = newManager();
|
||||
List<Message> messages = new ArrayList<>(List.of(
|
||||
trm("load_skill", LOAD_SKILL_BODY),
|
||||
trm("read_file", READ_FILE_BODY)));
|
||||
|
||||
int cleared = mgr.hardClearToolResults(messages);
|
||||
|
||||
// read_file WAS cleared (non-exempt)
|
||||
assertTrue(cleared >= 1, "non-exempt read_file should be cleared");
|
||||
// load_skill body is UNCHANGED
|
||||
ToolResponseMessage loadSkillTrm = (ToolResponseMessage) messages.get(0);
|
||||
assertEquals(LOAD_SKILL_BODY, loadSkillTrm.getResponses().get(0).responseData(),
|
||||
"load_skill body must survive hardClear verbatim — "
|
||||
+ "pre-Move-4 this would have been replaced with a placeholder");
|
||||
assertTrue(loadSkillTrm.getResponses().get(0).responseData().contains(
|
||||
"Always confirm before writing"),
|
||||
"constraint text must survive hardClear");
|
||||
}
|
||||
|
||||
@Test
|
||||
@DisplayName("hardClearToolResults preserves delegateToAgent body verbatim")
|
||||
void hardClearPreservesDelegateBody() {
|
||||
ConversationWindowManager mgr = newManager();
|
||||
List<Message> messages = new ArrayList<>(List.of(
|
||||
trm("delegateToAgent", DELEGATE_BODY)));
|
||||
|
||||
mgr.hardClearToolResults(messages);
|
||||
|
||||
ToolResponseMessage trm = (ToolResponseMessage) messages.get(0);
|
||||
assertEquals(DELEGATE_BODY, trm.getResponses().get(0).responseData(),
|
||||
"delegateToAgent body must survive hardClear verbatim");
|
||||
}
|
||||
|
||||
// ==================== Move 4.4: prePruneForSummary preserves exempt ====================
|
||||
|
||||
@Test
|
||||
@DisplayName("prePruneForSummary preserves load_skill body verbatim (would FAIL pre-Move-4)")
|
||||
void prePrunePreservesLoadSkillBody() {
|
||||
// Pre-Move-4: prePruneForSummary replaced ANY non-spill body >200 chars
|
||||
// with "[旧工具输出已清理以节省上下文空间]". load_skill's SKILL.md
|
||||
// snapshot is typically >200 chars, so it was ALWAYS pruned here.
|
||||
ConversationWindowManager mgr = newManager();
|
||||
List<Message> messages = new ArrayList<>(List.of(
|
||||
trm("load_skill", LOAD_SKILL_BODY),
|
||||
trm("read_file", READ_FILE_BODY)));
|
||||
|
||||
int pruned = mgr.prePruneForSummary(messages);
|
||||
|
||||
// read_file WAS pruned (non-exempt, >200 chars)
|
||||
assertTrue(pruned >= 1, "non-exempt read_file should be pruned");
|
||||
// load_skill body is UNCHANGED
|
||||
ToolResponseMessage loadSkillTrm = (ToolResponseMessage) messages.get(0);
|
||||
assertEquals(LOAD_SKILL_BODY, loadSkillTrm.getResponses().get(0).responseData(),
|
||||
"load_skill body must survive prePrune verbatim — "
|
||||
+ "pre-Move-4 this would have been replaced with a placeholder");
|
||||
}
|
||||
|
||||
@Test
|
||||
@DisplayName("prePruneForSummary skips ToolResponseMessage entirely when all responses are exempt")
|
||||
void prePruneSkipsAllExemptMessage() {
|
||||
ConversationWindowManager mgr = newManager();
|
||||
List<Message> messages = new ArrayList<>(List.of(
|
||||
trm("load_skill", LOAD_SKILL_BODY)));
|
||||
|
||||
int pruned = mgr.prePruneForSummary(messages);
|
||||
|
||||
assertEquals(0, pruned,
|
||||
"a ToolResponseMessage with only exempt responses must not be pruned at all");
|
||||
}
|
||||
|
||||
// ==================== Move 4.5: spillEvictToolResults (new method) ====================
|
||||
|
||||
@Test
|
||||
@DisplayName("spillEvictToolResults returns 0 when toolResultStorage is null (defensive)")
|
||||
void spillEvictNoStorage() {
|
||||
ConversationWindowManager mgr = newManager();
|
||||
List<Message> messages = new ArrayList<>(List.of(
|
||||
trm("read_file", READ_FILE_BODY)));
|
||||
int spilled = mgr.spillEvictToolResults(messages, "conv-1", "/tmp/ws");
|
||||
assertEquals(0, spilled, "null toolResultStorage must no-op");
|
||||
}
|
||||
|
||||
@Test
|
||||
@DisplayName("spillEvictToolResults returns 0 when conversationId is null")
|
||||
void spillEvictNoConversationId() {
|
||||
// Even with storage wired, null conversationId must no-op to avoid
|
||||
// writing spill files to a meaningless path.
|
||||
ConversationWindowManager mgr = newManagerWithStorage(mock(ToolResultStorage.class));
|
||||
List<Message> messages = new ArrayList<>(List.of(
|
||||
trm("read_file", READ_FILE_BODY)));
|
||||
int spilled = mgr.spillEvictToolResults(messages, null, "/tmp/ws");
|
||||
assertEquals(0, spilled);
|
||||
}
|
||||
|
||||
@Test
|
||||
@DisplayName("spillEvictToolResults skips exempt tools (load_skill not spilled)")
|
||||
void spillEvictSkipsExempt() {
|
||||
// Exempt tools must never be spilled — their content is not
|
||||
// safely recoverable (load_skill returns a snapshot that may
|
||||
// have been edited since).
|
||||
ToolResultStorage storage = mock(ToolResultStorage.class);
|
||||
ConversationWindowManager mgr = newManagerWithStorage(storage);
|
||||
List<Message> messages = new ArrayList<>(List.of(
|
||||
trm("load_skill", LOAD_SKILL_BODY)));
|
||||
|
||||
int spilled = mgr.spillEvictToolResults(messages, "conv-1", "/tmp/ws");
|
||||
|
||||
assertEquals(0, spilled, "load_skill must not be spilled");
|
||||
}
|
||||
|
||||
// ==================== Move 4.6: Phase 2.7 integration ====================
|
||||
|
||||
@Test
|
||||
@DisplayName("Move 4 invariant: exempt tools survive ALL three phases unmodified")
|
||||
void exemptToolsSurviveAllPhases() {
|
||||
// This is the integration test: run softTrim → hardClear → prePrune
|
||||
// in sequence (same order as compactMessages) and verify the
|
||||
// load_skill body is still intact at the end.
|
||||
ConversationWindowManager mgr = newManager();
|
||||
List<Message> messages = new ArrayList<>(List.of(
|
||||
new UserMessage("load the shopping skill"),
|
||||
trm("load_skill", LOAD_SKILL_BODY),
|
||||
new AssistantMessage("Now let me read a file"),
|
||||
trm("read_file", READ_FILE_BODY)));
|
||||
|
||||
mgr.softTrimToolResults(messages);
|
||||
mgr.hardClearToolResults(messages);
|
||||
mgr.prePruneForSummary(messages);
|
||||
|
||||
ToolResponseMessage loadSkillTrm = (ToolResponseMessage) messages.get(1);
|
||||
assertEquals(LOAD_SKILL_BODY, loadSkillTrm.getResponses().get(0).responseData(),
|
||||
"load_skill body must survive ALL three phases — "
|
||||
+ "this is the core Move 4 fix for 'compression causes attention failure'");
|
||||
|
||||
// read_file WAS modified (cleared to placeholder)
|
||||
ToolResponseMessage readFileTrm = (ToolResponseMessage) messages.get(3);
|
||||
assertFalse(readFileTrm.getResponses().get(0).responseData().equals(READ_FILE_BODY),
|
||||
"non-exempt read_file should have been modified by at least one phase");
|
||||
}
|
||||
|
||||
// ==================== helpers ====================
|
||||
|
||||
private static ConversationWindowManager newManagerWithStorage(ToolResultStorage storage) {
|
||||
ConversationWindowManager mgr = new ConversationWindowManager(null, null, null);
|
||||
mgr.setToolResultStorage(storage);
|
||||
return mgr;
|
||||
}
|
||||
}
|
||||
@ -0,0 +1,255 @@
|
||||
package vip.mate.agent.graph.node;
|
||||
|
||||
import org.junit.jupiter.api.BeforeEach;
|
||||
import org.junit.jupiter.api.Test;
|
||||
import org.springframework.ai.chat.messages.Message;
|
||||
import org.springframework.ai.chat.messages.SystemMessage;
|
||||
import vip.mate.agent.runtime.EnvironmentEventRouter;
|
||||
import vip.mate.agent.runtime.EnvironmentNotification;
|
||||
import vip.mate.agent.runtime.RunningConversationRegistry;
|
||||
import vip.mate.skill.event.SkillUpdatedEvent;
|
||||
import vip.mate.tool.mcp.event.McpConnectionLostEvent;
|
||||
import vip.mate.tool.mcp.event.McpServerChangedEvent;
|
||||
import vip.mate.tool.mcp.event.McpServerRemovedEvent;
|
||||
|
||||
import java.time.Instant;
|
||||
import java.util.List;
|
||||
|
||||
import static org.assertj.core.api.Assertions.assertThat;
|
||||
|
||||
/**
|
||||
* Black-box test for the C-class environment-awareness pipeline:
|
||||
* MCP / skill event fires → {@link EnvironmentEventRouter} translates it →
|
||||
* {@link RunningConversationRegistry} queues it → next reasoning turn drains
|
||||
* the queue → {@link ReasoningNode#renderEnvironmentNotifications} renders
|
||||
* the LLM-visible block.
|
||||
*
|
||||
* <p>Verifies the LLM actually receives <b>actionable</b> text on the next
|
||||
* turn — not just that an event was queued. This is the user-facing
|
||||
* contract: the agent must be told, in plain language, which tool prefix
|
||||
* broke, which skill changed, and what to do about it.
|
||||
*
|
||||
* <p>The rendering helper is exercised via the real production static
|
||||
* method on {@link ReasoningNode} (package-private for testability), so a
|
||||
* format regression in the helper fails this test rather than a duplicate.
|
||||
*/
|
||||
class EnvironmentNotificationRenderingTest {
|
||||
|
||||
private RunningConversationRegistry registry;
|
||||
private EnvironmentEventRouter router;
|
||||
|
||||
@BeforeEach
|
||||
void setUp() {
|
||||
registry = new RunningConversationRegistry();
|
||||
router = new EnvironmentEventRouter(registry);
|
||||
}
|
||||
|
||||
// ==================== Render helper contract ====================
|
||||
|
||||
@Test
|
||||
void renderNullReturnsNull() {
|
||||
assertThat(ReasoningNode.renderEnvironmentNotifications(null)).isNull();
|
||||
}
|
||||
|
||||
@Test
|
||||
void renderEmptyReturnsNull() {
|
||||
// Empty drain should NOT inject "(no notifications)" noise — the
|
||||
// caller checks for null and skips the SystemMessage entirely.
|
||||
assertThat(ReasoningNode.renderEnvironmentNotifications(List.of())).isNull();
|
||||
}
|
||||
|
||||
@Test
|
||||
void renderSingleNotificationHasHeaderAndDirective() {
|
||||
EnvironmentNotification n = new EnvironmentNotification(
|
||||
"mcp-lost", "⚠️ server 7 down", Instant.now());
|
||||
|
||||
String block = ReasoningNode.renderEnvironmentNotifications(List.of(n));
|
||||
|
||||
assertThat(block).isNotNull();
|
||||
assertThat(block).contains("📢 环境变更通知");
|
||||
assertThat(block).contains("⚠️ server 7 down");
|
||||
// Authority + directive tail must be present — the LLM is told this
|
||||
// is Java-injected truth, not a heuristic, and must adapt.
|
||||
assertThat(block).contains("Java 运行时检测");
|
||||
assertThat(block).contains("立即据此调整计划");
|
||||
}
|
||||
|
||||
@Test
|
||||
void renderMultipleNotificationsAllVisible() {
|
||||
List<EnvironmentNotification> notes = List.of(
|
||||
new EnvironmentNotification("mcp-lost", "⚠️ server 7 down", Instant.now()),
|
||||
new EnvironmentNotification("skill-updated", "🔄 web-scraper updated", Instant.now()),
|
||||
new EnvironmentNotification("mcp-removed", "❌ server 3 removed", Instant.now()));
|
||||
|
||||
String block = ReasoningNode.renderEnvironmentNotifications(notes);
|
||||
|
||||
assertThat(block).isNotNull();
|
||||
assertThat(block).contains("⚠️ server 7 down");
|
||||
assertThat(block).contains("🔄 web-scraper updated");
|
||||
assertThat(block).contains("❌ server 3 removed");
|
||||
// Each notification renders as its own bullet
|
||||
assertThat(block.split("\n")).anyMatch(line -> line.trim().startsWith("- ⚠️"));
|
||||
}
|
||||
|
||||
// ==================== End-to-end: event → drain → render ====================
|
||||
|
||||
@Test
|
||||
void mcpConnectionLostEventEndToEnd_producesActionableLLMText() {
|
||||
registry.register("conv-1", 42L);
|
||||
router.onMcpConnectionLost(new McpConnectionLostEvent(7L, "stdio-process-exited"));
|
||||
|
||||
List<EnvironmentNotification> notes = registry.drain("conv-1");
|
||||
String block = ReasoningNode.renderEnvironmentNotifications(notes);
|
||||
|
||||
assertThat(block).isNotNull();
|
||||
// Critical actionable info: serverId, tool prefix, explicit "don't retry"
|
||||
assertThat(block).contains("7");
|
||||
assertThat(block).contains("mcp_7_");
|
||||
assertThat(block).contains("不要反复重试");
|
||||
}
|
||||
|
||||
@Test
|
||||
void mcpServerRemovedEventEndToEnd_producesActionableLLMText() {
|
||||
registry.register("conv-1", 42L);
|
||||
router.onMcpServerRemoved(new McpServerRemovedEvent(3L, "fetch-server"));
|
||||
|
||||
String block = ReasoningNode.renderEnvironmentNotifications(registry.drain("conv-1"));
|
||||
|
||||
assertThat(block).isNotNull();
|
||||
assertThat(block).contains("fetch-server");
|
||||
assertThat(block).contains("mcp_3_");
|
||||
assertThat(block).contains("永久失效");
|
||||
}
|
||||
|
||||
@Test
|
||||
void mcpServerChangedEventEndToEnd_producesActionableLLMText() {
|
||||
registry.register("conv-1", 42L);
|
||||
router.onMcpServerChanged(new McpServerChangedEvent("mcp-rescan-complete"));
|
||||
|
||||
String block = ReasoningNode.renderEnvironmentNotifications(registry.drain("conv-1"));
|
||||
|
||||
assertThat(block).isNotNull();
|
||||
assertThat(block).contains("MCP 工具列表已变更");
|
||||
assertThat(block).contains("mcp-rescan-complete");
|
||||
}
|
||||
|
||||
@Test
|
||||
void skillUpdatedEventEndToEnd_producesActionableLLMText() {
|
||||
registry.register("conv-1", 42L);
|
||||
router.onSkillUpdated(new SkillUpdatedEvent(10L, "web-scraper", "update"));
|
||||
|
||||
String block = ReasoningNode.renderEnvironmentNotifications(registry.drain("conv-1"));
|
||||
|
||||
assertThat(block).isNotNull();
|
||||
assertThat(block).contains("web-scraper");
|
||||
assertThat(block).contains("已更新");
|
||||
// The LLM is told to re-load_skill to refresh constraints
|
||||
assertThat(block).contains("load_skill");
|
||||
}
|
||||
|
||||
@Test
|
||||
void multipleEventsFiredMidTurnAllReachNextReasoningTurn() {
|
||||
// Simulate a chaotic mid-turn environment: 3 changes fire while the
|
||||
// agent is mid-tool-call. The next reasoning turn must see ALL of
|
||||
// them, in order, in a single SystemMessage.
|
||||
registry.register("conv-1", 42L);
|
||||
router.onMcpServerChanged(new McpServerChangedEvent("change-A"));
|
||||
router.onMcpConnectionLost(new McpConnectionLostEvent(5L, "lost-B"));
|
||||
router.onSkillUpdated(new SkillUpdatedEvent(1L, "skill-C", "update"));
|
||||
|
||||
List<EnvironmentNotification> notes = registry.drain("conv-1");
|
||||
String block = ReasoningNode.renderEnvironmentNotifications(notes);
|
||||
|
||||
assertThat(notes).hasSize(3);
|
||||
assertThat(block).isNotNull();
|
||||
// Order preserved: change-A first, lost-B second, skill-C third
|
||||
int idxA = block.indexOf("change-A");
|
||||
int idxB = block.indexOf("mcp_5_");
|
||||
int idxC = block.indexOf("skill-C");
|
||||
assertThat(idxA).isGreaterThan(-1);
|
||||
assertThat(idxB).isGreaterThan(idxA);
|
||||
assertThat(idxC).isGreaterThan(idxB);
|
||||
}
|
||||
|
||||
// ==================== At-most-once delivery ====================
|
||||
|
||||
@Test
|
||||
void drainIsEmptyOnSecondCallSoNotificationIsInjectedAtMostOnce() {
|
||||
registry.register("conv-1", 42L);
|
||||
router.onMcpConnectionLost(new McpConnectionLostEvent(7L, "first"));
|
||||
|
||||
List<EnvironmentNotification> first = registry.drain("conv-1");
|
||||
List<EnvironmentNotification> second = registry.drain("conv-1");
|
||||
|
||||
assertThat(first).hasSize(1);
|
||||
assertThat(second).isEmpty();
|
||||
// The second turn's render must be skipped (returns null) — the
|
||||
// notification must NOT echo into a third turn.
|
||||
assertThat(ReasoningNode.renderEnvironmentNotifications(second)).isNull();
|
||||
}
|
||||
|
||||
@Test
|
||||
void newEventsFiredAfterFirstDrainAreVisibleOnSecondTurn() {
|
||||
// Sequential delivery: event 1 fires → drained on turn 1 → event 2
|
||||
// fires → drained on turn 2. Each turn sees only what's new since
|
||||
// the previous drain.
|
||||
registry.register("conv-1", 42L);
|
||||
router.onMcpConnectionLost(new McpConnectionLostEvent(1L, "first"));
|
||||
|
||||
String turn1 = ReasoningNode.renderEnvironmentNotifications(registry.drain("conv-1"));
|
||||
assertThat(turn1).contains("mcp_1_");
|
||||
|
||||
router.onMcpConnectionLost(new McpConnectionLostEvent(2L, "second"));
|
||||
String turn2 = ReasoningNode.renderEnvironmentNotifications(registry.drain("conv-1"));
|
||||
assertThat(turn2).contains("mcp_2_");
|
||||
assertThat(turn2).doesNotContain("mcp_1_"); // first event not re-delivered
|
||||
}
|
||||
|
||||
// ==================== Survives compression (nonHistoryPrefix invariant) ====================
|
||||
|
||||
@Test
|
||||
void notificationBlockIsExactlyOneSystemMessage_soSurvivesCompressionByDesign() {
|
||||
// The C4 injection site is nonHistoryPrefix, which is built fresh
|
||||
// every reasoning turn and NEVER touched by ConversationWindowManager
|
||||
// compaction (verified by ContextCompressionLedgerSurvivalTest for
|
||||
// the ledger snapshot — same invariant applies here). This test
|
||||
// pins the SHAPE of the injected payload so a future refactor that
|
||||
// accidentally puts the notification into the history window (which
|
||||
// IS subject to compaction) would fail.
|
||||
registry.register("conv-1", 42L);
|
||||
router.onMcpConnectionLost(new McpConnectionLostEvent(7L, "test"));
|
||||
|
||||
List<EnvironmentNotification> notes = registry.drain("conv-1");
|
||||
String block = ReasoningNode.renderEnvironmentNotifications(notes);
|
||||
|
||||
Message injection = new SystemMessage(block);
|
||||
assertThat(injection).isInstanceOf(SystemMessage.class);
|
||||
// SystemMessage is treated as non-history by the window manager —
|
||||
// it's never trimmed, never compacted, never summarised away.
|
||||
assertThat(injection.getText()).isEqualTo(block);
|
||||
}
|
||||
|
||||
// ==================== Inactive conversation safety ====================
|
||||
|
||||
@Test
|
||||
void eventFiredWhenNoConversationActiveIsDroppedSilently() {
|
||||
// No active conversation → router must not throw, no notification
|
||||
// lingers to be delivered to a future conversation that happens to
|
||||
// reuse the same conversationId.
|
||||
router.onMcpConnectionLost(new McpConnectionLostEvent(7L, "no-listener"));
|
||||
|
||||
registry.register("conv-1", 42L); // register AFTER the event
|
||||
assertThat(registry.drain("conv-1")).isEmpty();
|
||||
}
|
||||
|
||||
@Test
|
||||
void eventFiredAfterUnregisterIsDroppedSilently() {
|
||||
registry.register("conv-1", 42L);
|
||||
registry.unregister("conv-1");
|
||||
router.onMcpConnectionLost(new McpConnectionLostEvent(7L, "post-unregister"));
|
||||
|
||||
// Re-registering must NOT receive the event that fired while inactive
|
||||
registry.register("conv-1", 42L);
|
||||
assertThat(registry.drain("conv-1")).isEmpty();
|
||||
}
|
||||
}
|
||||
@ -0,0 +1,153 @@
|
||||
package vip.mate.agent.graph.node;
|
||||
|
||||
import org.junit.jupiter.api.DisplayName;
|
||||
import org.junit.jupiter.api.Test;
|
||||
|
||||
import java.util.LinkedHashSet;
|
||||
import java.util.Set;
|
||||
|
||||
import static org.junit.jupiter.api.Assertions.assertFalse;
|
||||
import static org.junit.jupiter.api.Assertions.assertNotNull;
|
||||
import static org.junit.jupiter.api.Assertions.assertNull;
|
||||
import static org.junit.jupiter.api.Assertions.assertTrue;
|
||||
|
||||
/**
|
||||
* Move 1 — coverage for {@link ReasoningNode#renderLoadedSkillsHint}.
|
||||
*
|
||||
* <p>Move 1 splits the previously-monolithic skill catalog rendering into:
|
||||
* <ol>
|
||||
* <li>A static catalog segment rendered with {@code Set.of()} for
|
||||
* loadedThisRun, so it stays prompt-cache-friendly across turns.</li>
|
||||
* <li>A per-turn volatile suffix that tells the model which skills it
|
||||
* already pulled in via {@code load_skill} this run.</li>
|
||||
* </ol>
|
||||
*
|
||||
* <p>This test exercises the volatile-suffix helper directly. The helper
|
||||
* is package-private static so tests can assert its exact format without
|
||||
* duplicating it. The contract being verified:
|
||||
* <ul>
|
||||
* <li>{@code null} or empty input → {@code null} (caller skips injection).</li>
|
||||
* <li>Non-empty input → a single-line SystemMessage body.</li>
|
||||
* <li>Each skill name is wrapped in backticks.</li>
|
||||
* <li>The text explicitly says "do not re-load" so the model knows not
|
||||
* to invoke {@code load_skill} again.</li>
|
||||
* <li>Order is preserved (LinkedHashSet semantics) so the most-recently
|
||||
* loaded skill is named first — useful when the model needs to
|
||||
* disambiguate two skills with overlapping tool names.</li>
|
||||
* </ul>
|
||||
*/
|
||||
class ReasoningNodeLoadedSkillsHintTest {
|
||||
|
||||
@Test
|
||||
@DisplayName("null loadedThisRun → null (caller skips injection)")
|
||||
void nullSetReturnsNull() {
|
||||
assertNull(ReasoningNode.renderLoadedSkillsHint(null),
|
||||
"null input must return null so the caller can skip injection");
|
||||
}
|
||||
|
||||
@Test
|
||||
@DisplayName("empty loadedThisRun → null (caller skips injection)")
|
||||
void emptySetReturnsNull() {
|
||||
assertNull(ReasoningNode.renderLoadedSkillsHint(Set.of()),
|
||||
"empty input must return null so the caller can skip injection");
|
||||
}
|
||||
|
||||
@Test
|
||||
@DisplayName("single skill → hint contains backtick-wrapped name + 'do not re-load'")
|
||||
void singleSkillRendersHint() {
|
||||
String hint = ReasoningNode.renderLoadedSkillsHint(Set.of("ckjia-shopping"));
|
||||
|
||||
assertNotNull(hint, "non-empty input must produce a hint");
|
||||
assertTrue(hint.contains("`ckjia-shopping`"),
|
||||
"skill name must be wrapped in backticks; hint was: " + hint);
|
||||
assertTrue(hint.contains("do not re-load"),
|
||||
"hint must explicitly say 'do not re-load'; hint was: " + hint);
|
||||
assertTrue(hint.contains("loaded this run"),
|
||||
"hint must mention 'loaded this run' for context; hint was: " + hint);
|
||||
}
|
||||
|
||||
@Test
|
||||
@DisplayName("multiple skills → comma-separated backtick-wrapped names")
|
||||
void multipleSkillsAreCommaSeparated() {
|
||||
// LinkedHashSet so iteration order is deterministic in the assertion
|
||||
Set<String> loaded = new LinkedHashSet<>();
|
||||
loaded.add("ckjia-shopping");
|
||||
loaded.add("browser-cdp");
|
||||
loaded.add("pdf-builtin");
|
||||
|
||||
String hint = ReasoningNode.renderLoadedSkillsHint(loaded);
|
||||
|
||||
assertNotNull(hint);
|
||||
assertTrue(hint.contains("`ckjia-shopping`"));
|
||||
assertTrue(hint.contains("`browser-cdp`"));
|
||||
assertTrue(hint.contains("`pdf-builtin`"));
|
||||
// All three on one line, comma-separated
|
||||
assertTrue(hint.contains("`ckjia-shopping`, `browser-cdp`, `pdf-builtin`"),
|
||||
"multiple skills must be comma-separated on one line; hint was: " + hint);
|
||||
}
|
||||
|
||||
@Test
|
||||
@DisplayName("hint is a single line (no embedded newlines)")
|
||||
void hintIsSingleLine() {
|
||||
String hint = ReasoningNode.renderLoadedSkillsHint(Set.of("skill-a", "skill-b"));
|
||||
|
||||
assertNotNull(hint);
|
||||
assertFalse(hint.contains("\n"),
|
||||
"hint must be a single line so it doesn't break SystemMessage formatting; hint was: " + hint);
|
||||
assertTrue(hint.endsWith("."),
|
||||
"hint must end with a period; hint was: " + hint);
|
||||
}
|
||||
|
||||
@Test
|
||||
@DisplayName("hint order matches the input iteration order (LinkedHashSet)")
|
||||
void hintPreservesIterationOrder() {
|
||||
Set<String> loaded = new LinkedHashSet<>();
|
||||
loaded.add("third-loaded");
|
||||
loaded.add("first-loaded");
|
||||
loaded.add("second-loaded");
|
||||
|
||||
String hint = ReasoningNode.renderLoadedSkillsHint(loaded);
|
||||
|
||||
assertNotNull(hint);
|
||||
int firstIdx = hint.indexOf("`first-loaded`");
|
||||
int secondIdx = hint.indexOf("`second-loaded`");
|
||||
int thirdIdx = hint.indexOf("`third-loaded`");
|
||||
assertTrue(thirdIdx < firstIdx && firstIdx < secondIdx,
|
||||
"iteration order must be preserved (third loaded first); hint was: " + hint);
|
||||
}
|
||||
|
||||
@Test
|
||||
@DisplayName("Move 1 invariant: hint text is volatile-suffix material, NOT a static-catalog segment")
|
||||
void hintTextIsVolatileSuffixMaterial() {
|
||||
// The hint must NOT contain language that implies it's part of the
|
||||
// static catalog — that would confuse the model about which skills
|
||||
// are statically visible vs. loaded this run.
|
||||
String hint = ReasoningNode.renderLoadedSkillsHint(Set.of("any-skill"));
|
||||
|
||||
assertNotNull(hint);
|
||||
assertFalse(hint.contains("| Skill |"),
|
||||
"hint must not look like a catalog table row; hint was: " + hint);
|
||||
assertFalse(hint.contains("### "),
|
||||
"hint must not look like a catalog section header; hint was: " + hint);
|
||||
}
|
||||
|
||||
@Test
|
||||
@DisplayName("Move 1 vs Move 2 boundary: hint does not duplicate constraints content")
|
||||
void hintDoesNotDuplicateConstraints() {
|
||||
// The hint's job is to tell the model "you already loaded this
|
||||
// skill, don't load it again" — it must NOT also embed the skill's
|
||||
// constraints. Those live in the Constraints column of the static
|
||||
// catalog (Move 2) and the ProgressLedger (B-class). Putting them
|
||||
// here too would (a) duplicate token spend, (b) break the
|
||||
// prompt-cache stability of the static prefix, and (c) violate
|
||||
// the "position is semantics" principle.
|
||||
String hint = ReasoningNode.renderLoadedSkillsHint(Set.of("ckjia-shopping"));
|
||||
|
||||
assertNotNull(hint);
|
||||
// Spot-check common constraint phrases that should NOT appear here
|
||||
assertFalse(hint.contains("Constraints"),
|
||||
"hint must not duplicate the Constraints column; hint was: " + hint);
|
||||
assertFalse(hint.contains("allowed tools"),
|
||||
"hint must not duplicate the allowed-tools block; hint was: " + hint);
|
||||
}
|
||||
}
|
||||
@ -0,0 +1,379 @@
|
||||
package vip.mate.agent.progress;
|
||||
|
||||
import com.fasterxml.jackson.databind.ObjectMapper;
|
||||
import com.fasterxml.jackson.datatype.jsr310.JavaTimeModule;
|
||||
import org.junit.jupiter.api.BeforeEach;
|
||||
import org.junit.jupiter.api.Test;
|
||||
|
||||
import java.util.List;
|
||||
import java.util.Map;
|
||||
import java.util.concurrent.ConcurrentHashMap;
|
||||
import java.util.concurrent.CountDownLatch;
|
||||
import java.util.concurrent.ExecutorService;
|
||||
import java.util.concurrent.Executors;
|
||||
import java.util.concurrent.TimeUnit;
|
||||
import java.util.concurrent.atomic.AtomicInteger;
|
||||
|
||||
import static org.assertj.core.api.Assertions.assertThat;
|
||||
import static org.assertj.core.api.Assertions.assertThatThrownBy;
|
||||
|
||||
/**
|
||||
* White-box tests for the prefix guard in {@link ProgressLedgerService#upsert}
|
||||
* and the display-name / key-uniqueness behavior of
|
||||
* {@link ProgressLedgerService#upsertAutoRecorded}.
|
||||
*/
|
||||
class ProgressLedgerPrefixGuardTest {
|
||||
|
||||
private InMemoryService service;
|
||||
|
||||
@BeforeEach
|
||||
void setUp() {
|
||||
service = new InMemoryService();
|
||||
}
|
||||
|
||||
// ==================== Prefix Guard ====================
|
||||
|
||||
@Test
|
||||
void upsertRejectsAutoPrefix() {
|
||||
assertThatThrownBy(() ->
|
||||
service.upsert("conv-1", "auto_read_file", "label", ProgressStatus.DONE, "note"))
|
||||
.isInstanceOf(IllegalArgumentException.class)
|
||||
.hasMessageContaining("reserved");
|
||||
}
|
||||
|
||||
@Test
|
||||
void upsertRejectsPinPrefix() {
|
||||
assertThatThrownBy(() ->
|
||||
service.upsert("conv-1", "pin_my-skill_0", "label", ProgressStatus.DONE, "note"))
|
||||
.isInstanceOf(IllegalArgumentException.class)
|
||||
.hasMessageContaining("reserved");
|
||||
}
|
||||
|
||||
@Test
|
||||
void upsertAcceptsRegularKey() {
|
||||
service.upsert("conv-1", "step_research", "Research", ProgressStatus.IN_PROGRESS, "working");
|
||||
ProgressLedger ledger = service.load("conv-1");
|
||||
assertThat(ledger.asMap()).containsKey("step_research");
|
||||
}
|
||||
|
||||
@Test
|
||||
void upsertAcceptsStepPrefix() {
|
||||
// "step_" is fine — only "auto_" and "pin_" are reserved
|
||||
service.upsert("conv-1", "step_1", "Step 1", ProgressStatus.PENDING, null);
|
||||
ProgressLedger ledger = service.load("conv-1");
|
||||
assertThat(ledger.asMap()).containsKey("step_1");
|
||||
}
|
||||
|
||||
// ==================== Auto-Recorded: Key Uniqueness ====================
|
||||
|
||||
@Test
|
||||
void autoRecordedUsesFullToolNameAsKey() {
|
||||
service.upsertAutoRecorded("conv-1", "mcp_4_search_a1b2c3", "search", "found 3 results");
|
||||
ProgressLedger ledger = service.load("conv-1");
|
||||
// Key should be auto_mcp_4_search_a1b2c3 (full name), NOT auto_search
|
||||
assertThat(ledger.asMap()).containsKey("auto_mcp_4_search_a1b2c3");
|
||||
ProgressEntry entry = ledger.asMap().get("auto_mcp_4_search_a1b2c3");
|
||||
assertThat(entry.getLabel()).isEqualTo("search"); // display name is slug only
|
||||
}
|
||||
|
||||
@Test
|
||||
void autoRecordedDifferentServersNoCollision() {
|
||||
// Two MCP servers both exposing "search" — must NOT collide
|
||||
service.upsertAutoRecorded("conv-1", "mcp_4_search_a1b2c3", "search", "server A result");
|
||||
service.upsertAutoRecorded("conv-1", "mcp_7_search_x9y8z7", "search", "server B result");
|
||||
|
||||
ProgressLedger ledger = service.load("conv-1");
|
||||
assertThat(ledger.asMap())
|
||||
.containsKey("auto_mcp_4_search_a1b2c3")
|
||||
.containsKey("auto_mcp_7_search_x9y8z7");
|
||||
assertThat(ledger.asMap()).hasSize(2); // two distinct entries
|
||||
}
|
||||
|
||||
@Test
|
||||
void autoRecordedDoesNotOverwriteLlmEntry() {
|
||||
// LLM writes a regular entry first
|
||||
service.upsert("conv-1", "read_file", "Read File", ProgressStatus.IN_PROGRESS, "LLM tracking");
|
||||
// Java tries to auto-record the same tool — should be a no-op because
|
||||
// the key "auto_read_file" is different from "read_file"
|
||||
service.upsertAutoRecorded("conv-1", "read_file", "read_file", "file content");
|
||||
|
||||
ProgressLedger ledger = service.load("conv-1");
|
||||
// Both entries coexist — LLM entry under "read_file", auto under "auto_read_file"
|
||||
assertThat(ledger.asMap()).hasSize(2);
|
||||
assertThat(ledger.asMap().get("read_file").getNote()).isEqualTo("LLM tracking");
|
||||
assertThat(ledger.asMap().get("auto_read_file").getStatus()).isEqualTo(ProgressStatus.DONE);
|
||||
}
|
||||
|
||||
// ==================== Auto-Recorded: Bounding ====================
|
||||
|
||||
@Test
|
||||
void autoRecordedBoundedToMaxFive() {
|
||||
for (int i = 0; i < 10; i++) {
|
||||
service.upsertAutoRecorded("conv-1", "tool_" + i, "tool_" + i, "result " + i);
|
||||
}
|
||||
ProgressLedger ledger = service.load("conv-1");
|
||||
long autoCount = ledger.asMap().keySet().stream()
|
||||
.filter(k -> k.startsWith(ProgressLedger.AUTO_RECORDED_PREFIX))
|
||||
.count();
|
||||
assertThat(autoCount).isEqualTo(ProgressLedgerService.MAX_AUTO_RECORDED);
|
||||
}
|
||||
|
||||
@Test
|
||||
void autoRecordedEvictsOldestFirst() {
|
||||
service.upsertAutoRecorded("conv-1", "tool_a", "tool_a", "first");
|
||||
service.upsertAutoRecorded("conv-1", "tool_b", "tool_b", "second");
|
||||
service.upsertAutoRecorded("conv-1", "tool_c", "tool_c", "third");
|
||||
service.upsertAutoRecorded("conv-1", "tool_d", "tool_d", "fourth");
|
||||
service.upsertAutoRecorded("conv-1", "tool_e", "tool_e", "fifth");
|
||||
// Now at max — adding a 6th should evict tool_a (oldest)
|
||||
service.upsertAutoRecorded("conv-1", "tool_f", "tool_f", "sixth");
|
||||
|
||||
ProgressLedger ledger = service.load("conv-1");
|
||||
assertThat(ledger.asMap()).doesNotContainKey("auto_tool_a");
|
||||
assertThat(ledger.asMap()).containsKey("auto_tool_f");
|
||||
}
|
||||
|
||||
// ==================== Pinned Entries ====================
|
||||
|
||||
@Test
|
||||
void upsertPinnedWritesToPinnedMap() {
|
||||
service.upsertPinned("conv-1", "pin_skill_0", "🔒 skill: Rule A", "Rule A detail");
|
||||
ProgressLedger ledger = service.load("conv-1");
|
||||
assertThat(ledger.pinnedEntries()).containsKey("pin_skill_0");
|
||||
assertThat(ledger.pinnedEntries().get("pin_skill_0").getLabel()).isEqualTo("🔒 skill: Rule A");
|
||||
}
|
||||
|
||||
@Test
|
||||
void upsertDoesNotTouchPinnedMap() {
|
||||
service.upsertPinned("conv-1", "pin_skill_0", "🔒 skill: Rule A", "Rule A");
|
||||
// LLM upsert should only touch entries, not pinned
|
||||
service.upsert("conv-1", "step_1", "Step 1", ProgressStatus.DONE, "done");
|
||||
|
||||
ProgressLedger ledger = service.load("conv-1");
|
||||
assertThat(ledger.pinnedEntries()).hasSize(1);
|
||||
assertThat(ledger.asMap()).hasSize(1);
|
||||
assertThat(ledger.asMap()).containsKey("step_1");
|
||||
}
|
||||
|
||||
@Test
|
||||
void clearPinnedByPrefixRemovesMatchingEntries() {
|
||||
service.upsertPinned("conv-1", "pin_skillA_0", "A: Rule 0", "detail");
|
||||
service.upsertPinned("conv-1", "pin_skillA_1", "A: Rule 1", "detail");
|
||||
service.upsertPinned("conv-1", "pin_skillB_0", "B: Rule 0", "detail");
|
||||
|
||||
service.clearPinnedByPrefix("conv-1", "pin_skillA_");
|
||||
|
||||
ProgressLedger ledger = service.load("conv-1");
|
||||
assertThat(ledger.pinnedEntries()).hasSize(1);
|
||||
assertThat(ledger.pinnedEntries()).containsKey("pin_skillB_0");
|
||||
}
|
||||
|
||||
// ==================== Rendering ====================
|
||||
|
||||
@Test
|
||||
void renderSnapshotShowsAllThreeSections() {
|
||||
// Pinned (B2)
|
||||
service.upsertPinned("conv-1", "pin_skill_0", "🔒 skill: No delete", "No delete outside /ws");
|
||||
// Auto-recorded (B5)
|
||||
service.upsertAutoRecorded("conv-1", "read_file", "read_file", "read config.yaml");
|
||||
// Regular (LLM)
|
||||
service.upsert("conv-1", "step_research", "Research", ProgressStatus.IN_PROGRESS, "investigating");
|
||||
|
||||
ProgressLedger ledger = service.load("conv-1");
|
||||
String snapshot = ledger.renderSnapshot();
|
||||
|
||||
assertThat(snapshot).contains("🔒 固定约束");
|
||||
assertThat(snapshot).contains("🔧 自动记录");
|
||||
assertThat(snapshot).contains("🔄 进行中");
|
||||
assertThat(snapshot).contains("No delete outside /ws");
|
||||
assertThat(snapshot).contains("read_file");
|
||||
assertThat(snapshot).contains("Research");
|
||||
}
|
||||
|
||||
@Test
|
||||
void renderSnapshotNullWhenEmpty() {
|
||||
ProgressLedger ledger = service.load("conv-1");
|
||||
assertThat(ledger.renderSnapshot()).isNull();
|
||||
}
|
||||
|
||||
@Test
|
||||
void staleReminderExcludesAutoRecordedFromRegularCheck() {
|
||||
// Only auto-recorded entries — should be treated as "no regular entries"
|
||||
service.upsertAutoRecorded("conv-1", "read_file", "read_file", "result");
|
||||
|
||||
ProgressLedger ledger = service.load("conv-1");
|
||||
// With only auto-recorded entries, the ledger is NOT "empty" (size > 0)
|
||||
// but has no regular entries — stale reminder should nudge
|
||||
String reminder = ledger.renderStaleReminder(10, java.time.Instant.now());
|
||||
// At iteration 10 (> EMPTY_LEDGER_NUDGE_ITERATIONS=5), should nudge
|
||||
assertThat(reminder).contains("进度账本是空的");
|
||||
}
|
||||
|
||||
// ==================== Concurrency ====================
|
||||
|
||||
@Test
|
||||
void concurrentUpsertAndAutoRecordAreSafe() throws InterruptedException {
|
||||
int threads = 8;
|
||||
int perThread = 20;
|
||||
ExecutorService pool = Executors.newFixedThreadPool(threads);
|
||||
CountDownLatch latch = new CountDownLatch(threads);
|
||||
AtomicInteger errors = new AtomicInteger();
|
||||
|
||||
for (int t = 0; t < threads; t++) {
|
||||
final int tid = t;
|
||||
pool.submit(() -> {
|
||||
try {
|
||||
for (int i = 0; i < perThread; i++) {
|
||||
String key = "step_t" + tid + "_i" + i;
|
||||
service.upsert("conv-1", key, key, ProgressStatus.PENDING, null);
|
||||
service.upsertAutoRecorded("conv-1", "tool_" + tid + "_" + i,
|
||||
"tool_" + i, "result");
|
||||
}
|
||||
} catch (Exception e) {
|
||||
errors.incrementAndGet();
|
||||
} finally {
|
||||
latch.countDown();
|
||||
}
|
||||
});
|
||||
}
|
||||
latch.await(30, TimeUnit.SECONDS);
|
||||
pool.shutdown();
|
||||
|
||||
assertThat(errors.get()).isZero();
|
||||
// Auto-recorded entries are bounded to MAX_AUTO_RECORDED regardless of concurrency
|
||||
ProgressLedger ledger = service.load("conv-1");
|
||||
long autoCount = ledger.asMap().keySet().stream()
|
||||
.filter(k -> k.startsWith(ProgressLedger.AUTO_RECORDED_PREFIX))
|
||||
.count();
|
||||
assertThat(autoCount).isLessThanOrEqualTo(ProgressLedgerService.MAX_AUTO_RECORDED);
|
||||
}
|
||||
|
||||
// ==================== Batch auto-record ====================
|
||||
|
||||
@Test
|
||||
void batchInsertProducesSameResultAsSequential() {
|
||||
// Insert 3 entries via batch on conv-A, 3 entries via sequential on conv-B
|
||||
List<ProgressLedgerService.AutoRecordEntry> batch = List.of(
|
||||
new ProgressLedgerService.AutoRecordEntry("web_search", "web_search", "found 5 results"),
|
||||
new ProgressLedgerService.AutoRecordEntry("read_file", "read_file", "read paper.pdf"),
|
||||
new ProgressLedgerService.AutoRecordEntry("write_file", "write_file", "wrote draft.md"));
|
||||
|
||||
service.upsertAutoRecordedBatch("conv-A", batch);
|
||||
|
||||
service.upsertAutoRecorded("conv-B", "web_search", "web_search", "found 5 results");
|
||||
service.upsertAutoRecorded("conv-B", "read_file", "read_file", "read paper.pdf");
|
||||
service.upsertAutoRecorded("conv-B", "write_file", "write_file", "wrote draft.md");
|
||||
|
||||
// Both conversations should have identical auto-recorded entries
|
||||
ProgressLedger ledgerA = service.load("conv-A");
|
||||
ProgressLedger ledgerB = service.load("conv-B");
|
||||
assertThat(ledgerA.asMap().keySet()).isEqualTo(ledgerB.asMap().keySet());
|
||||
for (String key : ledgerA.asMap().keySet()) {
|
||||
ProgressEntry a = ledgerA.asMap().get(key);
|
||||
ProgressEntry b = ledgerB.asMap().get(key);
|
||||
assertThat(a.getLabel()).isEqualTo(b.getLabel());
|
||||
assertThat(a.getStatus()).isEqualTo(b.getStatus());
|
||||
assertThat(a.getNote()).isEqualTo(b.getNote());
|
||||
}
|
||||
}
|
||||
|
||||
@Test
|
||||
void batchInsertBoundedToMaxFiveEvenWithLargeBatch() {
|
||||
// Insert 10 entries in a single batch — should be bounded to MAX_AUTO_RECORDED
|
||||
List<ProgressLedgerService.AutoRecordEntry> big = new java.util.ArrayList<>();
|
||||
for (int i = 0; i < 10; i++) {
|
||||
big.add(new ProgressLedgerService.AutoRecordEntry("tool_" + i, "tool_" + i, "result " + i));
|
||||
}
|
||||
service.upsertAutoRecordedBatch("conv-1", big);
|
||||
|
||||
ProgressLedger ledger = service.load("conv-1");
|
||||
long autoCount = ledger.asMap().keySet().stream()
|
||||
.filter(k -> k.startsWith(ProgressLedger.AUTO_RECORDED_PREFIX))
|
||||
.count();
|
||||
assertThat(autoCount).isEqualTo(ProgressLedgerService.MAX_AUTO_RECORDED);
|
||||
// The newest 5 (tool_5 through tool_9) should survive; oldest evicted
|
||||
assertThat(ledger.asMap()).containsKey("auto_tool_9");
|
||||
assertThat(ledger.asMap()).containsKey("auto_tool_5");
|
||||
assertThat(ledger.asMap()).doesNotContainKey("auto_tool_4");
|
||||
}
|
||||
|
||||
@Test
|
||||
void batchInsertSkipsNullAndBlankToolNames() {
|
||||
List<ProgressLedgerService.AutoRecordEntry> mixed = List.of(
|
||||
new ProgressLedgerService.AutoRecordEntry(null, "null-tool", "result"),
|
||||
new ProgressLedgerService.AutoRecordEntry("", "blank-tool", "result"),
|
||||
new ProgressLedgerService.AutoRecordEntry(" ", "whitespace-tool", "result"),
|
||||
new ProgressLedgerService.AutoRecordEntry("valid_tool", "valid", "valid result"));
|
||||
|
||||
service.upsertAutoRecordedBatch("conv-1", mixed);
|
||||
|
||||
ProgressLedger ledger = service.load("conv-1");
|
||||
assertThat(ledger.asMap()).containsKey("auto_valid_tool");
|
||||
assertThat(ledger.asMap()).doesNotContainKey("auto_null");
|
||||
assertThat(ledger.asMap()).doesNotContainKey("auto_");
|
||||
}
|
||||
|
||||
@Test
|
||||
void batchInsertDoesNotOverwriteLlmAuthoredEntries() {
|
||||
// LLM writes an entry with the same key the batch would use
|
||||
service.upsertPinned("conv-1", "pin_skip", "pinned", "pinned-note");
|
||||
// Simulate LLM writing via direct map manipulation — write to entries
|
||||
// with the auto_ prefix (this is what the LLM would do if the prefix
|
||||
// guard weren't there; the guard prevents it, but we test the batch's
|
||||
// skip-if-exists behavior by pre-seeding via the internal store)
|
||||
// Actually, since upsert() rejects auto_ prefix, we can't pre-seed
|
||||
// via the public API. Instead, test that batch doesn't overwrite
|
||||
// an entry it just inserted in the same batch (dedup within batch).
|
||||
List<ProgressLedgerService.AutoRecordEntry> dupBatch = List.of(
|
||||
new ProgressLedgerService.AutoRecordEntry("dup_tool", "dup", "first result"),
|
||||
new ProgressLedgerService.AutoRecordEntry("dup_tool", "dup", "second result"));
|
||||
|
||||
service.upsertAutoRecordedBatch("conv-1", dupBatch);
|
||||
|
||||
ProgressLedger ledger = service.load("conv-1");
|
||||
ProgressEntry entry = ledger.asMap().get("auto_dup_tool");
|
||||
assertThat(entry).isNotNull();
|
||||
// First insert wins; second is skipped (same behavior as sequential)
|
||||
assertThat(entry.getNote()).isEqualTo("first result");
|
||||
}
|
||||
|
||||
@Test
|
||||
void emptyBatchIsNoOp() {
|
||||
service.upsertAutoRecordedBatch("conv-1", List.of());
|
||||
assertThat(service.load("conv-1").isEmpty()).isTrue();
|
||||
}
|
||||
|
||||
@Test
|
||||
void nullAndBlankConversationIdIgnoredInBatch() {
|
||||
List<ProgressLedgerService.AutoRecordEntry> batch = List.of(
|
||||
new ProgressLedgerService.AutoRecordEntry("tool", "tool", "result"));
|
||||
service.upsertAutoRecordedBatch(null, batch);
|
||||
service.upsertAutoRecordedBatch("", batch);
|
||||
service.upsertAutoRecordedBatch(" ", batch);
|
||||
// No exception, no state change
|
||||
}
|
||||
|
||||
// ==================== Test infrastructure ====================
|
||||
|
||||
/**
|
||||
* In-memory subclass that overrides DB I/O — same pattern as
|
||||
* {@link ProgressLedgerServiceConcurrencyTest}.
|
||||
*/
|
||||
private static final class InMemoryService extends ProgressLedgerService {
|
||||
private final Map<String, String> store = new ConcurrentHashMap<>();
|
||||
|
||||
InMemoryService() {
|
||||
super(null, new ObjectMapper().registerModule(new JavaTimeModule()));
|
||||
}
|
||||
|
||||
@Override
|
||||
protected String loadLedgerJson(String conversationId) {
|
||||
return store.get(conversationId);
|
||||
}
|
||||
|
||||
@Override
|
||||
protected void saveLedgerJson(String conversationId, String json) {
|
||||
store.put(conversationId, json);
|
||||
}
|
||||
}
|
||||
}
|
||||
@ -23,27 +23,27 @@ class ProgressLedgerStaleReminderTest {
|
||||
private static final Instant NOW = Instant.parse("2026-05-24T19:30:00Z");
|
||||
|
||||
@Test
|
||||
@DisplayName("Iteration < 10 → no reminder regardless of ledger state.")
|
||||
@DisplayName("Iteration < 3 → no reminder regardless of ledger state.")
|
||||
void warmupPeriodNoReminder() {
|
||||
assertNull(ProgressLedger.empty().renderStaleReminder(0, NOW));
|
||||
assertNull(ProgressLedger.empty().renderStaleReminder(5, NOW));
|
||||
assertNull(ProgressLedger.empty().renderStaleReminder(9, NOW));
|
||||
assertNull(ProgressLedger.empty().renderStaleReminder(1, NOW));
|
||||
assertNull(ProgressLedger.empty().renderStaleReminder(2, NOW));
|
||||
}
|
||||
|
||||
@Test
|
||||
@DisplayName("Empty ledger between iter 10 and 14 → still no reminder.")
|
||||
@DisplayName("Empty ledger between iter 3 and 4 → still no reminder.")
|
||||
void emptyLedgerBelowNudgeThreshold() {
|
||||
assertNull(ProgressLedger.empty().renderStaleReminder(10, NOW));
|
||||
assertNull(ProgressLedger.empty().renderStaleReminder(14, NOW));
|
||||
assertNull(ProgressLedger.empty().renderStaleReminder(3, NOW));
|
||||
assertNull(ProgressLedger.empty().renderStaleReminder(4, NOW));
|
||||
}
|
||||
|
||||
@Test
|
||||
@DisplayName("Empty ledger at iter ≥ 15 → emit empty-ledger reminder.")
|
||||
@DisplayName("Empty ledger at iter ≥ 5 → emit empty-ledger reminder.")
|
||||
void emptyLedgerTriggersReminder() {
|
||||
String out = ProgressLedger.empty().renderStaleReminder(15, NOW);
|
||||
String out = ProgressLedger.empty().renderStaleReminder(5, NOW);
|
||||
assertNotNull(out);
|
||||
assertTrue(out.contains("进度账本是空的"), out);
|
||||
assertTrue(out.contains("15 轮"), out);
|
||||
assertTrue(out.contains("5 轮"), out);
|
||||
assertTrue(out.contains("progress_update"), out);
|
||||
}
|
||||
|
||||
@ -57,7 +57,7 @@ class ProgressLedgerStaleReminderTest {
|
||||
}
|
||||
|
||||
@Test
|
||||
@DisplayName("Non-empty ledger with last update ≥ 90s ago → emit stale reminder.")
|
||||
@DisplayName("Non-empty ledger with last update ≥ 45s ago → emit stale reminder.")
|
||||
void staleUpdateTriggersReminder() {
|
||||
Map<String, ProgressEntry> entries = new LinkedHashMap<>();
|
||||
entries.put("a", new ProgressEntry("a", "A", ProgressStatus.DONE, null,
|
||||
@ -78,7 +78,7 @@ class ProgressLedgerStaleReminderTest {
|
||||
Map<String, ProgressEntry> entries = new LinkedHashMap<>();
|
||||
entries.put("a", new ProgressEntry("a", "A", ProgressStatus.DONE, null,
|
||||
NOW.minusSeconds(600)));
|
||||
assertNull(new ProgressLedger(entries).renderStaleReminder(5, NOW));
|
||||
assertNull(new ProgressLedger(entries).renderStaleReminder(2, NOW));
|
||||
}
|
||||
|
||||
@Test
|
||||
|
||||
@ -0,0 +1,309 @@
|
||||
package vip.mate.agent.runtime;
|
||||
|
||||
import org.junit.jupiter.api.BeforeEach;
|
||||
import org.junit.jupiter.api.Test;
|
||||
import vip.mate.skill.event.SkillUpdatedEvent;
|
||||
import vip.mate.tool.mcp.event.McpConnectionLostEvent;
|
||||
import vip.mate.tool.mcp.event.McpServerChangedEvent;
|
||||
import vip.mate.tool.mcp.event.McpServerRemovedEvent;
|
||||
|
||||
import java.time.Instant;
|
||||
import java.util.List;
|
||||
|
||||
import static org.assertj.core.api.Assertions.assertThat;
|
||||
|
||||
/**
|
||||
* White-box tests for {@link RunningConversationRegistry} and
|
||||
* {@link EnvironmentEventRouter}. These verify the C-class event-routing
|
||||
* pipeline: register → event fires → notification queued → drain on next turn.
|
||||
*/
|
||||
class RunningConversationRegistryTest {
|
||||
|
||||
private RunningConversationRegistry registry;
|
||||
private EnvironmentEventRouter router;
|
||||
|
||||
@BeforeEach
|
||||
void setUp() {
|
||||
registry = new RunningConversationRegistry();
|
||||
router = new EnvironmentEventRouter(registry);
|
||||
}
|
||||
|
||||
// ==================== Registry lifecycle ====================
|
||||
|
||||
@Test
|
||||
void registerMakesConversationActive() {
|
||||
assertThat(registry.isActive("conv-1")).isFalse();
|
||||
registry.register("conv-1", 42L);
|
||||
assertThat(registry.isActive("conv-1")).isTrue();
|
||||
assertThat(registry.activeConversations()).contains("conv-1");
|
||||
}
|
||||
|
||||
@Test
|
||||
void unregisterMakesConversationInactive() {
|
||||
registry.register("conv-1", 42L);
|
||||
registry.unregister("conv-1");
|
||||
assertThat(registry.isActive("conv-1")).isFalse();
|
||||
assertThat(registry.activeConversations()).doesNotContain("conv-1");
|
||||
}
|
||||
|
||||
@Test
|
||||
void registerIsIdempotent() {
|
||||
registry.register("conv-1", 42L);
|
||||
registry.register("conv-1", 42L); // no-op, just refreshes lastActiveAt
|
||||
assertThat(registry.activeConversations()).hasSize(1);
|
||||
}
|
||||
|
||||
@Test
|
||||
void unregisterUnknownConversationIsSafe() {
|
||||
registry.unregister("never-registered");
|
||||
// no exception thrown
|
||||
}
|
||||
|
||||
@Test
|
||||
void nullAndBlankConversationIdIgnored() {
|
||||
registry.register(null, 42L);
|
||||
registry.register("", 42L);
|
||||
registry.register(" ", 42L);
|
||||
assertThat(registry.activeConversations()).isEmpty();
|
||||
}
|
||||
|
||||
// ==================== Queue + drain ====================
|
||||
|
||||
@Test
|
||||
void drainReturnsEmptyForInactiveConversation() {
|
||||
List<EnvironmentNotification> notes = registry.drain("never-active");
|
||||
assertThat(notes).isEmpty();
|
||||
}
|
||||
|
||||
@Test
|
||||
void drainEmptiesTheQueue() {
|
||||
registry.register("conv-1", 42L);
|
||||
registry.enqueue("conv-1", new EnvironmentNotification("test", "msg-1", Instant.now()));
|
||||
registry.enqueue("conv-1", new EnvironmentNotification("test", "msg-2", Instant.now()));
|
||||
|
||||
List<EnvironmentNotification> first = registry.drain("conv-1");
|
||||
assertThat(first).hasSize(2);
|
||||
|
||||
List<EnvironmentNotification> second = registry.drain("conv-1");
|
||||
assertThat(second).isEmpty(); // queue was drained
|
||||
}
|
||||
|
||||
@Test
|
||||
void enqueueToInactiveConversationDropsMessage() {
|
||||
// Event fires when no conversation is active — message is lost (by design)
|
||||
registry.enqueue("never-active", new EnvironmentNotification("test", "msg", Instant.now()));
|
||||
assertThat(registry.drain("never-active")).isEmpty();
|
||||
}
|
||||
|
||||
@Test
|
||||
void queueBoundedToTenEvictsOldest() {
|
||||
registry.register("conv-1", 42L);
|
||||
for (int i = 0; i < 15; i++) {
|
||||
registry.enqueue("conv-1", new EnvironmentNotification("test", "msg-" + i, Instant.now()));
|
||||
}
|
||||
List<EnvironmentNotification> notes = registry.drain("conv-1");
|
||||
assertThat(notes).hasSize(RunningConversationRegistry.MAX_NOTIFICATIONS_PER_CONVERSATION);
|
||||
// Oldest messages (msg-0 through msg-4) should have been evicted
|
||||
assertThat(notes.get(0).message()).isEqualTo("msg-5");
|
||||
assertThat(notes.get(9).message()).isEqualTo("msg-14");
|
||||
}
|
||||
|
||||
// ==================== Broadcast ====================
|
||||
|
||||
@Test
|
||||
void broadcastReachesAllActiveConversations() {
|
||||
registry.register("conv-A", 1L);
|
||||
registry.register("conv-B", 2L);
|
||||
registry.register("conv-C", 3L);
|
||||
|
||||
registry.broadcast(new EnvironmentNotification("mcp-lost", "server down", Instant.now()));
|
||||
|
||||
assertThat(registry.drain("conv-A")).hasSize(1);
|
||||
assertThat(registry.drain("conv-B")).hasSize(1);
|
||||
assertThat(registry.drain("conv-C")).hasSize(1);
|
||||
}
|
||||
|
||||
@Test
|
||||
void broadcastToNoActiveConversationsIsNoOp() {
|
||||
registry.broadcast(new EnvironmentNotification("test", "msg", Instant.now()));
|
||||
// no exception, no side effect
|
||||
}
|
||||
|
||||
// ==================== Event Router ====================
|
||||
|
||||
@Test
|
||||
void mcpServerChangedEventQueuesNotification() {
|
||||
registry.register("conv-1", 42L);
|
||||
router.onMcpServerChanged(new McpServerChangedEvent("mcp-tools-changed:99"));
|
||||
|
||||
List<EnvironmentNotification> notes = registry.drain("conv-1");
|
||||
assertThat(notes).hasSize(1);
|
||||
assertThat(notes.get(0).type()).isEqualTo("mcp-changed");
|
||||
assertThat(notes.get(0).message()).contains("MCP 工具列表已变更");
|
||||
assertThat(notes.get(0).message()).contains("mcp-tools-changed:99");
|
||||
}
|
||||
|
||||
@Test
|
||||
void mcpConnectionLostEventQueuesNotificationWithServerId() {
|
||||
registry.register("conv-1", 42L);
|
||||
router.onMcpConnectionLost(new McpConnectionLostEvent(7L, "stdio-process-exited"));
|
||||
|
||||
List<EnvironmentNotification> notes = registry.drain("conv-1");
|
||||
assertThat(notes).hasSize(1);
|
||||
assertThat(notes.get(0).type()).isEqualTo("mcp-lost");
|
||||
assertThat(notes.get(0).message()).contains("7");
|
||||
assertThat(notes.get(0).message()).contains("mcp_7_");
|
||||
}
|
||||
|
||||
@Test
|
||||
void mcpServerRemovedEventQueuesNotification() {
|
||||
registry.register("conv-1", 42L);
|
||||
router.onMcpServerRemoved(new McpServerRemovedEvent(3L, "my-server"));
|
||||
|
||||
List<EnvironmentNotification> notes = registry.drain("conv-1");
|
||||
assertThat(notes).hasSize(1);
|
||||
assertThat(notes.get(0).type()).isEqualTo("mcp-removed");
|
||||
assertThat(notes.get(0).message()).contains("my-server");
|
||||
assertThat(notes.get(0).message()).contains("mcp_3_");
|
||||
}
|
||||
|
||||
@Test
|
||||
void skillUpdatedEventQueuesNotification() {
|
||||
registry.register("conv-1", 42L);
|
||||
router.onSkillUpdated(new SkillUpdatedEvent(10L, "web-scraper", "update"));
|
||||
|
||||
List<EnvironmentNotification> notes = registry.drain("conv-1");
|
||||
assertThat(notes).hasSize(1);
|
||||
assertThat(notes.get(0).type()).isEqualTo("skill-updated");
|
||||
assertThat(notes.get(0).message()).contains("web-scraper");
|
||||
assertThat(notes.get(0).message()).contains("已更新");
|
||||
}
|
||||
|
||||
@Test
|
||||
void skillUpdatedEnableEventUsesCorrectVerb() {
|
||||
registry.register("conv-1", 42L);
|
||||
router.onSkillUpdated(new SkillUpdatedEvent(11L, "data-processor", "enable"));
|
||||
|
||||
List<EnvironmentNotification> notes = registry.drain("conv-1");
|
||||
assertThat(notes.get(0).message()).contains("已启用");
|
||||
}
|
||||
|
||||
@Test
|
||||
void eventWithNoActiveConversationsIsSilentlyDropped() {
|
||||
// No conversation registered — router should not throw
|
||||
router.onMcpServerChanged(new McpServerChangedEvent("test"));
|
||||
router.onMcpConnectionLost(new McpConnectionLostEvent(1L, "test"));
|
||||
router.onMcpServerRemoved(new McpServerRemovedEvent(1L, "test"));
|
||||
// No exception thrown
|
||||
}
|
||||
|
||||
@Test
|
||||
void multipleEventsQueueInOrder() {
|
||||
registry.register("conv-1", 42L);
|
||||
router.onMcpServerChanged(new McpServerChangedEvent("change-1"));
|
||||
router.onMcpConnectionLost(new McpConnectionLostEvent(5L, "lost-1"));
|
||||
router.onSkillUpdated(new SkillUpdatedEvent(1L, "skill", "update"));
|
||||
|
||||
List<EnvironmentNotification> notes = registry.drain("conv-1");
|
||||
assertThat(notes).hasSize(3);
|
||||
assertThat(notes.get(0).type()).isEqualTo("mcp-changed");
|
||||
assertThat(notes.get(1).type()).isEqualTo("mcp-lost");
|
||||
assertThat(notes.get(2).type()).isEqualTo("skill-updated");
|
||||
}
|
||||
|
||||
// ==================== Stale-handle cleanup ====================
|
||||
|
||||
@Test
|
||||
void cleanupStaleRemovesOldHandles() throws Exception {
|
||||
registry.register("stale-conv", 1L);
|
||||
// Backdate lastActiveAt to 1 hour ago via reflection
|
||||
backdateLastActive("stale-conv", java.time.Instant.now().minusSeconds(3600));
|
||||
|
||||
registry.register("fresh-conv", 2L); // fresh — just registered
|
||||
|
||||
int removed = registry.cleanupStale(java.time.Duration.ofMinutes(30));
|
||||
|
||||
assertThat(removed).isEqualTo(1);
|
||||
assertThat(registry.isActive("stale-conv")).isFalse();
|
||||
assertThat(registry.isActive("fresh-conv")).isTrue();
|
||||
}
|
||||
|
||||
@Test
|
||||
void cleanupStaleKeepsActiveConversations() {
|
||||
registry.register("conv-1", 1L);
|
||||
registry.register("conv-2", 2L);
|
||||
registry.register("conv-3", 3L);
|
||||
|
||||
int removed = registry.cleanupStale(java.time.Duration.ofMinutes(30));
|
||||
|
||||
assertThat(removed).isZero();
|
||||
assertThat(registry.activeConversations()).hasSize(3);
|
||||
}
|
||||
|
||||
@Test
|
||||
void cleanupStaleWithZeroOrNegativeDurationIsNoOp() {
|
||||
registry.register("conv-1", 1L);
|
||||
assertThat(registry.cleanupStale(java.time.Duration.ZERO)).isZero();
|
||||
assertThat(registry.cleanupStale(java.time.Duration.ofMillis(-1))).isZero();
|
||||
assertThat(registry.cleanupStale(null)).isZero();
|
||||
assertThat(registry.isActive("conv-1")).isTrue();
|
||||
}
|
||||
|
||||
@Test
|
||||
void cleanupStaleDoesNotRemoveRefreshedHandle() throws Exception {
|
||||
registry.register("conv-1", 1L);
|
||||
// Backdate, then re-register (which refreshes lastActiveAt)
|
||||
backdateLastActive("conv-1", java.time.Instant.now().minusSeconds(3600));
|
||||
registry.register("conv-1", 1L); // refresh
|
||||
|
||||
int removed = registry.cleanupStale(java.time.Duration.ofMinutes(30));
|
||||
|
||||
assertThat(removed).isZero();
|
||||
assertThat(registry.isActive("conv-1")).isTrue();
|
||||
}
|
||||
|
||||
@Test
|
||||
void cleanupStaleRemovesMultipleStaleHandles() throws Exception {
|
||||
registry.register("stale-1", 1L);
|
||||
registry.register("stale-2", 2L);
|
||||
registry.register("stale-3", 3L);
|
||||
registry.register("fresh-1", 4L);
|
||||
|
||||
for (String conv : new String[]{"stale-1", "stale-2", "stale-3"}) {
|
||||
backdateLastActive(conv, java.time.Instant.now().minusSeconds(3600));
|
||||
}
|
||||
|
||||
int removed = registry.cleanupStale(java.time.Duration.ofMinutes(30));
|
||||
|
||||
assertThat(removed).isEqualTo(3);
|
||||
assertThat(registry.isActive("stale-1")).isFalse();
|
||||
assertThat(registry.isActive("stale-2")).isFalse();
|
||||
assertThat(registry.isActive("stale-3")).isFalse();
|
||||
assertThat(registry.isActive("fresh-1")).isTrue();
|
||||
}
|
||||
|
||||
@Test
|
||||
void scheduledCleanupIsSafeToCallWithNoActiveConversations() {
|
||||
registry.scheduledCleanup();
|
||||
// No exception, no side effect
|
||||
}
|
||||
|
||||
/**
|
||||
* Helper: use reflection to backdate a conversation handle's
|
||||
* {@code lastActiveAt} field, simulating a stale registration.
|
||||
*/
|
||||
private void backdateLastActive(String conversationId, java.time.Instant past) throws Exception {
|
||||
java.lang.reflect.Field activeField = RunningConversationRegistry.class
|
||||
.getDeclaredField("active");
|
||||
activeField.setAccessible(true);
|
||||
@SuppressWarnings("unchecked")
|
||||
java.util.concurrent.ConcurrentMap<String, Object> active =
|
||||
(java.util.concurrent.ConcurrentMap<String, Object>) activeField.get(registry);
|
||||
Object handle = active.get(conversationId);
|
||||
assertThat(handle).as("handle must exist for conv " + conversationId).isNotNull();
|
||||
java.lang.reflect.Field lastActiveField = handle.getClass()
|
||||
.getDeclaredField("lastActiveAt");
|
||||
lastActiveField.setAccessible(true);
|
||||
lastActiveField.set(handle, past);
|
||||
}
|
||||
}
|
||||
@ -0,0 +1,110 @@
|
||||
package vip.mate.skill.manifest;
|
||||
|
||||
import org.junit.jupiter.api.Test;
|
||||
import vip.mate.skill.runtime.SkillFrontmatterParser;
|
||||
|
||||
import static org.assertj.core.api.Assertions.assertThat;
|
||||
|
||||
/**
|
||||
* White-box test: verifies that {@link SkillManifestParser} now populates the
|
||||
* {@code constraints} field from YAML frontmatter. Before the fix, the field
|
||||
* was declared on {@link SkillManifest} but the parser never called
|
||||
* {@code .constraints(...)} on the builder, so B2 (pinSkillConstraints) and
|
||||
* agent-4 (catalog anchor) were dead code.
|
||||
*/
|
||||
class SkillManifestConstraintsParsingTest {
|
||||
|
||||
private final SkillManifestParser parser = new SkillManifestParser(new SkillFrontmatterParser());
|
||||
|
||||
@Test
|
||||
void constraintsBlockPopulatesField() {
|
||||
String skillMd = """
|
||||
---
|
||||
name: my-skill
|
||||
description: A skill with constraints
|
||||
constraints:
|
||||
- Never delete files outside /workspace
|
||||
- Always confirm before running shell commands
|
||||
- Use read_file before write_file
|
||||
---
|
||||
# My Skill
|
||||
Body content here.
|
||||
""";
|
||||
|
||||
SkillManifest manifest = parser.parse(skillMd);
|
||||
|
||||
assertThat(manifest).isNotNull();
|
||||
assertThat(manifest.getConstraints())
|
||||
.hasSize(3)
|
||||
.containsExactly(
|
||||
"Never delete files outside /workspace",
|
||||
"Always confirm before running shell commands",
|
||||
"Use read_file before write_file");
|
||||
}
|
||||
|
||||
@Test
|
||||
void noConstraintsYieldsEmptyList() {
|
||||
String skillMd = """
|
||||
---
|
||||
name: simple-skill
|
||||
description: A skill without constraints
|
||||
---
|
||||
# Simple Skill
|
||||
""";
|
||||
|
||||
SkillManifest manifest = parser.parse(skillMd);
|
||||
|
||||
assertThat(manifest).isNotNull();
|
||||
assertThat(manifest.getConstraints()).isEmpty();
|
||||
}
|
||||
|
||||
@Test
|
||||
void singleStringConstraintWrapsIntoOneElementList() {
|
||||
String skillMd = """
|
||||
---
|
||||
name: single-constraint-skill
|
||||
constraints: "Always be polite"
|
||||
---
|
||||
# Single Constraint Skill
|
||||
""";
|
||||
|
||||
SkillManifest manifest = parser.parse(skillMd);
|
||||
|
||||
assertThat(manifest).isNotNull();
|
||||
assertThat(manifest.getConstraints()).hasSize(1);
|
||||
assertThat(manifest.getConstraints().get(0)).isEqualTo("Always be polite");
|
||||
}
|
||||
|
||||
@Test
|
||||
void constraintsNotLeakedIntoExtras() {
|
||||
String skillMd = """
|
||||
---
|
||||
name: extras-test
|
||||
constraints:
|
||||
- Rule A
|
||||
---
|
||||
# Extras Test
|
||||
""";
|
||||
|
||||
SkillManifest manifest = parser.parse(skillMd);
|
||||
|
||||
// constraints should be a typed field, NOT in extras
|
||||
assertThat(manifest.getExtras()).doesNotContainKey("constraints");
|
||||
}
|
||||
|
||||
@Test
|
||||
void constraintsSurviveRoundTripThroughBuilder() {
|
||||
SkillManifest original = SkillManifest.builder()
|
||||
.name("round-trip")
|
||||
.constraints(java.util.List.of("Rule 1", "Rule 2"))
|
||||
.build();
|
||||
|
||||
// Rebuild from the same values — verifies the field is wired correctly
|
||||
SkillManifest rebuilt = SkillManifest.builder()
|
||||
.name(original.getName())
|
||||
.constraints(original.getConstraints())
|
||||
.build();
|
||||
|
||||
assertThat(rebuilt.getConstraints()).isEqualTo(original.getConstraints());
|
||||
}
|
||||
}
|
||||
@ -0,0 +1,359 @@
|
||||
package vip.mate.skill.runtime;
|
||||
|
||||
import org.junit.jupiter.api.DisplayName;
|
||||
import org.junit.jupiter.api.Test;
|
||||
import vip.mate.skill.acp.AcpSkillBridge;
|
||||
import vip.mate.skill.lessons.SkillLessonsService;
|
||||
import vip.mate.skill.manifest.SkillManifest;
|
||||
import vip.mate.skill.mcp.McpSkillBridge;
|
||||
import vip.mate.skill.model.SkillEntity;
|
||||
import vip.mate.skill.runtime.model.ResolvedSkill;
|
||||
import vip.mate.skill.service.SkillService;
|
||||
import vip.mate.skill.usage.SkillUsageService;
|
||||
|
||||
import java.util.List;
|
||||
import java.util.Set;
|
||||
import java.util.LinkedHashSet;
|
||||
|
||||
import static org.junit.jupiter.api.Assertions.assertFalse;
|
||||
import static org.junit.jupiter.api.Assertions.assertTrue;
|
||||
import static org.mockito.Mockito.mock;
|
||||
import static org.mockito.Mockito.when;
|
||||
|
||||
/**
|
||||
* Move 2 & Move 3 — coverage for the new catalog segments:
|
||||
* <ol>
|
||||
* <li>Move 2: a 4th {@code Constraints} column in the catalog table,
|
||||
* populated only for bound skills whose manifest declares
|
||||
* {@code constraints[]}.</li>
|
||||
* <li>Move 3: a {@code ### Bound skill allowed tools} block after the
|
||||
* table, listing each bound skill's effective tool allowlist.</li>
|
||||
* </ol>
|
||||
*
|
||||
* <p>Existing tests in {@link SkillRuntimeServicePromptBudgetTest} use a
|
||||
* {@code resolved()} helper that builds a {@link ResolvedSkill} without a
|
||||
* manifest, so neither the Constraints column nor the allowed-tools block
|
||||
* is exercised. This class fills that gap.
|
||||
*/
|
||||
class SkillRuntimeServiceConstraintsAndToolsTest {
|
||||
|
||||
@Test
|
||||
@DisplayName("Move 2: bound skill with manifest constraints renders a Constraints cell")
|
||||
void boundSkillWithConstraintsRendersConstraintsColumn() {
|
||||
SkillService skillService = mock(SkillService.class);
|
||||
SkillPackageResolver resolver = mock(SkillPackageResolver.class);
|
||||
SkillLessonsService lessonsService = mock(SkillLessonsService.class);
|
||||
McpSkillBridge mcpBridge = mock(McpSkillBridge.class);
|
||||
AcpSkillBridge acpBridge = mock(AcpSkillBridge.class);
|
||||
SkillUsageService usageService = mock(SkillUsageService.class);
|
||||
|
||||
SkillEntity entity = entity(99L, "ckjia-shopping", "builtin");
|
||||
when(skillService.listEnabledSkills()).thenReturn(List.of(entity));
|
||||
SkillManifest manifest = SkillManifest.builder()
|
||||
.constraints(List.of("Always confirm before writing", "Use markdown links"))
|
||||
.build();
|
||||
ResolvedSkill bound = resolvedWithManifest(entity, manifest);
|
||||
when(resolver.resolve(entity)).thenReturn(bound);
|
||||
when(mcpBridge.listMcpDerivedResolvedSkills()).thenReturn(List.of());
|
||||
when(acpBridge.listAcpDerivedResolvedSkills()).thenReturn(List.of());
|
||||
when(usageService.recentLoadedSkillNames(null, 8)).thenReturn(Set.of());
|
||||
when(usageService.frequentlyLoadedSkillNames(8)).thenReturn(Set.of());
|
||||
|
||||
SkillRuntimeService runtime = new SkillRuntimeService(
|
||||
skillService, resolver, lessonsService, mcpBridge, acpBridge, usageService);
|
||||
|
||||
String prompt = runtime.buildSkillPromptEnhancement(Set.of(99L), null, 8192);
|
||||
|
||||
// The table header has the Constraints column
|
||||
assertTrue(prompt.contains("| Skill | Status | Description | Constraints |"),
|
||||
"catalog table must declare the Constraints column; prompt was: " + prompt);
|
||||
// Both constraints survive in the cell
|
||||
assertTrue(prompt.contains("Always confirm before writing"),
|
||||
"first constraint must render in the Constraints cell; prompt was: " + prompt);
|
||||
assertTrue(prompt.contains("Use markdown links"),
|
||||
"second constraint must render in the Constraints cell; prompt was: " + prompt);
|
||||
}
|
||||
|
||||
@Test
|
||||
@DisplayName("Move 2: non-bound (recommended) skill omits constraints even when manifest has them")
|
||||
void nonBoundSkillOmitsConstraints() {
|
||||
SkillService skillService = mock(SkillService.class);
|
||||
SkillPackageResolver resolver = mock(SkillPackageResolver.class);
|
||||
SkillLessonsService lessonsService = mock(SkillLessonsService.class);
|
||||
McpSkillBridge mcpBridge = mock(McpSkillBridge.class);
|
||||
AcpSkillBridge acpBridge = mock(AcpSkillBridge.class);
|
||||
SkillUsageService usageService = mock(SkillUsageService.class);
|
||||
|
||||
SkillEntity entity = entity(7L, "pdf-builtin", "builtin");
|
||||
when(skillService.listEnabledSkills()).thenReturn(List.of(entity));
|
||||
// Manifest has constraints, but the agent does NOT bind this skill
|
||||
SkillManifest manifest = SkillManifest.builder()
|
||||
.constraints(List.of("Never delete source files"))
|
||||
.build();
|
||||
ResolvedSkill skill = resolvedWithManifest(entity, manifest);
|
||||
when(resolver.resolve(entity)).thenReturn(skill);
|
||||
when(mcpBridge.listMcpDerivedResolvedSkills()).thenReturn(List.of());
|
||||
when(acpBridge.listAcpDerivedResolvedSkills()).thenReturn(List.of());
|
||||
when(usageService.recentLoadedSkillNames(null, 8)).thenReturn(Set.of());
|
||||
when(usageService.frequentlyLoadedSkillNames(8)).thenReturn(Set.of());
|
||||
|
||||
SkillRuntimeService runtime = new SkillRuntimeService(
|
||||
skillService, resolver, lessonsService, mcpBridge, acpBridge, usageService);
|
||||
|
||||
// boundSkillIds = null → "recommended" branch, no skill is bound
|
||||
String prompt = runtime.buildSkillPromptEnhancement(null, null, 8192);
|
||||
|
||||
assertTrue(prompt.contains("| Skill | Status | Description | Constraints |"),
|
||||
"header still declares Constraints column; prompt was: " + prompt);
|
||||
assertFalse(prompt.contains("Never delete source files"),
|
||||
"non-bound skill constraints must NOT render; prompt was: " + prompt);
|
||||
}
|
||||
|
||||
@Test
|
||||
@DisplayName("Move 2: long constraints are truncated to the per-cell budget")
|
||||
void longConstraintsAreTruncated() {
|
||||
SkillService skillService = mock(SkillService.class);
|
||||
SkillPackageResolver resolver = mock(SkillPackageResolver.class);
|
||||
SkillLessonsService lessonsService = mock(SkillLessonsService.class);
|
||||
McpSkillBridge mcpBridge = mock(McpSkillBridge.class);
|
||||
AcpSkillBridge acpBridge = mock(AcpSkillBridge.class);
|
||||
SkillUsageService usageService = mock(SkillUsageService.class);
|
||||
|
||||
SkillEntity entity = entity(42L, "long-constraints-skill", "builtin");
|
||||
when(skillService.listEnabledSkills()).thenReturn(List.of(entity));
|
||||
// Build a constraint well past CONSTRAINTS_SUMMARY_LIMIT (80 chars)
|
||||
String longConstraint = "A".repeat(120);
|
||||
SkillManifest manifest = SkillManifest.builder()
|
||||
.constraints(List.of(longConstraint))
|
||||
.build();
|
||||
ResolvedSkill bound = resolvedWithManifest(entity, manifest);
|
||||
when(resolver.resolve(entity)).thenReturn(bound);
|
||||
when(mcpBridge.listMcpDerivedResolvedSkills()).thenReturn(List.of());
|
||||
when(acpBridge.listAcpDerivedResolvedSkills()).thenReturn(List.of());
|
||||
when(usageService.recentLoadedSkillNames(null, 8)).thenReturn(Set.of());
|
||||
when(usageService.frequentlyLoadedSkillNames(8)).thenReturn(Set.of());
|
||||
|
||||
SkillRuntimeService runtime = new SkillRuntimeService(
|
||||
skillService, resolver, lessonsService, mcpBridge, acpBridge, usageService);
|
||||
|
||||
String prompt = runtime.buildSkillPromptEnhancement(Set.of(42L), null, 8192);
|
||||
|
||||
// The truncation marker appears
|
||||
assertTrue(prompt.contains("..."),
|
||||
"truncated constraints must end with '...'; prompt was: " + prompt);
|
||||
// The full 120-char string is NOT present
|
||||
assertFalse(prompt.contains("A".repeat(120)),
|
||||
"long constraint must be truncated, not rendered whole; prompt was: " + prompt);
|
||||
// The truncated prefix IS present (80 chars)
|
||||
assertTrue(prompt.contains("A".repeat(80)),
|
||||
"truncated prefix (80 'A's) must be in the cell; prompt was: " + prompt);
|
||||
}
|
||||
|
||||
@Test
|
||||
@DisplayName("Move 2: pipe characters in constraints are escaped to protect the table layout")
|
||||
void pipeInConstraintsIsEscaped() {
|
||||
SkillService skillService = mock(SkillService.class);
|
||||
SkillPackageResolver resolver = mock(SkillPackageResolver.class);
|
||||
SkillLessonsService lessonsService = mock(SkillLessonsService.class);
|
||||
McpSkillBridge mcpBridge = mock(McpSkillBridge.class);
|
||||
AcpSkillBridge acpBridge = mock(AcpSkillBridge.class);
|
||||
SkillUsageService usageService = mock(SkillUsageService.class);
|
||||
|
||||
SkillEntity entity = entity(11L, "pipe-skill", "builtin");
|
||||
when(skillService.listEnabledSkills()).thenReturn(List.of(entity));
|
||||
SkillManifest manifest = SkillManifest.builder()
|
||||
.constraints(List.of("Use option A | option B"))
|
||||
.build();
|
||||
ResolvedSkill bound = resolvedWithManifest(entity, manifest);
|
||||
when(resolver.resolve(entity)).thenReturn(bound);
|
||||
when(mcpBridge.listMcpDerivedResolvedSkills()).thenReturn(List.of());
|
||||
when(acpBridge.listAcpDerivedResolvedSkills()).thenReturn(List.of());
|
||||
when(usageService.recentLoadedSkillNames(null, 8)).thenReturn(Set.of());
|
||||
when(usageService.frequentlyLoadedSkillNames(8)).thenReturn(Set.of());
|
||||
|
||||
SkillRuntimeService runtime = new SkillRuntimeService(
|
||||
skillService, resolver, lessonsService, mcpBridge, acpBridge, usageService);
|
||||
|
||||
String prompt = runtime.buildSkillPromptEnhancement(Set.of(11L), null, 8192);
|
||||
|
||||
assertTrue(prompt.contains("Use option A \\| option B"),
|
||||
"pipe must be escaped as \\|; prompt was: " + prompt);
|
||||
}
|
||||
|
||||
@Test
|
||||
@DisplayName("Move 3: bound skill with allowedTools renders the 'Bound skill allowed tools' block")
|
||||
void boundSkillWithAllowedToolsRendersBlock() {
|
||||
SkillService skillService = mock(SkillService.class);
|
||||
SkillPackageResolver resolver = mock(SkillPackageResolver.class);
|
||||
SkillLessonsService lessonsService = mock(SkillLessonsService.class);
|
||||
McpSkillBridge mcpBridge = mock(McpSkillBridge.class);
|
||||
AcpSkillBridge acpBridge = mock(AcpSkillBridge.class);
|
||||
SkillUsageService usageService = mock(SkillUsageService.class);
|
||||
|
||||
SkillEntity entity = entity(99L, "ckjia-shopping", "builtin");
|
||||
when(skillService.listEnabledSkills()).thenReturn(List.of(entity));
|
||||
SkillManifest manifest = SkillManifest.builder()
|
||||
.allowedTools(List.of("read_file", "execute_code"))
|
||||
.build();
|
||||
ResolvedSkill bound = resolvedWithManifest(entity, manifest);
|
||||
when(resolver.resolve(entity)).thenReturn(bound);
|
||||
when(mcpBridge.listMcpDerivedResolvedSkills()).thenReturn(List.of());
|
||||
when(acpBridge.listAcpDerivedResolvedSkills()).thenReturn(List.of());
|
||||
when(usageService.recentLoadedSkillNames(null, 8)).thenReturn(Set.of());
|
||||
when(usageService.frequentlyLoadedSkillNames(8)).thenReturn(Set.of());
|
||||
|
||||
SkillRuntimeService runtime = new SkillRuntimeService(
|
||||
skillService, resolver, lessonsService, mcpBridge, acpBridge, usageService);
|
||||
|
||||
String prompt = runtime.buildSkillPromptEnhancement(Set.of(99L), null, 8192);
|
||||
|
||||
assertTrue(prompt.contains("### Bound skill allowed tools"),
|
||||
"allowed-tools block header must render; prompt was: " + prompt);
|
||||
assertTrue(prompt.contains("`ckjia-shopping`: `read_file`, `execute_code`"),
|
||||
"allowed-tools line must list skill + tools; prompt was: " + prompt);
|
||||
}
|
||||
|
||||
@Test
|
||||
@DisplayName("Move 3: bound skill with no allowedTools omits the block entirely")
|
||||
void boundSkillWithoutAllowedToolsOmitsBlock() {
|
||||
SkillService skillService = mock(SkillService.class);
|
||||
SkillPackageResolver resolver = mock(SkillPackageResolver.class);
|
||||
SkillLessonsService lessonsService = mock(SkillLessonsService.class);
|
||||
McpSkillBridge mcpBridge = mock(McpSkillBridge.class);
|
||||
AcpSkillBridge acpBridge = mock(AcpSkillBridge.class);
|
||||
SkillUsageService usageService = mock(SkillUsageService.class);
|
||||
|
||||
SkillEntity entity = entity(99L, "doc-only-skill", "builtin");
|
||||
when(skillService.listEnabledSkills()).thenReturn(List.of(entity));
|
||||
// Manifest exists (so Constraints column could render) but has no
|
||||
// allowedTools — the skill is documentation-only.
|
||||
SkillManifest manifest = SkillManifest.builder()
|
||||
.constraints(List.of("Read-only"))
|
||||
.build();
|
||||
ResolvedSkill bound = resolvedWithManifest(entity, manifest);
|
||||
when(resolver.resolve(entity)).thenReturn(bound);
|
||||
when(mcpBridge.listMcpDerivedResolvedSkills()).thenReturn(List.of());
|
||||
when(acpBridge.listAcpDerivedResolvedSkills()).thenReturn(List.of());
|
||||
when(usageService.recentLoadedSkillNames(null, 8)).thenReturn(Set.of());
|
||||
when(usageService.frequentlyLoadedSkillNames(8)).thenReturn(Set.of());
|
||||
|
||||
SkillRuntimeService runtime = new SkillRuntimeService(
|
||||
skillService, resolver, lessonsService, mcpBridge, acpBridge, usageService);
|
||||
|
||||
String prompt = runtime.buildSkillPromptEnhancement(Set.of(99L), null, 8192);
|
||||
|
||||
assertFalse(prompt.contains("### Bound skill allowed tools"),
|
||||
"no allowedTools → block must be omitted; prompt was: " + prompt);
|
||||
// Constraints still render
|
||||
assertTrue(prompt.contains("Read-only"),
|
||||
"constraints still render even without allowedTools; prompt was: " + prompt);
|
||||
}
|
||||
|
||||
@Test
|
||||
@DisplayName("Move 3: non-bound skill with allowedTools does NOT render the block")
|
||||
void nonBoundSkillWithAllowedToolsOmitsBlock() {
|
||||
SkillService skillService = mock(SkillService.class);
|
||||
SkillPackageResolver resolver = mock(SkillPackageResolver.class);
|
||||
SkillLessonsService lessonsService = mock(SkillLessonsService.class);
|
||||
McpSkillBridge mcpBridge = mock(McpSkillBridge.class);
|
||||
AcpSkillBridge acpBridge = mock(AcpSkillBridge.class);
|
||||
SkillUsageService usageService = mock(SkillUsageService.class);
|
||||
|
||||
SkillEntity entity = entity(7L, "pdf-builtin", "builtin");
|
||||
when(skillService.listEnabledSkills()).thenReturn(List.of(entity));
|
||||
SkillManifest manifest = SkillManifest.builder()
|
||||
.allowedTools(List.of("read_file"))
|
||||
.build();
|
||||
ResolvedSkill skill = resolvedWithManifest(entity, manifest);
|
||||
when(resolver.resolve(entity)).thenReturn(skill);
|
||||
when(mcpBridge.listMcpDerivedResolvedSkills()).thenReturn(List.of());
|
||||
when(acpBridge.listAcpDerivedResolvedSkills()).thenReturn(List.of());
|
||||
when(usageService.recentLoadedSkillNames(null, 8)).thenReturn(Set.of());
|
||||
when(usageService.frequentlyLoadedSkillNames(8)).thenReturn(Set.of());
|
||||
|
||||
SkillRuntimeService runtime = new SkillRuntimeService(
|
||||
skillService, resolver, lessonsService, mcpBridge, acpBridge, usageService);
|
||||
|
||||
// boundSkillIds = null → no skill is bound
|
||||
String prompt = runtime.buildSkillPromptEnhancement(null, null, 8192);
|
||||
|
||||
assertFalse(prompt.contains("### Bound skill allowed tools"),
|
||||
"non-bound skill must not render the block; prompt was: " + prompt);
|
||||
}
|
||||
|
||||
@Test
|
||||
@DisplayName("Move 1+2+3: static catalog (render(Set.of())) omits both bound-only segments")
|
||||
void staticCatalogRenderedWithEmptyBoundSetOmitsBoundSegments() {
|
||||
// This mirrors how ReasoningNode now calls render(Set.of()) —
|
||||
// the static catalog must NOT contain bound-only segments
|
||||
// (Constraints cells, allowed-tools block), because no skill is
|
||||
// bound in the static-render pass.
|
||||
SkillService skillService = mock(SkillService.class);
|
||||
SkillPackageResolver resolver = mock(SkillPackageResolver.class);
|
||||
SkillLessonsService lessonsService = mock(SkillLessonsService.class);
|
||||
McpSkillBridge mcpBridge = mock(McpSkillBridge.class);
|
||||
AcpSkillBridge acpBridge = mock(AcpSkillBridge.class);
|
||||
SkillUsageService usageService = mock(SkillUsageService.class);
|
||||
|
||||
SkillEntity entity = entity(99L, "skill-with-everything", "builtin");
|
||||
when(skillService.listEnabledSkills()).thenReturn(List.of(entity));
|
||||
SkillManifest manifest = SkillManifest.builder()
|
||||
.constraints(List.of("Always confirm"))
|
||||
.allowedTools(List.of("read_file"))
|
||||
.build();
|
||||
ResolvedSkill skill = resolvedWithManifest(entity, manifest);
|
||||
when(resolver.resolve(entity)).thenReturn(skill);
|
||||
when(mcpBridge.listMcpDerivedResolvedSkills()).thenReturn(List.of());
|
||||
when(acpBridge.listAcpDerivedResolvedSkills()).thenReturn(List.of());
|
||||
when(usageService.recentLoadedSkillNames(null, 8)).thenReturn(Set.of());
|
||||
when(usageService.frequentlyLoadedSkillNames(8)).thenReturn(Set.of());
|
||||
|
||||
SkillRuntimeService runtime = new SkillRuntimeService(
|
||||
skillService, resolver, lessonsService, mcpBridge, acpBridge, usageService);
|
||||
|
||||
// boundSkillIds = null simulates the static-render pass (no skill
|
||||
// is "bound" from the cache-stability perspective)
|
||||
String prompt = runtime.buildSkillPromptEnhancement(null, null, 8192);
|
||||
|
||||
// Header still present (it's part of the table layout)
|
||||
assertTrue(prompt.contains("| Skill | Status | Description | Constraints |"),
|
||||
"header still declares the column; prompt was: " + prompt);
|
||||
// But the bound-only content is absent
|
||||
assertFalse(prompt.contains("Always confirm"),
|
||||
"bound-only constraints must not render in static pass; prompt was: " + prompt);
|
||||
assertFalse(prompt.contains("### Bound skill allowed tools"),
|
||||
"bound-only allowed-tools block must not render in static pass; prompt was: " + prompt);
|
||||
}
|
||||
|
||||
// ============================ helpers ============================
|
||||
|
||||
private static SkillEntity entity(Long id, String name, String type) {
|
||||
SkillEntity entity = new SkillEntity();
|
||||
entity.setId(id);
|
||||
entity.setName(name);
|
||||
entity.setDescription("Description for " + name);
|
||||
entity.setSkillType(type);
|
||||
entity.setEnabled(true);
|
||||
entity.setSecurityScanStatus("PASSED");
|
||||
return entity;
|
||||
}
|
||||
|
||||
private static ResolvedSkill resolvedWithManifest(SkillEntity entity, SkillManifest manifest) {
|
||||
// passesActiveGate requires hasAnyActiveFeature() when a manifest is
|
||||
// present — without an activeFeatures entry the skill is filtered out
|
||||
// by refreshActiveSkills() and the catalog comes back empty.
|
||||
Set<String> activeFeatures = new LinkedHashSet<>();
|
||||
activeFeatures.add("default");
|
||||
return ResolvedSkill.builder()
|
||||
.id(entity.getId())
|
||||
.name(entity.getName())
|
||||
.description(entity.getDescription())
|
||||
.enabled(Boolean.TRUE.equals(entity.getEnabled()))
|
||||
.runtimeAvailable(true)
|
||||
.dependencyReady(true)
|
||||
.securityBlocked(false)
|
||||
.manifest(manifest)
|
||||
.activeFeatures(activeFeatures)
|
||||
.build();
|
||||
}
|
||||
}
|
||||
@ -148,11 +148,14 @@ class ToolDisclosureServiceTest {
|
||||
}
|
||||
|
||||
@Test
|
||||
@DisplayName("MCP tool whose server has no tier set defaults to core (visible)")
|
||||
void mcpDefaultsCoreWhenServerTierUnset() {
|
||||
@DisplayName("Move 5: MCP tool whose server has no tier set defaults to EXTENSION (on-demand)")
|
||||
void mcpDefaultsExtensionWhenServerTierUnset() {
|
||||
// Move 5: MCP tools default to EXTENSION so they don't flood the
|
||||
// CORE tool list. Pre-Move-4 this returned CORE.
|
||||
var svc = service(List.of(), List.of(server(7L, "github", null)),
|
||||
List.of(mcpDto("mcp_github_create_issue", 7L)));
|
||||
assertEquals(DisclosureTier.CORE, svc.resolveTierByName("mcp_github_create_issue"));
|
||||
assertEquals(DisclosureTier.EXTENSION, svc.resolveTierByName("mcp_github_create_issue"),
|
||||
"Move 5: MCP tools with no explicit tier must default to EXTENSION");
|
||||
}
|
||||
|
||||
@Test
|
||||
|
||||
Loading…
Reference in New Issue
Block a user