100 Commits

This Branch

This Branch

All Branches

Author	SHA1	Message	Date
Kyle Carberry	58f70b4488	fix(coderd/x/chatd): sanitize workspace MCP tool names (#26928) ... ## Summary Workspace MCP tools (servers a workspace declares in `.mcp.json`) take their model-facing name from the server key joined with the tool name as `serverName__toolName`. That name reached the model **unsanitized**, so a server or tool name containing a character outside `^[a-zA-Z0-9_-]{1,128}$` (for example `@`) produced an invalid tool name. Anthropic and Bedrock reject the whole request with `HTTP 400`: ``` tools.N.custom.name: String should match pattern '^[a-zA-Z0-9_-]{1,128}$' ``` which fails the entire turn, not just the one tool. The remote MCP path (`mcpclient`) and the AI Gateway path (`aibridge/mcp`) already sanitize; the workspace path did not. Alternative to #26853 (thanks @ibdafna for the report and repro). ## Fix Sanitize and length-cap the **model-facing** name, and keep the original `serverName__toolName` as a `routingName` the workspace agent uses to reach the original server and tool. `NewWorkspaceMCPTools` builds a whole set and disambiguates names that collide after sanitization (for example server keys `foo.bar` and `foo_bar` both exposing `echo`) so every tool stays addressable in the model's name-keyed dispatch map. Names already within the allowed set are unchanged, so there is no behavior change for valid names. The sanitizer is local to `coderd/x/chatd/chattool`; the fix does **not** touch the `aibridge` package or the remote MCP client. ### Changes - `coderd/x/chatd/chattool/mcpworkspace.go`: local provider-safe sanitizer + length cap, `routingName` for the agent proxy, and `NewWorkspaceMCPTools` for set-level collision disambiguation. - `coderd/x/chatd/chatd.go`: build the pinned workspace tool set via `NewWorkspaceMCPTools`. ## Why sanitize here (not at `.mcp.json` / agent parse)? The agent uses `serverName__toolName` to route to the real downstream server (it splits on `__` and calls the original tool name), so sanitizing at parse time would break routing or merely relocate the original->sanitized mapping. Sanitization is also a provider constraint the agent has no knowledge of, and coderd/agent version skew means coderd must sanitize at its own boundary regardless. The model-facing boundary in chatd is the right place. ## Test plan - `@` in a name is sanitized for the model while the original routes to the agent; a valid name is unchanged; an over-length name is truncated; colliding names in a set are disambiguated while each still routes to its own original name. - `go build`, `go vet`, `golangci-lint`, and `go test ./coderd/x/chatd/chattool/...` pass locally. <details> <summary>Design notes / decision log</summary> **Constraint that drives the design.** The tool name is both the identifier shown to the model (and the key the model layer dispatches tool calls by) and, for the workspace path, the string the agent splits on `__` to route back to the original server and tool. Those roles conflict once sanitization changes the name, so the name is sanitized for the model while the unsanitized form is kept as `routingName`. **Options considered.** 1. **Chosen:** sanitize in the workspace path only, with helpers local to `chattool`. Smallest blast radius; no new cross-package dependency. This matches the shape of the other MCP paths (`mcpclient` keeps `originalName` + `configID`) without sharing code. 2. Sanitize at `.mcp.json` parse time or in the agent. Rejected: breaks routing (the agent needs the original name), pushes a provider concern into the agent, and coderd must still defend its own boundary because the agent and coderd version independently. Tool names also come from the downstream server at list time, not from `.mcp.json`, so parsing cannot fully validate them. 3. Extract a shared sanitize/truncate/dedupe helper into `aibridge/mcp` and adopt it in `mcpclient` too (so the remote path also gains collision disambiguation). This DRYs all paths, but it grows chatd's coupling to the `aibridge` subsystem and expands scope/behavior/tests in the remote path for what is a workspace-path bug. Left out deliberately to keep this change minimal and self-contained; it can be a separate refactor. 4. Sanitize once at the provider serialization boundary (chat loop). The only truly generic spot, but the model dispatches by name, so it needs a reverse (sanitized -> original) mapping and set-wide collision handling in the model layer. Larger, riskier change. **Notes.** - The workspace path defines its own sanitizer (`[^a-zA-Z0-9_-]` -> `_`) and a `maxModelToolNameLen = 64` constant that mirrors the strictest provider limit (OpenAI 64, Bedrock 128), rather than importing `aibridge/mcp`, so it carries no new dependency. - The set builder sorts before assigning suffixes so disambiguation is stable across turns. </details> --- _Opened by Coder Agents on behalf of @kylecarbs. Alternative to #26853._	2026-07-01 20:34:25 +02:00
Kyle Carberry	d6daf273aa	fix: tighten single tool result byte budget (#26763) ... ## Problem #26637 caps each locally-executed tool result (built-in, global/deployment MCP, workspace MCP) at a per-result byte budget derived from the model's context window. The budget was `ContextLimit/2 * 4 bytes` — i.e. **half the window at an optimistic 4 bytes/token**. On large-context models that is far too generous. With a `1,000,000`-token `ContextLimit` the per-result cap is **~2 MB**. A user hit exactly this with a chatd (deployment-pinned) MCP tool: the result was truncated to **1,998,709 characters** and still overflowed the prompt. 2 MB of dense text (JSON/logs/code) is ~650k–1M tokens — most or all of the window for a *single* result — so the cap fired but didn't actually prevent the overflow. ## Fix Tighten the two budget constants in `tooltruncate.go`: | constant | before | after | | --- | --- | --- | | `toolResultContextDivisor` | `2` (½ window) | `3` (⅓ window) | | `bytesPerTokenEstimate` | `4` | `3` (conservative) | The budget becomes `ContextLimit/3 * 3 ≈ ContextLimit` bytes: | ContextLimit | before | after | | --- | --- | --- | | 1,000,000 | ~2 MB | ~1 MB | | 200,000 | ~400 KB | ~200 KB | | unknown (≤0) | 64 KB | 64 KB (unchanged) | The 16 KB floor and 64 KB unknown-window default are unchanged. A conservative bytes-per-token estimate is intentional: dense payloads run well under 4 B/tok, so a lower estimate yields a smaller byte budget that is less likely to underestimate the true token cost. No behavioral code paths change — only the two constants and their doc comments. The existing `tooltruncate_internal_test.go` cases derive their expectations from the constants (`LargeWindow`) or exercise the floor/default (`BelowFloor`, `Unknown`), so they remain green. <details> <summary>Investigation notes</summary> Global/deployment MCP tools (`mcpclient.ConnectAll`) are appended to `prepared.Tools` and execute locally via `ExecuteLocalTools → executeTools → executeSingleTool`, so the #26637 cap *does* apply to them for text results (`convertCallResult` joins text content into `resp.Content`). The cap was simply too large: `toolResultByteBudget(ContextLimit)` = `ContextLimit/2*4` ≈ 2 MB for a 1M-token window. Reverse-engineering the reported `1,998,709` truncated characters confirms a `ContextLimit` of ~1,000,000 tokens. Known gaps left for follow-ups (out of scope here): - **Per-step aggregate is unbounded.** MCP tools advertise `Parallel: true` and `executeSingleTool` caps each result independently, so N parallel calls in one step can sum to N × the per-result cap. - **Binary/media `Data` bypasses the cap.** Only the text payload is bounded; `image`/`media`/blob embedded-resource results are base64-encoded untouched in `executeSingleTool`. - **Compaction is reactive.** It is gated on the prior step's reported usage (`latestPromptUsage`), so it can't pre-empt a single large result appended on the current step. </details> --- Generated by Coder Agents on behalf of @kylecarbs.	2026-06-26 13:33:55 -06:00
Kyle Carberry	a163d43981	fix(site/src/pages/AgentsPage): keep agent chat sticky truncation in sync as the transcript grows (#26714) ... ## Problem In `/agents`, the sticky user-message truncation sometimes does not update as new content arrives. While pinned to the bottom with the transcript overflowing, several messages (or a streaming response) can land and the sticky bubble keeps a stale clip height, overflowing and overlapping the content below it. It only snaps back once you scroll manually. ## Root cause `StickyUserMessage` recomputes its clip height (`--clip-h`) and push-up `top` in an `update()` driven by three triggers: a scroll listener, a window-resize listener, and a `ResizeObserver` meant to catch the transcript growing. The observer watched `scroller.firstElementChild`, but in `ChatScrollContainer` the scroller's first child is the `flex-1 basis-0` spacer that pins content to the bottom, not the content wrapper. That spacer collapses to `0px` the moment the transcript overflows (exactly when truncation engages) and then never resizes again, so the observer goes silent. The other triggers do not cover this case either: in `flex-col-reverse` the `scrollTop` stays at `0` while pinned to the bottom, so no scroll event fires as content grows. The result is a stale `--clip-h` until the next manual scroll. ## Fix - Observe the real content wrapper instead of the collapsing spacer. The wrapper is tagged with `data-chat-scroll-content` (it contains both the committed timeline and the streaming live tail), and the sticky code resolves it via `sentinel.closest(...)`, falling back to the previous node only if the marker is absent. - Recompute the scroller geometry (`scrollerTop`/`scrollerHeight`) inside `update()` on every tick instead of caching it at effect setup, so the clip and push-up math cannot drift when the scroller moves or resizes without a window resize (for example the composer growing). This also removes the now-redundant `onResize` handler. No change to the sticky visuals or the rAF throttling. ## Testing - New story `StickyUserMessageClipUpdatesWhilePinned` grows the transcript while pinned (no scroll dispatched) and asserts the clip tracks the new geometry, plus structural guards that the observed node is the content marker and not the `aria-hidden` spacer. - Verified as a true regression guard: with the fix reverted the new story fails; with the fix it passes. The existing `StickyUserMessagePinsOnScroll` is unaffected. - `biome check`, `tsc -p .`, React Compiler check, emdash check, and `vitest --project=storybook` for this stories file all pass (48/48). <details> <summary>Decision log</summary> - Considered centralizing the per-message scroll/resize/observer wiring into a single coordinator in `ConversationTimeline` (it already centralizes sentinels) to cut N observers/listeners down to one. Deferred as a follow-up to keep this PR a surgical, low-risk fix; this change alone resolves the staleness. - Chose a semantic `data-chat-scroll-content` marker over reusing the `chat-timeline-wrapper` test id so runtime behavior does not depend on a test-only attribute. The marker sits on the wrapper that contains both the timeline and the live tail, so streaming growth is observed too. - Kept the `scroller.firstElementChild` fallback so other `ConversationTimeline` consumers and stories without the marker keep working. </details> --- Filed via Coder Agents on behalf of @kylecarbs.	2026-06-25 17:57:28 -06:00
Kyle Carberry	1742003785	fix(agent): gate workspace context collection until the agent is ready (#26715) ... ## Problem Workspace context surfaced in chat (Coder Agents) is incomplete and racy on a fresh boot: - The context panel is missing personal skills (only repo-level skills under `.claude/skills` show up). - The MCP section lists `.mcp.json` files but no MCP servers are registered. - The Issues panel reports instruction files as unreadable, e.g. `CLAUDE.md (file: unreadable)` and `.cursorrules (file: unreadable)` with `symlink resolve: lstat .../AGENTS.md: no such file or directory`. ## Root cause `agentcontext.Manager` collected and pushed context too eagerly: - `NewManager` ran an eager resolve at agent `init()`. - `RunPush` starts as a normal connection routine (`startAgentAPI210`) with no lifecycle gating, so the first snapshot was pushed (`Initial=true`) as soon as the agent API connected. Both happened **before startup scripts finish** and before the lifecycle reaches `ready`. At that point: - `CLAUDE.md` / `.cursorrules` symlinks to `AGENTS.md` don't resolve yet, so `EvalSymlinks` fails and the resolver emits `StatusUnreadable` "symlink resolve" issues. - Personal skills haven't synced yet, so they're missing. - MCP servers connect via `mcpManager.Reload(...)` only **after** `ready`, so only `.mcp.json` configs appear, with no servers. That partial, error-laden snapshot is persisted by coderd and can hydrate a chat. ## Fix Gate `agentcontext.Manager` until the agent is ready, unconditionally: - The Manager always starts gated. `NewManager` leaves the zero-value (version 0) snapshot in place and never walks the filesystem; `RunPush` withholds version-0 snapshots, so nothing reaches coderd. - The agent calls `Manager.SetReady()` from the lifecycle transition in `handleManifest`, right after startup scripts finish (`ready`, or terminal `start_error` / `start_timeout` so a failed startup still surfaces whatever context exists). - On `SetReady`, the Manager performs the first real resolve (version 1) and broadcasts it; `RunPush` ships it with `Initial=true`. Later changes (MCP connect, skill edits) re-resolve and push as before. Eager resolution before `ready` was the bug, not a mode worth preserving, so the gate is always on rather than an opt-in option. This aligns the agent-side push with chatd, which already waits for agent readiness before loading context. No proto/coderd/DB changes: coderd simply never receives a pre-ready snapshot. <details> <summary>Design notes &amp; decisions</summary> - **Unconditional, not opt-in.** An earlier iteration added the gate as an opt-in `ManagerOptions.GateUntilReady`. Since the eager resolve-on-construct was the defect, the option, the eager first resolve, and the now-dead `resolveLocked` helper were all removed; the Manager is always gated until `SetReady`. - **Version 0 is the pre-ready sentinel.** The gated placeholder is just the zero-value snapshot (version 0); the first real resolve is version 1, so the push loop withholds anything at version 0. An earlier revision carried a dedicated `Snapshot.Initializing` bool plus an HTTP `/resync` field, but the push loop was the only consumer and nothing read the HTTP field, so both were dropped. - **Defer, don't retry symlinks.** Transient "unreadable" symlinks are an artifact of collecting before checkout. Deferring until `ready` fixes all three symptom classes at once and avoids masking genuine post-ready errors (a broken symlink at `ready` is still reported). - **Release on terminal startup states too** (`start_error`, `start_timeout`), so a failed startup still surfaces whatever context exists instead of gating forever. On reconnect the Manager instance is reused and stays ready. </details> ## Tests - `agentcontext.TestManager_WithholdsCollectionUntilReady` simulates collection running before startup finishes (broken `CLAUDE.md` / `.cursorrules` -> `AGENTS.md` symlinks): asserts the gated snapshot is the empty version-0 placeholder with no resources and no `unreadable` issues, and that after `SetReady` (target now present) the inventory resolves cleanly to a single instruction file with no spurious issues. - `agentcontext.TestRunPush_WaitsForReady` asserts the push loop ships nothing while gated even when content exists, then ships the full inventory with `Initial=true` after `SetReady`. - `agentcontext.TestManager_SetReadyIsIdempotent` covers the version-0 placeholder before ready, the single resolve to version 1 on `SetReady`, and idempotency across repeated calls. - Updated `agent.TestAgent_ContextStatePushed`: the first push now already contains `AGENTS.md` with `Initial=true` and no `UNREADABLE` resources (no pre-startup empty/partial push). Validated on the changed packages: `go test -race ./agent/agentcontext/...`, `go test ./agent/ -run TestAgent_ContextStatePushed`, `golangci-lint run`, `go vet`, `gofmt` (all clean). --- 🤖 Generated by Coder Agents on behalf of @kylecarbs.	2026-06-25 14:17:50 -06:00
Kyle Carberry	48fd0ef4bc	feat: return workspace skill directory from read_skill (#26713) ... Workspace skills live on the workspace filesystem, and the agent's read_file and execute tools already operate there. read_skill now returns "dir", the absolute skill directory, for workspace skills, so the agent can read or run bundled supporting files (for example a scripts/ helper) with the workspace tools. The field is omitted for personal skills, which are database-backed and have no files. read_skill_file is unchanged. Generated with Coder Agents on behalf of @kylecarbs.	2026-06-25 12:05:54 -06:00
Kyle Carberry	32217259b7	feat: cap tool output to fit the model context window (#26637) ... ## Problem Local tool results were persisted and replayed to the model verbatim, with no size cap. A single oversized result, most often a multi-megabyte response from an MCP tool, overflows the prompt on the next request. Every retry rebuilds the same history and fails the same way, leaving the chat wedged in `error`. Auto-compaction is reactive (token usage is only known after a response), so it can't catch a single result that blows the very next request. ## Fix Cap every locally-executed tool result at its single choke point, `executeSingleTool` in `chatloop`, so the cap covers built-in tools, **global (deployment-pinned) MCP**, **workspace MCP**, and provider runners uniformly. Because this runs before the result is published to the live stream and before it is committed, the SSE preview, the persisted message, and the model replay all see the same bounded output. The budget is token-aware: a single tool result may use at most half the model's context window (`~4 bytes/token`), with a `16KB` floor and a `64KB` default when the window is unknown. Truncation keeps the head and tail of the output and replaces the middle with a marker telling the model how much was removed and to narrow its query; it is UTF-8 safe and never exceeds the budget. Binary media `Data` is passed through untouched (only the text payload is bounded). A `coderd_chatd_tool_result_truncated_total{provider,model,tool_name}` counter and a warning log record each truncation. ## Out of scope - Provider-executed results (e.g. web search) arrive via the stream, not `executeSingleTool`. - Dynamic/external tool results submitted through the `/tool-results` API are validated as JSON elsewhere. - Cumulative growth across many results is still handled by context compaction; this change only bounds any single result. <details> <summary>Implementation notes</summary> - New `coderd/x/chatd/chatloop/tooltruncate.go`: `toolResultByteBudget(contextLimitTokens)` and `truncateToolResultText(text, maxBytes)` (pure, unit-tested). - `chatloop.go`: added `ContextLimit` to `ExecuteLocalToolsOptions`; threaded a computed byte budget through `executeTools` into `executeSingleTool`, where `resp.Content` is capped for the text, media-text, and error branches. - `generation.go`: passes `ContextLimit: prepared.ContextLimitFallback` (the model's configured context limit). - `metrics.go`: new `ToolResultTruncatedTotal` counter + `RecordToolResultTruncated`. - Tunable knobs live as constants in `tooltruncate.go` (`toolResultContextDivisor = 2`, `bytesPerTokenEstimate`, `minToolResultBytes`, `defaultToolResultBytes`). Verified: `go build ./coderd/x/chatd/...`, `go test ./coderd/x/chatd/chatloop/...`, and the `chatd` test binary compiles. </details> --- Resolves CODAGT-678 Generated by Coder Agents on behalf of @kylecarbs.	2026-06-24 09:16:38 -06:00
Kyle Carberry	bafc86310c	fix(agent/agentcontext): identify context sources by lexical path (#26616) ... ## What Identify agent workspace-context **sources** by their lexical (configured) path so `coder exp chat context list` no longer shows the same directory twice, and so a source is shown as the path the operator actually configured. ## Why (the bug) Source identity was the canonical path from `CanonicalizePath`, which resolves symlinks via `EvalSymlinks` **only when the target exists**. That makes canonicalization time-dependent: - At boot the agent seeds sources from `CODER_AGENT_EXP_*_DIRS`. If `~/.coder/skills -> ~/my-agent/agent-rules/skills` and the target does not exist yet (a startup script creates it later), `~/.coder/skills` canonicalizes to the lexical `/home/coder/.coder/skills`. - After the manifest lands (or the target is added directly), the same configured source canonicalizes to the resolved `/home/coder/my-agent/agent-rules/skills`. The same configured source produced two different strings, so dedupe keyed on the string registered both and the list showed one directory twice. Resolving symlinks for identity is also misleading on its own (per @mafredri's review): a source added by a symlink path appears in the list as its resolved target, as if that target had been added explicitly. ## How Source identity is now the **lexical** path: cleaned, `~`-expanded, absolute, with symlinks **not** resolved (new `lexicalPath`; `CanonicalizePath` is refactored to build on it). `AddSource`, `SeedSources`, `HasSource`, `RemoveSource`, and boot seeding all key on this stable identity. `AddSource` still **validates** the resolved (`CanonicalizePath`) path against the allowed roots, so a symlink cannot escape them. Only the identity/display path changed. This replaces the earlier `os.SameFile`/inode dedupe, which was unstable and failed on Windows runners. ## Testing - `go test ./agent/agentcontext/` (full package) and `go vet` pass; `gofmt` clean. - `TestManager_SourceIdentityIsLexicalAndStable`: adds the same symlinked source before and after its target exists and asserts one source whose path is the lexical link (skipped on Windows, matching the package's other symlink tests). - Existing `TestCanonicalizePath_FollowsSymlinks` and `TestValidateSourcePath_*` confirm symlink resolution and the security boundary are unchanged. <details> <summary>Related review findings</summary> Fixes the "duplicate symlinked paths in `context list`" issue from the chat-context system review and the dedupe-ordering question (lexical identity preserves first-come-first-served order). Showing the configured path for **resources** (not just sources) and restoring scope-based skill precedence are separate, larger changes tracked elsewhere. </details> --- *This PR was created by Coder Agents on behalf of @kylecarbs.*	2026-06-23 15:46:26 +00:00
Kyle Carberry	44f2a77b34	feat(site/src/pages/AgentsPage/components): show context sizes and full .mcp.json paths (#26614)	2026-06-23 07:36:05 -06:00
Kyle Carberry	27ecd17991	refactor: consolidate agent MCP onto a single persistent engine (#26599) ... Two MCP code paths both spawned the servers declared in a workspace's `.mcp.json`: the persistent engine in `agent/x/agentmcp` (which owns tool-call execution via `CallTool`) and an ephemeral one-shot runner in `agent/agentcontext` (`mcprunner.go`) that connected, listed tools, and immediately closed each server purely for discovery. Every declared server was launched twice, and the discovery path duplicated the engine's `.mcp.json` parse, transport-build, env-resolve, and connect logic. This makes `agent/x/agentmcp` the single persistent MCP engine. The `agentcontext` manager now reads that engine's per-server catalog in-process through an injected `MCPCatalog` option and surfaces each server as a `KindMCPServer` resource. The engine wires `SetOnReload` to the manager's `Trigger`, so a reload (startup connect or `.mcp.json` edit) re-resolves and re-pushes the pinned resources. Tool-call execution is unchanged: it still flows through the engine's `CallTool` over `POST /api/v0/mcp/call-tool`. The now-dead HTTP discovery surface is removed: the agent `GET /api/v0/mcp/tools` route with `agentmcp.API.handleListTools`, and `workspacesdk.AgentConn.ListMCPTools` with `ListMCPToolsResponse` (mock regenerated). The change nets roughly `-1370` lines, mostly the deleted duplicate runner and its tests. <details> <summary>Decision log</summary> The merge of #26585 made pinned `chat_context_resources` the sole source of workspace context, which surfaced the duplicate spawning. Two options were considered: - **Option A + dependency injection (chosen):** keep `agent/x/agentmcp` as the single persistent engine; `agentcontext` consumes its catalog in-process and stays the orchestrator/owner at the API boundary (it still pushes `KindMCPServer` resources). This is low-risk because `agentcontext` already exposed the `resolver.MCPResources` seam, so the change just rebinds it from the ephemeral runner to the shared engine. - **Option B (rejected):** reimplement persistent pooling, reconnect, singleflight, and race handling inside `agentcontext` and delete `agentmcp`. Too broad, and it discards the engine's tested lifecycle for no behavioral gain. `agentcontext`'s discovery was never what kept servers alive; its runner closed each server immediately after listing tools. The component holding persistent connections was always `agentmcp`, which is why execution already lived there. Consolidating onto it removes the duplicated stack rather than a whole package: both packages survive with distinct roles (`agentmcp` is the engine, `agentcontext` is the orchestrator/owner). </details> Coder Agents generated on behalf of @kylecarbs	2026-06-22 22:21:58 -06:00
Kyle Carberry	0f37522e6f	refactor(agent/agentcontext): fixed-location shallow context discovery (#26596) ... ## Problem `agent/agentcontext` resolved workspace context by walking the working directory recursively (depth 8) and matching files by basename. This over-injected context: - Instruction-file matching was case-insensitive, so the generated `docs/reference/api/agents.md` was treated as an instruction file. - Symlinked instruction files (`CLAUDE.md`, `.cursorrules` -> `AGENTS.md`) shipped as duplicate resources. - Nested `AGENTS.md` (e.g. `site/AGENTS.md`) were collected from anywhere in the tree. - Skills were discovered from *any* `skills/` directory anywhere in the tree, and `.mcp.json` from any depth. Resolving the repo root produced six instruction sources for what was effectively one file of guidance, plus skills/MCP found by an open-ended walk. ## What changed Replace the recursive scan with **fixed-location, shallow** discovery. Each scan root is inspected at its top level only: the resolver never descends into subdirectories and never climbs to a parent. Additional directories are added explicitly as sources (HTTP API) or via the `CODER_AGENT_EXP_*_DIRS` seeding env vars. - **Single working-dir scan root.** The working directory is one scan root. Instruction files and `.mcp.json` are read only at its top level. - **Fixed-location skills.** Skills are discovered only from `skills`, `.agents/skills`, `.claude/skills`, `.codex/skills` (one skill per immediate subdir with a `SKILL.md`), not from arbitrary `skills/` directories. - **Case-sensitive instruction names.** Exact `AGENTS.md`/`CLAUDE.md`/`.cursorrules`; a lower-case `agents.md` is ignored. - **Symlink dedup.** Resources are attributed to their resolved target, so symlinked `CLAUDE.md`/`.cursorrules` collapse into the single `AGENTS.md`. - **Watcher** mirrors the same fixed-location set instead of recursively watching every scan root (no more walking `node_modules`). - The recursive `walkDir`, `skipDirNames`, `MaxScanDepth`, and `isSkillsContainer` are removed. Resolving the repo root now yields `AGENTS.md`, `.mcp.json`, and the `.agents/skills`/`.claude/skills` skills, with no nested instruction-file noise. ## Behavior change `site/AGENTS.md` is no longer auto-injected when the working dir is the repo root. It loads when the working dir **is** `site/` (its top level), or when `site/` is added as an explicit source. There is intentionally **no walk-up** to a `.git` project root: an agent started in a subdirectory does not auto-inherit ancestor `AGENTS.md`; those directories are added explicitly. <details> <summary>Verification &amp; decision log</summary> **codex research (confirmed via source).** Instruction files: codex walks up to the first `.git` ancestor and reads root-&gt;cwd, one file per directory, exact-cased names (`codex-rs/core/src/agents_md.rs`). Skills: fixed roots (`.agents/skills`, `.codex/skills`, `$CODEX_HOME/skills`, ...) with bounded in-root recursion (`core-skills/src/loader.rs`). MCP: `.codex/config.toml` via walk-up; a project `.mcp.json` is not a runtime source in codex. No resource type triggers an unbounded downward walk. **Decisions.** - Adopt codex's fixed-location, shallow discovery (case-sensitive names, symlink dedup, top-level-only files, container-only skills). - **Deliberately omit codex's walk-up to the `.git` project root.** In Coder the working dir is the scan root and extra directories are added explicitly (HTTP API / `CODER_AGENT_EXP_*_DIRS`), so the implicit ancestor climb added surprise without benefit (e.g. `context add ./site` should scan `./site`, not the repo root). - Keep `.mcp.json` (codex uses `config.toml`, intentionally not added). - Include `.claude/skills` and `.codex/skills` in the container list so the repo's existing `.claude/skills` skills are not regressed; skills recurse one level inside a container. **Tests.** `TestManager_WorkingDirScannedShallow` (working dir read at top level; ancestor root and nested subdir both excluded); `TestResolver_SkillsOnlyFromFixedContainers`, `TestResolver_MCPConfigOnlyAtScanRoot`, `TestResolver_SymlinkedInstructionFilesDeduplicated`, `TestResolver_InstructionFilesOnlyAtScanRoot`, `TestResolver_InstructionNamesAreCaseSensitive`. Cap tests use multiple scan roots. **Local checks.** `gofmt`, `go vet`, `golangci-lint`, `go test -race`, and `make lint/emdash` pass for the package. </details> --- 🤖 Generated by Coder Agents on behalf of @kylecarbs.	2026-06-23 03:42:40 +00:00
Kyle Carberry	688ee63f9d	feat(site/src/pages/AgentsPage): group context indicator resources by directory (#26598) ... ## What Groups the **Skills** and **Context files** lists in the context-usage popover (`ContextUsageIndicator`) by their parent directory. Resources pulled from different roots, for example a repo-root `AGENTS.md` and a nested `site/AGENTS.md`, or same-named skills from `.coder/skills` vs `.agents/skills`, previously collapsed to identical basenames and lost their provenance. Each group now renders a dimmed directory header with a folder icon, and its items are indented beneath it. ## Why `ChatContextResource.source` carries the real path for instruction files and skill directories, but the popover only rendered the basename, so multiple context roots were indistinguishable. ## Notes - The directory header shows even for a single root, so provenance is always visible. - MCP is intentionally left flat for now: an `mcp_server`'s `source` is a server name (not a path) and the row carries no reference to the `mcp_config` that declared it, so it does not fit the directory model without a backend change. - Adds a `getPathDirname` helper (with tests) and switches the skill list `key` from `skill.name` to `skill.source` to avoid collisions across roots. ## Test plan - `ContextUsageIndicator.stories.tsx`: new `MultipleContextRoots` story (files + skills across several directories); `Clean` story updated to assert the single-root header. Storybook play tests pass (4/4). - `path.test.ts`: added `getPathDirname` cases (unit pass). - `biome check`, `tsc -p .`, and `make pre-commit` all pass. <details> <summary>Design decision log</summary> - **Header gating:** started by only showing the directory header when a section spanned more than one directory; changed to **always show** because the provenance is useful even for a single root. - **Presentation:** chose a grouped layout (dimmed directory header + folder icon, items indented beneath) over an inline dimmed path prefix, since there is typically a lot of shared structure to convey. - **Directory labels** currently render the full path (dimmed, truncated with a `title` tooltip). Open follow-ups: collapse `$HOME` to `~` or show fewer path segments if the labels feel long. - **MCP:** evaluated applying the same grouping. An `mcp_server` is keyed by name and has no link to its `mcp_config` (confirmed in `codersdk.ChatContextResource` and the `pinnedContextResources` builder), so grouping servers under a config would require a backend data change. Left flat for this PR. </details> --- 🤖 This PR was created by Coder Agents on behalf of @kylecarbs.	2026-06-22 21:39:34 -06:00
Kyle Carberry	0b856ef637	fix(site/src/pages/AgentsPage/components/DiffViewer): dedupe diff files to prevent CodeView duplicate id crash (#26597) ... Viewing a git diff on `/agents` could crash the whole diff view with `CodeView.addItem: duplicate id "agent/x/agentmcp/api_internal_test.go"`. When a diff body lists the same post-image path in more than one `diff --git` section, `parsePatchFiles` returns one `FileDiffMetadata` per section. `DiffViewer` then maps each file to a `CodeView` item keyed by `file.name` (the file tree is likewise keyed by path), so the repeated path produced two items with the same id and `CodeView.addItem` threw, tearing down the entire view. Deduplicate the parsed files by path in `useParsedDiff`, keeping the first occurrence, so both the `CodeView` and the file tree always receive unique ids. `useParsedDiff` is the single source feeding both panels, so a malformed diff now degrades gracefully and logs one warning instead of crashing. <details> <summary>Root cause and verification</summary> Confirmed against `@pierre/diffs` `parsePatchFiles` that a single patch with two `diff --git` sections for one path yields `patchCount: 1`, `fileCount: 2` with both entries named `agent/x/agentmcp/api_internal_test.go`. That is the exact input that made `CodeView.addItem` throw. Dropping (rather than merging) duplicates matches the renderer's model, which can only show one item per id anyway. Verified on the branch: `pnpm test` (DiffViewer suite, including the new `dedupeFilesByName` tests), `tsc -p .`, and the full `pnpm lint` (biome, types, circular-deps, React Compiler, knip) all pass. </details> > Opened by Coder Agents on behalf of @kylecarbs.	2026-06-22 22:34:54 -05:00
Kyle Carberry	cd56ab9e33	refactor: remove legacy live-read and injected-history chat context paths (#26585) ... This PR makes the agent-pushed pinned snapshot (`chat_context_resources`) the sole source of workspace context for chats, completing the "Release 5" cleanup. It removes legacy mechanisms now superseded by the snapshot that agents push over dRPC (`PushContextState`) and refresh via `chat-context/refresh`. Removed: - **Live-read at turn time.** MCP tool discovery, skill live-body reads, and the instruction/skill history fallback that dialed the workspace on every turn. - **Context injected as message history.** The `persist_workspace_context` generation action and its decision-loop guard. - **The legacy write path.** `POST`/`DELETE /api/v2/workspaceagents/me/experimental/chat-context`, the agentsdk `AddChatContext`/`ClearChatContext` methods, and the CLI one-shot writer. - **The `chats.last_injected_context` column** and all of its plumbing (migration `000529`, queries, `db2sdk`, `dbauthz`, audit table, and the frontend `ContextUsageIndicator` fallback). Subagent context inheritance no longer copies parent context messages; children now hydrate the parent's pinned `chat_context_resources` on create, which yields an identical pin for the same workspace and agent. What stays (still served by the live agent connection, not the snapshot): `read_skill_file` supporting-file reads, `read_skill` supporting-file listing, and MCP tool execution. > [!NOTE] > Migration `000529` drops `chats.last_injected_context` and recreates the `chats_expanded` view without it. The down migration restores both. <details> <summary>Decision log (D1-D5)</summary> - **D1 (subagent inheritance):** Re-point inheritance from the legacy message copy to a pinned hydrate. Children call `hydrateChatContextOnCreate` instead of copying parent context messages. - **D2 (`persist_workspace_context`):** Remove the generation action entirely along with the decision-loop guard it existed to satisfy, since context is never injected into history anymore. - **D3 (legacy HTTP + CLI):** Remove the experimental `chat-context` POST/DELETE endpoints, the agentsdk methods, and the CLI one-shot. The dRPC push + `chat-context/refresh` replace them. - **D4 (frontend fallback):** Remove the `last_injected_context` fallback in `ContextUsageIndicator`; pinned `resources` are the sole source. - **D5 (sequencing):** Ship as a single PR rather than a stacked pair. </details> --- Coder Agents generated on behalf of @kylecarbs.	2026-06-22 19:26:34 -06:00
Kyle Carberry	78c5ab96c9	feat(coderd/x/chatd): serve workspace MCP tools and read_skill from pinned context (#26581) ... This wires the last two consumer-side gaps of the agent-pushed workspace context refactor. coderd already hydrates each chat's pinned context (`chat_context_resources`), and `resolveTurnWorkspaceContext` already prefers the pin for instruction files and skill metadata. This change extends that preference to workspace MCP tools and the `read_skill` body. Workspace MCP tools are now built from the chat's pinned `mcp_server` resources instead of a live `ListMCPTools` pull. `resolveWorkspaceMCPTools` prefers the pin and falls back to live discovery for chats whose agent has not reported context yet, gated the same way as the instruction/skills pin: the pin wins whenever the chat has any pinned rows, so a workspace with no MCP servers contributes no tools rather than resurrecting stale ones. Because the agent reports tool names unprefixed, each tool is re-prefixed to the `{server}__{tool}` form and the pushed JSON Schema is split into `properties` and `required` so the result matches what live discovery produced. Calls still proxy through the workspace agent connection; the snapshot carries tool definitions, not a way to execute them. `read_skill` now serves a workspace skill's `SKILL.md` body from the pinned snapshot (`SkillMeta.Meta`) instead of dialing the agent, so a pinned chat keeps returning the same instructions even when the workspace is unreachable. The supporting-file list stays a best-effort live lookup, since the snapshot carries only the meta file per the agent push contract. The legacy live paths remain as the fallback for agents that have not pushed context; RFC Release-5 cleanup of those paths is out of scope here. <details> <summary>Implementation plan and decisions</summary> ### Background The agentcontext refactor is mostly shipped across earlier PRs (#25983, #26526, #26533, #26577, #26570, #26573): the agent resolves instruction files, skills, and MCP servers into a snapshot, pushes it via `PushContextState`, and coderd hydrates each chat's pinned context (`chat_context_resources`). This PR closes the two remaining consumer-side gaps. ### Key facts established from the code - Pushed MCP tool names are **unprefixed** (`mcprunner` stores `tool.Name`); the agent MCP proxy and `CallMCPTool` expect the `{server}__{tool}` form (`agentmcp.ToolNameSep`). The pinned path reconstructs the prefix for execution, matching the model-facing names the legacy path produced. - Legacy `agentmcp` sets `MCPToolInfo.Schema = InputSchema.Properties` and `Required = InputSchema.Required` separately. The pushed `input_schema` is the full JSON Schema object, so the pinned builder extracts `properties` and `required` to match that shape. - `SkillMetaBody.meta` is the verbatim SKILL.md. The supporting-file list is **not** in the snapshot, so it is fetched live on demand (best-effort). - Gating mirrors `resolveTurnWorkspaceContext`: the pin wins when the chat has any pinned rows; otherwise the live path is used. ### Changes 1. `chattool/skill.go`: add `SkillMeta.Meta []byte`; extract `listSkillFiles` from `LoadSkillBody`; in `readWorkspaceSkillBody`, when `Meta` is present, parse the body from it without dialing and list files best-effort, else use the legacy live read. 2. `context_prompt.go`: populate `SkillMeta.Meta` in `contextResourcesToPrompt`; add `workspaceMCPToolInfosFromResources` (pinned `mcp_server` rows to `[]workspacesdk.MCPToolInfo` with prefixed names and split properties/required) and `splitMCPInputSchema`. 3. `chatd.go`: add `pinnedWorkspaceMCPTools` (build tools from the pin, ok-gated) and `resolveWorkspaceMCPTools` (pin-first, fall back to `discoverWorkspaceMCPTools`). 4. `generation_preparer.go`: call `resolveWorkspaceMCPTools` instead of `discoverWorkspaceMCPTools`. ### Tests - `chattool/skill_test.go`: read_skill serves the pinned body without dialing, lists files via LS, and still returns the body when the workspace is unreachable. - `context_prompt_internal_test.go`: `SkillMeta.Meta` is populated; `workspaceMCPToolInfosFromResources` prefixing/properties/required/skip behavior; `pinnedWorkspaceMCPTools` ok-gating and fallback dispatch. </details> --- *Opened by Coder Agents on behalf of @kylecarbs.*	2026-06-22 13:13:27 -06:00
Kyle Carberry	966dd89537	feat: add chat context source CLI and agent-token refresh (#26577) ... Adds the `coder exp chat context` CLI for managing workspace context sources, plus the agent-token refresh endpoint the in-workspace refresh relies on. Part of breaking the "Workspace Context Sources for Coder Agents" RFC (#26466) into small, reviewable PRs. ## What this adds **CLI (`coder exp chat context`)**, talking to the agent's local IPC socket from inside the workspace: - `list` lists the registered scan roots (built-in defaults are not shown). - `show <path>` shows a source and the resources the agent resolves from it, including failures. - `add <path>` registers a path as an additional context source. With `--chat`, it keeps the legacy one-shot behavior (read context from the path once and inject it into a single chat). - `remove <path>` unregisters a source. - `refresh [<chat>]` re-pins chat context to the agent's latest snapshot. **Agent-token refresh path** for the no-argument `refresh`: - `refresh <chat>` uses the existing user-facing `ExperimentalClient.RefreshChatContext` (already on main) and works from anywhere. - `refresh` with no argument runs inside the workspace: it re-resolves the agent's sources over the context socket (catching freshly-cloned repos and startup-script writes), then asks the agent, authenticating with its own token, to re-pin every drifted chat. No `coder login` required. - This adds `agentsdk.RefreshChatContext` and `POST /api/v2/workspaceagents/me/experimental/chat-context/refresh` (`workspaceAgentRefreshChatContext`), mirroring the existing clear endpoint's agent-token auth model. ## Testing - `go test ./cli` (`TestExpChatContextAdd`, `TestParseChatID`, `TestResolveContextSourcePath`) - `go test ./coderd/x/chatd -run TestChatContextRefreshFromAgentToken` (end-to-end: echo-provisioned agent pushes a snapshot, drifts a bound chat, the agent-token refresh re-pins it, and an agent-less chat stays untouched) - `go build ./...`, `go vet`, `golangci-lint`, `make gen` (no generated changes; experimental commands are excluded from CLI golden/doc generation) <details> <summary>Design notes</summary> This is **Split 4** of #26466. Split sequence: 1. #26558 - prompt pin consumption (merged) 2. #26570 - `codersdk` context resource types (merged) 3. #26573 - the context indicator UI (merged) 4. **This PR** - the CLI + agent-token refresh. 5. The context diff (`changes`, `ChatContextResourceChange`, the changes dialog, `buildContentPatch`) - last. Key points: - The agent-local context subsystem (`agent/agentsocket` IPC for source CRUD, snapshot, resync), the user-facing `ExperimentalClient.RefreshChatContext`, and the per-chat `chatd.RefreshChatContext` all already exist on main, so this split is the CLI surface plus the small agent-token refresh endpoint that fans out per-chat refresh across an agent's drifted chats. - `add <path>` resolves relative paths to absolute before handing them to the agent (which requires canonical paths) but preserves a leading `~` for the agent to expand against its own home. `TestResolveContextSourcePath` covers this. - The agent endpoint is annotated `@x-apidocgen {"skip": true}`, matching the other agent-token chat-context endpoints. - No diff/changes rendering is involved; that lands in the final split. </details> *This PR was created by Coder Agents on behalf of @kylecarbs.*	2026-06-22 12:15:27 -06:00
Kyle Carberry	e6a9b59abe	feat(site/src): surface pinned chat workspace context in the UI (#26573) ... Adds the workspace-context indicator UI for agent chats, part of breaking the "Workspace Context Sources for Coder Agents" RFC (#26466) into small, reviewable PRs. ## What this adds - **Pinned context popover**: the context-usage indicator now lists the chat's pinned resources, instruction files, skills, and MCP servers with their tools. Unusable resources (invalid skill, unreadable/oversize file) are surfaced in an "Issues" section with their error rather than dropped silently. - **Drift and error states**: when the pinned context differs from the agent's latest snapshot, the ring shows a warning marker and the popover explains the drift. A snapshot-level error gets a distinct treatment. - **Refresh context**: a button re-pins the chat to the agent's latest snapshot via `PUT /api/experimental/chats/{id}/context`. - **Live updates**: `context_dirty` watch events apply the lightweight dirty flags across the cached chat lists and refetch the open chat so the full pinned detail loads. ## Not included The context **changes/diff** view (the "View changes" affordance and its dialog) is intentionally deferred to a later split, so this PR contains no diff rendering. ## Testing - `pnpm lint` (types, biome, circular deps, React Compiler, knip) - `pnpm test src/api/queries/chats.test.ts` (cache-merge unit tests, including the new `context_dirty` cases) - `pnpm test:storybook src/pages/AgentsPage/components/ContextUsageIndicator.stories.tsx` (4 stories) - `pnpm format` <details> <summary>Design notes</summary> This is **Split 3** of #26466. Split sequence: 1. #26558 - prompt pin consumption (merged) 2. #26570 - `codersdk` context resource types (merged). During review the type was renamed `ChatContextMCPTool` -> `ChatContextTool` and the field `mcp_tools` -> `tools`; this PR consumes those merged names. 3. **This PR** - the UI. 4. CLI `coder exp chat context` source CRUD + `refresh` (next). 5. The context diff (`changes`, `ChatContextResourceChange`, the changes dialog, and `buildContentPatch`) - last. Key decisions: - The backend already publishes `ChatWatchEventKindContextDirty` and exposes the refresh endpoint (#26389). Watch/pubsub payloads stay lightweight: they carry only the `dirty`/`dirty_since`/`error` flags and omit `resources`. So `mergeWatchedChatSummary` merges (not replaces) the cached context to preserve the pinned `resources` a single-chat GET populated, and the `AgentsPage` watch handler refetches only the open chat to pull the full pinned detail. - The indicator prefers the chat's pinned `resources`; while they have not loaded it falls back to the agent's `last_injected_context`, skipping the empty context-file placeholder so it never renders a nameless row. - All diff/changes rendering is excluded here and lands in the final split to keep this PR focused on the read-only pinned view and the refresh action. </details> *This PR was created by Coder Agents on behalf of @kylecarbs.*	2026-06-22 11:09:39 -06:00
Kyle Carberry	c0f854c289	feat: report pinned chat context resources on chat API (#26570) ... Surfaces a chat's pinned workspace-context resources on the single-chat GET and refresh responses, so clients can show *what* context the prompt was built from, not just whether it drifted. ## What's included - **codersdk**: `ChatContextResource` (plus `ChatContextResourceKind` and `ChatContextResourceStatus`) and `ChatContextMCPTool`, and a new `Chat.Context.Resources` field (metadata only, no bodies). It is populated only on the single-chat GET/refresh response; list and watch payloads stay nil to remain lightweight. - **coderd/x/chatd**: `Server.ContextResources`, which builds the metadata-only list from the chat's pinned `chat_context_resources` rows. Non-OK resources (invalid / unreadable / oversize / excluded) are reported with their status and error so the UI can explain why a resource was dropped from the prompt instead of silently omitting it. The shared protojson body decoders are extracted so the prompt and detail paths reuse them. - **coderd**: `getChat` and `refreshChatContext` enrich the response with the resource list. Failures are non-fatal (the chat stays usable without the detail). ## Scope / what's deferred This is an incremental split from #26466. This PR reports only the **resource inventory**. The pinned-context drift *diff* (the per-source `changes` set and the "View changes" dialog) is intentionally deferred to a later split; the existing `dirty` bit already signals that context changed. MCP resources are reported for display only; they are not injected into the prompt (a future RFC item). <details> <summary>Design notes</summary> - The resource list is the chat's full pinned inventory (instruction files, skills, and MCP configs/servers), preserving the query's `source ASC` order. OK-but-empty instruction files, OK skills with no name, and untracked kinds (reserved plugin/hook/subagent/command) are skipped. - MCP tool names are reported with the agent's `"<server>__"` prefix stripped so they read as the server exposes them. - The detail is computed on read and attached only on the single-chat GET and refresh responses; list and watch payloads omit it to stay lightweight. - `refreshChatContext` enriches its own response (mirroring `getChat`) so the client reflects a refresh immediately, without a full reload. </details> <details> <summary>Testing</summary> - `go test ./coderd/x/chatd/ -run 'TestPinnedContextResources|TestContextResources|TestChatContextDirtyFromAgentPush'` (unit + integration on embedded Postgres) passes. The integration test exercises the GET and refresh enrichment end-to-end. - `go build`, `go vet`, `golangci-lint`, and `gofmt` are clean. - `make gen` regenerated `apidoc`, `swagger.json`, `docs/reference/api/*`, and `typesGenerated.ts`. </details> --- *This PR was created by Coder Agents on behalf of @kylecarbs.*	2026-06-22 10:00:15 -06:00
Kyle Carberry	46916bf899	feat(coderd/x/chatd): consume the pinned chat context in prompt generation (#26558) ... ## What `prepareGeneration` now builds the system-prompt instruction block and workspace skills from a chat's **pinned context copy** (`chat_context_resources`, populated in #26438) instead of re-scanning per-turn history, when the chat has a pinned copy. This is the first production reader of the pin. Selection is **presence-based, no experiment**: a chat with pinned rows builds its prompt from the pin; a chat without them falls back to the existing per-turn history path. The two paths are mutually exclusive, so older agents that never report context keep their current behavior and the per-turn pull stays as the fallback. ## How - `contextResourcesToPrompt` maps the protojson resource bodies (instruction files and skills) into the instruction block and skill metadata, skipping non-OK statuses, non-prompt body kinds, and malformed bodies (the malformed count is logged so a proto/encoding regression cannot silently drop context). - `pinnedWorkspaceContext` reads the pin and reports `ok=false` (history fallback) when there are no pinned rows; read errors propagate. The bound agent only decorates the instruction header with OS and directory, so the pin still resolves when the workspace is unreachable. - `resolveTurnWorkspaceContext` dispatches between the pinned and history paths; `prepareGeneration` calls it. ## Testing - `go test ./coderd/x/chatd/` for `TestContextResourcesToPrompt`, `TestPinnedWorkspaceContext` (incl. `...FromHydratedPin` against real Postgres), and `TestResolveTurnWorkspaceContext`: pass. - `make gen` (no drift), `golangci-lint`, `gofmt`, emdash scan, and `go build`/`go vet` on `./coderd/x/chatd/...`: all clean. ## Scope This is the foundational backend slice split from #26466 (the full-stack staging PR). It changes no API surface, schema, proto, or generated files. The remaining pieces land as follow-ups in dependency order: 1. `ChatContext` drift/diff API (`resources` + `changes`, `ContextDetail`). This also extracts the body decoders inlined here so they are shared with the diff path. 2. Context-ring drift indicator, changes dialog, and refresh (UI). 3. In-workspace `coder exp chat context` source CRUD and `refresh` (CLI). <details> <summary>Why this is the first split</summary> The coderd hydration, the `PUT /chats/{id}/context` refresh endpoint (#26389), the `chat_context_resources` table (#26430), and the copy-into-pin logic (#26438) are already merged, as is the agent-side push (#26526, #26533). Consuming the pin in prompt building is the step #26438 explicitly deferred, and it is the bottom of the remaining dependency stack: the drift/diff API, the UI indicator, and the CLI are only meaningful once the chat actually builds its prompt from the pin. Keeping it presence-based means it is independently revertable and leaves the per-turn pull intact as a fallback, matching the RFC's Release 3 rollout. The files are taken verbatim from the reviewed #26466 boundary commit (before the diff-API work began), so the deep-review feedback already applied there (CRF-1 through CRF-10) is preserved. </details> --- *This PR was created by Coder Agents on behalf of @kylecarbs.* Split from #26466.	2026-06-22 08:51:57 -06:00
Kyle Carberry	2f8bba792a	feat: push MCP server context and tools from agentcontext (#26533) ... ## What Live MCP servers and their tools now flow into the `agentcontext` snapshot and are pushed to coderd via `PushContextState`, stored alongside instruction files and skills. Previously the resolver's MCP seam was unimplemented, so live MCP tool lists never reached the pushed snapshot. `agentcontext` is now **fully self-contained** for MCP: it connects to the MCP servers declared in the `.mcp.json` files its own watcher already discovers, lists their tools, and emits `KindMCPServer` resources. It does **not** depend on or modify `agent/x/agentmcp` — that package is left pristine and keeps serving the agent's MCP HTTP API. The two MCP paths run independently, which means the legacy package can be deleted later without touching this code. ## How - **Self-contained runner** (`agentcontext/mcprunner.go`): a one-shot MCP client (connect → initialize → list tools → close) with its own `.mcp.json` parser. A Manager goroutine (`runMCPSync`) reloads it whenever the discovered `KindMCPConfig` `path:contenthash` set changes, then re-resolves so the new tools are published. Per-server connects run in parallel (bounded) with a per-server timeout; a server that fails to connect is recorded as a failure rather than aborting the batch. Each connect also force-kills its subprocess on close, because mcp-go's stdio `Close()` closes stdin and then blocks on `cmd.Wait()` with no kill — a server that ignores stdin-close would otherwise stall the whole reload loop. - **Resource production** (`agentcontext/mcp.go`): `buildMCPServerResources` turns the runner's non-blocking per-server snapshot into `KindMCPServer` resources. Connected servers carry their sorted tools (`StatusOK`); failed servers surface as `StatusUnreadable` issues instead of vanishing; connected-but-no-tools-yet are skipped until a later reload. The content hash is tool-set sensitive. The resolver consumes this through a plain `MCPResources func() []Resource` field (no `MCPProvider` interface). - **Tool names**: emitted exactly as the server reports them. Flattening into a single namespace (e.g. `server__tool`) is left to the control plane in the next step, since each resource already carries the server name. - **Drift**: MCP resources are excluded from the snapshot aggregate/drift hash (`driftResources`). MCP servers connect asynchronously after boot; without this, a server finishing its connect would dirty every hydrated chat even though nothing the user pinned changed. - **Wiring** (`agent.go`): the manager is given `ManagerOptions.MCPExecer`/`MCPUpdateEnv`; `agent/x/agentmcp` is untouched. - **Config validation**: a structurally broken `.mcp.json` surfaces as `StatusInvalid` rather than silently dropping all its servers. coderd already persists `mcp_server`/`mcp_config` resource bodies (including tools), so no coderd or proto changes were required. ## Testing - **Unit**: `buildMCPServerResources` (grouping/sort/skip/failed/hash sensitivity), MCP resources applied via the resolver seam, MCP exclusion from the aggregate hash, `.mcp.json` parsing (transport inference, env expansion), `toolInputSchema`, and `mcpConfigSet` change detection. - **Proto serialization** (`TestDRPCPusher_HappyPathSerializesAllFields`): a `KindMCPServer` resource (tools + input schema) round-trips through `PushContextState` into the `MCPServerBody` wire form, asserting the server name, tool name/description, and the decoded `input_schema`. - **Manager-level, real subprocess** (`TestManager_MCPServerToolsInSnapshot`): a `.mcp.json` points at a re-exec'd fake stdio MCP server; the runner connects it and its `echo` tool surfaces as a `KindMCPServer` resource in the Manager snapshot — the same snapshot pushed to coderd — exercising `runMCPSync` and the resolver wiring end to end. - **Regression** (`TestManager_MCPServerHangingCloseDoesNotStall`): the fake server ignores stdin-close; the test asserts its tool still surfaces, proving the runner force-kills the subprocess instead of stalling the reload. Verified to fail without the fix. - All pass under `-race`; `go build ./...`, `go vet`, and `golangci-lint` are clean on the touched packages. ## Scope / follow-ups This is the agent-side production+push half. The chatd consumer (reading the pinned MCP resources for prompt/tool injection, including any server-prefix flattening of tool names) and removing the legacy `workspaceMCPToolsCache` pull path remain follow-ups, per the RFC rollout. While both `agent/x/agentmcp` and `agentcontext` exist, stdio MCP servers are spawned by both; this is intentional and temporary until `agentmcp` is removed. <details> <summary>Implementation plan and decisions</summary> **Goal:** produce live MCP server resources (with tools) from `agentcontext` and push them to coderd. **Starting state (main):** proto (`PushContextState`, `MCPServerBody`, `MCPTool`), the drpc adapter, coderd storage (`workspace_agent_context_resources`, body kind `mcp_server`), and the resolver's MCP seam already existed; nothing implemented the seam or fed live tools into the snapshot. **Decision (agentcontext fully separate from agentmcp):** `agentcontext` starts and lists its own MCP servers using only the connect-and-list half of an mcp-go client, driven by the `.mcp.json` files its existing watcher discovers. It shares no state with `agent/x/agentmcp` and does not import it. Two earlier revisions of this branch were discarded: (1) relocating `agentmcp` into `agentcontext` (rejected — it duplicates config parsing and file watching `agentcontext` already does); (2) reading `agentmcp`'s cached server snapshot via new accessors (rejected — unnecessary coupling between two packages that should simply run independently while one is being retired). The temporary double-spawn of stdio servers is the accepted cost of keeping the two paths cleanly separated until `agentmcp` is removed. **Decision (no tool-name prefixing, no MCPProvider interface):** the agent pushes raw, unflattened data — server name plus verbatim tool names — and lets the control plane own any `server__tool` flattening. With a single self-contained producer, the `MCPProvider` interface was collapsed into a `func() []Resource` field on the resolver. **Invariants held:** no secrets (env/headers) in pushed resources, only server/tool metadata; MCP excluded from the drift hash; the seam is non-blocking so the resolver never stalls on MCP I/O. </details> --- *This PR was created by Coder Agents on behalf of @kylecarbs.*	2026-06-21 16:31:05 -06:00
Kyle Carberry	992b1ffed1	feat(agent): serve context sources over the agent socket (#26526) ... ## Overview Split from #26466, scoped to **agent-only** changes. This PR exposes the agent's context sources and snapshots over the existing agent socket. There are no changes outside `agent/`. ## What's included - **agentsocket**: context source CRUD (`ContextSources`, `GetContextSource`, `AddContextSource`, `RemoveContextSource`) plus `GetContextSnapshot` and `ResyncContext` RPCs, with matching client methods and proto. The server receives the context `Manager` via `WithContextManager` and returns a clean error when it is absent. - **agentcontext**: the resync JSON response now carries the per-resource `Name`, keeping the HTTP resync payload in sync with the drpc `PushContextState` path in `agentsocket`. - **agent**: passes the context `Manager` to the socket server via `WithContextManager`. ## What's intentionally NOT here - No MCP wiring. There are no MCP additions in `agent.go` or `agentcontext`. MCP ownership will land later in `agentcontext`; this PR does not build on `agent/x/agentmcp`. - No changes to `agent/x/agentmcp` or the `agentcontext` resolver. The socket serves whatever context resources the `Manager` already resolves. <details> <summary>Context for reviewers</summary> This is one of several PRs split out of #26466. Earlier revisions also wired live MCP servers through the socket; that scope was removed so this PR stays purely socket + context plumbing inside `agent/`. The agentcontext resolver, `agent/x/agentmcp`, and `agent.go` MCP startup behavior are unchanged from `main`. </details> --- _Created by Coder Agents on behalf of @kylecarbs._	2026-06-18 13:00:28 -07:00
Kyle Carberry	1c78bd84b7	feat(coderd): copy agent context resources into the per-chat pin (#26438) ... ## What Populates `chat_context_resources` (the per-chat pinned copy added in #26430) by copying from `workspace_agent_context_resources` at the points where a chat's `context_aggregate_hash` is set, in the same transaction, so the pinned hash and pinned bodies always agree. No prompt-building change yet; consuming the pinned copy in `prepareGeneration` is a later, experiment-gated PR. ## How - `HydrateAgentChatsContext` now hydrates NULL-hash chats **and** copies the agent's resources onto them in one statement (a data-modifying CTE), so the chat-create and agent-push paths need no Go change. - New queries `InsertAgentContextResourcesIntoChat`, `DeleteChatContextResources`, `ListChatContextResources`, each with a hand-written dbauthz wrapper (per-chat update/read) and a `MethodTestSuite` entry. - `RefreshChatContext` re-pins resources via a shared `repinChatContext` helper (clear-then-copy in a transaction). A dirty chat keeps its old bodies until refresh. - On agent rebind (e.g. a workspace rebuild produces a new agent), the chat's context is re-pinned to the new agent so it stops injecting the previous agent's resources. Best-effort: a context error never fails the binding. ## Invariant A chat's `chat_context_resources` always correspond to its `context_aggregate_hash`. Bodies are (re)written only when the hash is set (hydrate, refresh, rebind); a dirty chat keeps its old bodies until refresh. ## Testing Extends the context integration test to push real resources and assert the copy across hydrate, dirty (no re-copy), and refresh. The dbauthz `MethodTestSuite` covers the three new methods. <details> <summary>Why clear-then-copy (two statements)</summary> The refresh/rebind re-pin clears the chat's rows then inserts the agent's. It uses two sequential statements inside the transaction rather than a single `WITH cleared AS (DELETE ...) INSERT ...`, because a data-modifying CTE cannot see its own delete under snapshot isolation, so overlapping sources (the common case: the same files re-pinned) would collide on the `(chat_id, source)` primary key. The hydrate path inserts into never-pinned (NULL-hash) chats and uses `ON CONFLICT DO UPDATE` defensively. </details> <details> <summary>Follow-ups</summary> - `prepareGeneration` consuming the pinned instructions and skills (experiment-gated). - `codersdk.ChatContext` resources plus changed diff, and the frontend indicator/refresh. - Removing the per-turn pull and `last_injected_context`. </details> --- *This PR was created by Coder Agents on behalf of @kylecarbs.* Builds on #26430.	2026-06-17 00:10:07 -07:00
Kyle Carberry	53a6459ecd	feat(coderd/database): add chat_context_resources table (#26430) ... Adds chat_context_resources: a per-chat pinned copy of the agent context resources a chat is hydrated against. The agent-side table (workspace_agent_context_resources) is last-writer-wins with no history, so a chat copies its resources at hydration/refresh to keep a stable view while the agent drifts. Schema foundation only (no queries/dbauthz/prepareGeneration/SDK yet). chat_id FK ON DELETE CASCADE for cleanup parity; no agent FK so the pin survives agent replacement; PK (chat_id, source); reuses the 000522 enum types.	2026-06-16 14:28:57 -07:00
Kyle Carberry	bca0ce04ca	feat: integrate agent context snapshots into chats (#26389) ... Makes the chat context foundation from #26385 live. That PR added the storage columns, writer queries, and a dormant `agentapi.ContextDirtyMarker` trigger with no production callers; this PR wires them together end to end. When a workspace agent pushes a context snapshot, bound chats now hydrate to that snapshot's hash, and a later push with a different hash flips already-pinned chats to dirty (emitting a `context_dirty` watch event after the transaction commits). Chat creation pins the agent's latest snapshot when one already exists. The experimental chat API exposes this as `Chat.Context` (`*ChatContext` with `dirty`, `dirty_since`, `error`), and a new `PUT /api/experimental/chats/{chat}/context` endpoint re-pins the agent's latest snapshot and clears the dirty marker. `context_dirty_resources` stays NULL (the resource-level diff is deferred to the UI phase) and the live per-turn context pull is unchanged. The end-to-end test provisions a workspace agent via the echo provisioner, connects it over the Agent API v2.10, and exercises the full path: an initial push hydrates a bound chat (clean), a second push with a different hash marks it dirty, the API reports the dirty state, and the refresh endpoint clears it. <details> <summary>Decision log</summary> - **API shape — sub-struct.** Dirty state is surfaced as `codersdk.Chat.Context *ChatContext { Dirty bool; DirtySince *time.Time; Error string }` rather than flat fields, matching the RFC's named `ChatContext` type and leaving room for future fields (resource diff, sources). `db2sdk.Chat` populates it when the chat is context-tracked (`len(ContextAggregateHash) > 0`), dirty, or carries a snapshot error, and leaves it nil (`omitempty`) otherwise. `Dirty` mirrors `context_dirty_since` being set. - **Marker wiring.** The chat daemon is injected directly as the `agentapi.ContextDirtyMarker`. It is unconditionally constructed (only its background worker is gated), so the marker is always non-nil and the wiring matches every other `api.chatDaemon` call site. `agentapi` still treats a nil marker as "chatd absent", so `PushContextState` stays a pure write path for any future caller that does not wire chatd in. - **Refresh is atomic.** `RefreshChatContext` reads the agent's latest snapshot and re-pins the chat in one repeatable-read transaction, so a concurrent push cannot land between the read and the write and leave the chat pinned to a stale hash with the dirty marker cleared. - **Hydrate + dirty run inside the push transaction.** The fan-out shares the push's transaction so a concurrent refresh cannot interleave with the version gate; `context_dirty` watch events publish only after commit. The pinned hash on dirtied chats is intentionally left unchanged — the refresh endpoint re-pins it. - **Dirtied chats keep their pinned hash.** Drift is advisory: a dirty chat stays usable, and refreshing is the only path that advances the pinned hash. - **Test binds `chats.agent_id` directly.** In production the binding is set lazily during a chat turn (`chatd.persistBuildAgentBinding`); the test sets it via `dbgen` so it exercises the context flow rather than turn resolution. Plan: `coderd/x/chatd` context integration + E2E (sub-struct API, create-time + push-time hydration, refresh endpoint; `context_dirty_resources` and the per-turn pull untouched). </details> 🤖 Generated by Coder Agents on behalf of @kylecarbs	2026-06-16 17:46:47 +00:00
Kyle Carberry	210261b143	feat: add chat context pinning storage and push trigger (#26385) ... Foundation for the Workspace Context Sources RFC (phase 3). The agent push (#25983) and coderd snapshot storage (#26145) already persist per-agent context snapshots; this PR lands the **chat-side storage** plus the **`agentapi` push trigger** that a follow-up will use to read them. It does **not** touch `chatd` and changes no behavior — nothing wires an implementation yet. ## What changed - Adds four nullable columns to `chats` — `context_aggregate_hash`, `context_dirty_since`, `context_dirty_resources`, and `context_error` — and rebuilds the `chats_expanded` view. - Adds three queries — `SetChatContextSnapshot`, `HydrateAgentChatsContext`, `MarkChatsContextDirtyByAgent` — with `dbauthz` wrappers and `audit` entries. They are store-interface methods covered by a Postgres test (`TestChatContextHydration`). - Adds the `agentapi.ContextDirtyMarker` interface and invokes it inside the `PushContextState` transaction, publishing collected events only after commit. ## Intentionally inert There are **no production callers** of the three queries and **no implementation** wired for `ContextDirtyMarker`, so the push trigger is dormant. This is deliberate: the PR is the durable storage/query foundation only. The actual integration — the `chatd` implementation that hydrates/dirties chats and backs a refresh endpoint, consuming the pinned context in prompt building, the rich SDK types + UI, and retiring the live per-turn pull — lands as a single follow-up PR. Splitting this way keeps the schema/query layer reviewable on its own and keeps the integration whole in one place. Refs #25983, #26145. <details> <summary>Decision log</summary> - **Columns over a side table.** The four `chats` columns are the durable model (accepting the one-time `chats_expanded` view/CTE churn). `last_injected_context` is deliberately left untouched — it is load-bearing for the live per-turn context pull. - **Keep `agentapi`, drop `chatd`.** The earlier revision wired the hydrate/dirty implementation through `chatd` and added a `PUT /chats/{chat}/context` refresh endpoint. Those were removed so this PR is pure foundation; `agentapi` defines the trigger + interface (it does not import `chatd`), and the `chatd` implementation arrives with the full integration. - **No new experiment flag.** The columns are dark and unread by prompt building. - **Authz.** The new query wrappers authorize chat updates under the chat RBAC object / `ResourceChat`, consistent with the existing system chat mutators. </details> --- 🤖 Generated by Coder Agents on behalf of @kylecarbs.	2026-06-15 14:41:00 -07:00
Kyle Carberry	b439b06ee6	feat: persist agent-pushed workspace context snapshots in coderd (#26145) ... Replaces the v2.10 `PushContextState` stub with a real coderd write path. Phase 1 of the chat-side persistence story; nothing reads these rows yet. Follows [#25983](https://github.com/coder/coder/pull/25983) and unblocks [CODAGT-569](https://linear.app/codercom/issue/CODAGT-569/enable-agent-api-v210-pushcontextstate-bump-currentminor-wire-coderd). ## What ships ### Schema (`000517_workspace_agent_context.{up,down}.sql`) Two new tables plus `api_key_scope` enum extensions: - `workspace_agent_context_snapshots` (PK `workspace_agent_id` to `workspace_agents(id) ON DELETE CASCADE`): one row per agent, overwritten per push. Holds `version`, `schema_version`, `aggregate_hash`, `snapshot_error`, `received_at`. - `workspace_agent_context_resources` (PK `(workspace_agent_id, source)`): per-resource state. `body_kind` and `status` are `TEXT` + `CHECK` so adding new wire kinds (the RFC's reserved PLUGIN/HOOK/SUBAGENT/COMMAND) is a one-line CHECK update plus a Go switch case. ### SQLC queries (`coderd/database/queries/workspaceagentcontext.sql`) - `UpsertWorkspaceAgentContextSnapshot` - `UpsertWorkspaceAgentContextResource` - `DeleteStaleWorkspaceAgentContextResources` (delete-where-source-not-in) - `GetLatestWorkspaceAgentContextSnapshot` - `ListWorkspaceAgentContextResources` ### Handler (`coderd/agentapi/context.go`) `ContextAPI` is a new sub-API. `PushContextState`: 1. Rejects `schema_version > 1` with a non-`Unimplemented` error so a forward-incompatible agent fails loudly during rollout instead of slipping into the permanent fallback path the `Unimplemented` translation reserves for old coderd deployments. 2. Validates resources: no empty/duplicate sources, every variant maps to a known body kind, every status maps to a known enum value, the `Body` oneof is set (even when status is non-OK, mirroring the wire guarantee so coderd can attribute failures to a known kind). 3. Inside `Database.InTx`, reads the existing snapshot. If the push is not `initial` and `version` is not strictly greater, returns `accepted = false` and leaves stored state untouched. Otherwise upserts the snapshot row, upserts each resource, then runs the stale-source prune so the snapshot and resource rows always agree. 4. Returns `accepted = true` on success. Resource bodies are stored as `protojson(body oneof variant)` in `body JSONB` with `body_kind` as the discriminator. Adding a new field to an existing variant is zero work since `protojson` tolerates new fields; adding a new variant is a CHECK + switch case. ### RBAC + dbauthz - New `ResourceWorkspaceAgentContext` (Create/Read/Update/Delete). - New `SubjectTypeAgentContext` plus `subjectAgentContext` system role and `dbauthz.AsAgentContext` helper. The push handler elevates to this subject; the agent's own role does not get direct write access to the table. - New `workspace_agent_context:*` API key scopes registered in the enum migration; internal-only (not added to `externalLowLevel`). ### Audit These rows are agent-pushed state, not user-authored. They are intentionally not added to `AuditActionMap` and not enumerated in `enterprise/audit/table.go`, matching `boundary_logs`, `workspace_agent_memory_resource_monitor`, etc. `enterprise/audit` tests pass unchanged. ## Tests - `coderd/agentapi/context_test.go`: 12 subtests covering accepts/rejects (schema version, empty/duplicate source, unknown status, missing body), version semantics (stale dropped, same-version replay dropped, `initial=true` overwrites lower version), variant coverage, non-OK status persistence, and the empty-active-set prune case. - `coderd/database/dbauthz/dbauthz_test.go`: 5 `MethodTestSuite` cases covering the new queries. - `coderd/rbac/roles_test.go`: `WorkspaceAgentContext` permission row asserting no human role currently has access. - `coderd/database/migrations/testdata/fixtures/000517_workspace_agent_context.up.sql`: one snapshot + one resource per known body kind plus a non-OK status, so the migration test suite never lands with these tables empty. ## Out of scope (later phases) - Chat hydration (`chats.context_aggregate_hash`, `last_injected_context`). - Dirty-bit fan-out and `PUT /chats/{id}/context`. - Agent-side `POST /api/v0/context/resync` barrier and the `coder exp chat context` CLI. - `codersdk` chat-context wire types and the dashboard Sources drawer. - Removal of the chatd per-turn pull fallback. ## Compat property This is a pure write path. If anything here returns errors the agent's `RunPush` loop backs off, no chat behavior changes, and the workspace keeps behaving exactly like it did before v2.10. <details> <summary>Implementation plan and decision log</summary> Key design calls: 1. **Concurrency**: Accept iff `req.Initial || req.Version > existing.Version`. The strict RFC reading ("version comparison is authoritative") locks restarted agents out because their per-process counter resets to 1; honoring `initial=true` reflects the real reboot reality while still rejecting steady-state replays/out-of-order pushes. 2. **Body encoding**: `protojson` over the oneof variant body proto, stored in JSONB with `body_kind` discriminator. Structured at the API/Go layer, schema-tolerant at the storage layer, and Phase 2 readers round-trip back via `protojson.Unmarshal`. 3. **Schema version rejection**: returns a normal error, not `Unimplemented`. The agent's `RunPush` loop only short-circuits on `Unimplemented`; that escape hatch is reserved for old coderd deployments. A forward-incompatible agent should retry-and-back-off, not flip the connection into permanent fallback. 4. **Validation strictness**: empty sources, duplicate sources, `STATUS_UNSPECIFIED`, and missing `Body` oneof variants are rejected before any write so a misbehaving agent cannot poison the snapshot table. Phase 2 readers can trust every row maps to a known proto variant. </details> _This PR was authored by Coder Agents on Kyle Carberry's behalf._	2026-06-15 09:38:52 -07:00
Kyle Carberry	dab1d3c81e	fix(cli): sort external auth env vars by numeric index (#26230)	2026-06-10 14:21:54 -07:00
Kyle Carberry	cc9f10fbf2	fix(agent/agentcontext): canonicalize scan root in symlink boundary check (#26175)	2026-06-09 19:21:55 -07:00
Kyle Carberry	cd3692c0c2	feat: add agent-side workspace context sources and Agent API v2.10 PushContextState (#25983) ... Adds the agent half of the workspace context sources RFC. The agent now resolves instruction files, skills, and MCP configs into a typed `Snapshot`, watches the relevant paths recursively, exposes the source list over a workspace-agent HTTP API, and pushes each `Snapshot` to coderd over a new `PushContextState` RPC on Agent API v2.10. The coderd-side handler is a stub returning `Unimplemented` for now. Real persistence to `workspace_agent_context`, chatd hydration on dirty events, and the `KindMCPServer` MCP provider are tracked by [CODAGT-569](https://linear.app/codercom/issue/CODAGT-569/enable-agent-api-v210-pushcontextstate-bump-currentminor-wire-coderd). This matches the pattern used for v2.7 `ReportBoundaryLogs` in [#21293](https://github.com/coder/coder/pull/21293), which bumped the version and shipped a stub server so the wire and client could iterate before the persistence layer landed. ## What ships ### agent/agentcontext (new package) - `Source`, `Resource` (kinds `instruction_file`, `skill`, `mcp_config`, `mcp_server` plus reserved `plugin`/`hook`/`subagent`/`command`), `ResourceStatus`, `Snapshot`, `ComputeAggregateHash`. - `Manager` owns the in-memory source list, performs the initial resolve synchronously in `NewManager`, runs a re-resolve/watcher loop in `Run`, exposes `AddSource`/`RemoveSource`/`Sources`/`HasSource`/`Snapshot`/`SubscribeChanges`/`Resync`/`SeedSources`/`Close`. - `Resolver` walks scan roots, classifies recognized files, enforces 64 KiB per-resource, 2 MiB aggregate, and 500-resource caps with `StatusOversize`/`StatusExcluded`/`StatusUnreadable`/`StatusInvalid` outcomes, skips `node_modules`/`vendor`/etc., validates symlink targets stay inside the scan root, stamps `SourcePath` on user-derived resources, and optionally pulls MCP server tool lists via an `MCPProvider` interface. MCP config resources ship metadata only (size, hash) so secrets in env blocks never leave the agent. - `Watcher` is a recursive `fsnotify` wrapper with a 250 ms debounce, dynamic arming of newly created directories, and an ENOSPC-tolerant degraded mode that no-ops further syncs until the manager resyncs explicitly. - HTTP API for `GET/POST /sources`, `GET/DELETE /sources/{path}`, `POST /resync` mounted at `/api/v0/context`. - `Pusher` interface plus `RunPush` goroutine with exponential backoff capped at 30 s. `DRPCPusher` adapts the generated `DRPCAgentClient210` to `Pusher` and translates `drpcerr.Unimplemented` to `ErrPushUnimplemented` so the push loop exits cleanly when talking to coderd deployments that have not enabled the real handler. ### agent/proto (v2.10) - New messages `ContextResource`, `PushContextStateRequest`, `PushContextStateResponse` and the `PushContextState` RPC on `service Agent`. - Generated `DRPCAgentClient210` interface and `codersdk/agentsdk.Client.ConnectRPC210` / `ConnectRPC210WithRole`. - `tailnet/proto.CurrentMinor` bumped from `9` to `10`. ### Agent wiring - `agent.Options.Client` declares both v2.9 and v2.10 connectors; `run()` dials with `ConnectRPC210WithRole`. - `apiConnRoutineManager` holds a `DRPCAgentClient210`. Existing v2.8 routines keep their narrower `DRPCAgentClient28` signature thanks to interface embedding. - `startAgentAPI210` is the v2.10 counterpart to `startAgentAPI` for routines that need the new client. The push context state routine uses it. - A `contextManager` is constructed in `agent.init()`, seeded from the existing `CODER_AGENT_EXP_*_DIRS` env vars, started in its own goroutine under `gracefulCtx`, and closed in `agent.Close`. - `handleManifest` calls `Manager.SeedSources` for sources rooted at the manifest directory, then `Resync` after `manifest.Swap`, so the snapshot reflects the workspace working directory immediately instead of waiting for the next filesystem event. - HTTP routes mounted at `/api/v0/context` when the manager is up. ### Coderd stub `coderd/agentapi/context.go` returns `drpcerr.Unimplemented` for `PushContextState`. The real handler that persists `workspace_agent_context` rows, hydrates chats, and emits dirty events lives in CODAGT-569. ## Tests 24 tests across `agent/agentcontext` cover types, paths, resolver behavior with file caps, skill containers, MCP secret omission, symlink target validation, the recursive watcher firing on real fsnotify events, manager source CRUD / `Resync` / `SeedSources` / `Run` lifetime, the HTTP API, the DRPC adapter, and the push retry / initial-flag / unimplemented paths. Passes `go test -race -count=2`. `TestAgent_ContextStatePushed` boots a full agent against `agenttest.FakeAgentAPI` (which now records `PushContextState` traffic) and asserts the seeded `AGENTS.md` appears in a snapshot push with `schema_version = 1`. <details> <summary>Notes for reviewers</summary> - Source CRUD is workspace-agent-token only; coderd is not in the path for source mutation. - Per-resource cap 64 KiB, aggregate 2 MiB, count cap 500; resources past the cap ship with `StatusExcluded` and an empty payload so the aggregate hash still detects content edits. MCP-emitted resources enforce both a per-provider count cap and the aggregate byte cap. - Symlinks inside the scan root are followed; symlinks pointing outside (or broken) are rejected with `StatusExcluded` so credentials reachable via a stray symlink stay off the wire. - The initial push gates `lifecycle = ready` in the eventual full design. For this PR the `SeedSources` plus `handleManifest`-driven `Resync` keeps the snapshot fresh; the live push loop ships now and DRPCPusher translates the coderd `Unimplemented` stub into a clean exit. - The `PLUGIN`/`HOOK`/`SUBAGENT`/`COMMAND` kinds are reserved in proto and Go enums but unused; the Claude Code plugin resolver ships in a follow-up that does not need a schema migration. - Two follow-ups remain, both tracked by [CODAGT-569](https://linear.app/codercom/issue/CODAGT-569/enable-agent-api-v210-pushcontextstate-bump-currentminor-wire-coderd): (1) the chatd-side handler that persists snapshots and dirties chats; (2) the `coder exp chat context` CLI command set for `list`/`show`/`add`/`remove`/`refresh`. </details> _This PR was authored by Coder Agents on Kyle Carberry's behalf._	2026-06-08 12:08:40 -07:00
Kyle Carberry	58f6b9c4d0	fix(coderd/externalauth): retry transient refresh failures with backoff (#25686) ... ## Summary Wraps external auth token refresh in an exponential-backoff retry so a brief upstream hiccup (5xx, network timeout, rate-limited 429) no longer surfaces as an `InvalidTokenError` and forces users to re-authenticate. GitHub in particular has been flaky enough lately that this is hitting real users. ## Behavior - `(*Config).RefreshToken` now calls a helper that retries the `TokenSource.Token()` exchange with exponential backoff (250ms → 2s), bounded by a 10s total budget. - Errors classified as permanent by `isFailedRefresh` (e.g. `bad_refresh_token`, `invalid_grant`, `unauthorized_client`, ...) skip the retry loop. Retrying a permanent failure wastes the refresh quota and, on providers with single-use refresh tokens, can mask a legitimate concurrent winner with repeated `bad_refresh_token` responses. - Refreshes with an empty refresh token still short-circuit without making an API call. - The existing concurrent-refresh-race detection and optimistic-lock paths are unchanged. ## Tunables Three new `time.Duration` fields on `externalauth.Config` (`RefreshRetryInitialBackoff`, `RefreshRetryMaxBackoff`, `RefreshRetryTimeout`) let callers override the defaults. They default to zero, which falls back to the package defaults, so existing call sites are unaffected. The fields exist primarily so tests can dial the timing way down without touching package globals (and therefore without serializing parallel tests). ## Tests - `TestRefreshToken/RefreshRetries` now disables internal retries via `RefreshRetryTimeout = time.Nanosecond` so its existing "1 IDP call per `RefreshToken` invocation" assertion still holds. Otherwise its assertions are unchanged. - New `TestRefreshToken/RefreshTokenWithBackoff` simulates 3 transient 5xx failures followed by success and verifies the refresh ultimately succeeds with 4 total IDP attempts. - New `TestRefreshToken/RefreshTokenBackoffPermanentError` returns `bad_refresh_token` and verifies the refresh is **not** retried even with a generous 1s budget. <details> <summary>Why the explicit <code>retryCtx.Err()</code> guard?</summary> `retry.Retrier.Wait` `select`s between `time.After(delay)` and `ctx.Done()`. The first call has `delay == 0`, so `time.After(0)` and an already-cancelled context both fire immediately and Go picks the case nondeterministically. Without the guard, a near-zero retry budget would still trigger an unwanted extra refresh attempt roughly half the time, which would have made the `RefreshRetries` test flaky. </details> This PR was opened by a Coder agent on behalf of @kylecarbs.	2026-05-26 15:35:22 -04:00
Kyle CarberryandCoder Agent	385146000b	feat: record created_at/completed_at on reasoning ChatMessageParts (#24789) ... Records reasoning start and end times on persisted reasoning `ChatMessagePart`s so reasoning duration can be computed for stored chats. Backend-only: no SSE changes and no frontend rendering ship in this PR. The `created_at` field on `ChatMessagePart` is extended to also be present on `reasoning` parts (it previously appeared only on `tool-call` and `tool-result`), and a new `completed_at` field is added for `reasoning` parts. ### How timestamps are recorded - `StreamPartTypeReasoningStart`: stamp `startedAt = dbtime.Now()` on the active reasoning state. - `StreamPartTypeReasoningEnd`: stamp `completedAt = dbtime.Now()` and append both into parallel `[]time.Time` slices on `stepResult`. - Persistence reads the slices in occurrence order (reasoning has no provider-side ID) and applies them to the matching `ChatMessagePart` via `buildAssistantPartsForPersist`. The first reasoning block's stamps go onto the first reasoning part, and so on. - `flushActiveState` flushes partial reasoning interrupted before `StreamPartTypeReasoningEnd` with `startedAt` from the active state and `completedAt = dbtime.Now()` at the interruption. ### Why two fields, not one? Tool calls and results are point events. The frontend computes their duration by subtracting the call's `created_at` from the result's `created_at`. Reasoning is one assistant part that brackets a span, so we record both endpoints on the part itself. ### Why not stamp in `PartFromContent`? Same rationale as #24101: `PartFromContent` is called during both SSE publishing and persistence. Stamping there would yield incorrect persistence-time timestamps for reasoning blocks that finished much earlier in the step. Instead we capture in the chatloop and apply during persistence. <details><summary>Implementation plan</summary> - `codersdk/chats.go`: extend `CreatedAt`'s `variants` to include `reasoning?`; add `CompletedAt *time.Time` with `variants:"reasoning?"`. - `coderd/x/chatd/chatloop/chatloop.go`: extend `reasoningState` with `startedAt`; extend `stepResult` and `PersistedStep` with parallel `[]time.Time` reasoning slices; stamp on `ReasoningStart`/`ReasoningEnd`; thread the slices through all `PersistStep` call sites including the interrupt-safe path; record partial reasoning in `flushActiveState`. - `coderd/x/chatd/attachments.go`: walk reasoning parts in occurrence order and apply `step.ReasoningStartedAt[i]` to `part.CreatedAt` and `step.ReasoningCompletedAt[i]` to `part.CompletedAt`. ### Tests - `codersdk/chats_test.go` round-trips `created_at` + `completed_at` on reasoning parts and verifies omission when absent and partial interrupted parts. - `coderd/x/chatd/chatprompt/chatprompt_test.go` asserts `PartFromContent(ReasoningContent{})` does NOT stamp timestamps. - `coderd/x/chatd/chatloop/chatloop_test.go` `TestRun_ReasoningTimestamps` drives a stream with two reasoning blocks and verifies parallel slices, monotonicity, ordering, non-zero values, and content-block ordering. `TestRun_InterruptedReasoningFlushesTimestamps` cancels mid-reasoning and verifies `flushActiveState` records a non-zero pair. - `coderd/x/chatd/attachments_test.go` covers `buildAssistantPartsForPersist` for normal interleaved reasoning, partial (zero `completed_at`), and missing slices. </details> > Generated by Coder Agents. Co-authored-by: Coder Agent <agent@coder.com>	2026-05-18 12:30:30 -04:00
Kyle Carberry	159089686a	fix(coderd/x/chatd): prime workspace MCP cache after create/start (#25298) ... ## Problem Mid-turn workspace MCP discovery was broken when an agent was still cold-starting. `PrepareTools` in `chatd.go` flipped `workspaceMCPDiscovered = true` *before* calling `discoverWorkspaceMCPTools`, so a failed discovery attempt permanently blocked retries within the turn. Customer-reported repro: - New chat with no pre-selected workspace. - LLM calls `create_workspace` mid-turn at `23:35:05`. - `PrepareTools` fires, dials the agent with a 30s timeout, dial times out at `23:38:15`, `discoverWorkspaceMCPTools` returns empty. - Agent connects at `23:38:29`, 14 seconds later. - `workspaceMCPDiscovered` was already true, so `PrepareTools` never retried for the rest of the turn. MCP tools only appeared on the next user message. A naive retry loop in `PrepareTools` would also miss the bigger picture: a workspace boot can take several minutes (EC2 cold start, 10 min startup scripts), and the chatloop only gets a chance to call `PrepareTools` between LLM steps. ## Fix Do the workspace MCP discovery from inside the tool that already waits for the agent. `chattool.CreateWorkspace` and `chattool.StartWorkspace` call `waitForAgentReady`, which has a 2 min agent-online budget plus a 10 min startup-script budget. By the time they fire `OnChatUpdated`, the agent is `Ready`. The chatd `onChatUpdated` callback now launches an async `primeWorkspaceMCPCache` goroutine on every bind that has a valid workspace ID: - The primer calls `discoverWorkspaceMCPTools` until it returns a non-empty list or `workspaceMCPPrimeMaxWait` (30s) elapses, with a 2s backoff between attempts. The bounded wait handles the short race between agent-online and the agent's MCP `Connect` settling. - The primer runs asynchronously so the tool itself never blocks. Some templates simply do not advertise MCP tools, in which case the primer would otherwise spend its full budget for nothing. - The primer shares the chat `ctx` (not a detached one) so it is canceled together with the chat. A dangling primer would re-dial the workspace conn after `runChat`'s deferred `workspaceCtx.close()` and leak that conn. - `inflight.Add(1)` ensures server shutdown still waits for any in-progress primer. - `PrepareTools` is simplified back to a single discovery call. It now only sets `workspaceMCPDiscovered = true` on success, so an empty result no longer permanently blocks discovery within the turn. The cache hit warmed by the primer makes that call cheap in the common case; the dial fallback handles the rare cache miss. ## Tests All in `coderd/x/chatd/chatd_internal_test.go`: - `TestPrimeWorkspaceMCPCache_SuccessOnFirstAttempt` — single `ListMCPTools` call returning tools populates the cache. - `TestPrimeWorkspaceMCPCache_RetriesUntilToolsAppear` — first call empty, second returns tools; primer retries past the backoff and writes the cache. Uses `quartz.Mock.Trap` on `NewTimer`. - `TestPrimeWorkspaceMCPCache_GivesUpAfterDeadline` — `ListMCPTools` always empty; primer stops at `workspaceMCPPrimeMaxWait` and refuses to cache the empty result so PrepareTools can retry on the next step. The existing integration test `TestRunChat_WorkspaceMCPDiscoveryAfterMidTurnCreateWorkspace` continues to pass and now also exercises the async-primer path end-to-end via the create_workspace tool. ``` go test ./coderd/x/chatd/... -count=1 go test ./coderd/x/chatd/ -race -count=1 make pre-commit ``` <details> <summary>Design notes</summary> - The first iteration of this PR added retry+cooldown+failure-cap logic inside `PrepareTools`. It worked for the customer's ~30s race window but did not help workspaces that take several minutes to boot, because `PrepareTools` only fires between LLM steps. Reviewer pointed out the right place to handle this is the tool itself; the current implementation does that. - Why async: a primer that ran synchronously inside the `OnChatUpdated` callback blocked the create_workspace tool from returning for up to `workspaceMCPPrimeMaxWait`, which broke `TestCreateWorkspaceTool_EndToEnd` and would hurt any template that does not expose MCP tools. Decoupling lets the tool return immediately and lets the primer warm the cache concurrently with the next LLM step. - Why share the chat `ctx` rather than `context.WithoutCancel(ctx)` (the title-generation pattern): the primer touches `workspaceCtx.getWorkspaceConn`, which `runChat`'s deferred `workspaceCtx.close()` invalidates. A detached primer outliving the chat would dial a fresh conn and leak it. - The constant naming distinguishes `workspaceMCPDiscoveryTimeout` (35s per-call dial budget, unchanged from #25169) from `workspaceMCPPrimeMaxWait` (30s total budget for the post-ready primer loop) and `workspaceMCPPrimeRetryInterval` (2s between empty-result retries). </details> Follow-up to #25169. --- _This pull request was generated by Coder Agents._	2026-05-18 07:55:56 -04:00
Kyle Carberry	9f99a7bc0b	fix(coderd): stabilize TestPatchChatMessage/ChangesModel flaky test (#25306) ... Fixes coder/internal#1535 ## Problem `TestPatchChatMessage/ChangesModel` is flaky because it races with the chat daemon's background processing. `CreateChat` sets the chat to `pending` and the daemon picks it up asynchronously. The test immediately calls `EditChatMessage` (which changes the model to an override) while the first processing round is still running. The `InsertChatMessages` SQL CTE unconditionally updates `chats.last_model_config_id` to the model of the last inserted message. When the daemon's in-flight message insertions commit after the edit transaction, they overwrite `last_model_config_id` back to the default model. Similarly, after the edit sets the chat back to `pending`, the daemon re-processes it. The test's `GetChat` call could race with this second round. ## Fix Poll for the chat to reach `waiting` (or `error`) status: 1. **Before editing**: wait for the initial processing round to complete 2. **After editing**: wait for the second processing round (triggered by the edit) to complete Then assert `last_model_config_id`, which is now stable. > Generated with [Coder Agents](https://coder.com/agents) by @kylecarbs	2026-05-15 09:33:54 -04:00
Kyle Carberry	5040ab6fca	feat: filter chats by diff URL via the q search parameter (#24970) ... Adds a `diff_url:` term to the `q` search parameter on `GET /api/experimental/chats` so callers can look up the chat associated with a particular pull request, merge request, or any other URL persisted on the chat's diff status. ``` q=diff_url:"https://github.com/coder/coder/pull/123" ``` Match is case-insensitive. When the URL lives on a delegated sub-agent's diff status, the parent chat is returned so the relationship surfaces from a single lookup. <details> <summary>Design notes</summary> - **Forge-agnostic.** Reuses the existing `chat_diff_statuses.url` column rather than introducing a `pr:` vocabulary, since the SDK already documents the URL as "may point to a pull request or a branch page depending on whether a PR has been opened." Works for GitHub PRs, GitLab MRs, branch pages, etc. - **Composes with `archived:`.** The two terms can be combined: `q=archived:true diff_url:"..."`. - **Case handling.** The parser used to lowercase the entire `q` string up front, which would mangle URL path segments. Switched to lowercasing only the field key inside `searchTerms` (already happens there) and keeping the value as the caller typed it. The SQL comparison lowercases on both sides. - **Validation.** `diff_url` must be a syntactically valid HTTP(S) URL with a non-empty host. No forge-specific validation. - **Index.** Adds `idx_chat_diff_statuses_url_lower` on `LOWER(url)` so the lookup is cheap even on large datasets. - **Sub-agent fan-in.** `EXISTS` clause matches when the URL lives on the chat itself or any chat with `root_chat_id` equal to the chat's id, so a delegated sub-agent's PR pulls in its parent. - **Deferred.** Sentinels like `pr:any` / `pr:none` and a forge-agnostic state filter (`diff_state:open|merged|closed`) were intentionally left out of this change. They couple cleanly to a second forge or a clearer product call, and shipping them now would lock in vocabulary we may want to revisit. </details> ## Tests - `coderd/searchquery`: parser tests for valid URLs, case handling (key insensitive, value preserved), composition with `archived:`, and validation errors (non-HTTP scheme, missing host, malformed URL). - `coderd/exp_chats_test.go`: end-to-end coverage hitting `ListChats`. Verifies a root chat matches its own URL, a parent chat surfaces when only a sub-agent has the URL, lookups are case-insensitive, non-matching URLs return empty, and invalid URLs return `400`. --- _This PR was authored by a Coder Agent on behalf of @kylecarbs._	2026-05-13 11:06:42 -04:00
Kyle Carberry	147f50c5e8	fix(agent/x/agentmcp): watch MCP config files for late-appearing or rewritten config (#25172) ... ## Bug `agent/x/agentmcp/Manager` resolves its config paths once at boot, calls `Reload`, and then only re-stats them lazily when a `GET /api/v0/mcp/tools` request arrives (PR #24700). If any of the manager's MCP config files (`~/.mcp.json` by default, or whatever paths `agentcontextconfig.MCPConfigFiles()` resolves from `CODER_AGENT_EXP_MCP_CONFIG_FILES`) is created, atomically rewritten, or removed _after_ that initial `Reload` and _before_ the next tools HTTP request, the manager keeps serving the stale (often empty) snapshot. `parseAndDedup` silently swallows `fs.ErrNotExist`, so a late-appearing file looks indistinguishable from "no config" until something pokes the manager again. This affects any workspace where the file lands after `MarkStartupSettled` fires, including: - a startup script that writes `~/.mcp.json` after MCP init - a user creating or editing the file mid-session - an installer, dotfiles step, or sync tool writing the file later in startup - another agent process (Claude Code, etc.) producing the file out-of-band - editor rewrites that land as `Write + Chmod + Rename` bursts ### Concrete repro (dual-agent workspace) The race is easiest to reproduce on dual-agent workspaces (inner sandbox + outer host), where the inner agent's `scriptRunner` has `script_count=0` and `mcpManager.Reload` fires at ~t+0.3 s while the host agent writes `~/.mcp.json` ~21 s later. Timeline from `workspace-otto-aa16`: - agent up `20:23:38.918` - lifecycle Ready `20:23:39.200` - MCP config file Birth `20:24:00.460` (~21 s gap) - no MCP log lines for 8 minutes - first `GET /api/v0/mcp/tools` at `20:32:11.812` logs `[warn] mcp: mcp reload canceled by caller`, takes 4946 ms; subsequent turns are cached at 2 ms. The single-agent case has the same race; it's just usually narrow enough that the next HTTP request masks it, at the cost of a multi-second stall on the first call that has to do the lazy reload itself. PR #25034's `MarkStartupSettled` does not help: "settled" fires before the file is necessarily on disk. ## Fix Add an fsnotify-backed `configWatcher` to `agent/x/agentmcp/Manager`. The watcher consumes whatever paths the manager is told to reload, which is the same `[]string` returned by `agentcontextconfig.MCPConfigFiles()`. For each path the watcher: - Watches the **parent directory** of the path, not the file itself. This handles late creation, atomic rewrite (rename + create), and deletion uniformly because inotify watches on individual non-existent files return `ENOENT` and are lost across renames. The pattern matches `agent/agentcontainers/watcher`. - Walks up to the first existing **ancestor directory** when the parent does not yet exist, and re-arms deeper on `Create` events that promote an unrealized path. - Refcounts directory watches so multiple configured paths sharing a parent dir only register one inotify watch. - Resolves symlinks **once at arming time** via `filepath.EvalSymlinks`; never chases arbitrary symlink targets on events. - Debounces multi-event editor writes through a single `quartz.AfterFunc` timer so a `Write + Chmod + Rename` burst produces one reload. - Fires a debounced callback that calls `Manager.Reload`, which routes through the existing singleflight so concurrent triggers coalesce. - Re-syncs on every `Reload` call so a future path-list change is picked up. Lifecycle: the watcher is armed lazily on the first `Reload` (no goroutine cost for unit tests that never reload). `Manager.Close` marks the manager closed and closes its `closedCh` before tearing down the watcher, so any in-flight watcher-driven reload observes the close via `waitReload` and returns `ErrManagerClosed` instead of blocking `firesWG.Wait()` on a stuck connect. The watcher then waits for its goroutine and any in-flight debounced `fire` callback before returning. `parseAndDedup` behavior is unchanged: `fs.ErrNotExist` still records an empty snapshot. With the watcher armed before that snapshot is committed, any `Create` event that races `parseAndDedup` is still delivered. This is the agent-side complement to PR #25169, which fixes chatd's mid-turn workspace MCP discovery. `MarkStartupSettled` semantics are not changed. ## Tests (`agent/x/agentmcp/configwatcher_internal_test.go`) All new tests pass with the fix and fail without it. No `time.Sleep`; synchronization uses `testutil.Eventually` for fsnotify-driven assertions and the quartz mock clock for debounce assertions. - `TestWatcher_LateFileTriggersReload` - the late-file regression: empty dir, settle startup, `Reload` sees no file, write the file later, watcher reloads, tools appear. - `TestWatcher_RewriteTriggersReload` - existing file overwritten with a new server list, watcher reloads, cache reflects new server. - `TestWatcher_RemovalTransitionsToEmpty` - delete the file, watcher reloads, manager transitions to empty cleanly. - `TestWatcher_DebouncesBurst` - quartz mock clock; three back-to-back `scheduleFire` calls produce exactly one `onChange` after `AdvanceNext`. - `TestWatcher_CloseStopsGoroutine` - construct/Reload/Close five times to surface goroutine or fd leaks under `-race`. - `TestWatcher_DualAgentHTTPNoStall` - integration: write file after `Reload`, wait for watcher reload, then `GET /tools` returns the MCP tools in less than `testutil.WaitShort` instead of the multi-second "reload canceled" stall. - `TestWatcher_LateParentDirTriggersReload` - parent dir doesn't exist at `Reload` time; create the dir then the file; watcher re-arms deeper and reloads. - `TestWatcher_SharedParentRefcount` - two configured paths share a parent dir; only one inotify watch is registered and both reload on changes. - `TestWatcher_CloseDoesNotStallOnInFlightReload` - installs a `connectStartedHook` to block a watcher-driven reload mid-`connectAll`, then asserts `Close()` returns within `WaitMedium`. Regression-verified: reverting the close-ordering causes the test to time out. ## Acceptance checklist - [x] `go test ./agent/x/agentmcp/... -race -count=1` passes (also `-count=5`). - [x] All `./agent/...` tests pass under `-race -short`. - [x] No emdash, endash, or ` -- `; `scripts/check_emdash.sh` clean. - [x] No `time.Sleep` in tests. - [x] New tests fail without the fix and pass with it (verified by temporarily disabling `m.armWatcher(paths)` and by reverting `Close()` ordering). ## Out of scope No changes to chatd's mid-turn workspace MCP discovery (PR #25169) or `MarkStartupSettled` semantics. --- <sub>This pull request was prepared by a [Coder Agents](https://coder.com/docs/admin/ai-coder) run.</sub>	2026-05-12 11:32:39 -04:00
Kyle Carberry	b0b07536fc	feat: add opt-in Coder identity headers for MCP servers (#25153)	2026-05-12 08:54:53 -04:00
Kyle Carberry	376fc80451	fix(coderd/x/chatd): discover workspace MCP tools mid-turn after create_workspace (#25169) ... ## Problem In `coderd/x/chatd/chatd.go` `runChat`, workspace MCP discovery is gated on `chat.WorkspaceID.Valid` at the start of each turn. New chats that bind their workspace mid-turn (via `create_workspace` or `start_workspace`) get an empty workspace tool list on the first step, and the model falls back to `execute` (bash) because no workspace MCP tools are advertised. **Repro:** new chat → "create a workspace and use MCP tools". No `/api/v0/mcp/tools` request hits the agent on turn 1; turn 2 in the same chat works fine. ## Fix - Add a `PrepareTools` callback to `chatloop.RunOptions`, analogous to `PrepareMessages`. It is invoked once before each LLM step with the current tool list. When it returns non-nil, the chatloop replaces `opts.Tools`, rebuilds the per-step tool definitions, and appends new tool names to `opts.ActiveTools` so newly injected tools are callable immediately. - Wire `PrepareTools` in `runChat` to trigger workspace MCP discovery the first time the chat snapshot reports a valid `WorkspaceID`. The previous top-of-turn discovery path is unchanged for chats that start with a workspace. - Extract the discovery logic into `Server.discoverWorkspaceMCPTools` so the top-of-turn and mid-turn paths share identical behavior (cache, agent resolution, `ListMCPTools` timeout, invalidation). Mid-turn discovery stays disabled in plan-mode turns and Explore subagents, matching the existing top-of-turn gate. The `workspaceMCPDiscovered` flag prevents redundant dials after the first successful discovery. ## Tests - `coderd/x/chatd/chatloop/chatloop_test.go`: two new `TestRun_PrepareTools*` cases covering injection on the next step and active-set merging when `ActiveTools` is non-empty. - `coderd/x/chatd/chatd_test.go`: `TestRunChat_WorkspaceMCPDiscoveryAfterMidTurnCreateWorkspace` drives `runChat` through a `create_workspace` tool call against a real Postgres + mocked agent conn and asserts the second streamed LLM request advertises the workspace MCP tool. Verified that the test fails (and pinpoints the missing tool) when the `PrepareTools` wiring is disabled. ## Validation ``` go test ./coderd/x/chatd/chatloop/... -count=1 go test ./coderd/x/chatd/... -count=1 make lint/emdash ``` <details> <summary>Decision log</summary> - Chose a per-step `PrepareTools` callback over mutating `opts.Tools` in place because `chatloop.Run` builds the `fantasy.Tool` definitions once at start; a hook is required to let the LLM see new tools on the next step. - Returned `[]fantasy.AgentTool` (not also active-tool-names) and let the chatloop derive name merges via `mergeNewToolNames`. This avoids leaking plan-mode gating decisions into the callback contract. - Kept the existing top-of-turn discovery path so chats that already have a workspace at turn start pay no extra latency. - Skipped reusing `ReloadMessages` (history reload) since this is purely a tool-availability concern; coupling it to a history reload would defeat the chatloop cache prefix optimizations. </details> --- _This pull request was generated by Coder Agents._	2026-05-12 00:30:56 -04:00
Kyle Carberry	5a5cd79c4c	fix: drop buffered chat parts after their durable message commits (#25164)	2026-05-12 00:30:38 -04:00
Kyle Carberry	07ff3b3f90	fix(coderd/exp_chats_test.go): stabilize TestListChats/Pagination by inserting chats directly (#25137)	2026-05-12 00:26:22 -04:00
Kyle Carberry	0ed57ee343	fix(coderd/x/chatd): checkpoint buffered message_parts to avoid stale replay (#25145)	2026-05-11 17:27:03 -04:00
Kyle Carberry	e3db203011	fix(coderd/azureidentity): set explicit roots to avoid macOS system verifier (#25136) ... Fixes [CODAGT-372](https://linear.app/codercom/issue/CODAGT-372/coderdazureidentity-testvalidateregular-fails-on-macos). Closes coder/internal#101. ## Problem `coderd/azureidentity TestValidate/regular` fails on macOS with: ``` verify signature: github.com/coder/coder/v2/coderd/azureidentity.Validate /Users/runner/work/coder/coder/coderd/azureidentity/azureidentity.go:75 - x509: “metadata.azure.com” certificate is not standards compliant ``` When `crypto/x509.VerifyOptions.Roots` is `nil`, Go's verifier on macOS/iOS falls back to the system verifier (`systemVerify` in `crypto/x509/root_darwin.go`), which delegates to Apple's `SecTrustEvaluateWithError`. Apple's framework enforces stricter standards-compliance checks than Go's pure-Go verifier and rejects some otherwise valid Azure instance-identity leaf certificates with `errSecCertificateIsNotStandardsCompliant`, surfaced as the `not standards compliant` error. The test had been skipped on darwin since #12979 (April 2024) as a workaround. ## Fix - Embed the three root CAs that Azure instance-identity certificates ultimately chain to: - DigiCert Global Root G2 - DigiCert Global Root G3 - Baltimore CyberTrust Root (kept for historical chains via `Microsoft RSA TLS CA 01/02`) - In `Validate`, populate `options.Roots` from those embedded roots when the caller does not supply its own pool. Because `Roots != nil`, Go no longer takes the `systemVerify` path on darwin and uses the pure-Go verifier on all platforms. - Remove the `runtime.GOOS == "darwin"` skip from `TestValidate`. - Add `TestEmbeddedRoots` to guard against future regressions in the embedded root list (parses each PEM, asserts self-signed, requires all three named roots). The caller's existing `Intermediates` handling is unchanged. Tests that pass their own `Roots` (e.g. `coderdtest.NewAzureInstanceIdentity`) are unaffected. ## Verification On Linux: ``` $ go test ./coderd/azureidentity/ -race -count=1 -v === RUN TestValidate === RUN TestValidate/regular === RUN TestValidate/govcloud === RUN TestValidate/rsa --- PASS: TestValidate (0.00s) --- PASS: TestValidate/regular (0.00s) --- PASS: TestValidate/rsa (0.00s) --- PASS: TestValidate/govcloud (0.00s) === RUN TestEmbeddedRoots --- PASS: TestEmbeddedRoots (0.00s) === RUN TestExpiresSoon --- SKIP: TestExpiresSoon (0.00s) PASS ok github.com/coder/coder/v2/coderd/azureidentity 1.020s ``` The `test-go-pg` job on `macos-latest` in CI is the authoritative confirmation of the fix on macOS; previously it would have failed `TestValidate/regular` had the skip been removed. <details> <summary>Why this is the correct fix</summary> From `/usr/local/go/src/crypto/x509/verify.go`: ```go // Use platform verifiers, where available, if Roots is from SystemCertPool. if runtime.GOOS == "windows" || runtime.GOOS == "darwin" || runtime.GOOS == "ios" { systemPool := systemRootsPool() if opts.Roots == nil && (systemPool == nil || systemPool.systemPool) { return c.systemVerify(&opts) } ... } ``` Setting `opts.Roots` to any non-nil, non-system pool deterministically routes verification through Go's pure-Go verifier, bypassing Apple's stricter compliance checks. The embedded roots are sufficient to validate every chain we currently care about, since every intermediate in `Certificates` ultimately issues to one of the three embedded roots. </details> > Generated by Coder Agents. Reviewed manually.	2026-05-11 13:53:33 -04:00
Kyle Carberry	aaa0dacdb3	fix: infer workspace claim time from build history for /agents delete dialog (#25057) ... Closes [CODAGT-317](https://linear.app/codercom/issue/CODAGT-317/pr-workspaces-sometimes-require-name-confirmation-to-delete). ## Problem The `/agents` archive-and-delete molly-guard (typing the workspace name) was firing for chats that had clearly created their own workspace. The heuristic in `resolveArchiveAndDeleteAction` decides whether confirmation is needed by comparing the workspace's `created_at` against the chat's `created_at`: ```ts return new Date(workspaceCreatedAt) >= new Date(chatCreatedAt); ``` That assumption breaks for **prebuilt workspaces**. `ClaimPrebuiltWorkspace` rewrites `owner_id`, `name`, `updated_at`, `last_used_at`, etc., but **never touches `created_at`**, which still reflects when the prebuild was provisioned by the reconciler, often hours before the chat exists. Result: every prebuild-claimed workspace looks pre-existing, so the molly-guard fires. Concrete example from a real chat: | Field | Value | |---|---| | `chat.created_at` | `2026-05-07T15:12:23Z` | | `workspace.created_at` (provision) | `2026-05-07T14:22:24Z` | | `latest_build.created_at` (claim) | `2026-05-07T15:19:09Z` | `14:22:24 < 15:12:23` so `isWorkspaceAutoCreated` returned false even though the chat issued the claim. ## Fix (frontend-only) Derive the moment a workspace was acquired from existing build history rather than relying on `workspace.created_at`: - Build #1 initiator = prebuilds system user → workspace was a prebuild → use `build_2.created_at` (the claim build) as the acquisition time. - Build #1 initiator = real user → workspace was created from scratch → use `workspace.created_at` (unchanged behavior). - Unclaimed prebuild or no build history → return `null` (force confirmation; safe degradation for a destructive flow). The resolver fetches the build list via the existing `getWorkspaceBuilds` endpoint when the dialog might fire. No new column, no migration, no schema change. Works retroactively for all existing claimed prebuilds; no backfill needed. The prebuilds system user UUID is exposed via `codersdk.PrebuildsSystemUserID` and typegen'd to `typesGenerated.ts`. `coderd/database.PrebuildsSystemUserID` parses that constant via `uuid.MustParse` so the two cannot drift; if the codersdk literal ever changes, package init fails fast. ## History The first draft of this PR added a `workspaces.claimed_at` column populated by `ClaimPrebuiltWorkspace`. After review feedback from @johnstcn pointing out that the same fact is already implicit in build history, I pivoted to the frontend-only approach. Subsequent review notes consolidated the prebuilds system user UUID into a single typegen'd constant. ## Why not the other open PRs - **#25055** (`chatKey` cache fallback) only fixes a different cache-miss path; it explicitly notes it does not address `created_at < chat.created_at`. - **#25053** (`chats.workspace_auto_created` boolean) puts the truth on the wrong side of the schema: "this workspace was claimed at time T" is a property of the workspace, not the chat. The MCP plumbing it adds is also unnecessary now that the same answer is available from build history. ## Test plan - `pnpm vitest run --project=unit src/pages/AgentsPage/utils/agentWorkspaceUtils.test.ts` — 40/40 pass; new cases cover prebuild claim before/after chat, unclaimed prebuild, missing-build-history fallback, and the fetch-skip when the chat is not in cache. - `pnpm lint:types`, `pnpm check`, `make pre-commit`. <details> <summary>Disclosure</summary> Opened on behalf of @kylecarbs by [Coder Agents](https://coder.com/coder-agents). </details>	2026-05-10 11:04:55 -04:00
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._	2026-05-04 17:37:57 -04:00
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.	2026-05-04 13:35:35 -04:00
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>	2026-05-05 02:22:26 +10:00
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>	2026-05-03 11:31:48 -04:00
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._	2026-04-28 12:50:51 -04:00
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.	2026-04-27 15:59:26 -04:00
Kyle Carberry	069223ae26	fix: recover web push subscriptions after PWA reinstall (#24720)	2026-04-26 14:49:10 -07:00
Kyle Carberry	8bc91d982f	fix(site): truncate long workspace name in chat input toolbar (#24412)	2026-04-16 01:04:19 -04:00
Kyle Carberry	9c74c8c674	fix: move OnChatUpdated call after agent is ready in create/start workspace (#24410)	2026-04-15 19:18:54 -04:00
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	2026-04-15 16:41:15 -04:00
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.	2026-04-13 13:00:39 -04:00
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**	2026-04-13 10:20:57 -04:00
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)	2026-04-10 14:25:12 -04:00
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>	2026-04-09 16:33:00 +02:00
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>	2026-04-08 12:42:03 -04:00
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	2026-04-08 11:54:44 -04:00
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	2026-04-08 09:47:44 -04:00
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	2026-04-07 13:41:33 -04:00
Kyle Carberry	cffc68df58	feat(site): render read_skill body as markdown (#24069)	2026-04-07 11:50:21 -04:00
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	2026-04-07 16:11:49 +01:00
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>	2026-04-07 10:25:46 -04:00
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	2026-04-07 02:08:49 +00:00
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	2026-04-07 02:04:48 +00:00
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	2026-04-06 16:20:14 -04:00
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)	2026-04-06 17:19:05 +00:00
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	2026-04-06 12:10:57 -04:00
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	2026-04-06 10:49:42 -04:00
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	2026-04-05 20:44:59 +00:00
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)	2026-04-05 14:10:26 -04:00
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	2026-04-05 13:27:21 -04:00
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)	2026-04-05 12:04:13 +00:00
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	2026-04-05 11:42:07 +00:00
Kyle Carberry	e18094825a	fix: retain message_part buffer for cross-replica relay (#24031)	2026-04-04 17:24:41 -04:00
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.	2026-04-04 12:45:46 -04:00
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>	2026-04-01 13:03:59 -04:00
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.	2026-04-01 16:33:51 +00:00
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>	2026-04-01 12:28:47 -04:00
Kyle Carberry	8c8b307b97	fix: persist session cookie to disk to prevent PWA logout (#23746)	2026-04-01 09:54:59 -04:00
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>	2026-04-01 13:04:59 +00:00
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>	2026-04-01 12:57:18 +00:00
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>	2026-04-01 12:08:13 +00:00
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>	2026-04-01 07:45:38 -04:00
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)	2026-03-31 13:40:26 -04:00
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>	2026-03-31 14:43:32 +00:00
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).	2026-03-30 18:23:47 +00:00
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>	2026-03-30 14:11:30 -04:00
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"`.	2026-03-30 12:46:51 -04:00
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>	2026-03-29 21:48:17 -04:00
Kyle Carberry	be99b3cb74	fix: prioritize context cancellation in WebSocket sendEvent (#23756) ... ## Problem Commit 386b449 (PR #23745) changed the `OneWayWebSocketEventSender` event channel from unbuffered to buffered(64) to reduce chat streaming latency. This introduced a nondeterministic race in `sendEvent`: ```go sendEvent := func(event codersdk.ServerSentEvent) error { select { case eventC <- event: // buffered channel — almost always ready case <-ctx.Done(): // also ready after cancellation } return nil } ``` After context cancellation, Go's `select` randomly picks between two ready cases, so `send()` sometimes returns `nil` instead of `ctx.Err()`. With the old unbuffered channel the send case was rarely ready (no reader), masking the bug. ## Fix Add a priority `select` that checks `ctx.Done()` before attempting the channel send: ```go select { case <-ctx.Done(): return ctx.Err() default: } select { case eventC <- event: case <-ctx.Done(): return ctx.Err() } ``` This is the standard Go pattern for prioritizing one channel over another. When the context is already cancelled, the first select returns immediately. The second select still handles the case where cancellation happens concurrently with the send. ## Verification - Ran the flaky test 20× in a loop (`-count=20`): all passed - Ran the full `TestOneWayWebSocketEventSender` suite 5× (`-count=5`): all passed - Ran the complete `coderd/httpapi` test package: all passed Fixes coder/internal#1429	2026-03-29 20:11:30 -04:00
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>	2026-03-28 15:26:42 -04:00
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	2026-03-28 12:26:23 -04:00
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>	2026-03-28 11:16:11 -04:00
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).	2026-03-27 15:22:13 -04:00
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.	2026-03-27 13:36:49 -04:00
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.	2026-03-27 12:15:04 -04:00
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>	2026-03-27 14:28:27 +00:00
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>	2026-03-27 14:23:25 +00:00
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>	2026-03-27 10:06:32 -04:00
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.	2026-03-26 19:57:02 +00:00