Handle previously ignored error return values in coderd:
- coderd/chats.go: check sendEvent errors, log on failure
- coderd/chatd/chattest: thread testing.TB through server structs,
replace log.Printf with t.Logf, check writeSSEEvent errors
- coderd/chatd/chattool/createworkspace.go: log UpdateChatWorkspace
failure instead of discarding both return values
- coderd/chatd/chattool/execute.go: surface ProcessOutput error in
the timeout message returned to the caller
- coderd/provisionerdserver: log stream.Send failure in the
DownloadFile error helper
- Adds `extractJSON()` to strip markdown code fences before JSON parsing and wire into the `json.Unmarshal` call in `generateFromAnthropic`.
- Accepts variadic `RequestOption` in `generateFromAnthropic` so tests can inject a mock Anthropic server via `WithBaseURL`.
- Adds table-driven cases covering bare JSON, fenced with/without language tag, surrounding whitespace, and multiline JSON.
- Adds end-to-end cases using `httptest.NewServer` to serve fake Anthropic SSE streams with bare and fenced responses.
Add cost tracking for LLM chat interactions with microdollar precision.
## Changes
- Add `chatcost` package for per-message cost calculation using
`shopspring/decimal` for intermediate arithmetic
- **Ceil rounding policy**: fractional micros round UP to next whole
micro (applied once after summing all components)
- Database migration: `total_cost_micros` BIGINT column with historical
backfill and `created_at` index
- API endpoints: per-user cost summary and admin rollup under
`/api/experimental/chats/cost/`
- SDK types: `ChatCostSummary`, `ChatCostModelBreakdown`,
`ChatCostUserRollup`
- Fix `modeloptionsgen` to handle `decimal.Decimal` as opaque numeric
type
- Update frontend pricing test fixtures for string decimal types
## Design decisions
- `NULL` = unpriced (no matching model config), `0` = free
- Reasoning tokens included in output tokens (no double-counting)
- Integer microdollars (BIGINT) for storage and API responses
- Price config uses `decimal.Decimal` for exact parsing; totals use
`int64`
Frontend: #23037
Migration 000434 converts chat_messages.role from text to a Postgres
enum, rebuilds the partial index, and adds content_version smallint.
The column is backfilled with DEFAULT 0, then the default is dropped
so future inserts must set it explicitly.
Version 0 uses the role-aware heuristic from #22958. Version 1 (all
new inserts) stores []ChatMessagePart JSON for all roles, including
system messages. ParseContent takes database.ChatMessage directly
and dispatches on version internally. Unknown versions error.
All string(codersdk.ChatMessageRole*) casts at DB write sites are
replaced with database.ChatMessageRole* constants from sqlc.
Refs #22958
File-reference parts in user messages were flattened to `TextContent` at
write time because fantasy has no file-reference content type. The
frontend never saw them as structured parts.
This moves all write paths (user, assistant, tool) from fantasy envelope
format to `codersdk.ChatMessagePart`. The streaming layer (`chatloop`)
is untouched, the conversion happens at the serialization boundary in
`persistStep`.
Old rows are still readable. `ParseContent` uses a structural heuristic
(`isFantasyEnvelopeFormat`) to distinguish legacy envelopes from SDK
parts. We chose this over try/fallback because fantasy envelopes
partially unmarshal into `ChatMessagePart` (the `type` field matches)
while silently losing content. A guard test enforces that no SDK part
can produce the envelope shape.
This is forward-only: new rows are unreadable by old code. Chat is
behind a feature flag so rollback risk is contained.
Also adds a typed `ChatMessageRole` to replace raw strings and
`fantasy.MessageRole*` casts at the persistence boundary. The type
covers `ChatMessage.Role`, `ChatStreamMessagePart.Role`, the
`PublishMessagePart` callback chain, and all DB write sites.
`fantasy.MessageRole*` remains only where we build `fantasy.Message`
structs for LLM dispatch.
Separately, `ProviderMetadata` was leaking to SSE clients via
`publishMessagePart`. `StripInternal` now runs on both the SSE and REST
paths, covering this.
Other cleanup:
- Old `db2sdk.contentBlockToPart` silently dropped metadata on
text/reasoning/tool-call content. New code preserves it.
- `providerMetadataToOptions` now logs warnings instead of silently
returning nil.
- `db2sdk` shrinks from ~250 lines of parallel conversion to ~15 lines
delegating to `chatprompt.ParseContent()`, removing the `fantasy` import
entirely.
Refs #22821
_Disclaimer: implemented by a Coder Agent using Claude Opus 4.6._
Marks the injected MCP approach in AI Bridge as deprecated across the
codebase.
## Changes
- **`codersdk/deployment.go`**: Deprecated `ExternalAuthConfig.MCPURL`,
`.MCPToolAllowRegex`, `.MCPToolDenyRegex` fields; deprecated and hid the
`--aibridge-inject-coder-mcp-tools` server flag; deprecated
`AIBridgeConfig.InjectCoderMCPTools`.
- **`coderd/externalauth/externalauth.go`**: Deprecated `Config.MCPURL`,
`.MCPToolAllowRegex`, `.MCPToolDenyRegex`.
- **`enterprise/aibridgedserver/aibridgedserver.go`**: Added runtime
deprecation warning when `CODER_AIBRIDGE_INJECT_CODER_MCP_TOOLS` is
enabled; deprecated `getCoderMCPServerConfig`.
- **`enterprise/aibridged/mcp.go`**: Deprecated `MCPProxyBuilder`
interface and `MCPProxyFactory` struct.
- **`docs/ai-coder/ai-bridge/mcp.md`**: Added deprecation warning
banner.
## Summary
Adds a new `GET /api/v2/debug/profile` endpoint that collects multiple
pprof profiles in a single request and returns them as a tar.gz archive.
This allows collecting profiles (including block and mutex) without
requiring `CODER_PPROF_ENABLE` to be set, and without restarting
`coderd`.
Closes#21679
## What it does
The endpoint:
- Temporarily enables block and mutex profiling (normally disabled at
runtime)
- Runs CPU profile and/or trace for a configurable duration (default
10s, max 60s)
- Collects snapshot profiles (heap, allocs, block, mutex, goroutine,
threadcreate)
- Returns a tar.gz archive containing all requested `.prof` files
- Uses an atomic bool to prevent concurrent collections (returns 409
Conflict)
- Is protected by the existing debug endpoint RBAC (owner-only)
**Supported profile types:** cpu, heap, allocs, block, mutex, goroutine,
threadcreate, trace
**Query parameters:**
- `duration`: How long to run timed profiles (default: `10s`, max:
`60s`)
- `profiles`: Comma-separated list of profile types (default:
`cpu,heap,allocs,block,mutex,goroutine`)
## Additional changes
- **SDK client method** (`codersdk.Client.DebugCollectProfile`) for easy
programmatic access
- **`coder support bundle --pprof` integration**: tries the consolidated
endpoint first, falls back to individual `/debug/pprof/*` endpoints for
older servers
- **8 new tests** covering defaults, custom profiles, trace+CPU,
validation errors, authorization, and conflict detection
## Summary
Moves the messages response out of `GET /chats/{id}` and into a
dedicated `GET /chats/{id}/messages` endpoint.
### Backend
- `GET /chats/{id}` now returns just the `Chat` object (no messages)
- `GET /chats/{id}/messages` is a new endpoint returning
`ChatMessagesResponse` with `messages` and `queued_messages`
- Added `ChatMessagesResponse` SDK type and `GetChatMessages` client
method
### Frontend
- `getChat()` API method returns `Chat` instead of `ChatWithMessages`
- Added `getChatMessages()` API method for the new endpoint
- Split `chatQuery` into two: `chatQuery` (metadata) and
`chatMessagesQuery` (messages)
- Updated all cache mutations, optimistic updates, and websocket
handlers
- Updated tests and stories
### Files changed
| File | Change |
|---|---|
| `coderd/coderd.go` | Register `GET /messages` route |
| `coderd/chats.go` | Simplify `getChat`, add `getChatMessages` handler
|
| `codersdk/chats.go` | New type + method, update `GetChat` return |
| `site/src/api/api.ts` | New method, update `getChat` |
| `site/src/api/queries/chats.ts` | New query, update cache mutations |
| `site/src/pages/AgentsPage/AgentDetail.tsx` | Use separate queries |
| `site/src/pages/AgentsPage/AgentDetail/ChatContext.ts` | Update types
and cache writes |
| `site/src/pages/AgentsPage/AgentsPage.tsx` | Update websocket cache
handler |
The timeout was started before the unbounded Stepper loop, so
under CI load the deadline could expire before reaching the
operations that actually use it.
Also bumps TestMigration000387 from WaitLong to WaitSuperLong.
Fixescoder/internal#1398
## Summary
Extract a `healthyChecker()` test helper that returns an all-healthy
baseline `testChecker` in `coderd/healthcheck`. Each `TestHealthcheck`
table-driven test case now only overrides the single report field being
tested, instead of repeating all 6 healthy report structs.
- Reduces `healthcheck_test.go` from 603 to 341 lines (~260 lines, 43%
reduction)
- Test coverage unchanged at 77.2%
- All test cases and assertions preserved exactly
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- `coderd/httpapi/websocket.go`: add `net.ErrClosed` +
`websocket.CloseStatus` checks; extract `heartbeatCloseWith` with
`quartz.Clock` parameter for testability
- `coderd/httpapi/websocket_internal_test.go`: new test file
## Problem
When a chat is interrupted while tools are executing, the step content
(text, reasoning, tool calls, and partial tool results) was being lost.
Two gaps existed:
1. **During tool execution**: `executeTools` returns with error results
for interrupted tools, but the subsequent `PersistStep(ctx, ...)` fails
on the canceled context and returns `ErrInterrupted` without persisting
anything.
2. **PersistStep race**: If the context is canceled between the
post-tool interrupt check and the `PersistStep` call, the same loss
occurs.
This is inconsistent with how we handle stream interruptions (which
properly flush and persist partial content via `persistInterruptedStep`)
and how [coder/blink](https://github.com/coder/blink) handles
interruptions (always inserting the response message regardless of
execution phase).
## Fix
Two changes in `chatloop.go`:
- **Post-tool-execution interrupt check**: After `executeTools` returns,
check if the context was interrupted and route through
`persistInterruptedStep` (which uses `context.WithoutCancel` internally)
to save the accumulated content.
- **PersistStep fallback**: If `PersistStep` returns `ErrInterrupted`,
retry via `persistInterruptedStep` so partial content is not lost.
## Tests
- `TestRun_InterruptedDuringToolExecutionPersistsStep`: Verifies that
when a tool is blocked and the chat is interrupted, the step (text +
reasoning + tool call + tool error result) is persisted via the
interrupt-safe path.
- `TestRun_PersistStepInterruptedFallback`: Verifies that when
`PersistStep` itself returns `ErrInterrupted`, the step is retried via
the fallback path and content is saved.
## Problem
When a step contains both provider-executed tool calls (e.g. Anthropic
web search) and local tool calls in parallel, the next loop iteration
fails with the Anthropic API claiming the regular tool call has no
result. However, sending a new user message (which reloads messages from
the DB) works fine.
## Root cause
`toResponseMessages` was placing **all** tool results into the tool-role
message, regardless of `ProviderExecuted`. When Fantasy's Anthropic
provider later converted these messages for the API, it moved the
provider tool result from the tool message to the **end** of the
previous assistant message (`prevMsg.Content = append(...)`). This
placed `web_search_tool_result` **after** the regular `tool_use` block:
```
assistant: [server_tool_use(A), tool_use(B), web_search_tool_result(A)] ← wrong order
user: [tool_result(B)]
```
The persistence layer in `chatd.go` already handles this correctly —
provider-executed tool results stay in the assistant message, producing
the expected ordering:
```
assistant: [server_tool_use(A), web_search_tool_result(A), tool_use(B)] ← correct order
user: [tool_result(B)]
```
This is why reloading from the DB fixed it.
## Fix
In the `ContentTypeToolResult` case of `toResponseMessages`, route
provider-executed results to `assistantParts` instead of `toolParts`,
matching the persistence layer's behavior.
## Testing
Added
`TestToResponseMessages_ProviderExecutedToolResultInAssistantMessage`
which verifies that mixed provider+local tool results are split
correctly between the assistant and tool messages.
## Problem
The gitsync worker polls every 10s and refreshes up to 50 stale
`chat_diff_status` rows **sequentially**, sharing a single 10-second
context timeout. With 50 rows × 1–3 HTTP calls each, the timeout is
exhausted quickly, causing cascading `context deadline exceeded` errors.
Rows with no linked OAuth token (`ErrNoTokenAvailable`) fail fast but
recur every 120s, wasting batch capacity.
## Solution
Three targeted fixes:
### 1. Concurrent refresh processing
`Refresher.Refresh()` now launches goroutines bounded by a semaphore
(`defaultConcurrency = 10`). Provider/token resolution remains
sequential (fast DB lookups); only the HTTP calls run in parallel.
Per-group rate-limit detection uses `atomic.Pointer[RateLimitError]`
with best-effort skip of remaining rows — a rate-limit hit on one
provider doesn't stall requests to other providers.
### 2. Decoupled tick timeout
New `defaultTickTimeout = 30s`, separate from `defaultInterval = 10s`.
The `tick()` method uses `tickTimeout` for its context deadline, giving
concurrent HTTP calls enough headroom to complete without stalling the
next polling cycle.
### 3. Longer backoff for no-token errors
New `NoTokenBackoff = 10 * time.Minute` (exported). When `errors.Is(err,
ErrNoTokenAvailable)`, the worker applies a 10-minute backoff instead of
`DiffStatusTTL` (2 minutes). Retrying every 2 minutes is pointless until
the user manually links their external auth account.
## Design decisions
- Both `NewRefresher` and `NewWorker` accept variadic option functions
(`RefresherOption`, `WorkerOption`) for backward compatibility —
existing callers in `coderd/coderd.go` need no changes.
- `WithConcurrency(n)` and `WithTickTimeout(d)` are available for tests
and future tuning.
- Added `resolvedGroup` struct to cleanly separate the pre-resolution
phase from the concurrent execution phase.
## Testing
- **`TestRefresher_RateLimitSkipsRemainingInGroup`** — rewritten to be
goroutine-order-independent (verifies aggregate counts instead of
per-index results).
- **`TestRefresher_ConcurrentProcessing`** — new test using a gate
channel to prove N goroutines enter `FetchPullRequestStatus`
simultaneously.
- **`TestWorker_RefresherError_BacksOffRow`** — rewritten to use
branch-name-based failure determination instead of non-deterministic
`callCount`.
- **`TestWorker_NoTokenBackoff`** — new test verifying
`ErrNoTokenAvailable` triggers 10-minute backoff.
- All tests pass under `-race -count=3`.
## Problem
Both `start_workspace` and `create_workspace` chattool tools failed to
handle soft-deleted workspaces correctly.
Coder uses soft-delete for workspaces (`deleted = true` on the row).
Both tools called `GetWorkspaceByID`, which queries
`workspaces_expanded` with **no** `deleted = false` filter — so it
returns the workspace row even when soft-deleted. The only deletion
check was for `sql.ErrNoRows`, which never fires because the row still
exists.
### `start_workspace` behavior (before fix)
1. Loads the soft-deleted workspace successfully
2. Finds the latest build (a delete transition)
3. Falls through to attempt to **start** the deleted workspace
4. Produces a confusing downstream error
### `create_workspace` behavior (before fix)
1. `checkExistingWorkspace` loads the soft-deleted workspace
2. If a delete build is **in-progress**: waits for it, then falsely
reports `already_exists` — blocks new workspace creation
3. If the delete build **succeeded**: accidentally allows creation
(because no agents are found), but via fragile logic rather than an
explicit check
## Fix
Add `ws.Deleted` checks immediately after `GetWorkspaceByID` succeeds in
both tools:
- **`startworkspace.go`**: Returns `"workspace was deleted; use
create_workspace to make a new one"`
- **`createworkspace.go`** (`checkExistingWorkspace`): Returns `(nil,
false, nil)` to allow new workspace creation
## Tests
- `TestStartWorkspace/DeletedWorkspace` — verifies `start_workspace`
returns deleted error and never calls `StartFn`
- `TestCheckExistingWorkspace_DeletedWorkspace` — verifies
`checkExistingWorkspace` allows creation for soft-deleted workspaces
WaitBuffer is a thread-safe io.Writer that supports blocking until
accumulated output matches a substring or custom predicate. It
replaces ad-hoc safeBuffer/syncWriter types and time.Sleep-based
poll loops in tests with signal-driven waits.
- WaitFor/WaitForNth/WaitForCond for blocking on output
- Replace custom buffer types in cli/sync_test.go and
provisionersdk/agent_test.go
- Convert time.Sleep poll loops to require.Eventually/require.Never
in cli/ssh_test.go, coderd/activitybump_test.go,
coderd/workspaceagentsrpc_test.go, workspaceproxy_test.go, and
scaletest tests
Fixes Anthropic 400 error on multi-turn conversations with web search:
> web_search tool use with id srvtoolu_... was found without a
corresponding web_search_tool_result block
Provider-executed tool results (e.g. `web_search`) had a nil `Result`
field, which serialized as `"result":null`. Fantasy's
`UnmarshalToolResultOutputContent` couldn't deserialize `null` back, so
the entire assistant message became unreadable after persistence. On the
next LLM call, Anthropic rejected the conversation because
`server_tool_use` had no matching `web_search_tool_result`.
**Fix:** Bump the fantasy fork to e4bbc7bb3054 which returns `nil, nil`
for null `Result` JSON instead of erroring.
**Testing:** Added `integration_test.go` with
`TestAnthropicWebSearchRoundTrip` (requires `ANTHROPIC_API_KEY`) that:
- Sends a query triggering web search
- Verifies the persisted assistant message contains all parts the UI
needs: `tool-call(PE)`, `source`, `tool-result(PE)`, and `text`
- Sends a follow-up to confirm the round-trip works with Anthropic
## Problem
Anthropic's API returns a 400 error when `web_search` tool results are
missing:
```
web_search tool use with id srvtoolu_... was found without a corresponding web_search_tool_result block
```
**Root cause:** `persistStep` in `chatd.go` splits ALL
`ToolResultContent` blocks into separate tool-role DB rows.
Provider-executed (PE) tool results like `web_search` must stay in the
assistant message — Anthropic expects `server_tool_use` and
`web_search_tool_result` in the same turn.
The previous fix (#22976) added repair passes to drop PE results during
reconstruction, which fixed cross-step orphans but broke the normal case
(PE result correctly in the same step).
## Fix
Three changes that address the root cause:
1. **`persistStep` (chatd.go):** Check `ProviderExecuted` before
splitting `ToolResultContent` into tool rows. PE results stay in
`assistantBlocks` and are stored in the assistant content column.
2. **`ToMessageParts` (chatprompt.go):** Propagate the
`ProviderExecuted` field to `ToolResultPart` so the fantasy Anthropic
provider can identify PE results and reconstruct the
`web_search_tool_result` block.
3. **Keep existing repair passes** for backward compatibility with
legacy DB data where PE results were incorrectly persisted as separate
tool messages.
## Tests
- `TestProviderExecutedResultInAssistantContent` — PE result stored
inline in assistant content round-trips correctly with
`ProviderExecuted` preserved.
- `TestProviderExecutedResult_LegacyToolRow` — legacy PE results in
tool-role rows are still dropped correctly.
- All existing tests pass (including the 3 PE tests from #22976).
## Summary
- avoid duplicating preset headers when cachecompress serves compressed
`/bin/*` responses
- add a cachecompress regression test for preset
`X-Original-Content-Length` and `ETag` headers
- strengthen site binary tests to assert those headers stay
single-valued
## Problem
`site/bin.go` sets `X-Original-Content-Length` and `ETag` on the real
response writer before delegating.
`cachecompress` then snapshotted those headers and replayed them with
`Header().Add(...)`, which duplicated them on compressed responses.
For `coder-desktop-macos`, duplicate `X-Original-Content-Length` values
can collapse into a comma-separated string and fail `Int64` parsing,
causing the file size to show as `Unknown`.
## Testing
- `/usr/local/go/bin/go test ./coderd/cachecompress -run
'TestCompressorPresetHeaders|TestCompressorHeadings' -count=1`
- `/usr/local/go/bin/go test ./site -run TestServingBin -count=1`
- `PATH=/usr/local/go/bin:$PATH make lint/go`
## Notes
- Skipped full `make pre-commit` with explicit approval because local
environment/tooling blocked it (Node version/path interaction in
generated site targets, plus missing local tools before setup).
## Problem
The summarization prompt explicitly tells the model to **"Omit
pleasantries and next-step suggestions"** and the summary prefix frames
the compacted context as passive history: `Summary of earlier chat
context:`. After compaction mid-task, the model reads a factual recap
with no forward momentum, loses its direction, and either stops or asks
the user what to do.
## Research
I compared our compaction prompt against several other agents:
| Agent | Key Pattern |
|---|---|
| **Codex** | Prompt says *"Include what remains to be done (clear next
steps)"*. Prefix: *"Another language model started to solve this
problem..."* |
| **Mux** | Includes *"Current state of the work (what's done, what's in
progress)"* + appends the user's follow-up intent |
| **Continue** | *"Make sure it is clear what the current stream of work
was at the very end prior to compaction so that you can continue exactly
where you left off"* |
| **Copilot Chat** | Dedicated sections for *Active Work State*, *Recent
Operations*, *Pre-Summary State*, and a *Continuation Plan* with
explicit next actions |
**Every other major agent explicitly preserves forward intent and
in-progress state.** Coder was the only one telling the model to omit
next steps.
## Changes
**Summary prompt:**
- Removes `Omit next-step suggestions`
- Adds structured `Include:` list with explicit items for in-progress
work, remaining work, and the specific action being performed when
compaction fired
- Frames the operation as `context compaction` (matching Codex's
framing)
**Summary prefix:**
- Old: `Summary of earlier chat context:`
- New: `The following is a summary of the earlier conversation. The
assistant was actively working when the context was compacted. Continue
the work described below:`
The prefix is the first thing the model reads post-compaction — framing
it as an active handoff with an explicit "Continue" directive primes the
model to resume work rather than wait.
## Summary
- add chat model pricing metadata to the agents admin form and SDK
metadata
- split pricing into its own section and show default pricing as
placeholders
- apply default pricing when admins leave pricing fields blank
## Problem
1. **Personal behavior prompt not applied**: The chatd background worker
was missing `ActionReadPersonal` on `ResourceUser` in its RBAC subject.
When `resolveUserPrompt` calls `GetUserChatCustomPrompt`, the dbauthz
layer checks `ActionReadPersonal` on the user — which the chatd role
didn't have. The error was silently swallowed (returns `""`), so the
user's custom prompt was never injected into the system messages.
2. **Sequential DB calls on chat startup**: Several independent database
queries in `runChat` and `resolveChatModel` were running sequentially,
adding unnecessary latency before the LLM stream begins.
## Changes
### RBAC fix (`dbauthz.go`)
- Add `rbac.ResourceUser.Type: {policy.ActionReadPersonal}` to
`subjectChatd` site permissions
- This is the minimal permission needed — `ActionRead` on User remains
denied
### Parallelization (`chatd.go`)
Three parallelization points using `errgroup.Group`:
1. **`resolveChatModel`**: `resolveModelConfig` and
`GetEnabledChatProviders` run concurrently (both needed for
`ModelFromConfig`, which stays sequential after the wait)
2. **`runChat` startup**: `resolveChatModel` and
`GetChatMessagesForPromptByChatID` run concurrently (completely
independent)
3. **`runChat` prompt assembly**: `resolveInstructions` and
`resolveUserPrompt` run concurrently (both produce strings;
`InsertSystem` calls maintain correct order after the wait)
Same pattern applied to the `ReloadMessages` callback.
### Test (`dbauthz_test.go`)
- Add assertion in `TestAsChatd/AllowedActions` that
`ActionReadPersonal` on `ResourceUser` is permitted
## What
Adds provider-native web search tools to the chat system. Anthropic,
OpenAI, and Google all offer server-side web search — this wires them up
as opt-in per-model config options using the existing
`ChatModelProviderOptions` JSONB column (no migration).
Web search is **off by default**.
## Config
Set `web_search_enabled: true` in the model config provider options:
```json
{
"provider_options": {
"anthropic": {
"web_search_enabled": true,
"allowed_domains": ["docs.coder.com", "github.com"]
}
}
}
```
Available options per provider:
- **Anthropic**: `web_search_enabled`, `allowed_domains`,
`blocked_domains`
- **OpenAI**: `web_search_enabled`, `search_context_size`
(`low`/`medium`/`high`), `allowed_domains`
- **Google**: `web_search_enabled`
## Backend
- `codersdk/chats.go` — new fields on the per-provider option structs
- `coderd/chatd/chatd.go` — `buildProviderTools()` reads config, creates
`ProviderDefinedTool` entries (uses `anthropic.WebSearchTool()` helper
from fantasy)
- `coderd/chatd/chatloop/chatloop.go` — `ProviderTools` on `RunOptions`,
merged into `Call.Tools`. Provider-executed tool calls skip local
execution. `StreamPartTypeToolResult` with `ProviderExecuted: true` is
accumulated inline (matching fantasy's own agent.go pattern) instead of
post-stream synthesis.
- `coderd/chatd/chatprompt/` — `MarshalToolResult` carries
`ProviderMetadata` through DB persistence so multi-turn round-trips work
(Anthropic needs `encrypted_content` back)
## Frontend
- Source citations render **inline** at the tool-call position (not
bottom-of-message), using `ToolCollapsible` so they look like other tool
cards — collapsed "Searched N results" with globe icon, expand to see
source pills
- Provider-executed tool calls/results are hidden from the normal tool
card UI
- Tool-role messages with only provider-executed results return `null`
(no empty bubble)
- Both persisted (messageParsing.ts) and streaming (streamState.ts)
paths group consecutive `source` parts into a single `{ type: "sources"
}` render block
## Fantasy changes
The fantasy fork (`kylecarbs/fantasy` branch `cj/go1.25`) has the
Anthropic tool code merged in, but will hopefully go upstream from:
https://github.com/charmbracelet/fantasy/pull/163
## Summary
Scale-tested the `chatd` package with mock-based benchmarks to identify
performance bottlenecks. This PR fixes 6 of the 8 identified issues,
ranked by severity.
## Changes
### 1. Parallel tool execution (HIGH) — `chatloop.go`
`executeTools` ran tool calls sequentially. Now dispatches all calls
concurrently via goroutines with `sync.WaitGroup`. Results are
pre-allocated by index (no mutex needed). `onResult` callbacks fire as
each tool completes.
### 2. Pubsub-backed subagent await (HIGH) — `subagent.go`
`awaitSubagentCompletion` polled the DB every 200ms. Now subscribes to
the child chat's `ChatStreamNotifyChannel` via pubsub for near-instant
notifications. Fallback poll reduced to 5s. Falls back to 200ms only
when `pubsub == nil` (single-instance / in-memory).
### 3. Per-chat stream locking (MEDIUM) — `chatd.go`
Replaced single global `streamMu` + `map[uuid.UUID]*chatStreamState`
with `sync.Map` where each `chatStreamState` has its own `sync.Mutex`.
Zero cross-chat contention.
### 4. Batch chat acquisition (MEDIUM) — `chatd.go`
`processOnce` acquired 1 chat per tick. Now loops up to
`maxChatsPerAcquire = 10` per tick, avoiding idle time when many chats
are pending.
### 5. Reduced heartbeat frequency (LOW-MEDIUM) — `chatd.go`
`chatHeartbeatInterval` changed from 30s to 60s. Safe given the 5-minute
`DefaultInFlightChatStaleAfter`.
### 6. O(depth) descendant check (LOW) — `subagent.go`
Replaced top-down BFS (`O(total_descendants)` queries) with bottom-up
parent-chain walk (`O(depth)` queries). Includes cycle protection.
## Not addressed (intentionally)
- Message serialization overhead
- Buffer eviction (`buffer[1:]` pattern)
## Problem
Two separate code paths refreshed chat diff statuses:
1. **HTTP handler** (`refreshChatDiffStatus`): resolved
provider/token/status inline, ran under the user's context. Worked fine
because the user owns their external auth links.
2. **Background worker** (`Refresher.Refresh`): ran under `AsChatd`
context, which lacked `ActionReadPersonal` on `ResourceUser`.
`GetExternalAuthLink` failed silently (`if err != nil { continue }`),
returning `ErrNoTokenAvailable` every time. Chat diff statuses got
`git_branch`/`git_remote_origin` from `MarkStale` but `refreshed_at`,
`url`, `pull_request_state` stayed nil.
Having two paths also meant bug fixes had to be applied twice.
## Fix
- **`Worker.RefreshChat`**: New method for synchronous, on-demand
refresh of a single chat. Uses the same `Refresher.Refresh` pipeline as
the background `tick()`. Called by the HTTP handler for instant
response.
- **`resolveChatGitAccessToken`**: Uses
`dbauthz.AsSystemRestricted(ctx)` specifically for `GetExternalAuthLink`
and `RefreshToken` calls. This is scoped to just those DB operations
rather than broadening the chatd RBAC role.
- **Removed**: `refreshChatDiffStatus`, `shouldRefreshChatDiffStatus`,
`resolveChatDiffStatusWithOptions` (all replaced by the single
`RefreshChat` path).
## Tests
Added 4 tests for `Worker.RefreshChat`:
- `TestRefreshChat_Success`: full refresh + upsert + publish
- `TestRefreshChat_NoPR`: no PR exists yet, nil result
- `TestRefreshChat_RefreshError`: provider resolution fails
- `TestRefreshChat_UpsertError`: refresh succeeds but DB write fails
## Why tests didn't catch the original bug
- Worker tests used mock stores (no dbauthz) and fake token resolvers
(hardcoded lambdas)
- No integration test exercised `AsChatd` -> `resolveChatGitAccessToken`
-> `GetExternalAuthLink` through dbauthz
## Problem
`insertAgentApp` mutated its input by writing to `app.Healthcheck` when
it was nil (line 3525):
```go
if app.Healthcheck == nil {
app.Healthcheck = &sdkproto.Healthcheck{} // mutation!
}
```
The Devcontainers subtests share the same `tt.resource` pointer across
two parallel goroutines (`WithProtoIDs` and `WithoutProtoIDs`), causing
a data race on the `Healthcheck` field (and its sub-fields `Url`,
`Interval`, `Threshold`).
## Fix
Replace the in-place mutation with a local variable:
```go
healthcheck := app.GetHealthcheck()
if healthcheck == nil {
healthcheck = &sdkproto.Healthcheck{}
}
```
This avoids writing back to the shared proto message. All downstream
reads now use the local `healthcheck` variable.
Adds `pull_request_title` and `pull_request_draft` to the chat diff
status pipeline (DB → provider → SDK → frontend). The GitHub provider
now fetches the PR title alongside existing status fields.
The agents sidebar now displays PR-state-aware icons for chats that have
a linked pull request (when the chat is in waiting/completed state):
- **Open PR**: `GitPullRequestArrow` (green)
- **Draft PR**: `GitPullRequestDraft` (gray)
- **Merged PR**: `GitMerge` (purple)
- **Closed PR**: `GitPullRequestClosed` (red)
Running/pending/paused/error chats keep their existing activity icons
(spinner, pause, error triangle).
### Changes
**Database migration** (`000432`): Adds `pull_request_title TEXT` and
`pull_request_draft BOOLEAN` columns to `chat_diff_statuses`.
**Backend pipeline**:
- `gitprovider.PRStatus` gains a `Title` field
- GitHub provider decodes the `title` from the API response
- `gitsync` and `coderd/chats.go` pass title + draft through to the DB
upsert
- `codersdk.ChatDiffStatus` exposes both new fields in the API response
**Frontend** (`AgentsSidebar.tsx`): New `getPRIconConfig()` function
resolves the appropriate Lucide git icon based on `pull_request_state`
and `pull_request_draft`. Only applies when the chat is in a terminal
state (waiting/completed).
**Real-time sync**: No changes needed — the existing
`diff_status_change` pubsub event already propagates the full
`ChatDiffStatus` including the new fields.
Replace the standalone `?archived=` query parameter on the chats listing
endpoint with a `?q=` search parameter, consistent with how workspaces,
tasks, templates, and other list endpoints work.
The `q` parameter uses the standard `key:value` search syntax parsed by
the `searchquery` package. Currently supports:
- `archived:true/false` (default: `false`, hides archived chats)
When `q` is empty or omits the archived filter, archived chats are
excluded by default. This is a behavioral change — the previous API
returned all chats (including archived) when no filter was specified.
### Changes
**Backend:**
- Add `searchquery.Chats()` parser following the same pattern as
`Tasks()`, `Workspaces()`, etc.
- Update `listChats` handler to read `q` instead of `archived`
- Update `codersdk.ListChatsOptions` to use `Q string` instead of
`Archived *bool`
**Frontend:**
- Update `getChats` API method to accept `q` parameter
- Update `infiniteChats` query to pass `q` instead of `archived`
**Tests:**
- Add `TestSearchChats` unit tests for the parser
- Update existing archive/unarchive integration tests to use `Q:
"archived:true"` syntax
Adds a `created_by` column (nullable UUID) to the `chat_messages` table
to track which user created each message. Only user-sent messages
populate this field; assistant, tool, system, and summary messages leave
it null.
The column is threaded through the full stack: SQL migration, query
updates, generated Go/TypeScript types, db2sdk conversion, chatd
(including subagent paths), and API handlers. All API handlers that
insert user messages now pass the authenticated user's ID as
`created_by`.
No foreign key constraint was added, matching the existing pattern used
by `chat_model_configs.created_by`.
Removes the backend and frontend logic that extracted compact titles
from reasoning/thinking blocks. The `Title` field on `ChatMessagePart`
remains for other part types (e.g. source), but reasoning blocks no
longer have titles derived from first-line markdown bold text or
provider metadata summaries.
**Backend:**
- Remove `ReasoningTitleFromFirstLine`, `reasoningTitleFromContent`,
`reasoningSummaryTitle`, `compactReasoningSummaryTitle`, and
`reasoningSummaryHeadline` from chatprompt
- Simplify `marshalContentBlock` to plain `json.Marshal` (no title
injection)
- Remove title tracking maps and `setReasoningTitleFromText` from
chatloop stream processing
- Remove `reasoningStoredTitle` from db2sdk
- Remove related tests from db2sdk_test
**Frontend:**
- Remove `mergeThinkingTitles` from blockUtils
- Simplify `appendTextBlock` to always merge consecutive thinking blocks
- Remove `applyStreamThinkingTitle` from streamState
- Simplify reasoning/thinking stream handler to ignore title-only parts
- Update tests accordingly
Net: **-487 lines / +42 lines**
A cursory glance at Grafana for error-level logs showed that the
following log line was appearing regularly:
```
2026-03-11 05:17:59.169 [erro] coderd: failed to heartbeat ping trace=xxx span=xxx request_id=xxx ...
error= failed to ping:
github.com/coder/coder/v2/coderd/httpapi.pingWithTimeout
/home/runner/work/coder/coder/coderd/httpapi/websocket.go:46
- failed to ping: failed to wait for pong: context canceled
```
This seems to be an "expected" error when the parent context is canceled
so doesn't make sense to log at level ERROR.
NOTE: I also saw this a bit and wonder if it also deserves similar
treatment:
```
2026-03-11 05:10:53.229 [erro] coderd.inbox_notifications_watcher: failed to heartbeat ping trace=xxx span=xxx request_id=xxx ...
error= failed to ping:
github.com/coder/coder/v2/coderd/httpapi.pingWithTimeout
/home/runner/work/coder/coder/coderd/httpapi/websocket.go:46
- failed to ping: failed to write control frame opPing: use of closed network connection
```
- Adds `_API_BASE_URL` to `CODER_EXTERNAL_AUTH_CONFIG_`
- Extracts and refactors existing GitHub PR sync logic to new packages
`coderd/gitsync` and `coderd/externalauth/gitprovider`
- Associated wiring and tests
Created using Opus 4.6
## Problem
When multiple concurrent callers (e.g., parallel workspace builds) read
the same single-use OAuth2 refresh token from the database and race to
exchange it with the provider, the first caller succeeds but subsequent
callers get `bad_refresh_token`. The losing caller then **clears the
valid new token** from the database, permanently breaking the auth link
until the user manually re-authenticates.
This is reliably reproducible when launching multiple workspaces
simultaneously with GitHub App external auth and user-to-server token
expiration enabled.
## Solution
Two layers of protection:
### 1. Singleflight deduplication (`Config.RefreshToken` +
`ObtainOIDCAccessToken`)
Concurrent callers for the same user/provider share a single refresh
call via `golang.org/x/sync/singleflight`, keyed by `userID`. The
singleflight callback re-reads the link from the database to pick up any
token already refreshed by a prior in-flight call, avoiding redundant
IDP round-trips entirely.
### 2. Optimistic locking on `UpdateExternalAuthLinkRefreshToken`
The SQL `WHERE` clause now includes `AND oauth_refresh_token =
@old_oauth_refresh_token`, so if two replicas (HA) race past
singleflight, the loser's destructive UPDATE is a harmless no-op rather
than overwriting the winner's valid token.
## Changes
| File | Change |
|------|--------|
| `coderd/externalauth/externalauth.go` | Added `singleflight.Group` to
`Config`; split `RefreshToken` into public wrapper +
`refreshTokenInner`; pass `OldOauthRefreshToken` to DB update |
| `coderd/provisionerdserver/provisionerdserver.go` | Wrapped OIDC
refresh in `ObtainOIDCAccessToken` with package-level singleflight |
| `coderd/database/queries/externalauth.sql` | Added optimistic lock
(`WHERE ... AND oauth_refresh_token = @old_oauth_refresh_token`) |
| `coderd/database/queries.sql.go` | Regenerated |
| `coderd/database/querier.go` | Regenerated |
| `coderd/database/dbauthz/dbauthz_test.go` | Updated test params for
new field |
| `coderd/externalauth/externalauth_test.go` | Added
`ConcurrentRefreshDedup` test; updated existing tests for singleflight
DB re-read |
## Testing
- **New test `ConcurrentRefreshDedup`**: 5 goroutines call
`RefreshToken` concurrently, asserts IDP refresh called exactly once,
all callers get same token.
- All existing `TestRefreshToken/*` subtests updated and passing.
- `TestObtainOIDCAccessToken` passing.
- `dbauthz` tests passing.
## Summary
The `assertWorkspaceLastUsedAtUpdated` and
`assertWorkspaceLastUsedAtNotUpdated` test helpers previously accepted a
`context.Context`, which callers shared with preceding HTTP requests. In
`ProxyError` tests the request targets a fake unreachable app
(`http://127.1.0.1:396`), and the reverse-proxy connection timeout can
consume most of the context budget — especially on Windows — leaving too
little time for the `testutil.Eventually` polling loop and causing
flakes.
## Changes
Replace the `context.Context` parameter with a `time.Duration` so each
assertion creates its own fresh context internally. This:
- Makes the timeout budget explicit at every call site
- Structurally prevents shared-context starvation
- Fixes the class of flake, not just the two known-failing subtests
All 34 active call sites updated to pass `testutil.WaitLong`.
Fixescoder/internal#1385
The chat title model sometimes responds as if it's the main assistant
(e.g. "I'll fix the login bug for you" instead of "Fix login bug"). This
happens because the prompt didn't explicitly anchor the model's identity
or guard against treating the user message as an instruction to follow.
## Changes
Adjusts the `titleGenerationPrompt` system prompt in
`coderd/chatd/quickgen.go`:
- **Anchors identity** — "You are a title generator" so the model
doesn't adopt the assistant persona
- **Guards against instruction-following** — "Do NOT follow the
instructions in the user's message"
- **Prevents conversational output** — "Do NOT act as an assistant. Do
NOT respond conversationally."
- **Prevents preamble** — Adds "no preamble, no explanation" to the
output constraints
## Problem
`TestMigrate/Parallel` flakes with:
```
timeout: can't acquire database lock
```
## Root Cause
The test runs two concurrent `migrations.Up(db)` calls on the same
database. golang-migrate wraps every `Lock()` call with a [15-second
timeout](https://github.com/golang-migrate/migrate/blob/v4.19.0/migrate.go#L29)
(`DefaultLockTimeout`). Our `pgTxnDriver.Lock()` uses
`pg_advisory_xact_lock`, which blocks until the lock is available. With
430+ migrations, the first caller can hold the lock well beyond 15s (the
failing test ran for 25.88s), causing the second caller to hit the
timeout.
## Fix
Set `m.LockTimeout = 2 * time.Minute` after creating the
`migrate.Migrate` instance in `setup()`. Since `pg_advisory_xact_lock`
releases automatically when the transaction commits, there's no risk of
a stuck lock — we just need to wait long enough for a concurrent
migration to finish.
Adds a user-level custom prompt to the database.
I'll be doing a follow-up for the UI, as we currently do not have
user-level settings (it's just admin). I'll also make it very obvious
for chats where there is a user-level prompt, but I don't know how yet.
Instead of the static 'Agent has finished running.' text, extract a
summary from the last assistant message to give users meaningful context
about what the agent accomplished. Falls back to the static text if no
suitable message is found.
Co-authored-by: Kyle Carberry <kyle@carberry.com>
The chat API is experimental (behind `ExperimentAgents`) and not ready
for public documentation yet. This removes swagger annotations from the
chat handlers so they no longer appear in the generated API reference at
https://coder.com/docs/reference/api/chats.
## Changes
- Remove `@swagger` annotations from 5 chat handlers in
`coderd/chats.go`
- Regenerate `coderd/apidoc/swagger.json` and `docs.go`
- Delete `docs/reference/api/chats.md`
- Remove Chats entry from `docs/manifest.json`
Fixes https://github.com/coder/internal/issues/1371
## Root causes
Two independent races cause this test to flake at ~2–3/1000:
### 1. Title-generation requests racing with the streaming request
counter
`maybeGenerateChatTitle` fires in a `context.WithoutCancel` goroutine
(line 2130) and makes a **non-streaming** request to the mock OpenAI
handler. The test handler was not filtering by request type, so these
title requests incremented the `requestCount` atomic — throwing off the
coordination logic that uses `requestCount == 1` to identify the first
streaming request and hold it open until shutdown.
**Fix:** Guard the test handler to return a canned response for
non-streaming requests before touching `requestCount`.
### 2. Phantom acquire: `AcquireChat` commits in Postgres but Go sees
`context.Canceled`
During `Close()`, the main loop's `select` can randomly pick
`acquireTicker.C` over `ctx.Done()` (Go spec: when multiple cases are
ready, one is chosen uniformly at random). This calls `processOnce(ctx)`
with an already-canceled context.
In the pq driver, `QueryContext` does **not** check `ctx.Err()` up
front. Instead it calls `watchCancel(ctx)` which spawns a goroutine
monitoring `ctx.Done()`, then sends the query on the existing
connection. When `ctx` is already canceled, a race ensues:
- **pq's watchCancel goroutine** immediately sees `<-done`, opens a
*new* TCP connection to Postgres, and sends a cancel request.
- **The query** is sent concurrently on the existing connection.
Because the `AcquireChat` UPDATE is fast (sub-millisecond, single row
with `SKIP LOCKED`), it often commits before the cancel arrives via the
second connection. Meanwhile in `database/sql`, `initContextClose`
spawns an `awaitDone` goroutine that fires immediately (context is
already canceled), stores `contextDone`, and calls `rs.close(ctx.Err())`
— which races with `Row.Scan` → `rows.Next()`. If `awaitDone` wins,
`Next()` sees `contextDone` is set and returns false, causing Scan to
return `context.Canceled` (or `ErrNoRows`).
**Result:** Postgres committed the UPDATE (chat is now `running` with
serverA's worker ID), but Go sees an error and never spawns a goroutine
to process it. The chat is stuck as `running` with no worker.
If the previous `processChat` cleanup already set the chat back to
`pending`, this phantom acquire flips it back to `running` — which is
exactly what the debug logs showed: after `Close()` returns, the DB
shows `status=running` with serverA's worker ID.
**Fix:** Three guards in `processOnce`:
1. Early `ctx.Err()` check — catches the common case where `select`
picked the ticker after cancellation.
2. `context.WithoutCancel(ctx)` for `AcquireChat` — prevents the pq
`watchCancel` race entirely, ensuring the driver sees the query
result if Postgres executed it.
3. Post-acquire `ctx.Err()` check — if the context was canceled while
`AcquireChat` ran (or between the early check and the call),
immediately release the chat back to `pending`.
## Verification
Passes 2000/2000 iterations (previously flaked at ~2–3/1000):
```
go test -run "TestCloseDuringShutdownContextCanceledShouldRetryOnNewReplica" \
-count=2000 -timeout 1800s -failfast ./coderd/chatd/
```
Mathias Fredriksson