mirror of
https://github.com/coder/coder.git
synced 2026-09-24 15:04:27 +08:00
be5e080de697bf7555ec0deb90db168d15d9683c
100
Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
 Kyle Carberry
|
13241a58ba |
fix(coderd/x/chatd/mcpclient): use dedicated HTTP transport per MCP connection (#23494)
## Problem `TestConnectAll_MultipleServers` flakes with: ``` net/http: HTTP/1.x transport connection broken: http: CloseIdleConnections called ``` Each MCP client connection implicitly uses `http.DefaultTransport`. When `httptest.Server.Close()` runs during parallel test cleanup, it calls `CloseIdleConnections` on `http.DefaultTransport`, breaking in-flight connections from other goroutines or parallel tests sharing that transport. ## Fix Clone the default transport for each MCP connection via `http.DefaultTransport.(*http.Transport).Clone()`, passed through `WithHTTPBasicClient` (StreamableHTTP) and `WithHTTPClient` (SSE). This scopes idle connection cleanup to a single MCP server so it cannot disrupt unrelated connections. Fixes coder/internal#1420 |
||
|
Kyle Carberry
|
631e4449bb |
fix: use actual config ID in MCP OAuth2 redirect URI during auto-discovery (#23491)
## Problem
During OAuth2 auto-discovery for MCP servers, the callback URL
registered with the remote authorization server via Dynamic Client
Registration (RFC 7591) contained the literal string `{id}` instead of
the actual config UUID:
```
https://coder.example.com/api/experimental/mcp/servers/{id}/oauth2/callback
```
This happened because the discovery and registration occurred **before**
the database insert that generates the ID. When the user later initiated
the OAuth2 connect flow, the redirect URL used the real UUID, causing
the authorization server to reject it with:
> The provided redirect URIs are not approved for use by this
authorization server
## Fix
Restructure the auto-discovery flow in `createMCPServerConfig` to:
1. **Insert** the MCP server config first (with empty OAuth2 fields) to
get the database-generated UUID
2. **Build** the callback URL with the actual UUID
3. **Perform** OAuth2 discovery and dynamic client registration with the
correct URL
4. **Update** the record with the discovered OAuth2 credentials
5. **Clean up** the record if discovery fails
## Testing
Added regression test
`TestMCPServerConfigsOAuth2AutoDiscovery/RedirectURIContainsRealConfigID`
that:
- Stands up mock auth + MCP servers
- Captures the `redirect_uris` sent during dynamic client registration
- Asserts the URI contains the real config UUID, not `{id}`
- Verifies the full callback path structure
All existing MCP server config tests continue to pass.
|
||
|
Kyle Carberry
|
f62f2ffe6a |
feat(site): add MCP server picker to agent chat UI (#23470)
## Summary
Adds a user-facing MCP server configuration panel to the chat input
toolbar. Users can toggle which MCP servers provide tools for their chat
sessions, and authenticate with OAuth2 servers via popup windows.
## Changes
### New Components
- **`MCPServerPicker`** (`MCPServerPicker.tsx`): Popover-based picker
that appears in the chat input toolbar next to the model selector. Shows
all enabled MCP servers with toggles.
- **`MCPServerPicker.stories.tsx`**: 13 Storybook stories covering all
states.
### Availability Policies
Respects the admin-configured availability for each server:
- **`force_on`**: Always active, toggle disabled, lock icon shown. User
cannot disable.
- **`default_on`**: Pre-selected by default, user can opt out via
toggle.
- **`default_off`**: Not selected by default, user must opt in via
toggle.
### OAuth2 Authentication
For servers with `auth_type: "oauth2"`:
- Shows auth status (connected/not connected)
- "Connect to authenticate" link opens a popup window to
`/api/experimental/mcp/servers/{id}/oauth2/connect`
- Listens for `postMessage` with `{type: "mcp-oauth2-complete"}` from
the callback page
- Same UX pattern as external auth on the Create Workspace screen
### Integration Points
- `AgentChatInput`: MCP picker appears in the toolbar after the model
selector
- `AgentDetail`: Manages MCP selection state, initializes from
`chat.mcp_server_ids` or defaults
- `AgentDetailView` / `AgentDetailContent`: Props plumbed through to
input
- `AgentCreatePage` / `AgentCreateForm`: MCP selection for new chats
- `mcp_server_ids` now sent with `CreateChatMessageRequest` and
`CreateChatRequest`
### Helper
- `getDefaultMCPSelection()`: Computes default selection from
availability policies (`force_on` + `default_on`)
## Storybook Stories
| Story | Description |
|-------|-------------|
| NoServers | No servers - picker hidden |
| AllDisabled | All disabled servers - picker hidden |
| SingleForceOn | Force-on server with locked toggle |
| SingleDefaultOnNoAuth | Default-on with no auth required |
| SingleDefaultOff | Optional server not selected |
| OAuthNeedsAuth | OAuth2 server needing authentication |
| OAuthConnected | OAuth2 server already connected |
| MixedServers | Multiple servers with mixed availability/auth |
| AllConnected | All OAuth2 servers authenticated |
| Disabled | Picker in disabled state |
| WithDisabledServer | Disabled servers filtered out |
| AllOptedOut | All toggled off except force_on |
| OptionalOAuthNeedsAuth | Optional OAuth2 needing auth |
|
||
|
Kyle Carberry
|
acbfb90c30 |
feat: auto-discover OAuth2 config for MCP servers via RFC 7591 DCR (#23406)
## Problem When adding an external MCP server with `auth_type=oauth2`, admins currently must manually provide: - `oauth2_client_id` - `oauth2_client_secret` - `oauth2_auth_url` - `oauth2_token_url` This requires the admin to manually register an OAuth2 client with the external MCP server's authorization server first — a friction-heavy process that contradicts the MCP spec's vision of plug-and-play discovery. ## Solution When an admin creates an MCP server config with `auth_type=oauth2` and omits the OAuth2 fields, Coder now automatically discovers and registers credentials following the MCP authorization spec: 1. **Protected Resource Metadata (RFC 9728)** — Fetches `/.well-known/oauth-protected-resource` from the MCP server to discover its authorization server. Falls back to probing the server URL for a `WWW-Authenticate` header with a `resource_metadata` parameter. 2. **Authorization Server Metadata (RFC 8414)** — Fetches `/.well-known/oauth-authorization-server` from the discovered auth server to find all endpoints. 3. **Dynamic Client Registration (RFC 7591)** — Registers Coder as an OAuth2 client at the auth server's registration endpoint, obtaining a `client_id` and `client_secret` automatically. The discovered/generated credentials are stored in the MCP server config, and the existing per-user OAuth2 connect flow works unchanged. ### Backward compatibility - **Manual config still works**: If all three fields (`oauth2_client_id`, `oauth2_auth_url`, `oauth2_token_url`) are provided, the existing behavior is unchanged. - **Partial config is rejected**: Providing some but not all fields returns a clear error explaining the two options. - **Discovery failure is clear**: If auto-discovery fails, the error message explains what went wrong and suggests manual configuration. ## Changes - **New package `coderd/mcpauth`** — Self-contained discovery and DCR logic with no `codersdk` dependency - **Modified `coderd/mcp.go`** — `createMCPServerConfig` handler now attempts auto-discovery when OAuth2 fields are omitted - **Tests** — Unit tests for discovery (happy path, WWW-Authenticate fallback, no registration endpoint, registration failure) and `parseResourceMetadataParam` helper |
||
|
Kyle Carberry
|
e388a88592 |
feat(coderd/chatd): connect to external MCP servers for chat tool invocation (#23333)
## Summary
Adds a new `coderd/chatd/mcpclient` package that connects to
admin-configured MCP servers and wraps their tools as
`fantasy.AgentTool` values that the chat loop can invoke.
## What changed
### New: `coderd/chatd/mcpclient/mcpclient.go`
The core package with a single entry point:
```go
func ConnectAll(
ctx context.Context,
logger slog.Logger,
configs []database.MCPServerConfig,
tokens []database.MCPServerUserToken,
) (tools []fantasy.AgentTool, cleanup func(), err error)
```
This:
1. Connects to each enabled MCP server using `mark3labs/mcp-go`
(streamable HTTP or SSE transport)
2. Discovers tools via the MCP `tools/list` method
3. Wraps each tool as a `fantasy.AgentTool` with namespaced name
(`serverslug__toolname`)
4. Applies tool allow/deny list filtering from the server config
5. Handles auth: OAuth2 bearer tokens, API keys, and custom headers
6. Skips broken servers with a warning (10s connect timeout per server)
7. Returns a cleanup function to close all MCP connections
### Modified: `coderd/chatd/chatd.go`
In `runChat()`, after loading the model/messages but before assembling
the tool list:
- Reads `chat.MCPServerIDs` from the chat record
- Loads the MCP server configs from the database
- Resolves the user's auth tokens
- Calls `mcpclient.ConnectAll()` to connect and discover tools
- Appends the MCP tools to the chat's tool set
- Defers cleanup to close connections when the chat turn ends
The chat loop (`chatloop.Run`) already handles tools generically —
MCP-backed tools are invoked identically to built-in workspace tools. No
changes needed in `chatloop/`.
### New: `coderd/chatd/mcpclient/mcpclient_test.go`
10 tests covering:
- Tool discovery and namespacing
- Tool call forwarding and result conversion
- Allow/deny list filtering
- Connection failure handling (graceful skip)
- Multi-server support with correct prefixes
- OAuth2 auth header injection
- Disabled server skipping
- Invalid input handling
- Tool info parameter propagation
## Design decisions
- **Tool namespacing**: `slug__toolname` with double underscore
separator. Avoids collisions with tools containing single underscores.
Stripped when forwarding to `tools/call`.
- **Connection lifecycle**: Fresh connections per chat turn, closed via
`defer`. Matches the `turnWorkspaceContext` pattern.
- **Failure isolation**: Each server connects independently. A broken
server doesn't fail the chat — its tools are simply unavailable.
- **No chatloop changes**: The existing `[]fantasy.AgentTool` interface
is already fully generic.
## What's NOT in this PR (follow-ups)
- Frontend MCP server picker UI (selecting servers for a chat)
- System prompt additions describing available MCP tools
- Token refresh on expiry mid-chat
- The deprecated `aibridged` MCP proxy cleanup
|
||
|
Kyle Carberry
|
4da273ba3c | feat(site): add usage indicator to agents sidebar (#23307) | ||
|
Kyle Carberry
|
7db77bbefa |
feat(site): add MCP server admin UI (#23301)
This adds the UI but does not add it to the Settings sidebar. Until it's actually functional and usable (which will come in future PRs) it will remain hidden. Next step is wiring this up to chats and actually testing the full flow end-to-end, but we aren't there yet. |
||
|
Kyle Carberry
|
742694eb20 |
fix: filter empty text/reasoning parts before sending to LLM (#23284)
## Problem Anthropic rejects requests containing empty text content blocks with: ``` messages: text content blocks must be non-empty ``` Empty text parts (`""` or whitespace-only like `" "`) get persisted in the database when a stream sends `TextStart`/`TextEnd` with no `TextDelta` in between. On the next turn, these parts are loaded from the DB and sent to Anthropic, which rejects them. ## Fix Filter empty/whitespace-only text and reasoning parts at the two LLM dispatch boundaries, without modifying persistence (the raw record is preserved): - **`partsToMessageParts()`** in `chatprompt.go` — filters when converting persisted DB messages to fantasy message parts for LLM calls. This is the last gateway before the Anthropic provider creates `TextBlockParam` objects. - **`toResponseMessages()`** in `chatloop.go` — filters when building in-flight conversation messages between steps within a single turn. Note: `flushActiveState()` (the interruption path) already had this guard — the normal `TextEnd` streaming path did not, but since we're not changing persistence, the fix is applied at the dispatch layer. |
||
|
Kyle Carberry
|
86cb313765 |
fix: update fantasy to fix OpenAI reasoning replay with Store enabled (#23297)
## Problem When `Store: true` is set for OpenAI Responses API calls (the new default), multi-turn conversations with reasoning models fail on the second message: ``` stream response: bad request: Item 'rs_xxx' of type 'reasoning' was provided without its required following item. ``` The fantasy library was reconstructing full `OfReasoning` input items (with encrypted content and summary) when replaying assistant messages. The API cannot pair these reconstructed reasoning items with the output items that originally followed them because the output items are sent as plain `OfMessage` without server-side IDs. ## Fix Updates the fantasy dependency (`kylecarbs/fantasy@cj/go1.25`) to skip reasoning parts during conversation replay in `toResponsesPrompt`. With `Store` enabled, the API already has the reasoning persisted server-side — it doesn't need to be replayed in the input. Fantasy PR: https://github.com/charmbracelet/fantasy/pull/181 ## Testing Adds `TestOpenAIReasoningRoundTrip` integration test that: 1. Sends a query to `o4-mini` (reasoning model with `Store: true`) 2. Verifies reasoning content is persisted 3. Sends a follow-up message — this was the failing step 4. Verifies the follow-up completes successfully Requires `OPENAI_API_KEY` env var to run. |
||
|
Kyle Carberry
|
d8ff67fb68 |
feat: add MCP server configuration backend for chats (#23227)
## Summary
Adds the database schema, API endpoints, SDK types, and encryption
wrappers for admin-managed MCP (Model Context Protocol) server
configurations that chatd can consume. This is the backend foundation
for allowing external MCP tools (Sentry, Linear, GitHub, etc.) to be
used during AI chat sessions.
## Database
Two new tables:
- **`mcp_server_configs`**: Admin-managed server definitions with URL,
transport (Streamable HTTP / SSE), auth config (none / OAuth2 / API key
/ custom headers), tool allow/deny lists, and an availability policy
(`force_on` / `default_on` / `default_off`). Includes CHECK constraints
on transport, auth_type, and availability values.
- **`mcp_server_user_tokens`**: Per-user OAuth2 tokens for servers
requiring individual authentication. Cascades on user/config deletion.
New column on `chats` table:
- **`mcp_server_ids UUID[]`**: Per-chat MCP server selection, following
the same pattern as `model_config_id` — passed at chat creation,
changeable per-message with nil-means-no-change semantics.
## API Endpoints
All routes are under `/api/experimental/mcp/servers/` and gated behind
the `agents` experiment.
**Admin endpoints** (`ResourceDeploymentConfig` auth):
- `POST /` — Create MCP server config
- `PATCH /{id}` — Update MCP server config (full-replace)
- `DELETE /{id}` — Delete MCP server config
**Authenticated endpoints** (all users, enabled servers only for
non-admins):
- `GET /` — List configs (admins see all, members see enabled-only with
admin fields redacted)
- `GET /{id}` — Get config by ID (with `auth_connected` populated
per-user)
**OAuth2 per-user auth flow:**
- `GET /{id}/oauth2/connect` — Initiate OAuth2 flow (state cookie CSRF
protection)
- `GET /{id}/oauth2/callback` — Handle OAuth2 callback, store tokens
- `DELETE /{id}/oauth2/disconnect` — Remove stored OAuth2 tokens
## Security
- **Secrets never returned**: `OAuth2ClientSecret`, `APIKeyValue`, and
`CustomHeaders` are never in API responses — only boolean indicators
(`has_oauth2_secret`, `has_api_key`, `has_custom_headers`).
- **Field redaction for non-admins**: `convertMCPServerConfigRedacted`
strips `OAuth2ClientID`, auth URLs, scopes, and `APIKeyHeader` from
non-admin responses.
- **dbcrypt encryption at rest**: All 5 secret fields use `dbcrypt_keys`
encryption with full encrypt-on-write / decrypt-on-read wrappers (11
dbcrypt method overrides + 2 helpers), following the same pattern as
`chat_providers.api_key`.
- **OAuth2 CSRF protection**: State parameter stored in `HttpOnly`
cookie with `HTTPCookies.Apply()` for correct `Secure`/`SameSite` behind
TLS-terminating proxies.
- **dbauthz authorization**: All 18 querier methods have authorization
wrappers. Read operations use `ActionRead`, write operations use
`ActionUpdate` on `ResourceDeploymentConfig`.
## Governance Model
| Control | Implementation |
|---------|---------------|
| **Global kill switch** | `enabled` defaults to `false` |
| **Availability policy** | `force_on` (always injected), `default_on`
(pre-selected), `default_off` (opt-in) |
| **Per-chat selection** | `mcp_server_ids` on `CreateChatRequest` /
`CreateChatMessageRequest` |
| **Auth gate** | OAuth2 servers require per-user auth before tools are
injected |
| **Tool-level allow/deny** | Arrays on `mcp_server_configs` for
granular tool filtering |
| **Secrets encrypted at rest** | Uses `dbcrypt_keys` (same pattern as
`chat_providers.api_key`) |
## Tests
8 test functions covering:
- Full CRUD lifecycle (create, list, update, delete)
- Non-admin visibility filtering (enabled-only, field redaction)
- `auth_connected` population for OAuth2 vs non-OAuth2 servers
- Availability policy validation (valid values + invalid rejection)
- Unique slug enforcement (409 Conflict)
- OAuth2 disconnect idempotency
- Chat creation with `mcp_server_ids` persistence
## Known Limitations (Deferred)
These are documented and intentional for an experimental feature:
- **Audit logging** not yet wired — will add when feature stabilizes
- **Cross-field validation** (e.g., OAuth2 fields required when
`auth_type=oauth2`) — admin-only endpoint, will add when stabilizing
- **`force_on` auto-injection** — query exists but not yet wired into
chatd tool injection (follow-up)
- **Additional test coverage** — 403 auth tests, GET-by-ID tests,
callback CSRF tests planned for follow-up
## What's NOT in this PR
- Frontend UI (admin panel + chat picker)
- Actual MCP client connections (`chatd/chatmcp/` manager)
- Tool injection into `chatloop/`
|
||
|
Kyle Carberry
|
fdc2366227 |
chore: update fantasy dep to rebased cj/go1.25 branch (#23242)
Updates the `charm.land/fantasy` replace to the rebased `cj/go1.25` branch on `kylecarbs/fantasy`, which now includes: - **chore: downgrade to Go 1.25** - **feat: anthropic computer use** - **chore: use kylecarbs/openai-go fork for coder/coder compat** Switches the `openai-go/v3` replace from `SasSwart/openai-go` → `kylecarbs/openai-go`, which is the same SasSwart perf fork plus a fix for `WithJSONSet` being clobbered by deferred body serialization. Without the fix, `NewStreaming` silently drops `stream: true` from requests. See https://github.com/kylecarbs/openai-go/pull/2 for details. |
||
|
Kyle Carberry
|
2245612ece | fix(site): fix browser back navigation between agents settings pages (#23254) | ||
|
Kyle Carberry
|
147d627505 |
fix: deduplicate PR insights, fix cost computation, simplify UI (#23251)
## Problem The `/agents/settings/insights` page had several issues: 1. **Duplicate PRs** in "Recent Pull Requests" — multiple chats referencing the same PR URL each produced a row 2. **Wildly wrong costs** — the cost subquery summed ALL messages across the entire chat *tree* (`GROUP BY root_chat_id`), so every chat in a tree got the same inflated total. When aggregated, the same tree cost was counted N× per PR in that tree 3. **UI clutter** — too many stat cards, too many table columns, mixed naming conventions ## Fix ### Backend (SQL) - **Deduplicate by PR URL** using `DISTINCT ON (COALESCE(cds.url, c.id::text))` across all 4 queries - **Fix cost computation**: use two CTEs — `pr_costs` sums cost from ALL chats that reference a PR (so review chats contribute), `deduped` picks one row per PR for state/additions/deletions via DISTINCT ON - **Tests**: 3 subtests covering multi-chat cost summing, different PRs no duplication, and duplicate URL counted once ### Frontend - **3 stat cards** (down from 5): Merged, Merge rate, Cost / merge - **2-line chart** (down from 3): created (dashed) + merged (solid) - **4-column model table** (down from 7): Model, Merged, Merge rate, Cost/merge - **4-column recent table** (down from 7): Title, Status, Cost, Created — with `table-fixed` to prevent overflow - **Consistent naming**: no mixed PR/PRs abbreviation, contextual labels since page title establishes context |
||
|
Kyle Carberry
|
1f0d896fc9 |
feat: add deleted flag to chat messages for soft-delete (#23223)
Adds a `deleted` boolean column to the `chat_messages` table. Messages are never physically deleted from the database — instead they are marked as deleted so that usage and cost data is preserved. ## Changes ### Migration - New migration (000444) adds `deleted boolean NOT NULL DEFAULT false` to `chat_messages` ### SQL queries - `DeleteChatMessagesAfterID` → `SoftDeleteChatMessagesAfterID` (UPDATE SET deleted=true instead of DELETE) - New `SoftDeleteChatMessageByID` query for single-message soft-delete - All read queries now filter `deleted = false`: - `GetChatMessageByID` - `GetChatMessagesByChatID` - `GetChatMessagesByChatIDDescPaginated` - `GetChatMessagesForPromptByChatID` (both CTE and main query) - `GetLastChatMessageByRole` - Cost/usage queries (`GetChatCostSummary`, `GetChatCostPerModel`, etc.) intentionally still include deleted messages to preserve accurate spend tracking ### EditMessage behavior - Previously: updated the message content in-place + hard-deleted subsequent messages - Now: soft-deletes the original message + soft-deletes subsequent messages + inserts a new message with the updated content - This preserves the original message data (tokens, cost, content) in the database |
||
|
Kyle Carberry
|
cbe29e4e25 |
fix: encode non-ASCII filenames in chat file upload header (#23241)
## Problem Uploading a file on the `/agents` chat page fails with: ``` Failed to execute 'setRequestHeader' on 'XMLHttpRequest': String contains non ISO-8859-1 code point. ``` This happens when the image filename contains non-ASCII characters (e.g. CJK characters from macOS screenshots like `スクリーンショット.png`, accented characters, emoji, etc.). HTTP headers only support ISO-8859-1 code points, and the filename was being interpolated directly into the `Content-Disposition` header. ## Fix Use [RFC 5987](https://datatracker.ietf.org/doc/html/rfc5987) `filename*=UTF-8''` encoding so the percent-encoded name is always valid in the header. A static ASCII `filename="file"` fallback is included for older clients. The server already uses Go's `mime.ParseMediaType` which decodes `filename*` automatically, so no backend changes are needed. ### Before ```ts "Content-Disposition": `attachment; filename="${file.name}"` ``` ### After ```ts "Content-Disposition": `attachment; filename="file"; filename*=UTF-8''${encodeURIComponent(file.name)}` ``` ## Testing Added a server-side test (`TestGetChatFile/UnicodeFilename`) that uploads with a Japanese filename and verifies it round-trips correctly through the `Content-Disposition` header. |
||
|
Kyle Carberry
|
90cf4f0a91 |
refactor: consolidate chat streaming endpoints under /stream (#23248)
Moves per-chat streaming/watch endpoints under a `/stream` sub-route for
better API consistency:
| Before | After |
|--------|-------|
| `GET /{chat}/stream` | `GET /{chat}/stream/` |
| `GET /{chat}/desktop` | `GET /{chat}/stream/desktop` |
| `GET /{chat}/git/watch` | `GET /{chat}/stream/git` |
### Changes
- **`coderd/coderd.go`** — Route definitions: replaced flat routes with
`r.Route("/stream", ...)` sub-router
- **`site/src/api/api.ts`** — Updated WebSocket URLs for `watchChatGit`
and `watchChatDesktop`
- **`coderd/chats_test.go`** — Updated desktop test URL
- **`coderd/workspaceagents_internal_test.go`** — Updated git watcher
test URLs (route mounts + dial URLs)
- **`site/src/pages/AgentsPage/AgentDetail.stories.tsx`** — Updated
storybook WebSocket mock paths
|
||
|
Kyle CarberryandCian Johnston
|
d4a072b61e |
fix: address review comments on InsertChatMessages (#23239)
Follow-up to #23220, addressing Cian's review comments: - **SQL casing**: Uppercase `UNNEST` to match `NULLIF`/`COALESCE` convention in the query. - **Builder pattern**: `chatMessage` struct now uses unexported fields with a `newChatMessage` constructor for required fields (role, content, visibility, modelConfigID, contentVersion) and chainable builder methods (`withCreatedBy`, `withCompressed`, `withUsage`, `withContextLimit`, `withTotalCostMicros`, `withRuntimeMs`) for optional/nullable fields. - **Batch test in chats_test**: Replaced the `for i := 0; i < 2` loop with a single batch insert of 2 messages to actually exercise the batch logic. - **Multi-message querier test**: Added `BatchInsertMultipleMessages` test verifying 3-message batch insert with role ordering, sequential IDs, nullable field semantics (NULL for zero UUIDs and zero ints), and token/cost assertions. --------- Co-authored-by: Cian Johnston <cian@coder.com> |
||
|
Kyle Carberry
|
2577d16af2 |
fix(site): use correct /api/experimental endpoint for PR insights (#23235)
## Problem
The `/agents/settings/insights` page was broken because
`InsightsContent` was calling `/api/v2/chats/insights/pull-requests`,
but the backend route is registered under
`/api/experimental/chats/insights/pull-requests` (the entire `/chats`
route block lives under `r.Route("/api/experimental", ...)` in
`coderd.go`).
Every other chat endpoint in the frontend correctly uses
`/api/experimental/chats/...`, but this one was missed.
## Fix
- Added `getPRInsights` method to the API client (`api.ts`) pointing to
`/api/experimental/chats/insights/pull-requests`
- Added a `prInsights` react-query helper in `api/queries/chats.ts`
(matching the pattern of `chatCostUsers`, etc.)
- Updated `InsightsContent.tsx` to use the query helper instead of a raw
`fetch()` with the wrong URL
|
||
|
Kyle Carberry
|
483adc59fe |
feat: replace InsertChatMessage with batch InsertChatMessages (#23220)
Replaces the singular `InsertChatMessage` query with `InsertChatMessages` that uses PostgreSQL's `unnest()` for batch inserts. This reduces the number of database round-trips when inserting multiple messages in a single transaction. ## Changes - **SQL**: New `InsertChatMessages :many` query using `unnest()` arrays following the existing codebase pattern (e.g., `InsertWorkspaceAgentStats`). Preserves the CTE that updates `chats.last_model_config_id` using the last non-null model config from the batch. Uses `NULLIF` for UUID columns to handle NULL foreign keys. - **Go layers**: Updated `querier.go`, `dbauthz.go`, `dbmetrics/querymetrics.go`, `dbmock/dbmock.go`, and `queries.sql.go` to use the new batch signature (`[]ChatMessage` return type, array params). - **chatd.go**: All call sites converted to batch inserts: - **CreateChat**: System prompt + user message batched into one call - **persistStep**: Assistant message + tool messages batched into one call - **persistSummary**: Hidden summary + assistant + tool messages batched into one call - Single-message sites use the same API with single-element arrays - **Helper**: New `appendChatMessage` function simplifies building batch params at each call site. - **Tests**: All test files updated to use the new API. Builds on top of #23213. |
||
|
Kyle Carberry
|
a130a7dc97 |
fix: renumber duplicate migration 000444 to 000445 (#23229)
Two migrations were merged with the same number 000444: - `000444_usage_events_ai_seats` (#22689, merged first at 09:30) — keeps 000444 - `000444_chat_message_runtime_ms` (#23219, merged second at 10:57) — renumbered to **000445** This collision causes `golang-migrate` to fail at runtime since it reads both files as the same version. **Fix:** Rename `000444_chat_message_runtime_ms.{up,down}.sql` → `000445_chat_message_runtime_ms.{up,down}.sql`. Closes https://github.com/coder/internal/issues/1411 |
||
|
Kyle Carberry
|
d6fef96d72 |
feat: add PR insights analytics dashboard (#23215)
## What Adds a new admin-only **PR Insights** page for the `/agents` analytics view — a dashboard for engineering leaders to understand code shipped by AI agents. ### Backend - `GET /api/v2/chats/insights/pull-requests` — admin-only endpoint - 4 SQL queries in `chatinsights.sql` aggregating `chat_diff_statuses` joined with chat cost data (via root chat tree rollup) - Runs 5 parallel DB queries: current summary, previous summary (for trends), time series, per-model breakdown, recent PRs - SDK types auto-generate to TypeScript ### Frontend (`PRInsightsView`) - **Stat cards**: PRs created, Merged, Merge rate, Lines shipped, Cost/merged PR — with trend badges comparing to previous period - **Activity chart**: Stacked area chart (created/merged/closed) using git color tokens (`git-added-bright`, `git-merged-bright`, `git-deleted-bright`) - **Model performance table**: Per-model PR counts, inline merge rate bars, diff stats, cost breakdown - **Recent PRs table**: Status badges, review state icons, author info, external links - **Time range filter**: 7d/14d/30d/90d button group - **4 Storybook stories**: Default, HighPerformance, LowVolume, NoPRs ### Data source All PR data comes from the existing `chat_diff_statuses` table (populated by the `gitsync.Worker` background job that polls GitHub every 120s). No new data collection required. ### Screenshot View in Storybook: `pages/AgentsPage/PRInsightsView` |
||
|
Kyle Carberry
|
4dd8531f37 |
feat: track step runtime_ms on chat messages (#23219)
## Summary Adds a `runtime_ms` column to `chat_messages` that records the wall-clock duration (in milliseconds) of each LLM step. This covers LLM streaming, tool execution, and retries — the full time the agent is "alive" for a step. This is the foundation for billing by agent alive time. The column follows the same pattern as `total_cost_micros`: stored per assistant message, aggregatable with `SUM()` over time periods by user. ## Changes - **Migration**: adds nullable `runtime_ms bigint` to `chat_messages`. - **chatloop**: adds `Runtime time.Duration` field to `PersistedStep`, measures `time.Since(stepStart)` at the beginning of each step (covering stream + tool execution + retries). - **chatd**: passes `step.Runtime.Milliseconds()` to the assistant message `InsertChatMessage` call; all other message types (system, user, tool) get `NULL`. - **Tests**: adds `runtime > 0` assertion in chatloop tests. ## Billing query pattern Once ready, aggregation mirrors the existing cost queries: ```sql SELECT COALESCE(SUM(cm.runtime_ms), 0)::bigint AS total_runtime_ms FROM chat_messages cm JOIN chats c ON c.id = cm.chat_id WHERE c.owner_id = @user_id AND cm.created_at >= @start_time AND cm.created_at < @end_time AND cm.runtime_ms IS NOT NULL; ``` |
||
|
Kyle Carberry
|
b83b93ea5c |
feat: add workspace awareness system message on chat creation (#23213)
When a chat is created via `chatd`, a system message is now inserted informing the model whether the chat was created with or without a workspace. **With workspace:** > This chat is attached to a workspace. You can use workspace tools like execute, read_file, write_file, etc. **Without workspace:** > There is no workspace associated with this chat yet. Create one using the create_workspace tool before using workspace tools like execute, read_file, write_file, etc. This is a model-only visibility system message (not shown to users) that helps the model understand its available capabilities upfront — particularly important for subagents spawned without a workspace, which previously would attempt to use workspace tools and fail. **Changes:** - `coderd/chatd/chatd.go`: Added workspace awareness constants and inserted the system message in `CreateChat` after the system prompt, before the initial user message. - `coderd/chatd/chatd_test.go`: Added `TestCreateChatInsertsWorkspaceAwarenessMessage` with sub-tests for both with-workspace and without-workspace cases. |
||
|
Kyle Carberry
|
d42008e93d |
fix: persist partial assistant response when chat is interrupted mid-stream (#23193)
## Problem
When a user cancels a streaming chat response mid-stream, the partial
content disappears entirely — both from the UI and the database. The
streamed text vanishes as if the response never happened.
## Root Causes
Three issues combine to prevent partial message persistence on
interrupt:
### 1. StreamPartTypeError only matched `context.Canceled`
(`chatloop.go`)
The interrupt detection in `processStepStream` checked:
```go
errors.Is(part.Error, context.Canceled) && errors.Is(context.Cause(ctx), ErrInterrupted)
```
But some providers propagate `ErrInterrupted` directly as the stream
error rather than wrapping it in `context.Canceled`. This caused the
condition to fail, so `flushActiveState` was never called and partial
text accumulated in `activeTextContent` was lost.
### 2. No post-loop interrupt check (`chatloop.go`)
If the stream iterator stops yielding parts without producing a
`StreamPartTypeError` (e.g., a provider that silently closes the
response body on cancel), there was no check after the `for part :=
range stream` loop to detect the interrupt and flush active state.
### 3. Worker ownership check blocked interrupted persists (`chatd.go`)
`InterruptChat` → `setChatWaiting` clears `worker_id` in the DB
**before** the chatloop detects the interrupt. When
`persistInterruptedStep` (using `context.WithoutCancel`) tried to write
the partial message, the ownership check:
```go
if !lockedChat.WorkerID.Valid || lockedChat.WorkerID.UUID != p.workerID {
return chatloop.ErrInterrupted // always blocks!
}
```
unconditionally rejected the write. The error was silently logged as a
warning.
## Fix
- **Broaden the `StreamPartTypeError` interrupt detection** to match
both `context.Canceled` and `ErrInterrupted` as the stream error.
- **Add a post-loop interrupt check** in `processStepStream` that
flushes active state when the context was canceled with
`ErrInterrupted`.
- **Allow `persistStep` to write when the chat is in `waiting` status**
(interrupt) even if `worker_id` was cleared. The `pending` status (from
`EditMessage`, where history is truncated) still correctly blocks stale
writes.
## Testing
Added `TestInterruptChatPersistsPartialResponse` — an end-to-end
integration test that:
1. Streams partial text chunks from a mock LLM
2. Waits for the chatloop to publish `message_part` events (confirming
chunks were processed)
3. Interrupts the chat mid-stream
4. Verifies the partial assistant message is persisted in the database
with the expected text content
|
||
|
Kyle Carberry
|
6fc9f195f1 |
fix: resolve chat message pagination scroll issues (#23169)
## Summary Fixes four interrelated issues that caused scroll position jumps and phantom scroll growth when paginating older chat messages. ## Changes ### 1. Removed client-side message windowing (`useMessageWindow`) There were two competing sentinel systems: server-side pagination and client-side windowing. The client windowing sentinel was nested deep inside the timeline with no explicit IntersectionObserver `root`, causing scroll position jumps when messages were prepended. Blink (coder/blink) has no client-side windowing. Removed it entirely; server pagination + `contentVisibility` handled performance. ### 2. Removed `contentVisibility: "auto"` from message sections Each section had `contentVisibility: "auto"` with `containIntrinsicSize: "1px 600px"`, causing the scroll region to grow/shrink as the browser swapped 600px placeholders for actual heights while scrolling. This created phantom scroll growth with no fetch involved. ### 3. Gated WebSocket on initial REST data The WebSocket `Subscribe` snapshot calls `GetChatMessagesByChatID` (no LIMIT) which returns every message when `afterMessageID` is 0. The WebSocket effect opened before the REST page resolved, so `lastMessageIdRef` was undefined, causing the server to replay the entire history and defeating pagination. Added `initialDataLoaded` guard so the socket waits for the first REST page. ### 4. Manual scroll position restoration Replaced unreliable CSS scroll anchoring in `flex-col-reverse` with a `ScrollAnchoredContainer` that snapshots `scrollHeight` before fetch and restores `scrollTop` via `useLayoutEffect` after render. Disabled browser scroll anchoring (`overflow-anchor: none`) to prevent conflicts. |
||
|
Kyle Carberry
|
497e1e6589 |
feat: render file references inline in user messages (#23174)
File references in user messages now render as inline chips (matching the chat input style) instead of in a separate bordered section at the bottom of the message bubble. This reimplements #23131 which was accidentally reverted during the merge of #23072 (the spend-limit UI PR resolved a merge conflict by dropping the inline chip logic). ## Changes - **FileReferenceNode.tsx**: Export `FileReferenceChip` so it can be imported for read-only use (no remove button when `onRemove` is omitted). - **ConversationTimeline.tsx**: Iterate through `parsed.blocks` in document order, rendering `response` blocks as text and `file-reference` blocks as inline `FileReferenceChip` components. Removes the old separated file-reference section with `border-t` divider. - **ConversationTimeline.stories.tsx**: Added `UserMessageWithInlineFileRef` and `UserMessageWithMultipleInlineFileRefs` stories. |
||
|
Kyle Carberry
|
b779c9ee33 |
fix: use SQL-level auth filtering for chat listing (#23159)
## Problem The chat listing endpoint (`GetChatsByOwnerID`) was using `fetchWithPostFilter`, which fetches N rows from the database and then filters them in Go memory using RBAC checks. This causes a pagination bug: if the user requests `limit=25` but some rows fail the auth check, fewer than 25 rows are returned even though more authorized rows exist in the database. The client may incorrectly assume it has reached the end of the list. ## Solution Switch to the same pattern used by `GetWorkspaces`, `GetTemplates`, and `GetUsers`: `prepareSQLFilter` + `GetAuthorized*` variant. The RBAC filter is compiled to a SQL WHERE clause and injected into the query before `ORDER BY`/`LIMIT`, so the database returns exactly the requested number of authorized rows. Additionally, `GetChatsByOwnerID` is renamed to `GetChats` with `OwnerID` as an optional (nullable) filter parameter, matching the `GetWorkspaces` naming convention. ## Changes | File | Change | |------|--------| | `queries/chats.sql` | Renamed to `GetChats`, `owner_id` now optional via CASE/NULL, added `-- @authorize_filter` | | `queries.sql.go` | Renamed constant, params struct (`GetChatsParams`), and method | | `querier.go` | Interface method renamed | | `modelqueries.go` | Added `chatQuerier` interface + `GetAuthorizedChats` impl | | `dbauthz/dbauthz.go` | `GetChats` now uses `prepareSQLFilter` instead of `fetchWithPostFilter` | | `dbauthz/dbauthz_test.go` | Updated tests for SQL filter pattern | | `dbmock/dbmock.go` | Renamed + added mock for `GetAuthorizedChats` | | `dbmetrics/querymetrics.go` | Renamed + added metrics wrapper | | `rbac/regosql/configs.go` | Added `ChatConverter` (maps `org_owner` to empty string literal since `chats` has no `organization_id` column) | | `rbac/authz.go` | Added `ConfigChats()` | | `chats.go` | Handler uses renamed method with `uuid.NullUUID` | | `searchquery/search.go` | Updated return type | | `gitsync/worker.go` | Updated interface and call site | | Various test files | Updated for renamed types | |
||
|
Kyle Carberry
|
a40716b6fe |
fix(site): stop spamming chats list endpoint on diff_status_change events (#23167)
## Problem
The WebSocket handler for `diff_status_change` events in
`AgentsPage.tsx` was triggering a burst of redundant HTTP requests on
every event:
1. **`invalidateChatListQueries(queryClient)`** — Full refetch of the
chats list endpoint. Unnecessary because `updateInfiniteChatsCache`
already writes `diff_status` into the sidebar cache optimistically on
every event.
2. **`invalidateQueries({ queryKey: chatKey(id) })`** — Refetch of the
individual chat. Also unnecessary — the SSE event carries `diff_status`
in its payload and the optimistic updater writes it into the `chatKey`
cache directly. Worse, this call was missing `exact: true`, so TanStack
Query's prefix matching cascaded the invalidation to `chatMessagesKey`,
`chatDiffContentsKey`, and every other query under `["chats", id]`.
Since diff status changes fire frequently during active agent work, this
spammed the chats list endpoint and caused redundant refetches of
messages and diff contents on every single event.
## Fix
Strip the handler down to the one invalidation that's actually needed —
`chatDiffContentsKey` (the file-level diff contents aren't in the SSE
payload):
```typescript
if (chatEvent.kind === "diff_status_change") {
void queryClient.invalidateQueries({
queryKey: chatDiffContentsKey(updatedChat.id),
exact: true,
});
}
```
## Why tests didn't catch this
The existing tests in `chats.test.ts` cover query utilities in isolation
(e.g. `invalidateChatListQueries` scoping, mutation invalidation). The
WebSocket event handler lives in the `AgentsPage` component — there was
no test covering what the `diff_status_change` code path actually
invalidates.
Added regression tests verifying that `exact: true` prevents
prefix-match cascade vs the old behavior.
|
||
|
Kyle Carberry
|
075dfecd12 |
refactor: consolidate experimental chats API types (#23143)
## Summary
Consolidates three areas of type duplication in the experimental chats
API:
### 1. Merge archive/unarchive into `PATCH /{chat}`
- **Before:** `POST /{chat}/archive` + `POST /{chat}/unarchive` (two
endpoints, two handlers with mirrored logic)
- **After:** `PATCH /{chat}` accepting `{ "archived": true/false }` via
`UpdateChatRequest`
- Removes one endpoint and ~30 lines of duplicated handler code
### 2. Collapse identical request/response prompt types
- `ChatSystemPromptResponse` + `UpdateChatSystemPromptRequest` →
`ChatSystemPrompt`
- `UserChatCustomPromptResponse` + `UpdateUserChatCustomPromptRequest` →
`UserChatCustomPrompt`
- These pairs were field-for-field identical (single string field)
### 3. Merge duplicate reasoning options types
- `ChatModelOpenRouterReasoningOptions` +
`ChatModelVercelReasoningOptions` → `ChatModelReasoningOptions`
- Same 4 fields, same types — only field ordering and enum value sets
differed
- Unified type uses the superset of enum values
### Files changed
- `codersdk/chats.go` — SDK types and client methods
- `coderd/chats.go` — Handler consolidation
- `coderd/coderd.go` — Route change
- `coderd/chats_test.go` — Test updates
- `site/src/api/api.ts` — Frontend API client
- `site/src/api/queries/chats.ts` — Query mutations
- `site/src/api/queries/chats.test.ts` — Test mocks
- `site/src/pages/AgentsPage/AgentsPage.tsx` — Call site
- Generated files (`typesGenerated.ts`,
`chatModelOptionsGenerated.json`)
### Testing
- All Go tests pass (`TestArchiveChat`, `TestUnarchiveChat`,
`TestChatSystemPrompt`)
- All frontend tests pass (31/31 in `chats.test.ts`)
|
||
|
Kyle Carberry
|
ca5158f94a |
fix: unify sidebar Git/Desktop tab styles with GitPanel tabs (#23164)
The active Git tab looked different than the Desktop tab, and they didn't match the actual tabs in the Git section. |
||
|
Kyle Carberry
|
eb828a6a86 | fix: skip input refocus after send on mobile viewports (#23141) | ||
|
Kyle Carberry
|
b69631cb35 | chore(site): improve mobile layout for agent chat (#23139) | ||
|
Kyle Carberry
|
7b0aa31b55 | feat: render file references inline in user messages (#23131) | ||
|
Kyle Carberry
|
6972d073a2 |
fix: improve background process handling for agent tools (#23132)
## Problem Models frequently use shell `&` instead of `run_in_background=true` when starting long-running processes through `/agents`, causing them to die shortly after starting. This happens because: 1. **No guidance in tool schema** — The `ExecuteArgs` struct had zero `description` tags. The model saw `run_in_background: boolean (optional)` with no explanation of when/why to use it. 2. **Shell `&` is silently broken** — `sh -c "command &"` forks the process, the shell exits immediately, and the forked child becomes an orphan not tracked by the process manager. 3. **No process group isolation** — The SSH subsystem sets `Setsid: true` on spawned processes, but the agent process manager set no `SysProcAttr` at all. Signals only hit the top-level `sh`, not child processes. ## Investigation Compared our implementation against **openai/codex** and **coder/mux**: | Aspect | codex | mux | coder/coder (before) | |--------|-------|-----|---------------------| | Background flag | Yield/resume with `session_id` | `run_in_background` with rich description | `run_in_background` with **no description** | | `&` handling | `setsid()` + `killpg()` | `detached: true` + `killProcessTree()` | **Nothing** — orphaned children escape | | Process isolation | `setsid()` on every spawn | `set -m; nohup ... setsid` for background | **No `SysProcAttr` at all** | | Signal delivery | `killpg(pgid, sig)` — entire group | `kill -15 -\$pid` — negative PID | `proc.cmd.Process.Signal()` — **PID only** | ## Changes ### Fix 1: Add descriptions to `ExecuteArgs` (highest impact) The model now sees explicit guidance: *"Use for long-running processes like dev servers, file watchers, or builds. Do NOT use shell & — it will not work correctly."* ### Fix 2: Update tool description The top-level execute tool description now reinforces: *"Use run_in_background=true for long-running processes. Never use shell '&' for backgrounding."* ### Fix 3: Detect trailing `&` and auto-promote to background Defense-in-depth: if the model still uses `command &`, we strip the `&` and promote to `run_in_background=true` automatically. Correctly distinguishes `&` from `&&`. ### Fix 4: Process group isolation (`Setpgid`) New platform-specific files (`proc_other.go` / `proc_windows.go`) following the same pattern as `agentssh/exec_other.go`. Every spawned process gets its own process group. ### Fix 5: Process group signaling `signal()` now uses `syscall.Kill(-pid, sig)` on Unix to signal the entire process group, ensuring child processes from shell pipelines are also cleaned up. ## Testing All existing `agent/agentproc` tests pass. Both packages compile cleanly. |
||
|
Kyle Carberry
|
89bb5bb945 |
ci: fix build job disk exhaustion on Depot runners (#23136)
## Problem The `build` job on `main` has been failing intermittently (and now consistently) with `no space left on device` on the `depot-ubuntu-22.04-8` runner. The runner's disk fills up during Docker image builds or SBOM generation, depending on how close to the limit a given run lands. The build was already at the boundary — the Go build cache alone is ~1.3 GB, build artifacts are ~2 GB, and Docker image builds + SBOM scans need several hundred MB of headroom in `/tmp`. No single commit caused this; cumulative growth in dependencies and the scheduled `coder-base:latest` rebuild on Monday morning nudged it past the limit. ## Fix Three changes to reclaim ~2 GB of disk before Docker runs: 1. **Build all platform archives and packages in the Build step** — moves arm64/armv7 `.tar.gz` and `.deb` from the Docker step to the Build step so we can clean caches in between. 2. **Clean up Go caches between Build and Docker** — once binaries are compiled, the Go build cache and module cache aren't needed. Also removes `.apk`/`.rpm` packages that are never uploaded. 3. **Set `DOCKER_IMAGE_NO_PREREQUISITES`** — tells make to skip redundantly building `.deb`/`.rpm`/`.apk`/`.tar.gz` as prerequisites of Docker image targets. The Makefile already supports this flag for exactly this purpose. |
||
|
Kyle Carberry
|
b7eab35734 |
fix(site): scope chat cache helpers to chat-list queries only (#23134)
## Problem
`updateInfiniteChatsCache`, `prependToInfiniteChatsCache`, and
`readInfiniteChatsCache` use `setQueriesData({ queryKey: ["chats"] })`
which prefix-matches **all** queries starting with `"chats"`, including
`["chats", chatId, "messages"]`.
After #23083 converted chat messages to `useInfiniteQuery`, the cached
messages data gained a `.pages` property containing
`ChatMessagesResponse` objects (not `Chat[]` arrays). The `if
(!prev.pages)` guard no longer bailed out, and the updater called
`.map()` on these objects — `TypeError: Z.map is not a function`.
## Fix
Extract the `isChatListQuery` predicate that already existed inline in
`invalidateChatListQueries` and apply it to all four cache helpers. This
scopes them to sidebar queries (`["chats"]` or `["chats",
<filterOpts>]`) and skips per-chat queries (`["chats", <id>, ...]`).
|
||
|
Kyle Carberry
|
741af057dc |
feat: paginate chat messages endpoint with cursor-based infinite scroll (#23083)
Adds cursor-based pagination to the chat messages endpoint. ## Backend - New `GetChatMessagesByChatIDPaginated` SQL query: returns messages in `id DESC` order with a `before_id` keyset cursor and configurable `limit` - Handler parses `?before_id=N&limit=N` query params, uses the `LIMIT N+1` trick to set `has_more` without a separate COUNT query - Queued messages only returned on the first page (no cursor) since they're always the most recent - SDK client updated with `ChatMessagesPaginationOptions` - Fully backward compatible: omitting params returns the 50 newest messages ## Frontend - Switches `getChatMessages` from `useQuery` to `useInfiniteQuery` with cursor chaining via `getNextPageParam` - Pages flattened and sorted by `id` ascending for chronological display - `MessagesPaginationSentinel` component uses `IntersectionObserver` (200px rootMargin prefetch) inside the existing `flex-col-reverse` scroll container - `flex-col-reverse` handles scroll anchoring natively when older messages are prepended — no manual `scrollTop` adjustment needed (same pattern as coder/blink) ## Why cursor-based instead of offset/limit Offset-based pagination breaks when new messages arrive while paginating backward (offsets shift, causing duplicates or missed messages). The `before_id` cursor is stable regardless of inserts — each page is deterministic. |
||
|
Kyle Carberry
|
32a894d4a7 |
fix: error on ambiguous matches in edit_files tool (#23125)
## Problem The `edit_files` tool used `strings.ReplaceAll` for exact substring matches, silently replacing **every** occurrence. When an LLM's search string wasn't unique in the file, this caused unintended edits. Fuzzy matches (passes 2 and 3) only replaced the first occurrence, creating inconsistent behavior. Zero matches were also silently ignored. ## Investigation Investigated how **coder/mux** and **openai/codex** handle this: | Tool | Multiple matches | No match | Flag | |---|---|---|---| | **coder/mux** `file_edit_replace_string` | Error (default `replace_count=1`) | Error | `replace_count` (int, default 1, -1=all) | | **openai/codex** `apply_patch` | Uses first match after cursor (structural disambiguation via context lines + `@@` markers) | Error | None (different paradigm) | | **coder/coder** `edit_files` (before) | Exact: replaces all. Fuzzy: replaces first. | Silent success | None | ## Solution Adopted the mux approach (error on ambiguity) with a simpler `replace_all: bool` instead of `replace_count: int`: - **Default (`replace_all: false`)**: search string must match exactly once. Multiple matches → error with guidance: *"search string matches N occurrences. Include more surrounding context to make the match unique, or set replace_all to true"* - **`replace_all: true`**: replaces all occurrences (opt-in for intentional bulk operations like variable renames) - **Zero matches**: now returns an error instead of silently succeeding Chose `bool` over `int` count because: 1. LLMs are bad at counting occurrences 2. The real intent is binary (one specific spot vs. all occurrences) 3. Simpler error recovery loop for the LLM ## Changes | File | Change | |---|---| | `codersdk/workspacesdk/agentconn.go` | Add `ReplaceAll bool` to `FileEdit` struct | | `agent/agentfiles/files.go` | Count matches before replacing; error if >1 and not opted in; error on zero matches; add `countLineMatches` helper | | `codersdk/toolsdk/toolsdk.go` | Expose `replace_all` in tool schema with description | | `agent/agentfiles/files_test.go` | Update existing tests, add `EditEditAmbiguous`, `EditEditReplaceAll`, `NoMatchErrors`, `AmbiguousExactMatch`, `ReplaceAllExact` | |
||
|
Kyle Carberry
|
af1be592cf |
fix: disable agent notification chime by default (#23124)
The completion chime on `/agents` was enabled by default for new users (or when no localStorage preference existed). This changes the default to disabled, so users must explicitly opt in via the sound toggle button. ## Changes - `getChimeEnabled()` now returns `false` when no preference is stored (was `true`) - `catch` fallback also returns `false` (was `true`) - Updated tests to reflect the new default and explicitly enable the chime in `maybePlayChime` tests |
||
|
Kyle Carberry
|
6f97539122 |
fix: update sidebar diff status on WebSocket events (#23116)
## Problem The sidebar diff status (PR icon, +additions/-deletions, file count) was not updating in real-time. Users had to reload the page to see changes. Two root causes: 1. **Frontend**: The `diff_status_change` WebSocket handler in `AgentsPage.tsx` had an early `return` (line 398) that skipped `updateInfiniteChatsCache`, so the sidebar's cache was never updated. Even for other event types, the cache merge only spread `status` and `title` — never `diff_status`. 2. **Server**: `publishChatPubsubEvent` in `chatd.go` constructed a minimal `Chat` payload without `DiffStatus`, so even if the frontend consumed the event, `updatedChat.diff_status` would be `undefined`. ## Fix ### Server (`coderd/chatd/chatd.go`) - `publishChatPubsubEvent` now accepts an optional `*codersdk.ChatDiffStatus` parameter; when non-nil it's set on the outgoing `Chat` payload. - `PublishDiffStatusChange` fetches the diff status from the DB, converts it, and passes it through. - Added `convertDBChatDiffStatus` (mirrors `coderd/chats.go`'s converter to avoid circular import). - All other callers pass `nil`. ### Frontend (`site/src/pages/AgentsPage/AgentsPage.tsx`) - Removed the early `return` so `diff_status_change` events fall through to the cache update logic. - Added `isDiffStatusEvent` flag and spread `diff_status` into both the infinite chats cache (sidebar) and the individual chat cache. |
||
|
Kyle Carberry
|
530872873e |
chore: remove swagger annotations from experimental chat endpoints (#23120)
The `/archive` and `/desktop` chat endpoints had swagger route comments (`@Summary`, `@ID`, `@Router`, etc.) that would cause them to appear in generated API docs. Since these live under `/experimental/chats`, they should not be documented. This removes the swagger annotations and adds the standard `// EXPERIMENTAL: this endpoint is experimental and is subject to change.` comment to `archiveChat` (the `watchChatDesktop` handler already had it, just needed the swagger block removed). |
||
|
Kyle Carberry
|
27cbf5474b |
refactor: remove /diff-status endpoint, include diff_status in chat payload (#23082)
The `/chats/{chat}/diff-status` endpoint was redundant because:
- The `Chat` type already has a `DiffStatus` field
- Listing chats already resolves and returns `diff_status`
- The `getChat` endpoint was the only one not resolving it (passing
`nil`)
## Changes
**Backend:**
- `getChat` now calls `resolveChatDiffStatus` and includes the result in
the response
- Removed `getChatDiffStatus` handler, route (`GET /diff-status`), and
SDK method
- Tests updated to use `GetChat` instead of `GetChatDiffStatus`
**Frontend:**
- `AgentDetail.tsx`: uses `chatQuery.data?.diff_status` instead of
separate query
- `RemoteDiffPanel.tsx`: accepts `diffStatus` as a prop instead of
fetching internally
- `AgentsPage.tsx`: `diff_status_change` events now invalidate the chat
query
- Removed `chatDiffStatus` query, `chatDiffStatusKey`, and
`getChatDiffStatus` API method
|
||
|
Kyle Carberry
|
3d9628c27e |
ci: split build artifacts into per-platform uploads (#23081)
Splits the single `coder` artifact (containing all platforms in a 1.3GB zip) into individual artifacts per OS/arch/format. ## Problem All CI build artifacts are uploaded as a single artifact named `coder`, producing a 1.3GB zip containing every platform's binary. This makes it impossible to download a single platform's binary without pulling the entire bundle. ## Solution Upload each platform/format combination as a separate artifact: | Artifact Name | Contents | |---|---| | `coder-linux-amd64.tar.gz` | Linux amd64 tarball | | `coder-linux-amd64.deb` | Linux amd64 deb package | | `coder-linux-arm64.tar.gz` | Linux arm64 tarball | | `coder-linux-arm64.deb` | Linux arm64 deb package | | `coder-linux-armv7.tar.gz` | Linux armv7 tarball | | `coder-linux-armv7.deb` | Linux armv7 deb package | | `coder-windows-amd64.zip` | Windows amd64 zip | ## Plan This is the first step toward letting customers install directly from `main` via: ```bash curl -L https://coder.com/install.sh | sh -s -- --unsafe-unstable ``` GitHub Actions artifact downloads require authentication even for public repos, so the next steps are to add a small Cloudflare Worker (similar to the one we already have for `install.sh`) that: 1. Lists artifacts via the GitHub API (unauthenticated) to find the latest artifact ID for the requested platform 2. Calls the download endpoint with a GitHub token (CF Worker secret) to get a 302 redirect to a time-limited Azure Blob URL 3. Redirects the caller to that URL (which requires no auth) This gives us publicly accessible per-platform URLs that the `--unsafe-unstable` flag would point at. The worker doesn't proxy the binary itself — it only proxies the metadata API call (~1KB) and redirects for the actual download. This PR splits the artifacts so the worker can serve individual platform downloads (~200MB each) instead of forcing a 1.3GB bundle. |
||
|
Kyle Carberry
|
266c611716 |
refactor(site): consolidate Git panel diff viewers and polish UI (#23080)
## Summary Refactors the Git panel in the Agents page to consolidate duplicated diff viewer code and significantly improve the UI. ### Deduplication - **RemoteDiffPanel** now uses the shared `DiffViewer` component instead of duplicating file tree, lazy loading, scroll tracking, and layout (~500 lines removed). - Renamed `RepoChangesPanel` → `LocalDiffPanel`, `FilesChangedPanel` → `RemoteDiffPanel` to reflect actual scope. - Removed `headerLeft`/`headerRight` abstraction from `DiffViewer` — each consumer owns its own header. - Replaced hand-rolled `ChatDiffStatusResponse` with auto-generated `ChatDiffStatus` from `typesGenerated.ts`. ### Tab Redesign - Per-repo tabs: each local repo gets its own tab (`Working <repo>`) instead of a single stacked view. - PR tab shows state icon + PR title; branch-only tab shows branch icon. - Tabs use `Button variant="outline"` matching the Git/Desktop tab style. - Radix `ScrollArea` with thin horizontal scrollbar for tab overflow. - Diff style toggle and refresh button lifted to shared toolbar, always visible. ### PR Header - Compact sub-header: `base_branch ←`, state badge (`Open`/`Draft`/`Merged`/`Closed`), diff stats, and `View PR` button. - GitHub-style state-aware icons (green open, gray draft, purple merged, red closed). - New API fields synced: `base_branch`, `author_login`, `pr_number`, `commits`, `approved`, `reviewer_count`. ### Local Changes Header - Compact sub-header: branch name, repo root path, diff stats, and `Commit` button (styled to match `View PR`). - `CircleDotIcon` (amber) for working changes tabs — universal "modified" indicator. ### Visual Polish - All text in sub-headers and buttons at 13px matching chat font size. - All badges (`DiffStatBadge`, PR state, `View PR`, `Commit`) use consistent `border-border-default`, `rounded-sm`, `leading-5`. - No background color on diff viewer header bars. - Tabs hidden when their view has no content; auto-switch when active tab disappears. ### Stories - New `GitPanel.stories.tsx` covering: open PR + working changes, draft PR, merged PR, closed PR, branch only, working changes only, multiple repos, empty state. - Removed old `LocalDiffPanel.stories.tsx` and `RemoteDiffPanel.stories.tsx`. |
||
|
Kyle Carberry
|
83e4f9f93e |
fix(agents): narrow chat mutation query invalidation (#23078)
## Problem
Sending a message on the `/agents` page triggers a burst of redundant
HTTP requests. The root cause is that chat mutations call
`invalidateQueries({ queryKey: ["chats"] })` which, due to React Query's
default **prefix matching**, cascades to every query whose key starts
with `["chats"]`:
- `["chats", {archived: false}]` — infinite sidebar list
- `["chats", chatId]` — individual chat detail
- `["chats", chatId, "messages"]` — all messages
- `["chats", chatId, "diff-status"]` — diff status
- `["chats", chatId, "diff-contents"]` — diff contents
- `["chats", "costSummary", ...]` — cost summaries
All of these have active subscribers on the page, so each one fires a
network request. The WebSocket stream already delivers these updates in
real-time, making the HTTP refetches completely redundant.
## Fix
| Mutation | Before | After |
|---|---|---|
| `createChatMessage` | `invalidateQueries({ queryKey: chatsKey })` —
prefix cascade | **Removed** — WebSocket delivers messages + sidebar
updates |
| `interruptChat` | `invalidateQueries({ queryKey: chatsKey })` — prefix
cascade | **Removed** — WebSocket delivers status changes |
| `editChatMessage` | 3 broad invalidations including `chatsKey` prefix
| 2 targeted with `exact: true`: `chatKey(id)` + `chatMessagesKey(id)` |
| `promoteChatQueuedMessage` | 3 broad invalidations including
`chatsKey` prefix | 2 targeted with `exact: true`: `chatKey(id)` +
`chatMessagesKey(id)` |
`editChatMessage` keeps `chatMessagesKey` invalidation because editing
truncates messages server-side and the WebSocket can only insert/update,
never remove stale entries.
## Net effect
Sending a message previously triggered **5–7 HTTP requests**. Now it
triggers **zero** — the WebSocket handles everything.
## Tests
Added `describe("mutation invalidation scope")` with 8 test cases
asserting that each mutation only invalidates the queries it genuinely
needs.
|
||
|
Kyle Carberry
|
ff9d061ae9 |
fix(site): prevent duplicate chat in agents sidebar on creation (#23077)
## Problem
When creating a new chat in the agents page (`/agents`), the chat could
appear multiple times in the sidebar. This was a race condition
triggered by the WebSocket `created` event handler.
## Root Cause
`updateInfiniteChatsCache` applies its updater function **independently
on each page** of the infinite query:
```ts
const nextPages = prev.pages.map((page) => updater(page));
```
When the `watchChats` WebSocket received a `"created"` event, the
handler checked `exists` only within the *current page*, then prepended
the new chat if not found:
```ts
updateInfiniteChatsCache(queryClient, (chats) => {
const exists = chats.some((c) => c.id === updatedChat.id);
// ...
if (chatEvent.kind === "created") {
return [updatedChat, ...chats]; // runs per page!
}
});
```
Since a brand-new chat doesn't exist in any page, **every loaded page**
prepends it. After `pages.flat()`, the chat appears once per loaded page
in the sidebar.
## Fix
- Added `prependToInfiniteChatsCache` in `chats.ts` that checks across
**all pages** before prepending, and only adds to page 0.
- Split the WebSocket handler so `"created"` events use the new safe
prepend, while update events (`title_change`, `status_change`) continue
using `updateInfiniteChatsCache` (which is safe for `.map()` operations
that don't add entries).
|
||
|
Kyle Carberry
|
0d3e39a24e |
feat: add head_branch to pull request diff status (#23076)
Adds the `head_branch` field (the source/feature branch name of a PR) to the diff status pipeline. Previously only `base_branch` (target branch) and the head commit SHA were captured from the GitHub API, but not the head branch name itself. ## Changes - **Migration 438**: Add `head_branch` nullable TEXT column to `chat_diff_statuses` - **gitprovider**: Parse `head.ref` from the GitHub API response (alongside `head.sha`) and add `HeadBranch` to `PRStatus` - **gitsync**: Wire `HeadBranch` through `refreshOne()` into the DB upsert params - **worker**: Map `HeadBranch` in `chatDiffStatusFromRow()` - **coderd**: Convert `HeadBranch` in `convertChatDiffStatus()` - **codersdk**: Expose as `head_branch` (`*string`, omitempty) in `ChatDiffStatus` API response - **Tests**: Updated `github_test.go` pull JSON fixtures and assertions |
||
|
Kyle Carberry
|
ddd1e86a90 |
fix(site): prevent infinite scroll from spamming duplicate chat list requests (#23075)
## Problem The agents sidebar infinite scroll was spamming the `/api/v2/chats` endpoint with duplicate requests at the same offset, caused by the `LoadMoreSentinel` component. ### Root cause `onLoadMore` is an inline arrow function (`() => void chatsQuery.fetchNextPage()`), creating a **new function reference on every render**. The `useEffect` in `LoadMoreSentinel` depended on `[onLoadMore]`, so it tore down and re-created the `IntersectionObserver` on every render. Each new observer immediately fired its callback when the sentinel was already visible, triggering duplicate fetches. ## Fix - Store `onLoadMore` and `isFetchingNextPage` in **refs** so the observer callback always reads the latest values without needing to tear down/re-create. - Create the `IntersectionObserver` **once on mount** (empty deps array). - **Guard** against calling `onLoadMore` while `isFetchingNextPage` is true. ## Tests - **LoadMoreSentinel behavior tests** (6 tests): verifies no duplicate calls across re-renders, proper `isFetchingNextPage` gating, ref-based observer stability, and correct resume after fetch completes. - **`infiniteChats` query factory tests** (6 tests): covers `getNextPageParam` and `queryFn` offset computation to prevent pagination regressions. |
||
|
Kyle Carberry
|
b492c42624 |
chore(dogfood): add Google Chrome to dogfood image (#23063)
Install Google Chrome stable directly from `dl.google.com`. Ubuntu 22.04 ships `chromium-browser` as a snap-only package, which does not work in Docker containers. ```dockerfile RUN wget -q https://dl.google.com/linux/direct/google-chrome-stable_current_amd64.deb && \\ apt-get install --yes ./google-chrome-stable_current_amd64.deb && \\ rm google-chrome-stable_current_amd64.deb ``` Verified in a running dogfood workspace: ``` $ google-chrome --version Google Chrome 146.0.7680.75 ``` |
||
|
Kyle Carberry
|
c5b8611c5a |
feat(gitsync): enrich PR status with author, base branch, review info (#23038)
## Summary Adds 7 new fields to the PR status stored by gitsync, all sourced from the existing GitHub API calls (**zero additional HTTP requests**): | Field | Source | Purpose | |---|---|---| | `author_login` | `pull.user.login` | PR author username | | `author_avatar_url` | `pull.user.avatar_url` | PR author avatar for UI | | `base_branch` | `pull.base.ref` | Target branch (e.g. `main`) | | `pr_number` | `pull.number` | Explicit PR number | | `commits` | `pull.commits` | Number of commits in PR | | `approved` | Derived from reviews | True when ≥1 approved, no outstanding changes requested | | `reviewer_count` | Derived from reviews | Distinct reviewers with a decisive state | ## Changes - **`gitprovider/gitprovider.go`**: Added 7 fields to `PRStatus` struct. - **`gitprovider/github.go`**: Expanded the anonymous struct in `FetchPullRequestStatus` to decode new JSON fields. Replaced `hasOutstandingChangesRequested()` with `summarizeReviews()` returning a `reviewStats` struct with `changesRequested`, `approved`, and `reviewerCount`. - **Migration 000434**: Adds 7 columns to `chat_diff_statuses`. - **`queries/chats.sql`**: Updated `UpsertChatDiffStatus` INSERT/VALUES/ON CONFLICT. - **`gitsync/gitsync.go`**: Maps new `PRStatus` fields into upsert params. - **`gitsync/worker.go`**: Maps new columns in row-to-model converter. - **`codersdk/chats.go`**: Added fields to SDK `ChatDiffStatus` type. - **`coderd/chats.go`**: Maps new DB fields in `convertChatDiffStatus()`. - Auto-generated: `models.go`, `queries.sql.go`, `dump.sql`, `typesGenerated.ts`. |
||
|
Kyle Carberry
|
1152b61ebb |
fix(site): fix speech recognition race condition and silent errors (#23043)
## Problem
Clicking the microphone button on `/agents` briefly activates recording
then immediately stops, focusing the text input.
## Root causes
**1. Race condition in `start()`** — When aborting a previous
recognition instance, the old instance's `onend` fires
**asynchronously** in real browsers (unlike our mock which fires it
synchronously). The stale `onend` callback then sets `isRecording=false`
and nullifies `recognitionRef.current`, killing the new recording
session. The unit tests didn't catch this because the mock fires `onend`
synchronously inside `abort()`.
**2. Silent `onerror` handler** — The `onerror` callback completely
discarded the error event. If the browser denied mic permission or the
speech service was unreachable (Chrome sends audio to Google servers),
recording silently died with no feedback.
**3. No cleanup on unmount** — The hook leaked a running recognition
instance if the component unmounted while recording.
## Fixes
- Guard `onend`/`onerror` callbacks with `recognitionRef.current !==
recognition` so stale instances are ignored
- Expose `error: string | null` state from the hook; surface it in the
UI ("Mic access denied" / "Voice input failed")
- Add a cleanup `useEffect` that aborts recognition on unmount
- Added 4 new tests covering the race condition, error exposure, and
error clearing
|
||
|
Kyle Carberry
|
690e3a87d8 |
feat: move chat messages to dedicated /chats/{id}/messages endpoint (#23021)
## 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 |
|
||
|
Kyle Carberry
|
0e7e0a959e |
feat(site): add voice-to-text input to agent chat (#23022)
## Summary Adds a microphone button to the agent chat input for browser-native voice-to-text transcription using the Web Speech API. ## Changes ### New: `site/src/hooks/useSpeechRecognition.ts` - Custom React hook wrapping the Web Speech API (`SpeechRecognition` / `webkitSpeechRecognition`) - Feature-detects browser support via `isSupported` - Provides `start()`, `stop()`, and `cancel()` controls - Accumulates real-time transcript from interim and final recognition results - Inline TypeScript declarations for the Web Speech API types ### Modified: `site/src/pages/AgentsPage/AgentChatInput.tsx` - **Mic button**: Appears to the right of the image attach button when the browser supports the Speech Recognition API. Shows a microphone icon when idle, X icon when recording. - **Send button**: Transforms into a checkmark during recording to accept the transcription. Always enabled during recording. - **Editor sync**: Live-updates the Lexical editor with the transcription as the user speaks. Preserves any pre-existing text. - **Cancel**: Restores the editor to its pre-recording content. ## How it works 1. User clicks the mic button → recording starts, real-time transcript appears in the editor 2. User clicks the checkmark (send button) → recording stops, transcribed text stays 3. User clicks X (mic button) → recording stops, editor reverts to original content |
||
|
Kyle Carberry
|
84dc1a3482 | fix: hide web_search tool — preserve ProviderExecuted on DB-loaded tool results (#23014) | ||
|
Kyle Carberry
|
0e1846fe2a | fix(agent): reap exited processes and scope process list by chat ID (#22944) | ||
|
Kyle Carberry
|
42c12176a0 |
fix(chatd): persist interrupted tool call steps instead of losing them (#23011)
## 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. |
||
|
Kyle Carberry
|
072e9a212f |
fix(chatloop): keep provider-executed tool results in assistant message (#23012)
## 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. |
||
|
Kyle Carberry
|
3407fa80a4 |
fix: only show subagent chevron on icon hover, not full row hover (#23010)
## Summary When hovering on an agent row that is **not running**, the expand/collapse chevron (`>`) for subagents was appearing by replacing the status icon. This was visually distracting when scanning the sidebar. ## Change The chevron now only appears when hovering directly over the **icon area** (`group-hover/icon`), not the entire row (`group-hover`). This was already the behavior for running agents — this PR makes it consistent for all states. - Unified both the status-icon-hide and chevron-show hover triggers to use `group-hover/icon` - Removed the now-unused `isExecuting` variable (net -15 lines) |
||
|
Kyle Carberry
|
1ac5418fc4 |
fix(agent-chat): use direct concatenation for completed message text blocks (#23009)
## Problem
LLMs stream list markers and item content as separate text blocks:
```json
{ "type": "text", "text": "Intro\n\n- " }
{ "type": "text", "text": "First item" }
{ "type": "text", "text": "\n- " }
{ "type": "text", "text": "Second item" }
```
The **streaming path** concatenated these directly → `"- First item"` ✅
The **completed path** used `appendText` which inserted `\n` between
chunks → `"- \nFirst item"` ❌
Every CommonMark parser treats `"- \nText"` (marker and content on
different lines, content not indented) as an empty `<li>` followed by a
sibling `<p>`, producing broken list rendering once a message finished
streaming.
## Fix
Make `appendText` use direct concatenation — the same as the streaming
path. The API text blocks already contain all necessary whitespace and
newlines; inserting extra `\n` between them was the bug.
## Changes
- **`messageParsing.ts`** — `appendText` simplified to direct concat
(skip whitespace-only chunks). `appendParsedTextBlock` no longer passes
a custom joiner, so it uses the same default as the streaming path.
- **`messageParsing.test.ts`** — Updated existing merge test
expectation; added regression test with the exact LLM list-marker
payload.
|
||
|
Kyle Carberry
|
b1e80e6f3a |
fix(gitsync): concurrent refresh, decoupled timeout, and no-token backoff (#23004)
## 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`. |
||
|
Kyle Carberry
|
fc9e04da67 |
fix(chatd): handle soft-deleted workspaces in chattool start/create (#22997)
## 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 |
||
|
Kyle Carberry
|
a6697b1b29 |
fix(chatd): fix PE tool result persistence via fantasy bump (#22996)
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 |
||
|
Kyle Carberry
|
c3923f2ccd |
fix(chatd): keep provider-executed tool results in assistant content (#22991)
## 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). |
||
|
Kyle Carberry
|
53bfbf7c03 |
fix(chatd): improve compaction prompt to preserve forward momentum (#22989)
## 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. |
||
|
Kyle Carberry
|
3325b86903 | fix(chatd): skip provider-executed tools in message repair (#22976) | ||
|
Kyle Carberry
|
d495a4eddb |
fix(site): deduplicate agent chime across browser tabs (#22972)
## Problem When multiple tabs are open on `/agents`, every tab receives the same WebSocket status transitions and independently plays the completion chime — resulting in overlapping sounds. ## Fix Use the [Web Locks API](https://developer.mozilla.org/en-US/docs/Web/API/Web_Locks_API) (`navigator.locks`) to coordinate across tabs. When a tab decides a chime should play: 1. It calls `navigator.locks.request(lockName, { ifAvailable: true }, callback)`. 2. Only the first tab to acquire the per-chatID lock plays the sound. 3. The lock is held for 2 seconds, covering any reasonable WebSocket delivery skew between tabs. 4. Other tabs get `lock === null` and silently skip. Falls back to immediate playback (original behavior) when the Web Locks API is unavailable. |
||
|
Kyle Carberry
|
58f295059c |
fix: grant chatd ActionReadPersonal on User and parallelize runChat DB calls (#22970)
## 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
|
||
|
Kyle Carberry
|
4d7eb2ae4b |
feat(agents): replace Show More with infinite scroll and add archived filter dropdown (#22960)
## Summary Replace the janky "Show more" button in the agents sidebar with IntersectionObserver-based infinite scroll. Add a filter dropdown near the top of the sidebar to switch between **Active** (default) and **Archived** views. The old collapsible "Archived" section at the bottom of the sidebar is removed in favor of server-side filtering via the query parameter. ## Changes ### API layer - `api.ts`: Accept `archived` param in `getChats()` - `chats.ts`: Accept `archived` option in `infiniteChats()`, pass it through to API ### Agents page - `AgentsPage.tsx`: Add `archivedFilter` state, pass `archived` to query, forward `isFetchingNextPage` - `AgentsPageView.tsx`: Pass new filter and pagination props through to sidebar ### Sidebar - `AgentsSidebar.tsx`: - Add `LoadMoreSentinel` component using `IntersectionObserver` for auto-loading - Add filter dropdown with Active/Archived options (with checkmarks) - Remove `Collapsible` archived section and related state - All visible chats now come from the server-side filtered query ### Stories - Updated stories with new required props (`archivedFilter`, etc.) - Replaced old archived collapsible stories with filter-based equivalents |
||
|
Kyle Carberry
|
57dc23f603 |
feat(chatd): add provider-native web search tools to chats (#22909)
## 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
|
||
|
Kyle Carberry
|
1f37df4db3 |
perf(chatd): fix six scale bottlenecks identified by benchmarking (#22957)
## 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) |
||
|
Kyle Carberry
|
77d53d2955 |
fix(coderd/gitsync): consolidate chat diff refresh paths through Worker.RefreshChat (#22938)
## 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
|
||
|
Kyle Carberry
|
d39f69f4c2 |
fix: avoid mutating proto App.Healthcheck in insertAgentApp (#22954)
## 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.
|
||
|
Kyle Carberry
|
c33dc3e459 |
fix(site): restore Add model button and fix header in Models/Providers sections (#22953)
## Problem The refactor in #22914 moved the `SectionHeader` rendering into `ConfigureAgentsDialog`, but `ModelsSection` and `ProvidersSection` only render their action buttons (including the "Add model" dropdown) inside their own `SectionHeader`, which is gated on the `sectionLabel` prop. Since the dialog stopped passing `sectionLabel`, the Add button disappeared entirely — there was no way to add a model. Additionally, when clicking a model to edit, the `ModelForm` was supposed to take over the full panel (the section early-returns the form without any header), but the outer `SectionHeader` from the dialog remained visible above it. ## Fix Remove the duplicate `SectionHeader` from `ConfigureAgentsDialog` for both the Providers and Models sections. Instead, pass `sectionLabel`, `sectionDescription`, and `sectionBadge` through `ChatModelAdminPanel` to the inner `ProvidersSection`/`ModelsSection` components, which render their own headers with the appropriate action buttons. This restores: 1. The "Add" model dropdown button in the top-right of the Models section 2. Proper header hiding when clicking into a model edit form 3. The AdminBadge and rich description text on each section header |
||
|
Kyle Carberry
|
7a83d825cf |
feat(agents): add PR title, draft, and status icons to sidebar (#22952)
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. |
||
|
Kyle Carberry
|
40114b8eea |
fix(site): remove custom li override to fix loose list paragraph nesting (#22951)
## Problem When streaming completes in the agent chat, `<p>` elements inside list items visually break out of the `<ul>`, rendering as `<ul> → <li>` then `<p>` after `</ul>` instead of staying nested as `<ul> → <li> → <p>`. ## Root Cause The `Response` component overrides streamdown's default `li` component to handle GFM task-list items (suppressing bullets when a checkbox is present). However, this override drops streamdown's built-in `[&>p]:inline` CSS class from `MarkdownLi`. When the final markdown from the LLM contains blank lines between list items, `remark-parse` treats it as a **loose list** per the CommonMark spec and wraps each item's content in `<p>` tags. Without `[&>p]:inline`, those `<p>` tags render as block elements with default margins, visually pushing content outside the list. During streaming this is less noticeable because `remend` preprocesses incomplete markdown and the list items tend to arrive without blank-line separators (tight list → no `<p>` wrapping). ## Fix Remove the custom `li` override entirely. Streamdown's built-in `MarkdownLi` already handles both: - Task-list bullet suppression - Paragraph nesting via `[&>p]:inline` The custom `input` override for styled checkboxes is unaffected since it's a separate component. |
||
|
Kyle Carberry
|
ac791e5bd3 |
fix(site): match Local tab scroll layout with Remote tab in Git panel (#22949)
The Local tab in the Git panel wrapped all repo sections in a single `ScrollArea`, which caused the file tree sidebar to scroll away with the diff content instead of staying pinned. The Remote tab (`FilesChangedPanel`) already uses the correct pattern where each `DiffViewer` manages its own independent `ScrollArea` for the file tree and diff list side-by-side. ## Changes - Replace the outer `ScrollArea` in `LocalContent` with a flex column container that gives each repo section a constrained height via `min-h-0` and `flex-1`, allowing `DiffViewer`'s internal `ScrollArea` components to activate properly - Add `shrink-0` to `RepoHeader` so it stays pinned at the top of each repo section - Remove unused `ScrollArea` import ## Root cause `LocalContent` wrapped everything in `<ScrollArea className="h-full">`, creating a single scrollable container. Inside, each `RepoChangesPanel` → `DiffViewer` has `h-full` but since it was inside an already-scrolling container, it never got a constrained height — so the inner `ScrollArea` components for the file tree and diff list never activated. Everything flowed in the outer scroll, making the file tree scroll away with the content. |
||
|
Kyle Carberry
|
196c6702fd |
feat(coderd): add q search parameter to chats endpoint (#22913)
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 |
||
|
Kyle Carberry
|
bb59477648 |
feat(db): add created_by column to chat_messages table (#22940)
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`. |
||
|
Kyle Carberry
|
71b132b9e7 |
fix(cli/sessionstore): don't run Windows keyring tests in parallel (#22937)
Removes `t.Parallel()` from `TestKeyring` and `TestWindowsKeyring_WriteReadDelete`. The OS keyring is a shared system resource that's flaky under concurrent access, especially Windows Credential Manager in CI. Fixes coder/internal#1370 |
||
|
Kyle Carberry
|
c72d3e4919 |
fix(site): include diff length in cache key to prevent stale highlight reuse (#22942)
When a PR diff update arrives via SSE, the diff content query is
invalidated and re-fetched. `parsePatchFiles` was called with the same
cache key prefix (`chat-{chatId}`) regardless of content, so the
`@pierre/diffs` worker pool's LRU cache returned the old highlighted
AST. The stale `code.additionLines`/`code.deletionLines` arrays no
longer matched the new diff's line structure, causing
`DiffHunksRenderer.processDiffResult` to throw:
```
DiffHunksRenderer.processDiffResult: deletionLine and additionLine are null, something is wrong
```
**Root cause:** The rendering pipeline has two phases that both call
`iterateOverDiff` but with different `diffStyle` parameters. Phase 1
(highlighting) uses `diffStyle: "both"` to populate
`code.deletionLines[]` and `code.additionLines[]`. Phase 2 (DOM
construction in `processDiffResult`) uses `diffStyle: "unified"` or
`"split"` to consume those arrays. When the cache returned stale phase 1
output for new diff content, the line indices from phase 2 pointed to
entries that didn't exist in the stale arrays.
**Fix:** Append `diff.length` to the cache key prefix so that content
changes produce a cache miss and trigger fresh highlighting. While not
collision-proof, it's vanishingly unlikely that two sequential PR diff
updates have the exact same byte length.
|
||
|
Kyle Carberry
|
f766ad064d |
fix(site): improve markdown rendering and top bar styling in agent chat (#22939)
## Changes ### Markdown rendering (response.tsx) - **Headings (h1-h6)**: Apple-like font scale from 13px base, with proper weight and spacing. - **Task-list checkboxes**: Replace disabled `<input>` with styled `<span>` (checked = filled blue + checkmark SVG, unchecked = bordered empty box). - **Table cells (th/td)**: Inherit 13px base font instead of streamdown's hardcoded `text-sm` (14px). - **Horizontal rules**: Explicit border styling to fix browser default inset/ridge when Tailwind preflight is off. - **List items**: Detect `task-list-item` class and remove default list marker. ### Top bar (TopBar.tsx) - Increased vertical padding (`py-0.5` -> `py-1.5`). - Parent chat button text size: `text-xs` -> `text-sm` to match active chat title. - ChevronRight icon: added `-ml-0.5` for even spacing around separator. - Removed redundant "Archived" badge (archived banner already shows below the top bar). ### Stories - Rewrote `WithMessageHistory` story with rich markdown covering headings, task lists, tables, code blocks, and horizontal rules. |
||
|
Kyle Carberry
|
0a026fde39 |
refactor: remove reasoning title extraction from chat pipeline (#22926)
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** |
||
|
Kyle Carberry
|
e7f8dfbe15 |
feat(agents): unify settings dialog for users and admins (#22914)
## Summary Refactors the admin-only "Configure Agents" dialog into a unified **Settings** dialog accessible to all users via a gear icon in the sidebar. ### What changed - **Settings gear in sidebar**: A gear icon now appears in the bottom-left of the sidebar (next to the user avatar dropdown). Clicking it opens the Settings dialog. This replaces the admin-only "Admin" button that was in the top toolbar. - **Custom Prompt tab** (all users): A new "Custom Prompt" tab is always visible in the dialog. Users can write personal instructions that are applied to all their new chats (stored per-user via the `/api/experimental/chats/config/user-prompt` endpoint). - **Admin tabs remain gated**: The Providers, Models, and Behavior (system prompt) tabs only appear for admin users, preserving the existing RBAC model. - **API + query hooks**: Added `getUserChatCustomPrompt` / `updateUserChatCustomPrompt` methods to the TypeScript API client and corresponding React Query hooks. ### Files changed | File | Change | |------|--------| | `site/src/api/api.ts` | Added GET/PUT methods for user custom prompt | | `site/src/api/queries/chats.ts` | Added query/mutation hooks for user custom prompt | | `site/src/pages/AgentsPage/ConfigureAgentsDialog.tsx` | Added "Custom Prompt" tab, renamed to "Settings" | | `site/src/pages/AgentsPage/AgentsSidebar.tsx` | Added settings gear button next to user dropdown | | `site/src/pages/AgentsPage/AgentsPageView.tsx` | Removed "Admin" button, pass `onOpenSettings` to sidebar | | `site/src/pages/AgentsPage/AgentsPage.tsx` | Wired up user prompt state, removed admin-only guard on dialog | | `*.stories.tsx` | Updated to match new prop interfaces | |
||
|
Kyle Carberry
|
53e52aef78 |
fix(externalauth): prevent race condition in token refresh with optimistic locking (#22904)
## 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. |
||
|
Kyle Carberry
|
983f362dff |
fix(chatd): harden title generation prompt to prevent conversational responses (#22912)
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 |
||
|
Kyle Carberry
|
b74d60e88c |
fix(site): correct stale queued messages when switching back to a chat (#22911)
## Problem When a user navigates away from a chat and its queued messages are processed server-side, switching back shows stale queued messages until a hard page refresh. The issue is purely frontend state — the backend is correct. ### Root cause Three things conspire to cause the bug: 1. **Stale React Query cache** — the `chatKey(chatId)` cache entry retains the old `queued_messages` from the last fetch. When the user is on a different chat, no refetch or WebSocket updates the cache for the inactive chat. 2. **One-shot hydration guard** — `queuedMessagesHydratedChatIDRef` blocks all REST-sourced re-hydration after the first hydration for a given chat ID. This was designed to prevent a stale REST refetch from overwriting a fresher `queue_update` from the WebSocket, but it also blocks the corrected data that arrives when the query actually refetches from the server. 3. **No unsolicited `queue_update`** — the WebSocket only sends `queue_update` events when the queue changes. If the queue was already drained before the WebSocket connected, no event is ever sent, so the stale data persists. ## Fix Add a `wsQueueUpdateReceivedRef` flag that tracks whether the WebSocket has delivered a `queue_update` for the current chat. The hydration guard now only blocks REST re-hydration **after** a `queue_update` has been received (since the stream is authoritative at that point). Before any `queue_update` arrives, REST refetches are allowed through to correct stale cached data. The flag is reset on chat switch alongside the existing hydration guard reset. ## Changes - **`ChatContext.ts`**: Add `wsQueueUpdateReceivedRef`, update hydration guard condition, set flag on `queue_update` events, reset on chat switch. - **`ChatContext.test.tsx`**: Add test covering the exact scenario — stale cached queued messages are corrected by a REST refetch when no `queue_update` has arrived. |
||
|
Kyle Carberry
|
d3986b53b9 |
perf(ci): use fast zstd compression for non-release CI builds (#22907)
## Problem The `build` job on `main` takes ~7m28s for the Build step alone (~13m total). Analysis of 10 recent CI runs on `main` shows the zstd compression of the slim binary archive is the second largest bottleneck: | Phase | Avg Duration | % of Build Step | |-------|-------------|----------------| | Fat Go builds (7 binaries w/ embed) | ~205s | 45.8% | | **zstd compression (`-22 --ultra`)** | **~123s** | **27.4%** | | Parallel block (vite + slim Go builds) | ~65s | 14.5% | | Packaging + signing | ~55s | 12.3% | The `zstd -22 --ultra` setting compresses a ~350 MB tar to ~71 MB, but it is **single-threaded** and takes ~102s on 8-core CI runners. Adding `-T8` does not help at level 22 — it remains CPU-bound on a single thread. ## Solution Use `zstd -6 -T0` (multithreaded, auto-detect cores) for non-release CI builds. Release builds (`CODER_RELEASE=true`) continue using `-22 --ultra`. ### Benchmarks (349 MB slim binary tar, 8 cores) | Setting | Wall Time | Output Size | Use Case | |---------|----------|------------|----------| | `-22 --ultra` | **102.4s** | 71 MB | Release builds | | `-6 -T0` | **0.8s** | 94 MB | CI builds (new) | | `-6` | 2.4s | 94 MB | Local dev (unchanged) | The 23 MB size increase is negligible for the main branch preview images (`ghcr.io/coder/coder-preview:main`). The archive is embedded in fat binaries and extracted once by the agent at startup — decompression time is identical regardless of compression ratio. ### Expected impact ~120s savings on the Build step, bringing it from ~7m28s to ~5m30s. ## Verification All three code paths confirmed: - `CODER_RELEASE=true CI=true` → `-22 --ultra` ✅ - `CI=true` (no `CODER_RELEASE`) → `-6 -T0` ✅ - Local (no `CI`) → `-6` ✅ - `CODER_RELEASE=false CI=true` (dry run) → `-6 -T0` ✅ |
||
|
Kyle Carberry
|
8cc6473736 |
fix: increase migration lock timeout to prevent flaky parallel test (#22910)
## 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. |
||
|
Kyle Carberry
|
30a63009aa |
fix(agents): persist right panel open/closed state to localStorage (#22906)
Removes the auto-open/close behavior that would force the right-side panel open whenever diff status or git repository data appeared. Instead, the panel's visibility is now persisted via the `agents.right-panel-open` localStorage key (matching the existing `agents.right-panel-width` pattern for the panel width). This gives users a consistent UX when switching between chats — the panel stays in whatever state they last set it to. ## Changes - **Removed** two auto-open blocks in `AgentDetailView` that tracked `prevHasDiffStatus` / `prevHasGitRepos` and forced `showSidebarPanel = true` - **Added** `localStorage` persistence for the panel open/closed state under key `agents.right-panel-open` - Initial state is read from localStorage on mount (defaults to closed) - Every toggle/close writes through to localStorage via `handleSetShowSidebarPanel` - Panel width was already persisted via `agents.right-panel-width` in `RightPanel.tsx` — no changes needed there |
||
|
Kyle Carberry
|
01f25dd9ae |
fix(agents): write WebSocket cache updates to infinite query key (#22905)
## Problem
Chat sidebar title/status updates from WebSocket events don't take
effect immediately — they only appear after a full server re-fetch.
**Root cause:** All `setQueryData(chatsKey, ...)` calls write to cache
key `["chats"]`, but the rendered chat list reads from
`useInfiniteQuery(infiniteChats())` on key `["chats", undefined]`.
TanStack Query v5 `setQueryData` requires an exact key match, so these
are different cache entries.
WebSocket events (`title_change`, `status_change`, `created`, `deleted`)
and `updateSidebarChat` were all updating a cache entry that nothing
rendered from. The only way changes reached the UI was via
`invalidateQueries` (which prefix-matches), triggering a full server
re-fetch. This caused visible flicker when the re-fetch raced with
subsequent events.
## Fix
Add `updateInfiniteChatsCache()` helper that uses `setQueriesData({
queryKey: chatsKey })` — this **prefix-matches** all infinite query
variants (`["chats", undefined]`, `["chats", { archived: true }]`, etc.)
and correctly updates the `{ pages, pageParams }` structure.
Replace all direct `setQueryData(chatsKey, ...)` calls:
- WebSocket handler in `AgentsPage.tsx` (deleted, created, title_change,
status_change events)
- `updateSidebarChat` in `ChatContext.ts`
- Archive/unarchive optimistic updates in `chats.ts`
Also adds `readInfiniteChatsCache()` helper for reading the flat chat
list from the infinite query (used by the chime status lookup).
## Files changed
| File | Change |
|------|--------|
| `site/src/api/queries/chats.ts` | Added helpers, updated
archive/unarchive mutations |
| `site/src/pages/AgentsPage/AgentsPage.tsx` | WebSocket handler uses
new helpers |
| `site/src/pages/AgentsPage/AgentDetail/ChatContext.ts` |
`updateSidebarChat` uses new helper |
| `site/src/api/queries/chats.test.ts` | Tests seed/read infinite query
format |
| `site/src/pages/AgentsPage/AgentDetail/ChatContext.test.tsx` | Tests
seed/read infinite query format |
|
||
|
Kyle Carberry
|
b6d1a11c58 |
feat(chatd): add user-level custom prompt for agent chats (#22896)
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. |
||
|
Kyle Carberry
|
fee5cc5e5b |
fix(chatd): fix flaky TestCloseDuringShutdownContextCanceledShouldRetryOnNewReplica (#22893)
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/ ``` |
||
|
Kyle Carberry
|
ba764a24ea |
fix(site): upgrade @pierre/diffs to 1.1.0-beta.19 (#22895)
Fixes a race condition in `DiffHunksRenderer` where a stale async highlight callback overwrites the render cache with an old diff, causing a hunk count mismatch: ``` DiffHunksRenderer.renderHunks: lineHunk doesn't exist ``` ## Root cause The `DiffHunksRenderer` in `@pierre/diffs@1.0.11` caches highlighted AST results keyed by diff object reference. When the shiki highlighter isn't fully loaded, it fires `asyncHighlight(diff)` which captures the current diff in a closure. If the diff changes before that promise resolves, `onHighlightSuccess` unconditionally overwrites `renderCache` with the stale diff/result pair. The subsequent `rerender()` then iterates the new diff's hunks against the old result's `code.hunks` array, crashing at an out-of-bounds index. ## Fix Upgrades `@pierre/diffs` from `1.0.11` to `1.1.0-beta.19`, which completely refactors the rendering pipeline: - Replaces the per-hunk `code.hunks[hunkIndex]` lookup with flat `additionLines`/`deletionLines` arrays indexed directly by line index - Uses a new `iterateOverDiff` callback pattern instead of the `renderHunks` method - The `lineHunk doesn't exist` error is gone from the codebase entirely The only code change on our side is adapting `extractDiffContent()` in `FilesChangedPanel.tsx` to the new `ChangeContent`/`ContextContent` types where `deletions`, `additions`, and `lines` are now counts with index pointers into top-level `FileDiffMetadata.deletionLines`/`additionLines` arrays. |
||
|
Kyle Carberry
|
8c70170ee7 |
fix(site): polish agent UI styling (#22889)
Fixes several small UI issues on the agent detail and sidebar pages: - **Sidebar lines changed indicator**: removed monospace font, matched styling to model text (text-[13px] leading-4) - **Git panel**: always shown instead of "No panels available" fallback - **Git tab active state**: added `text-content-primary` so the tab looks selected - **Attachment button**: switched to `subtle` variant (lighter color, no border) - **Context indicator / attachment button**: matched sizes (`size-7` container, `size-icon-sm` icon) and swapped positions |
||
|
Kyle Carberry
|
e18ce505ec |
feat(coderd): add pagination to chat list endpoint (#22887)
Adds offset and cursor-based pagination to the `GET /api/experimental/chats` endpoint, following the exact same patterns used by `GetUsers` and `GetTemplateVersionsByTemplateID`. ## Changes ### Database - Add `after_id`, `offset_opt`, `limit_opt` params to `GetChatsByOwnerID` SQL query - Use composite `(updated_at, id) DESC` cursor for stable, deterministic pagination - Add migration with composite index on `chats (owner_id, updated_at DESC, id DESC)` ### Backend - Use `ParsePagination()` in `listChats` handler (matches `users.go` pattern) - Add `Pagination` field to `ListChatsOptions` SDK struct ### Frontend - Add `infiniteChats()` query factory using `useInfiniteQuery` with offset-based page params (same pattern as `infiniteWorkspaceBuilds`) - Update `AgentsPage` to use `useInfiniteQuery` - Add "Show more" button at the bottom of the agents sidebar (matches `HistorySidebar` pattern) - Keep existing `chats()` query for non-paginated uses (e.g., parent chat lookup in `AgentDetail`) ### Tests - Add `TestListChats/Pagination` covering `limit`, `after_id` cursor, `offset`, and no-limit behavior |
||
|
Kyle Carberry
|
f35b99a4fa |
fix(chatd): preserve context.Canceled in persistStep during shutdown (#22890)
## Problem
When a chat worker shuts down gracefully (e.g. Kubernetes pod SIGTERM)
while a tool is executing (like `wait_agent` polling for a subagent),
the chat gets stuck in `waiting` status forever — no other worker will
pick it up.
### Root Cause
`persistStep` in `chatd.go` unconditionally returned
`chatloop.ErrInterrupted` for **any** canceled context:
```go
if persistCtx.Err() != nil {
return chatloop.ErrInterrupted // BUG: doesn't check WHY the context was canceled
}
```
During shutdown, the context cause is `context.Canceled` (not
`ErrInterrupted`). But because `persistStep` returned `ErrInterrupted`,
the error handling in `processChat` hit the `ErrInterrupted` check first
(line 2011) and set status to `waiting` — the `isShutdownCancellation`
check (line 2017) was never reached:
```go
// Checked FIRST — matches because persistStep returned ErrInterrupted
if errors.Is(err, chatloop.ErrInterrupted) {
status = database.ChatStatusWaiting // Stuck forever
return
}
// NEVER REACHED during shutdown
if isShutdownCancellation(ctx, chatCtx, err) {
status = database.ChatStatusPending // Would have been correct
return
}
```
### Trigger scenario (from production logs)
1. Chat spawns a subagent via `spawn_agent`, then calls `wait_agent`
2. `wait_agent` blocks in `awaitSubagentCompletion` polling loop
3. Worker pod receives SIGTERM → `Close()` cancels server context
4. Context cancellation propagates to `awaitSubagentCompletion` →
returns `context.Canceled`
5. Tool execution completes, `persistStep` is called with canceled
context
6. `persistStep` returns `ErrInterrupted` (wrong!) → status set to
`waiting` (stuck!)
## Fix
Check `context.Cause()` before deciding which error to return:
```go
if persistCtx.Err() != nil {
if errors.Is(context.Cause(persistCtx), chatloop.ErrInterrupted) {
return chatloop.ErrInterrupted // Intentional interruption
}
return persistCtx.Err() // Shutdown → context.Canceled
}
```
This preserves `context.Canceled` for shutdown, allowing
`isShutdownCancellation` to match and set status to `pending` so another
worker retries the chat.
## Test
Added `TestRun_ShutdownDuringToolExecutionReturnsContextCanceled` which:
1. Streams a tool call to a blocking tool (simulating `wait_agent`)
2. Cancels the server context (simulating shutdown) while the tool
blocks
3. Verifies `Run` returns `context.Canceled`, NOT `ErrInterrupted`
|
||
|
Kyle Carberry
|
b898e45ec4 |
feat(site): rewrite localhost URLs in agent chat to port-forward links (#22891)
Uses streamdown's built-in `urlTransform` prop to intercept `http://localhost:PORT` URLs in agent chat messages and rewrite them to port-forwarded workspace URLs. When the agent outputs a bare URL like `http://localhost:3000` or a markdown link like `[app](http://localhost:8080/path)`, the URL is rewritten to the workspace's port-forward subdomain (e.g. `https://3000--agent--workspace--user.wildcard.host`). This makes links clickable directly from the chat without manual port-forwarding. ## How it works The transform is built in `AgentDetail` where workspace and proxy context are available, then threaded as an optional prop through the component tree: ``` AgentDetail → AgentDetailView → AgentDetailTimeline → ConversationTimeline → Response → Streamdown ``` - Uses streamdown's first-class `urlTransform` API — no monkey-patching or rehype plugins - Reuses the existing `portForwardURL()` utility from `utils/portForward` - Matches the same localhost detection as the terminal page (`localhost`, `127.0.0.1`, `0.0.0.0`) - Preserves pathname and search params - Gracefully degrades: when any required context is missing (no workspace, no wildcard proxy host), URLs pass through unchanged ## What gets transformed | Markdown input | Transformed? | |---|---| | `http://localhost:8080` (bare URL, auto-linked by remark-gfm) | Yes | | `[my app](http://localhost:3000/path)` (explicit link) | Yes | | `\`http://localhost:8080\`` (inline code) | No (correct — code spans are literal) | | `https://example.com` (non-localhost) | No | |
||
|
Kyle Carberry
|
47846c0ee4 |
fix(site): inject permissions and organizations metadata to eliminate loading spinners (#22741)
## Problem Two network requests were blocking the initial page render with fullscreen `<Loader fullscreen />` spinners: 1. **`POST /api/v2/authcheck`** (permissions) — blocked in `RequireAuth` via `AuthProvider.isLoading` 2. **`GET /api/v2/organizations`** — blocked in `DashboardProvider` All other bootstrap queries (`user`, `entitlements`, `appearance`, `experiments`, `build-info`, `regions`) already used server-side metadata injection via `index.html` meta tags and resolved instantly. These two did not. ## Solution Follow the existing `cachedQuery` + `<meta>` tag pattern to inject both datasets server-side: ### Server-side (`site/site.go`) - Add `Permissions` and `Organizations` fields to `htmlState` - Fetch organizations via `GetOrganizationsByUserID` in parallel with existing queries - Evaluate all `permissionChecks` using the RBAC authorizer directly - Inject results as HTML-escaped JSON into `<meta>` tags ### Frontend - Register `permissions` and `organizations` in `useEmbeddedMetadata` - Update `checkAuthorization()` to accept optional metadata and use `disabledRefetchOptions` when available - Update `organizations()` to accept optional metadata and use `cachedQuery` when available - Wire metadata through `AuthProvider` and `DashboardProvider` ### Note The Go `permissionChecks` map in `site/site.go` mirrors `site/src/modules/permissions/index.ts` and must be kept in sync. |
||
|
Kyle Carberry
|
3f939375fa |
refactor: unify agent sidebar into generic tabbed panel with Git sub-views (#22837)
## Summary Refactors the right-side panel in the Agents page into a generic tabbed container with a unified Git panel. ### Changes **Architecture** - `SidebarTabView` is now a generic tabbed container with no git-specific logic, ready for additional tabs - All Git content lives in a new `GitPanel` component with an internal Remote/Local segmented control **Git Panel** - Remote view: branch/PR diff via `FilesChangedPanel` - Local view: working tree changes with per-repo headers, commit & refresh actions - Split/unified diff toggle restored in the toolbar - `DiffStatBadge` rendered inside the Remote/Local segmented buttons (full-height, no rounding, inactive opacity 50%) **Visual polish** - Active/inactive/hover states match the sidebar agent selection styles (`bg-surface-quaternary/25`, `hover:bg-surface-tertiary/50`) - Inactive tab text uses `text-content-secondary` (not primary) - Tab button sizing fixed: `min-w-0` + `px-2` to prevent inflated width - Chat title centered via absolute positioning when panel is fullscreen - Polished empty states with boxed icons (`GitCompareArrowsIcon` for Remote, `FileDiffIcon` for Local) - Unified header styles between Remote and Local sections (both use `bg-surface-secondary` with consistent icon/text sizing) - Panel toggle always visible in top bar (not gated on having diff data) **Cleanup** - Removed dead code: `DiffStatsInline`, `computeDiffStats` export, `workingDiffStats` memo, `ChatDiffStatusResponse` import - Simplified `RepoChangesPanel` to a pure `DiffViewer` wrapper - Simplified `TopBar` to use a generic `panel` prop instead of diff-specific props |
||
|
Kyle Carberry
|
aba3832b15 |
fix: update the compaction message to be the "user" role (#22819)
## Bug
After compaction in the chat loop, the loop re-enters and calls the LLM
with a prompt that has **no non-system messages**. Anthropic (and most
providers) require at least one user/assistant/tool message, so the API
errors with empty messages.
## Root Cause
The compaction summary was stored as `role=system`. After compaction,
`GetChatMessagesForPromptByChatID` returns only:
- The compressed system summary (matched by the CTE)
- Original non-compressed system messages (system prompts)
All original user/assistant/tool messages are excluded (they predate the
summary). The compaction assistant/tool messages are `compressed=TRUE`
and don't match the main query's `compressed=FALSE` clauses.
So `ReloadMessages` returned only system messages. The Anthropic
provider moves system messages into a separate `system` field, leaving
the `messages` API field as `[]`.
## Fix
1. **Changed compaction summary from `role=system` to `role=user`** —
the summary now appears as a user message in the reloaded prompt, giving
the model valid conversational context to respond to.
2. **Simplified the CTE** — removed the `role = 'system'` check and
narrowed `visibility IN ('model', 'both')` to just `visibility =
'model'`. The summary is the only compressed message with
`visibility=model` (the assistant has `visibility=user`, the tool has
`visibility=both`), so the role check was redundant.
## Test
`PostRunCompactionReEntryIncludesUserSummary`: verifies the re-entry
prompt contains a user message (the compaction summary) after compaction
+ reload.
|