Commit Graph

3419 Commits

Author SHA1 Message Date
Mathias Fredriksson 0a0c976a1a test(coderd/chatd): add P0 coverage tests for subagent auth and panic recovery (#23309)
The processChat defer at line 2464 catches panics on its main
goroutine and transitions the chat to error status. This was
previously untested.

The test wraps the database Store to panic during PersistStep's
InTx call, which runs synchronously on the processChat goroutine.
A tool-level panic wouldn't work because executeTools has its own
recover that converts panics into tool error results.
2026-03-19 17:54:03 +00:00
Steven Masley 91d7516dc1 test: remove classic params from ephemeral params test (#23302)
Dynamic parameters supports ephemeral parameters. Updated the test to
use dynamic parameters.

Ephemeral params **require** a default value.
Closes https://github.com/coder/coder/issues/19065
2026-03-19 11:32:36 -05:00
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.
2026-03-19 12:10:54 -04:00
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.
2026-03-19 15:36:29 +00:00
Michael Suchacz 6d214644f6 fix: make TestInterruptAutoPromotionIgnoresLaterUsageLimitIncrease deterministic (#23279)
Eliminates the timing flake in
`TestInterruptAutoPromotionIgnoresLaterUsageLimitIncrease` by making the
chatd worker loop clock-controllable.

## Changes

**`coderd/chatd/chatd.go`**
- Replace `time.NewTicker` calls in `Server.start()` with
`p.clock.NewTicker` using named quartz tags `("chatd", "acquire")` and
`("chatd", "stale-recovery")`.

**`coderd/chatd/chatd_test.go`**
- Inject `quartz.NewMock(t)` into the test via `newActiveTestServer`
config override.
- Trap the acquire ticker so the test controls exactly when pending
chats are reacquired.
- Rewrite the test flow as explicit clock-advance steps instead of
wall-clock polling.

**`AGENTS.md`**
- Document the PR title scope rule (scope must be a real path containing
all changed files).

## Validation
- `go test ./coderd/chatd -run
TestInterruptAutoPromotionIgnoresLaterUsageLimitIncrease -count=100` 
- `go test ./coderd/chatd` 
- `make lint` 
2026-03-19 15:14:00 +00:00
Thomas Kosiewski 83809bb380 feat: add token-to-cookie endpoint for embedded chat WebSocket auth (#23280)
## Problem

The VS Code extension embeds the Coder agent chat UI in an iframe,
passing the session token via `postMessage`. HTTP requests use the
`Coder-Session-Token` header, but browser WebSocket connections **cannot
carry custom headers** — they rely on cookies. This causes all WebSocket
requests (e.g. streaming chat messages) to fail with authorization
errors in the embedded iframe.

## Solution

Add `POST /api/v2/users/me/session/token-to-cookie` — a lightweight
endpoint that converts the current (already-validated) session token
into a `Set-Cookie` response. The frontend embed bootstrap flow calls
this immediately after `API.setSessionToken(token)`, before any
WebSocket connections are opened.

### Backend (`coderd/userauth.go`, `coderd/coderd.go`)
- New handler `postSessionTokenCookie` behind `apiKeyMiddleware`.
- Reads the validated token via `httpmw.APITokenFromRequest(r)`.
- Sets an `HttpOnly` cookie with the API key's expiry, applying
site-wide cookie config (Secure, SameSite, host prefix) via
`HTTPCookies.Apply`.
- Returns `204 No Content`.

### Frontend (`site/src/pages/AgentsPage/EmbedContext.tsx`)
- `bootstrapChatEmbedSessionFn` now calls the new endpoint after setting
the header token and before fetching user/permissions.
- The cookie is in place before any WebSocket connections are opened.

## Security

- **No privilege escalation**: The token is already valid — this just
moves it from a header credential to a cookie credential.
- **POST only**: Avoids CSRF-via-navigation.
- **Same origin**: The iframe loads from the Coder server, so the cookie
applies to the correct domain.
- **HttpOnly**: The cookie is not accessible to JavaScript.

> Built with [Coder Agents](https://coder.com/agents) 🤖
2026-03-19 16:12:31 +01:00
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/`
2026-03-19 14:07:36 +00:00
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.
2026-03-19 12:59:39 +00:00
Ehab Younes 7a98b4a876 fix(coderd): gate OAuth2 well-known endpoints behind experiment flag (#23278)
- Add `RequireExperimentWithDevBypass` middleware to
`/.well-known/oauth-authorization-server` and
`/.well-known/oauth-protected-resource` routes, matching the existing
`/oauth2` routes.
- Clients can now detect OAuth2 support via unauthenticated discovery
(404 = not available).

Fixes #21608
2026-03-19 14:42:04 +03:00
Ethan cda460f5df perf(coderd/chatd): skip same-replica stream DB rereads (#23218)
## Problem

Scaletest follow-up storms showed that the chat stream path was doing a
same-replica DB reread for every durable message it had already
delivered locally.

In a 600-chat / 10-turn run, `/stream`-attributed
`GetChatMessagesByChatID` calls reached about 14.2k across 5,400
follow-up turns — roughly **2.63 rereads per turn**. The primary coderd
replicas saturated their DB pools at 60/60 open connections during the
storm window.

The root cause: when pubsub was active, `Subscribe()` suppressed local
durable `message` events and relied entirely on pubsub notify →
`GetChatMessagesByChatID` for catch-up. Same-replica subscribers paid
the full DB round-trip even though the persisting process was on the
same replica.

## Solution

Add a bounded per-chat **durable message cache** to `chatStreamState` so
that same-replica subscribers can catch up from memory instead of the
database.

### How it works

1. `publishMessage()` caches the SDK event in `chatStreamState` before
local fanout and pubsub notify.
2. `publishEditedMessage()` replaces the cache with only the edited
message, then publishes `FullRefresh`.
3. `Subscribe()` handles ordinary `AfterMessageID` notifies by first
consulting the per-chat durable cache and only falling back to
`GetChatMessagesByChatID` on cache miss.
4. `FullRefresh` always forces a DB reread (cache is bypassed).

### Safety properties

- If the cache misses (e.g. message expired or remote replica), the DB
catch-up still runs — no silent message loss.
- `FullRefresh` (edits) always rereads from the database.
- Remote replicas still use the pubsub + DB path unchanged.
- The cache is bounded (`maxDurableMessageCacheSize = 256`) and scoped
per chat — no unbounded memory growth.

## Impact

This change removes the entire same-replica portion of the stream
rereads. Based on the 600-chat follow-up run, the upper bound on saved
work is the same-replica share of about 14.2k `GetChatMessagesByChatID`
rereads, with the observed total stream reread rate at about 2.63
rereads per follow-up turn.
2026-03-19 14:02:00 +11:00
Cian Johnston be1c06dec9 feat: add endpoint and CLI for users to view their own OIDC claims (#23053)
- Adds a new API endpoint `GET /api/v2/users/oidc-claims` that returns
only the **merged claims** (not the separate id_token/userinfo
breakdown). Scoped exclusively to the authenticated user's own identity
— no user parameter, so users cannot view each other's claims.
- Adds a new CLI command:** `coder users oidc-claims` that hits the
above endpoint.
- The existing owner-only debug endpoint is preserved unchanged for
admins who need the full claim breakdown.


> 🤖 This PR was created with the help of Coder Agents, and will be
reviewed by my human. 🧑‍💻
2026-03-18 22:10:04 +00:00
Hugo Dutka d285a3e74e fix: handle null bytes in chat messages (#22946)
This PR fixes a bug where if a tool result contained binary data it
wouldn't be persisted to the database.

`jsonb` in Postgres is unable to store null bytes which are sometimes
output by tool results. This change makes it so that we encode them with
a special escape sequence before saving them to the database, and decode
them on read.

<img width="808" height="637" alt="Screenshot 2026-03-11 at 13 14 06"
src="https://github.com/user-attachments/assets/9be353eb-ff26-40ec-9f0a-195022b11f43"
/>
2026-03-18 21:19:25 +01:00
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
2026-03-18 15:50:50 -04:00
Cian Johnston 14ed3e3644 feat: bump workspace last_used_at on chat heartbeat (#23205)
- coderd: Wires `options.WorkspaceUsageTracker` into the chatd config.
- chatd: Adds `UsageTracker` and calls `UsageTracker.Add(workspaceID)`
on each heartbeat tick
- chatd: adds tests to verify `last_used_at` bump behaviour

> 🤖 This PR was created with the help of Coder Agents, and will be
reviewed by my human. 🧑‍💻
2026-03-18 19:07:21 +00:00
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
2026-03-18 14:37:09 -04:00
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.
2026-03-18 14:11:30 -04:00
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
2026-03-18 18:04:42 +00:00
Cian Johnston 0b13ba978a fix: rename chat logger from coderd.chats.chat-processor to coderd.chatd.processor (#23246)
- Rename logger `coderd.chats` to `coderd.chatd` in `coderd.go`
- Rename sub-logger `chat-processor` to `processor` in `chatd/chatd.go`
2026-03-18 17:48:47 +00:00
Kyle Carberry 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>
2026-03-18 17:06:44 +00:00
Steven Masley c46136ff73 chore: update coder/trivy override (#23230)
Coder/preview does this update as well. Because it is a `replace`, we
have to manually update our `replace` too
2026-03-18 12:03:56 -05:00
Cian Johnston 65b7658568 chore: extract testutil.FakeSink for slog test assertions (#23208)
Follow-up to [review comment on
#23025](https://github.com/coder/coder/pull/23025#discussion_r2930309487)
from @mafredri.

Extracts the repeated `logSink` / `fakeSink` test pattern into a shared
`testutil.FakeSink` and migrates all existing call sites.

> 🤖 This PR was created with the help of Coder Agents, and will be
reviewed by my human. 🧑‍💻

---------

Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
2026-03-18 17:02:38 +00:00
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.
2026-03-18 16:27:07 +00:00
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`
2026-03-18 15:29:29 +00:00
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;
```
2026-03-18 10:57:35 -04:00
Kacper Sawicki 1e07ec49a6 feat: add merge_strategy support for coder_env resources (#23107)
## Description

Implements the server-side merge logic for the `merge_strategy`
attribute added to `coder_env` in [terraform-provider-coder
v2.15.0](https://github.com/coder/terraform-provider-coder/pull/489).
This allows template authors to control how duplicate environment
variable names are combined across multiple `coder_env` resources.

Relates to https://github.com/coder/coder/issues/21885

## Supported strategies

| Strategy | Behavior |
|----------|----------|
| `replace` (default) | Last value wins — backward compatible |
| `append` | Joins values with `:` separator (e.g. PATH additions) |
| `prepend` | Prepends value with `:` separator |
| `error` | Fails the build if the variable is already defined |

## Example

```hcl
resource "coder_env" "path_tools" {
  agent_id       = coder_agent.dev.id
  name           = "PATH"
  value          = "/home/coder/tools/bin"
  merge_strategy = "append"
}
```

## Changes

- **Proto**: Added `merge_strategy` field to `Env` message in
`provisioner.proto`
- **State reader**: Updated `agentEnvAttributes` struct and proto
construction in `resources.go`
- **Merge logic**: Added `mergeExtraEnvs()` function in
`provisionerdserver.go` with strategy-aware merging for both agent envs
and devcontainer subagent envs
- **Tests**: 15 unit tests covering all strategies, edge cases (empty
values, mixed strategies, multiple appends)
- **Dependency**: Bumped `terraform-provider-coder` v2.14.0 → v2.15.0
- **Fixtures**: Updated `duplicate-env-keys` test fixtures and golden
files

## Ordering

When multiple resources `append` or `prepend` to the same key, they are
processed in alphabetical order by Terraform resource address (per the
determinism fix in #22706).
2026-03-18 15:43:28 +01:00
Steven Masley 84de391f26 chore: add tallyman events for ai seat tracking (#22689)
AI seat tracking inserted as heartbeat into usage table.
2026-03-18 09:30:22 -05:00
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.
2026-03-18 14:01:46 +00:00
Ethan fc3508dc60 feat: configure acquire chat batch size (#23196)
## Summary
- add a hidden deployment config option for chat acquire batch size
(`CODER_CHAT_ACQUIRE_BATCH_SIZE` / `chat.acquireBatchSize`)
- thread the configured value into chatd startup while preserving the
existing default of `10`
- clamp the deployment value to the `int32` range before passing it into
chatd
- regenerate the API/docs/types/testdata artifacts for the new config
field

## Why
`chatd` currently acquires pending chats in batches of `10` via a
compile-time default. This change makes that batch size
operator-configurable from deployment config, so we can tune acquisition
behavior without another code change.
2026-03-19 00:54:32 +11:00
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
2026-03-18 11:48:28 +00:00
Atif Ali bd5b62c976 feat: expose MCP tool annotations for tool grouping (#23195)
## Summary
- add shared MCP annotation metadata to toolsdk tools
- emit MCP tool annotations from both coderd and CLI MCP servers
- cover annotation serialization in toolsdk, coderd MCP e2e, and CLI MCP
tests

## Why
- Coder already exposed MCP tools, but it did not populate MCP tool
annotation hints (`readOnlyHint`, `destructiveHint`, `idempotentHint`,
`openWorldHint`).
- Hosts such as Claude Desktop use those hints to classify and group
tools, so without them Coder tools can get lumped together.
- This change adds a shared annotation source in `toolsdk` and has both
MCP servers emit those hints through `mcp.Tool.Annotations`, avoiding
drift between local and remote MCP implementations.

## Testing
- Tested locally on Cladue Desktop and the tools are categorized
correctly.

<table>
<tr>
 <td> Before
 <td> After
<tr>
<td> <img width="613" height="183" alt="image"
src="https://github.com/user-attachments/assets/29d2e3fb-53bc-4ea7-bdb3-f10df4ef996b"
/>
<td> <img width="600" height="457" alt="image"
src="https://github.com/user-attachments/assets/cc384036-c9a7-4db9-9400-43ad51920ff5"
/>
</table>

Note: Done using Coder Agents, reviewed and tested by human locally
2026-03-18 10:21:45 +00:00
Hugo Dutka 2cf47ec384 feat: virtual desktop settings toggle backend (#23171)
Adds a new `site_config` entry that controls whether the virtual desktop
feature for Coder Agents is enabled. It can be set via a new
`/api/experimental/chats/config/desktop-enabled` endpoint, which will be
used by the frontend.
2026-03-18 09:35:13 +01:00
Ethan 11481d7bed perf(coderd/chatd): reduce lock contention in instruction cache and persistStep (#23144)
## Summary

Two targeted performance improvements to the chatd server, identified
through benchmarking.

### 1. RWMutex for instruction cache

The instruction cache is read on every chat turn to fetch the home
instruction file for a workspace agent. Writes only occur on cache
misses (once per agent per 5-minute TTL window), making the access
pattern ~90%+ reads.

Switching from `sync.Mutex` to `sync.RWMutex` and using
`RLock`/`RUnlock` on the read path allows concurrent readers instead of
serializing them.

**Benchmark (200 concurrent chats):**
| | ns/op |
|---|---|
| Mutex | 108 |
| RWMutex | 32 |
| **Speedup** | **3.4x** |

### 2. Hoist JSON marshaling out of persistStep transaction

`MarshalParts`, `PartFromContent`, `CalculateTotalCostMicros`, and the
`usageForCost` struct population are pure CPU work that ran inside the
`FOR UPDATE` transaction in `persistStep`. They have zero dependency on
the database transaction.

Moving all marshal and cost-calculation calls above `p.db.InTx()` means
the row lock is held only for `GetChatByIDForUpdate` +
`InsertChatMessage` calls.

**Benchmark (16 goroutines contending on same lock):**
| Tool calls | Inside lock | Outside lock | Speedup |
|---|---|---|---|
| 1 | 13,977 ns/op | 1,055 ns/op | 13x |
| 5 | 38,203 ns/op | 3,769 ns/op | 10x |
| 10 | 67,353 ns/op | 7,284 ns/op | 9x |
| 20 | 145,864 ns/op | 14,045 ns/op | 10x |

No behavioral changes in either commit.
2026-03-18 16:12:14 +11:00
Kayla はな 49e5547c22 feat: add support for creating service accounts (#23140) 2026-03-17 15:36:20 -06:00
George K 91ec0f1484 feat: add service_accounts workspace sharing mode (#23093)
Introduce a three-way workspace sharing setting (none, everyone,
service_accounts) replacing the boolean workspace_sharing_disabled.
In service_accounts mode, only service account-owned workspaces can be
shared while regular members' share permissions are removed. Adds a
new organization-service-account system role with per-org permissions
reconciled alongside the existing organization-member system role.

Related to:
https://linear.app/codercom/issue/PLAT-28/feat-service-accounts-sharing-mode-and-rbac-role

---------

Co-authored-by: Steven Masley <Emyrk@users.noreply.github.com>
Co-authored-by: Kayla はな <mckayla@hey.com>
2026-03-17 12:16:43 -07:00
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 |
2026-03-17 12:46:24 -04:00
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`)
2026-03-17 14:31:11 +00:00
Ethan 41bd7acf66 perf(chatd): remove redundant chat rereads (#23161)
## Summary
This PR removes two redundant chat rereads in `chatd`.

### Archive / unarchive
- `archiveChat` and `unarchiveChat` already come through
`httpmw.ChatParam`, so the handlers already have the `database.Chat`
row.
- Pass that row into `chatd.ArchiveChat` / `chatd.UnarchiveChat` instead
of rereading by ID before publishing the sidebar events.

### End-of-turn cleanup
- `processChat` no longer calls `GetChatByID` after the cleanup
transaction just to refresh the chat snapshot.
- Title generation already persists the generated title and emits its
own `title_change` event.
- To preserve best-effort title freshness for the cleanup path, the
async title-generation goroutine stores the generated title in per-turn
shared state and cleanup overlays it if available before publishing the
`status_change` event and dispatching push notifications.

## Why
- removes one DB read from archive / unarchive requests
- removes one DB read from completed turns, which is the larger hot-path
win
- keeps the existing pubsub/event contract intact instead of broadening
this into a larger event-model redesign

## Notes
- `title_change` remains the authoritative title update for clients
- cleanup does not wait for title generation; it uses the generated
title only when it is already available
2026-03-18 00:52:06 +11:00
Ethan a33605df58 perf(coderd/chatd): reuse workspace context within a turn (#23145)
## Summary
- reuse workspace agent context within a single `runChat()` turn
- remove duplicate latest-build agent lookups between
`resolveInstructions()` and `getWorkspaceConn()`
- avoid the extra `GetWorkspaceAgentByID` fetch when the selected
`WorkspaceAgent` already has the needed metadata
- add focused internal tests for reuse and refresh-on-dial-failure

## Why
This came out of a 5000-chat / 10-turn scaletest on bravo against a
single workspace.

The run completed successfully, but coderd stayed DB-pool bound, and one
workspace-backed hot path stood out:
- `GetWorkspaceAgentsInLatestBuildByWorkspaceID ≈ 46.7k`
- `GetWorkspaceByID ≈ 48.0k`
- `GetWorkspaceAgentByID ≈ 2.2k`

Within one `runChat()` turn, chatd was rediscovering the same workspace
agent multiple times just to resolve instructions and open the workspace
connection.

## What this changes
This PR introduces a **turn-local** workspace context helper so a single
acquired turn can:
- resolve the selected workspace agent once
- reuse that agent for instruction resolution
- reuse the same `AgentConn` for workspace tools and reload/compaction

This stays turn-local only, so a later turn on another replica still
rebuilds fresh context from the DB.

## Expected impact
This is an incremental improvement, not a full fix.

It should reduce duplicated workspace-agent lookups and shave some DB
pressure from a hot path for workspace-backed chats, while preserving
multi-replica correctness.

## Testing
- `go test ./coderd/chatd/...`
- `golangci-lint run ./coderd/chatd/...`
2026-03-18 00:33:44 +11:00
Danny Kopping 365de3e367 feat: record model thoughts (#22676)
Depends on https://github.com/coder/aibridge/pull/203
Closes https://github.com/coder/internal/issues/1337

---------

Signed-off-by: Danny Kopping <danny@coder.com>
2026-03-17 11:41:10 +00:00
Michael Suchacz 5d0eb772da fix(cored): fix flaky TestInterruptAutoPromotionIgnoresLaterUsageLimitIncrease (#23147) 2026-03-17 19:08:22 +11:00
Ethan 04fca84872 perf(coderd): reduce duplicated reads in push and webpush paths (#23115)
## Background

A 5000-chat scaletest (~50k turns, ~2m45s wall time) completed
successfully,
but the main bottleneck was **DB pool starvation from repeated reads**,
not
individually expensive SQL. The push/webpush path showed a few
especially noisy
reads:

- `GetLastChatMessageByRole` for push body generation
- `GetEnabledChatProviders` + `GetChatModelConfigByID` for push summary
model
  resolution
- `GetWebpushSubscriptionsByUserID` for every webpush dispatch

This PR keeps the optimizations that remove those duplicate reads while
leaving
stream behavior unchanged.

## What changes in this PR

### 1. Reuse resolved chat state for push notifications

`maybeSendPushNotification` used to re-read the last assistant message
and
re-resolve the chat model/provider after `runChat` had already done that
work.

Now `runChat` returns the final assistant text plus the already-resolved
model
and provider keys, and the push goroutine uses that state directly.

That removes the extra push-path reads for:

- `GetLastChatMessageByRole`
- the second `resolveChatModel` path
- the provider/model lookups that came with that second resolution

### 2. Cache webpush subscriptions during dispatch

`Dispatch()` previously hit `GetWebpushSubscriptionsByUserID` on every
push. A
small per-user in-memory cache now avoids those repeated reads.

The follow-up fix keeps that optimization correct: `InvalidateUser()`
bumps a
per-user generation so an older in-flight fetch cannot repopulate the
cache with
pre-mutation data after subscribe/unsubscribe.

That preserves the cache win without letting local subscription changes
be
silently overwritten by stale fetch results.

## Why this is safe

- The push change only reuses data already produced during the same chat
run. It
does not change notification semantics; if there is no assistant text to
  summarize, the existing fallback body still applies.
- The webpush change keeps the existing TTL and `410 Gone` cleanup
behavior. The
generation guard only prevents stale in-flight fetches from poisoning
the
  shared cache after invalidation.
- The final PR does **not** change stream setup, pubsub/relay behavior,
or chat
  status snapshot timing.

## Deliberately not included

- No stream-path optimization in `Subscribe`.
- No inline pubsub message payloads.
- No distributed cross-replica webpush cache invalidation.
2026-03-17 13:50:47 +11:00
Michael Suchacz 1031da9738 feat: add agent chat spend limiting (backend) (#23071)
Introduces deployment-scoped spend limiting for Coder Agents, enabling
administrators to control LLM costs at global, group, and individual
user levels.

## Changes

- **Database migration (000437)**: `chat_usage_limit_config`
(singleton), `chat_usage_limit_overrides` (per-user),
`chat_usage_limit_group_overrides` (per-group)
- **Single-query limit resolution**: individual override > min(group) >
global default via `ResolveUserChatSpendLimit`
- **Fail-open enforcement** in chatd with documented TOCTOU trade-off
- **Experimental API** under `/api/experimental/chats/usage-limits` for
CRUD on limits
- **`AsChatd` RBAC subject** for narrowly-scoped daemon access (replaces
`AsSystemRestricted`)
- **Generated TypeScript types** for the frontend SDK

## Hierarchy

1. Individual user override (highest)
2. Minimum of group limits
3. Global default
4. Disabled / unlimited

Currency stored as micro-dollars (`1,000,000` = $1.00).

Frontend PR: #23072
2026-03-17 01:24:03 +01:00
Steven Masley 93b9d70a9b chore: add audit log entry when ai seat is consumed (#22683)
When an ai seat is consumed, an audit log entry is made. This only happens the first time a seat is used.
2026-03-16 15:30:25 -05:00
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.
2026-03-16 16:22:10 -04:00
Steven Masley abf59ee7a6 feat: track ai seat usage (#22682)
When a user uses an AI feature, we record them in the `ai_seat_state` as consuming a seat. 

Added in debouching to prevent excessive writes to the db for this feature. There is no need for frequent updates.
2026-03-16 12:36:26 -05:00
Steven Masley cabb611fd9 chore: implement database crud for AI seat usage (#22681)
Creates a new table `ai_seat_state` to keep track of when users consume an ai_seat. Once a user consumes an AI seat, they will forever in this table (as it stands today).
2026-03-16 11:53:20 -05:00
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.
2026-03-16 16:40:59 +00:00
Charlie Voiselle e94de0bdab fix(coderd): render HTML error page for OIDC email validation failures (#23059)
## Summary

When the email address returned from an OIDC provider doesn't match the
configured allowed domain list (or isn't verified), users previously saw
raw JSON dumped directly in the browser — an ugly and confusing
experience during a browser-redirect flow.

This PR replaces those JSON responses with the same styled static HTML
error page already used for group allow-list errors, signups-disabled,
and wrong-login-type errors.

## Changes

### `coderd/userauth.go`
Replaced 3 `httpapi.Write` calls in `userOIDC` with
`site.RenderStaticErrorPage`:

| Error case | Title shown |
|---|---|
| Email domain not in allowed list | "Unauthorized email" |
| Malformed email (no `@`) with domain restrictions | "Unauthorized
email" |
| `email_verified` is `false` | "Email not verified" |

All render HTTP 403 with `HideStatus: true` and a "Back to login" action
button.

### `coderd/userauth_test.go`
- Updated `AssertResponse` callbacks on existing table-driven tests
(`EmailNotVerified`, `NotInRequiredEmailDomain`,
`EmailDomainForbiddenWithLeadingAt`) to verify HTML Content-Type and
page content.
- Extended `TestOIDCDomainErrorMessage` to additionally assert HTML
rendering.
- Added new `TestOIDCErrorPageRendering` with 3 subtests covering all
error scenarios, verifying: HTML doctype, expected title/description,
"Back to login" link, and absence of JSON markers.

---------

Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
2026-03-16 11:56:59 -04:00
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.
2026-03-16 15:41:32 +00:00
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).
2026-03-16 08:41:13 -07:00