From 06bad73df41861102120b0cf075aff0b58c7bee3 Mon Sep 17 00:00:00 2001 From: Thomas Kosiewski Date: Thu, 30 Apr 2026 14:53:08 +0200 Subject: [PATCH] feat: add admin-configurable advisor API, SDK, and queries (#24621) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## 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` ---
πŸ“‹ Implementation Plan (shared across the advisor stack) # 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
Mux reference and current chatd seams **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.
### 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 `` 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
--- _Generated with [`mux`](https://github.com/coder/mux) β€’ Model: `anthropic:claude-opus-4-7` β€’ Thinking: `max`_ --- coderd/coderd.go | 2 + coderd/database/dbauthz/dbauthz.go | 18 ++ coderd/database/dbauthz/dbauthz_test.go | 8 + coderd/database/dbmetrics/querymetrics.go | 16 ++ coderd/database/dbmock/dbmock.go | 29 ++ coderd/database/querier.go | 9 + coderd/database/queries.sql.go | 29 ++ coderd/database/queries/siteconfig.sql | 15 ++ coderd/exp_chats.go | 100 +++++++ coderd/exp_chats_test.go | 315 ++++++++++++++++++++++ codersdk/chats.go | 57 ++++ scripts/dbgen/main.go | 63 +++++ site/src/api/typesGenerated.ts | 75 ++++++ 13 files changed, 736 insertions(+) diff --git a/coderd/coderd.go b/coderd/coderd.go index c0b9b0d3ce..dfa2d055b6 100644 --- a/coderd/coderd.go +++ b/coderd/coderd.go @@ -1192,6 +1192,8 @@ func New(options *Options) *API { r.Put("/debug-logging", api.putChatDebugLogging) r.Get("/user-debug-logging", api.getUserChatDebugLogging) r.Put("/user-debug-logging", api.putUserChatDebugLogging) + r.Get("/advisor", api.getChatAdvisorConfig) + r.Put("/advisor", api.putChatAdvisorConfig) r.Get("/user-prompt", api.getUserChatCustomPrompt) r.Put("/user-prompt", api.putUserChatCustomPrompt) r.Get("/user-compaction-thresholds", api.getUserChatCompactionThresholds) diff --git a/coderd/database/dbauthz/dbauthz.go b/coderd/database/dbauthz/dbauthz.go index 19acd82638..7cf4bf227b 100644 --- a/coderd/database/dbauthz/dbauthz.go +++ b/coderd/database/dbauthz/dbauthz.go @@ -2573,6 +2573,17 @@ func (q *querier) GetAuthorizationUserRoles(ctx context.Context, userID uuid.UUI return q.db.GetAuthorizationUserRoles(ctx, userID) } +func (q *querier) GetChatAdvisorConfig(ctx context.Context) (string, error) { + // The advisor configuration is a deployment-wide setting read by any + // authenticated chat user and by chatd when deciding whether to attach + // advisor behavior. We only require that an explicit actor is present + // in the context so unauthenticated calls fail closed. + if _, ok := ActorFromContext(ctx); !ok { + return "", ErrNoActor + } + return q.db.GetChatAdvisorConfig(ctx) +} + func (q *querier) GetChatAutoArchiveDays(ctx context.Context, defaultAutoArchiveDays int32) (int32, error) { // Chat auto-archive is a deployment-wide config read by dbpurge. // Only requires a valid actor in context. The HTTP GET handler @@ -7405,6 +7416,13 @@ func (q *querier) UpsertBoundaryUsageStats(ctx context.Context, arg database.Ups return q.db.UpsertBoundaryUsageStats(ctx, arg) } +func (q *querier) UpsertChatAdvisorConfig(ctx context.Context, value string) error { + if err := q.authorizeContext(ctx, policy.ActionUpdate, rbac.ResourceDeploymentConfig); err != nil { + return err + } + return q.db.UpsertChatAdvisorConfig(ctx, value) +} + func (q *querier) UpsertChatAutoArchiveDays(ctx context.Context, autoArchiveDays int32) error { if err := q.authorizeContext(ctx, policy.ActionUpdate, rbac.ResourceDeploymentConfig); err != nil { return err diff --git a/coderd/database/dbauthz/dbauthz_test.go b/coderd/database/dbauthz/dbauthz_test.go index 85e5d3fef7..db39439930 100644 --- a/coderd/database/dbauthz/dbauthz_test.go +++ b/coderd/database/dbauthz/dbauthz_test.go @@ -568,6 +568,14 @@ func (s *MethodTestSuite) TestChats() { dbm.EXPECT().UpsertChatDebugLoggingAllowUsers(gomock.Any(), true).Return(nil).AnyTimes() check.Args(true).Asserts(rbac.ResourceDeploymentConfig, policy.ActionUpdate) })) + s.Run("GetChatAdvisorConfig", s.Mocked(func(dbm *dbmock.MockStore, _ *gofakeit.Faker, check *expects) { + dbm.EXPECT().GetChatAdvisorConfig(gomock.Any()).Return("{}", nil).AnyTimes() + check.Args().Asserts().Returns("{}") + })) + s.Run("UpsertChatAdvisorConfig", s.Mocked(func(dbm *dbmock.MockStore, _ *gofakeit.Faker, check *expects) { + dbm.EXPECT().UpsertChatAdvisorConfig(gomock.Any(), "{}").Return(nil).AnyTimes() + check.Args("{}").Asserts(rbac.ResourceDeploymentConfig, policy.ActionUpdate) + })) s.Run("GetChatByID", s.Mocked(func(dbm *dbmock.MockStore, faker *gofakeit.Faker, check *expects) { chat := testutil.Fake(s.T(), faker, database.Chat{}) dbm.EXPECT().GetChatByID(gomock.Any(), chat.ID).Return(chat, nil).AnyTimes() diff --git a/coderd/database/dbmetrics/querymetrics.go b/coderd/database/dbmetrics/querymetrics.go index aa5017e912..58dbfa3a87 100644 --- a/coderd/database/dbmetrics/querymetrics.go +++ b/coderd/database/dbmetrics/querymetrics.go @@ -1120,6 +1120,14 @@ func (m queryMetricsStore) GetAuthorizationUserRoles(ctx context.Context, userID return r0, r1 } +func (m queryMetricsStore) GetChatAdvisorConfig(ctx context.Context) (string, error) { + start := time.Now() + r0, r1 := m.s.GetChatAdvisorConfig(ctx) + m.queryLatencies.WithLabelValues("GetChatAdvisorConfig").Observe(time.Since(start).Seconds()) + m.queryCounts.WithLabelValues(httpmw.ExtractHTTPRoute(ctx), httpmw.ExtractHTTPMethod(ctx), "GetChatAdvisorConfig").Inc() + return r0, r1 +} + func (m queryMetricsStore) GetChatAutoArchiveDays(ctx context.Context, defaultAutoArchiveDays int32) (int32, error) { start := time.Now() r0, r1 := m.s.GetChatAutoArchiveDays(ctx, defaultAutoArchiveDays) @@ -5296,6 +5304,14 @@ func (m queryMetricsStore) UpsertBoundaryUsageStats(ctx context.Context, arg dat return r0, r1 } +func (m queryMetricsStore) UpsertChatAdvisorConfig(ctx context.Context, value string) error { + start := time.Now() + r0 := m.s.UpsertChatAdvisorConfig(ctx, value) + m.queryLatencies.WithLabelValues("UpsertChatAdvisorConfig").Observe(time.Since(start).Seconds()) + m.queryCounts.WithLabelValues(httpmw.ExtractHTTPRoute(ctx), httpmw.ExtractHTTPMethod(ctx), "UpsertChatAdvisorConfig").Inc() + return r0 +} + func (m queryMetricsStore) UpsertChatAutoArchiveDays(ctx context.Context, autoArchiveDays int32) error { start := time.Now() r0 := m.s.UpsertChatAutoArchiveDays(ctx, autoArchiveDays) diff --git a/coderd/database/dbmock/dbmock.go b/coderd/database/dbmock/dbmock.go index a218b84a41..baa9a4ab93 100644 --- a/coderd/database/dbmock/dbmock.go +++ b/coderd/database/dbmock/dbmock.go @@ -2056,6 +2056,21 @@ func (mr *MockStoreMockRecorder) GetAuthorizedWorkspacesAndAgentsByOwnerID(ctx, return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetAuthorizedWorkspacesAndAgentsByOwnerID", reflect.TypeOf((*MockStore)(nil).GetAuthorizedWorkspacesAndAgentsByOwnerID), ctx, ownerID, prepared) } +// GetChatAdvisorConfig mocks base method. +func (m *MockStore) GetChatAdvisorConfig(ctx context.Context) (string, error) { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "GetChatAdvisorConfig", ctx) + ret0, _ := ret[0].(string) + ret1, _ := ret[1].(error) + return ret0, ret1 +} + +// GetChatAdvisorConfig indicates an expected call of GetChatAdvisorConfig. +func (mr *MockStoreMockRecorder) GetChatAdvisorConfig(ctx any) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetChatAdvisorConfig", reflect.TypeOf((*MockStore)(nil).GetChatAdvisorConfig), ctx) +} + // GetChatAutoArchiveDays mocks base method. func (m *MockStore) GetChatAutoArchiveDays(ctx context.Context, defaultAutoArchiveDays int32) (int32, error) { m.ctrl.T.Helper() @@ -9951,6 +9966,20 @@ func (mr *MockStoreMockRecorder) UpsertBoundaryUsageStats(ctx, arg any) *gomock. return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "UpsertBoundaryUsageStats", reflect.TypeOf((*MockStore)(nil).UpsertBoundaryUsageStats), ctx, arg) } +// UpsertChatAdvisorConfig mocks base method. +func (m *MockStore) UpsertChatAdvisorConfig(ctx context.Context, value string) error { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "UpsertChatAdvisorConfig", ctx, value) + ret0, _ := ret[0].(error) + return ret0 +} + +// UpsertChatAdvisorConfig indicates an expected call of UpsertChatAdvisorConfig. +func (mr *MockStoreMockRecorder) UpsertChatAdvisorConfig(ctx, value any) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "UpsertChatAdvisorConfig", reflect.TypeOf((*MockStore)(nil).UpsertChatAdvisorConfig), ctx, value) +} + // UpsertChatAutoArchiveDays mocks base method. func (m *MockStore) UpsertChatAutoArchiveDays(ctx context.Context, autoArchiveDays int32) error { m.ctrl.T.Helper() diff --git a/coderd/database/querier.go b/coderd/database/querier.go index 84c142e4de..cd28f742fd 100644 --- a/coderd/database/querier.go +++ b/coderd/database/querier.go @@ -273,6 +273,11 @@ type sqlcQuerier interface { // This function returns roles for authorization purposes. Implied member roles // are included. GetAuthorizationUserRoles(ctx context.Context, userID uuid.UUID) (GetAuthorizationUserRolesRow, error) + // GetChatAdvisorConfig returns the deployment-wide runtime configuration + // for the experimental chat advisor as a JSON blob. Callers unmarshal the + // result into codersdk.AdvisorConfig. Returns '{}' when unset so zero + // values apply by default. + GetChatAdvisorConfig(ctx context.Context) (string, error) // Auto-archive window in days. 0 disables. GetChatAutoArchiveDays(ctx context.Context, defaultAutoArchiveDays int32) (int32, error) GetChatByID(ctx context.Context, id uuid.UUID) (Chat, error) @@ -1183,6 +1188,10 @@ type sqlcQuerier interface { // cumulative values for unique counts (accurate period totals). Request counts // are always deltas, accumulated in DB. Returns true if insert, false if update. UpsertBoundaryUsageStats(ctx context.Context, arg UpsertBoundaryUsageStatsParams) (bool, error) + // UpsertChatAdvisorConfig stores the deployment-wide runtime configuration + // for the experimental chat advisor. Callers marshal codersdk.AdvisorConfig + // to JSON before invoking this query. + UpsertChatAdvisorConfig(ctx context.Context, value string) error UpsertChatAutoArchiveDays(ctx context.Context, autoArchiveDays int32) error // UpsertChatDebugLoggingAllowUsers updates the runtime admin setting that // allows users to opt into chat debug logging. diff --git a/coderd/database/queries.sql.go b/coderd/database/queries.sql.go index 14546f09e1..e3f652c8eb 100644 --- a/coderd/database/queries.sql.go +++ b/coderd/database/queries.sql.go @@ -20536,6 +20536,22 @@ func (q *sqlQuerier) GetApplicationName(ctx context.Context) (string, error) { return value, err } +const getChatAdvisorConfig = `-- name: GetChatAdvisorConfig :one +SELECT + COALESCE((SELECT value FROM site_configs WHERE key = 'agents_advisor_config'), '{}') :: text AS advisor_config +` + +// GetChatAdvisorConfig returns the deployment-wide runtime configuration +// for the experimental chat advisor as a JSON blob. Callers unmarshal the +// result into codersdk.AdvisorConfig. Returns '{}' when unset so zero +// values apply by default. +func (q *sqlQuerier) GetChatAdvisorConfig(ctx context.Context) (string, error) { + row := q.db.QueryRowContext(ctx, getChatAdvisorConfig) + var advisor_config string + err := row.Scan(&advisor_config) + return advisor_config, err +} + const getChatAutoArchiveDays = `-- name: GetChatAutoArchiveDays :one SELECT COALESCE( (SELECT value::integer FROM site_configs @@ -20914,6 +20930,19 @@ func (q *sqlQuerier) UpsertApplicationName(ctx context.Context, value string) er return err } +const upsertChatAdvisorConfig = `-- name: UpsertChatAdvisorConfig :exec +INSERT INTO site_configs (key, value) VALUES ('agents_advisor_config', $1) +ON CONFLICT (key) DO UPDATE SET value = $1 WHERE site_configs.key = 'agents_advisor_config' +` + +// UpsertChatAdvisorConfig stores the deployment-wide runtime configuration +// for the experimental chat advisor. Callers marshal codersdk.AdvisorConfig +// to JSON before invoking this query. +func (q *sqlQuerier) UpsertChatAdvisorConfig(ctx context.Context, value string) error { + _, err := q.db.ExecContext(ctx, upsertChatAdvisorConfig, value) + return err +} + const upsertChatAutoArchiveDays = `-- name: UpsertChatAutoArchiveDays :exec INSERT INTO site_configs (key, value) VALUES ('agents_chat_auto_archive_days', CAST($1 AS integer)::text) diff --git a/coderd/database/queries/siteconfig.sql b/coderd/database/queries/siteconfig.sql index 0a02ace3a8..2001b910e3 100644 --- a/coderd/database/queries/siteconfig.sql +++ b/coderd/database/queries/siteconfig.sql @@ -203,6 +203,21 @@ SET value = CASE END WHERE site_configs.key = 'agents_desktop_enabled'; +-- GetChatAdvisorConfig returns the deployment-wide runtime configuration +-- for the experimental chat advisor as a JSON blob. Callers unmarshal the +-- result into codersdk.AdvisorConfig. Returns '{}' when unset so zero +-- values apply by default. +-- name: GetChatAdvisorConfig :one +SELECT + COALESCE((SELECT value FROM site_configs WHERE key = 'agents_advisor_config'), '{}') :: text AS advisor_config; + +-- UpsertChatAdvisorConfig stores the deployment-wide runtime configuration +-- for the experimental chat advisor. Callers marshal codersdk.AdvisorConfig +-- to JSON before invoking this query. +-- name: UpsertChatAdvisorConfig :exec +INSERT INTO site_configs (key, value) VALUES ('agents_advisor_config', $1) +ON CONFLICT (key) DO UPDATE SET value = $1 WHERE site_configs.key = 'agents_advisor_config'; + -- GetChatDebugLoggingAllowUsers returns the runtime admin setting that -- allows users to opt into chat debug logging when the deployment does -- not already force debug logging on globally. diff --git a/coderd/exp_chats.go b/coderd/exp_chats.go index 98e36320f1..6ac231359a 100644 --- a/coderd/exp_chats.go +++ b/coderd/exp_chats.go @@ -4218,6 +4218,106 @@ func (api *API) putUserChatDebugLogging(rw http.ResponseWriter, r *http.Request) rw.WriteHeader(http.StatusNoContent) } +// EXPERIMENTAL: this endpoint is experimental and is subject to change. +// +//nolint:revive // get-return: revive assumes get* must be a getter, but this is an HTTP handler. +func (api *API) getChatAdvisorConfig(rw http.ResponseWriter, r *http.Request) { + ctx := r.Context() + raw, err := api.Database.GetChatAdvisorConfig(ctx) + if err != nil { + httpapi.Write(ctx, rw, http.StatusInternalServerError, codersdk.Response{ + Message: "Internal error fetching advisor configuration.", + Detail: err.Error(), + }) + return + } + + var resp codersdk.AdvisorConfig + if err := json.Unmarshal([]byte(raw), &resp); err != nil { + httpapi.Write(ctx, rw, http.StatusInternalServerError, codersdk.Response{ + Message: "Stored advisor configuration is invalid.", + Detail: err.Error(), + }) + return + } + resp.MaxUsesPerRun = max(resp.MaxUsesPerRun, 0) + resp.MaxOutputTokens = max(resp.MaxOutputTokens, 0) + + httpapi.Write(ctx, rw, http.StatusOK, resp) +} + +// EXPERIMENTAL: this endpoint is experimental and is subject to change. +func (api *API) putChatAdvisorConfig(rw http.ResponseWriter, r *http.Request) { + ctx := r.Context() + if !api.Authorize(r, policy.ActionUpdate, rbac.ResourceDeploymentConfig) { + httpapi.Forbidden(rw) + return + } + + var req codersdk.UpdateAdvisorConfigRequest + if !httpapi.Read(ctx, rw, r, &req) { + return + } + if req.MaxUsesPerRun < 0 { + httpapi.Write(ctx, rw, http.StatusBadRequest, codersdk.Response{ + Message: fmt.Sprintf("max_uses_per_run %d must be non-negative.", req.MaxUsesPerRun), + }) + return + } + if req.MaxOutputTokens < 0 { + httpapi.Write(ctx, rw, http.StatusBadRequest, codersdk.Response{ + Message: fmt.Sprintf("max_output_tokens %d must be non-negative.", req.MaxOutputTokens), + }) + return + } + switch req.ReasoningEffort { + case "", "low", "medium", "high": + default: + httpapi.Write(ctx, rw, http.StatusBadRequest, codersdk.Response{ + Message: fmt.Sprintf(`reasoning_effort %q is not valid; must be one of "", "low", "medium", or "high".`, req.ReasoningEffort), + }) + return + } + if req.ModelConfigID != uuid.Nil { + // Use system context because GetChatModelConfigByID requires + // deployment-config read access, which can be broader than the + // handler's explicit update check. The lookup only validates that + // the referenced model exists before persisting deployment config. + //nolint:gocritic // This admin-authorized validation lookup intentionally bypasses read authz. + if _, err := api.Database.GetChatModelConfigByID(dbauthz.AsSystemRestricted(ctx), req.ModelConfigID); err != nil { + if errors.Is(err, sql.ErrNoRows) || httpapi.Is404Error(err) { + httpapi.Write(ctx, rw, http.StatusBadRequest, codersdk.Response{ + Message: fmt.Sprintf("model_config_id %q does not match any existing model config.", req.ModelConfigID), + }) + return + } + httpapi.Write(ctx, rw, http.StatusInternalServerError, codersdk.Response{ + Message: "Internal error validating advisor model config.", + Detail: err.Error(), + }) + return + } + } + + raw, err := json.Marshal(req) + if err != nil { + httpapi.Write(ctx, rw, http.StatusInternalServerError, codersdk.Response{ + Message: "Internal error encoding advisor configuration.", + Detail: err.Error(), + }) + return + } + if err := api.Database.UpsertChatAdvisorConfig(ctx, string(raw)); err != nil { + httpapi.Write(ctx, rw, http.StatusInternalServerError, codersdk.Response{ + Message: "Internal error updating advisor configuration.", + Detail: err.Error(), + }) + return + } + + rw.WriteHeader(http.StatusNoContent) +} + // EXPERIMENTAL: this endpoint is experimental and is subject to change. // //nolint:revive // get-return: revive assumes get* must be a getter, but this is an HTTP handler. diff --git a/coderd/exp_chats_test.go b/coderd/exp_chats_test.go index 3e163273d4..da382bf787 100644 --- a/coderd/exp_chats_test.go +++ b/coderd/exp_chats_test.go @@ -11141,6 +11141,321 @@ func TestChatDebugRun(t *testing.T) { }) } +func TestChatAdvisorConfig_GetDefault(t *testing.T) { + t.Parallel() + + ctx := testutil.Context(t, testutil.WaitLong) + adminClient := newChatClient(t) + coderdtest.CreateFirstUser(t, adminClient.Client) + + resp, err := adminClient.GetChatAdvisorConfig(ctx) + require.NoError(t, err) + require.Equal(t, codersdk.AdvisorConfig{}, resp) +} + +func TestChatAdvisorConfig_Update(t *testing.T) { + t.Parallel() + + ctx := testutil.Context(t, testutil.WaitLong) + adminClient := newChatClient(t) + coderdtest.CreateFirstUser(t, adminClient.Client) + + want := codersdk.AdvisorConfig{ + Enabled: true, + MaxUsesPerRun: 5, + MaxOutputTokens: 1024, + ReasoningEffort: "high", + } + + err := adminClient.UpdateChatAdvisorConfig(ctx, want) + require.NoError(t, err) + + resp, err := adminClient.GetChatAdvisorConfig(ctx) + require.NoError(t, err) + require.Equal(t, want, resp) +} + +func TestChatAdvisorConfig_MemberCannotWriteButCanRead(t *testing.T) { + t.Parallel() + + ctx := testutil.Context(t, testutil.WaitLong) + adminClient := newChatClient(t) + firstUser := coderdtest.CreateFirstUser(t, adminClient.Client) + memberClientRaw, _ := coderdtest.CreateAnotherUser(t, adminClient.Client, firstUser.OrganizationID) + memberClient := codersdk.NewExperimentalClient(memberClientRaw) + + want := codersdk.AdvisorConfig{ + Enabled: true, + MaxUsesPerRun: 2, + MaxOutputTokens: 256, + } + + err := adminClient.UpdateChatAdvisorConfig(ctx, want) + require.NoError(t, err) + + resp, err := adminClient.GetChatAdvisorConfig(ctx) + require.NoError(t, err) + require.Equal(t, want, resp) + + err = memberClient.UpdateChatAdvisorConfig(ctx, codersdk.UpdateAdvisorConfigRequest{ + Enabled: true, + }) + requireSDKError(t, err, http.StatusForbidden) + + // Members must still be able to read the advisor config: the dbauthz + // layer only requires an authenticated actor, and the GET handler has + // no RBAC check because the admin settings UI and chatd runtime are + // the planned consumers. This assertion pins that behavior so a + // future RBAC tightening is a deliberate change. + memberResp, err := memberClient.GetChatAdvisorConfig(ctx) + require.NoError(t, err) + require.Equal(t, want, memberResp) + + resp, err = adminClient.GetChatAdvisorConfig(ctx) + require.NoError(t, err) + require.Equal(t, want, resp) +} + +func TestChatAdvisorConfig_NegativeMaxUsesPerRunRejected(t *testing.T) { + t.Parallel() + + ctx := testutil.Context(t, testutil.WaitLong) + adminClient := newChatClient(t) + coderdtest.CreateFirstUser(t, adminClient.Client) + + err := adminClient.UpdateChatAdvisorConfig(ctx, codersdk.UpdateAdvisorConfigRequest{ + MaxUsesPerRun: -1, + }) + sdkErr := requireSDKError(t, err, http.StatusBadRequest) + require.Contains(t, sdkErr.Message, "max_uses_per_run") + require.Contains(t, sdkErr.Message, "-1") + require.Contains(t, sdkErr.Message, "non-negative") +} + +func TestChatAdvisorConfig_NegativeMaxOutputTokensRejected(t *testing.T) { + t.Parallel() + + ctx := testutil.Context(t, testutil.WaitLong) + adminClient := newChatClient(t) + coderdtest.CreateFirstUser(t, adminClient.Client) + + err := adminClient.UpdateChatAdvisorConfig(ctx, codersdk.UpdateAdvisorConfigRequest{ + MaxOutputTokens: -1, + }) + sdkErr := requireSDKError(t, err, http.StatusBadRequest) + require.Contains(t, sdkErr.Message, "max_output_tokens") + require.Contains(t, sdkErr.Message, "-1") + require.Contains(t, sdkErr.Message, "non-negative") +} + +func TestChatAdvisorConfig_RoundTripModelConfigID(t *testing.T) { + t.Parallel() + + ctx := testutil.Context(t, testutil.WaitLong) + adminClient := newChatClient(t) + coderdtest.CreateFirstUser(t, adminClient.Client) + + modelConfig := createChatModelConfig(t, adminClient) + + want := codersdk.AdvisorConfig{ + Enabled: true, + MaxUsesPerRun: 3, + MaxOutputTokens: 2048, + ModelConfigID: modelConfig.ID, + ReasoningEffort: "medium", + } + + err := adminClient.UpdateChatAdvisorConfig(ctx, want) + require.NoError(t, err) + + resp, err := adminClient.GetChatAdvisorConfig(ctx) + require.NoError(t, err) + require.Equal(t, want, resp) +} + +func TestChatAdvisorConfig_InvalidReasoningEffort(t *testing.T) { + t.Parallel() + + ctx := testutil.Context(t, testutil.WaitLong) + adminClient := newChatClient(t) + coderdtest.CreateFirstUser(t, adminClient.Client) + + err := adminClient.UpdateChatAdvisorConfig(ctx, codersdk.UpdateAdvisorConfigRequest{ + ReasoningEffort: "ultra", + }) + sdkErr := requireSDKError(t, err, http.StatusBadRequest) + require.Contains(t, sdkErr.Message, `reasoning_effort "ultra"`) + require.Contains(t, sdkErr.Message, "not valid") +} + +func TestChatAdvisorConfig_InvalidModelConfigID(t *testing.T) { + t.Parallel() + + ctx := testutil.Context(t, testutil.WaitLong) + adminClient := newChatClient(t) + coderdtest.CreateFirstUser(t, adminClient.Client) + + unknownID := uuid.New() + err := adminClient.UpdateChatAdvisorConfig(ctx, codersdk.UpdateAdvisorConfigRequest{ + ModelConfigID: unknownID, + }) + sdkErr := requireSDKError(t, err, http.StatusBadRequest) + require.Contains(t, sdkErr.Message, unknownID.String()) + require.Contains(t, sdkErr.Message, "does not match any existing model config") +} + +func TestChatAdvisorConfig_RoundTripZeroValues(t *testing.T) { + t.Parallel() + + ctx := testutil.Context(t, testutil.WaitLong) + adminClient := newChatClient(t) + coderdtest.CreateFirstUser(t, adminClient.Client) + + want := codersdk.AdvisorConfig{ + Enabled: true, + MaxUsesPerRun: 0, + MaxOutputTokens: 0, + } + + err := adminClient.UpdateChatAdvisorConfig(ctx, want) + require.NoError(t, err) + + resp, err := adminClient.GetChatAdvisorConfig(ctx) + require.NoError(t, err) + require.Equal(t, want, resp) +} + +// TestChatAdvisorConfig_OverwriteClearsPreviousValues pins PUT to +// full-replace semantics. A second write with zero-valued fields must +// clear every field set by a prior non-zero write, so nothing leaks if +// someone later introduces merge/patch semantics. +func TestChatAdvisorConfig_OverwriteClearsPreviousValues(t *testing.T) { + t.Parallel() + + ctx := testutil.Context(t, testutil.WaitLong) + adminClient := newChatClient(t) + coderdtest.CreateFirstUser(t, adminClient.Client) + + modelConfig := createChatModelConfig(t, adminClient) + + rich := codersdk.AdvisorConfig{ + Enabled: true, + MaxUsesPerRun: 5, + MaxOutputTokens: 1024, + ModelConfigID: modelConfig.ID, + ReasoningEffort: "high", + } + err := adminClient.UpdateChatAdvisorConfig(ctx, rich) + require.NoError(t, err) + + sparse := codersdk.AdvisorConfig{Enabled: true} + err = adminClient.UpdateChatAdvisorConfig(ctx, sparse) + require.NoError(t, err) + + resp, err := adminClient.GetChatAdvisorConfig(ctx) + require.NoError(t, err) + require.Equal(t, sparse, resp) +} + +// TestChatAdvisorConfig_CanBeDisabledAfterEnabled pins the feature +// gate's "off" path. The downstream runtime gates the advisor tool and +// prompt guidance on Enabled, so a regression that silently drops or +// ignores Enabled: false on PUT would leave the feature stuck on. +func TestChatAdvisorConfig_CanBeDisabledAfterEnabled(t *testing.T) { + t.Parallel() + + ctx := testutil.Context(t, testutil.WaitLong) + adminClient := newChatClient(t) + coderdtest.CreateFirstUser(t, adminClient.Client) + + err := adminClient.UpdateChatAdvisorConfig(ctx, codersdk.AdvisorConfig{ + Enabled: true, + MaxUsesPerRun: 2, + }) + require.NoError(t, err) + + enabledResp, err := adminClient.GetChatAdvisorConfig(ctx) + require.NoError(t, err) + require.True(t, enabledResp.Enabled) + + err = adminClient.UpdateChatAdvisorConfig(ctx, codersdk.AdvisorConfig{ + Enabled: false, + }) + require.NoError(t, err) + + disabledResp, err := adminClient.GetChatAdvisorConfig(ctx) + require.NoError(t, err) + require.False(t, disabledResp.Enabled) +} + +func TestChatAdvisorConfig_ClampsNegativeStoredValues(t *testing.T) { + t.Parallel() + + ctx := testutil.Context(t, testutil.WaitLong) + adminClient, db := newChatClientWithDatabase(t) + coderdtest.CreateFirstUser(t, adminClient.Client) + + stored := `{"enabled":true,"max_uses_per_run":-3,"max_output_tokens":-99}` + err := db.UpsertChatAdvisorConfig(dbauthz.AsSystemRestricted(ctx), stored) + require.NoError(t, err) + + resp, err := adminClient.GetChatAdvisorConfig(ctx) + require.NoError(t, err) + require.Equal(t, codersdk.AdvisorConfig{ + Enabled: true, + MaxUsesPerRun: 0, + MaxOutputTokens: 0, + }, resp) + + raw, err := db.GetChatAdvisorConfig(dbauthz.AsSystemRestricted(ctx)) + require.NoError(t, err) + require.JSONEq(t, stored, raw) +} + +// TestChatAdvisorConfig_CorruptStoredJSONReturnsError pins that the GET +// handler surfaces a 500 when the stored site_configs row contains bytes +// that are not valid JSON. Unlike the neighboring chat config endpoints, +// this handler unmarshals the raw string server-side, so DB corruption +// must not present as a default-valued 200. +func TestChatAdvisorConfig_CorruptStoredJSONReturnsError(t *testing.T) { + t.Parallel() + + ctx := testutil.Context(t, testutil.WaitLong) + adminClient, db := newChatClientWithDatabase(t) + coderdtest.CreateFirstUser(t, adminClient.Client) + + err := db.UpsertChatAdvisorConfig(dbauthz.AsSystemRestricted(ctx), "not-json") + require.NoError(t, err) + + _, err = adminClient.GetChatAdvisorConfig(ctx) + sdkErr := requireSDKError(t, err, http.StatusInternalServerError) + require.Contains(t, sdkErr.Message, "invalid") +} + +// TestChatAdvisorConfig_UnauthenticatedFails pins that the advisor config +// endpoints are gated by apiKeyMiddleware at the /chats route level. The +// handler itself has no auth check, so this test protects against a future +// route restructuring that would accidentally expose these settings. +func TestChatAdvisorConfig_UnauthenticatedFails(t *testing.T) { + t.Parallel() + ctx := testutil.Context(t, testutil.WaitLong) + + adminClient := newChatClient(t) + coderdtest.CreateFirstUser(t, adminClient.Client) + + anonClient := codersdk.NewExperimentalClient(codersdk.New(adminClient.URL)) + _, err := anonClient.GetChatAdvisorConfig(ctx) + var sdkErr *codersdk.Error + require.ErrorAs(t, err, &sdkErr) + require.Equal(t, http.StatusUnauthorized, sdkErr.StatusCode()) + + err = anonClient.UpdateChatAdvisorConfig(ctx, codersdk.UpdateAdvisorConfigRequest{ + Enabled: true, + }) + require.ErrorAs(t, err, &sdkErr) + require.Equal(t, http.StatusUnauthorized, sdkErr.StatusCode()) +} + func TestChatWorkspaceTTL(t *testing.T) { t.Parallel() ctx := testutil.Context(t, testutil.WaitLong) diff --git a/codersdk/chats.go b/codersdk/chats.go index 689b80bb7e..66c4d3f3b8 100644 --- a/codersdk/chats.go +++ b/codersdk/chats.go @@ -639,6 +639,36 @@ type UpdateChatDesktopEnabledRequest struct { EnableDesktop bool `json:"enable_desktop"` } +// AdvisorConfig is the deployment-wide runtime configuration for the +// experimental chat advisor. +// +// EXPERIMENTAL: this type is experimental and is subject to change. +type AdvisorConfig struct { + // Enabled toggles the advisor runtime. When false, advisor is not + // attached to new chats. + Enabled bool `json:"enabled"` + // MaxUsesPerRun caps how many times the advisor can be invoked per + // chat run. 0 means unlimited. + MaxUsesPerRun int `json:"max_uses_per_run"` + // MaxOutputTokens caps the advisor model response tokens. 0 means + // use the runtime default. + MaxOutputTokens int64 `json:"max_output_tokens"` + // ModelConfigID selects a specific chat model config to power the + // advisor. uuid.Nil means reuse the outer chat model. The runtime + // must fall back to the outer chat model when this ID cannot be + // resolved (e.g. the referenced model config was soft-deleted or + // its provider was disabled after the admin saved this config). + ModelConfigID uuid.UUID `json:"model_config_id" format:"uuid"` + // ReasoningEffort overlays provider reasoning effort on the advisor + // call config when supported. Allowed: "", "low", "medium", "high". + ReasoningEffort string `json:"reasoning_effort"` +} + +// UpdateAdvisorConfigRequest is the request body for updating advisor +// runtime configuration. It is a type alias for AdvisorConfig because +// the request and response shapes are currently identical. +type UpdateAdvisorConfigRequest = AdvisorConfig + // ChatDebugLoggingAdminSettings describes the runtime admin setting // that allows users to opt into chat debug logging. type ChatDebugLoggingAdminSettings struct { @@ -2146,6 +2176,33 @@ func (c *ExperimentalClient) UpdateChatDesktopEnabled(ctx context.Context, req U return nil } +// GetChatAdvisorConfig returns the deployment-wide advisor configuration. +func (c *ExperimentalClient) GetChatAdvisorConfig(ctx context.Context) (AdvisorConfig, error) { + res, err := c.Request(ctx, http.MethodGet, "/api/experimental/chats/config/advisor", nil) + if err != nil { + return AdvisorConfig{}, err + } + defer res.Body.Close() + if res.StatusCode != http.StatusOK { + return AdvisorConfig{}, ReadBodyAsError(res) + } + var resp AdvisorConfig + return resp, json.NewDecoder(res.Body).Decode(&resp) +} + +// UpdateChatAdvisorConfig updates the deployment-wide advisor configuration. +func (c *ExperimentalClient) UpdateChatAdvisorConfig(ctx context.Context, req UpdateAdvisorConfigRequest) error { + res, err := c.Request(ctx, http.MethodPut, "/api/experimental/chats/config/advisor", req) + if err != nil { + return err + } + defer res.Body.Close() + if res.StatusCode != http.StatusNoContent { + return ReadBodyAsError(res) + } + return nil +} + // GetChatWorkspaceTTL returns the configured chat workspace TTL. func (c *ExperimentalClient) GetChatWorkspaceTTL(ctx context.Context) (ChatWorkspaceTTLResponse, error) { res, err := c.Request(ctx, http.MethodGet, "/api/experimental/chats/config/workspace-ttl", nil) diff --git a/scripts/dbgen/main.go b/scripts/dbgen/main.go index 71fdcbbeef..265503dad5 100644 --- a/scripts/dbgen/main.go +++ b/scripts/dbgen/main.go @@ -107,6 +107,14 @@ type stubParams struct { func orderAndStubDatabaseFunctions(filePath, receiver, structName string, stub func(params stubParams) string) error { declByName := map[string]*dst.FuncDecl{} packageName := filepath.Base(filepath.Dir(filePath)) + externalMethods, err := loadExternalReceiverMethods( + filepath.Dir(filePath), + filepath.Base(filePath), + structName, + ) + if err != nil { + return xerrors.Errorf("load external receiver methods: %w", err) + } contents, err := os.ReadFile(filePath) if err != nil { @@ -149,6 +157,10 @@ func orderAndStubDatabaseFunctions(filePath, receiver, structName string, stub f } for _, fn := range funcs { + if _, ok := externalMethods[fn.Name]; ok { + continue + } + var bodyStmts []dst.Stmt decl, ok := declByName[fn.Name] @@ -316,6 +328,57 @@ func parseDBFile(filename string) (*dst.File, error) { return f, err } +func loadExternalReceiverMethods( + dirPath string, + excludeFile string, + structName string, +) (map[string]struct{}, error) { + methods := make(map[string]struct{}) + entries, err := os.ReadDir(dirPath) + if err != nil { + return nil, xerrors.Errorf("read dir %s: %w", dirPath, err) + } + + for _, entry := range entries { + name := entry.Name() + if entry.IsDir() || name == excludeFile || !strings.HasSuffix(name, ".go") || strings.HasSuffix(name, "_test.go") { + continue + } + + contents, err := os.ReadFile(filepath.Join(dirPath, name)) + if err != nil { + return nil, xerrors.Errorf("read %s: %w", name, err) + } + f, err := decorator.Parse(contents) + if err != nil { + return nil, xerrors.Errorf("parse %s: %w", name, err) + } + for _, decl := range f.Decls { + funcDecl, ok := decl.(*dst.FuncDecl) + if !ok || funcDecl.Recv == nil || len(funcDecl.Recv.List) == 0 { + continue + } + + var ident *dst.Ident + switch recv := funcDecl.Recv.List[0].Type.(type) { + case *dst.Ident: + ident = recv + case *dst.StarExpr: + ident, ok = recv.X.(*dst.Ident) + if !ok { + continue + } + } + if ident == nil || ident.Name != structName { + continue + } + methods[funcDecl.Name.Name] = struct{}{} + } + } + + return methods, nil +} + func loadInterfaceFuncs(f *dst.File, interfaceName string) ([]querierFunction, error) { var querier *dst.InterfaceType for _, decl := range f.Decls { diff --git a/site/src/api/typesGenerated.ts b/site/src/api/typesGenerated.ts index 468c24a110..7039bf0c70 100644 --- a/site/src/api/typesGenerated.ts +++ b/site/src/api/typesGenerated.ts @@ -797,6 +797,44 @@ export type Addon = "ai_governance"; export const Addons: Addon[] = ["ai_governance"]; +// From codersdk/chats.go +/** + * AdvisorConfig is the deployment-wide runtime configuration for the + * experimental chat advisor. + * + * EXPERIMENTAL: this type is experimental and is subject to change. + */ +export interface AdvisorConfig { + /** + * Enabled toggles the advisor runtime. When false, advisor is not + * attached to new chats. + */ + readonly enabled: boolean; + /** + * MaxUsesPerRun caps how many times the advisor can be invoked per + * chat run. 0 means unlimited. + */ + readonly max_uses_per_run: number; + /** + * MaxOutputTokens caps the advisor model response tokens. 0 means + * use the runtime default. + */ + readonly max_output_tokens: number; + /** + * ModelConfigID selects a specific chat model config to power the + * advisor. uuid.Nil means reuse the outer chat model. The runtime + * must fall back to the outer chat model when this ID cannot be + * resolved (e.g. the referenced model config was soft-deleted or + * its provider was disabled after the admin saved this config). + */ + readonly model_config_id: string; + /** + * ReasoningEffort overlays provider reasoning effort on the advisor + * call config when supported. Allowed: "", "low", "medium", "high". + */ + readonly reasoning_effort: string; +} + // From codersdk/workspacebuilds.go export interface AgentConnectionTiming { readonly started_at: string; @@ -7720,6 +7758,43 @@ export interface UpdateActiveTemplateVersion { readonly id: string; } +// From codersdk/chats.go +/** + * UpdateAdvisorConfigRequest is the request body for updating advisor + * runtime configuration. It is a type alias for AdvisorConfig because + * the request and response shapes are currently identical. + */ +export interface UpdateAdvisorConfigRequest { + /** + * Enabled toggles the advisor runtime. When false, advisor is not + * attached to new chats. + */ + readonly enabled: boolean; + /** + * MaxUsesPerRun caps how many times the advisor can be invoked per + * chat run. 0 means unlimited. + */ + readonly max_uses_per_run: number; + /** + * MaxOutputTokens caps the advisor model response tokens. 0 means + * use the runtime default. + */ + readonly max_output_tokens: number; + /** + * ModelConfigID selects a specific chat model config to power the + * advisor. uuid.Nil means reuse the outer chat model. The runtime + * must fall back to the outer chat model when this ID cannot be + * resolved (e.g. the referenced model config was soft-deleted or + * its provider was disabled after the admin saved this config). + */ + readonly model_config_id: string; + /** + * ReasoningEffort overlays provider reasoning effort on the advisor + * call config when supported. Allowed: "", "low", "medium", "high". + */ + readonly reasoning_effort: string; +} + // From codersdk/deployment.go export interface UpdateAppearanceConfig { readonly application_name: string;