Skip to content

Restore state-backed tool call IDs - #617

Closed
brandonkachen wants to merge 6623 commits into
mainfrom
jahooma/restore-tui-change-safely
Closed

Restore state-backed tool call IDs#617
brandonkachen wants to merge 6623 commits into
mainfrom
jahooma/restore-tui-change-safely

Conversation

@brandonkachen

Copy link
Copy Markdown
Collaborator

Summary

  • Restore deterministic tool call IDs using the functions.<tool>.<index> shape with a run/session-backed monotonic counter.
  • Seed the counter from retained history and pending calls, including the older colon-delimited deterministic ID shape.
  • Share the counter through spawned agent state while keeping the public agent template types free of internal allocator state.

Validation

  • bun test packages/agent-runtime/src/util/__tests__/tool-call-id.test.ts packages/agent-runtime/src/__tests__/run-programmatic-step.test.ts packages/agent-runtime/src/__tests__/tool-validation-error.test.ts
  • bun run --cwd packages/agent-runtime typecheck
  • bun run --cwd common typecheck
  • bun run --cwd packages/agent-runtime test
  • NODE_ENV=production bun test cli/src/utils/__tests__/sdk-event-handlers.test.ts cli/src/utils/__tests__/message-block-helpers.test.ts cli/src/utils/__tests__/send-message-helpers.test.ts
  • bun run --cwd cli typecheck
  • bun run --cwd cli src/index.tsx --help

github-actions[bot] and others added 30 commits April 19, 2026 08:20
Emit queueDepth and activeCount every 15s with metric=freebuff_waiting_room
so the waiting line length and concurrent admitted users can be charted over
time from the log stream.

Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
PREFILL_QUEUE_DEGRADED_MS was lowered to 200 in 59640d5, so the
"healthy baseline" fixtures using p50=300 now classify as degraded.
Drop them to 150 to keep the healthy-path tests meaningful.

Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
Deep-thinking models (Minimax M2.5, Kimi K2.5, GLM-5.1, GPT-5) can spend
15+ minutes in the reasoning phase before emitting the first token.
The 10-min headersTimeout was cutting them off mid-think and surfacing
as "Agent run error: The operation timed out."

Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
Decouples the user-facing wait estimate from the admission tick rate.
The estimate is now a rough one-minute-per-spot rule of thumb, which
reads more intuitively in the CLI than a tick-derived number that
shifts with deployment cadence.

Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
Opus 4.7's tokenizer differs from most models; using 4.6 gives more
representative counts for open-source models routed through the
Anthropic count_tokens endpoint.

Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
@codebuff-team

ghost commented Aug 20, 2026

Copy link
Copy Markdown
Contributor

The core of this PR — reintroducing functions.<tool>.<index> deterministic IDs backed by a monotonic counter in AgentState.toolCallState, seeded from history (including the legacy colon-delimited shape) and pending calls — is well done. tool-call-id.ts is clean, createToolCallIdGenerator/ensureToolCallState correctly thread the counter through spawned agents, and the new test file (tool-call-id.test.ts) plus updates to run-programmatic-step.test.ts and tool-validation-error.test.ts cover the important cases (history seeding, hidden/legacy IDs, shared counter across spawned agents).

What's concerning is that packages/agent-runtime/src/tools/tool-executor.ts also contains a second, unrelated change to agent-spawn validation that has nothing to do with tool call IDs:

  • agentIdToLoad is now normalized via normalizeAgentIdForLookup for base agents, and the returned agent object is rewritten with agent_type: agentIdToLoad.
  • The effectiveInput = { ...effectiveInput, agents: validAgents } line was moved outside the if (errors.length > 0) block, so it now runs unconditionally instead of only when some agents failed to spawn.

Neither of these is mentioned in the PR description, and there's no test covering the new unconditional overwrite of effectiveInput.agents. That's a real behavior change (previously a successful spawn left effectiveInput untouched; now it always gets rebuilt from validAgents), and mixing it into a PR titled 'Restore state-backed tool call IDs' makes it easy for a reviewer to miss and hard to isolate for porting.

Please split the spawn-agent normalization change into its own PR with its own tests and description so each change can be evaluated (and reverted, if needed) independently. The tool-call-id work itself looks portable once separated.

@codebuff-team codebuff-team added bot:triaged Classified by the community triage bot pr:needs-work Right idea, not mergeable as written labels Aug 20, 2026
@victorxheng
victorxheng force-pushed the jahooma/restore-tui-change-safely branch from 9361c1b to 1f5ce3c Compare August 31, 2026 21:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bot:triaged Classified by the community triage bot pr:needs-work Right idea, not mergeable as written

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants