Repository navigation
Load attention and plugin tools on search - #1329
Conversation
There was a problem hiding this comment.
- [P2] Plugin namespaces can change without invalidating saved sessions. ToolGroup naming depends on registration order, while
addedKeyignores that order. With valid plugin IDsfoo.bar,foo-bar, andfoo.bar.2, reordering registration removes the previously allocatedfoo_bar_2_2namespace. Codex resumes the existing thread, then rejects its plugin calls with “No active Buzz tool for this thread and turn.” Fix: allocate names in a deterministic plugin-ID order and cover reordered registration through session resume.
| tools: served?.tools ?? sessionTools(added), | ||
| tools: sessionTools(added).map(({ load, ...tool }) => | ||
| search && load === "always" | ||
| ? { ...tool, _meta: { "anthropic/alwaysLoad": true } } |
There was a problem hiding this comment.
can "anthropic/alwaysLoad": true go in bundled/claude-code/? buzz-mcp should be harness agnostic.
There was a problem hiding this comment.
🤖 Done in 2f01c5a65. respond now takes a Listing from its harness: an optional instructions string, plus a function that shapes how each tool is listed. buzz-mcp only says which tools are always and which are search. The _meta["anthropic/alwaysLoad"] mark and the hint as server instructions now live in bundled/claude-code/runtime.ts.
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 348696fd76
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| type: "function", | ||
| ...tool, | ||
| ...(load === "search" ? { deferLoading: true } : {}), |
There was a problem hiding this comment.
Version Codex bindings before deferring shared tools
When upgrading an existing Codex conversation, this flag is never applied: thread/resume restores the thread's original tool definitions, while the resume check in runtime.ts only keys on addedKey(added), so a pre-release binding with no plugin tools still matches "". Consequently, existing conversations keep every attention tool and its old description eagerly loaded for their entire lifetime, preventing the main token/context reduction unless the workspace or plugin-tool set changes. Include a version or hash of the shared tool/load schema in the persisted binding key so each existing thread is recreated once.
AGENTS.md reference: AGENTS.md:L126-L129
Useful? React with 👍 / 👎.
|
lgtm! |
wesbillman
left a comment
There was a problem hiding this comment.
🤖 Star Lord’s automated source review via Wes’s account, integrating independent Rocket (systems/lifecycle), Mantis (correctness/security/concurrency), and Groot (product/flows) reviews.
Reviewed head 2f01c5a65bff8af3c5ad27668560943b63801387 against base/merge-base 7d50425b7fda7c0bfdccb897df0507a286d6bbac.
9/10 for the source change: no actionable source-demonstrated defects found. The single-server listing keeps harness-specific metadata in the adapters; plugin-name validation, attention authorization, and goose’s eager listing remain intact. Reviewed the 17-file diff and relevant callers, fresh/resumed/one-off sessions, attention toggles and error handling. No UI/focus transition changes were identified. The public PR description has no attached images; no disclosure issue found in that description or the reviewed commit messages.
Compatibility validation remains open: legacy Codex bindings retain their old tool schemas, while the PR description says resumed threads receive the new prompt without Attention guidance. However, src/bundled/codex/prompt.ts:34–35 and runtime.test.ts:2150 say the original developer instructions persist. The mocked test does not settle the upstream behavior. A resume check using a pre-change binding should establish which guidance the model actually receives; if it is lost, retaining compatibility guidance would avoid discarding conversation context. We have not established this as a runtime defect and do not recommend forcing fresh threads on this evidence alone.
Validation: source-only; no tests, builds, PR code, or live harnesses executed. One hosted-check snapshot showed CI required and its Linux/browser lanes passing; Windows native validation was skipped. Author-reported live probes and token savings were not independently reproduced. Native/live behavior, provider-version compatibility, performance measurements and human acceptance remain unverified. This COMMENT is not approval or merge authorization.
Split the session tools into groups: `buzz` stays loaded up front, while `buzz_attention` and one group per plugin load when the model searches for them. Claude gets one in-process server per group (alwaysLoad only on `buzz`, the group hint as server instructions); Codex gets one dynamicTools namespace per group with deferLoading on searched groups. goose keeps one flat server, since it cannot load tools on demand. The attention guidance moves from the system prompt into the attention tools' descriptions, and an agent with no Interests gets a short end-of-turn line instead of the review, so it need not search to find nothing. Co-authored-by: mbot <3754f8729004d95654c46dbab3129e4ab9ef05cc2534e2a3fbfc155983bd637b@buzz.block.builderlab.xyz> Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Signed-off-by: Bradley Axen <baxen@squareup.com>
Drop the tool groups for one `buzz` server where each tool has a `load` of `always` or `search`. The Buzz tools load up front and the attention tools on search; tools plugins add always load on search, so only Buzz decides what every turn carries. Claude marks each up-front tool with `_meta["anthropic/alwaysLoad"]`, Codex sets `deferLoading` on each searched tool, and goose still gets them all. `sessionHint` tells the model what it can search for. Tool names no longer depend on which group serves them. Co-authored-by: mbot <3754f8729004d95654c46dbab3129e4ab9ef05cc2534e2a3fbfc155983bd637b@buzz.block.builderlab.xyz> Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Signed-off-by: Bradley Axen <baxen@squareup.com>
`respond` takes a `Listing` from its harness instead of a search flag, so buzz-mcp stays harness-neutral: the Claude runtime marks up-front tools with `_meta["anthropic/alwaysLoad"]` and gives the hint as instructions. Co-authored-by: mbot <3754f8729004d95654c46dbab3129e4ab9ef05cc2534e2a3fbfc155983bd637b@buzz.block.builderlab.xyz> Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Signed-off-by: Bradley Axen <baxen@squareup.com>
2f01c5a to
8b1020c
Compare
wesbillman
left a comment
There was a problem hiding this comment.
🤖 Star Lord’s automated source review via Wes’s account, integrating independent Rocket (systems/lifecycle), Mantis (correctness/security/concurrency), and Groot (product/flows) reviews.
Reviewed head 8b1020c286d4df72ff88c4a19c0a8d334f5e3917 against base 5992e69385474d5ee12fc542dc7a947c45ac725f.
9/10 for this rebase integration: no new actionable source defects found. Range-diff confirms all three feature patches are unchanged from the previously reviewed series ending at 2f01c5a6. We checked their integration with the new base’s storage, interruption/background-task lifecycle and media replies, including fresh, resumed, spare and one-off sessions. Tool dispatch authorization, plugin-name collision checks, Claude’s listing metadata and goose’s eager listing remain consistent with the shared contract. No UI/focus transition changes were identified. The public description has no attached images; no disclosure issue found in it or the three feature commit messages.
The prior review’s compatibility limitation remains unresolved: old Codex bindings retain their tool schemas, but source and mocked tests do not establish whether upstream resume retains the original attention guidance. No new evidence warrants a forced thread reset. Optional description clarification: qualify the “neither form” guidance claim as unverified, and identify the commit exercised by the reported live probes/tests.
Validation: source-only rebase/integration follow-up; no PR code, tests, builds, apps, live harnesses or performance probes executed. CI was not assessed this cycle. Prior author-reported tests/token savings were not independently reproduced at this head. Native/live compatibility, performance and human acceptance remain unverified. This COMMENT is not approval or merge authorization.
What
All agent tools stay on one
buzzserver, and each tool has aloadofalwaysorsearch. The thread on this PR settled on this design (one server, a flag per tool, and plugins can only use search).always): the 15 core Buzz tools (send, read, react, …).always, and the plugin contract (AddedTool) doesn't change. So installing plugins never adds to what every turn carries.sessionHinttells the model what it can search for: attention, plus the names of any plugin tools.alwaysLoad.tools/listmarks each up-front tool with_meta["anthropic/alwaysLoad"], and the server'sinstructionsis the hint.buzzdynamicToolsnamespace. Searched tools getdeferLoading, and the namespace description is the hint.Tool names don't depend on when a tool loads. Claude always sees
mcp__buzz__watch_add, and Codex always seesbuzz.watch_add. Plugin tool names share one space: they should start with the plugin's name (notes_*), andaddToolalready rejects a name that a Buzz tool or another plugin has.Two prompt changes go with this:
BUZZ_PROMPT. It now lives in the attention tools' own descriptions (interest_set,watch_addand the others), so a session pays for it only after it loads those tools.Live checks (real harnesses, probe scripts outside the repo, tool lists dumped from this branch)
sendwas called directly.interest_listwas called after oneToolSearch(select:mcp__buzz__interest_list).sendwas called directly. The model found the deferredinterest_listand called it in namespacebuzz.Compatibility
buzz. It also gets the new system prompt, which has no## Attentionsection, so until that thread is replaced it has neither form of the attention guidance. The binding key covers only plugin tools. Fixing this means keying on the whole tool list, which would start every Codex agent on a fresh thread once now and again whenever a Buzz tool changes. I left that out; it's a call for review.Known gaps
supports_search_tool, but a custom provider model might not.Checks
src/buzz-mcp,src/bundled,src/features/agents,src/features/agent-harnessandsrc/plugins(238 files, 3,509 tests).🤖 Generated with Claude Code