feat(agent): stream correctness, hardening, lifecycle, and missing helpers from the bug-report sweep#19
Merged
Merged
Conversation
- TurnTranslator duplicated delta-streamed assistant text/reasoning on every message after the first: the trailing snapshot's id differs from the previous snapshot's, so the new-message boundary reset sawDelta and the length cursor, re-emitting the full text (multi-round turns yielded "ABBCC" and a stale open block absorbed the next message's deltas). Track deltas-arrived-since-the-last-assistant-snapshot; when set, skip the boundary close/reset so that snapshot stays a text/reasoning no-op for the message it finalizes. Snapshot-only messages (no deltas since the previous snapshot) keep the reset and still diff-and-emit. - 'source' message parts were silently dropped. Emit them as standalone V3 url sources (id = source_id, falling back to url; parts without a url are skipped) from both the message_part and snapshot paths, deduped by emitted id so trailing snapshots don't double-emit. doGenerate already forwarded source stream parts into content. - Surface server-side cost: optional total_cost_micros/total_runtime_ms on ChatMessageUsage; finish parts carry providerMetadata.coder with exactly the wire-present keys (omitted entirely when none, so old servers look unchanged) and usage.raw carries the verbatim wire usage; doGenerate forwards the finish part's providerMetadata into its result. Change-Id: I304af4c533b286cb9b641af4aa6cb4b742c41e31 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Signed-off-by: Thomas Kosiewski <tk@coder.com>
- resolveModelConfigId: tolerate partial model-config payloads (entries
missing provider/model/display_name, null entries, empty or non-array
bodies) instead of rejecting the turn with a TypeError; the no-match
contract (undefined => chatd picks its default) is unchanged. Mark
provider/model/display_name optional on ChatModelConfig to mirror the
wire format across server versions.
- interruptChat: the second parameter now also accepts
{ wait?, signal? } (a positional AbortSignal still works). wait: true
sends ?wait=true asking the server to hold the response until the run
actually stops; servers without the param ignore it harmlessly and
keep returning immediately.
- watchChats: new client helper wrapping the per-user
/api/experimental/chats/watch WebSocket (chat lifecycle events) with
automatic redial and exponential backoff (1s doubling to a 30s cap,
reset once an event arrives), prompt abort via signal, and a terminal
CoderApiError on 4xx upgrade rejections instead of retrying forever.
Change-Id: I5a0b6d083f373c0f284d0dd201385a0c35edfad5
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Signed-off-by: Thomas Kosiewski <tk@coder.com>
…eview helpers
- WS-3: new src/ai-version.ts asserts the installed `ai` major is 6 at
CoderAgent/CoderLanguageModel construction, with an actionable error
(install ai@^6). Resolution goes through createRequire on
process.getBuiltinModule so it fails open on any non-Node runtime,
bundler, or missing module; the verdict is memoized and the
parse/decision logic is a pure, unit-tested function.
- WS-5c: interrupt()/archive() now accept { signal } and plumb it to the
client. archive() retries only settling 409s (~1s apart, ~15s overall,
both overridable via settleDeadlineMs/settleRetryDelayMs settings) and
rethrows the last 409 at the deadline; other errors rethrow
immediately. [Symbol.asyncDispose] shares one bounded deadline across
interrupt + archive-with-retry and still never throws (JSDoc'd
tradeoff: call archive() directly for guaranteed cleanup).
- WS-6: new src/coder/workspaces.ts wraps the stable v2 endpoints
(workspace lookup, wildcard app host, port-share upsert) with the chat
client's auth/error conventions, and composes them into
CoderAgent.getPreview / sharePreview building the dashboard's
{port}{s?}--{agent}--{workspace}--{owner} subdomain URL. Single-agent
default with name-listing errors, clear failures for missing
workspaceId/credentials/wildcard host, and a port-share 404 mapped to
a "server may predate port sharing" hint for old deployments. New
option/result types exported from the package index.
Change-Id: Iaab1f9092469b66861747cb3bafccc2eabfca0e6
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Signed-off-by: Thomas Kosiewski <tk@coder.com>
- Add src/model/history.ts: pure batch converter from persisted chatd
history (client.getMessages) to AI SDK v6 UIMessage[] for
useChat({ messages }) rehydration
- Fold role:tool messages' tool-result parts into the originating
assistant dynamic-tool part (output-available / output-error via
is_error / input-available when unresolved); tool messages produce no
standalone UIMessage
- Map text/reasoning with state done, source parts to source-url
(sourceId falls back to url, skipped without url), file parts to file
UI parts only when a URL resolves (opts.fileUrl, then part.url)
- Skip file-reference/context-file/skill/unknown part kinds so history
from newer Coder servers degrades gracefully
- Export from index and cover with unit tests modeled on the wire
fixtures in translate.test.ts
Change-Id: Ic0e76d10f0654d8188fdd8e39ad9f9bfc7c18813
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Signed-off-by: Thomas Kosiewski <tk@coder.com>
…UUID - add optional `id` to WorkspaceStatus and parse the workspace UUID from `coder list -o json` (tolerates old CLIs that omit it — never required) - export ensureCoderWorkspace(): standalone get-or-create + start-if-stopped + wait-until-ready, sharing internals with createCoderWorkspace's create mode via a new ensureWorkspaceReady core (error messages name the actual public entry point); returns the final status plus `id?`/`name`/`created` so composers can bind e.g. a CoderAgent chat via `workspaceId: ws.id` - export EnsureCoderWorkspaceSettings/EnsuredCoderWorkspace from the index - tests: id parsing present/absent, ensure happy path, create-when-missing, start-if-stopped, ready polling, preset validation, ifExists/timeout/abort errors, export surface, and fake-coder integration coverage for the UUID Change-Id: I56472f5e0d8b8ca32c7ec406cb18ed24dd72241a Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Signed-off-by: Thomas Kosiewski <tk@coder.com>
…l migration note - pin install line to ai@^6 and document the version guard - server-tool dynamic-tool migration note (key off toolName, not part.type) - new sections: history rehydration, watching chats, workspace previews, usage & cost, sources; lifecycle settling/dispose semantics - sandbox: ensureCoderWorkspace + CoderAgent workspaceId composition - re-export watchChatEvents from the package root Change-Id: Ie4bb2fefe180b6730b8eea81cef5d1142a7b11a7 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Signed-off-by: Thomas Kosiewski <tk@coder.com>
- keep the reconnect backoff timer ref'd so a standalone watch loop
survives redial delays instead of silently exiting (abort still
clears it); regression-tested via prompt-teardown tests
- wrap watchChatEvents so iterator return()/throw() abort the loop
immediately instead of queuing behind a pending read on this
rarely-eventing stream (socket closes promptly)
- clamp settleDeadlineMs/settleRetryDelayMs to valid timer delays so
AbortSignal.timeout can never make dispose throw (Infinity means max)
- qualify the live/rehydrated tool-typing identity claim (client
ToolSet tools stream as static tool-{name} parts; recommend keying
off the tool name) in history docs and README
- update the stale example comment: archive()/await using now retry
the settling 409 window themselves
Change-Id: I310c5e5d5a2cbfef95cc1936009d5c5f21b9df85
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Signed-off-by: Thomas Kosiewski <tk@coder.com>
Member
Author
|
@codex review |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 288497fd91
ℹ️ 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".
…ew ports
Codex review follow-ups, both verified against coder/coder source:
- chatMessagesToUIMessages now sorts by message id: the messages endpoint
pages newest-first by default (GetChatMessagesByChatIDDescPaginated),
so passing the response straight into useChat rendered transcripts
reversed; the converter accepts any input order now
- getPreview/sharePreview reject ports below 1000 with an actionable
error: appurl.PortRegex (^\d{4,5}s?$) only treats 4-5 digit labels
as ports, so an 80--agent-- URL would be parsed as an app named "80"
and never resolve
Change-Id: I8b494128052d1cb2e44bd1a4ceb2b09b3a3b0a07
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Signed-off-by: Thomas Kosiewski <tk@coder.com>
Member
Author
|
@codex review Both P2s addressed in abf3f9a (verified against coder/coder source first): |
|
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". |
…n terminal exit
Final-review follow-ups on the watchChatEvents cancellation wrapper:
- add [Symbol.asyncDispose] so `await using` keeps working like the
native async generator the wrapper replaced
- detach the chained {once} abort listener when the loop settles for
good (done or terminal 4xx rejection) — the iteration protocol never
calls return() on a failed iterator, so re-created watchers on one
long-lived signal would otherwise accumulate listeners
Change-Id: I1d17f55eeeb8fc6f7eecfb290eaaf9c82def3ca1
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Signed-off-by: Thomas Kosiewski <tk@coder.com>
Member
Author
|
@codex review Final-review follow-ups on the watch wrapper in |
|
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". |
This was referenced Jul 9, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Implements every SDK-side item from the external bug report (verified item-by-item beforehand; WS numbers below reference it), in one integrated branch.
Stream correctness (
TurnTranslator)messagesnapshots no longer re-emit text/reasoning that already streamed as deltas. A#deltasSinceSnapshotflag makes the new-assistant-message boundary a no-op for text/reasoning when the snapshot merely finalizes streamed deltas, while snapshot-only messages (the server-tools fixture path) still emit. Multi-round turns now yieldA,B,C— previouslyA,B,BC,Cwith corrupted block boundaries, which also pollutedgenerateTextcontent.sourceparts were silently dropped: both ingest paths now emit AI SDKsourceparts (sourceType: "url", id fromsource_idfalling back tourl), deduped across delta/snapshot paths;result.sourcesandsource-urlUI parts populate.total_cost_micros/total_runtime_ms(exact upstream names) onChatMessageUsage, surfaced verbatim viaproviderMetadata.coderonfinishplususage.raw. Absent on old servers → absent in output, no behavior change.Client hardening & helpers (
CoderChatClient)resolveModelConfigIdcrashed with a bareTypeErroron payloads with missingprovider/model/display_name, undefined/non-array bodies, or null entries (all repro'd). Now fully guarded with the documented no-match contract preserved (undefined→ server default). Matching precedence is unchanged.interruptChat(chatId, { wait: true })sends?wait=true(non-breaking overload; positionalAbortSignalstill works). Upstream has nowaitsupport yet — the JSDoc/README say so honestly; old servers ignore the param.watchChats()/watchChatEvents: wraps the (real, verified) WebSocketGET /api/experimental/chats/watch— per-authenticated-user scope — as an async iterable with 1s→30s exponential backoff (reset on events), prompt teardown onsignalabort and bare iteratorreturn()/throw()even mid-read, and terminalCoderApiErroron 4xx upgrades (bad token, pre-watch servers).Agent layer (
CoderAgent)aimajor-version guard: constructors throw an actionableCoderAgentErrorwhen the installedaimajor ≠ 6 (withai@7the SDK previously failed with a cryptic Proxy brand-checkTypeError/ silently emptytextStream). Fails open whenever the version can't be resolved (bundlers, exotic runtimes).interrupt({ signal? })/archive({ signal? });archive()retries the post-interrupt 409 wind-down window (~1s apart, ~15s deadline, 409-only), andawait usingdisposal is bounded and never throws. Knobs (settleDeadlineMs/settleRetryDelayMs) are sanitized so hostile values can't make dispose throw. Closes the known deferred follow-ups from the fix(agent): mark server tool calls dynamic so the tool loop continues past them #15/fix(agent): bound recovery requests and harden the structured-output example #17 reviews (dispose retry + signal params).getPreview({ port })/sharePreview({ port, shareLevel })composed from the stable v2 APIs (workspace lookup + wildcard apps host + port-share upsert; URL format verified againstcoderd/workspaceapps/appurl). No chat-preview endpoint exists upstream — this is composition only, with clear errors for missing wildcard host / pre-port-sharing servers.History & sandbox
chatMessagesToUIMessages()(model/history.ts): persistedgetMessages()history →UIMessage[]foruseChat({ messages })rehydration;role:"tool"results folded intodynamic-toolparts, sources/files mapped (files via afileUrlresolver), unknown part kinds skipped forward-compatibly.ensureCoderWorkspace()exported from the sandbox package (get-or-create → start → wait-ready) andWorkspaceStatusnow carries the workspace UUID (id) parsed fromcoder list -o json, enabling the documented composition withnew CoderAgent({ workspaceId: ws.id }). (feat(sandbox): export ensureCoderWorkspace and surface the workspace UUID)Docs (WS-1 residual, WS-11/12)
Install line pinned to
ai@^6; v0.2.1 migration note (server tools surface asdynamic-toolUI parts — key offtoolName, notpart.type); new sections for rehydration, watching, previews, usage & cost (finish-stepmetadata forwarding), sources; lifecycle/cleanup semantics; sandbox provisioning + composition example. WS-1 itself (phantom tool errors) had already shipped in 0.2.1 (#15) — only its doc note was outstanding.Not included (deliberately)
GET /chats/{chat}/costis still an unmerged server PR (#26649); the optional usage fields here tolerate whatever ships.Validation
pnpm checkand the full suite pass: 146 agent tests (50 on main) and 124 sandbox tests. Every wire/endpoint claim was verified againstcoder/coder@mainsource before implementation (watch endpoint shape, no?waitsupport, aggregate-only cost fields, no preview endpoints,appurlformat, port-share API, CLIidfield). The integrated diff then went through an adversarial multi-agent review; confirmed findings (unref'd backoff timer silently exiting standalone watchers, iteratorreturn()queuing behind pending reads, dispose-throwing settle knobs, two doc overclaims) are fixed in the final commit with regression tests.Old-server tolerance throughout: new wire fields are optional, unknown query params are harmless, and new endpoints fail with clear errors on older deployments.
🤖 Generated with Claude Code