coder

mirror of https://github.com/coder/coder.git synced 2026-06-02 20:48:20 +00:00

Author	SHA1	Message	Date
Dean Sheather	e57525002c	chore: remove agents experiment flag and mark feature as beta (#24432 ) Remove the `ExperimentAgents` feature flag so the Agents feature is always available without requiring `--experiments=agents`. The feature is now in beta. Existing deployments that still pass `--experiments=agents` will get a harmless "ignoring unknown experiment" warning on startup. ### Changes Backend: - Remove `RequireExperimentWithDevBypass` middleware from chat and MCP server routes - Always include `AgentsAccessRole` in assignable site roles (later refactored to org-scoped on main; rebase keeps that) - Always set `AgentsTabVisible = true`, then drop the entire dead `AgentsTabVisible` metadata pipeline (Go htmlState field, populateHTMLState goroutine, HTML meta tag, useEmbeddedMetadata registration, mock); no production consumer reads it. `AgentsNavItem` already gates on `permissions.createChat`. - Make `blob:` CSP `img-src` addition unconditional - Remove `ExperimentAgents` constant, `DisplayName` case, and `ExperimentsKnown` entry CLI: - Graduate the agents TUI from `coder exp agents` to `coder agents` (moved from `AGPLExperimental()` to `CoreSubcommands()`) - Drop the `agent` alias so it does not collide with the hidden workspace-agent command - Rename implementation files `cli/exp_agents_.go` -> `cli/agents_.go` and internal identifiers (`expChatsTUIModel` -> `chatsTUIModel`, `newExpChatsTUIModel` -> `newChatsTUIModel`, `setupExpAgentsBackend` -> `setupAgentsBackend`, `startExpAgentsSession` -> `startAgentsSession`, `expAgentsPtr` -> `agentsPtr`, `expAgentsSession` -> `agentsSession`, `TestExpAgents` -> `TestAgents`). `expClient` (the `codersdk.ExperimentalClient` local) is kept; `coderd/exp_chats.go` and other still-experimental `cli/exp_.go` commands are intentionally untouched. Frontend:* - Remove experiment check from `AgentsNavItem` - render when `canCreateChat` is true - Remove `agentsEnabled` experiment check from `WorkspacesPage`, then gate `chatsByWorkspace` on `permissions.createChat` so users without chat access don't trigger the per-page DB query (Copilot review feedback) - Add `FeatureStageBadge` (beta) next to the Coder logo in the Agents sidebar (desktop + mobile) Docs: - Remove experiment flag setup instructions from `early-access.md` and `getting-started.md` (and rename `early-access.md`'s "Enable Coder Agents" heading to "Set up Coder Agents", since there is no enablement step left) - Update `chats-api.md` and `getting-started.md`'s Chats API note to say "beta" instead of "experimental" - `docs/manifest.json`: drop "experimental" from the Chats API sidebar description - `make gen` regenerated `docs/reference/cli/agents.md` and the CLI index - `scripts/check_emdash.sh`: exclude `cli/testdata/.golden` and `enterprise/cli/testdata/.golden` from the new repo-wide emdash lint, since serpent emits emdash borders in every generated `--help` golden file Tests: - Remove `ExperimentAgents` setup from all test files (14 occurrences across 7 files) - Update stale "with the agents experiment" comments in `coderd/x/chatd/integration_test.go` and `coderd/mcp_test.go` <img width="1185" height="900" alt="image" src="https://github.com/user-attachments/assets/b420bc8f-41d6-42c6-abd8-ad572533d651" /> > 🤖 Generated by Coder Agents	2026-05-01 01:49:00 +10:00
Danny Kopping	b975262a97	fix(site): remove Request Logs from admin menu, redirect /aibridge to sessions (#24840 ) Disclaimer: implemented by a Coder Agent using Claude Opus 4.6 Remove the "AI Bridge Logs" menu item from the Admin settings dropdown, keeping only the "AI Bridge Sessions" link. The `/aibridge` index now redirects to `/aibridge/sessions` instead of `/aibridge/request-logs`. The request-logs route and view remain accessible via direct URL (`/aibridge/request-logs`) for users who still need it. Fixes https://linear.app/codercom/issue/AIGOV-205 <details><summary>Changes</summary> - `DeploymentDropdown.tsx`: Removed "AI Bridge Logs" menu item; the `canViewAIBridge` block now only renders the "AI Bridge Sessions" link - `router.tsx`: Changed `/aibridge` index redirect from `request-logs` to `/aibridge/sessions` - All request-logs page files, route, and components are preserved for direct URL access </details>	2026-04-30 10:04:20 -05:00
Thomas Kosiewski	0b9b94ae4e	feat(site/src): add advisor admin settings UI (#24624 ) ## Summary Add the admin-only Advisor settings form on the Agents Behavior page, wired to the PR 3 HTTP API via TanStack Query. Admins can enable/disable the advisor, tune per-run caps and max output tokens, pick a reasoning effort, and optionally select a dedicated advisor model. ## Motivation PR 3 introduced the configuration storage and API but no surface to change it. This PR puts the knobs in the existing admin Behavior UI so operators can turn the feature on/off and tune it without touching the database. ## Changes - `site/src/pages/AgentsPage/components/AdvisorSettings.tsx`: the admin-only settings panel. - TanStack Query hooks (`chatAdvisorConfig`, `updateChatAdvisorConfig`) with optimistic cache invalidation on save. - Inputs for `Enabled`, `MaxUsesPerRun`, `MaxOutputTokens`, `ReasoningEffort`, `ModelConfigID`. - Model-override dropdown renders "Use chat model" when unset. - `ZERO_UUID` / `isUnsetModelConfigId` helpers normalize the model-override payload in both directions: the backend serializes the unset sentinel as `"00000000-0000-0000-0000-000000000000"`, and the frontend must both recognize that sentinel on read and never send `""` on write (which would trigger a backend `invalid UUID length: 0`). - Behavior page wiring so the panel appears in the existing settings flow alongside the rest of the agent behavior toggles. ## Stack context This is PR 6 of 6 in the advisor feature stack. It is stacked directly on PR 3 (the config API) and is a sibling of PRs 4/5, so it can be reviewed independently of the chat-rendering work. ## Scope / non-goals - Does not change advisor runtime behavior; that is PR 4's responsibility. - No new audit entries for this settings change; it rides on the same storage mechanism as the existing site-configs pattern. ## Validation - `pnpm --filter site lint:types` - Manual dogfood against `./scripts/develop.sh`: toggle Enabled on/off, save, confirm `GET /api/experimental/chats/config/advisor` reflects changes, confirm non-admin users cannot reach the form. --- <details> <summary>📋 Implementation Plan (shared across the advisor stack)</summary> # Plan: Add a Mux-style advisor tool to coder agents/chatd ## Outcome Add a first-class `advisor` tool to agent chats in `coderd/x/chatd` that feels native to Coder: - it is a built-in server-side tool, not an MCP/dynamic-tool workaround; - it performs a nested tool-less model call for strategic advice; - it is exposed only when eligible, and the prompt mentions it only when it is actually available; - it is treated as a planning-only tool so it does not run alongside action tools in the same batch; - it tracks usage/cost separately enough for operators to reason about it; - it has a minimally polished UI in the Agents page; - and it ships with explicit dogfooding evidence, including screenshots and repro videos. ## Design decisions to lock before coding 1. Primary architecture: native built-in tool in `chattool/`, backed by a small `chatadvisor` package. 2. Nested model execution: reuse chatd's existing model/provider stack for a one-step, tool-less advisor call rather than inventing a new provider pathway. 3. Execution policy: treat `advisor` as an exclusive/planning-only tool; mixed batches must return structured policy errors and force the model to retry cleanly. 4. Availability: initial rollout is for root agent chats only; disable for child/sub-agent chats until recursion/cost policy is proven. 5. Prompt sync: use one eligibility boolean to drive both tool registration and advisor guidance injection. 6. Persistence/cost split: MVP should keep advisor usage visible in result metadata and server metrics; only add DB schema if product/billing explicitly needs queryable advisor-specific cost. 7. UI scope: generic tool rendering is an acceptable temporary milestone during backend bring-up, but the release candidate should include a dedicated lightweight advisor renderer. ## Delivery model The work should be executed as coordinated workstreams with one integration owner and parallel contributors for low-conflict areas. The integration owner should own `coderd/x/chatd/chatd.go` because prompt assembly, tool registration, and model resolution all converge there. ## Detailed workstreams ### Repo evidence used for this plan <details> <summary>Mux reference and current chatd seams</summary> Mux reference implementation - `src/node/services/tools/advisor.ts` — native advisor tool implementation. - `src/common/constants/advisor.ts` — advisor prompt/constants and truncation policy. - `src/common/utils/tools/tools.ts` — conditional tool registration. - `src/node/services/streamContextBuilder.ts` — injects advisor guidance only when the tool is available. Current chatd seams - `coderd/x/chatd/chatd.go` - `processChat()` — tool assembly, prompt assembly, and chatloop invocation. - `resolveChatModel()` — current model/provider/key resolution seam. - `type Config struct` — server-level chatd configuration surface. - `coderd/x/chatd/chatloop/chatloop.go` - `Run()` — main streaming/model loop. - `executeTools()` — built-in tool execution/batching seam. - `coderd/x/chatd/chattool/` — built-in tool implementations. - `site/src/pages/AgentsPage/components/ChatElements/tools/Tool.tsx` — tool renderer dispatch. - `site/src/pages/AgentsPage/components/ChatConversation/messageParsing.ts` and `ConversationTimeline.tsx` — tool/result merge and rendering flow. </details> ### Workstream map and ownership \| Workstream \| Primary owner \| Main files \| Can run in parallel? \| Done when \| \|---\|---\|---\|---\|---\| \| 0. Integration + gating \| Integration lead \| `coderd/x/chatd/chatd.go` \| No; central merge lane \| Tool registration, prompt sync, and model selection are wired together \| \| 1. Advisor runtime + tool \| Backend agent \| new `coderd/x/chatd/chatadvisor/`, new `coderd/x/chatd/chattool/advisor.go` \| Yes \| Tool can perform a tool-less advisor call in memory and return structured results \| \| 2. Planning-only execution policy \| Chatloop agent \| `coderd/x/chatd/chatloop/chatloop.go`, related tests \| Yes \| Mixed `advisor` + action-tool batches are rejected cleanly and deterministically \| \| 3. Metrics/usage/config \| Backend/telemetry agent \| `chatd.go`, `chatloop/metrics.go`, optional config plumbing \| Partially; coordinate with integration lead \| Advisor usage is separately visible in metadata/metrics and limits are enforced \| \| 4. Frontend rendering \| Frontend agent \| `site/.../tools/Tool.tsx`, new `AdvisorTool.tsx`, stories \| Yes after result schema stabilizes \| Advisor renders as a readable card and story tests pass \| \| 5. Dogfood + QA evidence \| QA agent \| dev server, Storybook, dogfood output \| After backend + UI are usable \| Repro videos, screenshots, and a concise QA report exist \| ### Parallelization rules - Do not split `coderd/x/chatd/chatd.go` across multiple execution agents without an integration lead. That file owns prompt building, tool registration, model resolution, and cost persistence. - Workstreams 1 and 2 can be developed in parallel and then stacked onto the integration branch. - Workstream 4 should begin once the backend result schema is agreed on, even if the backend is still behind a feature flag. - Any agent that needs to re-check Mux behavior should clone `coder/mux` into a temporary directory (for example, `$(mktemp -d)/mux`) and inspect it read-only; do not vendor or copy code from Mux directly. ## Phase 0 — Preflight and guardrails ### Goals - Align the team on the smallest shippable architecture. - Prevent scope creep into MCP/dynamic-tool/sub-agent variants. - Decide upfront what is MVP vs. follow-up. ### Tasks 1. Confirm the MVP boundary. - Ship a built-in advisor tool first. - Do not make MCP, dynamic tools, or sub-agents the primary implementation. - Do not add transient streaming phases in the first backend PR unless they fall out almost for free. 2. Confirm local workflow hygiene before coding. - Ensure the repo is using the project git hooks from `scripts/githooks`. - Do not bypass hooks with `--no-verify`. - Use `./scripts/develop.sh` for the full dev server rather than manual build/run commands. 3. Lock the model-selection policy. - Recommended MVP: advisor uses the same resolved provider/model/cost config as the current chat, with advisor-specific max-output and usage caps. - Follow-up only if required: add a separate `AdvisorModelConfigID`-style override that resolves through the existing `configCache`/model-config path. Do not invent a new free-form `provider:model` parser if chatd already stores provider/model separately. 4. Lock the persistence policy. - Recommended MVP: no DB migration. Persist advisor-visible metadata in the tool result and record separate metrics in memory/Prometheus. - Only if product/billing explicitly asks for queryable advisor cost: add a later DB migration or usage table, following the normal `queries/.sql` + `make gen` workflow. 5. Create an execution ADR note in the work item or tracking doc.* - Capture: built-in tool, tool-less nested call, root-chat-only rollout, exclusive execution policy, MVP no-DB-migration default. ### Quality gate - Everyone on the team can state the same answers to these questions: - Is advisor a built-in tool? Yes. - Can advisor run with action tools in the same batch? No. - Does advisor get tools of its own? No. - Is a DB migration required for MVP? No, unless billing insists. ## Phase 1 — Build the advisor runtime and tool wrapper ### Goals Create the core advisor implementation in a way that is easy to test and keeps `chattool/` thin. ### Files to add - `coderd/x/chatd/chatadvisor/types.go` - `coderd/x/chatd/chatadvisor/guidance.go` - `coderd/x/chatd/chatadvisor/handoff.go` - `coderd/x/chatd/chatadvisor/runtime.go` - `coderd/x/chatd/chatadvisor/runner.go` - `coderd/x/chatd/chattool/advisor.go` ### Responsibilities by file 1. `types.go` - Define the input/result schema used by the tool and UI. - Keep the result shape close to Mux so the UI and model both have predictable cases. - Recommended result variants: - `advice` - `limit_reached` - `error` Recommended shape: ```go type AdvisorArgs struct { Question string `json:"question"` } type AdvisorResult struct { Type string `json:"type"` Advice string `json:"advice,omitempty"` Error string `json:"error,omitempty"` AdvisorModel string `json:"advisor_model,omitempty"` RemainingUses int `json:"remaining_uses,omitempty"` Usage AdvisorUsageResult `json:"usage,omitempty"` } ``` 2. `guidance.go`* - Hold two strings: - the nested advisor system prompt; - the parent-agent guidance block to inject into the outer system prompt. - The nested advisor prompt must say, in plain language: - you are advising the parent agent; - you do not address the end user directly; - you do not claim actions happened; - you return concise strategic guidance and tradeoffs. 3. `runtime.go` - Define the per-run runtime state. - Recommended fields: - resolved model + model config; - provider keys/options reused from the outer chat; - `MaxUsesPerRun`; - `MaxOutputTokens`; - atomic/current call counter; - callback(s) to obtain the current prompt snapshot and current-step snapshot; - optional metrics/usage hook. - Add fail-fast validation for impossible config: nil model, non-positive limits, empty prompt builders, etc. 4. `handoff.go` - Build the advisor handoff message from: - the explicit question; - the exact prompt/messages the parent model just used; - the current step's text/reasoning snapshot, if available; - the most recent relevant tool outputs, if they are already in the prompt snapshot. - Important: use the already-prepared outer prompt tail, not a fresh DB reload. That keeps the advisor aligned with compaction and the exact context the outer model saw. - Apply hard truncation budgets with recent-context bias. 5. `runner.go` - Execute the nested advisor call. - Recommended implementation: call `chatloop.Run()` in an in-memory, one-step mode: - `Tools: nil` - `ProviderTools: nil` - `MaxSteps: 1` - `PersistStep`: capture the assistant output in memory instead of writing DB rows - Reuse the existing provider/model/cost path instead of building a second provider runner. - Assert that no tool definitions are passed to the nested call. 6. `chattool/advisor.go` - Keep this file thin and consistent with other built-ins. - Responsibilities: - decode `AdvisorArgs`; - validate `Question` is non-empty and bounded; - call the `chatadvisor` runner; - return a structured tool response. ### Defensive programming requirements - Assert `Question` is non-empty after trimming. - Assert runtime limits are positive. - Assert the nested advisor call runs with zero tools/provider tools. - Assert `AdvisorResult.Type` is one of the known variants before returning. - Assert remaining uses never goes negative. ### Acceptance criteria - A unit test can call the advisor tool with a fake model and receive a stable `advice` result. - The nested advisor call is impossible to run with tools accidentally attached. - The core logic lives in `chatadvisor/`, not embedded inside `chatd.go`. ## Phase 2 — Wire advisor into chatd and keep prompt/tool availability in sync ### Goals Register the tool in the right place, expose it only when eligible, and inject system guidance only when the tool is present. ### Files to modify - `coderd/x/chatd/chatd.go` - optionally a small helper file if `chatd.go` becomes too crowded ### Tasks 1. Compute one eligibility boolean in `processChat()`. Recommended inputs: - server-level advisor enabled flag; - root chat only (`chat.ParentChatID == uuid.Nil` or equivalent existing root/child check); - a usable resolved model/provider exists; - optional experiment/workspace/org gate if product wants staged rollout. 2. Create the runtime once per outer chat run. - Use the model/config/keys resolved by `resolveChatModel()`. - Reuse provider options from the current chat's `ChatModelCallConfig`. - Set `MaxUsesPerRun` and `MaxOutputTokens` from advisor config defaults. 3. Register the tool in the built-in tool block. - Insert after the skill tools and before MCP tools in `processChat()`. - Record `builtinToolNames["advisor"] = true` so metrics stay bounded. 4. Inject advisor guidance into the outer system prompt using the same boolean. - Use `chatprompt.InsertSystem()` in the same prompt assembly path that already injects user/system instructions. - Place the block near the existing instruction insertion, before plan-path/skill context blocks. - Wrap the guidance in an explicit tag like `<advisor-guidance>` so it is easy to spot in tests and future refactors. 5. Keep advisor out of child chats for the first release. - That avoids recursion/cost blowups with `spawn_agent` / `wait_agent` flows. - Document this explicitly in the rollout notes and tests. ### Acceptance criteria - If advisor is disabled, neither the tool nor the prompt guidance appears. - If advisor is enabled, both the tool and the prompt guidance appear. - Root chats can use advisor; child chats cannot. - Built-in tool names include `advisor` so metrics do not collapse it into the generic `mcp` label. ## Phase 3 — Enforce planning-only execution policy in `chatloop` ### Goals Prevent the model from calling `advisor` and action tools in the same execution batch. ### Files to modify - `coderd/x/chatd/chatloop/chatloop.go` - related chatloop tests ### Recommended implementation Keep the MVP small; do not build a general policy engine yet. 1. Add a minimal field to `chatloop.RunOptions`, for example: ```go ExclusiveToolName string ``` 2. In `Run()` / `executeTools()`, detect the case where the exclusive tool appears in the same local-tool batch as any other locally executed tool. 3. When that happens, synthesize structured tool-result errors for the affected calls instead of executing anything in the batch. - `advisor` should receive a clear error like: _advisor must be called by itself before action tools_. - The sibling action tools should receive a paired policy error like: _this tool was skipped because advisor must run alone_. 4. Let the outer model see those tool errors and retry cleanly. - This is simpler and safer than partial execution or hidden deferral. - It preserves deterministic transcript history for debugging. 5. Pass the just-finished step snapshot into the tool execution context. - The advisor runtime should be able to see the current step's text/reasoning content, because that is often the best hint about what the outer model is trying to decide. ### Why this is the right fit - It matches the intended semantics: advisor is consulted before* taking action. - It avoids subtle race conditions caused by concurrent built-in tool execution. - It keeps the behavior easy to test with fake models. ### Acceptance criteria - A model-emitted batch containing only `advisor` succeeds. - A model-emitted batch containing `advisor` plus any other locally executed tool returns deterministic policy errors and executes nothing. - Non-advisor tool execution stays unchanged for normal chats. ## Phase 4 — Usage limits, metrics, and configuration ### Goals Make advisor safe to operate without over-designing billing/storage in the first release. ### Files to modify - `coderd/x/chatd/chatd.go` - `coderd/x/chatd/chatloop/metrics.go` as needed - `coderd/x/chatd/chatd.go` `Config` struct and constructor path - optional follow-up config/db files only if a separate advisor model or persistent billing is required ### Tasks 1. Add explicit server config knobs for MVP. Recommended fields on `chatd.Config` or a nested advisor config struct: - `AdvisorEnabled bool` - `AdvisorMaxUsesPerRun int` - `AdvisorMaxOutputTokens int64` 2. Track usage per outer run. - Reset the counter for each `processChat()` invocation. - Return `remaining_uses` in the tool result. - Return `limit_reached` when the cap is exhausted. 3. Expose advisor usage metadata in the tool result. - Include model name and token/cost summary if available. - Use the same `callConfig.Cost` calculation path as the outer chat for MVP if advisor reuses the same model. 4. Record server-side metrics. - Count advisor invocations, failures, and latency. - Ensure they show up under the built-in tool label `advisor`. 5. Optional decision gate: separate advisor model. - If product insists on a stronger/different advisor model, add a follow-up config hook that resolves another existing chat model config through the same `configCache` path. - Keep that out of the first landing PR unless it is required for acceptance. 6. Optional decision gate: queryable advisor cost. - If this becomes required, spin a follow-up DB task: - update `coderd/database/queries/.sql`; - add migration files; - run `make gen`; - update audit mappings if a new auditable type/field is introduced. ### Acceptance criteria - Advisor calls are capped per outer run. - Limit exhaustion is user-visible in the tool result. - Metrics distinguish advisor calls from other built-in tools. - MVP does not require a schema migration unless explicitly approved. ## Phase 5 — Frontend rendering and Storybook coverage ### Goals Make advisor feel intentional in the Agents UI without blocking the backend on fancy streaming UI. ### Files to modify - `site/src/pages/AgentsPage/components/ChatElements/tools/Tool.tsx` - new `site/src/pages/AgentsPage/components/ChatElements/tools/AdvisorTool.tsx` - Storybook story file(s) in the same tools directory ### Delivery strategy 1. Intermediate milestone during backend bring-up:* rely on the existing generic tool renderer if needed. - This is acceptable only as a short-lived integration checkpoint. 2. Release milestone: add a dedicated lightweight `AdvisorTool` renderer. - Reuse existing primitives: - `ToolCollapsible` - `ToolIcon` - `Response` for markdown/prose rendering - `ScrollArea` if the advice can be long - Keep styling light and consistent with the Agents page. - Do not add unnecessary React memoization in `site/src/pages/AgentsPage/`; that area is already React-Compiler aware. 3. Render the structured result states cleanly. - `advice` — readable prose/markdown with optional metadata footer. - `limit_reached` — warning-style message. - `error` — error state with visible fallback text. - `running` — existing tool loading state/spinner is enough for MVP. 4. Add Storybook coverage instead of ad-hoc component tests. Recommended stories: - successful advice; - running/loading; - limit reached; - error. 5. Keep the UI contract narrow. - Prefer one text field like `advice` plus small metadata rather than a deeply nested schema. - That keeps the UI resilient to prompt iteration. ### Acceptance criteria - The advisor tool card renders readable content rather than raw quoted JSON in the final release branch. - Running, limit, and error states are visibly distinct. - Storybook stories and play assertions cover the new states. - Existing tool rendering flows remain unchanged. ## Phase 6 — Automated tests and validation gates ### Backend tests to add 1. Advisor runtime/tool tests - question validation; - tool-less nested execution assertion; - success result shaping; - limit-reached result shaping; - error result shaping. 2. Prompt/gating tests in chatd - advisor disabled ⇒ no tool, no guidance; - advisor enabled/root chat ⇒ tool + guidance; - child chat ⇒ advisor absent. 3. Chatloop policy tests - advisor alone runs; - advisor + action tool mixed batch returns deterministic policy errors; - non-advisor tools still execute normally. 4. Usage/metrics tests - per-run cap resets correctly; - builtin tool labeling includes `advisor`; - returned metadata includes model/usage summary when available. ### Frontend tests to add - Storybook `play()` assertions for the advisor renderer states. - Verify expand/collapse behavior and visible fallback text. - Verify the message timeline still renders adjacent tools correctly. ### Recommended command sequence Run these as the implementation matures, not only at the end: 1. Backend-focused gate after phases 1–4: - `make test RUN=TestAdvisor` - `make test RUN=TestChatloopAdvisor` - `make lint` 2. Frontend-focused gate after phase 5: - `pnpm test:storybook src/pages/AgentsPage/components/ChatElements/tools/AdvisorTool.stories.tsx` - `pnpm lint` - `pnpm format` 3. Final repo gate before handoff: - `make pre-commit` - run any additional targeted `make test RUN=...` selections covering touched chatd paths > Use the exact new test names the implementing agents create; the names above are recommended anchors, not existing tests. ## Dogfooding plan ### Principle Dogfood the change as a real agent feature, not just a unit-tested backend. Per the dogfood and `agent-browser` skills, the reviewer should get watchable repro videos plus screenshots that make the behavior obvious without reading logs. ### Required setup 1. Start the full dev environment with: - `./scripts/develop.sh` 2. If the frontend renderer changes, also start Storybook from `site/` with: - `pnpm storybook --no-open` 3. Use `agent-browser` directly — never `npx agent-browser`. 4. Use named browser sessions and an output folder such as: - `./dogfood-output/advisor/` - with subfolders `screenshots/` and `videos/` ### Evidence protocol For every interactive scenario below: 1. Start video recording before the action. 2. Capture step-by-step screenshots at human pace. 3. Capture one annotated screenshot of the final state. 4. Stop the recording. 5. Note the exact pass/fail observation in the QA report. For static UI states (for example Storybook error/limit cards), an annotated screenshot is sufficient; video is optional but still encouraged by this project’s review preference. ### Dogfood scenarios #### Scenario A — Happy path in the real Agents UI Goal: prove that a root agent chat can invoke advisor and produce a readable recommendation before taking further action. Steps: 1. Open the Agents page with an advisor-enabled root chat. 2. Start a repro video. 3. Send a prompt that should reasonably trigger strategic planning, such as an architecture or multi-tradeoff question. 4. Capture screenshots of: - the prompt before send; - the running advisor state; - the completed advisor card and the assistant’s follow-up response. 5. Stop recording. Pass criteria: - advisor appears in the timeline; - the rendered result is readable; - the assistant can continue after consuming the advisor output. #### Scenario B — Advisor unavailable path Goal: prove the feature is truly gated. Suggested variants (at least one is required, both are better): - feature flag/config off; - child/sub-agent chat. Evidence: - annotated screenshot of the chat/tool state showing advisor is absent; - short video if toggling the gate live is part of the repro. Pass criteria: - no advisor tool is available; - no advisor-specific prompt behavior leaks through. #### Scenario C — UI states in Storybook Goal: prove the renderer handles non-happy states cleanly. Required story states: - success/advice; - running; - limit reached; - error. Evidence: - one screenshot per state; - at least one short video showing collapse/expand behavior. Pass criteria: - success renders readable advice; - limit/error have visible fallback text; - the component behaves like the other tool cards. #### Scenario D — Regression sweep of nearby tools Goal: ensure advisor does not break the surrounding chat timeline. Check at minimum: - another existing built-in tool still renders correctly near advisor; - sub-agent/tool cards still expand/collapse normally; - no obvious console errors appear in the Agents page during the advisor flow. Evidence: - screenshots of adjacent tool cards; - console/error capture if anything suspicious appears. ### `agent-browser` usage notes for the QA agent - Prefer `agent-browser batch` for 2+ sequential commands when no intermediate parsing is needed. - Use `snapshot -i` to discover interactive refs. - Re-snapshot after navigation or major DOM changes. - Avoid `wait --load networkidle` unless the page is known to go idle; prefer explicit element/text waits or short fixed waits. - Record videos at human pace and include pauses that a reviewer can follow. ## Rollout plan ### Initial rollout - Gate behind a server-side advisor-enabled flag. - Enable only for selected internal/root agent chats first. - Watch metrics for: - invocation count; - failure rate; - latency; - obvious retry loops. ### Expansion conditions Expand beyond the initial rollout only after the following are true: - mixed-batch policy behavior is stable; - cost impact is understood; - frontend UX is readable in production-like dogfood; - no recursion surprises have appeared with sub-agent flows. ### Explicit non-goals for the first release - advisor inside child/sub-agent chats; - provider-agnostic streaming phase UI; - MCP-based external advisor implementation; - mandatory DB-backed advisor cost reporting. ## Final acceptance checklist - [ ] `advisor` is a built-in chatd tool, not an MCP/dynamic-tool substitute. - [ ] The nested advisor call is tool-less and bounded to one in-memory step. - [ ] One eligibility boolean controls both tool registration and prompt guidance injection. - [ ] Root chats can use advisor; child chats cannot in the initial rollout. - [ ] Mixed advisor/action batches produce deterministic policy errors instead of partial execution. - [ ] Per-run usage caps and limit-reached behavior work. - [ ] Advisor usage is visible in metadata/metrics without forcing a DB migration for MVP. - [ ] The Agents UI has a readable advisor card and Storybook coverage. - [ ] Dogfooding produced screenshots and repro videos for the required scenarios. - [ ] Validation commands (`make lint`, targeted `make test`, Storybook tests, `make pre-commit`) passed before handoff. ## Suggested PR split 1. PR 1 — Backend foundation - `chatadvisor/` package - `chattool/advisor.go` - `chatloop` exclusive policy - chatd gating/prompt sync - backend tests 2. PR 2 — Frontend + QA - advisor renderer - stories/play assertions - dogfood artifacts and QA notes 3. PR 3 — Optional follow-ups only if demanded by stakeholders - separate advisor model override - persistent advisor billing/queryability - transient phase-stream UX </details> --- _Generated with [`mux`](https://github.com/coder/mux) • Model: `anthropic:claude-opus-4-7` • Thinking: `max`_	2026-04-30 15:41:09 +02:00
Thomas Kosiewski	07e86ae565	feat(site/src/pages/AgentsPage): add advisor chat tool renderer (#24623 ) ## Summary Add a dedicated lightweight advisor chat card to the Agents page so advisor results render as readable prose instead of raw JSON. Includes Storybook coverage for the renderer's states. ## Motivation Once PR 4 lands, the backend can emit advisor tool results, but the generic tool renderer shows them as quoted JSON. A small dedicated renderer turns the recommendation into something an operator or reviewer can actually read, without blocking the backend on full streaming UI. ## Changes - `site/src/pages/AgentsPage/components/ChatElements/tools/AdvisorTool.tsx`: renders the `AdvisorResult` states: - `advice` as markdown prose via the existing `Response` primitive, - `limit_reached` as a warning card, - `error` as an error card with fallback text, - `running` via the existing tool loading spinner. Reuses `ToolCollapsible`, `ToolIcon`, and `Response`. Question is shown only in the card subtitle (no duplicate label in the body). - Metadata footer surfaces `advisor_model` and `remaining_uses` when present. - `Tool.tsx` dispatcher routes the `advisor` tool to the new renderer. - Storybook stories for each state (success/running/limit/error) with `play()` assertions for expand/collapse behavior. - No new React memoization; the Agents page is already React-Compiler-aware. ## Stack context This is PR 5 of 6 in the advisor feature stack, stacked on top of PR 4. ## Scope / non-goals - No changes to the backend schema. - No changes to adjacent tool renderers. ## Validation - `pnpm --filter site lint:types` - `pnpm --filter site test:storybook src/pages/AgentsPage/components/ChatElements/tools/AdvisorTool.stories.tsx` --- <details> <summary>📋 Implementation Plan (shared across the advisor stack)</summary> # Plan: Add a Mux-style advisor tool to coder agents/chatd ## Outcome Add a first-class `advisor` tool to agent chats in `coderd/x/chatd` that feels native to Coder: - it is a built-in server-side tool, not an MCP/dynamic-tool workaround; - it performs a nested tool-less model call for strategic advice; - it is exposed only when eligible, and the prompt mentions it only when it is actually available; - it is treated as a planning-only tool so it does not run alongside action tools in the same batch; - it tracks usage/cost separately enough for operators to reason about it; - it has a minimally polished UI in the Agents page; - and it ships with explicit dogfooding evidence, including screenshots and repro videos. ## Design decisions to lock before coding 1. Primary architecture: native built-in tool in `chattool/`, backed by a small `chatadvisor` package. 2. Nested model execution: reuse chatd's existing model/provider stack for a one-step, tool-less advisor call rather than inventing a new provider pathway. 3. Execution policy: treat `advisor` as an exclusive/planning-only tool; mixed batches must return structured policy errors and force the model to retry cleanly. 4. Availability: initial rollout is for root agent chats only; disable for child/sub-agent chats until recursion/cost policy is proven. 5. Prompt sync: use one eligibility boolean to drive both tool registration and advisor guidance injection. 6. Persistence/cost split: MVP should keep advisor usage visible in result metadata and server metrics; only add DB schema if product/billing explicitly needs queryable advisor-specific cost. 7. UI scope: generic tool rendering is an acceptable temporary milestone during backend bring-up, but the release candidate should include a dedicated lightweight advisor renderer. ## Delivery model The work should be executed as coordinated workstreams with one integration owner and parallel contributors for low-conflict areas. The integration owner should own `coderd/x/chatd/chatd.go` because prompt assembly, tool registration, and model resolution all converge there. ## Detailed workstreams ### Repo evidence used for this plan <details> <summary>Mux reference and current chatd seams</summary> Mux reference implementation - `src/node/services/tools/advisor.ts` — native advisor tool implementation. - `src/common/constants/advisor.ts` — advisor prompt/constants and truncation policy. - `src/common/utils/tools/tools.ts` — conditional tool registration. - `src/node/services/streamContextBuilder.ts` — injects advisor guidance only when the tool is available. Current chatd seams - `coderd/x/chatd/chatd.go` - `processChat()` — tool assembly, prompt assembly, and chatloop invocation. - `resolveChatModel()` — current model/provider/key resolution seam. - `type Config struct` — server-level chatd configuration surface. - `coderd/x/chatd/chatloop/chatloop.go` - `Run()` — main streaming/model loop. - `executeTools()` — built-in tool execution/batching seam. - `coderd/x/chatd/chattool/` — built-in tool implementations. - `site/src/pages/AgentsPage/components/ChatElements/tools/Tool.tsx` — tool renderer dispatch. - `site/src/pages/AgentsPage/components/ChatConversation/messageParsing.ts` and `ConversationTimeline.tsx` — tool/result merge and rendering flow. </details> ### Workstream map and ownership \| Workstream \| Primary owner \| Main files \| Can run in parallel? \| Done when \| \|---\|---\|---\|---\|---\| \| 0. Integration + gating \| Integration lead \| `coderd/x/chatd/chatd.go` \| No; central merge lane \| Tool registration, prompt sync, and model selection are wired together \| \| 1. Advisor runtime + tool \| Backend agent \| new `coderd/x/chatd/chatadvisor/`, new `coderd/x/chatd/chattool/advisor.go` \| Yes \| Tool can perform a tool-less advisor call in memory and return structured results \| \| 2. Planning-only execution policy \| Chatloop agent \| `coderd/x/chatd/chatloop/chatloop.go`, related tests \| Yes \| Mixed `advisor` + action-tool batches are rejected cleanly and deterministically \| \| 3. Metrics/usage/config \| Backend/telemetry agent \| `chatd.go`, `chatloop/metrics.go`, optional config plumbing \| Partially; coordinate with integration lead \| Advisor usage is separately visible in metadata/metrics and limits are enforced \| \| 4. Frontend rendering \| Frontend agent \| `site/.../tools/Tool.tsx`, new `AdvisorTool.tsx`, stories \| Yes after result schema stabilizes \| Advisor renders as a readable card and story tests pass \| \| 5. Dogfood + QA evidence \| QA agent \| dev server, Storybook, dogfood output \| After backend + UI are usable \| Repro videos, screenshots, and a concise QA report exist \| ### Parallelization rules - Do not split `coderd/x/chatd/chatd.go` across multiple execution agents without an integration lead. That file owns prompt building, tool registration, model resolution, and cost persistence. - Workstreams 1 and 2 can be developed in parallel and then stacked onto the integration branch. - Workstream 4 should begin once the backend result schema is agreed on, even if the backend is still behind a feature flag. - Any agent that needs to re-check Mux behavior should clone `coder/mux` into a temporary directory (for example, `$(mktemp -d)/mux`) and inspect it read-only; do not vendor or copy code from Mux directly. ## Phase 0 — Preflight and guardrails ### Goals - Align the team on the smallest shippable architecture. - Prevent scope creep into MCP/dynamic-tool/sub-agent variants. - Decide upfront what is MVP vs. follow-up. ### Tasks 1. Confirm the MVP boundary. - Ship a built-in advisor tool first. - Do not make MCP, dynamic tools, or sub-agents the primary implementation. - Do not add transient streaming phases in the first backend PR unless they fall out almost for free. 2. Confirm local workflow hygiene before coding. - Ensure the repo is using the project git hooks from `scripts/githooks`. - Do not bypass hooks with `--no-verify`. - Use `./scripts/develop.sh` for the full dev server rather than manual build/run commands. 3. Lock the model-selection policy. - Recommended MVP: advisor uses the same resolved provider/model/cost config as the current chat, with advisor-specific max-output and usage caps. - Follow-up only if required: add a separate `AdvisorModelConfigID`-style override that resolves through the existing `configCache`/model-config path. Do not invent a new free-form `provider:model` parser if chatd already stores provider/model separately. 4. Lock the persistence policy. - Recommended MVP: no DB migration. Persist advisor-visible metadata in the tool result and record separate metrics in memory/Prometheus. - Only if product/billing explicitly asks for queryable advisor cost: add a later DB migration or usage table, following the normal `queries/.sql` + `make gen` workflow. 5. Create an execution ADR note in the work item or tracking doc.* - Capture: built-in tool, tool-less nested call, root-chat-only rollout, exclusive execution policy, MVP no-DB-migration default. ### Quality gate - Everyone on the team can state the same answers to these questions: - Is advisor a built-in tool? Yes. - Can advisor run with action tools in the same batch? No. - Does advisor get tools of its own? No. - Is a DB migration required for MVP? No, unless billing insists. ## Phase 1 — Build the advisor runtime and tool wrapper ### Goals Create the core advisor implementation in a way that is easy to test and keeps `chattool/` thin. ### Files to add - `coderd/x/chatd/chatadvisor/types.go` - `coderd/x/chatd/chatadvisor/guidance.go` - `coderd/x/chatd/chatadvisor/handoff.go` - `coderd/x/chatd/chatadvisor/runtime.go` - `coderd/x/chatd/chatadvisor/runner.go` - `coderd/x/chatd/chattool/advisor.go` ### Responsibilities by file 1. `types.go` - Define the input/result schema used by the tool and UI. - Keep the result shape close to Mux so the UI and model both have predictable cases. - Recommended result variants: - `advice` - `limit_reached` - `error` Recommended shape: ```go type AdvisorArgs struct { Question string `json:"question"` } type AdvisorResult struct { Type string `json:"type"` Advice string `json:"advice,omitempty"` Error string `json:"error,omitempty"` AdvisorModel string `json:"advisor_model,omitempty"` RemainingUses int `json:"remaining_uses,omitempty"` Usage AdvisorUsageResult `json:"usage,omitempty"` } ``` 2. `guidance.go`* - Hold two strings: - the nested advisor system prompt; - the parent-agent guidance block to inject into the outer system prompt. - The nested advisor prompt must say, in plain language: - you are advising the parent agent; - you do not address the end user directly; - you do not claim actions happened; - you return concise strategic guidance and tradeoffs. 3. `runtime.go` - Define the per-run runtime state. - Recommended fields: - resolved model + model config; - provider keys/options reused from the outer chat; - `MaxUsesPerRun`; - `MaxOutputTokens`; - atomic/current call counter; - callback(s) to obtain the current prompt snapshot and current-step snapshot; - optional metrics/usage hook. - Add fail-fast validation for impossible config: nil model, non-positive limits, empty prompt builders, etc. 4. `handoff.go` - Build the advisor handoff message from: - the explicit question; - the exact prompt/messages the parent model just used; - the current step's text/reasoning snapshot, if available; - the most recent relevant tool outputs, if they are already in the prompt snapshot. - Important: use the already-prepared outer prompt tail, not a fresh DB reload. That keeps the advisor aligned with compaction and the exact context the outer model saw. - Apply hard truncation budgets with recent-context bias. 5. `runner.go` - Execute the nested advisor call. - Recommended implementation: call `chatloop.Run()` in an in-memory, one-step mode: - `Tools: nil` - `ProviderTools: nil` - `MaxSteps: 1` - `PersistStep`: capture the assistant output in memory instead of writing DB rows - Reuse the existing provider/model/cost path instead of building a second provider runner. - Assert that no tool definitions are passed to the nested call. 6. `chattool/advisor.go` - Keep this file thin and consistent with other built-ins. - Responsibilities: - decode `AdvisorArgs`; - validate `Question` is non-empty and bounded; - call the `chatadvisor` runner; - return a structured tool response. ### Defensive programming requirements - Assert `Question` is non-empty after trimming. - Assert runtime limits are positive. - Assert the nested advisor call runs with zero tools/provider tools. - Assert `AdvisorResult.Type` is one of the known variants before returning. - Assert remaining uses never goes negative. ### Acceptance criteria - A unit test can call the advisor tool with a fake model and receive a stable `advice` result. - The nested advisor call is impossible to run with tools accidentally attached. - The core logic lives in `chatadvisor/`, not embedded inside `chatd.go`. ## Phase 2 — Wire advisor into chatd and keep prompt/tool availability in sync ### Goals Register the tool in the right place, expose it only when eligible, and inject system guidance only when the tool is present. ### Files to modify - `coderd/x/chatd/chatd.go` - optionally a small helper file if `chatd.go` becomes too crowded ### Tasks 1. Compute one eligibility boolean in `processChat()`. Recommended inputs: - server-level advisor enabled flag; - root chat only (`chat.ParentChatID == uuid.Nil` or equivalent existing root/child check); - a usable resolved model/provider exists; - optional experiment/workspace/org gate if product wants staged rollout. 2. Create the runtime once per outer chat run. - Use the model/config/keys resolved by `resolveChatModel()`. - Reuse provider options from the current chat's `ChatModelCallConfig`. - Set `MaxUsesPerRun` and `MaxOutputTokens` from advisor config defaults. 3. Register the tool in the built-in tool block. - Insert after the skill tools and before MCP tools in `processChat()`. - Record `builtinToolNames["advisor"] = true` so metrics stay bounded. 4. Inject advisor guidance into the outer system prompt using the same boolean. - Use `chatprompt.InsertSystem()` in the same prompt assembly path that already injects user/system instructions. - Place the block near the existing instruction insertion, before plan-path/skill context blocks. - Wrap the guidance in an explicit tag like `<advisor-guidance>` so it is easy to spot in tests and future refactors. 5. Keep advisor out of child chats for the first release. - That avoids recursion/cost blowups with `spawn_agent` / `wait_agent` flows. - Document this explicitly in the rollout notes and tests. ### Acceptance criteria - If advisor is disabled, neither the tool nor the prompt guidance appears. - If advisor is enabled, both the tool and the prompt guidance appear. - Root chats can use advisor; child chats cannot. - Built-in tool names include `advisor` so metrics do not collapse it into the generic `mcp` label. ## Phase 3 — Enforce planning-only execution policy in `chatloop` ### Goals Prevent the model from calling `advisor` and action tools in the same execution batch. ### Files to modify - `coderd/x/chatd/chatloop/chatloop.go` - related chatloop tests ### Recommended implementation Keep the MVP small; do not build a general policy engine yet. 1. Add a minimal field to `chatloop.RunOptions`, for example: ```go ExclusiveToolName string ``` 2. In `Run()` / `executeTools()`, detect the case where the exclusive tool appears in the same local-tool batch as any other locally executed tool. 3. When that happens, synthesize structured tool-result errors for the affected calls instead of executing anything in the batch. - `advisor` should receive a clear error like: _advisor must be called by itself before action tools_. - The sibling action tools should receive a paired policy error like: _this tool was skipped because advisor must run alone_. 4. Let the outer model see those tool errors and retry cleanly. - This is simpler and safer than partial execution or hidden deferral. - It preserves deterministic transcript history for debugging. 5. Pass the just-finished step snapshot into the tool execution context. - The advisor runtime should be able to see the current step's text/reasoning content, because that is often the best hint about what the outer model is trying to decide. ### Why this is the right fit - It matches the intended semantics: advisor is consulted before* taking action. - It avoids subtle race conditions caused by concurrent built-in tool execution. - It keeps the behavior easy to test with fake models. ### Acceptance criteria - A model-emitted batch containing only `advisor` succeeds. - A model-emitted batch containing `advisor` plus any other locally executed tool returns deterministic policy errors and executes nothing. - Non-advisor tool execution stays unchanged for normal chats. ## Phase 4 — Usage limits, metrics, and configuration ### Goals Make advisor safe to operate without over-designing billing/storage in the first release. ### Files to modify - `coderd/x/chatd/chatd.go` - `coderd/x/chatd/chatloop/metrics.go` as needed - `coderd/x/chatd/chatd.go` `Config` struct and constructor path - optional follow-up config/db files only if a separate advisor model or persistent billing is required ### Tasks 1. Add explicit server config knobs for MVP. Recommended fields on `chatd.Config` or a nested advisor config struct: - `AdvisorEnabled bool` - `AdvisorMaxUsesPerRun int` - `AdvisorMaxOutputTokens int64` 2. Track usage per outer run. - Reset the counter for each `processChat()` invocation. - Return `remaining_uses` in the tool result. - Return `limit_reached` when the cap is exhausted. 3. Expose advisor usage metadata in the tool result. - Include model name and token/cost summary if available. - Use the same `callConfig.Cost` calculation path as the outer chat for MVP if advisor reuses the same model. 4. Record server-side metrics. - Count advisor invocations, failures, and latency. - Ensure they show up under the built-in tool label `advisor`. 5. Optional decision gate: separate advisor model. - If product insists on a stronger/different advisor model, add a follow-up config hook that resolves another existing chat model config through the same `configCache` path. - Keep that out of the first landing PR unless it is required for acceptance. 6. Optional decision gate: queryable advisor cost. - If this becomes required, spin a follow-up DB task: - update `coderd/database/queries/.sql`; - add migration files; - run `make gen`; - update audit mappings if a new auditable type/field is introduced. ### Acceptance criteria - Advisor calls are capped per outer run. - Limit exhaustion is user-visible in the tool result. - Metrics distinguish advisor calls from other built-in tools. - MVP does not require a schema migration unless explicitly approved. ## Phase 5 — Frontend rendering and Storybook coverage ### Goals Make advisor feel intentional in the Agents UI without blocking the backend on fancy streaming UI. ### Files to modify - `site/src/pages/AgentsPage/components/ChatElements/tools/Tool.tsx` - new `site/src/pages/AgentsPage/components/ChatElements/tools/AdvisorTool.tsx` - Storybook story file(s) in the same tools directory ### Delivery strategy 1. Intermediate milestone during backend bring-up:* rely on the existing generic tool renderer if needed. - This is acceptable only as a short-lived integration checkpoint. 2. Release milestone: add a dedicated lightweight `AdvisorTool` renderer. - Reuse existing primitives: - `ToolCollapsible` - `ToolIcon` - `Response` for markdown/prose rendering - `ScrollArea` if the advice can be long - Keep styling light and consistent with the Agents page. - Do not add unnecessary React memoization in `site/src/pages/AgentsPage/`; that area is already React-Compiler aware. 3. Render the structured result states cleanly. - `advice` — readable prose/markdown with optional metadata footer. - `limit_reached` — warning-style message. - `error` — error state with visible fallback text. - `running` — existing tool loading state/spinner is enough for MVP. 4. Add Storybook coverage instead of ad-hoc component tests. Recommended stories: - successful advice; - running/loading; - limit reached; - error. 5. Keep the UI contract narrow. - Prefer one text field like `advice` plus small metadata rather than a deeply nested schema. - That keeps the UI resilient to prompt iteration. ### Acceptance criteria - The advisor tool card renders readable content rather than raw quoted JSON in the final release branch. - Running, limit, and error states are visibly distinct. - Storybook stories and play assertions cover the new states. - Existing tool rendering flows remain unchanged. ## Phase 6 — Automated tests and validation gates ### Backend tests to add 1. Advisor runtime/tool tests - question validation; - tool-less nested execution assertion; - success result shaping; - limit-reached result shaping; - error result shaping. 2. Prompt/gating tests in chatd - advisor disabled ⇒ no tool, no guidance; - advisor enabled/root chat ⇒ tool + guidance; - child chat ⇒ advisor absent. 3. Chatloop policy tests - advisor alone runs; - advisor + action tool mixed batch returns deterministic policy errors; - non-advisor tools still execute normally. 4. Usage/metrics tests - per-run cap resets correctly; - builtin tool labeling includes `advisor`; - returned metadata includes model/usage summary when available. ### Frontend tests to add - Storybook `play()` assertions for the advisor renderer states. - Verify expand/collapse behavior and visible fallback text. - Verify the message timeline still renders adjacent tools correctly. ### Recommended command sequence Run these as the implementation matures, not only at the end: 1. Backend-focused gate after phases 1–4: - `make test RUN=TestAdvisor` - `make test RUN=TestChatloopAdvisor` - `make lint` 2. Frontend-focused gate after phase 5: - `pnpm test:storybook src/pages/AgentsPage/components/ChatElements/tools/AdvisorTool.stories.tsx` - `pnpm lint` - `pnpm format` 3. Final repo gate before handoff: - `make pre-commit` - run any additional targeted `make test RUN=...` selections covering touched chatd paths > Use the exact new test names the implementing agents create; the names above are recommended anchors, not existing tests. ## Dogfooding plan ### Principle Dogfood the change as a real agent feature, not just a unit-tested backend. Per the dogfood and `agent-browser` skills, the reviewer should get watchable repro videos plus screenshots that make the behavior obvious without reading logs. ### Required setup 1. Start the full dev environment with: - `./scripts/develop.sh` 2. If the frontend renderer changes, also start Storybook from `site/` with: - `pnpm storybook --no-open` 3. Use `agent-browser` directly — never `npx agent-browser`. 4. Use named browser sessions and an output folder such as: - `./dogfood-output/advisor/` - with subfolders `screenshots/` and `videos/` ### Evidence protocol For every interactive scenario below: 1. Start video recording before the action. 2. Capture step-by-step screenshots at human pace. 3. Capture one annotated screenshot of the final state. 4. Stop the recording. 5. Note the exact pass/fail observation in the QA report. For static UI states (for example Storybook error/limit cards), an annotated screenshot is sufficient; video is optional but still encouraged by this project’s review preference. ### Dogfood scenarios #### Scenario A — Happy path in the real Agents UI Goal: prove that a root agent chat can invoke advisor and produce a readable recommendation before taking further action. Steps: 1. Open the Agents page with an advisor-enabled root chat. 2. Start a repro video. 3. Send a prompt that should reasonably trigger strategic planning, such as an architecture or multi-tradeoff question. 4. Capture screenshots of: - the prompt before send; - the running advisor state; - the completed advisor card and the assistant’s follow-up response. 5. Stop recording. Pass criteria: - advisor appears in the timeline; - the rendered result is readable; - the assistant can continue after consuming the advisor output. #### Scenario B — Advisor unavailable path Goal: prove the feature is truly gated. Suggested variants (at least one is required, both are better): - feature flag/config off; - child/sub-agent chat. Evidence: - annotated screenshot of the chat/tool state showing advisor is absent; - short video if toggling the gate live is part of the repro. Pass criteria: - no advisor tool is available; - no advisor-specific prompt behavior leaks through. #### Scenario C — UI states in Storybook Goal: prove the renderer handles non-happy states cleanly. Required story states: - success/advice; - running; - limit reached; - error. Evidence: - one screenshot per state; - at least one short video showing collapse/expand behavior. Pass criteria: - success renders readable advice; - limit/error have visible fallback text; - the component behaves like the other tool cards. #### Scenario D — Regression sweep of nearby tools Goal: ensure advisor does not break the surrounding chat timeline. Check at minimum: - another existing built-in tool still renders correctly near advisor; - sub-agent/tool cards still expand/collapse normally; - no obvious console errors appear in the Agents page during the advisor flow. Evidence: - screenshots of adjacent tool cards; - console/error capture if anything suspicious appears. ### `agent-browser` usage notes for the QA agent - Prefer `agent-browser batch` for 2+ sequential commands when no intermediate parsing is needed. - Use `snapshot -i` to discover interactive refs. - Re-snapshot after navigation or major DOM changes. - Avoid `wait --load networkidle` unless the page is known to go idle; prefer explicit element/text waits or short fixed waits. - Record videos at human pace and include pauses that a reviewer can follow. ## Rollout plan ### Initial rollout - Gate behind a server-side advisor-enabled flag. - Enable only for selected internal/root agent chats first. - Watch metrics for: - invocation count; - failure rate; - latency; - obvious retry loops. ### Expansion conditions Expand beyond the initial rollout only after the following are true: - mixed-batch policy behavior is stable; - cost impact is understood; - frontend UX is readable in production-like dogfood; - no recursion surprises have appeared with sub-agent flows. ### Explicit non-goals for the first release - advisor inside child/sub-agent chats; - provider-agnostic streaming phase UI; - MCP-based external advisor implementation; - mandatory DB-backed advisor cost reporting. ## Final acceptance checklist - [ ] `advisor` is a built-in chatd tool, not an MCP/dynamic-tool substitute. - [ ] The nested advisor call is tool-less and bounded to one in-memory step. - [ ] One eligibility boolean controls both tool registration and prompt guidance injection. - [ ] Root chats can use advisor; child chats cannot in the initial rollout. - [ ] Mixed advisor/action batches produce deterministic policy errors instead of partial execution. - [ ] Per-run usage caps and limit-reached behavior work. - [ ] Advisor usage is visible in metadata/metrics without forcing a DB migration for MVP. - [ ] The Agents UI has a readable advisor card and Storybook coverage. - [ ] Dogfooding produced screenshots and repro videos for the required scenarios. - [ ] Validation commands (`make lint`, targeted `make test`, Storybook tests, `make pre-commit`) passed before handoff. ## Suggested PR split 1. PR 1 — Backend foundation - `chatadvisor/` package - `chattool/advisor.go` - `chatloop` exclusive policy - chatd gating/prompt sync - backend tests 2. PR 2 — Frontend + QA - advisor renderer - stories/play assertions - dogfood artifacts and QA notes 3. PR 3 — Optional follow-ups only if demanded by stakeholders - separate advisor model override - persistent advisor billing/queryability - transient phase-stream UX </details> --- _Generated with [`mux`](https://github.com/coder/mux) • Model: `anthropic:claude-opus-4-7` • Thinking: `max`_	2026-04-30 15:17:32 +02:00
Thomas Kosiewski	17409a515c	feat(coderd): wire advisor runtime to admin config (#24622 ) ## Summary Wire the advisor runtime into `chatd`: read the admin config on every `runChat`, gate tool registration and system-prompt guidance on a single eligibility boolean, register the `advisor` built-in tool, and apply the exclusive-tool policy from PR 1. ## Motivation This is the integration seam where PRs 1–3 come together into an actual user-visible feature. Gating is deliberately root-chat-only for the initial rollout; child/sub-agent chats still do not see the tool or the guidance block. ## Changes ### `coderd/x/chatd/chatd.go` - `loadAdvisorConfig(ctx, logger)` reads the admin config (from PR 3) on each run. If `ModelConfigID` is set, it resolves the override model via `configCache.ModelConfigByID`; otherwise it falls back to the outer chat's model and provider options. Reasoning effort is plumbed into provider options via `applyAdvisorReasoningEffort`. - One computed `advisorEligible` boolean drives both tool registration (after skill tools, before MCP tools) and guidance injection via `chatprompt.InsertSystem(prompt, chatadvisor.ParentGuidanceBlock)`. - `setAdvisorPromptSnapshot` closures capture the outer prompt state at the right points in the lifecycle (`renderPlanPathPrompt`, `ReloadMessages`, `PrepareMessages`) so the advisor handoff uses the same context the outer model saw. - `ExclusiveToolNames["advisor"] = true` is passed to `chatloop.Run()` so mixed batches are rejected cleanly (PR 1 machinery). - `builtinToolNames["advisor"] = true` so metrics keep advisor distinct from the generic `mcp` label. ### Child-chat guard - Child/sub-agent chats deliberately do not see the advisor tool or guidance block, to avoid recursion/cost blowups until the pattern is proven. This is covered by `TestAdvisorGating_ChildChat` (currently skipped pending a rewrite against the new `plan`/`explore` subagent infrastructure; core gating logic is still exercised by `TestAdvisorGating_Disabled` and `TestAdvisorGating_RootChat`). ## Stack context This is PR 4 of 6 in the advisor feature stack. It depends on PRs 1–3. ## Scope / non-goals - No frontend changes. The feature is invocable via the backend but renders generically until PR 5. - No separate provider runner; the nested advisor call reuses the existing model/provider path. - No DB migration. ## Validation - `go test ./coderd/x/chatd/... -run TestAdvisor` - `go build ./...` - `make lint` --- <details> <summary>📋 Implementation Plan (shared across the advisor stack)</summary> # Plan: Add a Mux-style advisor tool to coder agents/chatd ## Outcome Add a first-class `advisor` tool to agent chats in `coderd/x/chatd` that feels native to Coder: - it is a built-in server-side tool, not an MCP/dynamic-tool workaround; - it performs a nested tool-less model call for strategic advice; - it is exposed only when eligible, and the prompt mentions it only when it is actually available; - it is treated as a planning-only tool so it does not run alongside action tools in the same batch; - it tracks usage/cost separately enough for operators to reason about it; - it has a minimally polished UI in the Agents page; - and it ships with explicit dogfooding evidence, including screenshots and repro videos. ## Design decisions to lock before coding 1. Primary architecture: native built-in tool in `chattool/`, backed by a small `chatadvisor` package. 2. Nested model execution: reuse chatd's existing model/provider stack for a one-step, tool-less advisor call rather than inventing a new provider pathway. 3. Execution policy: treat `advisor` as an exclusive/planning-only tool; mixed batches must return structured policy errors and force the model to retry cleanly. 4. Availability: initial rollout is for root agent chats only; disable for child/sub-agent chats until recursion/cost policy is proven. 5. Prompt sync: use one eligibility boolean to drive both tool registration and advisor guidance injection. 6. Persistence/cost split: MVP should keep advisor usage visible in result metadata and server metrics; only add DB schema if product/billing explicitly needs queryable advisor-specific cost. 7. UI scope: generic tool rendering is an acceptable temporary milestone during backend bring-up, but the release candidate should include a dedicated lightweight advisor renderer. ## Delivery model The work should be executed as coordinated workstreams with one integration owner and parallel contributors for low-conflict areas. The integration owner should own `coderd/x/chatd/chatd.go` because prompt assembly, tool registration, and model resolution all converge there. ## Detailed workstreams ### Repo evidence used for this plan <details> <summary>Mux reference and current chatd seams</summary> Mux reference implementation - `src/node/services/tools/advisor.ts` — native advisor tool implementation. - `src/common/constants/advisor.ts` — advisor prompt/constants and truncation policy. - `src/common/utils/tools/tools.ts` — conditional tool registration. - `src/node/services/streamContextBuilder.ts` — injects advisor guidance only when the tool is available. Current chatd seams - `coderd/x/chatd/chatd.go` - `processChat()` — tool assembly, prompt assembly, and chatloop invocation. - `resolveChatModel()` — current model/provider/key resolution seam. - `type Config struct` — server-level chatd configuration surface. - `coderd/x/chatd/chatloop/chatloop.go` - `Run()` — main streaming/model loop. - `executeTools()` — built-in tool execution/batching seam. - `coderd/x/chatd/chattool/` — built-in tool implementations. - `site/src/pages/AgentsPage/components/ChatElements/tools/Tool.tsx` — tool renderer dispatch. - `site/src/pages/AgentsPage/components/ChatConversation/messageParsing.ts` and `ConversationTimeline.tsx` — tool/result merge and rendering flow. </details> ### Workstream map and ownership \| Workstream \| Primary owner \| Main files \| Can run in parallel? \| Done when \| \|---\|---\|---\|---\|---\| \| 0. Integration + gating \| Integration lead \| `coderd/x/chatd/chatd.go` \| No; central merge lane \| Tool registration, prompt sync, and model selection are wired together \| \| 1. Advisor runtime + tool \| Backend agent \| new `coderd/x/chatd/chatadvisor/`, new `coderd/x/chatd/chattool/advisor.go` \| Yes \| Tool can perform a tool-less advisor call in memory and return structured results \| \| 2. Planning-only execution policy \| Chatloop agent \| `coderd/x/chatd/chatloop/chatloop.go`, related tests \| Yes \| Mixed `advisor` + action-tool batches are rejected cleanly and deterministically \| \| 3. Metrics/usage/config \| Backend/telemetry agent \| `chatd.go`, `chatloop/metrics.go`, optional config plumbing \| Partially; coordinate with integration lead \| Advisor usage is separately visible in metadata/metrics and limits are enforced \| \| 4. Frontend rendering \| Frontend agent \| `site/.../tools/Tool.tsx`, new `AdvisorTool.tsx`, stories \| Yes after result schema stabilizes \| Advisor renders as a readable card and story tests pass \| \| 5. Dogfood + QA evidence \| QA agent \| dev server, Storybook, dogfood output \| After backend + UI are usable \| Repro videos, screenshots, and a concise QA report exist \| ### Parallelization rules - Do not split `coderd/x/chatd/chatd.go` across multiple execution agents without an integration lead. That file owns prompt building, tool registration, model resolution, and cost persistence. - Workstreams 1 and 2 can be developed in parallel and then stacked onto the integration branch. - Workstream 4 should begin once the backend result schema is agreed on, even if the backend is still behind a feature flag. - Any agent that needs to re-check Mux behavior should clone `coder/mux` into a temporary directory (for example, `$(mktemp -d)/mux`) and inspect it read-only; do not vendor or copy code from Mux directly. ## Phase 0 — Preflight and guardrails ### Goals - Align the team on the smallest shippable architecture. - Prevent scope creep into MCP/dynamic-tool/sub-agent variants. - Decide upfront what is MVP vs. follow-up. ### Tasks 1. Confirm the MVP boundary. - Ship a built-in advisor tool first. - Do not make MCP, dynamic tools, or sub-agents the primary implementation. - Do not add transient streaming phases in the first backend PR unless they fall out almost for free. 2. Confirm local workflow hygiene before coding. - Ensure the repo is using the project git hooks from `scripts/githooks`. - Do not bypass hooks with `--no-verify`. - Use `./scripts/develop.sh` for the full dev server rather than manual build/run commands. 3. Lock the model-selection policy. - Recommended MVP: advisor uses the same resolved provider/model/cost config as the current chat, with advisor-specific max-output and usage caps. - Follow-up only if required: add a separate `AdvisorModelConfigID`-style override that resolves through the existing `configCache`/model-config path. Do not invent a new free-form `provider:model` parser if chatd already stores provider/model separately. 4. Lock the persistence policy. - Recommended MVP: no DB migration. Persist advisor-visible metadata in the tool result and record separate metrics in memory/Prometheus. - Only if product/billing explicitly asks for queryable advisor cost: add a later DB migration or usage table, following the normal `queries/.sql` + `make gen` workflow. 5. Create an execution ADR note in the work item or tracking doc.* - Capture: built-in tool, tool-less nested call, root-chat-only rollout, exclusive execution policy, MVP no-DB-migration default. ### Quality gate - Everyone on the team can state the same answers to these questions: - Is advisor a built-in tool? Yes. - Can advisor run with action tools in the same batch? No. - Does advisor get tools of its own? No. - Is a DB migration required for MVP? No, unless billing insists. ## Phase 1 — Build the advisor runtime and tool wrapper ### Goals Create the core advisor implementation in a way that is easy to test and keeps `chattool/` thin. ### Files to add - `coderd/x/chatd/chatadvisor/types.go` - `coderd/x/chatd/chatadvisor/guidance.go` - `coderd/x/chatd/chatadvisor/handoff.go` - `coderd/x/chatd/chatadvisor/runtime.go` - `coderd/x/chatd/chatadvisor/runner.go` - `coderd/x/chatd/chattool/advisor.go` ### Responsibilities by file 1. `types.go` - Define the input/result schema used by the tool and UI. - Keep the result shape close to Mux so the UI and model both have predictable cases. - Recommended result variants: - `advice` - `limit_reached` - `error` Recommended shape: ```go type AdvisorArgs struct { Question string `json:"question"` } type AdvisorResult struct { Type string `json:"type"` Advice string `json:"advice,omitempty"` Error string `json:"error,omitempty"` AdvisorModel string `json:"advisor_model,omitempty"` RemainingUses int `json:"remaining_uses,omitempty"` Usage AdvisorUsageResult `json:"usage,omitempty"` } ``` 2. `guidance.go`* - Hold two strings: - the nested advisor system prompt; - the parent-agent guidance block to inject into the outer system prompt. - The nested advisor prompt must say, in plain language: - you are advising the parent agent; - you do not address the end user directly; - you do not claim actions happened; - you return concise strategic guidance and tradeoffs. 3. `runtime.go` - Define the per-run runtime state. - Recommended fields: - resolved model + model config; - provider keys/options reused from the outer chat; - `MaxUsesPerRun`; - `MaxOutputTokens`; - atomic/current call counter; - callback(s) to obtain the current prompt snapshot and current-step snapshot; - optional metrics/usage hook. - Add fail-fast validation for impossible config: nil model, non-positive limits, empty prompt builders, etc. 4. `handoff.go` - Build the advisor handoff message from: - the explicit question; - the exact prompt/messages the parent model just used; - the current step's text/reasoning snapshot, if available; - the most recent relevant tool outputs, if they are already in the prompt snapshot. - Important: use the already-prepared outer prompt tail, not a fresh DB reload. That keeps the advisor aligned with compaction and the exact context the outer model saw. - Apply hard truncation budgets with recent-context bias. 5. `runner.go` - Execute the nested advisor call. - Recommended implementation: call `chatloop.Run()` in an in-memory, one-step mode: - `Tools: nil` - `ProviderTools: nil` - `MaxSteps: 1` - `PersistStep`: capture the assistant output in memory instead of writing DB rows - Reuse the existing provider/model/cost path instead of building a second provider runner. - Assert that no tool definitions are passed to the nested call. 6. `chattool/advisor.go` - Keep this file thin and consistent with other built-ins. - Responsibilities: - decode `AdvisorArgs`; - validate `Question` is non-empty and bounded; - call the `chatadvisor` runner; - return a structured tool response. ### Defensive programming requirements - Assert `Question` is non-empty after trimming. - Assert runtime limits are positive. - Assert the nested advisor call runs with zero tools/provider tools. - Assert `AdvisorResult.Type` is one of the known variants before returning. - Assert remaining uses never goes negative. ### Acceptance criteria - A unit test can call the advisor tool with a fake model and receive a stable `advice` result. - The nested advisor call is impossible to run with tools accidentally attached. - The core logic lives in `chatadvisor/`, not embedded inside `chatd.go`. ## Phase 2 — Wire advisor into chatd and keep prompt/tool availability in sync ### Goals Register the tool in the right place, expose it only when eligible, and inject system guidance only when the tool is present. ### Files to modify - `coderd/x/chatd/chatd.go` - optionally a small helper file if `chatd.go` becomes too crowded ### Tasks 1. Compute one eligibility boolean in `processChat()`. Recommended inputs: - server-level advisor enabled flag; - root chat only (`chat.ParentChatID == uuid.Nil` or equivalent existing root/child check); - a usable resolved model/provider exists; - optional experiment/workspace/org gate if product wants staged rollout. 2. Create the runtime once per outer chat run. - Use the model/config/keys resolved by `resolveChatModel()`. - Reuse provider options from the current chat's `ChatModelCallConfig`. - Set `MaxUsesPerRun` and `MaxOutputTokens` from advisor config defaults. 3. Register the tool in the built-in tool block. - Insert after the skill tools and before MCP tools in `processChat()`. - Record `builtinToolNames["advisor"] = true` so metrics stay bounded. 4. Inject advisor guidance into the outer system prompt using the same boolean. - Use `chatprompt.InsertSystem()` in the same prompt assembly path that already injects user/system instructions. - Place the block near the existing instruction insertion, before plan-path/skill context blocks. - Wrap the guidance in an explicit tag like `<advisor-guidance>` so it is easy to spot in tests and future refactors. 5. Keep advisor out of child chats for the first release. - That avoids recursion/cost blowups with `spawn_agent` / `wait_agent` flows. - Document this explicitly in the rollout notes and tests. ### Acceptance criteria - If advisor is disabled, neither the tool nor the prompt guidance appears. - If advisor is enabled, both the tool and the prompt guidance appear. - Root chats can use advisor; child chats cannot. - Built-in tool names include `advisor` so metrics do not collapse it into the generic `mcp` label. ## Phase 3 — Enforce planning-only execution policy in `chatloop` ### Goals Prevent the model from calling `advisor` and action tools in the same execution batch. ### Files to modify - `coderd/x/chatd/chatloop/chatloop.go` - related chatloop tests ### Recommended implementation Keep the MVP small; do not build a general policy engine yet. 1. Add a minimal field to `chatloop.RunOptions`, for example: ```go ExclusiveToolName string ``` 2. In `Run()` / `executeTools()`, detect the case where the exclusive tool appears in the same local-tool batch as any other locally executed tool. 3. When that happens, synthesize structured tool-result errors for the affected calls instead of executing anything in the batch. - `advisor` should receive a clear error like: _advisor must be called by itself before action tools_. - The sibling action tools should receive a paired policy error like: _this tool was skipped because advisor must run alone_. 4. Let the outer model see those tool errors and retry cleanly. - This is simpler and safer than partial execution or hidden deferral. - It preserves deterministic transcript history for debugging. 5. Pass the just-finished step snapshot into the tool execution context. - The advisor runtime should be able to see the current step's text/reasoning content, because that is often the best hint about what the outer model is trying to decide. ### Why this is the right fit - It matches the intended semantics: advisor is consulted before* taking action. - It avoids subtle race conditions caused by concurrent built-in tool execution. - It keeps the behavior easy to test with fake models. ### Acceptance criteria - A model-emitted batch containing only `advisor` succeeds. - A model-emitted batch containing `advisor` plus any other locally executed tool returns deterministic policy errors and executes nothing. - Non-advisor tool execution stays unchanged for normal chats. ## Phase 4 — Usage limits, metrics, and configuration ### Goals Make advisor safe to operate without over-designing billing/storage in the first release. ### Files to modify - `coderd/x/chatd/chatd.go` - `coderd/x/chatd/chatloop/metrics.go` as needed - `coderd/x/chatd/chatd.go` `Config` struct and constructor path - optional follow-up config/db files only if a separate advisor model or persistent billing is required ### Tasks 1. Add explicit server config knobs for MVP. Recommended fields on `chatd.Config` or a nested advisor config struct: - `AdvisorEnabled bool` - `AdvisorMaxUsesPerRun int` - `AdvisorMaxOutputTokens int64` 2. Track usage per outer run. - Reset the counter for each `processChat()` invocation. - Return `remaining_uses` in the tool result. - Return `limit_reached` when the cap is exhausted. 3. Expose advisor usage metadata in the tool result. - Include model name and token/cost summary if available. - Use the same `callConfig.Cost` calculation path as the outer chat for MVP if advisor reuses the same model. 4. Record server-side metrics. - Count advisor invocations, failures, and latency. - Ensure they show up under the built-in tool label `advisor`. 5. Optional decision gate: separate advisor model. - If product insists on a stronger/different advisor model, add a follow-up config hook that resolves another existing chat model config through the same `configCache` path. - Keep that out of the first landing PR unless it is required for acceptance. 6. Optional decision gate: queryable advisor cost. - If this becomes required, spin a follow-up DB task: - update `coderd/database/queries/.sql`; - add migration files; - run `make gen`; - update audit mappings if a new auditable type/field is introduced. ### Acceptance criteria - Advisor calls are capped per outer run. - Limit exhaustion is user-visible in the tool result. - Metrics distinguish advisor calls from other built-in tools. - MVP does not require a schema migration unless explicitly approved. ## Phase 5 — Frontend rendering and Storybook coverage ### Goals Make advisor feel intentional in the Agents UI without blocking the backend on fancy streaming UI. ### Files to modify - `site/src/pages/AgentsPage/components/ChatElements/tools/Tool.tsx` - new `site/src/pages/AgentsPage/components/ChatElements/tools/AdvisorTool.tsx` - Storybook story file(s) in the same tools directory ### Delivery strategy 1. Intermediate milestone during backend bring-up:* rely on the existing generic tool renderer if needed. - This is acceptable only as a short-lived integration checkpoint. 2. Release milestone: add a dedicated lightweight `AdvisorTool` renderer. - Reuse existing primitives: - `ToolCollapsible` - `ToolIcon` - `Response` for markdown/prose rendering - `ScrollArea` if the advice can be long - Keep styling light and consistent with the Agents page. - Do not add unnecessary React memoization in `site/src/pages/AgentsPage/`; that area is already React-Compiler aware. 3. Render the structured result states cleanly. - `advice` — readable prose/markdown with optional metadata footer. - `limit_reached` — warning-style message. - `error` — error state with visible fallback text. - `running` — existing tool loading state/spinner is enough for MVP. 4. Add Storybook coverage instead of ad-hoc component tests. Recommended stories: - successful advice; - running/loading; - limit reached; - error. 5. Keep the UI contract narrow. - Prefer one text field like `advice` plus small metadata rather than a deeply nested schema. - That keeps the UI resilient to prompt iteration. ### Acceptance criteria - The advisor tool card renders readable content rather than raw quoted JSON in the final release branch. - Running, limit, and error states are visibly distinct. - Storybook stories and play assertions cover the new states. - Existing tool rendering flows remain unchanged. ## Phase 6 — Automated tests and validation gates ### Backend tests to add 1. Advisor runtime/tool tests - question validation; - tool-less nested execution assertion; - success result shaping; - limit-reached result shaping; - error result shaping. 2. Prompt/gating tests in chatd - advisor disabled ⇒ no tool, no guidance; - advisor enabled/root chat ⇒ tool + guidance; - child chat ⇒ advisor absent. 3. Chatloop policy tests - advisor alone runs; - advisor + action tool mixed batch returns deterministic policy errors; - non-advisor tools still execute normally. 4. Usage/metrics tests - per-run cap resets correctly; - builtin tool labeling includes `advisor`; - returned metadata includes model/usage summary when available. ### Frontend tests to add - Storybook `play()` assertions for the advisor renderer states. - Verify expand/collapse behavior and visible fallback text. - Verify the message timeline still renders adjacent tools correctly. ### Recommended command sequence Run these as the implementation matures, not only at the end: 1. Backend-focused gate after phases 1–4: - `make test RUN=TestAdvisor` - `make test RUN=TestChatloopAdvisor` - `make lint` 2. Frontend-focused gate after phase 5: - `pnpm test:storybook src/pages/AgentsPage/components/ChatElements/tools/AdvisorTool.stories.tsx` - `pnpm lint` - `pnpm format` 3. Final repo gate before handoff: - `make pre-commit` - run any additional targeted `make test RUN=...` selections covering touched chatd paths > Use the exact new test names the implementing agents create; the names above are recommended anchors, not existing tests. ## Dogfooding plan ### Principle Dogfood the change as a real agent feature, not just a unit-tested backend. Per the dogfood and `agent-browser` skills, the reviewer should get watchable repro videos plus screenshots that make the behavior obvious without reading logs. ### Required setup 1. Start the full dev environment with: - `./scripts/develop.sh` 2. If the frontend renderer changes, also start Storybook from `site/` with: - `pnpm storybook --no-open` 3. Use `agent-browser` directly — never `npx agent-browser`. 4. Use named browser sessions and an output folder such as: - `./dogfood-output/advisor/` - with subfolders `screenshots/` and `videos/` ### Evidence protocol For every interactive scenario below: 1. Start video recording before the action. 2. Capture step-by-step screenshots at human pace. 3. Capture one annotated screenshot of the final state. 4. Stop the recording. 5. Note the exact pass/fail observation in the QA report. For static UI states (for example Storybook error/limit cards), an annotated screenshot is sufficient; video is optional but still encouraged by this project’s review preference. ### Dogfood scenarios #### Scenario A — Happy path in the real Agents UI Goal: prove that a root agent chat can invoke advisor and produce a readable recommendation before taking further action. Steps: 1. Open the Agents page with an advisor-enabled root chat. 2. Start a repro video. 3. Send a prompt that should reasonably trigger strategic planning, such as an architecture or multi-tradeoff question. 4. Capture screenshots of: - the prompt before send; - the running advisor state; - the completed advisor card and the assistant’s follow-up response. 5. Stop recording. Pass criteria: - advisor appears in the timeline; - the rendered result is readable; - the assistant can continue after consuming the advisor output. #### Scenario B — Advisor unavailable path Goal: prove the feature is truly gated. Suggested variants (at least one is required, both are better): - feature flag/config off; - child/sub-agent chat. Evidence: - annotated screenshot of the chat/tool state showing advisor is absent; - short video if toggling the gate live is part of the repro. Pass criteria: - no advisor tool is available; - no advisor-specific prompt behavior leaks through. #### Scenario C — UI states in Storybook Goal: prove the renderer handles non-happy states cleanly. Required story states: - success/advice; - running; - limit reached; - error. Evidence: - one screenshot per state; - at least one short video showing collapse/expand behavior. Pass criteria: - success renders readable advice; - limit/error have visible fallback text; - the component behaves like the other tool cards. #### Scenario D — Regression sweep of nearby tools Goal: ensure advisor does not break the surrounding chat timeline. Check at minimum: - another existing built-in tool still renders correctly near advisor; - sub-agent/tool cards still expand/collapse normally; - no obvious console errors appear in the Agents page during the advisor flow. Evidence: - screenshots of adjacent tool cards; - console/error capture if anything suspicious appears. ### `agent-browser` usage notes for the QA agent - Prefer `agent-browser batch` for 2+ sequential commands when no intermediate parsing is needed. - Use `snapshot -i` to discover interactive refs. - Re-snapshot after navigation or major DOM changes. - Avoid `wait --load networkidle` unless the page is known to go idle; prefer explicit element/text waits or short fixed waits. - Record videos at human pace and include pauses that a reviewer can follow. ## Rollout plan ### Initial rollout - Gate behind a server-side advisor-enabled flag. - Enable only for selected internal/root agent chats first. - Watch metrics for: - invocation count; - failure rate; - latency; - obvious retry loops. ### Expansion conditions Expand beyond the initial rollout only after the following are true: - mixed-batch policy behavior is stable; - cost impact is understood; - frontend UX is readable in production-like dogfood; - no recursion surprises have appeared with sub-agent flows. ### Explicit non-goals for the first release - advisor inside child/sub-agent chats; - provider-agnostic streaming phase UI; - MCP-based external advisor implementation; - mandatory DB-backed advisor cost reporting. ## Final acceptance checklist - [ ] `advisor` is a built-in chatd tool, not an MCP/dynamic-tool substitute. - [ ] The nested advisor call is tool-less and bounded to one in-memory step. - [ ] One eligibility boolean controls both tool registration and prompt guidance injection. - [ ] Root chats can use advisor; child chats cannot in the initial rollout. - [ ] Mixed advisor/action batches produce deterministic policy errors instead of partial execution. - [ ] Per-run usage caps and limit-reached behavior work. - [ ] Advisor usage is visible in metadata/metrics without forcing a DB migration for MVP. - [ ] The Agents UI has a readable advisor card and Storybook coverage. - [ ] Dogfooding produced screenshots and repro videos for the required scenarios. - [ ] Validation commands (`make lint`, targeted `make test`, Storybook tests, `make pre-commit`) passed before handoff. ## Suggested PR split 1. PR 1 — Backend foundation - `chatadvisor/` package - `chattool/advisor.go` - `chatloop` exclusive policy - chatd gating/prompt sync - backend tests 2. PR 2 — Frontend + QA - advisor renderer - stories/play assertions - dogfood artifacts and QA notes 3. PR 3 — Optional follow-ups only if demanded by stakeholders - separate advisor model override - persistent advisor billing/queryability - transient phase-stream UX </details> --- _Generated with [`mux`](https://github.com/coder/mux) • Model: `anthropic:claude-opus-4-7` • Thinking: `max`_	2026-04-30 15:07:33 +02:00
Thomas Kosiewski	06bad73df4	feat: add admin-configurable advisor API, SDK, and queries (#24621 ) ## Summary Add the admin-configurable advisor configuration: database-backed storage, SDK types, and the experimental HTTP handlers that back the admin settings UI (later PRs). Follows the same "site-configs" pattern as Virtual Desktop. ## Motivation The advisor needs runtime-tunable knobs (enable/disable, per-run cap, max output tokens, reasoning effort, optional model override) without a service restart or redeploy. Using the existing `site_configs` K/V table keeps this pattern consistent with other admin features and avoids a bespoke schema. ## Changes ### Database (`coderd/database/queries/siteconfig.sql`) - `GetChatAdvisorConfig` returns the stored JSON blob (default `'{}'`) under key `agents_advisor_config`. - `UpsertChatAdvisorConfig` uses the standard `INSERT ... ON CONFLICT` pattern. - Regenerated via `make gen` (queries.sql.go + mocks). ### SDK (`codersdk/chats.go`) - `AdvisorConfig` type with `Enabled`, `MaxUsesPerRun`, `MaxOutputTokens`, `ReasoningEffort` (`""` / `low` / `medium` / `high`), `ModelConfigID uuid.UUID`. - Client methods: `ChatAdvisorConfig(ctx)` / `UpdateChatAdvisorConfig(ctx, cfg)`. ### API (`coderd/exp_chats.go`) - `GET /api/experimental/chats/config/advisor`: reads current config; relies on `ActorFromContext` validation. - `PUT /api/experimental/chats/config/advisor`: requires `policy.ActionUpdate` on `rbac.ResourceDeploymentConfig`. - Handlers unmarshal `{}` to a typed zero value and re-marshal on upsert for schema stability. - Tests in `exp_chats_test.go` cover empty defaults, round-trip update, unauthorized update, and invalid body. ## Stack context This is PR 3 of 6 in the advisor feature stack. Consumed by: - PR 4 (`feat/advisor-04-chatd-runtime`), which reads this config on every `runChat`. - PR 6 (`feat/advisor-06-admin-settings-ui`), which renders the admin form. ## Scope / non-goals - No `chatd` read path (lands in PR 4). - No UI (lands in PR 6). - `agents_advisor_config` remains a single-row JSON blob; we intentionally do not shard per-org/per-template yet. ## Validation - `make gen` - `go test ./coderd/database/... -run TestChatAdvisor` - `go test ./coderd/... -run TestChatAdvisorConfig` - `make lint` --- <details> <summary>📋 Implementation Plan (shared across the advisor stack)</summary> # Plan: Add a Mux-style advisor tool to coder agents/chatd ## Outcome Add a first-class `advisor` tool to agent chats in `coderd/x/chatd` that feels native to Coder: - it is a built-in server-side tool, not an MCP/dynamic-tool workaround; - it performs a nested tool-less model call for strategic advice; - it is exposed only when eligible, and the prompt mentions it only when it is actually available; - it is treated as a planning-only tool so it does not run alongside action tools in the same batch; - it tracks usage/cost separately enough for operators to reason about it; - it has a minimally polished UI in the Agents page; - and it ships with explicit dogfooding evidence, including screenshots and repro videos. ## Design decisions to lock before coding 1. Primary architecture: native built-in tool in `chattool/`, backed by a small `chatadvisor` package. 2. Nested model execution: reuse chatd's existing model/provider stack for a one-step, tool-less advisor call rather than inventing a new provider pathway. 3. Execution policy: treat `advisor` as an exclusive/planning-only tool; mixed batches must return structured policy errors and force the model to retry cleanly. 4. Availability: initial rollout is for root agent chats only; disable for child/sub-agent chats until recursion/cost policy is proven. 5. Prompt sync: use one eligibility boolean to drive both tool registration and advisor guidance injection. 6. Persistence/cost split: MVP should keep advisor usage visible in result metadata and server metrics; only add DB schema if product/billing explicitly needs queryable advisor-specific cost. 7. UI scope: generic tool rendering is an acceptable temporary milestone during backend bring-up, but the release candidate should include a dedicated lightweight advisor renderer. ## Delivery model The work should be executed as coordinated workstreams with one integration owner and parallel contributors for low-conflict areas. The integration owner should own `coderd/x/chatd/chatd.go` because prompt assembly, tool registration, and model resolution all converge there. ## Detailed workstreams ### Repo evidence used for this plan <details> <summary>Mux reference and current chatd seams</summary> Mux reference implementation - `src/node/services/tools/advisor.ts` — native advisor tool implementation. - `src/common/constants/advisor.ts` — advisor prompt/constants and truncation policy. - `src/common/utils/tools/tools.ts` — conditional tool registration. - `src/node/services/streamContextBuilder.ts` — injects advisor guidance only when the tool is available. Current chatd seams - `coderd/x/chatd/chatd.go` - `processChat()` — tool assembly, prompt assembly, and chatloop invocation. - `resolveChatModel()` — current model/provider/key resolution seam. - `type Config struct` — server-level chatd configuration surface. - `coderd/x/chatd/chatloop/chatloop.go` - `Run()` — main streaming/model loop. - `executeTools()` — built-in tool execution/batching seam. - `coderd/x/chatd/chattool/` — built-in tool implementations. - `site/src/pages/AgentsPage/components/ChatElements/tools/Tool.tsx` — tool renderer dispatch. - `site/src/pages/AgentsPage/components/ChatConversation/messageParsing.ts` and `ConversationTimeline.tsx` — tool/result merge and rendering flow. </details> ### Workstream map and ownership \| Workstream \| Primary owner \| Main files \| Can run in parallel? \| Done when \| \|---\|---\|---\|---\|---\| \| 0. Integration + gating \| Integration lead \| `coderd/x/chatd/chatd.go` \| No; central merge lane \| Tool registration, prompt sync, and model selection are wired together \| \| 1. Advisor runtime + tool \| Backend agent \| new `coderd/x/chatd/chatadvisor/`, new `coderd/x/chatd/chattool/advisor.go` \| Yes \| Tool can perform a tool-less advisor call in memory and return structured results \| \| 2. Planning-only execution policy \| Chatloop agent \| `coderd/x/chatd/chatloop/chatloop.go`, related tests \| Yes \| Mixed `advisor` + action-tool batches are rejected cleanly and deterministically \| \| 3. Metrics/usage/config \| Backend/telemetry agent \| `chatd.go`, `chatloop/metrics.go`, optional config plumbing \| Partially; coordinate with integration lead \| Advisor usage is separately visible in metadata/metrics and limits are enforced \| \| 4. Frontend rendering \| Frontend agent \| `site/.../tools/Tool.tsx`, new `AdvisorTool.tsx`, stories \| Yes after result schema stabilizes \| Advisor renders as a readable card and story tests pass \| \| 5. Dogfood + QA evidence \| QA agent \| dev server, Storybook, dogfood output \| After backend + UI are usable \| Repro videos, screenshots, and a concise QA report exist \| ### Parallelization rules - Do not split `coderd/x/chatd/chatd.go` across multiple execution agents without an integration lead. That file owns prompt building, tool registration, model resolution, and cost persistence. - Workstreams 1 and 2 can be developed in parallel and then stacked onto the integration branch. - Workstream 4 should begin once the backend result schema is agreed on, even if the backend is still behind a feature flag. - Any agent that needs to re-check Mux behavior should clone `coder/mux` into a temporary directory (for example, `$(mktemp -d)/mux`) and inspect it read-only; do not vendor or copy code from Mux directly. ## Phase 0 — Preflight and guardrails ### Goals - Align the team on the smallest shippable architecture. - Prevent scope creep into MCP/dynamic-tool/sub-agent variants. - Decide upfront what is MVP vs. follow-up. ### Tasks 1. Confirm the MVP boundary. - Ship a built-in advisor tool first. - Do not make MCP, dynamic tools, or sub-agents the primary implementation. - Do not add transient streaming phases in the first backend PR unless they fall out almost for free. 2. Confirm local workflow hygiene before coding. - Ensure the repo is using the project git hooks from `scripts/githooks`. - Do not bypass hooks with `--no-verify`. - Use `./scripts/develop.sh` for the full dev server rather than manual build/run commands. 3. Lock the model-selection policy. - Recommended MVP: advisor uses the same resolved provider/model/cost config as the current chat, with advisor-specific max-output and usage caps. - Follow-up only if required: add a separate `AdvisorModelConfigID`-style override that resolves through the existing `configCache`/model-config path. Do not invent a new free-form `provider:model` parser if chatd already stores provider/model separately. 4. Lock the persistence policy. - Recommended MVP: no DB migration. Persist advisor-visible metadata in the tool result and record separate metrics in memory/Prometheus. - Only if product/billing explicitly asks for queryable advisor cost: add a later DB migration or usage table, following the normal `queries/.sql` + `make gen` workflow. 5. Create an execution ADR note in the work item or tracking doc.* - Capture: built-in tool, tool-less nested call, root-chat-only rollout, exclusive execution policy, MVP no-DB-migration default. ### Quality gate - Everyone on the team can state the same answers to these questions: - Is advisor a built-in tool? Yes. - Can advisor run with action tools in the same batch? No. - Does advisor get tools of its own? No. - Is a DB migration required for MVP? No, unless billing insists. ## Phase 1 — Build the advisor runtime and tool wrapper ### Goals Create the core advisor implementation in a way that is easy to test and keeps `chattool/` thin. ### Files to add - `coderd/x/chatd/chatadvisor/types.go` - `coderd/x/chatd/chatadvisor/guidance.go` - `coderd/x/chatd/chatadvisor/handoff.go` - `coderd/x/chatd/chatadvisor/runtime.go` - `coderd/x/chatd/chatadvisor/runner.go` - `coderd/x/chatd/chattool/advisor.go` ### Responsibilities by file 1. `types.go` - Define the input/result schema used by the tool and UI. - Keep the result shape close to Mux so the UI and model both have predictable cases. - Recommended result variants: - `advice` - `limit_reached` - `error` Recommended shape: ```go type AdvisorArgs struct { Question string `json:"question"` } type AdvisorResult struct { Type string `json:"type"` Advice string `json:"advice,omitempty"` Error string `json:"error,omitempty"` AdvisorModel string `json:"advisor_model,omitempty"` RemainingUses int `json:"remaining_uses,omitempty"` Usage AdvisorUsageResult `json:"usage,omitempty"` } ``` 2. `guidance.go`* - Hold two strings: - the nested advisor system prompt; - the parent-agent guidance block to inject into the outer system prompt. - The nested advisor prompt must say, in plain language: - you are advising the parent agent; - you do not address the end user directly; - you do not claim actions happened; - you return concise strategic guidance and tradeoffs. 3. `runtime.go` - Define the per-run runtime state. - Recommended fields: - resolved model + model config; - provider keys/options reused from the outer chat; - `MaxUsesPerRun`; - `MaxOutputTokens`; - atomic/current call counter; - callback(s) to obtain the current prompt snapshot and current-step snapshot; - optional metrics/usage hook. - Add fail-fast validation for impossible config: nil model, non-positive limits, empty prompt builders, etc. 4. `handoff.go` - Build the advisor handoff message from: - the explicit question; - the exact prompt/messages the parent model just used; - the current step's text/reasoning snapshot, if available; - the most recent relevant tool outputs, if they are already in the prompt snapshot. - Important: use the already-prepared outer prompt tail, not a fresh DB reload. That keeps the advisor aligned with compaction and the exact context the outer model saw. - Apply hard truncation budgets with recent-context bias. 5. `runner.go` - Execute the nested advisor call. - Recommended implementation: call `chatloop.Run()` in an in-memory, one-step mode: - `Tools: nil` - `ProviderTools: nil` - `MaxSteps: 1` - `PersistStep`: capture the assistant output in memory instead of writing DB rows - Reuse the existing provider/model/cost path instead of building a second provider runner. - Assert that no tool definitions are passed to the nested call. 6. `chattool/advisor.go` - Keep this file thin and consistent with other built-ins. - Responsibilities: - decode `AdvisorArgs`; - validate `Question` is non-empty and bounded; - call the `chatadvisor` runner; - return a structured tool response. ### Defensive programming requirements - Assert `Question` is non-empty after trimming. - Assert runtime limits are positive. - Assert the nested advisor call runs with zero tools/provider tools. - Assert `AdvisorResult.Type` is one of the known variants before returning. - Assert remaining uses never goes negative. ### Acceptance criteria - A unit test can call the advisor tool with a fake model and receive a stable `advice` result. - The nested advisor call is impossible to run with tools accidentally attached. - The core logic lives in `chatadvisor/`, not embedded inside `chatd.go`. ## Phase 2 — Wire advisor into chatd and keep prompt/tool availability in sync ### Goals Register the tool in the right place, expose it only when eligible, and inject system guidance only when the tool is present. ### Files to modify - `coderd/x/chatd/chatd.go` - optionally a small helper file if `chatd.go` becomes too crowded ### Tasks 1. Compute one eligibility boolean in `processChat()`. Recommended inputs: - server-level advisor enabled flag; - root chat only (`chat.ParentChatID == uuid.Nil` or equivalent existing root/child check); - a usable resolved model/provider exists; - optional experiment/workspace/org gate if product wants staged rollout. 2. Create the runtime once per outer chat run. - Use the model/config/keys resolved by `resolveChatModel()`. - Reuse provider options from the current chat's `ChatModelCallConfig`. - Set `MaxUsesPerRun` and `MaxOutputTokens` from advisor config defaults. 3. Register the tool in the built-in tool block. - Insert after the skill tools and before MCP tools in `processChat()`. - Record `builtinToolNames["advisor"] = true` so metrics stay bounded. 4. Inject advisor guidance into the outer system prompt using the same boolean. - Use `chatprompt.InsertSystem()` in the same prompt assembly path that already injects user/system instructions. - Place the block near the existing instruction insertion, before plan-path/skill context blocks. - Wrap the guidance in an explicit tag like `<advisor-guidance>` so it is easy to spot in tests and future refactors. 5. Keep advisor out of child chats for the first release. - That avoids recursion/cost blowups with `spawn_agent` / `wait_agent` flows. - Document this explicitly in the rollout notes and tests. ### Acceptance criteria - If advisor is disabled, neither the tool nor the prompt guidance appears. - If advisor is enabled, both the tool and the prompt guidance appear. - Root chats can use advisor; child chats cannot. - Built-in tool names include `advisor` so metrics do not collapse it into the generic `mcp` label. ## Phase 3 — Enforce planning-only execution policy in `chatloop` ### Goals Prevent the model from calling `advisor` and action tools in the same execution batch. ### Files to modify - `coderd/x/chatd/chatloop/chatloop.go` - related chatloop tests ### Recommended implementation Keep the MVP small; do not build a general policy engine yet. 1. Add a minimal field to `chatloop.RunOptions`, for example: ```go ExclusiveToolName string ``` 2. In `Run()` / `executeTools()`, detect the case where the exclusive tool appears in the same local-tool batch as any other locally executed tool. 3. When that happens, synthesize structured tool-result errors for the affected calls instead of executing anything in the batch. - `advisor` should receive a clear error like: _advisor must be called by itself before action tools_. - The sibling action tools should receive a paired policy error like: _this tool was skipped because advisor must run alone_. 4. Let the outer model see those tool errors and retry cleanly. - This is simpler and safer than partial execution or hidden deferral. - It preserves deterministic transcript history for debugging. 5. Pass the just-finished step snapshot into the tool execution context. - The advisor runtime should be able to see the current step's text/reasoning content, because that is often the best hint about what the outer model is trying to decide. ### Why this is the right fit - It matches the intended semantics: advisor is consulted before* taking action. - It avoids subtle race conditions caused by concurrent built-in tool execution. - It keeps the behavior easy to test with fake models. ### Acceptance criteria - A model-emitted batch containing only `advisor` succeeds. - A model-emitted batch containing `advisor` plus any other locally executed tool returns deterministic policy errors and executes nothing. - Non-advisor tool execution stays unchanged for normal chats. ## Phase 4 — Usage limits, metrics, and configuration ### Goals Make advisor safe to operate without over-designing billing/storage in the first release. ### Files to modify - `coderd/x/chatd/chatd.go` - `coderd/x/chatd/chatloop/metrics.go` as needed - `coderd/x/chatd/chatd.go` `Config` struct and constructor path - optional follow-up config/db files only if a separate advisor model or persistent billing is required ### Tasks 1. Add explicit server config knobs for MVP. Recommended fields on `chatd.Config` or a nested advisor config struct: - `AdvisorEnabled bool` - `AdvisorMaxUsesPerRun int` - `AdvisorMaxOutputTokens int64` 2. Track usage per outer run. - Reset the counter for each `processChat()` invocation. - Return `remaining_uses` in the tool result. - Return `limit_reached` when the cap is exhausted. 3. Expose advisor usage metadata in the tool result. - Include model name and token/cost summary if available. - Use the same `callConfig.Cost` calculation path as the outer chat for MVP if advisor reuses the same model. 4. Record server-side metrics. - Count advisor invocations, failures, and latency. - Ensure they show up under the built-in tool label `advisor`. 5. Optional decision gate: separate advisor model. - If product insists on a stronger/different advisor model, add a follow-up config hook that resolves another existing chat model config through the same `configCache` path. - Keep that out of the first landing PR unless it is required for acceptance. 6. Optional decision gate: queryable advisor cost. - If this becomes required, spin a follow-up DB task: - update `coderd/database/queries/.sql`; - add migration files; - run `make gen`; - update audit mappings if a new auditable type/field is introduced. ### Acceptance criteria - Advisor calls are capped per outer run. - Limit exhaustion is user-visible in the tool result. - Metrics distinguish advisor calls from other built-in tools. - MVP does not require a schema migration unless explicitly approved. ## Phase 5 — Frontend rendering and Storybook coverage ### Goals Make advisor feel intentional in the Agents UI without blocking the backend on fancy streaming UI. ### Files to modify - `site/src/pages/AgentsPage/components/ChatElements/tools/Tool.tsx` - new `site/src/pages/AgentsPage/components/ChatElements/tools/AdvisorTool.tsx` - Storybook story file(s) in the same tools directory ### Delivery strategy 1. Intermediate milestone during backend bring-up:* rely on the existing generic tool renderer if needed. - This is acceptable only as a short-lived integration checkpoint. 2. Release milestone: add a dedicated lightweight `AdvisorTool` renderer. - Reuse existing primitives: - `ToolCollapsible` - `ToolIcon` - `Response` for markdown/prose rendering - `ScrollArea` if the advice can be long - Keep styling light and consistent with the Agents page. - Do not add unnecessary React memoization in `site/src/pages/AgentsPage/`; that area is already React-Compiler aware. 3. Render the structured result states cleanly. - `advice` — readable prose/markdown with optional metadata footer. - `limit_reached` — warning-style message. - `error` — error state with visible fallback text. - `running` — existing tool loading state/spinner is enough for MVP. 4. Add Storybook coverage instead of ad-hoc component tests. Recommended stories: - successful advice; - running/loading; - limit reached; - error. 5. Keep the UI contract narrow. - Prefer one text field like `advice` plus small metadata rather than a deeply nested schema. - That keeps the UI resilient to prompt iteration. ### Acceptance criteria - The advisor tool card renders readable content rather than raw quoted JSON in the final release branch. - Running, limit, and error states are visibly distinct. - Storybook stories and play assertions cover the new states. - Existing tool rendering flows remain unchanged. ## Phase 6 — Automated tests and validation gates ### Backend tests to add 1. Advisor runtime/tool tests - question validation; - tool-less nested execution assertion; - success result shaping; - limit-reached result shaping; - error result shaping. 2. Prompt/gating tests in chatd - advisor disabled ⇒ no tool, no guidance; - advisor enabled/root chat ⇒ tool + guidance; - child chat ⇒ advisor absent. 3. Chatloop policy tests - advisor alone runs; - advisor + action tool mixed batch returns deterministic policy errors; - non-advisor tools still execute normally. 4. Usage/metrics tests - per-run cap resets correctly; - builtin tool labeling includes `advisor`; - returned metadata includes model/usage summary when available. ### Frontend tests to add - Storybook `play()` assertions for the advisor renderer states. - Verify expand/collapse behavior and visible fallback text. - Verify the message timeline still renders adjacent tools correctly. ### Recommended command sequence Run these as the implementation matures, not only at the end: 1. Backend-focused gate after phases 1–4: - `make test RUN=TestAdvisor` - `make test RUN=TestChatloopAdvisor` - `make lint` 2. Frontend-focused gate after phase 5: - `pnpm test:storybook src/pages/AgentsPage/components/ChatElements/tools/AdvisorTool.stories.tsx` - `pnpm lint` - `pnpm format` 3. Final repo gate before handoff: - `make pre-commit` - run any additional targeted `make test RUN=...` selections covering touched chatd paths > Use the exact new test names the implementing agents create; the names above are recommended anchors, not existing tests. ## Dogfooding plan ### Principle Dogfood the change as a real agent feature, not just a unit-tested backend. Per the dogfood and `agent-browser` skills, the reviewer should get watchable repro videos plus screenshots that make the behavior obvious without reading logs. ### Required setup 1. Start the full dev environment with: - `./scripts/develop.sh` 2. If the frontend renderer changes, also start Storybook from `site/` with: - `pnpm storybook --no-open` 3. Use `agent-browser` directly — never `npx agent-browser`. 4. Use named browser sessions and an output folder such as: - `./dogfood-output/advisor/` - with subfolders `screenshots/` and `videos/` ### Evidence protocol For every interactive scenario below: 1. Start video recording before the action. 2. Capture step-by-step screenshots at human pace. 3. Capture one annotated screenshot of the final state. 4. Stop the recording. 5. Note the exact pass/fail observation in the QA report. For static UI states (for example Storybook error/limit cards), an annotated screenshot is sufficient; video is optional but still encouraged by this project’s review preference. ### Dogfood scenarios #### Scenario A — Happy path in the real Agents UI Goal: prove that a root agent chat can invoke advisor and produce a readable recommendation before taking further action. Steps: 1. Open the Agents page with an advisor-enabled root chat. 2. Start a repro video. 3. Send a prompt that should reasonably trigger strategic planning, such as an architecture or multi-tradeoff question. 4. Capture screenshots of: - the prompt before send; - the running advisor state; - the completed advisor card and the assistant’s follow-up response. 5. Stop recording. Pass criteria: - advisor appears in the timeline; - the rendered result is readable; - the assistant can continue after consuming the advisor output. #### Scenario B — Advisor unavailable path Goal: prove the feature is truly gated. Suggested variants (at least one is required, both are better): - feature flag/config off; - child/sub-agent chat. Evidence: - annotated screenshot of the chat/tool state showing advisor is absent; - short video if toggling the gate live is part of the repro. Pass criteria: - no advisor tool is available; - no advisor-specific prompt behavior leaks through. #### Scenario C — UI states in Storybook Goal: prove the renderer handles non-happy states cleanly. Required story states: - success/advice; - running; - limit reached; - error. Evidence: - one screenshot per state; - at least one short video showing collapse/expand behavior. Pass criteria: - success renders readable advice; - limit/error have visible fallback text; - the component behaves like the other tool cards. #### Scenario D — Regression sweep of nearby tools Goal: ensure advisor does not break the surrounding chat timeline. Check at minimum: - another existing built-in tool still renders correctly near advisor; - sub-agent/tool cards still expand/collapse normally; - no obvious console errors appear in the Agents page during the advisor flow. Evidence: - screenshots of adjacent tool cards; - console/error capture if anything suspicious appears. ### `agent-browser` usage notes for the QA agent - Prefer `agent-browser batch` for 2+ sequential commands when no intermediate parsing is needed. - Use `snapshot -i` to discover interactive refs. - Re-snapshot after navigation or major DOM changes. - Avoid `wait --load networkidle` unless the page is known to go idle; prefer explicit element/text waits or short fixed waits. - Record videos at human pace and include pauses that a reviewer can follow. ## Rollout plan ### Initial rollout - Gate behind a server-side advisor-enabled flag. - Enable only for selected internal/root agent chats first. - Watch metrics for: - invocation count; - failure rate; - latency; - obvious retry loops. ### Expansion conditions Expand beyond the initial rollout only after the following are true: - mixed-batch policy behavior is stable; - cost impact is understood; - frontend UX is readable in production-like dogfood; - no recursion surprises have appeared with sub-agent flows. ### Explicit non-goals for the first release - advisor inside child/sub-agent chats; - provider-agnostic streaming phase UI; - MCP-based external advisor implementation; - mandatory DB-backed advisor cost reporting. ## Final acceptance checklist - [ ] `advisor` is a built-in chatd tool, not an MCP/dynamic-tool substitute. - [ ] The nested advisor call is tool-less and bounded to one in-memory step. - [ ] One eligibility boolean controls both tool registration and prompt guidance injection. - [ ] Root chats can use advisor; child chats cannot in the initial rollout. - [ ] Mixed advisor/action batches produce deterministic policy errors instead of partial execution. - [ ] Per-run usage caps and limit-reached behavior work. - [ ] Advisor usage is visible in metadata/metrics without forcing a DB migration for MVP. - [ ] The Agents UI has a readable advisor card and Storybook coverage. - [ ] Dogfooding produced screenshots and repro videos for the required scenarios. - [ ] Validation commands (`make lint`, targeted `make test`, Storybook tests, `make pre-commit`) passed before handoff. ## Suggested PR split 1. PR 1 — Backend foundation - `chatadvisor/` package - `chattool/advisor.go` - `chatloop` exclusive policy - chatd gating/prompt sync - backend tests 2. PR 2 — Frontend + QA - advisor renderer - stories/play assertions - dogfood artifacts and QA notes 3. PR 3 — Optional follow-ups only if demanded by stakeholders - separate advisor model override - persistent advisor billing/queryability - transient phase-stream UX </details> --- _Generated with [`mux`](https://github.com/coder/mux) • Model: `anthropic:claude-opus-4-7` • Thinking: `max`_	2026-04-30 14:53:08 +02:00
Thomas Kosiewski	eaf2609bb8	feat(coderd/x/chatd/chatadvisor): add advisor runtime and tool wrapper (#24620 ) ## Summary Introduce the `coderd/x/chatd/chatadvisor` package: the self-contained runtime that performs a nested, tool-less, single-step model call to return strategic guidance to the parent agent. Also ships the thin `AgentTool` wrapper that the agent exposes as the `advisor` built-in. ## Motivation The advisor is a "consult before you act" planning step. Keeping its runtime in its own package (rather than inside `chatd.go` or `chattool/`) makes the behavior easy to unit-test, keeps `chattool/` thin, and avoids import cycles with `chatprompt`. ## Changes - `chatadvisor/types.go`: `AdvisorArgs` and `AdvisorResult` (`advice` / `limit_reached` / `error` variants) plus usage metadata. Stable JSON shape for the UI. - `chatadvisor/guidance.go`: the nested advisor system prompt and `ParentGuidanceBlock` injected into the outer agent (tagged `<advisor-guidance>`). The nested prompt explicitly tells the advisor it is advising the parent agent, must not address the user, and must not claim actions happened. - `chatadvisor/handoff.go`: builds the advisor input from the already-prepared outer prompt tail (not a fresh DB reload) with hard truncation budgets and a recent-context bias so the advisor sees the exact context the outer model saw. - `chatadvisor/runner.go`: wraps `chatloop.Run()` in strict one-step mode (`Tools: nil`, `ProviderTools: nil`, `MaxSteps: 1`) so the nested call is structurally incapable of calling tools. - `chatadvisor/tool.go`: the `AgentTool` wrapper. Validates the question (non-empty, bounded to 2000 runes), invokes the runtime, and maps the result to a JSON tool response. - Defensive assertions throughout (question non-empty after trim, positive limits, zero tools on nested call, known result variants, non-negative remaining uses). - Unit tests cover question validation, tool-less execution, and each result variant. ## Stack context This is PR 2 of 6 in the advisor feature stack. It depends on PR 1's `ExclusiveToolNames` hook; no consumer of this package exists yet (that lands in PR 4). ## Scope / non-goals - No wiring into `chatd`. - No HTTP/API surface. - No UI. - No separate provider; the runtime reuses the outer chat's resolved model/provider/keys. ## Validation - `go test ./coderd/x/chatd/chatadvisor/...` - `make lint` --- <details> <summary>📋 Implementation Plan (shared across the advisor stack)</summary> # Plan: Add a Mux-style advisor tool to coder agents/chatd ## Outcome Add a first-class `advisor` tool to agent chats in `coderd/x/chatd` that feels native to Coder: - it is a built-in server-side tool, not an MCP/dynamic-tool workaround; - it performs a nested tool-less model call for strategic advice; - it is exposed only when eligible, and the prompt mentions it only when it is actually available; - it is treated as a planning-only tool so it does not run alongside action tools in the same batch; - it tracks usage/cost separately enough for operators to reason about it; - it has a minimally polished UI in the Agents page; - and it ships with explicit dogfooding evidence, including screenshots and repro videos. ## Design decisions to lock before coding 1. Primary architecture: native built-in tool in `chattool/`, backed by a small `chatadvisor` package. 2. Nested model execution: reuse chatd's existing model/provider stack for a one-step, tool-less advisor call rather than inventing a new provider pathway. 3. Execution policy: treat `advisor` as an exclusive/planning-only tool; mixed batches must return structured policy errors and force the model to retry cleanly. 4. Availability: initial rollout is for root agent chats only; disable for child/sub-agent chats until recursion/cost policy is proven. 5. Prompt sync: use one eligibility boolean to drive both tool registration and advisor guidance injection. 6. Persistence/cost split: MVP should keep advisor usage visible in result metadata and server metrics; only add DB schema if product/billing explicitly needs queryable advisor-specific cost. 7. UI scope: generic tool rendering is an acceptable temporary milestone during backend bring-up, but the release candidate should include a dedicated lightweight advisor renderer. ## Delivery model The work should be executed as coordinated workstreams with one integration owner and parallel contributors for low-conflict areas. The integration owner should own `coderd/x/chatd/chatd.go` because prompt assembly, tool registration, and model resolution all converge there. ## Detailed workstreams ### Repo evidence used for this plan <details> <summary>Mux reference and current chatd seams</summary> Mux reference implementation - `src/node/services/tools/advisor.ts` — native advisor tool implementation. - `src/common/constants/advisor.ts` — advisor prompt/constants and truncation policy. - `src/common/utils/tools/tools.ts` — conditional tool registration. - `src/node/services/streamContextBuilder.ts` — injects advisor guidance only when the tool is available. Current chatd seams - `coderd/x/chatd/chatd.go` - `processChat()` — tool assembly, prompt assembly, and chatloop invocation. - `resolveChatModel()` — current model/provider/key resolution seam. - `type Config struct` — server-level chatd configuration surface. - `coderd/x/chatd/chatloop/chatloop.go` - `Run()` — main streaming/model loop. - `executeTools()` — built-in tool execution/batching seam. - `coderd/x/chatd/chattool/` — built-in tool implementations. - `site/src/pages/AgentsPage/components/ChatElements/tools/Tool.tsx` — tool renderer dispatch. - `site/src/pages/AgentsPage/components/ChatConversation/messageParsing.ts` and `ConversationTimeline.tsx` — tool/result merge and rendering flow. </details> ### Workstream map and ownership \| Workstream \| Primary owner \| Main files \| Can run in parallel? \| Done when \| \|---\|---\|---\|---\|---\| \| 0. Integration + gating \| Integration lead \| `coderd/x/chatd/chatd.go` \| No; central merge lane \| Tool registration, prompt sync, and model selection are wired together \| \| 1. Advisor runtime + tool \| Backend agent \| new `coderd/x/chatd/chatadvisor/`, new `coderd/x/chatd/chattool/advisor.go` \| Yes \| Tool can perform a tool-less advisor call in memory and return structured results \| \| 2. Planning-only execution policy \| Chatloop agent \| `coderd/x/chatd/chatloop/chatloop.go`, related tests \| Yes \| Mixed `advisor` + action-tool batches are rejected cleanly and deterministically \| \| 3. Metrics/usage/config \| Backend/telemetry agent \| `chatd.go`, `chatloop/metrics.go`, optional config plumbing \| Partially; coordinate with integration lead \| Advisor usage is separately visible in metadata/metrics and limits are enforced \| \| 4. Frontend rendering \| Frontend agent \| `site/.../tools/Tool.tsx`, new `AdvisorTool.tsx`, stories \| Yes after result schema stabilizes \| Advisor renders as a readable card and story tests pass \| \| 5. Dogfood + QA evidence \| QA agent \| dev server, Storybook, dogfood output \| After backend + UI are usable \| Repro videos, screenshots, and a concise QA report exist \| ### Parallelization rules - Do not split `coderd/x/chatd/chatd.go` across multiple execution agents without an integration lead. That file owns prompt building, tool registration, model resolution, and cost persistence. - Workstreams 1 and 2 can be developed in parallel and then stacked onto the integration branch. - Workstream 4 should begin once the backend result schema is agreed on, even if the backend is still behind a feature flag. - Any agent that needs to re-check Mux behavior should clone `coder/mux` into a temporary directory (for example, `$(mktemp -d)/mux`) and inspect it read-only; do not vendor or copy code from Mux directly. ## Phase 0 — Preflight and guardrails ### Goals - Align the team on the smallest shippable architecture. - Prevent scope creep into MCP/dynamic-tool/sub-agent variants. - Decide upfront what is MVP vs. follow-up. ### Tasks 1. Confirm the MVP boundary. - Ship a built-in advisor tool first. - Do not make MCP, dynamic tools, or sub-agents the primary implementation. - Do not add transient streaming phases in the first backend PR unless they fall out almost for free. 2. Confirm local workflow hygiene before coding. - Ensure the repo is using the project git hooks from `scripts/githooks`. - Do not bypass hooks with `--no-verify`. - Use `./scripts/develop.sh` for the full dev server rather than manual build/run commands. 3. Lock the model-selection policy. - Recommended MVP: advisor uses the same resolved provider/model/cost config as the current chat, with advisor-specific max-output and usage caps. - Follow-up only if required: add a separate `AdvisorModelConfigID`-style override that resolves through the existing `configCache`/model-config path. Do not invent a new free-form `provider:model` parser if chatd already stores provider/model separately. 4. Lock the persistence policy. - Recommended MVP: no DB migration. Persist advisor-visible metadata in the tool result and record separate metrics in memory/Prometheus. - Only if product/billing explicitly asks for queryable advisor cost: add a later DB migration or usage table, following the normal `queries/.sql` + `make gen` workflow. 5. Create an execution ADR note in the work item or tracking doc.* - Capture: built-in tool, tool-less nested call, root-chat-only rollout, exclusive execution policy, MVP no-DB-migration default. ### Quality gate - Everyone on the team can state the same answers to these questions: - Is advisor a built-in tool? Yes. - Can advisor run with action tools in the same batch? No. - Does advisor get tools of its own? No. - Is a DB migration required for MVP? No, unless billing insists. ## Phase 1 — Build the advisor runtime and tool wrapper ### Goals Create the core advisor implementation in a way that is easy to test and keeps `chattool/` thin. ### Files to add - `coderd/x/chatd/chatadvisor/types.go` - `coderd/x/chatd/chatadvisor/guidance.go` - `coderd/x/chatd/chatadvisor/handoff.go` - `coderd/x/chatd/chatadvisor/runtime.go` - `coderd/x/chatd/chatadvisor/runner.go` - `coderd/x/chatd/chattool/advisor.go` ### Responsibilities by file 1. `types.go` - Define the input/result schema used by the tool and UI. - Keep the result shape close to Mux so the UI and model both have predictable cases. - Recommended result variants: - `advice` - `limit_reached` - `error` Recommended shape: ```go type AdvisorArgs struct { Question string `json:"question"` } type AdvisorResult struct { Type string `json:"type"` Advice string `json:"advice,omitempty"` Error string `json:"error,omitempty"` AdvisorModel string `json:"advisor_model,omitempty"` RemainingUses int `json:"remaining_uses,omitempty"` Usage AdvisorUsageResult `json:"usage,omitempty"` } ``` 2. `guidance.go`* - Hold two strings: - the nested advisor system prompt; - the parent-agent guidance block to inject into the outer system prompt. - The nested advisor prompt must say, in plain language: - you are advising the parent agent; - you do not address the end user directly; - you do not claim actions happened; - you return concise strategic guidance and tradeoffs. 3. `runtime.go` - Define the per-run runtime state. - Recommended fields: - resolved model + model config; - provider keys/options reused from the outer chat; - `MaxUsesPerRun`; - `MaxOutputTokens`; - atomic/current call counter; - callback(s) to obtain the current prompt snapshot and current-step snapshot; - optional metrics/usage hook. - Add fail-fast validation for impossible config: nil model, non-positive limits, empty prompt builders, etc. 4. `handoff.go` - Build the advisor handoff message from: - the explicit question; - the exact prompt/messages the parent model just used; - the current step's text/reasoning snapshot, if available; - the most recent relevant tool outputs, if they are already in the prompt snapshot. - Important: use the already-prepared outer prompt tail, not a fresh DB reload. That keeps the advisor aligned with compaction and the exact context the outer model saw. - Apply hard truncation budgets with recent-context bias. 5. `runner.go` - Execute the nested advisor call. - Recommended implementation: call `chatloop.Run()` in an in-memory, one-step mode: - `Tools: nil` - `ProviderTools: nil` - `MaxSteps: 1` - `PersistStep`: capture the assistant output in memory instead of writing DB rows - Reuse the existing provider/model/cost path instead of building a second provider runner. - Assert that no tool definitions are passed to the nested call. 6. `chattool/advisor.go` - Keep this file thin and consistent with other built-ins. - Responsibilities: - decode `AdvisorArgs`; - validate `Question` is non-empty and bounded; - call the `chatadvisor` runner; - return a structured tool response. ### Defensive programming requirements - Assert `Question` is non-empty after trimming. - Assert runtime limits are positive. - Assert the nested advisor call runs with zero tools/provider tools. - Assert `AdvisorResult.Type` is one of the known variants before returning. - Assert remaining uses never goes negative. ### Acceptance criteria - A unit test can call the advisor tool with a fake model and receive a stable `advice` result. - The nested advisor call is impossible to run with tools accidentally attached. - The core logic lives in `chatadvisor/`, not embedded inside `chatd.go`. ## Phase 2 — Wire advisor into chatd and keep prompt/tool availability in sync ### Goals Register the tool in the right place, expose it only when eligible, and inject system guidance only when the tool is present. ### Files to modify - `coderd/x/chatd/chatd.go` - optionally a small helper file if `chatd.go` becomes too crowded ### Tasks 1. Compute one eligibility boolean in `processChat()`. Recommended inputs: - server-level advisor enabled flag; - root chat only (`chat.ParentChatID == uuid.Nil` or equivalent existing root/child check); - a usable resolved model/provider exists; - optional experiment/workspace/org gate if product wants staged rollout. 2. Create the runtime once per outer chat run. - Use the model/config/keys resolved by `resolveChatModel()`. - Reuse provider options from the current chat's `ChatModelCallConfig`. - Set `MaxUsesPerRun` and `MaxOutputTokens` from advisor config defaults. 3. Register the tool in the built-in tool block. - Insert after the skill tools and before MCP tools in `processChat()`. - Record `builtinToolNames["advisor"] = true` so metrics stay bounded. 4. Inject advisor guidance into the outer system prompt using the same boolean. - Use `chatprompt.InsertSystem()` in the same prompt assembly path that already injects user/system instructions. - Place the block near the existing instruction insertion, before plan-path/skill context blocks. - Wrap the guidance in an explicit tag like `<advisor-guidance>` so it is easy to spot in tests and future refactors. 5. Keep advisor out of child chats for the first release. - That avoids recursion/cost blowups with `spawn_agent` / `wait_agent` flows. - Document this explicitly in the rollout notes and tests. ### Acceptance criteria - If advisor is disabled, neither the tool nor the prompt guidance appears. - If advisor is enabled, both the tool and the prompt guidance appear. - Root chats can use advisor; child chats cannot. - Built-in tool names include `advisor` so metrics do not collapse it into the generic `mcp` label. ## Phase 3 — Enforce planning-only execution policy in `chatloop` ### Goals Prevent the model from calling `advisor` and action tools in the same execution batch. ### Files to modify - `coderd/x/chatd/chatloop/chatloop.go` - related chatloop tests ### Recommended implementation Keep the MVP small; do not build a general policy engine yet. 1. Add a minimal field to `chatloop.RunOptions`, for example: ```go ExclusiveToolName string ``` 2. In `Run()` / `executeTools()`, detect the case where the exclusive tool appears in the same local-tool batch as any other locally executed tool. 3. When that happens, synthesize structured tool-result errors for the affected calls instead of executing anything in the batch. - `advisor` should receive a clear error like: _advisor must be called by itself before action tools_. - The sibling action tools should receive a paired policy error like: _this tool was skipped because advisor must run alone_. 4. Let the outer model see those tool errors and retry cleanly. - This is simpler and safer than partial execution or hidden deferral. - It preserves deterministic transcript history for debugging. 5. Pass the just-finished step snapshot into the tool execution context. - The advisor runtime should be able to see the current step's text/reasoning content, because that is often the best hint about what the outer model is trying to decide. ### Why this is the right fit - It matches the intended semantics: advisor is consulted before* taking action. - It avoids subtle race conditions caused by concurrent built-in tool execution. - It keeps the behavior easy to test with fake models. ### Acceptance criteria - A model-emitted batch containing only `advisor` succeeds. - A model-emitted batch containing `advisor` plus any other locally executed tool returns deterministic policy errors and executes nothing. - Non-advisor tool execution stays unchanged for normal chats. ## Phase 4 — Usage limits, metrics, and configuration ### Goals Make advisor safe to operate without over-designing billing/storage in the first release. ### Files to modify - `coderd/x/chatd/chatd.go` - `coderd/x/chatd/chatloop/metrics.go` as needed - `coderd/x/chatd/chatd.go` `Config` struct and constructor path - optional follow-up config/db files only if a separate advisor model or persistent billing is required ### Tasks 1. Add explicit server config knobs for MVP. Recommended fields on `chatd.Config` or a nested advisor config struct: - `AdvisorEnabled bool` - `AdvisorMaxUsesPerRun int` - `AdvisorMaxOutputTokens int64` 2. Track usage per outer run. - Reset the counter for each `processChat()` invocation. - Return `remaining_uses` in the tool result. - Return `limit_reached` when the cap is exhausted. 3. Expose advisor usage metadata in the tool result. - Include model name and token/cost summary if available. - Use the same `callConfig.Cost` calculation path as the outer chat for MVP if advisor reuses the same model. 4. Record server-side metrics. - Count advisor invocations, failures, and latency. - Ensure they show up under the built-in tool label `advisor`. 5. Optional decision gate: separate advisor model. - If product insists on a stronger/different advisor model, add a follow-up config hook that resolves another existing chat model config through the same `configCache` path. - Keep that out of the first landing PR unless it is required for acceptance. 6. Optional decision gate: queryable advisor cost. - If this becomes required, spin a follow-up DB task: - update `coderd/database/queries/.sql`; - add migration files; - run `make gen`; - update audit mappings if a new auditable type/field is introduced. ### Acceptance criteria - Advisor calls are capped per outer run. - Limit exhaustion is user-visible in the tool result. - Metrics distinguish advisor calls from other built-in tools. - MVP does not require a schema migration unless explicitly approved. ## Phase 5 — Frontend rendering and Storybook coverage ### Goals Make advisor feel intentional in the Agents UI without blocking the backend on fancy streaming UI. ### Files to modify - `site/src/pages/AgentsPage/components/ChatElements/tools/Tool.tsx` - new `site/src/pages/AgentsPage/components/ChatElements/tools/AdvisorTool.tsx` - Storybook story file(s) in the same tools directory ### Delivery strategy 1. Intermediate milestone during backend bring-up:* rely on the existing generic tool renderer if needed. - This is acceptable only as a short-lived integration checkpoint. 2. Release milestone: add a dedicated lightweight `AdvisorTool` renderer. - Reuse existing primitives: - `ToolCollapsible` - `ToolIcon` - `Response` for markdown/prose rendering - `ScrollArea` if the advice can be long - Keep styling light and consistent with the Agents page. - Do not add unnecessary React memoization in `site/src/pages/AgentsPage/`; that area is already React-Compiler aware. 3. Render the structured result states cleanly. - `advice` — readable prose/markdown with optional metadata footer. - `limit_reached` — warning-style message. - `error` — error state with visible fallback text. - `running` — existing tool loading state/spinner is enough for MVP. 4. Add Storybook coverage instead of ad-hoc component tests. Recommended stories: - successful advice; - running/loading; - limit reached; - error. 5. Keep the UI contract narrow. - Prefer one text field like `advice` plus small metadata rather than a deeply nested schema. - That keeps the UI resilient to prompt iteration. ### Acceptance criteria - The advisor tool card renders readable content rather than raw quoted JSON in the final release branch. - Running, limit, and error states are visibly distinct. - Storybook stories and play assertions cover the new states. - Existing tool rendering flows remain unchanged. ## Phase 6 — Automated tests and validation gates ### Backend tests to add 1. Advisor runtime/tool tests - question validation; - tool-less nested execution assertion; - success result shaping; - limit-reached result shaping; - error result shaping. 2. Prompt/gating tests in chatd - advisor disabled ⇒ no tool, no guidance; - advisor enabled/root chat ⇒ tool + guidance; - child chat ⇒ advisor absent. 3. Chatloop policy tests - advisor alone runs; - advisor + action tool mixed batch returns deterministic policy errors; - non-advisor tools still execute normally. 4. Usage/metrics tests - per-run cap resets correctly; - builtin tool labeling includes `advisor`; - returned metadata includes model/usage summary when available. ### Frontend tests to add - Storybook `play()` assertions for the advisor renderer states. - Verify expand/collapse behavior and visible fallback text. - Verify the message timeline still renders adjacent tools correctly. ### Recommended command sequence Run these as the implementation matures, not only at the end: 1. Backend-focused gate after phases 1–4: - `make test RUN=TestAdvisor` - `make test RUN=TestChatloopAdvisor` - `make lint` 2. Frontend-focused gate after phase 5: - `pnpm test:storybook src/pages/AgentsPage/components/ChatElements/tools/AdvisorTool.stories.tsx` - `pnpm lint` - `pnpm format` 3. Final repo gate before handoff: - `make pre-commit` - run any additional targeted `make test RUN=...` selections covering touched chatd paths > Use the exact new test names the implementing agents create; the names above are recommended anchors, not existing tests. ## Dogfooding plan ### Principle Dogfood the change as a real agent feature, not just a unit-tested backend. Per the dogfood and `agent-browser` skills, the reviewer should get watchable repro videos plus screenshots that make the behavior obvious without reading logs. ### Required setup 1. Start the full dev environment with: - `./scripts/develop.sh` 2. If the frontend renderer changes, also start Storybook from `site/` with: - `pnpm storybook --no-open` 3. Use `agent-browser` directly — never `npx agent-browser`. 4. Use named browser sessions and an output folder such as: - `./dogfood-output/advisor/` - with subfolders `screenshots/` and `videos/` ### Evidence protocol For every interactive scenario below: 1. Start video recording before the action. 2. Capture step-by-step screenshots at human pace. 3. Capture one annotated screenshot of the final state. 4. Stop the recording. 5. Note the exact pass/fail observation in the QA report. For static UI states (for example Storybook error/limit cards), an annotated screenshot is sufficient; video is optional but still encouraged by this project’s review preference. ### Dogfood scenarios #### Scenario A — Happy path in the real Agents UI Goal: prove that a root agent chat can invoke advisor and produce a readable recommendation before taking further action. Steps: 1. Open the Agents page with an advisor-enabled root chat. 2. Start a repro video. 3. Send a prompt that should reasonably trigger strategic planning, such as an architecture or multi-tradeoff question. 4. Capture screenshots of: - the prompt before send; - the running advisor state; - the completed advisor card and the assistant’s follow-up response. 5. Stop recording. Pass criteria: - advisor appears in the timeline; - the rendered result is readable; - the assistant can continue after consuming the advisor output. #### Scenario B — Advisor unavailable path Goal: prove the feature is truly gated. Suggested variants (at least one is required, both are better): - feature flag/config off; - child/sub-agent chat. Evidence: - annotated screenshot of the chat/tool state showing advisor is absent; - short video if toggling the gate live is part of the repro. Pass criteria: - no advisor tool is available; - no advisor-specific prompt behavior leaks through. #### Scenario C — UI states in Storybook Goal: prove the renderer handles non-happy states cleanly. Required story states: - success/advice; - running; - limit reached; - error. Evidence: - one screenshot per state; - at least one short video showing collapse/expand behavior. Pass criteria: - success renders readable advice; - limit/error have visible fallback text; - the component behaves like the other tool cards. #### Scenario D — Regression sweep of nearby tools Goal: ensure advisor does not break the surrounding chat timeline. Check at minimum: - another existing built-in tool still renders correctly near advisor; - sub-agent/tool cards still expand/collapse normally; - no obvious console errors appear in the Agents page during the advisor flow. Evidence: - screenshots of adjacent tool cards; - console/error capture if anything suspicious appears. ### `agent-browser` usage notes for the QA agent - Prefer `agent-browser batch` for 2+ sequential commands when no intermediate parsing is needed. - Use `snapshot -i` to discover interactive refs. - Re-snapshot after navigation or major DOM changes. - Avoid `wait --load networkidle` unless the page is known to go idle; prefer explicit element/text waits or short fixed waits. - Record videos at human pace and include pauses that a reviewer can follow. ## Rollout plan ### Initial rollout - Gate behind a server-side advisor-enabled flag. - Enable only for selected internal/root agent chats first. - Watch metrics for: - invocation count; - failure rate; - latency; - obvious retry loops. ### Expansion conditions Expand beyond the initial rollout only after the following are true: - mixed-batch policy behavior is stable; - cost impact is understood; - frontend UX is readable in production-like dogfood; - no recursion surprises have appeared with sub-agent flows. ### Explicit non-goals for the first release - advisor inside child/sub-agent chats; - provider-agnostic streaming phase UI; - MCP-based external advisor implementation; - mandatory DB-backed advisor cost reporting. ## Final acceptance checklist - [ ] `advisor` is a built-in chatd tool, not an MCP/dynamic-tool substitute. - [ ] The nested advisor call is tool-less and bounded to one in-memory step. - [ ] One eligibility boolean controls both tool registration and prompt guidance injection. - [ ] Root chats can use advisor; child chats cannot in the initial rollout. - [ ] Mixed advisor/action batches produce deterministic policy errors instead of partial execution. - [ ] Per-run usage caps and limit-reached behavior work. - [ ] Advisor usage is visible in metadata/metrics without forcing a DB migration for MVP. - [ ] The Agents UI has a readable advisor card and Storybook coverage. - [ ] Dogfooding produced screenshots and repro videos for the required scenarios. - [ ] Validation commands (`make lint`, targeted `make test`, Storybook tests, `make pre-commit`) passed before handoff. ## Suggested PR split 1. PR 1 — Backend foundation - `chatadvisor/` package - `chattool/advisor.go` - `chatloop` exclusive policy - chatd gating/prompt sync - backend tests 2. PR 2 — Frontend + QA - advisor renderer - stories/play assertions - dogfood artifacts and QA notes 3. PR 3 — Optional follow-ups only if demanded by stakeholders - separate advisor model override - persistent advisor billing/queryability - transient phase-stream UX </details> --- _Generated with [`mux`](https://github.com/coder/mux) • Model: `anthropic:claude-opus-4-7` • Thinking: `max`_	2026-04-30 14:22:33 +02:00
Thomas Kosiewski	a84534315c	feat(coderd/x/chatd/chatloop): add exclusive tool execution policy (#24619 ) ## Summary Introduce an exclusive-tool execution policy in `chatloop.Run()` so that a designated "planning-only" tool can never execute in the same batch as any other locally executed tool. This is the generic foundation for the advisor tool landed in the stacked PRs above. ## Motivation The advisor tool (stacked on top) is intentionally a pre-action planning step. It must be consulted alone, not interleaved with side-effecting tools, so the model has to reason strategically before committing to actions. Rather than building advisor-specific plumbing into `chatloop`, this PR adds a small, general-purpose policy hook that any future tool can opt into. ## Changes - `RunOptions.ExclusiveToolNames map[string]bool` declares which tool names must run exclusively within a single tool-execution batch. - `executeTools()` detects mixed batches: if any exclusive tool name appears next to any other locally executed tool, no tool runs. Instead, structured `ToolResultOutputContentError` entries are synthesized for every tool in the batch so the model can cleanly retry. - Deterministic, model-facing error copy: - Exclusive tool gets: "must be called by itself before action tools". - Sibling tools get: "skipped because `<exclusive>` must run alone". - New tests in `chatloop_test.go` cover: single exclusive batch runs normally, mixed batch returns policy errors and executes nothing, non-exclusive batches are untouched. ## Stack context This is PR 1 of 6 in the advisor feature stack. ``` main └─ feat/advisor-01-chatloop-exclusive-policy ← this PR └─ feat/advisor-02-chatadvisor-pkg └─ feat/advisor-03-config-api ├─ feat/advisor-04-chatd-runtime │ └─ feat/advisor-05-chat-tool-renderer └─ feat/advisor-06-admin-settings-ui ``` ## Scope / non-goals - No advisor-specific code lives in this PR. - Zero behavior change when `ExclusiveToolNames` is empty (current default for every existing caller). ## Validation - `go test ./coderd/x/chatd/chatloop/... -run TestExclusive` - `make lint` --- <details> <summary>📋 Implementation Plan (shared across the advisor stack)</summary> # Plan: Add a Mux-style advisor tool to coder agents/chatd ## Outcome Add a first-class `advisor` tool to agent chats in `coderd/x/chatd` that feels native to Coder: - it is a built-in server-side tool, not an MCP/dynamic-tool workaround; - it performs a nested tool-less model call for strategic advice; - it is exposed only when eligible, and the prompt mentions it only when it is actually available; - it is treated as a planning-only tool so it does not run alongside action tools in the same batch; - it tracks usage/cost separately enough for operators to reason about it; - it has a minimally polished UI in the Agents page; - and it ships with explicit dogfooding evidence, including screenshots and repro videos. ## Design decisions to lock before coding 1. Primary architecture: native built-in tool in `chattool/`, backed by a small `chatadvisor` package. 2. Nested model execution: reuse chatd's existing model/provider stack for a one-step, tool-less advisor call rather than inventing a new provider pathway. 3. Execution policy: treat `advisor` as an exclusive/planning-only tool; mixed batches must return structured policy errors and force the model to retry cleanly. 4. Availability: initial rollout is for root agent chats only; disable for child/sub-agent chats until recursion/cost policy is proven. 5. Prompt sync: use one eligibility boolean to drive both tool registration and advisor guidance injection. 6. Persistence/cost split: MVP should keep advisor usage visible in result metadata and server metrics; only add DB schema if product/billing explicitly needs queryable advisor-specific cost. 7. UI scope: generic tool rendering is an acceptable temporary milestone during backend bring-up, but the release candidate should include a dedicated lightweight advisor renderer. ## Delivery model The work should be executed as coordinated workstreams with one integration owner and parallel contributors for low-conflict areas. The integration owner should own `coderd/x/chatd/chatd.go` because prompt assembly, tool registration, and model resolution all converge there. ## Detailed workstreams ### Repo evidence used for this plan <details> <summary>Mux reference and current chatd seams</summary> Mux reference implementation - `src/node/services/tools/advisor.ts` — native advisor tool implementation. - `src/common/constants/advisor.ts` — advisor prompt/constants and truncation policy. - `src/common/utils/tools/tools.ts` — conditional tool registration. - `src/node/services/streamContextBuilder.ts` — injects advisor guidance only when the tool is available. Current chatd seams - `coderd/x/chatd/chatd.go` - `processChat()` — tool assembly, prompt assembly, and chatloop invocation. - `resolveChatModel()` — current model/provider/key resolution seam. - `type Config struct` — server-level chatd configuration surface. - `coderd/x/chatd/chatloop/chatloop.go` - `Run()` — main streaming/model loop. - `executeTools()` — built-in tool execution/batching seam. - `coderd/x/chatd/chattool/` — built-in tool implementations. - `site/src/pages/AgentsPage/components/ChatElements/tools/Tool.tsx` — tool renderer dispatch. - `site/src/pages/AgentsPage/components/ChatConversation/messageParsing.ts` and `ConversationTimeline.tsx` — tool/result merge and rendering flow. </details> ### Workstream map and ownership \| Workstream \| Primary owner \| Main files \| Can run in parallel? \| Done when \| \|---\|---\|---\|---\|---\| \| 0. Integration + gating \| Integration lead \| `coderd/x/chatd/chatd.go` \| No; central merge lane \| Tool registration, prompt sync, and model selection are wired together \| \| 1. Advisor runtime + tool \| Backend agent \| new `coderd/x/chatd/chatadvisor/`, new `coderd/x/chatd/chattool/advisor.go` \| Yes \| Tool can perform a tool-less advisor call in memory and return structured results \| \| 2. Planning-only execution policy \| Chatloop agent \| `coderd/x/chatd/chatloop/chatloop.go`, related tests \| Yes \| Mixed `advisor` + action-tool batches are rejected cleanly and deterministically \| \| 3. Metrics/usage/config \| Backend/telemetry agent \| `chatd.go`, `chatloop/metrics.go`, optional config plumbing \| Partially; coordinate with integration lead \| Advisor usage is separately visible in metadata/metrics and limits are enforced \| \| 4. Frontend rendering \| Frontend agent \| `site/.../tools/Tool.tsx`, new `AdvisorTool.tsx`, stories \| Yes after result schema stabilizes \| Advisor renders as a readable card and story tests pass \| \| 5. Dogfood + QA evidence \| QA agent \| dev server, Storybook, dogfood output \| After backend + UI are usable \| Repro videos, screenshots, and a concise QA report exist \| ### Parallelization rules - Do not split `coderd/x/chatd/chatd.go` across multiple execution agents without an integration lead. That file owns prompt building, tool registration, model resolution, and cost persistence. - Workstreams 1 and 2 can be developed in parallel and then stacked onto the integration branch. - Workstream 4 should begin once the backend result schema is agreed on, even if the backend is still behind a feature flag. - Any agent that needs to re-check Mux behavior should clone `coder/mux` into a temporary directory (for example, `$(mktemp -d)/mux`) and inspect it read-only; do not vendor or copy code from Mux directly. ## Phase 0 — Preflight and guardrails ### Goals - Align the team on the smallest shippable architecture. - Prevent scope creep into MCP/dynamic-tool/sub-agent variants. - Decide upfront what is MVP vs. follow-up. ### Tasks 1. Confirm the MVP boundary. - Ship a built-in advisor tool first. - Do not make MCP, dynamic tools, or sub-agents the primary implementation. - Do not add transient streaming phases in the first backend PR unless they fall out almost for free. 2. Confirm local workflow hygiene before coding. - Ensure the repo is using the project git hooks from `scripts/githooks`. - Do not bypass hooks with `--no-verify`. - Use `./scripts/develop.sh` for the full dev server rather than manual build/run commands. 3. Lock the model-selection policy. - Recommended MVP: advisor uses the same resolved provider/model/cost config as the current chat, with advisor-specific max-output and usage caps. - Follow-up only if required: add a separate `AdvisorModelConfigID`-style override that resolves through the existing `configCache`/model-config path. Do not invent a new free-form `provider:model` parser if chatd already stores provider/model separately. 4. Lock the persistence policy. - Recommended MVP: no DB migration. Persist advisor-visible metadata in the tool result and record separate metrics in memory/Prometheus. - Only if product/billing explicitly asks for queryable advisor cost: add a later DB migration or usage table, following the normal `queries/.sql` + `make gen` workflow. 5. Create an execution ADR note in the work item or tracking doc.* - Capture: built-in tool, tool-less nested call, root-chat-only rollout, exclusive execution policy, MVP no-DB-migration default. ### Quality gate - Everyone on the team can state the same answers to these questions: - Is advisor a built-in tool? Yes. - Can advisor run with action tools in the same batch? No. - Does advisor get tools of its own? No. - Is a DB migration required for MVP? No, unless billing insists. ## Phase 1 — Build the advisor runtime and tool wrapper ### Goals Create the core advisor implementation in a way that is easy to test and keeps `chattool/` thin. ### Files to add - `coderd/x/chatd/chatadvisor/types.go` - `coderd/x/chatd/chatadvisor/guidance.go` - `coderd/x/chatd/chatadvisor/handoff.go` - `coderd/x/chatd/chatadvisor/runtime.go` - `coderd/x/chatd/chatadvisor/runner.go` - `coderd/x/chatd/chattool/advisor.go` ### Responsibilities by file 1. `types.go` - Define the input/result schema used by the tool and UI. - Keep the result shape close to Mux so the UI and model both have predictable cases. - Recommended result variants: - `advice` - `limit_reached` - `error` Recommended shape: ```go type AdvisorArgs struct { Question string `json:"question"` } type AdvisorResult struct { Type string `json:"type"` Advice string `json:"advice,omitempty"` Error string `json:"error,omitempty"` AdvisorModel string `json:"advisor_model,omitempty"` RemainingUses int `json:"remaining_uses,omitempty"` Usage AdvisorUsageResult `json:"usage,omitempty"` } ``` 2. `guidance.go`* - Hold two strings: - the nested advisor system prompt; - the parent-agent guidance block to inject into the outer system prompt. - The nested advisor prompt must say, in plain language: - you are advising the parent agent; - you do not address the end user directly; - you do not claim actions happened; - you return concise strategic guidance and tradeoffs. 3. `runtime.go` - Define the per-run runtime state. - Recommended fields: - resolved model + model config; - provider keys/options reused from the outer chat; - `MaxUsesPerRun`; - `MaxOutputTokens`; - atomic/current call counter; - callback(s) to obtain the current prompt snapshot and current-step snapshot; - optional metrics/usage hook. - Add fail-fast validation for impossible config: nil model, non-positive limits, empty prompt builders, etc. 4. `handoff.go` - Build the advisor handoff message from: - the explicit question; - the exact prompt/messages the parent model just used; - the current step's text/reasoning snapshot, if available; - the most recent relevant tool outputs, if they are already in the prompt snapshot. - Important: use the already-prepared outer prompt tail, not a fresh DB reload. That keeps the advisor aligned with compaction and the exact context the outer model saw. - Apply hard truncation budgets with recent-context bias. 5. `runner.go` - Execute the nested advisor call. - Recommended implementation: call `chatloop.Run()` in an in-memory, one-step mode: - `Tools: nil` - `ProviderTools: nil` - `MaxSteps: 1` - `PersistStep`: capture the assistant output in memory instead of writing DB rows - Reuse the existing provider/model/cost path instead of building a second provider runner. - Assert that no tool definitions are passed to the nested call. 6. `chattool/advisor.go` - Keep this file thin and consistent with other built-ins. - Responsibilities: - decode `AdvisorArgs`; - validate `Question` is non-empty and bounded; - call the `chatadvisor` runner; - return a structured tool response. ### Defensive programming requirements - Assert `Question` is non-empty after trimming. - Assert runtime limits are positive. - Assert the nested advisor call runs with zero tools/provider tools. - Assert `AdvisorResult.Type` is one of the known variants before returning. - Assert remaining uses never goes negative. ### Acceptance criteria - A unit test can call the advisor tool with a fake model and receive a stable `advice` result. - The nested advisor call is impossible to run with tools accidentally attached. - The core logic lives in `chatadvisor/`, not embedded inside `chatd.go`. ## Phase 2 — Wire advisor into chatd and keep prompt/tool availability in sync ### Goals Register the tool in the right place, expose it only when eligible, and inject system guidance only when the tool is present. ### Files to modify - `coderd/x/chatd/chatd.go` - optionally a small helper file if `chatd.go` becomes too crowded ### Tasks 1. Compute one eligibility boolean in `processChat()`. Recommended inputs: - server-level advisor enabled flag; - root chat only (`chat.ParentChatID == uuid.Nil` or equivalent existing root/child check); - a usable resolved model/provider exists; - optional experiment/workspace/org gate if product wants staged rollout. 2. Create the runtime once per outer chat run. - Use the model/config/keys resolved by `resolveChatModel()`. - Reuse provider options from the current chat's `ChatModelCallConfig`. - Set `MaxUsesPerRun` and `MaxOutputTokens` from advisor config defaults. 3. Register the tool in the built-in tool block. - Insert after the skill tools and before MCP tools in `processChat()`. - Record `builtinToolNames["advisor"] = true` so metrics stay bounded. 4. Inject advisor guidance into the outer system prompt using the same boolean. - Use `chatprompt.InsertSystem()` in the same prompt assembly path that already injects user/system instructions. - Place the block near the existing instruction insertion, before plan-path/skill context blocks. - Wrap the guidance in an explicit tag like `<advisor-guidance>` so it is easy to spot in tests and future refactors. 5. Keep advisor out of child chats for the first release. - That avoids recursion/cost blowups with `spawn_agent` / `wait_agent` flows. - Document this explicitly in the rollout notes and tests. ### Acceptance criteria - If advisor is disabled, neither the tool nor the prompt guidance appears. - If advisor is enabled, both the tool and the prompt guidance appear. - Root chats can use advisor; child chats cannot. - Built-in tool names include `advisor` so metrics do not collapse it into the generic `mcp` label. ## Phase 3 — Enforce planning-only execution policy in `chatloop` ### Goals Prevent the model from calling `advisor` and action tools in the same execution batch. ### Files to modify - `coderd/x/chatd/chatloop/chatloop.go` - related chatloop tests ### Recommended implementation Keep the MVP small; do not build a general policy engine yet. 1. Add a minimal field to `chatloop.RunOptions`, for example: ```go ExclusiveToolName string ``` 2. In `Run()` / `executeTools()`, detect the case where the exclusive tool appears in the same local-tool batch as any other locally executed tool. 3. When that happens, synthesize structured tool-result errors for the affected calls instead of executing anything in the batch. - `advisor` should receive a clear error like: _advisor must be called by itself before action tools_. - The sibling action tools should receive a paired policy error like: _this tool was skipped because advisor must run alone_. 4. Let the outer model see those tool errors and retry cleanly. - This is simpler and safer than partial execution or hidden deferral. - It preserves deterministic transcript history for debugging. 5. Pass the just-finished step snapshot into the tool execution context. - The advisor runtime should be able to see the current step's text/reasoning content, because that is often the best hint about what the outer model is trying to decide. ### Why this is the right fit - It matches the intended semantics: advisor is consulted before* taking action. - It avoids subtle race conditions caused by concurrent built-in tool execution. - It keeps the behavior easy to test with fake models. ### Acceptance criteria - A model-emitted batch containing only `advisor` succeeds. - A model-emitted batch containing `advisor` plus any other locally executed tool returns deterministic policy errors and executes nothing. - Non-advisor tool execution stays unchanged for normal chats. ## Phase 4 — Usage limits, metrics, and configuration ### Goals Make advisor safe to operate without over-designing billing/storage in the first release. ### Files to modify - `coderd/x/chatd/chatd.go` - `coderd/x/chatd/chatloop/metrics.go` as needed - `coderd/x/chatd/chatd.go` `Config` struct and constructor path - optional follow-up config/db files only if a separate advisor model or persistent billing is required ### Tasks 1. Add explicit server config knobs for MVP. Recommended fields on `chatd.Config` or a nested advisor config struct: - `AdvisorEnabled bool` - `AdvisorMaxUsesPerRun int` - `AdvisorMaxOutputTokens int64` 2. Track usage per outer run. - Reset the counter for each `processChat()` invocation. - Return `remaining_uses` in the tool result. - Return `limit_reached` when the cap is exhausted. 3. Expose advisor usage metadata in the tool result. - Include model name and token/cost summary if available. - Use the same `callConfig.Cost` calculation path as the outer chat for MVP if advisor reuses the same model. 4. Record server-side metrics. - Count advisor invocations, failures, and latency. - Ensure they show up under the built-in tool label `advisor`. 5. Optional decision gate: separate advisor model. - If product insists on a stronger/different advisor model, add a follow-up config hook that resolves another existing chat model config through the same `configCache` path. - Keep that out of the first landing PR unless it is required for acceptance. 6. Optional decision gate: queryable advisor cost. - If this becomes required, spin a follow-up DB task: - update `coderd/database/queries/.sql`; - add migration files; - run `make gen`; - update audit mappings if a new auditable type/field is introduced. ### Acceptance criteria - Advisor calls are capped per outer run. - Limit exhaustion is user-visible in the tool result. - Metrics distinguish advisor calls from other built-in tools. - MVP does not require a schema migration unless explicitly approved. ## Phase 5 — Frontend rendering and Storybook coverage ### Goals Make advisor feel intentional in the Agents UI without blocking the backend on fancy streaming UI. ### Files to modify - `site/src/pages/AgentsPage/components/ChatElements/tools/Tool.tsx` - new `site/src/pages/AgentsPage/components/ChatElements/tools/AdvisorTool.tsx` - Storybook story file(s) in the same tools directory ### Delivery strategy 1. Intermediate milestone during backend bring-up:* rely on the existing generic tool renderer if needed. - This is acceptable only as a short-lived integration checkpoint. 2. Release milestone: add a dedicated lightweight `AdvisorTool` renderer. - Reuse existing primitives: - `ToolCollapsible` - `ToolIcon` - `Response` for markdown/prose rendering - `ScrollArea` if the advice can be long - Keep styling light and consistent with the Agents page. - Do not add unnecessary React memoization in `site/src/pages/AgentsPage/`; that area is already React-Compiler aware. 3. Render the structured result states cleanly. - `advice` — readable prose/markdown with optional metadata footer. - `limit_reached` — warning-style message. - `error` — error state with visible fallback text. - `running` — existing tool loading state/spinner is enough for MVP. 4. Add Storybook coverage instead of ad-hoc component tests. Recommended stories: - successful advice; - running/loading; - limit reached; - error. 5. Keep the UI contract narrow. - Prefer one text field like `advice` plus small metadata rather than a deeply nested schema. - That keeps the UI resilient to prompt iteration. ### Acceptance criteria - The advisor tool card renders readable content rather than raw quoted JSON in the final release branch. - Running, limit, and error states are visibly distinct. - Storybook stories and play assertions cover the new states. - Existing tool rendering flows remain unchanged. ## Phase 6 — Automated tests and validation gates ### Backend tests to add 1. Advisor runtime/tool tests - question validation; - tool-less nested execution assertion; - success result shaping; - limit-reached result shaping; - error result shaping. 2. Prompt/gating tests in chatd - advisor disabled ⇒ no tool, no guidance; - advisor enabled/root chat ⇒ tool + guidance; - child chat ⇒ advisor absent. 3. Chatloop policy tests - advisor alone runs; - advisor + action tool mixed batch returns deterministic policy errors; - non-advisor tools still execute normally. 4. Usage/metrics tests - per-run cap resets correctly; - builtin tool labeling includes `advisor`; - returned metadata includes model/usage summary when available. ### Frontend tests to add - Storybook `play()` assertions for the advisor renderer states. - Verify expand/collapse behavior and visible fallback text. - Verify the message timeline still renders adjacent tools correctly. ### Recommended command sequence Run these as the implementation matures, not only at the end: 1. Backend-focused gate after phases 1–4: - `make test RUN=TestAdvisor` - `make test RUN=TestChatloopAdvisor` - `make lint` 2. Frontend-focused gate after phase 5: - `pnpm test:storybook src/pages/AgentsPage/components/ChatElements/tools/AdvisorTool.stories.tsx` - `pnpm lint` - `pnpm format` 3. Final repo gate before handoff: - `make pre-commit` - run any additional targeted `make test RUN=...` selections covering touched chatd paths > Use the exact new test names the implementing agents create; the names above are recommended anchors, not existing tests. ## Dogfooding plan ### Principle Dogfood the change as a real agent feature, not just a unit-tested backend. Per the dogfood and `agent-browser` skills, the reviewer should get watchable repro videos plus screenshots that make the behavior obvious without reading logs. ### Required setup 1. Start the full dev environment with: - `./scripts/develop.sh` 2. If the frontend renderer changes, also start Storybook from `site/` with: - `pnpm storybook --no-open` 3. Use `agent-browser` directly — never `npx agent-browser`. 4. Use named browser sessions and an output folder such as: - `./dogfood-output/advisor/` - with subfolders `screenshots/` and `videos/` ### Evidence protocol For every interactive scenario below: 1. Start video recording before the action. 2. Capture step-by-step screenshots at human pace. 3. Capture one annotated screenshot of the final state. 4. Stop the recording. 5. Note the exact pass/fail observation in the QA report. For static UI states (for example Storybook error/limit cards), an annotated screenshot is sufficient; video is optional but still encouraged by this project’s review preference. ### Dogfood scenarios #### Scenario A — Happy path in the real Agents UI Goal: prove that a root agent chat can invoke advisor and produce a readable recommendation before taking further action. Steps: 1. Open the Agents page with an advisor-enabled root chat. 2. Start a repro video. 3. Send a prompt that should reasonably trigger strategic planning, such as an architecture or multi-tradeoff question. 4. Capture screenshots of: - the prompt before send; - the running advisor state; - the completed advisor card and the assistant’s follow-up response. 5. Stop recording. Pass criteria: - advisor appears in the timeline; - the rendered result is readable; - the assistant can continue after consuming the advisor output. #### Scenario B — Advisor unavailable path Goal: prove the feature is truly gated. Suggested variants (at least one is required, both are better): - feature flag/config off; - child/sub-agent chat. Evidence: - annotated screenshot of the chat/tool state showing advisor is absent; - short video if toggling the gate live is part of the repro. Pass criteria: - no advisor tool is available; - no advisor-specific prompt behavior leaks through. #### Scenario C — UI states in Storybook Goal: prove the renderer handles non-happy states cleanly. Required story states: - success/advice; - running; - limit reached; - error. Evidence: - one screenshot per state; - at least one short video showing collapse/expand behavior. Pass criteria: - success renders readable advice; - limit/error have visible fallback text; - the component behaves like the other tool cards. #### Scenario D — Regression sweep of nearby tools Goal: ensure advisor does not break the surrounding chat timeline. Check at minimum: - another existing built-in tool still renders correctly near advisor; - sub-agent/tool cards still expand/collapse normally; - no obvious console errors appear in the Agents page during the advisor flow. Evidence: - screenshots of adjacent tool cards; - console/error capture if anything suspicious appears. ### `agent-browser` usage notes for the QA agent - Prefer `agent-browser batch` for 2+ sequential commands when no intermediate parsing is needed. - Use `snapshot -i` to discover interactive refs. - Re-snapshot after navigation or major DOM changes. - Avoid `wait --load networkidle` unless the page is known to go idle; prefer explicit element/text waits or short fixed waits. - Record videos at human pace and include pauses that a reviewer can follow. ## Rollout plan ### Initial rollout - Gate behind a server-side advisor-enabled flag. - Enable only for selected internal/root agent chats first. - Watch metrics for: - invocation count; - failure rate; - latency; - obvious retry loops. ### Expansion conditions Expand beyond the initial rollout only after the following are true: - mixed-batch policy behavior is stable; - cost impact is understood; - frontend UX is readable in production-like dogfood; - no recursion surprises have appeared with sub-agent flows. ### Explicit non-goals for the first release - advisor inside child/sub-agent chats; - provider-agnostic streaming phase UI; - MCP-based external advisor implementation; - mandatory DB-backed advisor cost reporting. ## Final acceptance checklist - [ ] `advisor` is a built-in chatd tool, not an MCP/dynamic-tool substitute. - [ ] The nested advisor call is tool-less and bounded to one in-memory step. - [ ] One eligibility boolean controls both tool registration and prompt guidance injection. - [ ] Root chats can use advisor; child chats cannot in the initial rollout. - [ ] Mixed advisor/action batches produce deterministic policy errors instead of partial execution. - [ ] Per-run usage caps and limit-reached behavior work. - [ ] Advisor usage is visible in metadata/metrics without forcing a DB migration for MVP. - [ ] The Agents UI has a readable advisor card and Storybook coverage. - [ ] Dogfooding produced screenshots and repro videos for the required scenarios. - [ ] Validation commands (`make lint`, targeted `make test`, Storybook tests, `make pre-commit`) passed before handoff. ## Suggested PR split 1. PR 1 — Backend foundation - `chatadvisor/` package - `chattool/advisor.go` - `chatloop` exclusive policy - chatd gating/prompt sync - backend tests 2. PR 2 — Frontend + QA - advisor renderer - stories/play assertions - dogfood artifacts and QA notes 3. PR 3 — Optional follow-ups only if demanded by stakeholders - separate advisor model override - persistent advisor billing/queryability - transient phase-stream UX </details> --- _Generated with [`mux`](https://github.com/coder/mux) • Model: `anthropic:claude-opus-4-7` • Thinking: `max`_	2026-04-30 14:08:59 +02:00
Marcin Tojek	f993b72628	fix: introduce ResourceAiSeat for fine-grained AI seat RBAC (#24613 ) Fixes: https://github.com/coder/internal/issues/1444	2026-04-30 12:29:35 +02:00
Susana Ferreira	dbb50ebaaf	feat: remove 429 from aibridge circuit breaker failure conditions (#24701 ) ## Description Removes 429 (Too Many Requests) from the circuit breaker failure conditions. Rate limiting is now handled by automatic key failover instead of tripping the circuit breaker. ## Changes `DefaultIsFailure` no longer treats 429 as a circuit breaker failure. The circuit breaker now only trips on server overload responses (503, 529). Tests and integration tests updated to use 503 instead of 429 for tripping circuits. Description strings in deployment config updated to reflect the change. Closes https://github.com/coder/internal/issues/1445 > [!NOTE] > Initially generated by Coder Agents, modified and reviewed by @ssncferreira	2026-04-30 09:31:32 +01:00
Susana Ferreira	101a4082dd	feat: support multiple keys per AI Bridge provider (#24683 ) ## Description Adds support for configuring multiple API keys per AI Bridge provider. This PR introduces the configuration parsing and validation only; wiring the key pools into the aibridge providers will happen in upstream PRs. ## Changes Providers now accept a comma-separated list of keys via the `KEYS` env var (or a single key via the existing `KEY` var). The two are mutually exclusive. Bedrock follows the same pattern with `BEDROCK_ACCESS_KEYS` / `BEDROCK_ACCESS_KEY_SECRETS`, with an additional validation that the two slices have matching lengths. Key validation at startup checks for empty values, duplicates, and a maximum of 5 keys per provider. Related to: https://github.com/coder/internal/issues/1445 > [!NOTE] > Initially generated by Coder Agents, modified and reviewed by @ssncferreira	2026-04-30 09:19:32 +01:00
Susana Ferreira	123c8dfc02	feat(aibridge): add key pool with state tracking and walker (#24681 ) ## Description Adds the `aibridge/keypool` package, a thread-safe key pool with per-key state tracking and cooldown expiry. This PR introduces the package only; wiring it into the aibridge providers and coder configuration will happen in upstream PRs. ## Changes Each key is in one of three states: Valid (available), Temporary (rate-limited with cooldown expiry), or Permanent (revoked/unauthorized, terminal until restart). The state is derived from the key fields rather than stored explicitly: once a cooldown expires, the key is valid again without any external action. A `Walker` provides per-request key traversal using a primary-with-fallback strategy: each request walks the pool from index 0, skipping unavailable keys, so the first key is always preferred when healthy. Walkers are independent, so concurrent requests traverse the pool without interfering with each other. `MarkTemporary` preserves the longer cooldown when concurrent requests both mark the same key. Relates to https://github.com/coder/internal/issues/1445 > [!NOTE] > Initially generated by Coder Agents, modified and reviewed by @ssncferreira	2026-04-30 09:07:57 +01:00
dylanhuff-at-coder	fb84e72319	feat: add secret requirement contract to dynamic parameters (#24785 ) Adds structured `secret_requirements` to dynamic parameter responses and enforces missing required secrets during workspace start. Stop, delete, and tag rendering paths skip secret requirement enforcement so unmet secrets do not prevent cleanup. The SDK, generated API docs/types, and backend render/resolver/wsbuilder tests are updated for the new contract.	2026-04-29 16:38:26 -07:00
Asher	fe6c8771df	fix: render non-typed parameter changes immediately (#24755 ) This was already done for the workspace creation page, but not the edit page, as it does not share the base form, so all I did was copy that fix into the edit page.	2026-04-29 13:00:58 -08:00
Asher	70d46943db	fix: match on ID instead of username (#24797 ) The username suffix could put the name past the 32 character limit, causing the test to flake. Instead of using a suffix, match on the expected ID instead.	2026-04-29 12:24:52 -08:00
Asher	be57af5ff0	feat: add exit code and status to workspace agent scripts (#24505 ) For scripts that have not finished or in dry run cases these will be omitted.	2026-04-29 12:24:26 -08:00
Zach	1c30d52b2b	feat: audit user secret create, update, and delete (#24756 ) Emit user secret audit log entries for create/update/delete operations. Reads stay un-audited, matching every other resource. Audit log entries record changes in user secret name, environment variable name, file path, and value. The secret value column is marked `ActionSecret` so the diff records the change without showing the ciphertext or plaintext. Closes a TOCTOU window on delete to ensure no phantom audit logs for a delete of a non-existent secret. Secret update accepts a small TOCTOU window matching the other audited resources (templates, workspaces, chats). The two-query pattern is wrapped in a transaction so audit state can't leak from a failed mutation.	2026-04-29 12:57:47 -06:00
George K	25ae415481	fix(site): fix download log size display (#24758 ) Previously, the workspace "Download logs" dialog formatted the original byte count with the promoted unit, so sizes above 1 KiB could be shown incorrectly, for example `4472 KB` instead of `4.37 KB`. Exact 1024-byte files also stayed in bytes. v2.34.0-rc.0	2026-04-29 10:52:01 -07:00
Callum Styan	950660e392	fix(cli): extend `exp scaletest cleanup` to properly clean up prebuilds (#23628 ) Signed-off-by: Callum Styan <callumstyan@gmail.com> Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>	2026-04-29 10:28:27 -07:00
Garrett Delfosse	54d650ea79	fix(tailnet): preserve DNS hosts across control plane reconnections (#24253 ) When the control plane connection drops and reconnects, a new `tunnelUpdater` is created with empty workspace state. This causes the in-memory DNS resolver to lose all host records, breaking `.coder` name resolution until the server sends a fresh workspace snapshot. If the API is unreachable (e.g., the route goes through a VPN that is also reconnecting), the snapshot never arrives and DNS stays broken indefinitely — requiring a full Coder Desktop restart. Fix: carry workspace state from the previous `tunnelUpdater` to the new one on reconnect, and immediately re-apply DNS hosts so the resolver stays populated during the reconnection window. Fixes https://linear.app/codercom/issue/PLAT-110 <details><summary>Investigation & decision log</summary> ### Root cause analysis Customer diagnostic data from Roblox (March 31) showed: - NRPT rule present (`.coder` → `fd60:627a:a42b::53`) — routing is correct - DNS resolver returns NXDOMAIN for everything including the sentinel `is.coder--connect--enabled--right--now.coder` — resolver is running but has zero host records - Coder Connect UI shows "connected" — the WireGuard data plane is up The resolver is empty because `TunnelAllWorkspaceUpdatesController.New()` creates a fresh `tunnelUpdater` with `workspaces: make(map[uuid.UUID]Workspace)` (empty). The previous updater's workspace data is discarded. If the server's workspace snapshot is delayed or the API is unreachable, the resolver has no records to serve. This is compounded by GlobalProtect VPN reconnects: the Coder API is behind the VPN, so when GP reconnects, the API route is temporarily lost and the snapshot can't arrive. ### What this PR changes - `TunnelAllWorkspaceUpdatesController.New()` now clones workspace state from the previous updater before creating the new one - Immediately re-applies DNS hosts with the inherited state (log: `re-applying DNS hosts from previous session`) - When the server's snapshot arrives, it replaces the inherited data normally - If `SetDNSHosts` fails during re-apply, it's logged as a warning and not fatal — the recvLoop will program DNS when the snapshot arrives ### What this PR does NOT fix (future work) - Tunnel binary restart: when the tunnel process itself is killed and relaunched, all in-memory state is lost. A DNS host cache on disk would be needed for this case. - NRPT rule cleanup on startup: the Tailscale fork's `nrptRuleDatabase` constructor unconditionally deletes all NRPT rules on engine creation. Deferring cleanup to the first successful `SetDNS` call would reduce the DNS gap. - Hosts file retry*: the `setHosts()` retry in the Tailscale fork (5×10ms) is too short for environments where endpoint security locks the file. These are tracked as follow-up items in the `coder/tailscale` fork. </details> > 🤖 Generated by Coder Agents	2026-04-29 12:29:44 -04:00
Paweł Banaszewski	a24dc19d49	chore: clean up env var usage in aibridge (#24783 ) > AI tools where used when creating this PR This PR removes environment variable parsing from `/aibridge` directory. Added env variables/flags for dump dir as coder options. Only added to new indexed provider options (`CODER_AIBRIDGE_PROVIDER_<N>_`) not to deprecated legacy env variables (`CODER_AIBRIDGE_ANTHROPIC_` and `CODER_AIBRIDGE_OPENAI_KEY_*`). Reverted adding `MaxRetries` option as it will be removed soon due to key failover work: https://github.com/coder/coder/pull/24783#discussion_r3155544808	2026-04-29 18:28:37 +02:00
Paweł Banaszewski	6ea9c61da0	chore: update AI Gateway docs (#24805 ) > AI tools where used when creating this PR This PR: * removes references to aibridge repository from coder docs * updates aibdrige/README.md * makes it clear aibridge (keeping old name) is a handler not a separate process * updates outdated sections about: metrics, recorded interface and supported paths. --------- Co-authored-by: Susana Ferreira <susana@coder.com>	2026-04-29 18:28:09 +02:00
Jeremy Ruppel	0754016512	feat: add role selector in the create user form (#24711 ) Adds a role selector to the create user form so admins can assign site-level roles at creation time rather than navigating to the user afterward. The `POST /api/v2/users` endpoint now accepts an optional `roles` field, wiring it through to the existing `RBACRoles` field on the internal `CreateUserRequest`. No database changes are needed since roles are already stored inline on the user row. On the frontend, a `RoleSelector` component renders the assignable roles as a scrollable multiselect checklist with the non-assignable Member role pinned as a non-interactive footer. The selector appears once a login type is chosen. Also adds a `condensed` size (690px) to `Margins` between the existing `small` (460px) and `medium` (1080px), and exposes a `size` prop on `FullPageForm`. The create user form uses `condensed` to give the role selector more breathing room. Also fixes `MockUserAdminRole` and `MockTemplateAdminRole` in test helpers to use hyphenated names (`user-admin`, `template-admin`) matching the canonical names in the Go RBAC layer. Fixes `sortRolesByAccessLevel` in `UserRoleCell` to sort unranked roles (e.g. `member`) after all known roles. Previously, `indexOf` returned -1 for unknown names, placing them first; now they receive `POSITIVE_INFINITY` as their rank. 🤖 Generated with [Claude Code](<https://claude.ai/claude-code>) --- https://github.com/user-attachments/assets/75e7c8c5-d0d2-481d-86e8-1fcfb574517c --------- Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>	2026-04-29 10:57:10 -04:00
Thomas Kosiewski	ab75e46f1d	fix: record debug runs for proposed chat titles (#24820 )	2026-04-29 16:45:48 +02:00
Paweł Banaszewski	5907730dcf	chore: update aibridge/AGENTS.md to reflect it is now part of coder/coder repo (#24822 ) > AI tools where used when creating this PR Updates `AGENTS.md` to reflect that `/aibridge` is now a directory in coder/coder repo not a separate repo.	2026-04-29 15:51:07 +02:00
DevCats	88c469c7a5	feat: add link to skills on create-a-template page (#24710 ) This pull request updates the `CreateTemplateGalleryPageView` component to enhance the page header actions by grouping them vertically and adding a new external link button for users. The most important changes are: UI improvements: * Groups the header action buttons into a vertical stack using a flex container with column direction and spacing for better layout and accessibility. New feature: * Adds a new button linking to the "template agent skill" documentation on GitHub, allowing users to easily access guidance on using the template agent skill. Screenshot <img width="1674" height="813" alt="image" src="https://github.com/user-attachments/assets/8d12df13-2310-464b-936c-1b4e2215c7aa" />	2026-04-29 08:17:26 -05:00
Cian Johnston	1856864472	fix(site/src/pages/AgentsPage): preserve ?archived in sibling navigation (#24777 ) Follow-up to #24742. Navigation paths were dropping `?archived` search params, silently resetting the filter now that it's URL-derived. Fixed all sibling navigation that used bare `/agents` paths or stored only `location.pathname` without `location.search`: - ChatTopBar mobile back button (`md:hidden`) - ChatTopBar parent chat breadcrumb - AgentsSidebar settings gear link (stored `state.from` without search) - AgentPageHeader mobile menu settings link (same `state.from` bug) - AgentAnalyticsPage mobile back button (widened `mobileBack.to` type to accept `To`) - SubagentTool "View agent" external link - AgentsPage `navigateAfterArchive` and `handleNewAgent` navigate calls Two Storybook play stories cover the ChatTopBar back button and the sidebar settings round-trip. _Generated by Coder Agents._	2026-04-29 14:14:28 +01:00
Mathias Fredriksson	5ceca94e0c	docs(coderd/x/chatd): improve edit_files tool description (#24627 ) Document the fuzzy matching behavior, error-on-ambiguity semantics, and atomic batch validation that were missing from the tool description. These are the three behaviors most likely to surprise callers who assume naive exact-match search/replace.	2026-04-29 11:30:27 +00:00
Mathias Fredriksson	782b7166a4	fix: preserve stream state on interrupt, fix auto-promote error handling (#24314 ) When tryAutoPromoteQueuedMessage's insert fails, return the error instead of swallowing it so the transaction rolls back and the queued message survives. Previously the POP DELETE committed while the INSERT silently failed, permanently losing the message. Remove clearStreamState() from the pending/waiting status handler in the frontend. The durable message event clears stream state via the existing needsStreamReset path, eliminating the visual gap where content vanishes before the persisted message arrives. Fixes CODAGT-61	2026-04-29 14:08:35 +03:00
Mathias Fredriksson	dd49a818f9	fix: export chatd.Start to separate server lifecycle (#24761 ) chatd.New() no longer auto-starts the acquire/wake loop. Callers that want chat processing call server.Start() explicitly. Tests that want a passive server skip Start(); heartbeat, stream janitor, and stale recovery still run. Closes coder/internal#1502	2026-04-29 13:54:49 +03:00
Muhammad Danish	f9068c2afa	ci: use env var instead of passing winget token inline (#24387 )	2026-04-29 06:50:14 +00:00
Rowan Smith	76242f8202	feat: add hostAliases support to Coder helm chart (#24729 ) adds support for specifying hostAliases entries in the Coder control plane pod. ``` ➜ coder git:(rowan/helm-hostaliases) helm template coder . --set coder.image.tag=v2.32.0 --set coder.hostAliases[0].hostnames[0]=coder.nicecorp.org --set coder.hostAliases[0].ip=1.1.1.1 .... --- # Source: coder/templates/coder.yaml apiVersion: apps/v1 kind: Deployment .... hostAliases: - hostnames: - coder.nicecorp.org ip: 1.1.1.1 restartPolicy: Always serviceAccountName: coder terminationGracePeriodSeconds: 60 volumes: [] ```	2026-04-29 12:45:54 +10:00
George K	9538390107	fix(coderd/healthcheck/derphealth): avoid data races in DERP report (#24795 ) Fixes two data races, one introduced in #24544 and one pre-existing. Related to: https://github.com/coder/internal/issues/1505	2026-04-28 13:06:45 -07:00
Kayla はな	dcb32165fa	feat: add `--skip-setup` flag to develop script (#24794 )	2026-04-28 13:49:43 -06:00
Kayla はな	12e9f5bb61	chore: upgrade to pnpm 10.33 (#24746 )	2026-04-28 12:12:13 -06:00
Kayla はな	5afb297042	refactor(site): remove `Stack` component (#24503 ) ## Summary Remove the deprecated `Stack` component and replace all usages with Tailwind flex utility classes. - Replaced `<Stack>` → `<div className="flex flex-col gap-4">` (and variants per props) - Updated `StackLabel` and `FormFields` to no longer depend on `Stack` - Deleted `Stack.tsx` and `Stack.stories.tsx` 74 files changed, -226 lines net. > 🤖 Generated by Coder Agents --------- Co-authored-by: Jake Howell <jacob@coder.com>	2026-04-28 12:02:13 -06:00
Atif Ali	55ed6cfa06	docs: add early access user secrets guide (#24735 )	2026-04-28 22:25:45 +05:00
Michael Suchacz	1d8e29815e	fix(coderd/x/chatd/chatdebug): restore request body after capture (#24784 ) > Mux working on behalf of Mike. Debug recording could consume request bodies when a provider SDK returned the active body from `GetBody`, which left the upstream request with an empty body after capture. Reset the request body after debug capture and add coverage for shared `GetBody` readers so debug logging does not alter the bytes sent upstream.	2026-04-28 19:09:27 +02: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
Mathias Fredriksson	881df9a5b0	feat: reload MCP config on change via lazy stat-on-request (#24700 ) The MCP manager previously read .mcp.json exactly once at agent startup. Editing the file had no effect until workspace rebuild or agent restart. handleListTools now stats config file mtimes on every tool-list request and triggers a differential reload when any file changed. Unchanged servers keep their client pointer so in-flight tool calls survive. Concurrent reload requests coalesce via singleflight. MCP stdio subprocesses use the agent's execer for resource limits and receive the same enriched environment as SSH sessions via updateEnv. On the chatd side, WorkspaceMCPTool.Run detects 404 responses from CallMCPTool (indicating the server was removed) and drops the chat's cached tool list so the next turn refetches from the agent.	2026-04-28 19:47:14 +03:00
George K	3f0e015fe5	fix: allow coderd to start with an empty DERP map when built-in DERP is disabled (#24544 ) Allow coderd to start with an empty base DERP map when built-in DERP is disabled and no static DERP map is configured, so DERP can come from workspace proxies after startup. Also add a DERP healthcheck warning when no DERP servers are currently available at runtime. Related to: https://linear.app/codercom/issue/PLAT-43/bug-coderd-unable-to-be-started-if-built-in-derp-server-disabled-and Related to: https://github.com/coder/coder/issues/22324	2026-04-28 09:17:08 -07:00
Mathias Fredriksson	1926b7e658	fix(coderd/externalauth): detect rate-limit 403/429 and narrow isFailedRefresh (#24334 ) ValidateToken treated all 403 responses as "token invalid," including GitHub rate limits. isFailedRefresh included 403 in the status code fallthrough, destroying tokens on rate-limited refresh attempts. Split the combined 401/403 check in ValidateToken into a switch on status code. On 403, inspect X-RateLimit-Remaining and Retry-After headers; if either indicates a rate limit, return optimistically valid. Handle 429 the same way. Plain 403 without rate-limit headers preserves the existing invalid-token behavior. Add incorrect_client_credentials and invalid_client to isFailedRefresh error code switch. Remove 403 from the status code fallthrough since no known provider returns 403 from the token endpoint.	2026-04-28 18:03:35 +03:00
Mathias Fredriksson	3c450899ea	fix: pass agent context config explicitly instead of reading env (#24759 ) The CODER_AGENT_EXP_* env vars are agent-internal options. When set in the workspace environment they leak to MCP subprocesses and user shells. ReadEnvConfig() captures the values and ClearEnvVars() strips them before the reinit loop, so config survives agent restarts. NewAPI and ReadEnvConfig both use applyDefaults() to fill zero fields. The chatd test passes config via agenttest.WithContextConfigFromEnv().	2026-04-28 17:58:28 +03:00
Cian Johnston	1666bff1f9	fix(coderd/x/chatd): block chain mode when provider missing tool results (#24782 ) When `StopAfterTool` fires (e.g., `propose_plan`), the LLM response containing a `function_call` is stored at OpenAI via `store=true`, but the tool result is only persisted locally. On the next user message, `resolveChainMode` sees the tool result in the local DB and concludes all calls are resolved. Chain mode activates with `previous_response_id`, but OpenAI rejects because its stored chain has an unresolved `function_call`. This adds a `providerMissingToolResults` check to `resolveChainMode` that detects the `assistant(tool-call) → tool(result) → user` pattern with no follow-up assistant message. The absence of a follow-up assistant proves the tool results were never round-tripped to the provider. When detected, chain mode is blocked and the system falls back to full history replay, which includes both the tool call and its result. Deploying this fix un-bricks existing affected chats with no DB migration needed. > Generated by Coder Agents.	2026-04-28 15:30:04 +01:00
david-fraley	5222db86c7	feat: add after_id pagination for chat messages (#24531 )	2026-04-28 08:31:33 -05:00
Michael Suchacz	8fe11e9b14	fix: match Bedrock streaming accept headers (#24781 ) > Mux is working on behalf of Mike. ## Summary - Bump `github.com/coder/anthropic-sdk-go` to the corrected Bedrock streaming header fix from coder/anthropic-sdk-go#14. - Match botocore's `InvokeModelWithResponseStream` request shape by using `X-Amzn-Bedrock-Accept` and omitting the HTTP `Accept` header. - Update chatd regression coverage for the corrected header shape. ## Context The previous fix set `Accept: application/vnd.amazon.eventstream`. Real boto3/botocore streaming requests do not send that header. They send `X-Amzn-Bedrock-Accept: application/json`, which is the modeled Bedrock request header for the desired model response MIME type. ## Validation - `go test ./coderd/x/chatd/chatprovider -run 'TestModelFromConfig_Bedrock(StreamingHeaders\|StripsAnthropicHeaders)?$' -count=1` - `go mod tidy -diff` - `git diff --check` - pre-commit hook during `git commit`	2026-04-28 14:39:10 +02:00
dependabot[bot]	8ba894ba46	chore: bump github.com/invopop/jsonschema from 0.13.0 to 0.14.0 (#24773 ) Bumps [github.com/invopop/jsonschema](https://github.com/invopop/jsonschema) from 0.13.0 to 0.14.0. <details> <summary>Release notes</summary> <p><em>Sourced from <a href="https://github.com/invopop/jsonschema/releases">github.com/invopop/jsonschema's releases</a>.</em></p> <blockquote> <h2>v0.14.0</h2> <h2>What's Changed</h2> <ul> <li>Upgrade to golangci-lint v2 by <a href="https://github.com/samlown"><code>@samlown</code></a> in <a href="https://redirect.github.com/invopop/jsonschema/pull/187">invopop/jsonschema#187</a></li> <li>Bump minimum Go version to 1.24 by <a href="https://github.com/samlown"><code>@samlown</code></a> in <a href="https://redirect.github.com/invopop/jsonschema/pull/188">invopop/jsonschema#188</a></li> <li>Support omitzero json tags by <a href="https://github.com/YvanGuidoin"><code>@YvanGuidoin</code></a> in <a href="https://redirect.github.com/invopop/jsonschema/pull/161">invopop/jsonschema#161</a></li> <li>feat: Respect json:",string" for integer fields in generated schema by <a href="https://github.com/fengxsong"><code>@fengxsong</code></a> in <a href="https://redirect.github.com/invopop/jsonschema/pull/183">invopop/jsonschema#183</a></li> <li>Split jsonschema_extras only on unescaped commas by <a href="https://github.com/liorokman"><code>@liorokman</code></a> in <a href="https://redirect.github.com/invopop/jsonschema/pull/173">invopop/jsonschema#173</a></li> <li>Fix nil pointer dereference in ReflectFromType with ExpandedStruct (fix <a href="https://redirect.github.com/invopop/jsonschema/issues/163">#163</a>) by <a href="https://github.com/edznux-dd"><code>@edznux-dd</code></a> in <a href="https://redirect.github.com/invopop/jsonschema/pull/186">invopop/jsonschema#186</a></li> <li>Replace wk8/go-ordered-map with pb33f/ordered-map by <a href="https://github.com/samlown"><code>@samlown</code></a> in <a href="https://redirect.github.com/invopop/jsonschema/pull/189">invopop/jsonschema#189</a></li> </ul> <h2>New Contributors</h2> <ul> <li><a href="https://github.com/YvanGuidoin"><code>@YvanGuidoin</code></a> made their first contribution in <a href="https://redirect.github.com/invopop/jsonschema/pull/161">invopop/jsonschema#161</a></li> <li><a href="https://github.com/fengxsong"><code>@fengxsong</code></a> made their first contribution in <a href="https://redirect.github.com/invopop/jsonschema/pull/183">invopop/jsonschema#183</a></li> <li><a href="https://github.com/liorokman"><code>@liorokman</code></a> made their first contribution in <a href="https://redirect.github.com/invopop/jsonschema/pull/173">invopop/jsonschema#173</a></li> <li><a href="https://github.com/edznux-dd"><code>@edznux-dd</code></a> made their first contribution in <a href="https://redirect.github.com/invopop/jsonschema/pull/186">invopop/jsonschema#186</a></li> </ul> <p><strong>Full Changelog</strong>: <a href="https://github.com/invopop/jsonschema/compare/v0.13.0...v0.14.0">https://github.com/invopop/jsonschema/compare/v0.13.0...v0.14.0</a></p> </blockquote> </details> <details> <summary>Commits</summary> <ul> <li><a href="https://github.com/invopop/jsonschema/commit/2c57d6074bf9004aaaf1fc9c07ff0ea730b23de7"><code>2c57d60</code></a> Merge pull request <a href="https://redirect.github.com/invopop/jsonschema/issues/189">#189</a> from invopop/replace-wk8-with-pb33f-ordered-map</li> <li><a href="https://github.com/invopop/jsonschema/commit/d8cc8ebd57b811474861dd25409560271f084128"><code>d8cc8eb</code></a> Replace wk8/go-ordered-map with pb33f/ordered-map</li> <li><a href="https://github.com/invopop/jsonschema/commit/0d5bd753ec797ec5366a2145bf8252bff5f6406f"><code>0d5bd75</code></a> Merge pull request <a href="https://redirect.github.com/invopop/jsonschema/issues/186">#186</a> from edznux-dd/fix/expanded-struct-nil-deref</li> <li><a href="https://github.com/invopop/jsonschema/commit/3d693733ab7bca092e8604299fb82ecb573b6b10"><code>3d69373</code></a> Merge pull request <a href="https://redirect.github.com/invopop/jsonschema/issues/173">#173</a> from liorokman/escape-extras-tags</li> <li><a href="https://github.com/invopop/jsonschema/commit/b43264d2a5a9b129a943a1603d5d9df80f705b1f"><code>b43264d</code></a> Silence revive unused-parameter on fuzz callback</li> <li><a href="https://github.com/invopop/jsonschema/commit/7b21bb5bcefbed61748f2ac0388ccfc5a07ce928"><code>7b21bb5</code></a> Merge remote-tracking branch 'origin/main' into pr-186-expanded-struct</li> <li><a href="https://github.com/invopop/jsonschema/commit/048739859f24dff300c94b8b2a75f17cb8f94c4c"><code>0487398</code></a> Fix ExtraWithComman typo in test struct field</li> <li><a href="https://github.com/invopop/jsonschema/commit/bc932369a8e17ddd0028658e1be49e35d6a748b5"><code>bc93236</code></a> Merge remote-tracking branch 'origin/main' into pr-173-escape-extras</li> <li><a href="https://github.com/invopop/jsonschema/commit/d39f13c8fc27de49b934bd043f64e2f3284c920b"><code>d39f13c</code></a> Merge pull request <a href="https://redirect.github.com/invopop/jsonschema/issues/183">#183</a> from fengxsong/feat/reflect-json-string-for-integers</li> <li><a href="https://github.com/invopop/jsonschema/commit/f2e2b913ec19ef878325e6ee1b78eb2dbcea26bb"><code>f2e2b91</code></a> Extend json:",string" support to number and boolean fields</li> <li>Additional commits viewable in <a href="https://github.com/invopop/jsonschema/compare/v0.13.0...v0.14.0">compare view</a></li> </ul> </details> <br /> [![Dependabot compatibility score](https://dependabot-badges.githubapp.com/badges/compatibility_score?dependency-name=github.com/invopop/jsonschema&package-manager=go_modules&previous-version=0.13.0&new-version=0.14.0)](https://docs.github.com/en/github/managing-security-vulnerabilities/about-dependabot-security-updates#about-compatibility-scores) Dependabot will resolve any conflicts with this PR as long as you don't alter it yourself. You can also trigger a rebase manually by commenting `@dependabot rebase`. [//]: # (dependabot-automerge-start) [//]: # (dependabot-automerge-end) --- <details> <summary>Dependabot commands and options</summary> <br /> You can trigger Dependabot actions by commenting on this PR: - `@dependabot rebase` will rebase this PR - `@dependabot recreate` will recreate this PR, overwriting any edits that have been made to it - `@dependabot show <dependency name> ignore conditions` will show all of the ignore conditions of the specified dependency - `@dependabot ignore this major version` will close this PR and stop Dependabot creating any more for this major version (unless you reopen the PR or upgrade to it yourself) - `@dependabot ignore this minor version` will close this PR and stop Dependabot creating any more for this minor version (unless you reopen the PR or upgrade to it yourself) - `@dependabot ignore this dependency` will close this PR and stop Dependabot creating any more for this dependency (unless you reopen the PR or upgrade to it yourself) </details> Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>	2026-04-28 11:52:56 +00:00
Jakub Domeracki	1c70c9638d	docs: document terminal command confirmation dialog (#24771 ) Documents the breaking change from #24650 and #24765 in the [Custom Commands](https://coder.com/docs/user-guides/workspace-access/web-terminal#custom-commands) section. - `?command=` URLs now show a confirmation dialog before executing. - Template-configured `coder_app` commands bypass the dialog via `?app=`. > 🤖 Generated by Coder Agents	2026-04-28 13:50:04 +02:00
Michael Suchacz	dec3e98e54	fix: set Bedrock streaming accept headers (#24776 ) > Mux is working on behalf of Mike. ## Summary - Bump `github.com/coder/anthropic-sdk-go` to the clean Bedrock streaming header fix from coder/anthropic-sdk-go#10. - Add chatd regression coverage that verifies Bedrock streaming requests use AWS event stream headers and include `X-Amzn-Bedrock-Accept` in the SigV4 signed headers. ## SDK follow-up - Reverted the bad coder/anthropic-sdk-go#8 merge with coder/anthropic-sdk-go#9. - Re-applied only the intended Bedrock streaming header change in coder/anthropic-sdk-go#10. ## Validation - `go test ./coderd/x/chatd/chatprovider -run 'TestModelFromConfig_Bedrock(StreamingHeaders\|StripsAnthropicHeaders)?$' -count=1` - `go test ./coderd/x/chatd/chatprovider -count=1` - `go mod tidy -diff` - `make lint` - pre-commit hook during `git commit`	2026-04-28 11:28:20 +00:00
dependabot[bot]	411dc1ca8e	chore: bump github.com/aws/smithy-go from 1.24.2 to 1.25.1 (#24775 ) Bumps [github.com/aws/smithy-go](https://github.com/aws/smithy-go) from 1.24.2 to 1.25.1. <details> <summary>Release notes</summary> <p><em>Sourced from <a href="https://github.com/aws/smithy-go/releases">github.com/aws/smithy-go's releases</a>.</em></p> <blockquote> <h2>v1.25.0</h2> <h1>Release (2026-04-15)</h1> <h2>General Highlights</h2> <ul> <li><strong>Dependency Update</strong>: Updated to the latest SDK module versions</li> </ul> <h2>Module Highlights</h2> <ul> <li><code>github.com/aws/smithy-go</code>: v1.25.0 <ul> <li><strong>Feature</strong>: Add support for endpointBdd trait</li> </ul> </li> </ul> </blockquote> </details> <details> <summary>Changelog</summary> <p><em>Sourced from <a href="https://github.com/aws/smithy-go/blob/main/CHANGELOG.md">github.com/aws/smithy-go's changelog</a>.</em></p> <blockquote> <h1>Release (2026-04-23)</h1> <h2>General Highlights</h2> <ul> <li><strong>Dependency Update</strong>: Updated to the latest SDK module versions</li> </ul> <h2>Module Highlights</h2> <ul> <li><code>github.com/aws/smithy-go</code>: v1.25.1 <ul> <li><strong>Bug Fix</strong>: Fixed a memory leak in the LRU cache implementation used by some AWS services.</li> </ul> </li> </ul> <h1>Release (2026-04-15)</h1> <h2>General Highlights</h2> <ul> <li><strong>Dependency Update</strong>: Updated to the latest SDK module versions</li> </ul> <h2>Module Highlights</h2> <ul> <li><code>github.com/aws/smithy-go</code>: v1.25.0 <ul> <li><strong>Feature</strong>: Add support for endpointBdd trait</li> </ul> </li> </ul> <h1>Release (2026-04-02)</h1> <h2>General Highlights</h2> <ul> <li><strong>Dependency Update</strong>: Updated to the latest SDK module versions</li> </ul> <h2>Module Highlights</h2> <ul> <li><code>github.com/aws/smithy-go</code>: v1.24.3 <ul> <li><strong>Bug Fix</strong>: Add additional sigv4 configuration.</li> </ul> </li> <li><code>github.com/aws/smithy-go/aws-http-auth</code>: <a href="https://github.com/aws/smithy-go/blob/main/aws-http-auth/CHANGELOG.md#v113-2026-04-02">v1.1.3</a> <ul> <li><strong>Bug Fix</strong>: Add additional sigv4 configuration.</li> </ul> </li> </ul> <h1>Release (2026-02-27)</h1> <h2>General Highlights</h2> <ul> <li><strong>Dependency Update</strong>: Bump minimum go version to 1.24.</li> </ul> <h1>Release (2026-02-20)</h1> <h2>General Highlights</h2> <ul> <li><strong>Dependency Update</strong>: Updated to the latest SDK module versions</li> </ul> <h2>Module Highlights</h2> <ul> <li><code>github.com/aws/smithy-go</code>: v1.24.1 <ul> <li><strong>Feature</strong>: Add new middleware functions to get event stream output from middleware</li> </ul> </li> </ul> <h1>Release (2025-12-01)</h1> <h2>General Highlights</h2> <ul> <li><strong>Dependency Update</strong>: Updated to the latest SDK module versions</li> </ul> <h2>Module Highlights</h2> <ul> <li><code>github.com/aws/smithy-go</code>: v1.24.0</li> </ul> <!-- raw HTML omitted --> </blockquote> <p>... (truncated)</p> </details> <details> <summary>Commits</summary> <ul> <li><a href="https://github.com/aws/smithy-go/commit/e094f45e716e33a1b950cf8bbe804790bf87f965"><code>e094f45</code></a> Release 2026-04-23</li> <li><a href="https://github.com/aws/smithy-go/commit/214d45be3be5188c4d2fd9cf744c21f8b3dfbabc"><code>214d45b</code></a> changelog</li> <li><a href="https://github.com/aws/smithy-go/commit/3477da0b4dbf31de58ac375fe5abe5d268280824"><code>3477da0</code></a> fix lrucache memory leak on existing item put (<a href="https://redirect.github.com/aws/smithy-go/issues/652">#652</a>)</li> <li><a href="https://github.com/aws/smithy-go/commit/0d0b4d00f2430e62a790203b89fd76dceb4ae213"><code>0d0b4d0</code></a> Bump Smithy version to 1.69.0 (<a href="https://redirect.github.com/aws/smithy-go/issues/650">#650</a>)</li> <li><a href="https://github.com/aws/smithy-go/commit/be5e5ef0d73560eac9d71df7995b0eaffb9a8d71"><code>be5e5ef</code></a> check <a href="https://github.com/enum"><code>@enum</code></a> on strings for cbor (<a href="https://redirect.github.com/aws/smithy-go/issues/649">#649</a>)</li> <li><a href="https://github.com/aws/smithy-go/commit/5beb80e9da6bcad40dc304f062c27d8269abd67d"><code>5beb80e</code></a> Ensure javadoc uses utf-8 (<a href="https://redirect.github.com/aws/smithy-go/issues/648">#648</a>)</li> <li><a href="https://github.com/aws/smithy-go/commit/73bb8a7d6e222332d46eec7209ba3cd0ba520239"><code>73bb8a7</code></a> Release 2026-04-15</li> <li><a href="https://github.com/aws/smithy-go/commit/f056c6fb0b43ba9bfeca6c29c8c1e1046437e45e"><code>f056c6f</code></a> Changelog</li> <li><a href="https://github.com/aws/smithy-go/commit/ee36afc3d70050ba990c8de8d65043ac11d1f9f4"><code>ee36afc</code></a> Implement BDD generator for <a href="https://github.com/endpointBdd"><code>@endpointBdd</code></a> Smithy trait (<a href="https://redirect.github.com/aws/smithy-go/issues/647">#647</a>)</li> <li><a href="https://github.com/aws/smithy-go/commit/3dbea7015f5ed79312e2a3cb6bbf39f7a26e46ea"><code>3dbea70</code></a> Release 2026-04-02</li> <li>Additional commits viewable in <a href="https://github.com/aws/smithy-go/compare/v1.24.2...v1.25.1">compare view</a></li> </ul> </details> <br /> [![Dependabot compatibility score](https://dependabot-badges.githubapp.com/badges/compatibility_score?dependency-name=github.com/aws/smithy-go&package-manager=go_modules&previous-version=1.24.2&new-version=1.25.1)](https://docs.github.com/en/github/managing-security-vulnerabilities/about-dependabot-security-updates#about-compatibility-scores) Dependabot will resolve any conflicts with this PR as long as you don't alter it yourself. You can also trigger a rebase manually by commenting `@dependabot rebase`. [//]: # (dependabot-automerge-start) [//]: # (dependabot-automerge-end) --- <details> <summary>Dependabot commands and options</summary> <br /> You can trigger Dependabot actions by commenting on this PR: - `@dependabot rebase` will rebase this PR - `@dependabot recreate` will recreate this PR, overwriting any edits that have been made to it - `@dependabot show <dependency name> ignore conditions` will show all of the ignore conditions of the specified dependency - `@dependabot ignore this major version` will close this PR and stop Dependabot creating any more for this major version (unless you reopen the PR or upgrade to it yourself) - `@dependabot ignore this minor version` will close this PR and stop Dependabot creating any more for this minor version (unless you reopen the PR or upgrade to it yourself) - `@dependabot ignore this dependency` will close this PR and stop Dependabot creating any more for this dependency (unless you reopen the PR or upgrade to it yourself) </details> Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>	2026-04-28 11:18:10 +00:00

1 2 3 4 5 ...

14028 Commits