Relates to CODAGT-432
Adds three new search filters to the chat list endpoint (`GET
/api/experimental/chats/`):
- `pr:<number>` - exact PR number match
- `repo:<owner/repo>` - substring match against git remote origin or URL
- `pr_title:<text>` - case-insensitive PR title substring match
Includes SQL filter clauses (EXISTS against `chat_diff_statuses`),
parser with validation, handler wiring, unit tests, swagger annotation
update, and a new search syntax documentation page.
> 🤖 Generated with [Coder Agents](https://coder.com/agents)
Fixes: ENG-2719
Fixes the flake in
`TestSendMessageWithModelOverrideUpdatesLastModelConfigID` (and the same
pattern in `TestSubsequentSendWithoutOverrideUsesPersistedModel`).
> Generated with [Coder Agents](https://coder.com/agents)
Since AI Gateway is now enabled by default, and if the AI Gateway Proxy is enabled too it's possible the server can start without any configured providers. This would previously block startup, which is unacceptable.
In an upstack PR we will handle reloading the providers at runtime, so the server needs to be able to start up even if it can't handle any proxy requests to AI Gateway.
This change was necessitated because if there are providers configured in the environment they need to be seeded _before_ the proxy starts.
Fixes CODAGT-311.
Users receive too many auto-archive notification emails because the
dbpurge loop runs every 10 minutes and archives chats on each tick using
timestamp-precise cutoffs, causing chats to trickle past the threshold
continuously.
Switch archive eligibility from timestamp arithmetic to date arithmetic
(UTC day boundaries). All chats whose last activity falls on the same
UTC date are now archived together on the first tick after midnight UTC,
reducing notification emails to ~at most~ probably one per day.
(Exception: if we hit the auto-archive limit)
- SQL compares `(last_activity AT TIME ZONE 'UTC')::date` against cutoff
date
- Go truncates current time to start-of-day before subtracting archive
days
- Tests verify date boundary semantics including late-activity and batch
edge cases
- Docs updated to describe UTC day boundary behavior and at-most-daily
notification cadence
> [!NOTE]
> Generated by Coder Agents
Normalize program names in shellparse.Parse to their basename.
Does not rely on filepath.Base because the server may run on either
Linux or Windows where the behavior would differ.
Closes CODAGT-470
### TL;DR
Introduces an in-process `TransportFactory` for aibridge so that chatd (coder-agent LLM traffic) can route requests through the aibridged handler without crossing the HTTP route or requiring a license entitlement check.
### What changed?
- Added a new `coderd/aibridge` package with a `TransportFactory` interface and a `Source` type for tagging the call site on request contexts. `SourceAgents` is defined as the constant for coder-agent traffic.
- Implemented `NewTransportFactory` in `coderd/aibridged/transport.go`, which returns an `http.RoundTripper` that dispatches requests to the aibridged handler in-process. The response body is streamed through an `io.Pipe` so SSE/NDJSON/chunked responses propagate token-by-token. Handler panics are recovered and surfaced as 500 responses, and context cancellation closes the pipe with the appropriate error.
- `RegisterInMemoryAIBridgedHTTPHandler` now also constructs a `TransportFactory` from the registered handler and stores it on `API.AIBridgeTransportFactory` (an `atomic.Pointer`), making it available to chatd without going through the license-gated HTTP route.
- Added `API.AIBridgeTransportFactory` as a public `atomic.Pointer[aibridge.TransportFactory]` field on `coderd.API`.
### How to test?
- `coderd/aibridged/transport_test.go` covers: transport creation, nil-handler errors, source attachment to context, header/status passthrough, streaming (SSE-style chunked writes visible before handler completion), context cancellation closing the body with an error, concurrent requests, handler panics producing 500s, and handlers that return without writing.
- `coderd/aibridge_test.go` verifies that `AIBridgeTransportFactory` starts as nil on AGPL coderd, can be stored and loaded atomically, and that the stored factory correctly dispatches requests through the stub handler.
### Why make this change?
Chatd needs to send LLM requests through aibridge in-process rather than via the external HTTP route, which is license-gated. The `TransportFactory` abstraction provides a clean seam: the entitlement check remains on the HTTP route for external callers, while in-process coder-agent traffic bypasses it through the factory. The `Source` type allows downstream handlers and logs to attribute traffic without gating behavior on the caller identity.
My agent added `//nolint:testpackage` to a test file on one of my PRs.
Again. This PR cleans it up across the entire repo and updates the
in-repo conventions so future agents stop doing it.
The repo already has a precedent for white-box tests that need to touch
unexported symbols: `*_internal_test.go` (145+ existing files). The
`testpackage` linter's default `skip-regexp` exempts that filename
suffix, so the `//nolint:testpackage` directive is unnecessary in every
case where someone reached for it. This PR renames 51 such files to
`*_internal_test.go` via `git mv` so blame and history follow, and
strips the dead directive from 2 files that were already correctly named
(`coderd/oauth2provider/authorize_internal_test.go`,
`coderd/x/chatd/advisor_internal_test.go`).
`.claude/docs/TESTING.md` now documents the rule explicitly under *Test
Package Naming*, which is imported into the root `AGENTS.md` via
`@.claude/docs/TESTING.md`. The rule: prefer `package foo_test`; if you
need internal access, rename the file to `*_internal_test.go` rather
than adding a nolint directive.
`TestWatchAgentContainers/CoderdWebSocketCanHandleClientClosing` spent
about 15 seconds waiting for the real websocket heartbeat ticker to
detect that the client closed.
Add a clock-aware `HeartbeatClose` wrapper and pass `api.Clock` through
the containers watch handler so the test can drive the heartbeat
deterministically with `quartz.Mock`. The test still verifies the same
client-close teardown path, but it advances the heartbeat tick instead
of waiting for wall-clock time.
Refs #25557
Discovered as part of the work on CODAGT-381.
In order to allow Coder Agents to use AI Gateway in OSS, we need to rehome the `aibridged`\-related code into the AGPL path.
The HTTP API is only registered under enterprise so will still require the AI Governance Add-on to be present in order to use it, whereas Coder Agents uses an in-memory pipe to the same handlers.
_Disclaimer: implemented by a Coder Agent using Claude Opus 4.7_
Part of the implementation of [RFC: Common AI Provider Configs](https://www.notion.so/coderhq/RFC-Common-AI-Provider-Configs-34bd579be59280ed958feffb82024797) (AIGOV-201).
## Note
This change can cause a previously working installation to fail to start should a conflict exist between the providers configured in the environment & those now migrated to the database.
I'll raise a PR upstack to document this process and workarounds should a startup fail.
## What this PR does
Reconciles environment-derived AI provider configuration with the `ai_providers` table at server startup. The seed runs **before** the aibridged daemon is initialized, so the runtime always reads providers from the database; the legacy `CODER_AIBRIDGE_*` environment variables become a one-shot migration source.
### Behavior
- Concurrent server starts are serialized through a Postgres advisory lock (`LockIDAIProvidersEnvSeed`).
- Missing rows are inserted with an audit entry attributed to the system actor.
- Existing rows whose canonical hash matches the env-derived hash are left alone (the common no-op restart path).
- Existing rows whose canonical hash does **not** match cause server startup to fail with a descriptive error so the operator can explicitly resolve the conflict in either env or DB.
- Soft-deleted rows are NOT resurrected from env; an explicit operator deletion is sticky across restarts.
- Indexed providers whose name conflicts with a legacy env var fail startup with a clear remediation message.
- Unknown provider types (e.g. `copilot`, until the DB enum is widened) are skipped with a log entry rather than failing startup.
### Canonical hashing
The `canonicalAIProvider` shape captures exactly the fields that determine runtime behavior — `type`, `base_url`, and the Bedrock subset of settings (access key, access key secret, region, model, small fast model) — and is hashed with SHA-256. The hash is **computed on demand from the row + env**, never persisted, so the database does not need a new column for it. API keys live in the separate `ai_provider_keys` table and are intentionally excluded from the hash so operators can rotate keys via the API without forcing a server restart.
<details>
<summary>Decision log</summary>
- The hash is intentionally not persisted in the database. The RFC discussed this trade-off; computing on demand keeps the schema minimal and lets the canonical shape evolve without a migration.
- The lock uses an `iota` slot in `coderd/database/lock.go` rather than `GenLockID` so it's stable, easy to audit, and matches the convention used for every other startup lock.
- A bearer-token Anthropic provider whose env vars also set Bedrock metadata but no AWS credentials does NOT store the Bedrock fields. Without credentials the discriminated settings would misrepresent the row as Bedrock auth.
- We deliberately do NOT publish to the `ai_providers_changed` pubsub channel from the seed because the seed completes before any subscriber is started; the follow-up PR introduces that channel.
</details>
> Mux updated this PR on behalf of Mike.
## Summary
- Add experimental personal skills API helpers and an Agents settings UI
for listing, creating, editing, deleting, and importing SKILL.md
content.
- Add docs, Storybook coverage, and unit tests for backend-compatible
SKILL.md parsing.
- Address review feedback by simplifying frontmatter scalar parsing,
clarifying the UI parser scope, defaulting personal skill queries to
`me`, and patching React Query caches after create, update, and delete.
- Merge latest `main` and resolve the Agents sidebar refactor conflicts.
## Validation
- pre-commit hook
- `go test ./codersdk/workspacesdk -run TestParseSkillFrontmatter
-count=1`
- `go test ./coderd/x/chatd/chattool -run 'Test' -count=1`
- `cd site && pnpm test --
src/pages/AgentsPage/utils/personalSkills.test.ts
src/api/queries/userSkills.test.ts src/utils/fileSize.test.ts
--runInBand`
- `cd site && pnpm lint:types`
- `cd site && pnpm lint:check`
> Mux is opening this PR on behalf of Mike.
Agents could interpret a generic "commit and push" request on `main` as
permission to commit on the current branch and push its upstream.
Add version-control safety guidance to the default agent system prompt
so agents check the current branch and push target, avoid default or
protected branch commits and pushes unless the user explicitly confirms
after a warning, avoid plain git push from those branches, and create a
feature branch first when no explicit confirmation is present.
<!--
If you have used AI to produce some or all of this PR, please ensure you have read our [AI Contribution guidelines](https://coder.com/docs/about/contributing/AI_CONTRIBUTING) before submitting.
-->
fixes https://github.com/coder/internal/issues/1541
Closing websockets can race whether they return an error or not if the remote side closes too. Dropping some test assertions about this since it is not critical to what we are testing.
Removes the coder_secret Terraform integration: the data.coder_secret
consumption path through provisionerdserver → provisioner.proto →
provisioner/terraform, the dynamic-parameter secret-requirement
validation, and the workspace-update / resolve-autostart surfaces that
depended on it. This is being done due to a product/feature direction
change (see PLAT-243). User-secret CRUD (DB, REST, CLI, UI, telemetry, audit)
and the agent-manifest secret-injection path are untouched.
The provisionerd API is bumped from v1.17 to v1.18 rather than rolled
back: v1.17 shipped in v2.33.x, so user_secrets field numbers are
reserved and the changelog documents both versions.
Generated with assistance from Coder Agents.
Fixes https://linear.app/codercom/issue/CODAGT-432
Adds structured search/filter capabilities to the `GET
/api/experimental/chats/` endpoint via the `q` query parameter. All
filters use explicit `key:value` syntax; bare terms are rejected to
reserve them for potential future full-text search.
> Generated by Coder Agents
Co-authored-by: Danielle Maywood <danielle@themaywoods.com>
Co-authored-by: Jaayden Halko <jaayden.halko@gmail.com>
Adds options matching new AI Gateway naming.
New options are added as alias for old options. Old options are still
working.
Old options have deprecated message.
No conflict detection was added.
Updated documentation so it mentions only new options. Added note about
old options still working.
> Various AI tools where used to create this PR
When the execute tool runs a chained shell command, the UI previously
rendered the raw string. Long chains like "cd /repo && git pull &&
git add . && git commit -m fix" were hard to scan.
A new ChatMessagePart.ParsedCommands [][]string field on tool-call
parts carries one entry per simple command, parsed in chatd from args
via mvdan.cc/sh/v3/syntax. The frontend renders the joined list ("cd,
git pull, git add, git commit") in place of the raw command, and falls
back to the raw command when the field is absent.
Closes CODAGT-446
The subscriber advanced a single delivery cursor on each notify and
trusted it for both lookups. Concurrent publishMessage calls and PG
NOTIFY commit ordering let cache appends and notifies arrive out of
ID order, after which a late notify would scan above its own message
and drop it. The DB fallback was also skipped whenever the cache
delivered anything, hiding cross-replica messages that only the DB
held.
The cursor becomes a high-water mark, not the lookup key. Notifies
trigger a rescan over the gap they describe and dedupe per
subscription, and the DB pass runs every time so cross-replica
messages can't get eaten by a local cache hit.
Closescoder/internal#1525
Closes CODAGT-357
> Mux updated this PR on behalf of Mike.
## Stack Context
This PR builds on #25365 in the experimental personal skills stack and
completes the chat integration.
Stack order:
1. #25362 personal skill resolver
2. #25363 storage, permissions, API, and SDK
3. #25365 API test coverage
4. #25366 chattool and chatd integration
5. #25066 settings UI and docs
6. #25386 personal skills slash menu
## What?
Updates chattool skill formatting and `read_skill` resolution so tools
can read personal skills from the database, then injects personal skill
metadata into chatd prompts and registers the skill-reading tools when
skills are available.
This branch has also been merged with current `origin/main` to resolve
merge conflicts.
## Why?
The chattool and chatd changes need to land together so the intermediate
stack state stays buildable. This completes personal skill availability
in chats without syncing personal skills into workspace filesystems.
## Validation
- `go test -count=1 ./coderd/x/chatd/chattool -run
'TestFormatResolvedSkillIndex|TestReadSkillTool|TestReadSkillFileTool'`
- `go test -count=1 ./coderd/x/chatd -run
'TestPersonalSkillsInSystemPrompt|TestPersonalAndWorkspaceSkillCollisionInSystemPrompt|TestSkillIndexRefreshReplacesStaleAliases|TestFetchPersonalSkillMetadata|TestLoadPersonalSkillBody'`
- `go test -count=1 ./coderd -run 'Test.*UserSkill'`
- `git diff --cached --check`
- `make lint`
- pre-commit hook
<!--
If you have used AI to produce some or all of this PR, please ensure you have read our [AI Contribution guidelines](https://coder.com/docs/about/contributing/AI_CONTRIBUTING) before submitting.
-->
relates to GRU-18
Adds basic implementation for Workspace Agent Connection Watch and tests.
Missing are handling of logs.
> Mux updated this PR on behalf of Mike.
## Stack Context
This PR is the API test coverage slice in the experimental personal
skills stack. The storage, schema, permissions, API, and SDK
implementation merged in #25363.
Stack order:
1. #25362 personal skill resolver
2. #25363 storage, permissions, API, and SDK
3. #25365 API test coverage
4. #25366 chattool and chatd integration
5. #25066 settings UI and docs
6. #25386 personal skills slash menu
## What?
Adds API and audit tests for personal skill CRUD, validation failures,
limits, authorization, soft-delete cleanup, and audit content tracking.
This PR is now test-only. It does not include migrations, generated
database code, or API implementation changes.
## Why?
The feature touches storage, permissions, and audit behavior. These
tests make the server behavior reviewable and protected without
re-reviewing the implementation that already merged in #25363.
## Validation
- `go test ./coderd -run '^(TestUserSkill|TestPatchUserSkill)' -count=1`
- `go test ./enterprise/coderd -run
'^TestUserSkillAuditDiffTracksContent$' -count=1`
- pre-commit hook via `gt modify --no-edit`
> Mux prepared this PR on behalf of Mike.
## Stack Context
This is PR 1 of 6 in the `mike/ai-providers` Graphite stack. The stack
migrates Agents chat provider configuration from legacy chat provider
tables to the unified AI provider tables used by the AI provider
administration surface.
See the stack comment for review order and links.
## What?
Adds a package-level `coderd/x/chatd/docs.go` glossary for AI Providers,
provider-scoped keys, user BYOK keys, and Agents as the consuming
feature area.
## Why?
Keeping the glossary next to chatd makes the migration language visible
where Agents consume AI Providers, without adding a separate PRD, root
context file, or ADR structure.
> Mux updated this PR on behalf of Mike.
## Stack Context
This PR is the storage, permissions, API, and SDK layer for experimental
personal skills. #25362 has landed on `main`, so this branch is
restacked directly on `main`.
Stack order:
1. #25363 storage, permissions, API, and SDK
2. #25365 API test coverage
3. #25366 chattool and chatd integration
4. #25066 settings UI and docs
5. #25386 personal skills slash menu
## What?
Adds the `user_skills` database table, generated queries, RBAC resources
and scopes, audit resource handling, experimental user-scoped CRUD
endpoints, SDK types, and generated API/site types.
Follow-up review and restack fixes:
- Enforce a bounded personal skill description in parser and database
constraints.
- Return `403 Forbidden` for unauthorized create and update attempts.
- Return explicit conflict responses when soft-deleted users are
targeted.
- Keep user admins out of personal skills, while site owners can read
and delete but not create or update.
- Document trigger-raised constraint names and keep schema constants
covered by tests.
- Reuse `UserSkillMetadata` in the full `UserSkill` SDK response type.
- Generate user skill IDs in Go instead of relying on a database
default.
- Rebase on latest `main` and renumber the user skills migration to
`000502_user_skills`.
## Why?
Personal skills need durable user-owned storage with owner
authorization, limited site-owner moderation, and a hidden API surface
before chatd can consume them.
## Validation
- `make gen`
- `go test ./coderd/database -run '^TestUserSkillSchemaConstants$'
-count=1`
- `go test ./coderd/database/dbauthz -run
'^TestMethodTestSuite/TestUserSkills$' -count=1`
- `go test ./coderd -run '^TestPatchUserSkill$' -count=1`
- `go test ./codersdk ./coderd/database/db2sdk`
- `make lint`
- pre-commit hook on `97fd58108d`
> Mux opened this PR on behalf of Mike.
Fixes CODAGT-451
Adds optional `model_intent` metadata to the built-in execute tool
schema so tool calls can carry a short user-facing intent label without
duplicating the command or duration.
The Agents UI now composes that intent with the existing execute command
and duration fields, displaying labels like `Checking repository state
using git fetch origin for 2.3s` while keeping the shell command visible
as the audit-relevant action.
Existing execute calls without an intent keep the previous `Ran
<command>` fallback label, so only intent-bearing calls get the new
composed label.
Add frontend API methods, mocks, and form helpers for user secrets CRUD. The new client methods cover list, get, create, update, and delete requests, including URL encoding for secret names used in route paths.
Add user secret form utilities for create and update payload construction, required create field checks, and structured API validation error mapping back to form fields. User secret name validation now lives in codersdk with tests, and coderd returns field-level validation errors for create, update, and uniqueness conflicts so the frontend can show backend-owned validation results consistently.
Advisor tool calls currently reject questions over 2000 runes, which can
leave the parent model retrying the same invalid call.
This documents the limit in the advisor tool schema and guidance, then
truncates oversized questions rune-safely before building the nested
advisor prompt.
> Mux working on behalf of Mike.
Anthropic replay can fail when stored history contains a
provider-executed tool call like `web_search` without the matching
provider-executed result. That orphaned call is incomplete
provider-internal state, so replaying it can make an otherwise usable
chat unreplayable even though there is no search result to preserve.
This fixes replay by dropping orphan provider-executed tool calls from
the model-visible prompt, preserving signed reasoning and the rest of
the assistant content, then revalidating before the request. We do not
synthesize tool results or drop reasoning. The database can retain the
historical artifact for inspection, while Anthropic only sees replayable
content.
This matches permissively licensed prior art. Vercel AI SDK
(Apache-2.0), used by mux, keeps incomplete tool state in UI/history but
omits it from model requests with `convertToModelMessages(..., {
ignoreIncompleteToolCalls: true })`. LangChain, LiteLLM, and OpenAI
Agents (MIT for the relevant open-source code) also preserve Anthropic
signed reasoning as opaque replay data. Coder applies that model-visible
replay boundary explicitly because our persisted history is already in
provider-message form.
This matches mux, is cleaner than the older idea around not persisting
the search query tool, and the model handles the repaired prompt fine.
Closes CODAGT-448
## Before
<img width="963" height="491" alt="image"
src="https://github.com/user-attachments/assets/a7788ebf-2728-4420-90cf-5e4f6905bdf7"
/>
## After
<img width="842" height="513" alt="image"
src="https://github.com/user-attachments/assets/ae39c262-7586-4e2d-b7db-1b639a7e8e15"
/>
`attach_file` was registered for plan-mode turns but never added to
`builtinPlanToolAllowed`, so the per-turn `ActiveTools` allowlist
filtered it out and calls failed with `Tool not active in this turn:
attach_file`. This was an omission rather than a deliberate block — the
tool (#24280) landed shortly after plan mode (#24236) and no subsequent
edit to the allowlist picked it up.
Add `attach_file` under the `isRootChat` case, matching how other
artifact-producing tools (`propose_plan`, `write_file`, `edit_files`)
are gated. The tool only reads from the workspace and writes to
chat-attachment storage, so it preserves plan mode's invariant of not
making implementation changes to the workspace. Subagents in plan mode
remain restricted to the minimal read-only surface.
Records reasoning start and end times on persisted reasoning
`ChatMessagePart`s so reasoning duration can be computed for stored
chats. Backend-only: no SSE changes and no frontend rendering ship in
this PR.
The `created_at` field on `ChatMessagePart` is extended to also be
present on `reasoning` parts (it previously appeared only on `tool-call`
and `tool-result`), and a new `completed_at` field is added for
`reasoning` parts.
### How timestamps are recorded
- `StreamPartTypeReasoningStart`: stamp `startedAt = dbtime.Now()` on
the active reasoning state.
- `StreamPartTypeReasoningEnd`: stamp `completedAt = dbtime.Now()` and
append both into parallel `[]time.Time` slices on `stepResult`.
- Persistence reads the slices in occurrence order (reasoning has no
provider-side ID) and applies them to the matching `ChatMessagePart` via
`buildAssistantPartsForPersist`. The first reasoning block's stamps go
onto the first reasoning part, and so on.
- `flushActiveState` flushes partial reasoning interrupted before
`StreamPartTypeReasoningEnd` with `startedAt` from the active state and
`completedAt = dbtime.Now()` at the interruption.
### Why two fields, not one?
Tool calls and results are point events. The frontend computes their
duration by subtracting the call's `created_at` from the result's
`created_at`. Reasoning is one assistant part that brackets a span, so
we record both endpoints on the part itself.
### Why not stamp in `PartFromContent`?
Same rationale as #24101: `PartFromContent` is called during both SSE
publishing and persistence. Stamping there would yield incorrect
persistence-time timestamps for reasoning blocks that finished much
earlier in the step. Instead we capture in the chatloop and apply during
persistence.
<details><summary>Implementation plan</summary>
- `codersdk/chats.go`: extend `CreatedAt`'s `variants` to include
`reasoning?`; add `CompletedAt *time.Time` with `variants:"reasoning?"`.
- `coderd/x/chatd/chatloop/chatloop.go`: extend `reasoningState` with
`startedAt`; extend `stepResult` and `PersistedStep` with parallel
`[]time.Time` reasoning slices; stamp on
`ReasoningStart`/`ReasoningEnd`; thread the slices through all
`PersistStep` call sites including the interrupt-safe path; record
partial reasoning in `flushActiveState`.
- `coderd/x/chatd/attachments.go`: walk reasoning parts in occurrence
order and apply `step.ReasoningStartedAt[i]` to `part.CreatedAt` and
`step.ReasoningCompletedAt[i]` to `part.CompletedAt`.
### Tests
- `codersdk/chats_test.go` round-trips `created_at` + `completed_at` on
reasoning parts and verifies omission when absent and partial
interrupted parts.
- `coderd/x/chatd/chatprompt/chatprompt_test.go` asserts
`PartFromContent(ReasoningContent{})` does NOT stamp timestamps.
- `coderd/x/chatd/chatloop/chatloop_test.go`
`TestRun_ReasoningTimestamps` drives a stream with two reasoning blocks
and verifies parallel slices, monotonicity, ordering, non-zero values,
and content-block ordering.
`TestRun_InterruptedReasoningFlushesTimestamps` cancels mid-reasoning
and verifies `flushActiveState` records a non-zero pair.
- `coderd/x/chatd/attachments_test.go` covers
`buildAssistantPartsForPersist` for normal interleaved reasoning,
partial (zero `completed_at`), and missing slices.
</details>
> Generated by Coder Agents.
Co-authored-by: Coder Agent <agent@coder.com>
## Problem
Mid-turn workspace MCP discovery was broken when an agent was still
cold-starting. `PrepareTools` in `chatd.go` flipped
`workspaceMCPDiscovered = true` *before* calling
`discoverWorkspaceMCPTools`, so a failed discovery attempt permanently
blocked retries within the turn.
Customer-reported repro:
- New chat with no pre-selected workspace.
- LLM calls `create_workspace` mid-turn at `23:35:05`.
- `PrepareTools` fires, dials the agent with a 30s timeout, dial times
out at `23:38:15`, `discoverWorkspaceMCPTools` returns empty.
- Agent connects at `23:38:29`, 14 seconds later.
- `workspaceMCPDiscovered` was already true, so `PrepareTools` never
retried for the rest of the turn. MCP tools only appeared on the next
user message.
A naive retry loop in `PrepareTools` would also miss the bigger picture:
a workspace boot can take several minutes (EC2 cold start, 10 min
startup scripts), and the chatloop only gets a chance to call
`PrepareTools` between LLM steps.
## Fix
Do the workspace MCP discovery from inside the tool that already waits
for the agent. `chattool.CreateWorkspace` and `chattool.StartWorkspace`
call `waitForAgentReady`, which has a 2 min agent-online budget plus a
10 min startup-script budget. By the time they fire `OnChatUpdated`, the
agent is `Ready`. The chatd `onChatUpdated` callback now launches an
async `primeWorkspaceMCPCache` goroutine on every bind that has a valid
workspace ID:
- The primer calls `discoverWorkspaceMCPTools` until it returns a
non-empty list or `workspaceMCPPrimeMaxWait` (30s) elapses, with a 2s
backoff between attempts. The bounded wait handles the short race
between agent-online and the agent's MCP `Connect` settling.
- The primer runs asynchronously so the tool itself never blocks. Some
templates simply do not advertise MCP tools, in which case the primer
would otherwise spend its full budget for nothing.
- The primer shares the chat `ctx` (not a detached one) so it is
canceled together with the chat. A dangling primer would re-dial the
workspace conn after `runChat`'s deferred `workspaceCtx.close()` and
leak that conn.
- `inflight.Add(1)` ensures server shutdown still waits for any
in-progress primer.
- `PrepareTools` is simplified back to a single discovery call. It now
only sets `workspaceMCPDiscovered = true` on success, so an empty result
no longer permanently blocks discovery within the turn. The cache hit
warmed by the primer makes that call cheap in the common case; the dial
fallback handles the rare cache miss.
## Tests
All in `coderd/x/chatd/chatd_internal_test.go`:
- `TestPrimeWorkspaceMCPCache_SuccessOnFirstAttempt` — single
`ListMCPTools` call returning tools populates the cache.
- `TestPrimeWorkspaceMCPCache_RetriesUntilToolsAppear` — first call
empty, second returns tools; primer retries past the backoff and writes
the cache. Uses `quartz.Mock.Trap` on `NewTimer`.
- `TestPrimeWorkspaceMCPCache_GivesUpAfterDeadline` — `ListMCPTools`
always empty; primer stops at `workspaceMCPPrimeMaxWait` and refuses to
cache the empty result so PrepareTools can retry on the next step.
The existing integration test
`TestRunChat_WorkspaceMCPDiscoveryAfterMidTurnCreateWorkspace` continues
to pass and now also exercises the async-primer path end-to-end via the
create_workspace tool.
```
go test ./coderd/x/chatd/... -count=1
go test ./coderd/x/chatd/ -race -count=1
make pre-commit
```
<details>
<summary>Design notes</summary>
- The first iteration of this PR added retry+cooldown+failure-cap logic
inside `PrepareTools`. It worked for the customer's ~30s race window but
did not help workspaces that take several minutes to boot, because
`PrepareTools` only fires between LLM steps. Reviewer pointed out the
right place to handle this is the tool itself; the current
implementation does that.
- Why async: a primer that ran synchronously inside the `OnChatUpdated`
callback blocked the create_workspace tool from returning for up to
`workspaceMCPPrimeMaxWait`, which broke
`TestCreateWorkspaceTool_EndToEnd` and would hurt any template that does
not expose MCP tools. Decoupling lets the tool return immediately and
lets the primer warm the cache concurrently with the next LLM step.
- Why share the chat `ctx` rather than `context.WithoutCancel(ctx)` (the
title-generation pattern): the primer touches
`workspaceCtx.getWorkspaceConn`, which `runChat`'s deferred
`workspaceCtx.close()` invalidates. A detached primer outliving the chat
would dial a fresh conn and leak it.
- The constant naming distinguishes `workspaceMCPDiscoveryTimeout` (35s
per-call dial budget, unchanged from #25169) from
`workspaceMCPPrimeMaxWait` (30s total budget for the post-ready primer
loop) and `workspaceMCPPrimeRetryInterval` (2s between empty-result
retries).
</details>
Follow-up to #25169.
---
_This pull request was generated by Coder Agents._
Anthropic is strict about replaying the latest assistant turn once it
contains signed or redacted reasoning. We were still mutating that turn
in a few Coder-owned places: dropping empty reasoning blocks on replay,
rewriting provider-tool history during sanitization, and in the worst
case sending a prompt we already knew Anthropic would reject.
This patch keeps the latest signed assistant immutable through Coder's
replay and sanitization paths, preserves empty signed or redacted
reasoning anywhere Coder owns the ledger, and fails before the provider
call if the prompt is still unsafe.
It also bumps the existing `coder/fantasy` `coder_2_33` fork that `main`
already uses to the commit containing coder/fantasy#35. These fixes have
also been upstreamed to charmbracelet/fantasy.
Closes CODAGT-409.
> Mux updated this PR on behalf of Mike.
## Stack Context
This stack splits experimental personal skills into smaller reviewable
PRs. Personal skills are user-owned `SKILL.md` files stored by Coder and
injected into chatd alongside workspace skills.
Stack order:
1. #25362 personal skill resolver
2. #25363 storage, permissions, API, and SDK
3. #25365 API test coverage
4. #25366 chattool and chatd integration
5. #25066 settings UI and docs
6. #25386 personal skills slash menu
## What?
Adds the shared personal skill parser and resolver package, plus
reusable skill-name validation exported from `workspacesdk`.
The parser enforces the full personal skill contract: max raw size,
kebab-case name, max name length, and non-empty body.
## Why?
The rest of the stack needs one source-aware resolver for personal and
workspace skills, including collision handling and qualified aliases.
Keeping personal skill constraints in the parser prevents callers from
accidentally parsing invalid personal skills.
## Validation
- `go test ./coderd/x/skills ./codersdk/workspacesdk`
- pre-commit hooks on this branch
Michael Suchacz
