Skip to content

fix(agent-runtime): restore deferred ToolSearch activations - #922

Merged
vastsa merged 4 commits into
vastsa:mainfrom
yg2224:fix/issue913-toolsearch-restore
Sep 23, 2026
Merged

vastsa merged 4 commits into
vastsa:mainfrom
yg2224:fix/issue913-toolsearch-restore

Conversation

@yg2224

@yg2224 yg2224 commented Sep 23, 2026 •

Copy link
Copy Markdown
Contributor

Summary

Fixes #913.

  • Write successful ToolSearch activations to canonical details.addedToolNames.
  • Restore canonical markers on the next prompt, while accepting historical details.activated and top-level addedToolNames rows.
  • Preserve the same compatibility when replaying resumed subagent history.
  • Keep existing success/error, placeholder, current-mode catalog, and permission boundaries unchanged.
  • Add regression coverage for activation without a tool call, partial tool use, legacy rows, and subagent replay.
  • Update the English and Chinese runtime/E2E specifications.

The cache-prefix observation in the issue is left separate because this change addresses the reproduced field mismatch directly; no unverified cache diagnosis was added.

Validation

  • pnpm --filter @pi-desktop/agent-runtime test — 68 files, 1,025 tests passed
  • pnpm --filter @pi-desktop/agent-runtime typecheck
  • pnpm --filter @pi-desktop/agent-runtime build
  • pnpm build:js
  • pnpm lint
  • pnpm docs:check
  • pnpm check:pr-base
  • pnpm test:e2e — 23/23 executed passed; 2 live-model checks skipped because PI_DESKTOP_TEST_API_KEY is unset
  • pnpm test:e2e:subagents — 34/34 passed
  • pnpm test:e2e:subagent-models — all fixture scenarios passed
  • cargo build -p host-core

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot review overview

🟡 Changes recommended

Fix canonical marker precedence and refresh restored tools after compaction.

Get a fresh assessment by requesting another Copilot review.

Review effort: Lite
Findings: 1 Medium severity

Open (1)
What changed in this PR

Restores deferred ToolSearch activations across prompts and runtime reloads.

Changes:

  • Persists canonical activation markers with legacy compatibility.
  • Adds runtime and delegation-history regression coverage.
  • Updates English and Chinese specifications.
File Summary
packages/​agent-runtime/​src/​runtime.ts Restores and persists activation markers.
packages/​agent-runtime/​src/​runtime.test.ts Tests runtime restoration behavior.
packages/​agent-runtime/​src/​delegation-history.ts Migrates legacy markers during resume.
packages/​agent-runtime/​src/​delegation-history.test.ts Tests delegation-history compatibility.
docs/​zh-CN/​spec/​06-delivery/​04-e2e-test-plan.md Updates Chinese E2E expectations.
docs/​zh-CN/​spec/​03-runtime/​03-tools-and-permissions.md Updates Chinese tool contracts.
docs/​zh-CN/​spec/​03-runtime/​02-agent-runtime.md Updates Chinese runtime behavior.
docs/​spec/​06-delivery/​04-e2e-test-plan.md Updates English E2E expectations.
docs/​spec/​03-runtime/​03-tools-and-permissions.md Updates English tool contracts.
docs/​spec/​03-runtime/​02-agent-runtime.md Updates English runtime behavior.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment on lines +468 to +474
const details =
legacyAddedToolNames.length > 0
? {
...(isRecord(rawDetails) ? rawDetails : {}),
addedToolNames: [...new Set(legacyAddedToolNames)],
}
: rawDetails;

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in cd3af1c. Delegate history now only promotes legacy top-level addedToolNames when details.addedToolNames has no valid names; canonical persisted markers remain authoritative. Added a regression assertion with divergent canonical and legacy arrays.

@yg2224

yg2224 commented Sep 23, 2026

Copy link
Copy Markdown
Contributor Author

Review follow-up: cd3af1c preserves canonical details.addedToolNames when legacy top-level markers disagree; legacy values are only promoted when canonical names are absent. Targeted agent-runtime tests (284/284), typecheck, lint, and pnpm test:e2e:subagent-models all pass on the updated commit.

@yg2224

yg2224 commented Sep 23, 2026

Copy link
Copy Markdown
Contributor Author

All required CI checks pass. I attempted the repository merge workflow, but GitHub returned MergePullRequest permission denied for the contributor account, so the PR remains open for a maintainer to merge. The branch is up to date with origin/main.

@yg2224

yg2224 commented Sep 23, 2026

Copy link
Copy Markdown
Contributor Author

After origin/main advanced to 510c3b5, I merged that latest base into the branch (candidate 8d29588) and reran the affected runtime tests and subagent E2E. PR-base, JS, Rust, and Docs CI are all green. Merge remains maintainer-only because this account lacks MergePullRequest permission.

@vastsa
vastsa merged commit 2af000b into vastsa:main Sep 23, 2026
4 checks passed
@vastsa

vastsa commented Sep 23, 2026

Copy link
Copy Markdown
Owner

Merged into main as 2af000ba1b8096622f5cb6fe1a675d1d9b2809cd, preserving the contributor commits. The branch was updated against the then-current main before merge; all required checks passed. On the matching candidate tree, local validation passed: agent-runtime (68 files / 1,025 tests), typecheck/build, test:e2e:subagent-models, and test:e2e (23/23; two live-model checks skipped because no test API key was set). The final merge also included only concurrent release-workflow/docs changes from main; no additional runtime delta. Issue #913 was auto-closed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug] ToolSearch 激活恢复字段不一致,引发跨回合工具集变化与潜在缓存前缀失效

3 participants