mirror of
https://github.com/coder/coder.git
synced 2026-09-24 15:04:27 +08:00
cfce751b8a5626978aa8ee8f7a25581fb30d64ea
100
Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
 Kyle Carberry
|
f0fd2111fd |
feat(site/src/pages/AgentsPage): render markdown attachments in preview popup (#24936)
Markdown attachments on `/agents` now render through the same `Response` component used for chat messages instead of falling back to a monospaced `<pre>` block. The popup detects markdown via an explicit `text/markdown` media type and falls back to the `.md`/`.markdown` filename extension when no media type is available. `PreviewTextAttachment` and `TextPreviewDialog` gain an optional `mediaType` so that callers (`AttachmentBlock` for already-sent messages and `AttachmentPreview` for live drafts) can plumb the upload metadata through. Plain `.txt` and unrecognized text attachments keep the existing monospaced rendering. ## Demo  ## Screenshots | Markdown rendering | Plain text rendering | | --- | --- | |  |  | Light theme also verified:  <details> <summary>Coverage details</summary> New stories in `TextPreviewDialog.stories.tsx` cover: - `MarkdownByExtension` — `.md` filename, headings/lists/tables/fenced code render natively. - `MarkdownByMediaType` — explicit `text/markdown` mediaType wins even without a `.md` suffix. - `MarkdownProseOnly` — inline `**bold**`, `_italic_`, and `` `code` `` render via streamdown. - `PlainTextStaysMonospaced` — `.txt` content stays inside `<pre>` so existing previews don't regress. Manual verification (desktop, Chromium, dark + light): all four stories above plus the existing `Default`, `LongContent`, and `NoFileName` stories pass. </details> _Coder Agents generated PR._ |
||
|
Kyle Carberry
|
1ecdad689b |
fix(site/src/pages/AgentsPage): restore sticky user message pinning after react-infinite-scroll-component refactor (#24937)
Restores the sticky user message pinning behavior in the Agents chat that regressed after #24687 swapped the chat scroll container for `react-infinite-scroll-component`. ## Root cause `react-infinite-scroll-component` renders two wrapper divs between the `.overflow-y-auto` scroller and the rendered messages, and its inner wrapper hard-codes `overflow: auto` in its inline style. With the new layout, `position: sticky` on a user message resolved against that inner wrapper rather than the real scroller, so the message scrolled out with its sentinel and the existing fade/clip overlay never engaged. ## Fix Force both InfiniteScroll wrappers to `display: contents` so they no longer participate in layout. The user message's nearest scrolling ancestor is once again the `.overflow-y-auto` element, and `position: sticky` anchors to the scroll container as it did before #24687. The outer wrapper is reached via the Tailwind arbitrary selector `[&>[class$=outerdiv]]:contents` because the library only exposes `style` for the inner wrapper. The inverse infinite-scroll behavior is preserved: the scroller itself stays `flex-col-reverse`, so it remains bottom-anchored and the library's load-more sentinel still lands at the visual top of the content stack. Also drops the dead `overflow-y-auto` class on the floating scroll-to-bottom button wrapper noted in the bug report. ## Test coverage Adds `StickyUserMessagePinsOnScroll` to `AgentChatPageView.stories.tsx`. With a 40-message conversation it walks the user-message sentinels in reverse DOM order to find the one currently pinned (the latest sentinel above the scroller's top edge) and asserts the matching sticky container is anchored within a few pixels of that edge. Without the fix the container ends up hundreds of pixels above the scroller because `position: sticky` silently no-ops. The existing structural `StickyUserMessageStructure` story in `ConversationTimeline.stories.tsx` continues to pass unchanged. <details> <summary>Verification</summary> ```sh pnpm exec tsc -p . # 0 errors pnpm run lint:check # passes pnpm exec vitest run --project=unit # 2303 passed pnpm exec vitest run --project=storybook \ src/pages/AgentsPage/AgentChatPage.stories.tsx \ src/pages/AgentsPage/AgentChatPageView.stories.tsx \ src/pages/AgentsPage/components/ChatConversation/ConversationTimeline.stories.tsx # 90 passed ``` Confirmed the new story fails on `main` (sticky container at the sentinel's position instead of the scroller top) and passes with the fix applied. </details> --- Generated by Coder Agents. |
||
|
Kyle Carberry
|
c16034d127 |
fix(site/src/pages/AgentsPage/components): match chat input font size to message bubbles (#24928)
The `/agents` chat composer rendered user input at 15px, but once a message was sent the user message bubble displays content at 13px (via `MessageContent` in `ChatElements/Message.tsx`). The size jump made what you typed not match what got displayed. Sets the `ChatMessageInput` className inside `AgentChatInput` to `text-[13px] leading-relaxed` so the composer matches the rendered user message. <sub>This PR was opened by a Coder agent on behalf of @kylecarbs.</sub> |
||
|
Kyle CarberryandCoder Agents
|
d889ba1842 |
feat: add user_oidc auth type for MCP servers (#24793)
Adds a 5th MCP server authentication mode, `user_oidc` ("User OIDC
Identity"), that forwards the calling user's OIDC access token from
`user_links.oauth_access_token` to the upstream MCP server as
`Authorization: Bearer <token>`.
The token is read from `user_links` and refreshed transparently via
`oauth2.TokenSource` before each MCP request. No new per-MCP-server
secret storage and no per-user connect/disconnect step.
**Limitation**: only users who logged in via OIDC have a forwardable
token. Users authenticated via password or GitHub will see requests sent
without an `Authorization` header, and the upstream MCP server is
expected to respond with 401. A pluggable token source (e.g. CLI-minted
E2E tokens) is left as future work.
<details>
<summary>Implementation notes</summary>
- Schema: new
`coderd/database/migrations/000481_mcp_user_oidc_auth.{up,down}.sql`
relaxes the `mcp_server_configs.auth_type` CHECK constraint to include
`user_oidc`. Down migration deletes affected rows before restoring the
old constraint.
- SDK validation: `codersdk/mcp.go` extends `oneof` for
`CreateMCPServerConfigRequest` and `UpdateMCPServerConfigRequest`.
- Handler: `coderd/mcp.go` adds `case "user_oidc":` to the
field-clearing switch on update. The existing list and detail handlers
already report `auth_connected = true` for any non-`oauth2` auth type.
- Header construction: `coderd/x/chatd/mcpclient/mcpclient.go`
introduces a `UserOIDCTokenSource` interface and adds the `user_oidc`
case to `buildAuthHeaders`. `ConnectAll` / `connectOne` /
`buildAuthHeaders` gain `userID uuid.UUID, oidcSrc UserOIDCTokenSource`
parameters.
- Wiring: `coderd/x/chatd/chatd.go` adds `OIDCTokenSource` to `Config` /
`Server` and passes `chat.OwnerID` plus the source through `ConnectAll`.
`coderd/coderd.go` constructs the source next to the `chatd.New` call
when `options.OIDCConfig` is non-nil.
- Token source: `oidcMCPTokenSource` lives in `coderd/mcp.go`. It reads
the user's OIDC link, refreshes via `oauth2.TokenSource`, and writes the
refreshed token back to `user_links`. Logic is duplicated from
`provisionerdserver.ObtainOIDCAccessToken` to avoid an MCP ->
provisionerdserver dependency. The two copies must be kept in sync; a
comment on `oidcMCPTokenSource` records this.
- Frontend: `MCPServerAdminPanel.tsx` adds the new dropdown option, an
explanatory helper block (no admin-configurable fields), and a Storybook
story (`CreateServerUserOIDC`).
- Tests:
- `mcpclient_test.go`: `TestConnectAll_UserOIDCAuth`,
`TestConnectAll_UserOIDCAuth_NoLink`,
`TestConnectAll_UserOIDCAuth_NilSource`. All existing tests updated for
the new signature.
- `mcp_test.go`: extends `TestMCPServerConfigsAuthConnected` to assert
`auth_connected=true` for `user_oidc`; adds
`TestMCPServerConfigsUserOIDCClearsFields` and
`TestMCPServerConfigsUserOIDCDirect`.
- Docs: `docs/ai-coder/agents/platform-controls/mcp-servers.md`
describes the new mode and its OIDC-only limitation.
</details>
This PR was created by Coder Agents.
---------
Co-authored-by: Coder Agents <agents@coder.com>
|
||
|
Kyle Carberry
|
4a91656fe5 |
refactor(site/src/pages/AgentsPage): align tool-call and message styling (#24790)
Tighten visual rhythm and typography in the agent chat page so tool calls, reasoning, and assistant text share the same baseline. ## Highlights - Unify font size to **13px** across user messages, assistant `Response`, reasoning, and every tool-call label. - Reuse the `text-content-secondary → hover:text-content-primary` transition on tool-call rows so labels, chevrons, and lucide icons brighten together. Icons inside hover-aware headers switch to `text-current` so they inherit the parent transition; static icons in non-collapsible cards (`ExecuteTool`, `ProcessOutputTool`) keep the constant secondary color. - Collapse padding between adjacent tool/thinking blocks via a shared `data-tool-call` attribute and adjacent-sibling selectors (`[&:has(+[data-tool-call])]:pb-0` + `[[data-tool-call]+&]:pt-0`). First/last items keep their padding against text and reasoning siblings. - `read_file` now mirrors `write_file`: `Reading <name>…` while running, `Read <name>` once complete. - `ask_user_question` flips the inline label from `Asking:` to `Asked:` once answered. - Subagent row layout: status icon + label + chevron sit together at the start of the row, while the `Worked for <duration>` text uses `ml-auto` to anchor the right edge. - New Storybook story `WithEveryTool` (under `Pages / AgentsPage / AgentChatPage`) exercises every tool renderer plus subagent variants and the generic MCP fallback in a single completed-then-streaming turn. --- _Authored with help from a Coder Agent._ |
||
|
Kyle Carberry
|
ad3095106d |
fix(site/src/pages/AgentsPage): align thinking disclosure (#24743)
The agent chat thinking disclosure used a smaller label with the caret on the left, which made collapsed and in-progress thinking look different from collapsible tool calls. Align the thinking disclosure with the shared tool-call row treatment by using the same label size, trailing caret placement, and hover color while preserving the streaming shimmer. Adds a Storybook story that renders collapsed thinking next to a tool call. <details> <summary>Storybook screenshots</summary> Captured from Storybook: - `pages/AgentsPage/ChatConversation/ConversationTimeline/ThinkingBlockWithToolCall` - `pages/AgentsPage/ChatConversation/ConversationTimeline/ThinkingBlockWithToolCall` hovered - `pages/AgentsPage/ChatConversation/StreamingOutput/ThinkingDuringStreamingWithToolCalls` Screenshots are attached in the Coder task. </details> Generated by Coder Agents. |
||
|
Kyle Carberry
|
069223ae26 | fix: recover web push subscriptions after PWA reinstall (#24720) | ||
|
Kyle Carberry
|
8bc91d982f | fix(site): truncate long workspace name in chat input toolbar (#24412) | ||
|
Kyle Carberry
|
9c74c8c674 | fix: move OnChatUpdated call after agent is ready in create/start workspace (#24410) | ||
|
Kyle Carberry
|
d11849d94a |
fix: re-fetch context files and skills from workspace on each turn (#24360)
Context files (AGENTS.md) and skills were only fetched from the
workspace on the first turn or when the agent changed. On subsequent
turns, stale content from persisted messages was used. This meant that
if AGENTS.md or skills were modified on the workspace between turns, the
agent wouldn't see the changes until the user created a new chat.
## Changes
- Extract `fetchWorkspaceContext` from `persistInstructionFiles` to
allow fetching workspace context without persisting
- On subsequent turns, re-fetch fresh context from the workspace instead
of reading stale persisted content; falls back to persisted messages if
the workspace dial fails
- Update `ReloadMessages` callback to re-derive instruction and skills
from reloaded database messages after compaction, instead of using
captured closure variables
- Add `formatSystemInstructionsFromParts` helper to build system
instructions directly from agent parts without requiring separate
OS/directory params
- Add tests for the new helper
<details><summary>Implementation Notes</summary>
### Root cause
In `runChat`, the `else if hasContextFiles` branch (subsequent turns)
called `instructionFromContextFiles(messages)` which read stale content
from persisted DB messages. The `ReloadMessages` callback
(post-compaction) also used captured `instruction`/`skills` closure
variables from the start of the turn, never re-deriving them.
### Approach
1. **Extract `fetchWorkspaceContext`** — Pure refactor of the fetch-only
part of `persistInstructionFiles` (agent connection, context config
retrieval, content sanitization, metadata stamping). Returns parts +
skills without persisting.
2. **Subsequent turns**: Instead of reading from persisted messages,
launch a `g2` goroutine that calls `fetchWorkspaceContext` to get fresh
context from the workspace. Falls back gracefully to persisted messages
if the workspace is unreachable.
3. **ReloadMessages**: Re-derive `instruction` from
`instructionFromContextFiles(reloadedMsgs)` and `skills` from
`skillsFromParts(reloadedMsgs)` using the freshly loaded messages, with
fallback to captured values if the reloaded messages don't contain
context (e.g. compacted away).
</details>
> 🤖 Generated by Coder Agents
|
||
|
Kyle Carberry
|
a414d37165 |
feat(site/src/pages/AgentsPage): add full-width chat layout toggle (#24307)
Adds a user preference to remove the `max-w-3xl` constraint on agent chat messages, letting the content fill the available viewport width. The toggle lives in **Agents > Settings > Behavior** under "Chat Layout" and persists via `localStorage` (`agents.chat-full-width`). A `useSyncExternalStore` hook (`useChatFullWidth`) provides same-tab reactivity so flipping the toggle updates all mounted consumers immediately — the chat timeline, chat input, the Suspense skeleton, and the in-page loading view. This was requested by a customer and is an individual user setting, so it seems fine to add. |
||
|
Kyle Carberry
|
69917b4516 |
fix: resolve flaky TestWatchChats/DiffStatusChangeIncludesDiffStatus (#24298)
## Problem `TestWatchChats/DiffStatusChangeIncludesDiffStatus` flakes with: ``` failed to read JSON message: failed to get reader: context deadline exceeded ``` Tracker: https://github.com/coder/internal/issues/1467 ## Root Cause The test published a pubsub event **once**, immediately after `client.Dial()` returned. However, `Dial` completes as soon as the HTTP upgrade finishes — before the server-side `watchChats` handler has called `SubscribeWithErr` on the pubsub channel. When the publish races ahead of the subscription, the message is silently dropped and the `wsjson.Read` loop blocks until the context deadline. ## Fix Publish the event on a short ticker (`testutil.IntervalFast` = 25ms) in a background goroutine instead of publishing once. This guarantees that at least one publish lands **after** the subscription is active, regardless of goroutine scheduling. The goroutine is cleanly stopped via a `publishDone` channel once the expected event is received. ## Verification - `go test -race -count=50 -run TestWatchChats/DiffStatusChangeIncludesDiffStatus` → **50/50 PASS, 0 races** - `go test -race -count=5 -run TestWatchChats` → **all 5 subtests pass 5/5 times** |
||
|
Kyle Carberry
|
8dff1cbc57 |
fix: resolve idle timeout recording test flake on macOS (#24240)
Fixes https://github.com/coder/internal/issues/1461 Two synchronization issues caused `TestPortableDesktop_IdleTimeout_StopsRecordings` (and the `MultipleRecordings` variant) to flake on macOS CI: 1. **`clk.Advance(idleTimeout)` was not awaited.** In `MultipleRecordings`, both idle timers fire simultaneously but their `fire()` goroutines race to remove themselves from the mock clock's event list. Without `MustWait`, the second timer may still be in `m.all` when the next `Advance` is called, causing `"cannot advance ... beyond next timer/ticker event in 0s"`. 2. **The test depended on SIGINT being handled promptly.** After the `stop_timeout` timer was released, the test relied entirely on the shell process handling SIGINT (via `rec.done`). On macOS, `/bin/sh` may not interrupt `wait` reliably, leaving `lockedStopRecordingProcess` blocked in its `select` while holding `p.mu` — deadlocking the `require.Eventually` callback. ### Fix Wait for each `Advance` to complete and advance past the 15s stop timeout so the process is forcibly killed via the timer path, independent of signal handling. Verified with 1000 iterations (500 per test) with zero failures. > Generated with [Coder Agents](https://coder.com/agents) |
||
|
Kyle CarberryandMichael Suchacz
|
391b22aef7 |
feat: add CLI commands for managing chat context from workspaces (#24105)
Adds `coder exp chat context add` and `coder exp chat context clear` commands that run inside a workspace to manage chat context files via the agent token. `add` reads instruction and skill files from a directory (defaulting to cwd) and inserts them as context-file messages into an active chat. Multiple calls are additive — `instructionFromContextFiles` already accumulates all context-file parts across messages. `clear` soft-deletes all context-file messages, causing `contextFileAgentID()` to return `!found` on the next turn, which triggers `needsInstructionPersist=true` and re-fetches defaults from the agent. Both commands auto-detect the target chat via `CODER_CHAT_ID` (already set by `agentproc` on chat-spawned processes), or fall back to single-active-chat resolution for the agent. The `--chat` flag overrides both. Also adds sub-agent context inheritance: `createChildSubagentChat` now copies parent context-file messages to child chats at spawn time, so delegated sub-agents share the same instruction context without independently re-fetching from the workspace agent. <details><summary>Implementation details</summary> **New files:** - `cli/exp_chat.go` — CLI command tree under `coder exp chat context` **Modified files:** - `agent/agentcontextconfig/api.go` — `ConfigFromDir()` reads context from an arbitrary directory without env vars - `codersdk/agentsdk/agentsdk.go` — `AddChatContext`/`ClearChatContext` SDK methods - `coderd/workspaceagents.go` — POST/DELETE handlers on `/workspaceagents/me/chat-context` - `coderd/coderd.go` — Route registration - `coderd/database/queries/chats.sql` — `GetActiveChatsByAgentID`, `SoftDeleteContextFileMessages` - `coderd/database/dbauthz/dbauthz.go` — RBAC implementations for new queries - `coderd/x/chatd/subagent.go` — `copyParentContextFiles` for sub-agent inheritance - `cli/root.go` — Register `chatCommand()` in `AGPLExperimental()` **Auth pattern:** Uses `AgentAuth` (same as `coder external-auth`) — agent token via `CODER_AGENT_TOKEN` + `CODER_AGENT_URL` env vars. </details> > 🤖 Generated by Coder Agents --------- Co-authored-by: Michael Suchacz <203725896+ibetitsmike@users.noreply.github.com> |
||
|
Kyle CarberryandEthan
|
35c26ce22a |
feat: add CreatedAt to tool-call and tool-result ChatMessageParts (#24101)
Adds an optional `CreatedAt` timestamp to `tool-call` and `tool-result` `ChatMessagePart` variants so the frontend can compute tool execution duration (`result.created_at - call.created_at`). Timestamps are recorded at the correct moments in the chatloop: - **Tool-call**: when the model stream emits the tool call - **Tool-result**: when tool execution completes (or is interrupted) These are passed through `PersistedStep.PartCreatedAt` so the persistence layer can apply accurate timestamps to stored parts. SSE-published parts also carry `CreatedAt` for real-time display. Old persisted messages without `created_at` deserialize to `nil` — fully backward compatible. <details><summary>Implementation notes (Coder Agents generated)</summary> ### Why not stamp in `PartFromContent`? `PartFromContent` is called both for SSE publishing (correct timing) and during persistence (wrong timing — both tool-call and tool-result would get the same "persistence time" timestamp, yielding ~0 duration). Instead, timestamps are captured in the chatloop at the right moments and carried through `PersistedStep.PartCreatedAt` as a `map[string]time.Time` keyed by `"call:<id>"` / `"result:<id>"`. ### Interrupted tool calls `persistInterruptedStep` also stamps `CreatedAt` on synthetic error results for cancelled/interrupted tool calls, so partial duration is available. ### Files changed | File | Change | |------|--------| | `codersdk/chats.go` | Add `CreatedAt *time.Time` field | | `codersdk/chats_test.go` | JSON round-trip test | | `coderd/database/dbtime/dbtime.go` | Add `TimePtr` helper | | `coderd/x/chatd/chatloop/chatloop.go` | Track timestamps, pass through `PersistedStep` | | `coderd/x/chatd/chatd.go` | Apply timestamps during persistence | | `coderd/x/chatd/chatprompt/chatprompt_test.go` | Verify `PartFromContent` does NOT stamp | | `site/src/api/typesGenerated.ts` | Auto-generated | </details> --------- Co-authored-by: Ethan <39577870+ethanndickson@users.noreply.github.com> |
||
|
Kyle Carberry
|
b969d66978 |
feat: add dynamic tools support for chat API (#24036)
Adds client-executed dynamic tools to the chat API. Dynamic tools are
declared by the client at chat creation time, presented to the LLM
alongside built-in tools, but executed by the client rather than chatd.
This enables external systems (Slack bots, IDE extensions, Discord bots,
CI/CD integrations) to plug custom tools into the LLM chat loop without
modifying chatd's built-in tool set.
Modeled after OpenAI's Assistants API: the chat pauses with
`requires_action` status when the LLM calls a dynamic tool, the client
POSTs results back via `POST /chats/{id}/tool-results`, and the chat
resumes.
See [this example](https://github.com/coder/coder-slackbot-poc) as a
reference for how this is used. It's highly-configurable, which would
enable creating chats from webhooks, periodically polling, or running as
a Slackbot.
<details>
<summary>Design context</summary>
### Architecture
The chatloop **exits** when it encounters dynamic tools and
**re-enters** when results arrive. No blocking channels, no pubsub for
tool results, no in-memory registry. The DB is the only coordination
mechanism.
```
Phase 1 (chatloop):
LLM response → execute built-in tools only →
Persist(assistant + built-in results) →
status = requires_action → chatloop exits
Phase 2 (POST /tool-results):
Persist(dynamic tool results) →
status = pending → wakeCh → chatloop re-enters
```
### Validation (POST /tool-results)
1. Chat status must be `requires_action` (409 if not)
2. Read chat's `dynamic_tools` → set of dynamic tool names
3. Read last assistant message → extract tool-call parts matching
dynamic tool names
4. Submitted tool_call_ids must match exactly (400 for missing/extra)
5. Persist tool-result message parts, set status to `pending`, signal
wake
### Idempotency
Tool call IDs scoped per LLM step. State machine (`requires_action` →
`pending`) is the guard. First POST wins, subsequent get 409.
### Mixed tool calls
When the LLM calls both built-in and dynamic tools in one step, built-in
tools execute immediately. Their results are persisted in phase 1.
Dynamic tool results arrive via POST in phase 2. The LLM sees all
results when the chatloop resumes.
</details>
> 🤖 Generated by Coder Agents
|
||
|
Kyle Carberry
|
c5d720f73d |
feat(coderd): add telemetry for agents chats and messages (#24068)
Adds telemetry collection for the agents chat system (`/agents`) to the
existing telemetry snapshot pipeline.
Three new snapshot fields:
- **`Chats`** — per-chat metadata (id, owner, status, mode,
workspace_id, root_chat_id, has_parent, archived, model config)
collected time-windowed via `createdAfter`
- **`ChatMessageSummaries`** — per-chat aggregated message metrics
(counts by role, token sums by type, cost, runtime, model count,
compression count) collected time-windowed
- **`ChatModelConfigs`** — model configuration metadata (provider,
model, context limit, enabled, default) collected as full dump
No PII is included — titles, message content, and URLs are excluded at
the SQL level. Only structural metadata flows through telemetry.
<details><summary>Implementation plan</summary>
### SQL Queries (`coderd/database/queries/chats.sql`)
- `GetChatsCreatedAfter` — time-windowed chat metadata
- `GetChatMessageSummariesPerChat` — per-chat message aggregates via
`GROUP BY`
- `GetChatModelConfigsForTelemetry` — full dump of model configs
### Telemetry (`coderd/telemetry/telemetry.go`)
- `Chat`, `ChatMessageSummary`, `ChatModelConfig` structs
- `ConvertChat`, `ConvertChatMessageSummary`, `ConvertChatModelConfig`
conversion functions
- Three `eg.Go()` blocks in `createSnapshot()` following the existing
collection pattern
### Authorization (`coderd/database/dbauthz/dbauthz.go`)
- System-only access for all three queries via `rbac.ResourceSystem`
### Tests
- `TestChatsTelemetry` in `coderd/telemetry/telemetry_test.go` — creates
chats (root + child), messages with token/cost data, model configs;
verifies all snapshot fields
- dbauthz test entries for all three queries in
`coderd/database/dbauthz/dbauthz_test.go`
</details>
> 🤖 Generated by Coder Agents
|
||
|
Kyle Carberry
|
f3f0a2c553 |
fix(enterprise/coderd/x/chatd): harden TestSubscribeRelayEstablishedMidStream against CI flakes (#24108)
Fixes coder/internal#1455 Three changes to eliminate the timing-sensitive flake in `TestSubscribeRelayEstablishedMidStream`: 1. **Reduce `PendingChatAcquireInterval` from `time.Hour` to `time.Second`.** The primary trigger is still `signalWake()` from `SendMessage`, but a short fallback poll ensures the worker picks up the pending chat even under heavy CI goroutine scheduling contention. 2. **Increase context timeout from `WaitLong` (25s) to `WaitSuperLong` (60s).** The worker pipeline (model resolution, message loading, LLM call) involves multiple DB round-trips that can be slow when PostgreSQL is shared with many parallel test packages. 3. **Add a status-polling loop while waiting for the streaming request.** If the worker errors out during chat processing, the test now fails immediately with the error status and message instead of silently timing out. > Generated by Coder Agents |
||
|
Kyle Carberry
|
cffc68df58 | feat(site): render read_skill body as markdown (#24069) | ||
|
Kyle Carberry
|
16265e834e |
chore: update fantasy fork to use github.com/coder/fantasy (#24100)
Moves the `charm.land/fantasy` replace directive from `github.com/kylecarbs/fantasy` to `github.com/coder/fantasy`, pointing at the same `cj/go1.25` branch and commit (`112927d9b6d8`). > Generated by Coder Agents |
||
|
Kyle Carberry
|
684f21740d |
perf(coderd): batch chat heartbeat queries into single UPDATE per interval (#24037)
## Summary
Replaces N per-chat heartbeat goroutines with a single centralized
heartbeat loop that issues one `UPDATE` per 30s interval for all running
chats on a worker.
## Problem
Each running chat spawned a dedicated goroutine that issued an
individual `UPDATE chats SET heartbeat_at = NOW() WHERE id = $1 AND
worker_id = $2 AND status = 'running'` query every 30 seconds. At 10,000
concurrent chats this produces **~333 DB queries/second** just for
heartbeats, plus ~333 `ActivityBumpWorkspace` CTE queries/second from
`trackWorkspaceUsage`.
## Solution
New `UpdateChatHeartbeats` (plural) SQL query replaces the old singular
`UpdateChatHeartbeat`:
```sql
UPDATE chats
SET heartbeat_at = @now::timestamptz
WHERE worker_id = @worker_id::uuid
AND status = 'running'::chat_status
RETURNING id;
```
A single `heartbeatLoop` goroutine on the `Server`:
1. Ticks every `chatHeartbeatInterval` (30s)
2. Issues one batch UPDATE for all registered chats
3. Detects stolen/completed chats via set-difference (equivalent of old
`rows == 0`)
4. Calls `trackWorkspaceUsage` for surviving chats
`processChat` registers an entry in the heartbeat registry instead of
spawning a goroutine.
## Impact
| Metric | Before (10K chats) | After (10K chats) |
|---|---|---|
| Heartbeat queries/sec | ~333 | ~0.03 (1 per 30s per replica) |
| Heartbeat goroutines | 10,000 | 1 |
| Self-interrupt detection | Per-chat `rows==0` | Batch set-difference |
---
> 🤖 Generated by Coder Agents
<details><summary>Implementation notes</summary>
- Uses `@now` parameter instead of `NOW()` so tests with `quartz.Mock`
can control timestamps.
- `heartbeatEntry` stores `context.CancelCauseFunc` + workspace state
for the centralized loop.
- `recoverStaleChats` is unaffected — it reads `heartbeat_at` which is
still updated.
- The old singular `UpdateChatHeartbeat` is removed entirely.
- `dbauthz` wrapper uses system-level `rbac.ResourceChat` authorization
(same pattern as `AcquireChats`).
</details>
|
||
|
Kyle Carberry
|
acd5f01b4b |
fix: use GreaterOrEqual for step runtime assertion in chatloop test (#24067)
Fixes https://github.com/coder/internal/issues/1418 The `TestRun_ActiveToolsPrepareBehavior` test asserts `persistedStep.Runtime > 0`, but on Windows the timer resolution (~15ms) means the in-memory mock model can complete within the same clock tick, producing a measured duration of `0s`. Change the assertion from `require.Greater` to `require.GreaterOrEqual` so that a legitimately measured zero duration on low-resolution clocks does not cause a flake. > Generated by Coder Agents |
||
|
Kyle Carberry
|
6c62d8f5e6 |
fix(coderd/x/chatd): fix flaky TestAwaitSubagentCompletion/CompletesViaPubsub (#24066)
## Fix flaky TestAwaitSubagentCompletion/CompletesViaPubsub Fixes coder/internal#1435 ### Root Cause During `createParentChildChats`, the processor publishes notifications on `ChatStreamNotifyChannel(child.ID)` via PostgreSQL `LISTEN/NOTIFY`. After `drainInflight()` returns, these stale notifications can still be buffered in the pgListener's `NotifyChan()`. When `awaitSubagentCompletion` subscribes and a stale notification is dispatched between `setChatStatus(Waiting)` and `insertAssistantMessage`, `checkSubagentCompletion` sees `done=true` (status is `Waiting`) but returns an empty report because the message hasn't been committed yet. ### Fix Swap the order: insert the assistant message **before** transitioning the status to `Waiting`. This guarantees the report is committed before the status makes the chat appear complete to `checkSubagentCompletion`. ### Verification - 50 consecutive runs of the specific test: all pass - 10 runs of the full `TestAwaitSubagentCompletion` suite: all pass - 20 runs with `-race`: all pass > Generated by Coder Agents |
||
|
Kyle Carberry
|
648787e739 |
feat: expose busy_behavior on chat message API (#24054)
The backend (`chatd.go`) already fully implements both `"queue"` and `"interrupt"` busy behaviors for `SendMessage`, and the `message_agent` subagent tool already leverages both internally. However the HTTP API hardcoded `"queue"` and the SDK had no way for callers to request interrupt-on-send. This adds a `ChatBusyBehavior` enum type to the SDK and an optional `busy_behavior` field on `CreateChatMessageRequest`. The HTTP handler validates the field and passes it through to `chatd.SendMessage`. Default remains `"queue"` for full backward compatibility. <details><summary>Implementation notes</summary> - `codersdk/chats.go`: New `ChatBusyBehavior` type with `ChatBusyBehaviorQueue` and `ChatBusyBehaviorInterrupt` constants. Added `BusyBehavior` field to `CreateChatMessageRequest` with `enums` tag for codegen. - `coderd/exp_chats.go`: `postChatMessages` now reads `req.BusyBehavior`, maps SDK constants to `chatd.SendMessageBusyBehavior*`, returns 400 on invalid values. - `site/src/api/typesGenerated.ts`: Auto-generated via `make gen`. - No frontend behavior changes — the field is available but unused by the UI. </details> > [!NOTE] > Generated by Coder Agents |
||
|
Kyle Carberry
|
4cfbf544a0 |
feat: add per-chat system prompt option (#24053)
Adds a `system_prompt` field to `CreateChatRequest` that allows API consumers to provide custom instructions when creating a chat. The per-chat prompt is stored as a separate system message (`role=system`, `visibility=model`) in the `chat_messages` table, inserted between the deployment system prompt and the workspace awareness message. Also moves deployment system prompt resolution from the HTTP handler (`resolvedChatSystemPrompt`) into `chatd.CreateChat` where it belongs. The handler no longer assembles system prompts — `CreateOptions.SystemPrompt` is now purely the per-chat user prompt, and the deployment prompt is resolved internally by chatd. No database schema changes required. **Message insertion order:** 1. Deployment system prompt (resolved by chatd, existing) 2. Per-chat user system prompt (new, from `CreateOptions.SystemPrompt`) 3. Workspace awareness (existing) 4. Initial user message (existing) 🤖 Generated with [Coder Agents](https://coder.com/agents) |
||
|
Kyle Carberry
|
a2ce74f398 |
feat: add total_runtime_ms to chat cost analytics endpoints (#24050)
Surface the aggregated `runtime_ms` from `chat_messages` through all
four cost analytics queries (summary, per-model, per-chat, per-user).
This is the key billing metric for agent compute time.
The per-chat breakdown already groups by `root_chat_id`, so subagent
runtime is automatically rolled up under the parent chat — no additional
query changes needed.
<details>
<summary>Implementation details</summary>
**SQL** (`coderd/database/queries/chats.sql`): Added
`COALESCE(SUM(cm.runtime_ms), 0)::bigint AS total_runtime_ms` to
`GetChatCostSummary`, `GetChatCostPerModel`, `GetChatCostPerChat`, and
`GetChatCostPerUser`.
**Go SDK** (`codersdk/chats.go`): Added `TotalRuntimeMs int64` to
`ChatCostSummary`, `ChatCostModelBreakdown`, `ChatCostChatBreakdown`,
and `ChatCostUserRollup`.
**Handler** (`coderd/exp_chats.go`): Wired the new field through all
converter functions and the response assembly.
**Tests** (`coderd/exp_chats_test.go`): Updated fixture to seed non-zero
`runtime_ms` values and added assertions for the new field at summary,
per-model, and per-chat levels.
</details>
> 🤖 Generated by Coder Agents
|
||
|
Kyle Carberry
|
500fc5e2a4 |
feat: polish model config form UI (#24047)
Polishes the AI model configuration form (add/edit model) with tighter
layout and better input affordances.
**Frontend changes:**
- Replace "Unset" with "Default" in select dropdowns to communicate
system fallback
- Show pricing fields inline instead of behind a collapsible toggle
- Use flat section dividers (`border-t`) instead of bordered fieldsets
- Move field descriptions into info-icon tooltips to fix input
misalignment
- Add InputGroup adornments: `$` prefix + `/1M` suffix on pricing,
`tokens` suffix on token fields, `%` suffix on compression threshold,
range placeholders on temperature/penalty fields
- Shorter pricing labels (Input, Output, Cache Read, Cache Write)
- Compact JSON textareas (1-row height, resizable)
- Smart grid layouts by field type (3-col provider, 4-col pricing, 3-col
advanced)
- Boolean fields render as a segmented control (Default · On · Off)
instead of a dropdown
**Backend changes:**
- Add `enum` tags to OpenAI `service_tier`
(`auto,default,flex,scale,priority`) and `reasoning_summary`
(`auto,concise,detailed`) so they render as select dropdowns instead of
free-text inputs
> 🤖 Generated by Coder Agents
|
||
|
Kyle Carberry
|
937f50f0ae |
fix: show message action tooltips at bottom on agents page (#24041)
The CopyButton tooltip on `/agents` defaulted to top (Radix default),
while the Edit button already used `side="bottom"`. This adds an
optional `tooltipSide` prop to `CopyButton` and passes `"bottom"` in the
agents `ConversationTimeline` so both tooltips appear below the buttons
consistently.
## Changes
- `CopyButton`: added optional `tooltipSide` prop, forwarded to
`<TooltipContent side={tooltipSide}>`
- `ConversationTimeline`: passed `tooltipSide="bottom"` to the
copy-message `CopyButton`
> Generated by Coder Agents
|
||
|
Kyle Carberry
|
a16755dd66 |
fix: prevent stale REST status from dropping streamed parts (#24040)
The `useEffect` that syncs `chatRecord.status` from React Query unconditionally overwrites the store's `chatStatus`. The `chat(chatId)` query has no `staleTime` (defaults to 0), so it refetches on window focus, remount, etc. If the REST response catches a transient `"pending"` status (e.g. between multi-step tool-call cycles), it regresses `chatStatus` from `"running"` to `"pending"`. Since `shouldApplyMessagePart()` drops ALL parts when status is `"pending"` or `"waiting"`, every incoming `message_part` event is silently discarded — not even buffered. Parts are visible on the WebSocket but nothing renders, and the UI shows "Response is taking longer than expected". A page reload fixes it because a fresh REST fetch returns the current status. **Fix:** Add `wsStatusReceivedRef` — once the WebSocket delivers a status event, it becomes the authoritative source and REST refetches can no longer overwrite it. This mirrors the existing `wsQueueUpdateReceivedRef` pattern already used for queued messages. The ref resets on chat change. > Generated with [Coder Agents](https://coder.com/agents) |
||
|
Kyle Carberry
|
8bdc35f91f |
refactor(site): unify message copy/edit UX across user and assistant messages (#24039)
Aligns the copy/edit action bar so both user and assistant messages use
the same hover-to-reveal pattern.
## Changes
- Replace bifurcated copy UX (inline `afterResponseSlot` for assistant,
floating toolbar for user) with a single unified action bar using
`CopyButton` + optional edit `Button`
- Remove `BlockList` `afterResponseSlot` prop and related machinery
- Remove per-message `copyHovered`/`useClipboard` state and left-border
highlight effect
- Remove `lastAssistantPerTurnIds`/`isTurnActive` computation — all
messages with content get actions on hover
- Hide actions on mid-chain assistant messages (only last in consecutive
chain shows buttons)
- Reduce inter-message gap from `gap-3` to `gap-2`
- Shrink action buttons to `size-6` for tighter vertical spacing
- Add 8px sticky top offset for user messages
> 🤖 Generated by Coder Agents
|
||
|
Kyle Carberry
|
5b32c4d79d |
fix: prevent stdio MCP server subprocess from dying after connect (#24035)
## Problem
MCP servers configured in `.mcp.json` with stdio transport are
discovered successfully (tools appear) but die immediately after
connection, making all tool calls fail.
## Root Cause
In `connectServer`, the subprocess is spawned with `connectCtx` — a
30-second timeout context whose `cancel()` is deferred:
```go
connectCtx, cancel := context.WithTimeout(ctx, connectTimeout)
defer cancel()
if err := c.Start(connectCtx); err != nil { ... }
```
The mcp-go stdio transport calls `exec.CommandContext(connectCtx, ...)`.
When `connectServer` returns, `cancel()` fires, and
`exec.CommandContext` sends SIGKILL to the subprocess. The process
immediately becomes a zombie.
Confirmed by checking `/proc/<pid>/status` after context cancellation:
```
State: Z (zombie)
```
## Fix
Pass the parent `ctx` (which is `a.gracefulCtx` — the agent's long-lived
context) to `c.Start()`. `connectCtx` continues to bound only the
`Initialize()` handshake. The subprocess is cleaned up when the Manager
is closed or the parent context is canceled.
## Regression Test
Added `TestConnectServer_StdioProcessSurvivesConnect` which:
- Spawns a real subprocess (re-execs the test binary as a fake MCP
server)
- Calls `connectServer` and lets it return (internal `connectCtx` gets
canceled)
- Verifies the subprocess is still alive by calling `ListTools`
The test **fails** on the old code with `transport error: context
deadline exceeded` and **passes** with the fix.
> Generated with [Coder Agents](https://coder.com/agents)
|
||
|
Kyle Carberry
|
8625543413 |
feat(coderd/x/chatd): parallelize ConvertMessagesWithFiles with g2 errgroup (#24034)
## Summary Move `ConvertMessagesWithFiles` into the `g2` errgroup so prompt conversion runs concurrently with instruction persistence, user prompt resolution, MCP server connections, and workspace MCP tool discovery. ## Problem In `runChat`, the setup before the first LLM `Stream()` call is sequential across two errgroups: ``` g.Wait() // model + messages + MCP configs ConvertMessagesWithFiles() // sequential — blocked on g2 starting g2.Wait() // instructions + user prompt + MCP connect + workspace MCP ``` `ConvertMessagesWithFiles` can take non-trivial time on conversations with file attachments (batch DB resolution), and it was blocking g2 from starting. ## Fix `ConvertMessagesWithFiles` only reads the `messages` slice (available after `g.Wait()`) and resolves file references via the database. No g2 task reads or writes the `prompt` variable. This makes it safe to overlap with g2: ``` g.Wait() g2.Wait() // now includes ConvertMessagesWithFiles in parallel ``` The `InsertSystem` call for parent chats and the `promptErr` check are deferred to after `g2.Wait()`, preserving correctness. <details><summary>Decision log</summary> - `ConvertMessagesWithFiles` is read-only on `messages` — no mutation, safe for concurrent access - `prompt` and `promptErr` are written only by the conversion goroutine, read only after `g2.Wait()` — no data race - Error from prompt conversion is checked immediately after `g2.Wait()`, before any code that uses `prompt` - `chatloop.Run` now uses `:=` instead of `=` since the prior `err` declaration from `prompt, err :=` was removed </details> > Generated by Coder Agents |
||
|
Kyle Carberry
|
e18094825a | fix: retain message_part buffer for cross-replica relay (#24031) | ||
|
Kyle Carberry
|
919dc299fc |
feat: agent reads context files and discovers skills locally (#23935)
Piggybacks on #23878. Moves instruction file reading and skill discovery from `chatd` (server-side, via multiple `LS`/`ReadFile` round-trips through the agent connection) to the agent itself (local filesystem access). This intentionally drops backward compatibility with older agents that don't support the context-config endpoint. Agents and server are deployed together; there is no rolling-update contract to maintain here. ## What changed The agent's `GET /api/v0/context-config` response now returns `[]ChatMessagePart` directly — the same types chatd persists. This eliminates intermediate type conversions and makes the protocol extensible. | Field | Type | Description | |---|---|---| | `parts` | `[]ChatMessagePart` | Context-file and skill parts, ready to persist | | `working_dir` | `string` | Agent's resolved working directory | Removed from the response: `instructions_dirs`, `instructions_file`, `skills_dirs`, `skill_meta_file`, `mcp_config_files` — the agent reads files locally and returns their content as parts. Removed from chatd: all legacy `LS`/`ReadFile` fallback code (`readHomeInstructionFile`, `readInstructionDirFile`, `DiscoverSkills` via LS, etc). ## Why The previous architecture had the agent resolve paths, serve them over HTTP, then `chatd` make N+1 round-trips back through the agent connection to read files. The agent has direct filesystem access and should just read the files. ## Key design decisions - **Agent returns `ChatMessagePart` directly** — same types chatd persists. No intermediate `InstructionFileEntry`/`SkillEntry` types needed. - **`SkillMeta.MetaFile`** — persisted via `ContextFileSkillMetaFile` on the skill part, so custom meta file names (`CODER_AGENT_EXP_SKILL_META_FILE`) survive across chat turns. - **No pre-read body** — `read_skill` always dials the workspace to fetch the skill body on demand. Simpler than caching the body in the response. - **MCP config paths kept agent-internal** — `MCPConfigFiles()` getter, not sent over the wire. - **No backward compat fallback** — old agents that don't support context-config get no instruction files. This is acceptable since agent and server deploy together. |
||
|
Kyle Carberry
|
7c048d8eb4 |
fix(site): fix "Thinking..." indicator disappearing prematurely (#23933)
The "Thinking..." indicator flickered or failed to appear when the user
sent a message.
## Problem
The server sends `status:pending` before `status:running` when
processing a new message. `selectIsAwaitingFirstStreamChunk` only
accepted `"running"`, so during the pending window the indicator was
hidden. When the optimistic `setChatStatus("running")` from `handleSend`
was overridden by the WS `status:pending` event, the indicator would
flash and disappear.
Secondarily, `StreamingOutput` hid the indicator as soon as
`streamState` became non-null, even when no text/reasoning blocks
existed yet (e.g. only tool-call parts or whitespace-only deltas had
arrived).
## Fix
1. **`chatStore.ts`** — `selectIsAwaitingFirstStreamChunk` now also
accepts `chatStatus === "pending"` when the latest durable message is a
user message (fresh send). Tool-call cycles (where latest =
assistant/tool) remain unaffected.
2. **`StreamingOutput.tsx`** — During streaming, the component keeps
showing "Thinking..." until a text or reasoning block appears, bridging
the visual gap between the startup placeholder and the first visible
content.
3. **`streamState.ts`** — Changed the early-return guard for
text/reasoning parts from `!part.text` to `!part.text?.trim()` so
whitespace-only deltas don't create a non-null `StreamState` with empty
blocks.
<details><summary>Decision log</summary>
- Including `"pending"` in `isAwaitingFirstStreamChunk` was previously
rejected because it caused the 15-second "startup taking longer" warning
during tool-call cycles. The `latestMessage?.role === "user"` guard now
prevents that — during tool cycles the latest durable message is
assistant/tool, not user.
- The `StreamingOutput` streaming-thinking check uses a synthetic
`"starting"` status for `ChatStatusCallout` rather than adding a new
phase to `LiveStatusModel`, keeping the status model clean.
- The whitespace trim fix in `streamState.ts` is defense-in-depth — the
`StreamingOutput` fix handles the rendering gap, but preventing
empty-block `StreamState` creation is the correct behavior at the
source.
</details>
|
||
|
Kyle Carberry
|
7dc81bdef1 |
fix(site): fix sticky user message clipping and fade-in behavior (#23928)
The sticky user message in the chat timeline had two visual issues: 1. **Dead space during scroll** — the clipping calculation subtracted 48px prematurely (`fullHeight - scrolledPast - 48`), causing the message to shrink before its content had actually left the viewport. Removed the offset so clipping begins exactly when content scrolls out of view. 2. **Blur/gradient popping in abruptly** — the `--fade-opacity` variable was a binary 0/1 toggle. Now it ramps 0→1 over the last 40px before `MIN_HEIGHT`, so the blur and bottom gradient only appear when the message is fully compressed. Also added a longer (~25 line) user message to the `WithMessageHistory` story to make the sticky behavior easier to test visually. |
||
|
Kyle Carberry
|
ee855f9618 |
feat: make agent context paths configurable via env vars (#23878)
Replace hardcoded paths for instruction files, skills, and MCP config
with
values read from `CODER_AGENT_EXP_*` environment variables. Template
authors
configure paths via the existing `coder_agent` `env` block. The agent
resolves `~`, relative, and absolute paths locally, then serves the
resolved config over `GET /api/v0/context-config`. `chatd` fetches this
once per workspace attach and falls back to today's defaults for older
agents.
All path env vars are comma-separated, allowing multiple directories:
| Env Var | Default | Controls |
|---|---|---|
| `CODER_AGENT_EXP_INSTRUCTIONS_DIRS` | `~/.coder` | Dirs containing the
instruction file |
| `CODER_AGENT_EXP_INSTRUCTIONS_FILE` | `AGENTS.md` | Instruction file
name |
| `CODER_AGENT_EXP_SKILLS_DIRS` | `.agents/skills` | Skills directories
|
| `CODER_AGENT_EXP_SKILL_META_FILE` | `SKILL.md` | Skill metadata file
name |
| `CODER_AGENT_EXP_MCP_CONFIG_FILES` | `.mcp.json` | MCP config files |
### Example
```hcl
resource "coder_agent" "main" {
os = "linux"
arch = "amd64"
env = {
CODER_AGENT_EXP_INSTRUCTIONS_DIRS = "/opt/company/agent-config,~/.coder"
CODER_AGENT_EXP_INSTRUCTIONS_FILE = "CLAUDE.md"
CODER_AGENT_EXP_SKILLS_DIRS = "/opt/company/ai-skills,.agents/skills"
CODER_AGENT_EXP_MCP_CONFIG_FILES = "/opt/company/mcp.json,.mcp.json"
}
}
```
<details>
<summary>Implementation Details</summary>
### Architecture
Follows the same pattern as MCP tool discovery:
agent resolves locally → exposes via HTTP → chatd consumes.
**Agent-side** (`agent/agentcontextconfig/`):
- `ResolvePath` / `ResolvePaths` handle `~`, relative, and absolute path
forms; returns `""` for relative paths when baseDir is empty
- `Config` reads env vars, falls back to defaults, resolves all paths
- `GET /api/v0/context-config` serves the resolved config as JSON
**chatd-side** (`coderd/x/chatd/`):
- Calls `conn.ContextConfig()` once on first workspace attach
- Falls back to hardcoded defaults on 404 (older agents)
- Iterates instruction dirs, skills dirs using resolved absolute paths
- `LSRelativityRoot` everywhere — no more home/root juggling
### Key design decisions
- **`EXP_` prefix**: env vars use `CODER_AGENT_EXP_*` to indicate
experimental status
- **Plural names**: comma-separated vars use plural names (`DIRS`,
`FILES`); single-value vars use singular (`FILE`)
- **Defaults in `workspacesdk`**: default constants live in
`codersdk/workspacesdk/` so both agent and server reference them without
cross-layer imports
- **`skillMetaFile` persistence**: stored on context-file parts via
`ContextFileSkillMetaFile` and restored on subsequent chat turns so
custom values survive across turns
- **Working dir dedup**: `slices.Contains` guard prevents reading the
same instruction file from both `InstructionsDirs` and the working
directory
- **MCP server dedup**: first-occurrence-wins dedup prevents leaking
duplicate connections from overlapping config files
- **ResolvePath safety**: returns `""` for relative paths when `baseDir`
is empty, so `ResolvePaths` filters them out
### Files changed
| File | Change |
|---|---|
| `agent/agentcontextconfig/` | New package — path resolution + HTTP
endpoint |
| `codersdk/workspacesdk/agentconn.go` | `ContextConfigResponse` type,
default constants, client method |
| `agent/agent.go` + `agent/api.go` | Wire up endpoint, pass config to
MCP |
| `agent/x/agentmcp/manager.go` | Accept `[]string` MCP config paths,
dedup by name |
| `coderd/x/chatd/chatd.go` | Fetch config, thread through, named
returns |
| `coderd/x/chatd/instruction.go` | Accept configurable dir + file name,
`skillMetaFileFromParts` |
| `coderd/x/chatd/chattool/skill.go` | Accept configurable dirs + meta
file |
| `codersdk/chats.go` | `ContextFileSkillMetaFile` field for persistence
|
### Test coverage
- `TestConfig` (4 cases): defaults, custom env vars, whitespace
trimming, comma-separated dirs
- `TestResolvePath` / `TestResolvePaths`: including empty baseDir edge
case
- `TestPersistInstructionFilesFallbackOnOlderAgent`: backward-compat
path when `ContextConfig` returns 404
- `TestChatMessagePartVariantTags`: updated exclusion list for new
internal field
### Backward compatibility
Older agents return 404 for the new endpoint. `chatd` catches this and
falls back to today's defaults via `readHomeInstructionFile` (using
`LSRelativityHome`). Existing workspaces work with no changes.
</details>
|
||
|
Kyle Carberry
|
8c8b307b97 | fix: persist session cookie to disk to prevent PWA logout (#23746) | ||
|
Kyle Carberry
|
19e44f4136 |
fix: target specific chat in MarkStale instead of broadcasting to all workspace chats (#23883)
## Problem Subagent chats were receiving git context (branch, remote origin, PR status) from their parent or sibling chats' git operations. When a git operation triggers external auth, the workspace agent sends `chat_id` identifying which chat initiated it — but this was broken at two levels: 1. **Agent side:** `CODER_CHAT_ID` was never injected into process environments. `chatd` sets `Coder-Chat-Id` HTTP headers and the agent extracts them for process isolation, but never propagated `CODER_CHAT_ID` to `cmd.Env`. So `gitaskpass` always sent an empty `chat_id`. 2. **Server side:** `workspaceAgentsExternalAuth` ignored the `chat_id` query param. `MarkStale` broadcast git context to **all** chats on the workspace via `filterChatsByWorkspaceID`. ## Fix - Inject `CODER_CHAT_ID` into `cmd.Env` in `agentproc` when the chat ID is known, so `gitaskpass` can read and forward it. - Read `chat_id` from query params in `workspaceAgentsExternalAuth` and thread it through `chatGitRef`. - Refactor `MarkStale` to accept a `MarkStaleParams` struct. When `ChatID` is provided, target only that specific chat. When empty (legacy agents, non-chat git operations), fall back to the existing workspace-wide broadcast. - Extract `markStaleSingle` helper to deduplicate the upsert+publish logic. <details><summary>Investigation notes</summary> ### Data flow before fix ``` chatd → sets Coder-Chat-Id header on agent conn agent → extracts chatID, stores on process struct agent → does NOT set CODER_CHAT_ID in cmd.Env ← gap 1 gitaskpass → reads CODER_CHAT_ID (always empty), sends chat_id="" server handler → ignores chat_id query param ← gap 2 MarkStale → broadcasts to ALL workspace chats ``` ### Data flow after fix ``` chatd → sets Coder-Chat-Id header on agent conn agent → extracts chatID, stores on process struct agent → sets CODER_CHAT_ID in cmd.Env gitaskpass → reads CODER_CHAT_ID, sends chat_id=<uuid> server handler → reads chat_id, passes to MarkStale MarkStale → targets only that specific chat ``` </details> |
||
|
Kyle Carberry
|
2ea89e1f1b |
fix(site/src/pages/AgentsPage): show Thinking indicator immediately after sending a message (#23904)
After sending a message, `handleSend` clears stream state and inserts the user message but did not set `chatStatus` to `"running"`. Combined with #23805 narrowing `selectIsAwaitingFirstStreamChunk` to only match `chatStatus === "running"` (instead of `isActiveChatStatus` which included `"pending"`), the "Thinking..." indicator could not appear until the WebSocket delivered `status:running` — a 50–500ms+ gap. Optimistically set `chatStatus` to `"running"` in both the send and edit paths after the POST returns (non-queued). The WebSocket `status:running` event no-ops via the `setChatStatus` guard; error/pending events override the optimistic value. <details><summary>Investigation & decision log</summary> ### Root cause chain 1. **PR #23805** (`953c3bdc0`) changed `selectIsAwaitingFirstStreamChunk` from `isActiveChatStatus(state.chatStatus)` → `state.chatStatus === "running"`. Valid fix: during `"pending"`, `shouldApplyMessagePart()` drops stream parts, so `streamState` stays null and the 15s "startup taking too long" warning fired spuriously during multi-turn tool-call cycles. 2. **PR #23884** (`4b5265695`) fixed event ordering within a WebSocket batch so both `[message_part, status:running]` and `[status:running, message_part]` orderings show "Thinking...". Correct fix, but only operates **after** `chatStatus` reaches `"running"`. 3. `handleSend` never set `chatStatus` optimistically — it relied entirely on the WebSocket `status:running` event. After #23805 narrowed the selector, the gap between POST completion and WebSocket event became visible. ### Why this fix is safe - Non-queued POST = server accepted the message → `"running"` is the correct next state. - `setChatStatus("running")` guard: `if (state.chatStatus === status) return` makes the subsequent WebSocket confirmation a no-op. - If the server transitions to error/pending instead, the WebSocket event overrides the optimistic value. - `shouldApplyMessagePart()` returns `true` for `"running"`, so early stream parts arriving before the WebSocket `status:running` will not be silently dropped. ### What was NOT regressed by PR #23884 PR #23884's `setTimeout(0)` deferred flush is correct. Both event orderings now produce a render cycle where `chatStatus === "running"` and `streamState === null`, allowing "Thinking..." to appear. The `setTimeout(0)` fires in a separate macrotask, giving the browser a paint opportunity. </details> |
||
|
Kyle Carberry
|
7861fcf1f6 |
perf(coderd): stop inline-resolving diff status on every GetChat call (#23901)
## Problem
Every `GET /api/experimental/chats/{chatID}` call was blocking for
200-800ms because the `getChat` handler called `resolveChatDiffStatus`,
which unconditionally hit the git provider API (e.g. GitHub's `GET
/repos/{owner}/{repo}/pulls?head=...`) via `ResolveBranchPullRequest` —
even when the cached diff status was fresh.
This made every chat page load at `/agents/{id}` noticeably slow.
## Root cause
The call chain was:
1. `getChat` → `resolveChatDiffStatus`
2. `resolveChatDiffStatus` → `resolveChatDiffReference` →
`gp.ResolveBranchPullRequest(...)` **(external HTTP call)**
3. Only **after** the external call: `chatDiffStatusIsStale(status,
now)` check
The staleness check happened after the expensive work, so every request
paid the cost regardless of cache freshness.
## Fix
`getChat` now returns the cached `chat_diff_statuses` row directly from
the database. The background `gitsync` worker already keeps these rows
fresh (every `DiffStatusTTL = 120s`), so inline resolution was
redundant.
The `resolveChatDiffContents` endpoint (which fetches actual diff
content) still uses the full resolution path since it needs to make
provider API calls by design.
## Changes
- `getChat` reads cached diff status from DB instead of calling
`resolveChatDiffStatus`
- Remove `resolveChatDiffStatus` (dead code — no production callers)
- Remove `chatDiffStatusIsStale` and `chatDiffStatusTTL` (dead code)
- Remove `RefreshesStaleStatusWithExternalAuth` test (tested the removed
inline refresh path)
<details><summary>Decision log</summary>
- **Why not just add a staleness gate?** The background worker already
handles refreshes on the same schedule. Adding an early-return-if-fresh
would work but leaves dead code for the stale path that's never
exercised in production (the worker gets there first). Removing the
inline path entirely is simpler and eliminates the external API
dependency from the read path.
- **Why keep `resolveChatDiffContents` unchanged?** That endpoint's job
is to fetch the actual diff content from the provider, so external API
calls are inherent to its purpose.
</details>
|
||
|
Kyle Carberry
|
4b52656958 |
fix(site): ensure Thinking indicator appears regardless of WebSocket event ordering (#23884)
The "Thinking..." indicator intermittently failed to render after submitting a message. The behavior depended on the order of events within a single WebSocket frame. ## Root Cause `flushMessageParts()` was called before **all** non-`message_part` events in the batch loop. When the server sent `[message_part, status:"running"]` in the same SSE chunk: 1. `message_part` → pushed to `partsBuf` 2. `status:"running"` → `flushMessageParts()` applied parts **first** → `streamState` became non-null → then `chatStatus` set to `"running"` 3. Subscriber saw `streamState != null && chatStatus == "running"` → `selectIsAwaitingFirstStreamChunk` returned `false` → no "Thinking..." When events arrived in the reverse order (`[status:"running", message_part]`), the indicator worked because the status was set before parts were applied. ## Fix - Move `flushMessageParts()` to only fire before `message` and `error` events (which need prior parts visible) - Add `discardBufferedParts()` for events that clear stream state (`status:"pending"/"waiting"`, `retry`) so the deferred `setTimeout(0)` flush doesn't re-populate cleared state - Status changes are now always applied before parts within a batch, and the deferred flush gives React one render cycle to show "Thinking..." | Event | Flush? | Rationale | |---|---|---| | `message` | YES | Durable commit must include all stream parts | | `error` | YES | Partial output should be visible alongside error | | `status` | NO | Status must be set before parts so "starting" phase renders | | `retry` | DISCARD | Retry clears stream state; flushing would re-populate it | | `queue_update` | NO | Doesn't interact with stream state | ## Tests (written first, failing before fix) 1. **"shows starting phase when message_part arrives before status:running in same batch"** — the exact bug scenario 2. **"shows starting phase when status:running arrives before message_part in same batch"** — verifies the "good" order still works 3. **"discards buffered parts when status transitions to pending"** — verifies parts don't leak through pending transitions All tests are deterministic (fake timers, no race conditions). <details><summary>Implementation plan & decision log</summary> ### Why not reorder events within the batch? Reordering would change the semantic ordering of events from the server, which could have subtle side effects. The simpler approach is to be selective about when parts are flushed. ### Why discard (not flush) before pending/waiting/retry? These events clear `streamState`. If parts were flushed before the clear, they'd be visible for one frame then disappear. If the deferred flush ran after the clear, it would re-populate the state. Discarding is the only correct behavior. ### Why keep flush before error? Errors should surface partial output so the user can see what the agent was doing when it failed. </details> |
||
|
Kyle Carberry
|
2c5e003c91 |
refactor(site): use hover popover for context indicator with nested skill tooltips (#23870)
Replaces the tooltip-inside-tooltip approach for the context usage
indicator with a hover-based Popover. Skill descriptions now appear as
nested tooltips to the right, matching the ModelSelector pattern.
**Before**: Tooltip with inline skill descriptions (truncated, janky
nested tooltips)
**After**: Popover opens on hover, skill names listed cleanly,
descriptions appear to the right on hover
- Popover opens on `mouseEnter`, closes after 150ms delay on
`mouseLeave`
- `onOpenAutoFocus` prevented to avoid stealing chat input focus
- Mobile keeps tap-to-toggle Popover behavior
- Skill rows get subtle `hover:bg-surface-tertiary` highlight
- `TooltipProvider` with `delayDuration={300}` wraps skill items (same
as ModelSelector)
|
||
|
Kyle Carberry
|
2953245862 |
feat(site): display loaded context files and skills in context indicator tooltip (#23853)
Renders the `last_injected_context` data (AGENTS.md files and skills) from the Chat API in the `ContextUsageIndicator` hover tooltip. On hover, users now see: - **Context files**: basename with full path on title hover, truncation indicator - **Skills**: name and optional description Separated from the existing token usage info by a border divider when both sections are present. Added `max-w-72` to prevent the tooltip from getting too wide. <img width="970" height="598" alt="image" src="https://github.com/user-attachments/assets/5bc25cb2-1d92-41d2-ab1a-63e5e49f667a" /> <details> <summary>Data flow</summary> ``` chatQuery.data.last_injected_context → AgentChatPage (AgentChatPageView prop) → AgentChatPageView (ChatPageInput prop) → ChatPageInput (spread into latestContextUsage) → AgentChatInput (contextUsage prop) → ContextUsageIndicator (usage.lastInjectedContext) ``` </details> <details> <summary>Files changed</summary> | File | Change | |---|---| | `ContextUsageIndicator.tsx` | Add `lastInjectedContext` to interface, render context files and skills sections in tooltip | | `ChatPageContent.tsx` | Thread `lastInjectedContext` prop, spread into context usage object | | `AgentChatPageView.tsx` | Thread `lastInjectedContext` prop to `ChatPageInput` | | `AgentChatPage.tsx` | Pass `chatQuery.data?.last_injected_context` down | </details> |
||
|
Kyle Carberry
|
b3d5b8d13c |
fix: stabilize flaky chatd subscribe/promote queued tests (#23816)
## Summary Fixes three flaky chatd tests that intermittently fail due to timing races with the background run loop. Closes coder/internal#1428 ## Root Cause `CreateChat` and `PromoteQueued` call `signalWake()` which writes to `wakeCh`, triggering `processOnce` immediately. Even though `newTestServer` sets `PendingChatAcquireInterval: testutil.WaitLong` to prevent ticker-based polling, the wake channel bypasses this. This causes `processOnce` to acquire and process the chat concurrently with the test's manual DB updates and assertions. ### Failing tests | Test | Failure | Cause | |------|---------|-------| | `TestPromoteQueuedAllowsAlreadyQueuedMessageWhenUsageLimitReached` | `expected: "pending", actual: "running"` | Wake from `CreateChat` races with manual `UpdateChatStatus`; wake from `PromoteQueued` acquires the chat before the status assertion | | `TestSendMessageInterruptBehaviorQueuesAndInterruptsWhenBusy` | `should have 1 item(s), but has 2` | Wake from `CreateChat` triggers `processChat` which auto-promotes a queued message, adding an extra row to `chat_messages` | | `TestSubscribeNoPubsubNoDuplicateMessageParts` | `Condition satisfied` (duplicate events) | Pre-existing `WaitGroup.Add/Wait` race in the `Eventually` + `WaitUntilIdleForTest` pattern | ## Fix Introduces a `waitForChatProcessed` helper that: 1. Polls until the chat reaches a **terminal state** (not pending AND not running) 2. Then calls `WaitUntilIdleForTest` to wait for the inflight `WaitGroup` Waiting for a terminal state (not just "not pending") avoids a `sync.WaitGroup` `Add/Wait` race: `AcquireChats` updates the DB status to `running` **before** `processOnce` calls `inflight.Add(1)`. Checking only `status != pending` could return while `Add(1)` hasn't happened yet, causing `Wait()` to return prematurely. ### Per-test changes - **`TestSendMessageInterruptBehaviorQueuesAndInterruptsWhenBusy`**: Call `waitForChatProcessed` after `CreateChat` before manually setting running status - **`TestPromoteQueuedAllowsAlreadyQueuedMessageWhenUsageLimitReached`**: Call `waitForChatProcessed` after `CreateChat`; remove the inherently racy `status == pending` assertion after `PromoteQueued` (the wake immediately acquires the chat). Key assertions on promoted message, queue state, and message count remain. - **`TestSubscribeNoPubsubNoDuplicateMessageParts`**: Replace inline `Eventually` with the safer `waitForChatProcessed` helper ## Verification All three tests pass 150 consecutive executions with `-race -count=10` across 15 runs (0 failures). |
||
|
Kyle Carberry
|
a5cc579453 |
feat: add last_injected_context column to chats table (#23798)
Adds a nullable JSONB column `last_injected_context` to the `chats` table that stores the most recently persisted injected context parts (AGENTS.md context-file and skill message parts). The column is updated only when `persistInstructionFiles()` runs — on first workspace attach or when the agent changes — so there are no redundant writes on subsequent turns. Internal fields (`ContextFileContent`, `ContextFileOS`, `ContextFileDirectory`, `SkillDir`) are stripped at write time so the column only holds small metadata. No stripping needed on the read path. <details> <summary>Implementation notes</summary> - New migration `000456` adds nullable `last_injected_context JSONB` column. - New SQL query `UpdateChatLastInjectedContext` writes the column without touching `updated_at`. - `persistInstructionFiles()` strips internal fields from parts via `StripInternal()` before persisting. - Sentinel path (no AGENTS.md) persists skill-only parts when skills exist. - `codersdk.Chat` exposes `LastInjectedContext []ChatMessagePart` (omitempty). - `db2sdk.Chat()` passes through the already-clean data. </details> |
||
|
Kyle Carberry
|
953c3bdc0f |
fix(site): prevent spurious startup warning during pending status (#23805)
## Problem The `/agents` page frequently shows "Response startup is taking longer than expected" even while the agent is actively working and messages are appearing in the transcript. ## Root Cause There's an inconsistency between `isActiveChatStatus` and `shouldApplyMessagePart` during `"pending"` status (the state between agent tool-call turns): | Component | Treats `"pending"` as... | |---|---| | `isActiveChatStatus` | **active** — includes both `"running"` and `"pending"` | | `shouldApplyMessagePart` | **inactive** — drops all `message_part` events during `"pending"` | | Status handler | clears `streamState` to `null` on `"pending"` | This creates a dead state during multi-turn tool-call cycles: 1. Agent finishes a turn → status = `"pending"` → `streamState` cleared to `null` 2. `selectIsAwaitingFirstStreamChunk` returns `true` (status is "active", stream is null, latest message isn't assistant) 3. Phase = `"starting"` → 15s timer starts 4. Stream parts from the server are **silently dropped** (`shouldApplyMessagePart()` returns `false` for `"pending"`) 5. `streamState` stays `null` — phase is stuck at `"starting"` 6. Meanwhile, durable messages (tool calls, tool results) appear normally in the transcript 7. After 15s → "Response startup is taking longer than expected" fires ## Fix Narrow `selectIsAwaitingFirstStreamChunk` to only check `chatStatus === "running"` instead of `isActiveChatStatus(chatStatus)`. `"running"` is the only status where the transport actually accepts stream parts, so it's the only status where we should be showing the "starting" indicator. `isActiveChatStatus` is left unchanged since its other caller (`shouldSurfaceReconnectState`) correctly needs to include `"pending"`. |
||
|
Kyle Carberry
|
4d2b0a2f82 |
feat: persist skills as message parts like AGENTS.md (#23748)
## Summary Skills are now discovered once on the first turn (or when the workspace agent changes) and persisted as `skill` message parts alongside `context-file` parts. On subsequent turns, the skill index is reconstructed from persisted parts instead of re-dialing the workspace agent. This makes skills consistent with the AGENTS.md pattern and is groundwork for a future `/context` endpoint that surfaces loaded workspace context to the frontend. ## Changes - Add `skill` `ChatMessagePartType` with `SkillName` and `SkillDescription` fields - Extend `persistInstructionFiles` to also discover and persist skills as parts - Add `skillsFromParts()` to reconstruct skill index from persisted parts on subsequent turns - Update `runChat()` to use `skillsFromParts` instead of re-dialing workspace for skills - Frontend: handle new `skill` part type (skip rendering, hide metadata-only messages) ## Before / After | | AGENTS.md | Skills | |---|---|---| | **Before** | Persist as `context-file` parts, reconstruct from parts | In-memory `skillsCache` only, re-dial workspace on cache miss | | **After** | Persist as `context-file` parts, reconstruct from parts | Persist as `skill` parts, reconstruct from parts | The in-memory `skillsCache` remains for `read_skill`/`read_skill_file` tool calls that need full skill bodies on demand. <details><summary>Design context</summary> This is the first step toward a unified workspace context representation. Currently: - Context files are persisted as message parts (works) - Skills were only in-memory (inconsistent) - Workspace MCP servers are cached in-memory (future work) Persisting skills as parts means a future `/context` endpoint can query both context files and skills from the same message parts in the DB, without depending on ephemeral server-side caches. </details> |
||
|
Kyle Carberry
|
be99b3cb74 |
fix: prioritize context cancellation in WebSocket sendEvent (#23756)
## Problem
Commit
|
||
|
Kyle Carberry
|
386b449273 |
perf(coderd): reduce chat streaming latency with event-driven acquisition (#23745)
Previously, when a user sent a message, there was a 0–1000ms (avg ~500ms) polling delay before processing began. `SendMessage`/`CreateChat`/`EditMessage` set `status='pending'` in the DB and returned, but nothing woke the processing loop — it was a blind 1-second ticker. ## Changes **Event-driven acquisition (main change):** Adds a `wakeCh` channel to the chatd `Server`. `CreateChat`, `SendMessage`, `EditMessage`, and `PromoteQueued` call `signalWake()` after committing their transactions, which wakes the run loop to call `processOnce` immediately. The 1-second ticker remains as a fallback safety net for edge cases (stale recovery, missed signals). **Buffer WebSocket write channel:** Changes the `OneWayWebSocketEventSender` event channel from unbuffered to buffered (64), decoupling the event producer from WebSocket write speed. The existing 10s write timeout guards against stuck connections. <details><summary>Implementation plan & analysis</summary> The full latency analysis identified these sources of delay in the streaming pipeline: 1. **Chat acquisition polling** — 0–1000ms (avg 500ms) dead time per message. Fixed by wake channel. 2. **Unbuffered WebSocket write channel** — each token blocked on the previous WS write completing. Fixed by buffering. 3. **PersistStep DB transaction per step** — `FOR UPDATE` lock + batch insert. Not addressed in this PR (medium risk, would overlap DB write with next provider TTFB). 4. **Multi-hop channel pipeline** — 4 channel hops per token. Not addressed (medium complexity). </details> <details><summary>Test stabilization notes</summary> `signalWake()` causes the chatd daemon to process chats immediately after creation/send/edit, which exposed timing assumptions in several tests that expected chats to remain in `pending` status long enough to assert on. These tests were updated with `require.Eventually` + `WaitUntilIdleForTest` patterns to wait for processing to settle before asserting. The race detector (`test-go-race-pg`) shows failures in `TestCreateWorkspaceTool_EndToEnd` and `TestAwaitSubagentCompletion` — these appear to be pre-existing races in the end-to-end chat flow that are now exercised more aggressively because processing starts immediately instead of after a 1s delay. Main branch CI (race detector) passes without these changes. </details> |
||
|
Kyle Carberry
|
a2799560eb |
fix: use Popover for context indicator on mobile viewports (#23747)
## Problem The context-usage indicator ring in the agents chat uses a Radix UI `Tooltip`, which only opens on hover. On mobile/touch devices there is no hover event, so tapping the indicator does nothing. ## Fix On mobile viewports (`< 640 px`, matching the existing `isMobileViewport()` helper), render a `Popover` instead of a `Tooltip` so that tapping the ring toggles the context-usage info. Desktop behavior (hover tooltip) is unchanged. - Extract the trigger button and content into shared variables to avoid duplication - Conditionally render `Popover` (mobile) or `Tooltip` (desktop) based on viewport width - Both `Popover` and `PopoverContent` were already imported in the file |
||
|
Kyle Carberry
|
a708e9d869 |
feat: add tool rendering for read_skill, read_skill_file, start_workspace (#23744)
These tools previously fell through to the `GenericToolRenderer` which
showed a wrench icon and the raw tool name. Now they get dedicated icons
and contextual labels.
## Changes
**ToolIcon.tsx** — new icon mappings:
- `read_skill` / `read_skill_file` → `BookOpenIcon`
- `start_workspace` → `PlayIcon`
- `web_search` → `SearchIcon`
**ToolLabel.tsx** — contextual labels:
- `read_skill`: "Reading skill {name}…" → "Read skill {name}"
- `read_skill_file`: "Reading {skill}/{path}…" → "Read {skill}/{path}"
- `start_workspace`: "Starting workspace…" → "Started {name}"
- `web_search`: "Searching \"{query}\"…" → "Searched \"{query}\""
**tool.stories.tsx** — 12 new stories covering running, completed, and
error states for all four tools.
<details>
<summary>Visually verified in Storybook</summary>
All 10 story variants verified:
- ReadSkillRunning / ReadSkillCompleted / ReadSkillError
- ReadSkillFileRunning / ReadSkillFileCompleted / ReadSkillFileError
- StartWorkspaceRunning / StartWorkspaceCompleted / StartWorkspaceError
- WebSearchRunning / WebSearchCompleted / WebSearchNoQuery
</details>
|
||
|
Kyle Carberry
|
839165818b |
feat(coderd/x/chatd): add skills discovery and tools for chatd (#23715)
Adds skill discovery and tools to chatd so the agent can discover and load `.agents/skills/` from workspaces, following the same pattern as AGENTS.md instruction loading and MCP tool discovery. ## What changed ### `chattool/skill.go` — discovery, loading, and tools - **DiscoverSkills** — walks `.agents/skills/` via `conn.LS()` + `conn.ReadFile()`, parses SKILL.md frontmatter (name + description), validates kebab-case names match directory names, silently skips broken/missing entries. - **FormatSkillIndex** — renders a compact `<available-skills>` XML block for system prompt injection (~60 tokens for 3 skills). Progressive disclosure: only names + descriptions in context, full body loaded on demand. - **LoadSkillBody** / **LoadSkillFile** — on-demand loading with path traversal protection and size caps (64KB for SKILL.md, 512KB for supporting files). - **read_skill** / **read_skill_file** tools — `fantasy.AgentTool` implementations following the same pattern as ReadFile and WorkspaceMCPTool. Receive pre-discovered `[]SkillMeta` via closure to avoid re-scanning on every call. ### `chatd.go` — integration into runChat - Skills discovered in the `g2` errgroup parallel with instructions and MCP tools. - `skillsCache` (sync.Map) per chat+agent, same invalidation pattern as MCP tools cache. - Skill index injected via `InsertSystem` after workspace instructions. - Re-injected in `ReloadMessages` callback so it survives compaction. - `read_skill` + `read_skill_file` tools registered when skills are present (for both root and subagent chats). - Cache cleaned up in `cleanupStreamIfIdle` alongside MCP tools cache. ## Format compatibility Uses the same `.agents/skills/<name>/SKILL.md` format as [coder/mux](https://github.com/coder/mux) and [openai/codex](https://github.com/openai/codex). |
||
|
Kyle Carberry
|
6b77fa74a1 |
fix: remove bold font-weight from unread chats in agents sidebar (#23725)
Removes the `font-semibold` class applied to unread chat titles in the `/agents` sidebar. The unread indicator dot already provides sufficient visual distinction for unread chats. |
||
|
Kyle Carberry
|
bcdc35ee3e |
feat: add chat read/unread indicator to sidebar (#23129)
## Summary Adds read/unread tracking for chats so users can see which agent conversations have new assistant messages they haven't viewed. ## Backend Changes - Adds `last_read_message_id` column to the `chats` table (migration 000439). - Computes `has_unread` as a virtual column in `GetChatsByOwnerID` using an `EXISTS` subquery checking for assistant messages beyond the read cursor. - Exposes `has_unread` on the `codersdk.Chat` struct and auto-generated TypeScript types. - Updates `last_read_message_id` on stream connect/disconnect in `streamChat`, avoiding per-message API calls during active streaming. - Uses `context.WithoutCancel` for the deferred disconnect write so the DB update succeeds even after the client disconnects. ## Frontend Changes - Bold title (`font-semibold`) for unread chats in the sidebar. - Small blue dot indicator next to the relative timestamp. - Suppresses unread indicator for the currently active chat via `isActive` from NavLink. ## Design Decisions - Only `assistant` messages count as unread — the user's own messages don't trigger the indicator. - No foreign key on `last_read_message_id` since messages can be deleted (via rollback/truncation) and the column is just a high-water mark. - Zero API calls during streaming: exactly 2 DB writes per stream session (connect + disconnect). - Unread state refreshes on chat list load and window focus. The `watchChats` WebSocket optimistically marks non-active chats as unread on `status_change` events, but does not carry a server-computed `has_unread` field. Navigating to a chat optimistically clears its unread indicator in the cache. |
||
|
Kyle Carberry
|
4ed9094305 |
perf(site): memoize chat rendering hot path (#23720)
Addresses chat page rendering performance. Profiling with React Profiler showed `AgentChat` actual render times of 20–31ms (exceeding the 16ms/60fps budget), with `StickyUserMessage` as the #1 component bottleneck at 35.7% of self time. ## Changes **Hoist `createComponents` to module scope** (`response.tsx`): Previously every `<Response>` instance called `createComponents()` per render, creating a fresh components map that forced Streamdown to discard its cached render tree. Now both light/dark variants are precomputed once at module scope. **Wrap `StickyUserMessage` in `memo()`** (`ConversationTimeline.tsx`): Profile-confirmed #1 bottleneck. Each instance carries IntersectionObserver + ResizeObserver + scroll handlers; skipping re-render avoids all that setup. **Wrap `ConversationTimeline` in `memo()`** (`ConversationTimeline.tsx`): Prevents cascade re-renders from the parent when props haven't changed. **Remove duplicate `buildSubagentTitles`** (`ConversationTimeline.tsx` → `AgentDetailContent.tsx`): Was computed in both `AgentDetailTimeline` and `ConversationTimeline`. Now computed once and passed as a prop. <details> <summary>Profiling data & analysis</summary> ### Profiler Metrics | Metric | Value | |--------|-------| | INP (Interaction to Next Paint) | 82ms | | Processing duration (event handlers) | 52ms | | AgentChat actual render | 20–31ms (budget: 16ms) | | AgentChat base render (no memo) | ~100ms | ### Top Bottleneck Components (self-time %) | Component | Self Time | % | |-----------|-----------|---| | StickyUserMessage | 11.0ms | 35.7% | | ForwardRef (radix-ui) | 7.4ms | 24.0% | | Presence (radix-ui) | 2.0ms | 6.5% | | AgentChatInput | 1.4ms | 4.5% | ### Decision log - Chose module-scope precomputation over `useMemo` for `createComponents` because there are only two possible theme variants and they're static. - Did not add virtualization — sticky user messages + scroll anchoring make it complex. The memoization fixes should be measured first. - Did not wrap `BlockList` in `memo()` — the React Compiler (enabled for `pages/AgentsPage/`) already auto-memoizes JSX elements inside it. - Phase 2 (verify React Compiler effectiveness on `parseMessagesWithMergedTools`) and Phase 3 (radix-ui Tooltip lazy-mounting) deferred to follow-up PRs. </details> |
||
|
Kyle Carberry
|
d973a709df |
feat: add model_intent option to MCP server configs (#23717)
Add a per-MCP-server `model_intent` toggle that wraps tool schemas with
a
`model_intent` field, requiring the LLM to provide a human-readable
description of each tool call's purpose. The intent string is shown as a
status label in the UI instead of opaque tool names, and is
transparently
stripped before the call reaches the remote MCP server.
Built-in tools have rich specialized renderers (terminal blocks, file
diffs,
etc.) and don't need this. MCP tools hit `GenericToolRenderer` which
only
shows raw tool names and JSON — that's where model_intent adds value.
The model learns what to provide via the JSON Schema `description` on
the
`model_intent` property itself — no system prompt changes needed.
<details>
<summary>Implementation details</summary>
### Architecture
Inspired by the `withModelIntent()` pattern from `coder/blink`, adapted
for
Go + React. The wrapping is entirely in the `mcpclient` layer — tool
implementations never see `model_intent`.
**Schema wrapping** (`mcpToolWrapper.Info()`): When enabled, wraps the
original tool parameters under a `properties` key and adds a
`model_intent`
string field with a rich description that teaches the model inline.
**Input unwrapping** (`mcpToolWrapper.Run()`): Strips `model_intent` and
unwraps `properties` before forwarding to the remote MCP server. Handles
three input shapes models may produce:
1. `{ model_intent, properties: {...} }` — correct format
2. `{ model_intent, key: val, ... }` — flat, no wrapper
3. Malformed — falls through gracefully
**Frontend extraction**: `streamState.ts` extracts `model_intent` from
incrementally parsed streaming JSON. `messageParsing.ts` extracts it
from
persisted tool call args.
**UI rendering**: `GenericToolRenderer` shows the capitalized intent
string
as the primary label when available, falling back to the raw tool name.
### Changes
- Database: `model_intent` boolean column on `mcp_server_configs`
- SDK: `ModelIntent` field on config/create/update types
- API: pass-through in create/update handlers + converter
- mcpclient: schema wrapping in `Info()`, input unwrapping in `Run()`
- Frontend: extraction from streaming + persisted args
- UI: intent label in `GenericToolRenderer`, toggle in admin panel
- Tests: 6 new tests (schema wrapping, unwrapping, passthrough,
fallback)
### Decision log
- **Option lives on MCPServerConfig, not model config**: Built-in tools
already have rich renderers; only MCP tools benefit from model_intent.
- **No system prompt changes**: The JSON Schema `description` on the
`model_intent` property teaches the model inline.
- **Pointer bool on update request**: Follows existing pattern (`*bool`)
so PATCH requests don't reset the value when omitted.
</details>
|
||
|
Kyle Carberry
|
50c0c89503 |
fix(coderd): refresh expired MCP OAuth2 tokens everywhere (#23713)
Fixes expired MCP OAuth2 tokens causing 401 errors and stale `auth_connected` status in the UI. When users authenticate MCP servers (e.g. GitHub) via OAuth2, the access token and refresh token are stored in the database. However, when the access token expired, nothing refreshed it anywhere: - **chatd**: sent the expired token as-is, getting a 401 and skipping the MCP server - **list/get endpoints**: reported `auth_connected: true` just because a token record existed, regardless of expiry ## Changes ### Shared utility: `mcpclient.RefreshOAuth2Token` Pure function that uses `golang.org/x/oauth2` `TokenSource` to check if a token is expired (or within 10s of expiry) and refresh it. No DB dependency — callers handle persistence. ### chatd (`coderd/x/chatd/chatd.go`) Before calling `mcpclient.ConnectAll`, refreshes expired tokens. Persists new credentials to the database. Falls back to the old token if refresh fails. ### List/get MCP server endpoints (`coderd/mcp.go`) Both `listMCPServerConfigs` and `getMCPServerConfig` now attempt refresh when checking `auth_connected`. If the token is expired: - **Has refresh token**: attempt refresh, persist result, report `auth_connected` based on success - **No refresh token**: report `auth_connected: false` if expired This means the UI accurately reflects whether the user's token is actually usable, rather than just whether a record exists. <details> <summary>Design notes</summary> - `RefreshOAuth2Token` lives in `mcpclient` to avoid circular imports (`coderd` → `chatd` → `mcpclient` is fine; `chatd` → `coderd` would be circular). - DB persistence is handled by each caller with their own authz context (`AsSystemRestricted` in both cases). - The `buildAuthHeaders` warning in mcpclient about expired tokens is kept as defense-in-depth logging. </details> |
||
|
Kyle Carberry
|
0f86c4237e |
feat: add workspace MCP tool discovery and proxying for chat (#23680)
Coder's chat (chatd) can now discover and use MCP servers configured in a workspace's `.mcp.json` file. This brings project-specific tooling (GitHub, databases, docs servers, etc.) into the chat without any manual configuration. ## How it works The workspace agent reads `.mcp.json` from the workspace directory (same format Claude Code uses), connects to the declared MCP servers — spawning child processes for stdio servers and connecting over the network for HTTP/SSE — and caches their tool lists. Two new agent HTTP endpoints expose this: - `GET /api/v0/mcp/tools` returns the cached tool list (supports `?refresh=true`) - `POST /api/v0/mcp/call-tool` proxies calls to the correct server On each chat turn, chatd calls `ListMCPTools` through the existing `AgentConn` tailnet connection, wraps each tool as a `fantasy.AgentTool`, and adds them to the LLM's tool set alongside built-in and admin-configured MCP tools. Tool names are prefixed with the server name (`github__create_issue`) to avoid collisions. Failed server connections are logged and skipped — they never block the agent or break the chat. Child stdio processes are terminated on agent shutdown. |
||
|
Kyle Carberry
|
61d2a4a9b8 |
fix(site): preserve streaming output when queued message is sent (#23595)
## Problem
When the user sends a message while the agent is actively streaming a
response, `handleSend` called `store.clearStreamState()`
**unconditionally before** the POST request. If the server queues the
message (`response.queued = true` because the agent is busy), the
in-progress stream output is immediately wiped from the UI. The full
text only reappears once the agent finishes and the durable message
arrives via WebSocket — causing a visible cutoff mid-stream.
## Fix
Move `clearStreamState()` from before the POST to **after** the
response, gated behind `!response.queued`:
- **Queued sends** (`response.queued === true`): `clearStreamState()` is
never called. The stream continues uninterrupted. The WebSocket `status`
handler already clears stream state when the chat transitions to
`"pending"` / `"waiting"` after the queued message is dequeued.
- **Non-queued sends** (`response.queued === false`):
`clearStreamState()` + `upsertDurableMessage()` fire immediately after
the POST, same net behavior as before.
- **Edit and promote paths**: Unchanged — those are intentional
interruptions where eager clearing is correct.
### Additional behavior changes (both improvements)
1. **Failed sends no longer wipe stream state.** Previously
`clearStreamState()` ran before the `try` block, so a network error
still wiped the agent's in-progress output. Now the `catch` re-throws
before reaching `clearStreamState()`, preserving the stream on failure.
2. **`clearStreamState()` fires for all non-queued responses**, not just
those with a `message` body. The original guard was `!response.queued &&
response.message`; now `clearStreamState()` is under `!response.queued`
while `upsertDurableMessage` retains the `response.message` check. The
server always sets `message` for non-queued responses, so this is a
no-op in practice but is semantically correct.
## Testing
**AgentDetail.stories.tsx**: New `StreamingSurvivesQueuedSend` story
exercises the full flow — mocks `createChatMessage` to return `{ queued:
true }`, delivers streaming text via WebSocket, sends a message through
the UI, and asserts the streaming text remains visible.
|
||
|
Kyle Carberry
|
09bcd0b260 |
fix: revert "refactor(site/src/pages/AgentsPage): normalize transcript scrolling" (#23638)
Reverts coder/coder#23576 |
||
|
Kyle Carberry
|
1f13324075 |
fix(coderd): use path-aware discovery for MCP OAuth2 metadata (RFC 9728, RFC 8414) (#23520)
## Problem
MCP OAuth2 auto-discovery stripped the path component from the MCP
server URL
before looking up Protected Resource Metadata. Per RFC 9728 §3.1, the
well-known
URL should be path-aware:
```
{origin}/.well-known/oauth-protected-resource{path}
```
For `https://api.githubcopilot.com/mcp/`, the correct metadata URL is
`https://api.githubcopilot.com/.well-known/oauth-protected-resource/mcp/`,
not
`https://api.githubcopilot.com/.well-known/oauth-protected-resource`
(which
returns 404).
The same issue applied to RFC 8414 Authorization Server Metadata for
issuers
with path components (e.g. `https://github.com/login/oauth` →
`/.well-known/oauth-authorization-server/login/oauth`).
## Fix
Replace the `mcp-go` `OAuthHandler`-based discovery with a
self-contained
implementation that correctly follows path-aware well-known URI
construction for
both RFC 9728 and RFC 8414, falling back to root-level URLs when the
path-aware
form returns an error. Also implements RFC 7591 registration directly,
removing
the `mcp-go/client/transport` dependency from the discovery path.
Note: this fix resolves the discovery half of the problem for servers
like
GitHub Copilot. Full OAuth2 support for GitHub's MCP server also
requires
dynamic client registration (RFC 7591), which GitHub's authorization
server
does not currently support — that will be addressed separately.
|
||
|
Kyle Carberry
|
c0f93583e4 |
fix(site): soften tool failure display and improve subagent timeout UX (#23617)
## Summary
Tool call failures in `/agents` previously displayed alarming red
styling (red icons, red text, red alert icons) that made it look like
the user did something wrong. This PR replaces the scary error
presentation with a calm, unified style and adds a dedicated timeout
display for subagent tools.
## Changes
### Unified failure style (all tools)
- Replace red `CircleAlertIcon` + `text-content-destructive` with a
muted `TriangleAlertIcon` in `text-content-secondary` across **all 11
tool renderers**.
- Remove red icon/label recoloring on error from `ToolIcon` and all
specialized tool components.
- Error details remain accessible via tooltip on hover.
### Subagent timeout display
- `ClockIcon` with "Timed out waiting for [Title]" instead of a generic
error display.
- `CircleXIcon` for non-timeout subagent errors with proper error verbs
("Failed to spawn", "Failed waiting for", etc.) instead of the
misleading running verb ("Waiting for").
- Timeout detection from result string/error field containing "timed
out".
### Title resolution for historical messages
- `ConversationTimeline` now computes `subagentTitles` via
`useMemo(buildSubagentTitles(...))` and passes it to historical
`ChatMessageItem` rendering, so `wait_agent` can resolve the actual
agent title from a prior `spawn_agent` result even outside streaming
mode.
### Stories
8 new stories: `GenericToolFailed`, `GenericToolFailedNoResult`,
`SubagentWaitTimedOut`, `SubagentWaitTimedOutWithTitle`,
`SubagentWaitTimedOutTitleFromMap`, `SubagentSpawnError`,
`SubagentWaitError`, `MCPToolFailedUnifiedStyle`.
## Files changed (15)
- `tool/Tool.tsx` — GenericToolRenderer + SubagentRenderer
- `tool/SubagentTool.tsx` — timeout/error verbs, icon changes
- `tool/ToolIcon.tsx` — remove destructive recoloring
- `tool/*.tsx` (10 specialized tools) — unified warning icon
- `ConversationTimeline.tsx` — pass subagentTitles to historical
rendering
- `tool.stories.tsx` — 8 new stories, updated existing assertions
|
||
|
Kyle Carberry
|
d4660d8a69 |
feat: add labels to chats (#23594)
## Summary
Adds a general-purpose `map[string]string` label system to chats, stored
as jsonb with a GIN index for efficient containment queries.
This is a standalone foundational feature that will be used by the
upcoming Automations feature for session identity (matching webhook
events to existing chats), replacing the need for bespoke session-key
tables.
## Changes
### Database
- **Migration 000451**: Adds `labels jsonb NOT NULL DEFAULT '{}'` column
to `chats` table with a GIN index (`idx_chats_labels`)
- **`InsertChat`**: Accepts labels on creation via `COALESCE(@labels,
'{}')`
- **`UpdateChatByID`**: Supports partial update —
`COALESCE(sqlc.narg('labels'), labels)` preserves existing labels when
NULL is passed
- **`GetChats`**: New `has_labels` filter using PostgreSQL `@>`
containment operator
- **`GetAuthorizedChats`**: Synced with generated `GetChats` (new column
scan + query param)
### API
- **Create chat** (`POST /chats`): Accepts optional `labels` field,
validated before creation
- **Update chat** (`PATCH /chats/{chat}`): Supports `labels` field for
atomic label replacement
- **List chats** (`GET /chats`): Supports `?label=key:value` query
parameters (multiple are AND-ed)
### SDK
- `Chat`, `CreateChatRequest`, `UpdateChatRequest`, `ListChatsOptions`
all gain `Labels` fields
- `UpdateChatRequest.Labels` is a pointer (`*map[string]string`) so
`nil` means "don't change" vs empty map means "clear all"
### Validation (`coderd/httpapi/labels.go`)
- Max 50 labels per chat
- Key: 1–64 chars, must match `[a-zA-Z0-9][a-zA-Z0-9._/-]*` (supports
namespaced keys like `github.repo`, `automation/pr-number`)
- Value: 1–256 chars
- 13 test cases covering all edge cases
### Chat runtime
- `chatd.CreateOptions` gains `Labels` field, threaded through to
`InsertChat`
- Existing `UpdateChatByID` callers (e.g., quickgen title updates) are
unaffected — NULL labels preserve existing values via COALESCE
|
||
|
Kyle Carberry
|
d9fc5a5be1 |
feat: persist chat instruction files as context-file message parts (#23592)
## Summary Introduces a new `context-file` ChatMessagePart type for persisting workspace instruction files (AGENTS.md) as durable, frontend-visible message parts. This is the foundation for showing loaded context files in the chat input's context indicator tooltip. ### Problem Previously, instruction files were resolved transiently on every turn via `resolveInstructions()` → `InsertSystem()` and injected into the in-memory prompt without persistence. The frontend had no knowledge that instruction files were loaded into context, and there was no way to surface this information to users. ### Solution Instruction files are now read **once** when a workspace is first attached to a chat (matching how [openai/codex handles it](https://developers.openai.com/codex/guides/agents-md)) and persisted as `user`-role, `both`-visibility message parts with a new `context-file` type. This ensures: - **Durability**: survives page refresh (data is in the DB, returned by `getChatMessages`) - **Cache-friendly**: `user`-role avoids the system-message hoisting that providers do, keeping the instruction content in a stable position for prompt caching - **Frontend-visible**: the frontend receives paths and truncation status for future context indicator rendering - **Extensible**: the same pattern works for Skills (future) ### Key changes | Layer | Change | |---|---| | **SDK** (`codersdk/chats.go`) | Add `ChatMessagePartTypeContextFile` with `context_file_path`, `context_file_content` (internal, stripped from API), `context_file_truncated` fields | | **Prompt expansion** (`chatprompt`) | Expand `context-file` parts to `<workspace-context>` text blocks in `partsToMessageParts()` | | **Chat engine** (`chatd.go`) | Add `persistInstructionFiles()`, called on first turn with a workspace. Remove per-turn `resolveInstructions()` + `InsertSystem()` from `processChat()` and `ReloadMessages` | | **Frontend** | Ignore `context-file` parts in `messageParsing.ts` and `streamState.ts` (no rendering yet — follow-up will add tooltip display) | ### How it works 1. On each turn, `processChat` checks if any loaded message contains `context-file` parts 2. If not (first turn with a workspace), reads AGENTS.md files via the workspace agent connection and persists them 3. For this first turn, also injects the instruction text into the prompt (since messages were loaded before persistence) 4. On all subsequent turns, `ConvertMessagesWithFiles()` encounters the persisted `context-file` parts and expands them into text automatically — no extra resolution needed |
||
|
Kyle Carberry
|
fdc9b3a7e4 |
fix: match text and image attachment heights in conversation timeline (#23593)
## Problem Text attachments (`InlineTextAttachmentButton`) and image thumbnails (`ImageThumbnail`) rendered at different heights when displayed side by side in user messages. Text cards had no explicit height (content-driven), while images used `h-16` (64px). ## Changes **`ConversationTimeline.tsx`** - Added `h-16` to `InlineTextAttachmentButton` to match `ImageThumbnail` - Added `isPlaceholder` prop: when the content hasn't been fetched yet (file_id path), renders "Pasted text" in sans-serif `text-sm` with `items-center` alignment instead of monospace `text-xs` - Once real content loads, it still renders in `font-mono text-xs` with `formatTextAttachmentPreview()` **`ConversationTimeline.stories.tsx`** - Added `UserMessageWithMixedAttachments` story showing a text attachment and image side by side as a visual regression guard |
||
|
Kyle Carberry
|
40395c6e32 |
fix(coderd): fast-retry PR discovery after git push (#23579)
## Problem
When chatd pushes a branch and then creates a PR (e.g. `git push`
followed by `gh pr create`), the gitsync background worker often picks
up the stale `chat_diff_statuses` row between the two operations. At
that point no PR exists yet, so the worker skips the row. However, the
acquisition SQL locks the row for **5 minutes** (crash-recovery
interval), creating a dead zone where the PR diff is invisible in the UI
until the user manually navigates to the chat.
### Root cause
1. `git push` triggers `GIT_ASKPASS` → coderd external-auth handler →
`MarkStale()` sets `stale_at = now - 1s`
2. Background worker acquires the row within ~10s, atomically bumps
`stale_at = NOW() + 5 min` (crash-recovery lock)
3. Worker calls `ResolveBranchPullRequest` → no PR exists yet → returns
`nil` → worker skips with `continue`
4. `gh pr create` completes moments later, but uses its own auth (not
`GIT_ASKPASS`), so no second `MarkStale` fires
5. Row is locked for 5 minutes before the worker can retry
Loading the chat works immediately because `GET /chats/{chat}` calls
`resolveChatDiffStatus` synchronously, which discovers the PR inline.
## Fix
When `ResolveBranchPullRequest` returns nil (no PR yet) **and** the row
was recently marked stale (within 2 minutes), apply a short 15-second
backoff via `BackoffChatDiffStatus` instead of letting the 5-minute
acquisition lock stand. Outside the retry window, the worker skips the
row as before — no indefinite fast-polling for branches that never
receive a PR.
To make the "recently marked stale" check work, `updated_at` is no
longer overwritten by the acquisition and backoff SQL queries. This
preserves it as a reliable "last externally changed" timestamp (set by
`MarkStale` or a successful refresh).
### Behavior summary
| Scenario | `updated_at` age | Backoff | Effective retry |
|---|---|---|---|
| Fresh push, no PR yet | < 2 min | 15s (`NoPRBackoff`) | ~15s |
| Old row, no PR | ≥ 2 min | None (skip) | ~5 min (acquisition lock) |
| Error (any age) | Any | 120s (`DiffStatusTTL`) | ~120s |
| Success (any age) | Any | 120s (`DiffStatusTTL`) | ~120s |
## Changes
- **`coderd/database/queries/chats.sql`** — Remove `updated_at = NOW()`
from `AcquireStaleChatDiffStatuses` and `BackoffChatDiffStatus`
- **`coderd/database/queries.sql.go`** — Regenerated
- **`coderd/x/gitsync/worker.go`** — Add `NoPRBackoff` (15s) and
`NoPRRetryWindow` (2 min) constants; apply short backoff only within the
retry window
- **`coderd/x/gitsync/worker_test.go`** — Add
`TestWorker_NoPR_RecentMarkStale_BacksOffShort` and
`TestWorker_NoPR_OldRow_Skips`
|
||
|
Kyle Carberry
|
44baac018a |
fix(site): replace model catalog loading text with skeleton (#23583)
## Changes Replaces the "Loading model catalog..." / "Loading models..." text flash on `/agents` with a clean skeleton loading state, and removes the admin-nag status messages entirely. ### Removed - `getModelCatalogStatusMessage()` function and `modelCatalogStatusMessage` prop chain — "Loading model catalog..." / "No chat models are configured. Ask an admin to configure one." text below the input - `inputStatusText` prop chain — "No models configured. Ask an admin." / "Models are configured but unavailable. Ask an admin." inline text - `modelCatalogError` prop from `AgentCreateForm` ### Changed - `AgentChatInput`: when `isModelCatalogLoading` is true, renders a `Skeleton` in place of the `ModelSelector` - `getModelSelectorPlaceholder()`: "No Models Configured" / "No Models Available" (title case) ### Added - `LoadingModelCatalog` story — skeleton where model selector sits - `NoModelsConfigured` story — selector shows "No Models Configured" Net -69 lines. |
||
|
Kyle Carberry
|
4ba9986301 |
fix(site): update sticky messages during streaming (#23577)
## Problem The sticky user message visual state (`--clip-h`, fade gradient, push-up positioning) is driven by an `update()` function that only ran on `scroll` events. The chat scroll container uses `flex-col-reverse`, where `scrollTop = 0` means "at bottom." When streaming content grows the transcript while the user is auto-scrolled to the bottom, `scrollTop` stays at `0` — no `scroll` event fires — so `update()` never runs and the sticky messages become visually stale until the user manually scrolls. ## Fix Add a `ResizeObserver` on the scroller's content wrapper inside the existing `useLayoutEffect` that sets up the scroll/resize listeners. When the content wrapper resizes (streaming growth), it fires the observer which calls `update()` through the same RAF-throttle pattern used by the scroll handler. Single observer per sticky message instance. Zero cost when nothing is resizing. Cleanup handled in the same effect teardown. |
||
|
Kyle Carberry
|
07dbee69df |
feat: collapse MCP tool results by default (#23568)
Wraps the `GenericToolRenderer` (used for MCP and unrecognized tools) in `ToolCollapsible` so the result content is hidden behind a click-to-expand chevron, matching the pattern used by `read_file`, `write_file`, and other built-in tool renderers. ### Changes - Move `ToolIcon` + `ToolLabel` into the `ToolCollapsible` `header` prop - Compute `hasContent` from `writeFileDiff` / `fileContent` / `resultOutput` — when there's no content, the header renders as a plain div with no chevron - Remove `ml-6` from `ScrollArea` classNames (the `ToolCollapsible` button handles its own layout) - `defaultExpanded` is `false` by default in `ToolCollapsible`, so results start collapsed ### Before MCP tool results were always fully visible inline. ### After MCP tool results are collapsed by default with a chevron toggle, consistent with `read_file`, `edit_files`, `list_templates`, etc. |
||
|
Kyle Carberry
|
f784b230ba |
fix(coderd/x/chatd/mcpclient): handle EmbeddedResource and ResourceLink in MCP tool results (#23569)
## Problem When an MCP tool returns an `EmbeddedResource` content item (e.g. GitHub MCP server returning file contents via `get_file_contents`), the `convertCallResult` function falls through to the `default` case, producing: ``` [unsupported content type: mcp.EmbeddedResource] ``` This loses the actual resource content and shows an unhelpful message in the chat UI. ## Root Cause The type switch in `convertCallResult` handles `TextContent`, `ImageContent`, and `AudioContent`, but not the other two `mcp.Content` implementations from `mcp-go`: - `mcp.EmbeddedResource` — wraps a `ResourceContents` (either `TextResourceContents` or `BlobResourceContents`) - `mcp.ResourceLink` — contains a URI, name, and description ## Fix Add two new cases to the type switch: 1. **`mcp.EmbeddedResource`**: nested type switch on `.Resource`: - `TextResourceContents` → append `.Text` to `textParts` - `BlobResourceContents` → base64-decode `.Blob` as binary (type `"image"` or `"media"` based on MIME) - Unknown → fallback `[unsupported embedded resource type: ...]` 2. **`mcp.ResourceLink`**: render as `[resource: Name (URI)]` text ## Testing Added 3 new test cases (all passing, full suite 23/23 PASS): - `TestConnectAll_EmbeddedResourceText` — text resource extraction - `TestConnectAll_EmbeddedResourceBlob` — binary blob decoding - `TestConnectAll_ResourceLink` — resource link rendering |
||
|
Kyle Carberry
|
6b105994c8 |
feat(site): persist MCP server selection in localStorage (#23572)
## Summary Previously the user's MCP server toggles were ephemeral — every page reload or navigation to a new chat reset them to the admin-configured defaults (`force_on` + `default_on`). This was frustrating for users who routinely disabled a default-on server or enabled a default-off one. This PR persists the MCP server picker selection to `localStorage` under the key `agents.selected-mcp-server-ids`. ## Changes ### `MCPServerPicker.tsx` - **`mcpSelectionStorageKey`** — exported constant for the localStorage key. - **`getSavedMCPSelection(servers)`** — reads from localStorage, filters out stale/disabled IDs, always includes `force_on` servers. - **`saveMCPSelection(ids)`** — writes the current selection to localStorage. ### `AgentCreateForm.tsx` - Initialises `userMCPServerIds` from `getSavedMCPSelection` instead of `null`. - Calls `saveMCPSelection` on every toggle. ### `AgentDetail.tsx` - Adds localStorage as a fallback tier in `effectiveMCPServerIds`: user override → chat record → **saved selection** → defaults. - Calls `saveMCPSelection` on every toggle. ### `MCPServerPicker.test.ts` (new) - 13 unit tests covering save, restore, stale-ID filtering, force_on merging, invalid JSON handling, and disabled server filtering. ## Fallback priority | Priority | Source | When | |----------|--------|------| | 1 | In-memory state | User toggled during this session | | 2 | Chat record | Existing conversation with `mcp_server_ids` | | 3 | localStorage | User has a saved selection from a prior session | | 4 | Server defaults | `force_on` + `default_on` servers | |
||
|
Kyle Carberry
|
894fcecfdc |
fix: inherit MCP server IDs from parent chat when spawning subagents (#23571)
Child chats created via `spawn_agent` and `spawn_computer_use_agent` were not inheriting the parent's `MCPServerIDs`, meaning subagents lost access to the parent's MCP server tools. ## Changes - Pass `parent.MCPServerIDs` in the `CreateOptions` for both `createChildSubagentChat()` and the `spawn_computer_use_agent` tool handler in `coderd/x/chatd/subagent.go`. ## Tests Added 3 tests in `subagent_internal_test.go`: - `TestCreateChildSubagentChat_InheritsMCPServerIDs` — verifies child chat gets parent's MCP server IDs (multiple servers) - `TestSpawnComputerUseAgent_InheritsMCPServerIDs` — verifies computer use subagent gets parent's MCP server IDs via the tool - `TestCreateChildSubagentChat_NoMCPServersStaysEmpty` — verifies no regression when parent has no MCP servers |
||
|
Kyle Carberry
|
3812b504fc | fix(coderd/x/chatd): prevent nil required field in MCP tool schemas for OpenAI (#23538) | ||
|
Kyle Carberry
|
dda985150d | feat: add MCP server config ID to tool-call message parts (#23522) | ||
|
Kyle Carberry
|
3495cad133 |
fix: resolve localhost URLs in markdown with correct port and protocol (#23513)
## Summary Fixes several bugs in the markdown URL transform that replaces `localhost` URLs with workspace port-forward URLs in the AI agent chat. ## Bugs Fixed ### 1. URLs without an explicit port produce `NaN` in the subdomain When an LLM outputs a URL like `http://localhost/path` (no port), `parsed.port` is the empty string `""`. `parseInt("", 10)` returns `NaN`, producing a broken URL like: ``` http://NaN--agent--workspace--user.proxy.example.com/path ``` Now defaults to port 80 for HTTP and 443 for HTTPS via the new `resolveLocalhostPort()` helper. ### 2. Protocol always hardcoded to `"http"` The `urlTransform` in `AgentDetail.tsx` always passed `"http"` as the protocol argument, silently discarding the original URL's scheme. This meant `https://localhost:8443/...` would not get the `s` suffix in the subdomain. Now extracts the protocol from the parsed URL, matching the existing behavior of `openMaybePortForwardedURL`. ### 3. `urlTransform` not memoized The closure was re-created on every render. Wrapped in `useCallback` with the four primitive dependencies (`proxyHost`, `agentName`, `wsName`, `wsOwner`). ### 4. Duplicated `localHosts` definition The localhost detection set was defined separately in both `AgentDetail.tsx` and `portForward.ts`. Consolidated into a single shared export from `portForward.ts`. ## Changes - **`site/src/utils/portForward.ts`**: Export shared `localHosts` set and new `resolveLocalhostPort()` helper. Update `openMaybePortForwardedURL` to use both. - **`site/src/pages/AgentsPage/AgentDetail.tsx`**: Import shared `localHosts` and `resolveLocalhostPort`. Fix protocol extraction. Memoize `urlTransform`. - **`site/src/utils/portForward.jest.ts`**: Add tests for `resolveLocalhostPort` and `localHosts`. Renamed from `.test.ts` to `.jest.ts` to match project convention. |
||
|
Kyle Carberry
|
e34162945a |
fix(coderd/x/chatd): normalize OAuth2 token type to canonical Bearer case (#23516)
Linear's MCP server (`mcp.linear.app`) returns `token_type="bearer"` (lowercase) in its OAuth2 token response but rejects requests that use the lowercase form in the `Authorization` header. RFC 6750 says the scheme is case-insensitive, but Linear enforces capital-B `Bearer`. Confirmed by running the actual Linear MCP OAuth flow end-to-end: - `Authorization: Bearer <token>` → **42 tools, works** - `Authorization: bearer <token>` → **401 invalid_token** This is a one-line fix: normalize any case variant of `bearer` to `Bearer` before building the `Authorization` header, matching the behavior of the mcp-go library's own OAuth handler. |
||
|
Kyle Carberry
|
e87ea1e0f5 |
fix(coderd): add PKCE support to MCP server OAuth2 flow (#23503)
## Problem
MCP servers like Linear (`mcp.linear.app`) require PKCE (RFC 7636) for
their OAuth2 flow. Without it, the token exchange may succeed but the
resulting access token is immediately rejected with a 401
`invalid_token` error when the chat daemon tries to connect to the MCP
server.
This means users can authenticate successfully in the UI (the OAuth
popup completes, `auth_connected` shows `true`), but the model never
receives the MCP tools — they silently fail to load.
### Root cause
The `mcpServerOAuth2Connect` handler was calling
`oauth2Config.AuthCodeURL(state)` without any PKCE parameters
(`code_challenge`, `code_challenge_method`). The callback was calling
`oauth2Config.Exchange(ctx, code)` without a `code_verifier`. Linear's
MCP OAuth endpoint decoded state confirms it expected PKCE with
`codeChallengeMethod: "plain"`.
### Investigation
- The chat (`c2c04fc5-5622-4b71-a5a9-80508e86f78e`) had the Linear MCP
server ID in `mcp_server_ids`
- `auth_connected: true` (token row exists in DB)
- No "expired" or "empty token" warnings in logs
- Server log showed: `skipping MCP server due to connection failure ...
error="initialize: transport error: request failed with status 401:
{"error":"invalid_token","error_description":"Missing or invalid access
token"}"`
- Decoding Linear's OAuth state revealed PKCE was expected
## Changes
- Generate a PKCE `code_verifier` during the OAuth2 connect step using
`oauth2.GenerateVerifier()` and store it in a cookie scoped to the
callback path
- Include `code_challenge` (S256) in the authorization redirect URL via
`oauth2.S256ChallengeOption()`
- Pass the `code_verifier` during the token exchange in the callback via
`oauth2.VerifierOption()`
- Fix a nil-pointer guard on `api.HTTPClient` in the callback
- Add tests verifying PKCE parameters are sent correctly and backwards
compatibility when no verifier cookie is present
|
||
|
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; ``` |
