ref(chat): Split agent-run request context into role-scoped groups#752
ref(chat): Split agent-run request context into role-scoped groups#752dcramer wants to merge 2 commits into
Conversation
ReplyRequestContext was a flat ~35-field bag where nearly every field was optional, so the type could not express which combinations were valid and call sites gave no hint of a field's role. Group the request into input, routing, policy, state, observers, and durability sub-objects, and make AgentRunner.run take the single grouped request (messageText now lives in input). The executor body keeps operating on the historical flat shape via a private flatten step; the flat type is an unexported intersection of the group interfaces, so no field is declared twice. The internal rewrite is deferred to the phase decomposition (#746 Phase 5), which consumes the groups directly. No behavior change, no field optionality changes; runtime destination and requester invariant checks are unchanged. Refs #746 Co-Authored-By: GPT-5.5 Codex <noreply@openai.com> Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
The chat-cli and provider-retry suites bounded their readiness polls by iteration count (20 setImmediate turns, 100 real sleeps), which starves under coverage runs with saturated workers: CI failed on chat-cli's readline poll and local full sweeps reproduced the same starvation in the provider-retry continuation polls. Make the ceilings generous real-time budgets and give the fake-timer advance loops a real-time grace period for already-scheduled continuations to settle. The loops still return as soon as the condition holds, so passing runs are unaffected. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit cb920d5. Configure here.
|
|
||
| if (!runArgs.replyContext?.requester?.userId) { | ||
| throw new Error("Resumed turn requires replyContext.requester.userId"); | ||
| if (!runArgs.replyContext?.routing.requester?.userId) { |
There was a problem hiding this comment.
Broken optional chaining on routing
Medium Severity
After nesting resume fields under routing, several checks use replyContext?.routing.requester (and similar) instead of replyContext?.routing?.requester. Optional chaining only guards replyContext; if it is missing or has no routing, evaluating .requester or .correlation throws a TypeError instead of the intended validation error or lockKey fallback.
Additional Locations (2)
Reviewed by Cursor Bugbot for commit cb920d5. Configure here.
There was a problem hiding this comment.
False positive: routing is a required property of ResumeReplyContext (Omit<AssistantReplyRequestContext, "input"> & {...} — the omit only touches input), so replyContext?.routing is the correct chain: the optional part is the container, not the group. The context is never persisted or deserialized — every producer (oauth-callback, mcp-oauth-callback, agent-continue-runner, slack-runtime) builds it in-process as a typed literal, so a missing routing cannot occur without a type error at the construction site. Adding ?. after routing would be dead defensive code (repo policy: no fallbacks when systems are expected to work) and at getResumeConversationId would silently reroute a programming error into the lockKey fallback instead of failing loudly.
1 participant
ReplyRequestContextwas a flat ~35-field bag where nearly every field was optional, so the type could not express which combinations were valid and call sites gave no hint of a field's role. The request is now grouped intoinput,routing,policy,state,observers, anddurabilitysub-objects, andAgentRunner.runtakes the single grouped request (messageTextnow lives ininput).No behavior change and no field optionality changes: every flat field maps 1:1 into exactly one group, runtime destination/requester invariant checks are unchanged, and the outcome contract from #750/#751 (
suspendedstatus, deferred pause handlers,resumeVersion) is untouched. The executor body still operates on the historical flat shape via a privateflattenReplyRequestContextstep; consuming the groups directly is deferred to the #746 Phase 5 decomposition. This ports the remaining regrouping commit from #748 onto the reworked outcome model.Review order: start with the group interfaces and flatten step in
src/chat/respond.ts, then the six call-site regroupings (runtime/reply-executor.ts,runtime/slack-resume.ts,agent-dispatch/runner.ts,local/runner.ts,runtime/agent-continue-runner.ts, the oauth handlers). The 36 test-file changes are mechanical regroupings of existing assertions — no tests added, removed, or weakened — with a sharedflattenReplyRequestForTestfixture replacing per-file copies of the flatten shim.Refs #746
🤖 Generated with Claude Code