fix(agent): bound recovery requests and harden the structured-output example#17
Conversation
…inal review Follow-up to #15 — final max-effort review findings on the example/README: - All settle/interrupt/archive attempts now carry AbortSignal.timeout: the interrupt fallback goes through client.interruptChat (was unbounded agent.interrupt()), and archiveQuietly calls archiveChat/interruptChat with real signals — a stalled request is cancelled, not abandoned (no duplicate in-flight requests, no ref'd socket pinning the event loop). The bespoke bounded()/TIMED_OUT race and its stale module-scope stack are gone; the deadline is checked BEFORE each attempt so 15s means 15s. - READ: last VALID call wins — a schema-invalid re-file no longer shadows a valid answer already filed earlier in the turn. - structuredOutput(schema, { tools }) merges extra client tools into the ToolSet so the spread cannot silently clobber user tools; documented. - Tool errors submit the SDK-style message (string/Error/JSON-stringify), never "[object Object]"; the toModelOutput boundary is documented. - README rule 4 now reads outcomes off content parts with is_error, in sync with the example. - CODER_TOOL_MODEL/CODER_MODEL fallbacks hoisted into loadEnv with || semantics shared by examples 03 and 06. Change-Id: I921c77b539f9376260e51d332777a917a15872fb Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Signed-off-by: Thomas Kosiewski <tk@coder.com>
|
@codex review |
|
Live smoke completed: the deployment's event stream recovered overnight, and the reworked example runs clean against dev.coder.com — |
|
Codex Review: Didn't find any major issues. Can't wait for the next one! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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". |
Final-gate review of #17's diff: - README rule 4 and the inline snippet taught what the example just replaced: unbounded agent.interrupt()/archive() recovery and last-call-wins reading. Both now match the example — bounded client.interruptChat/archiveChat with AbortSignal, last-VALID-call-wins scan — and the structuredOutput(schema, { tools }) composition path (and its constructor-clobber hazard) is documented. - structuredOutput throws on an opts.tools entry named structured_output instead of silently replacing the caller's tool. - errorText is total: returns a string for symbols/functions (JSON.stringify yields undefined) and circular values (String() can throw), never throws. - archiveQuietly logs the last failure when the deadline expires and skips the doomed interrupt+sleep once the budget is spent. - loadEnv's printed setup help documents CODER_TOOL_MODEL; dead array copy in the READ scan dropped. Deliberate tradeoffs kept (documented in-file): last-valid-wins returns an earlier valid answer instead of burning the nudge on an invalid re-file; the archive deadline is strict rather than overtime-forgiving. Change-Id: Ia7a326c11af9e3b015021dee32eecf8c89d481c6 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Signed-off-by: Thomas Kosiewski <tk@coder.com>
|
@codex review |
|
Codex Review: Didn't find any major issues. 🚀 Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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". |
…lly total Convergence review of the previous commit: - README rule 4: the settle fragment now carries the AbortSignal third arg (its own bold sentence mandates bounding every recovery request), and the prose spells out the agent.chatId guard (undefined before the first turn) instead of prescribing a fragment that neither typechecks nor no-ops safely. - errorText: an Error subclass with a throwing `message` getter could escape the try blocks — the instanceof branch is now wrapped too, so the totality contract holds. Change-Id: I8580e2de0d4326f29fc40cccbbd4625df21803f5 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Signed-off-by: Thomas Kosiewski <tk@coder.com>
|
@codex review |
|
Codex Review: Didn't find any major issues. Chef's kiss. Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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". |
1 participant
Follow-up to #15, carrying the final max-effort review findings that were still local when #15 merged. All changes are in the example/docs surface (no library code):
client.interruptChat(chatId, AbortSignal.timeout(8s))(was an unboundedagent.interrupt()— the one path reached precisely when the server is stalled could hang ~5 min).archiveQuietlynow callsarchiveChat/interruptChatwith real signals, so a stalled attempt aborts the socket instead of stacking duplicate in-flight requests and pinning the event loop; the bespokebounded()/TIMED_OUTrace is gone, and the 15s deadline is checked before each attempt so it actually bounds the function.structuredOutput(schema, { tools })merges extra client tools into the ToolSet, closing the...agentOptsspread-clobber trap; documented at the call site.Error.message/ JSON-stringify — never"[object Object]"); thetoModelOutputboundary is documented explicitly.tool-errorparts submitted withis_error: true.CODER_MODEL/CODER_TOOL_MODELfallbacks hoisted intoloadEnv()with||semantics, shared by examples 03 and 06.Deliberately deferred (candidate follow-up issue): package-level ownership of the stranded-turn settle and a wind-down-aware
archive()/asyncDispose— both flagged by review as library-grade recovery currently living in example code.Verification
pnpm check(format, lint, typecheck) and unit tests green (50/50).🤖 Generated with Claude Code