Use testing.Testing() inside createTransport to automatically
clone http.DefaultTransport when running in tests. In production,
DefaultTransport is used as-is (efficient connection pooling).
This fixes the CloseIdleConnections flake class: httptest.Server.Close()
calls http.DefaultTransport.CloseIdleConnections(), which disrupts
any MCP client sharing that transport. The testing.Testing() check
means every MCP transport created during tests gets isolation
automatically, with no caller changes needed.
Closescoder/internal#1016
Closes PLAT-291
When a caller sends multiple entries for the same literal path, merge
their edits into a single entry rather than returning 400. Symlink
aliases (different paths, same real file) are still rejected.
ContextPartsFromDir scans ~/.coder/skills via DefaultSkillsDir.
On machines with real skills installed, these leaked into test
results. Set HOME/USERPROFILE to temp dirs on the parent test
so subtests run in a clean environment.
handleProcessOutput read proc.output() then proc.info() using
separate locks. Between the two reads the exit goroutine could
finish I/O and set running=false, pairing stale output with final
status. On Windows CI this caused OutputExceedsBuffer to flake
when the buffer snapshot caught mid-write data (OmittedBytes=0)
but info reported the process as exited.
Swap the read order so info is read first. The exit goroutine
completes cmd.Wait (draining all pipe data) before setting
running=false, so seeing Running=false guarantees the subsequent
output read reflects the final buffer state.
Closes CODAGT-399
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.
> Mux is updating this PR on behalf of Mike.
## Summary
- Set a UTF-8 `LC_CTYPE` fallback for reconnecting PTYs when no
effective UTF-8 locale is present.
- Preserve non-empty `LC_ALL` so explicit user locale choices still win.
- Add tmux glyph regression coverage for reconnecting PTYs, plus unit
coverage for the env helper.
- Stabilize the tmux regression by keeping the pane alive until the
glyph output is observed.
- Keep the env helper unit test expectations OS-aware for Windows and
cover unhyphenated UTF8 locales.
## Validation
- `go test ./agent/reconnectingpty -run TestWithTerminalEnv -count=1`
- `go test ./agent -run '^TestAgent_ReconnectingPTY$/Buffered$'
-count=1`
- `go test ./agent -run '^TestAgent_ReconnectingPTY$' -count=1`
- `make lint`
- `git commit` pre-commit hook
- `git push` pre-push hook
> 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
`TestWatcher_SharedParentRefcount` was deterministically broken on
macOS: `t.TempDir()` lives under `/var` which is a symlink to
`/private/var`, but the watcher canonicalizes paths via
`filepath.EvalSymlinks` before storing them, so the test's `w.dirs[dir]`
lookup missed and returned `0` instead of `2`.
Adds `testutil.TempDirResolved`, a shared helper that returns
`t.TempDir()` with symlinks resolved and falls back to the raw temp dir
on error (Windows-friendly). Migrates the matching inline
`EvalSymlinks(t.TempDir())` callsites in
`agent/agentgit/agentgit_test.go` to use it.
Closes https://github.com/coder/internal/issues/1531
The default skills lookup only scanned the project-relative
.agents/skills directory, so personal skills had to be repeated
per project or wired in via CODER_AGENT_EXP_SKILLS_DIRS. Now the
default is the comma-separated list ~/.coder/skills,.agents/skills,
which lets discoverSkills's existing first-occurrence-wins policy
prefer home-scoped skills over project ones with the same name.
The change is additive when ~/.coder/skills is absent
(missing directories are silently skipped in discoverSkills) and
unaffects users who set the env var explicitly.
Closes CODAGT-403
## Bug
`agent/x/agentmcp/Manager` resolves its config paths once at boot, calls
`Reload`, and then only re-stats them lazily when a `GET
/api/v0/mcp/tools` request arrives (PR #24700). If any of the manager's
MCP config files (`~/.mcp.json` by default, or whatever paths
`agentcontextconfig.MCPConfigFiles()` resolves from
`CODER_AGENT_EXP_MCP_CONFIG_FILES`) is created, atomically rewritten, or
removed _after_ that initial `Reload` and _before_ the next tools HTTP
request, the manager keeps serving the stale (often empty) snapshot.
`parseAndDedup` silently swallows `fs.ErrNotExist`, so a late-appearing
file looks indistinguishable from "no config" until something pokes the
manager again.
This affects any workspace where the file lands after
`MarkStartupSettled` fires, including:
- a startup script that writes `~/.mcp.json` after MCP init
- a user creating or editing the file mid-session
- an installer, dotfiles step, or sync tool writing the file later in
startup
- another agent process (Claude Code, etc.) producing the file
out-of-band
- editor rewrites that land as `Write + Chmod + Rename` bursts
### Concrete repro (dual-agent workspace)
The race is easiest to reproduce on dual-agent workspaces (inner sandbox
+ outer host), where the inner agent's `scriptRunner` has
`script_count=0` and `mcpManager.Reload` fires at ~t+0.3 s while the
host agent writes `~/.mcp.json` ~21 s later. Timeline from
`workspace-otto-aa16`:
- agent up `20:23:38.918`
- lifecycle Ready `20:23:39.200`
- MCP config file Birth `20:24:00.460` (~21 s gap)
- no MCP log lines for 8 minutes
- first `GET /api/v0/mcp/tools` at `20:32:11.812` logs `[warn] mcp: mcp
reload canceled by caller`, takes 4946 ms; subsequent turns are cached
at 2 ms.
The single-agent case has the same race; it's just usually narrow enough
that the next HTTP request masks it, at the cost of a multi-second stall
on the first call that has to do the lazy reload itself. PR #25034's
`MarkStartupSettled` does not help: "settled" fires before the file is
necessarily on disk.
## Fix
Add an fsnotify-backed `configWatcher` to `agent/x/agentmcp/Manager`.
The watcher consumes whatever paths the manager is told to reload, which
is the same `[]string` returned by
`agentcontextconfig.MCPConfigFiles()`.
For each path the watcher:
- Watches the **parent directory** of the path, not the file itself.
This handles late creation, atomic rewrite (rename + create), and
deletion uniformly because inotify watches on individual non-existent
files return `ENOENT` and are lost across renames. The pattern matches
`agent/agentcontainers/watcher`.
- Walks up to the first existing **ancestor directory** when the parent
does not yet exist, and re-arms deeper on `Create` events that promote
an unrealized path.
- Refcounts directory watches so multiple configured paths sharing a
parent dir only register one inotify watch.
- Resolves symlinks **once at arming time** via `filepath.EvalSymlinks`;
never chases arbitrary symlink targets on events.
- Debounces multi-event editor writes through a single
`quartz.AfterFunc` timer so a `Write + Chmod + Rename` burst produces
one reload.
- Fires a debounced callback that calls `Manager.Reload`, which routes
through the existing singleflight so concurrent triggers coalesce.
- Re-syncs on every `Reload` call so a future path-list change is picked
up.
Lifecycle: the watcher is armed lazily on the first `Reload` (no
goroutine cost for unit tests that never reload). `Manager.Close` marks
the manager closed and closes its `closedCh` before tearing down the
watcher, so any in-flight watcher-driven reload observes the close via
`waitReload` and returns `ErrManagerClosed` instead of blocking
`firesWG.Wait()` on a stuck connect. The watcher then waits for its
goroutine and any in-flight debounced `fire` callback before returning.
`parseAndDedup` behavior is unchanged: `fs.ErrNotExist` still records an
empty snapshot. With the watcher armed before that snapshot is
committed, any `Create` event that races `parseAndDedup` is still
delivered.
This is the agent-side complement to PR #25169, which fixes chatd's
mid-turn workspace MCP discovery. `MarkStartupSettled` semantics are not
changed.
## Tests (`agent/x/agentmcp/configwatcher_internal_test.go`)
All new tests pass with the fix and fail without it. No `time.Sleep`;
synchronization uses `testutil.Eventually` for fsnotify-driven
assertions and the quartz mock clock for debounce assertions.
- `TestWatcher_LateFileTriggersReload` - the late-file regression: empty
dir, settle startup, `Reload` sees no file, write the file later,
watcher reloads, tools appear.
- `TestWatcher_RewriteTriggersReload` - existing file overwritten with a
new server list, watcher reloads, cache reflects new server.
- `TestWatcher_RemovalTransitionsToEmpty` - delete the file, watcher
reloads, manager transitions to empty cleanly.
- `TestWatcher_DebouncesBurst` - quartz mock clock; three back-to-back
`scheduleFire` calls produce exactly one `onChange` after `AdvanceNext`.
- `TestWatcher_CloseStopsGoroutine` - construct/Reload/Close five times
to surface goroutine or fd leaks under `-race`.
- `TestWatcher_DualAgentHTTPNoStall` - integration: write file after
`Reload`, wait for watcher reload, then `GET /tools` returns the MCP
tools in less than `testutil.WaitShort` instead of the multi-second
"reload canceled" stall.
- `TestWatcher_LateParentDirTriggersReload` - parent dir doesn't exist
at `Reload` time; create the dir then the file; watcher re-arms deeper
and reloads.
- `TestWatcher_SharedParentRefcount` - two configured paths share a
parent dir; only one inotify watch is registered and both reload on
changes.
- `TestWatcher_CloseDoesNotStallOnInFlightReload` - installs a
`connectStartedHook` to block a watcher-driven reload mid-`connectAll`,
then asserts `Close()` returns within `WaitMedium`. Regression-verified:
reverting the close-ordering causes the test to time out.
## Acceptance checklist
- [x] `go test ./agent/x/agentmcp/... -race -count=1` passes (also
`-count=5`).
- [x] All `./agent/...` tests pass under `-race -short`.
- [x] No emdash, endash, or ` -- `; `scripts/check_emdash.sh` clean.
- [x] No `time.Sleep` in tests.
- [x] New tests fail without the fix and pass with it (verified by
temporarily disabling `m.armWatcher(paths)` and by reverting `Close()`
ordering).
## Out of scope
No changes to chatd's mid-turn workspace MCP discovery (PR #25169) or
`MarkStartupSettled` semantics.
---
<sub>This pull request was prepared by a [Coder
Agents](https://coder.com/docs/admin/ai-coder) run.</sub>
When fuzzyReplace exhausts its passes, append a hint to the generic
"search string not found" error.
Inversion: if search did not match but replace does, list the lines
where replace appears.
Miscount: when a search line agrees with a file line except for the
count of one repeated rune, name the codepoint and counts.
Miscount takes precedence; both firing could direct an agent to swap
fields and corrupt the inversion anchor.
Did you swap "search" and "replace"? Your replace string appears
at line 12, 47, 89.
Your search has 32 "─" (U+2500); the file has 37 at line 182.
Closes CODAGT-330
The first `/mcp/tools` request could race workspace startup and return
an empty tool list before startup scripts had a chance to write
`.mcp.json`. Chatd may only discover tools once for a turn, so that
empty response could hide workspace MCP tools even though the agent
loaded them later.
Make the manager wait for startup to settle before treating missing MCP
config files as a real empty state. Tool listing now goes through one
manager-owned path that starts reload work independently of caller
cancellation; caller contexts only bound that caller's wait. After the
first reload body settles, transient reload errors return cached tools
with the error so the HTTP handler can degrade to the last known tool
set instead of returning `[]`.
The handler is intentionally thin: it asks the manager for tools, logs
any degraded path, and still returns the tool response shape callers
already expect. Tests cover startup gating, caller-canceled waits,
manager close, reload timeout via quartz, and cached-tool fallback after
a later reload error.
Workspace-agent logs emitted while serving chatd-driven requests were
not correlated with the originating chat, making agent logs hard to
attribute to the corresponding/originating chat.
This adds agent-side chat context middleware that parses `Coder-Chat-Id`
once, enriches agent access logs and structured handler/background logs,
and adds a chatd bridge log when chat headers are attached to an agent
connection.
Closes CODAGT-324
1 of 9 [next >>](https://github.com/coder/coder/pull/24811)
RFC: [Bridge ↔ Boundaries Correlation
RFC](https://www.notion.so/Bridge-Boundaries-Correlation-313d579be59281f3b4efdbfd6896775a)
Adds three new proto fields for boundary session correlation.
**`ReportBoundaryLogsRequest`**
- `session_id` (string, field 2) — UUID generated by boundary at
startup,
shared across all batches from a single run.
- `confined_process` (string, field 3) — name of the confined process
(e.g. `claude-code`, `codex`, `copilot`).
**`BoundaryLog`**
- `sequence_number` (uint64, field 4) — monotonically increasing counter
per session, primary ordering key when boundary is in use.
`BoundaryLog.time` already existed at field 2; no change needed there.
API version bumped to v2.9.
No behaviour change in coderd or the agent. This is a pure schema bump
that the boundary repo will consume in its own stack.
> Generated by Coder Agents
Adds a deployment-wide setting to select the computer-use provider
(Anthropic or OpenAI) for AI agents, plus the OpenAI computer-use runner
needed to honor that selection.
The setting is stored in `site_configs` under
`agents_computer_use_provider`, defaults to Anthropic when unset, and is
exposed via experimental GET/PUT endpoints under
`/api/experimental/chats/config/computer-use-provider`. The chatd
computer-use tool now dispatches to either `runAnthropicComputerUse` or
`runOpenAIComputerUse` based on the resolved provider, with
provider-specific result metadata for OpenAI screenshots.
Frontend adds a provider dropdown to the Agents Experiments settings
page nested under the virtual desktop toggle, with disabled state
handling while virtual desktop is off and skeleton loaders while config
queries are in flight.
Hugo and Codex review follow-up:
- Uses shared provider validation and clearer computer-use constant
names.
- Removes stale OpenAI pending-safety-checks commentary.
- Documents why provider result metadata is needed for OpenAI
screenshots.
- Keeps the computer-use subagent visible when provider credentials are
missing, then returns a clear spawn-time configuration error.
- Uses OpenAI's recommended 1600x900 screenshot geometry to preserve the
native 16:9 aspect ratio.
- Moves OpenAI-specific computer-use helpers into
`coderd/x/chatd/chatopenai/computeruse` after rebasing onto the provider
package refactor in `main`.
- Converts OpenAI pixel scroll deltas to Coder desktop wheel-click
amounts.
- Preserves OpenAI pointer modifiers with key down/up desktop actions
and rejects unsupported non-left double-click buttons explicitly.
- Maps OpenAI back/forward side-button clicks to browser navigation key
actions.
- Defaults omitted OpenAI click buttons to left-click.
- Retries mouse release cleanup if the final OpenAI drag release fails.
- Keeps computer-use subagent availability messages stable when provider
config cannot be loaded, while logging the backend error.
- Releases remaining OpenAI modifier keys if a synthetic key-up cleanup
action fails.
- Updates Storybook interaction stories so provider snapshots show the
selected final provider.
> Mux updated this PR description on behalf of Mike.
The MCP manager previously read .mcp.json exactly once at agent startup.
Editing the file had no effect until workspace rebuild or agent restart.
handleListTools now stats config file mtimes on every tool-list request
and triggers a differential reload when any file changed. Unchanged
servers keep their client pointer so in-flight tool calls survive.
Concurrent reload requests coalesce via singleflight.
MCP stdio subprocesses use the agent's execer for resource limits and
receive the same enriched environment as SSH sessions via updateEnv.
On the chatd side, WorkspaceMCPTool.Run detects 404 responses from
CallMCPTool (indicating the server was removed) and drops the chat's
cached tool list so the next turn refetches from the agent.
The CODER_AGENT_EXP_* env vars are agent-internal options. When set
in the workspace environment they leak to MCP subprocesses and user
shells.
ReadEnvConfig() captures the values and ClearEnvVars() strips them
before the reinit loop, so config survives agent restarts. NewAPI
and ReadEnvConfig both use applyDefaults() to fill zero fields.
The chatd test passes config via agenttest.WithContextConfigFromEnv().
- feat(agent/agentgit): shorten fallback poll to 5s
- fix(site/AgentsPage): keep git tab visible after reverting to clean
- feat(site/AgentsPage): show last-checked time in git tab
> 🤖
Fixes three classes of edit_files bugs and adds structured per-file
diff output for tool callers:
- New IncludeDiff flag on FileEditRequest; when set, the agent
returns FileEditResponse.Files[]{Path, Diff} with unified diffs
computed via go-udiff v0.4.1 Lines + ToUnified (not Unified,
which calls log.Fatalf on internal error).
- Fuzzy match comparators split each line into leading whitespace,
body, trailing whitespace, and ending. The splice substitutes at
each position: on agreement between search and replace the file's
bytes win; on disagreement the replacement's bytes are spliced
verbatim. Carve-outs for empty-body lines, multi-line EOF splices,
and level-aware indent translation for inserted lines.
- Indent-unit detection (GCD for spaces, tab-priority) lets a 4sp
LLM search insert correctly into tab or 2sp files. Falls back to
the previous cLead-inheritance path when units can't be detected
cleanly.
- Empty search is rejected with "search string must not be empty".
- Duplicate file paths in one request are rejected; symlink aliases
resolved via api.resolvePath before the dedup check.
- Frontend EditFilesRenderer consumes the structured files array by
explicit path (no label munging) with per-file synthetic fallback
for older agents or mismatched paths. On error, no diff is
rendered so the synthetic fallback doesn't misrepresent a
rejected edit as applied.
Breaking change: AgentConn.EditFiles changes from (ctx, req) error
to (ctx, req) (FileEditResponse, error) in codersdk/workspacesdk.
Source-breaking for external Go consumers; no compat shim per plan
owner.
Out of scope (tracked in CODAGT-214): level-aware indent for
middle-substituted splice lines. Locked in
TestEditFiles_FuzzyIndent_InsertionLevelAware's Lock_* cases plus
TestEditFiles_ReplaceAll_FuzzyIndentGap.
Injects user secrets into workspace agents at runtime via the agent
manifest. Secrets with an environment variable name are set as
environment variables in every agent session and startup script. Secrets
with a file path are written to disk before startup scripts run.
- Fetch user secrets in GetManifest and convert to proto
- Defensively strip secrets from manifests received by the agent to
avoid accidental leakage
- Add WorkspaceSecret type and proto conversion helpers to agentsdk
- Write secret files eagerly on manifest fetch (0600 perms, 0700 dirs)
- Inject secret env vars per-session in updateCommandEnv
- Expand ~/paths using caller-resolved home directory
- Log file write errors without blocking workspace startup
Wire DERPTLSConfig through the CLI, SDK, tailnet, VPN client, agent, and
health checks to allow custom TLS configuration for DERP connections.
The main use case is to be able to set a custom CA and also present
client certs (mTLS). See https://github.com/coder/tailscale/pull/105 for
related changes.
Adds three new global CLI flags:
- `--client-tls-ca-file` / `CODER_CLIENT_TLS_CA_FILE`
- `--client-tls-cert-file` / `CODER_CLIENT_TLS_CERT_FILE`
- `--client-tls-key-file` / `CODER_CLIENT_TLS_KEY_FILE`
Based on community PR #22695 by @ibdafna, with autogeneration issues
fixed (protobuf version mismatches in .pb.go files, golden file
regeneration, lint fixes).
> [!NOTE]
> This PR was authored by Coder Agents on behalf of a Coder team member.
<details>
<summary>Relationship to #22695</summary>
This is a clean reimplementation of the changes from #22695 on top of
current `main`, with the following differences:
- **Removed**: Accidental protobuf version changes in `.pb.go` files
(contributor had `protoc v6.33.4` vs project's `protoc v4.23.4`)
- **Added**: Properly regenerated golden files and docs via `make gen`
- **Fixed**: Lint issue (`var-declaration` revive warning on explicit
type in `createHTTPClient`)
- All meaningful code changes are identical to the original PR
</details>
> This PR was authored by Mux on behalf of Mike.
## Summary
- add persistent plan mode for chats and the chat-specific plan file
flow
- add structured planning tools such as `ask_user_question` and
`propose_plan`
- keep `write_file` and `edit_files` constrained to the chat-specific
plan file during plan turns
- allow shell exploration in plan mode, including subagents, via
`execute` and `process_output`
- block implementation-oriented, provider-native, MCP, dynamic, and
computer-use tools during plan turns
- update the chat UI, tests, and docs for the new planning flow
Add workspace secrets as a field in the agent manifest protobuf schema.
This allows the control plane to pass user secrets to agents for runtime
injection into workspace sessions.
Message fields:
- env_name: environment variable name (empty for file-only secrets)
- file_path: file path (empty for env-only secrets)
- value: the decrypted secret value as bytes
Fixes https://github.com/coder/internal/issues/1461
Two synchronization issues caused
`TestPortableDesktop_IdleTimeout_StopsRecordings` (and the
`MultipleRecordings` variant) to flake on macOS CI:
1. **`clk.Advance(idleTimeout)` was not awaited.** In
`MultipleRecordings`, both idle timers fire simultaneously but their
`fire()` goroutines race to remove themselves from the mock clock's
event list. Without `MustWait`, the second timer may still be in `m.all`
when the next `Advance` is called, causing `"cannot advance ... beyond
next timer/ticker event in 0s"`.
2. **The test depended on SIGINT being handled promptly.** After the
`stop_timeout` timer was released, the test relied entirely on the shell
process handling SIGINT (via `rec.done`). On macOS, `/bin/sh` may not
interrupt `wait` reliably, leaving `lockedStopRecordingProcess` blocked
in its `select` while holding `p.mu` — deadlocking the
`require.Eventually` callback.
### Fix
Wait for each `Advance` to complete and advance past the 15s stop
timeout so the process is forcibly killed via the timer path,
independent of signal handling.
Verified with 1000 iterations (500 per test) with zero failures.
> Generated with [Coder Agents](https://coder.com/agents)
When a devcontainer subagent is terraform-managed, the provisioner sets
its directory to the host-side `workspace_folder` path at build time. At
runtime, the agent injection code determines the correct
container-internal
path from `devcontainer read-configuration` and sends it via
`CreateSubAgent`.
However, the `CreateSubAgent` handler only updated `display_apps` for
pre-existing agents, ignoring the `Directory` field. This caused
SSH/terminal
sessions to land in `~` instead of the workspace folder (e.g.
`/workspaces/foo`).
Add `UpdateWorkspaceAgentDirectoryByID` query and call it in the
terraform-managed subagent update path to also persist the directory.
Fixes PLAT-118
<details><summary>Root cause analysis</summary>
Two code paths set the subagent `Directory` field:
1. **Provisioner (build time):** `insertDevcontainerSubagent` in
`provisionerdserver.go`
stores `dc.GetWorkspaceFolder()` — the **host-side** path from the
`coder_devcontainer` Terraform resource (e.g. `/home/coder/project`).
2. **Agent injection (runtime):**
`maybeInjectSubAgentIntoContainerLocked` in
`api.go` reads the devcontainer config and gets the correct
**container-internal**
path (e.g. `/workspaces/project`), then calls `client.Create(ctx,
subAgentConfig)`.
For terraform-managed subagents (those with `req.Id != nil`),
`CreateSubAgent`
in `coderd/agentapi/subagent.go` recognized the pre-existing agent and
entered
the update path — but only called `UpdateWorkspaceAgentDisplayAppsByID`,
discarding the `Directory` field from the request. The agent kept the
stale
host-side path, which doesn't exist inside the container, causing
`expandPathToAbs` to fall back to `~`.
</details>
> [!NOTE]
> Generated by Coder Agents
Adds `coder exp chat context add` and `coder exp chat context clear`
commands that run inside a workspace to manage chat context files via
the agent token.
`add` reads instruction and skill files from a directory (defaulting to
cwd) and inserts them as context-file messages into an active chat.
Multiple calls are additive — `instructionFromContextFiles` already
accumulates all context-file parts across messages.
`clear` soft-deletes all context-file messages, causing
`contextFileAgentID()` to return `!found` on the next turn, which
triggers `needsInstructionPersist=true` and re-fetches defaults from the
agent.
Both commands auto-detect the target chat via `CODER_CHAT_ID` (already
set by `agentproc` on chat-spawned processes), or fall back to
single-active-chat resolution for the agent. The `--chat` flag overrides
both.
Also adds sub-agent context inheritance: `createChildSubagentChat` now
copies parent context-file messages to child chats at spawn time, so
delegated sub-agents share the same instruction context without
independently re-fetching from the workspace agent.
<details><summary>Implementation details</summary>
**New files:**
- `cli/exp_chat.go` — CLI command tree under `coder exp chat context`
**Modified files:**
- `agent/agentcontextconfig/api.go` — `ConfigFromDir()` reads context
from an arbitrary directory without env vars
- `codersdk/agentsdk/agentsdk.go` — `AddChatContext`/`ClearChatContext`
SDK methods
- `coderd/workspaceagents.go` — POST/DELETE handlers on
`/workspaceagents/me/chat-context`
- `coderd/coderd.go` — Route registration
- `coderd/database/queries/chats.sql` — `GetActiveChatsByAgentID`,
`SoftDeleteContextFileMessages`
- `coderd/database/dbauthz/dbauthz.go` — RBAC implementations for new
queries
- `coderd/x/chatd/subagent.go` — `copyParentContextFiles` for sub-agent
inheritance
- `cli/root.go` — Register `chatCommand()` in `AGPLExperimental()`
**Auth pattern:** Uses `AgentAuth` (same as `coder external-auth`) —
agent token via `CODER_AGENT_TOKEN` + `CODER_AGENT_URL` env vars.
</details>
> 🤖 Generated by Coder Agents
---------
Co-authored-by: Michael Suchacz <203725896+ibetitsmike@users.noreply.github.com>
The agents chat interface displays thumbnails for videos recorded by the
computer use agent. Currently, to display a thumbnail, the frontend
downloads the entire video and shows the first frame. This PR starts
storing a new thumbnail file in the database for every recorded video,
and exposes the file id in the `wait_agent` tool result alongside the
recording file id, so the frontend can fetch just the thumbnail.
The agent SSH server unconditionally allows all four SSH forwarding
paths (TCP local, TCP reverse, Unix local, Unix reverse). This is a
sandbox escape vector when workspaces are used for AI agent containment
— a reverse tunnel lets anything inside the workspace reach the user's
local machine, bypassing network isolation.
This adds two new agent CLI flags / environment variables:
- `--block-reverse-port-forwarding` /
`CODER_AGENT_BLOCK_REVERSE_PORT_FORWARDING` — blocks both TCP (`ssh -R`)
and Unix socket reverse forwarding
- `--block-local-port-forwarding` /
`CODER_AGENT_BLOCK_LOCAL_PORT_FORWARDING` — blocks both TCP (`ssh -L`)
and Unix socket local forwarding
Template admins can set these via the `env` block on the container/VM
resource that runs the agent (e.g. `docker_container`,
`kubernetes_pod`), or via `coder_env` resources tied to the agent.
Fixes https://github.com/coder/coder/issues/22275
<details>
<summary>Implementation notes</summary>
Follows the existing `BlockFileTransfer` pattern:
1. `agent/agentssh/agentssh.go` — New `BlockReversePortForwarding` and
`BlockLocalPortForwarding` fields on `Config`. TCP callbacks check these
before allowing forwarding. The `direct-streamlocal@openssh.com` channel
handler is wrapped to reject Unix local forwards.
2. `agent/agentssh/forward.go` — `forwardedUnixHandler` gains a
`blockReversePortForwarding` field to reject
`streamlocal-forward@openssh.com` requests.
3. `agent/agent.go` — New fields on `Options` and `agent` struct,
plumbed to SSH config.
4. `cli/agent.go` — New serpent flags with env vars.
5. Tests cover all four blocked paths: TCP local, TCP reverse, Unix
local, Unix reverse.
</details>
> 🤖 Generated by Coder Agents
## Problem
MCP servers configured in `.mcp.json` with stdio transport are
discovered successfully (tools appear) but die immediately after
connection, making all tool calls fail.
## Root Cause
In `connectServer`, the subprocess is spawned with `connectCtx` — a
30-second timeout context whose `cancel()` is deferred:
```go
connectCtx, cancel := context.WithTimeout(ctx, connectTimeout)
defer cancel()
if err := c.Start(connectCtx); err != nil { ... }
```
The mcp-go stdio transport calls `exec.CommandContext(connectCtx, ...)`.
When `connectServer` returns, `cancel()` fires, and
`exec.CommandContext` sends SIGKILL to the subprocess. The process
immediately becomes a zombie.
Confirmed by checking `/proc/<pid>/status` after context cancellation:
```
State: Z (zombie)
```
## Fix
Pass the parent `ctx` (which is `a.gracefulCtx` — the agent's long-lived
context) to `c.Start()`. `connectCtx` continues to bound only the
`Initialize()` handshake. The subprocess is cleaned up when the Manager
is closed or the parent context is canceled.
## Regression Test
Added `TestConnectServer_StdioProcessSurvivesConnect` which:
- Spawns a real subprocess (re-execs the test binary as a fake MCP
server)
- Calls `connectServer` and lets it return (internal `connectCtx` gets
canceled)
- Verifies the subprocess is still alive by calling `ListTools`
The test **fails** on the old code with `transport error: context
deadline exceeded` and **passes** with the fix.
> Generated with [Coder Agents](https://coder.com/agents)
Piggybacks on #23878. Moves instruction file reading and skill discovery
from `chatd` (server-side, via multiple `LS`/`ReadFile` round-trips
through the agent connection) to the agent itself (local filesystem
access).
This intentionally drops backward compatibility with older agents that
don't support the context-config endpoint. Agents and server are
deployed together; there is no rolling-update contract to maintain here.
## What changed
The agent's `GET /api/v0/context-config` response now returns
`[]ChatMessagePart` directly — the same types chatd persists. This
eliminates intermediate type conversions and makes the protocol
extensible.
| Field | Type | Description |
|---|---|---|
| `parts` | `[]ChatMessagePart` | Context-file and skill parts, ready to
persist |
| `working_dir` | `string` | Agent's resolved working directory |
Removed from the response: `instructions_dirs`, `instructions_file`,
`skills_dirs`, `skill_meta_file`, `mcp_config_files` — the agent reads
files locally and returns their content as parts.
Removed from chatd: all legacy `LS`/`ReadFile` fallback code
(`readHomeInstructionFile`, `readInstructionDirFile`, `DiscoverSkills`
via LS, etc).
## Why
The previous architecture had the agent resolve paths, serve them over
HTTP, then `chatd` make N+1 round-trips back through the agent
connection to read files. The agent has direct filesystem access and
should just read the files.
## Key design decisions
- **Agent returns `ChatMessagePart` directly** — same types chatd
persists. No intermediate `InstructionFileEntry`/`SkillEntry` types
needed.
- **`SkillMeta.MetaFile`** — persisted via `ContextFileSkillMetaFile` on
the skill part, so custom meta file names
(`CODER_AGENT_EXP_SKILL_META_FILE`) survive across chat turns.
- **No pre-read body** — `read_skill` always dials the workspace to
fetch the skill body on demand. Simpler than caching the body in the
response.
- **MCP config paths kept agent-internal** — `MCPConfigFiles()` getter,
not sent over the wire.
- **No backward compat fallback** — old agents that don't support
context-config get no instruction files. This is acceptable since agent
and server deploy together.
This PR introduces screen recording of the computer use agent using the
virtual desktop.
- Screen recording is triggered by a `wait_agent` tool call. Recording
is stopped by a successful `wait_agent` tool call or when there hasn't
been any desktop activity for 10 minutes.
- Recordings are handled by the `portabledesktop` cli via the `record`
command. The videos are sped up in periods of inactivity.
- Recordings are saved to the database to the `chat_files` table.
There's a hard limit of 100MB per recording. Larger recordings are
dropped.
- A successful `wait_agent` on a computer use subagent tool call returns
a `recording_file_id`, later allowing the frontend to display the
corresponding video.
Fixes: coder/internal#1441
- Move `contextConfigAPI` init from `handleManifest` to `init()`,
matching all other API fields
- Change `agentcontextconfig.NewAPI` to accept `func() string` closure
(lazy directory evaluation)
- `Config()` and HTTP handler now compute on demand via
`a.manifest.Load().Directory`
- Widen `TestAgent_Reconnect` to loop 5 reconnections with a non-empty
manifest directory
- Add `TestContextConfigAPI_InitOnce` internal test verifying lazy eval
across manifest changes
- Add `TestNewAPI_LazyDirectory` unit test for the lazy contract
> 🤖 Written by a Coder Agent. Reviewed by a human.
Replace hardcoded paths for instruction files, skills, and MCP config
with
values read from `CODER_AGENT_EXP_*` environment variables. Template
authors
configure paths via the existing `coder_agent` `env` block. The agent
resolves `~`, relative, and absolute paths locally, then serves the
resolved config over `GET /api/v0/context-config`. `chatd` fetches this
once per workspace attach and falls back to today's defaults for older
agents.
All path env vars are comma-separated, allowing multiple directories:
| Env Var | Default | Controls |
|---|---|---|
| `CODER_AGENT_EXP_INSTRUCTIONS_DIRS` | `~/.coder` | Dirs containing the
instruction file |
| `CODER_AGENT_EXP_INSTRUCTIONS_FILE` | `AGENTS.md` | Instruction file
name |
| `CODER_AGENT_EXP_SKILLS_DIRS` | `.agents/skills` | Skills directories
|
| `CODER_AGENT_EXP_SKILL_META_FILE` | `SKILL.md` | Skill metadata file
name |
| `CODER_AGENT_EXP_MCP_CONFIG_FILES` | `.mcp.json` | MCP config files |
### Example
```hcl
resource "coder_agent" "main" {
os = "linux"
arch = "amd64"
env = {
CODER_AGENT_EXP_INSTRUCTIONS_DIRS = "/opt/company/agent-config,~/.coder"
CODER_AGENT_EXP_INSTRUCTIONS_FILE = "CLAUDE.md"
CODER_AGENT_EXP_SKILLS_DIRS = "/opt/company/ai-skills,.agents/skills"
CODER_AGENT_EXP_MCP_CONFIG_FILES = "/opt/company/mcp.json,.mcp.json"
}
}
```
<details>
<summary>Implementation Details</summary>
### Architecture
Follows the same pattern as MCP tool discovery:
agent resolves locally → exposes via HTTP → chatd consumes.
**Agent-side** (`agent/agentcontextconfig/`):
- `ResolvePath` / `ResolvePaths` handle `~`, relative, and absolute path
forms; returns `""` for relative paths when baseDir is empty
- `Config` reads env vars, falls back to defaults, resolves all paths
- `GET /api/v0/context-config` serves the resolved config as JSON
**chatd-side** (`coderd/x/chatd/`):
- Calls `conn.ContextConfig()` once on first workspace attach
- Falls back to hardcoded defaults on 404 (older agents)
- Iterates instruction dirs, skills dirs using resolved absolute paths
- `LSRelativityRoot` everywhere — no more home/root juggling
### Key design decisions
- **`EXP_` prefix**: env vars use `CODER_AGENT_EXP_*` to indicate
experimental status
- **Plural names**: comma-separated vars use plural names (`DIRS`,
`FILES`); single-value vars use singular (`FILE`)
- **Defaults in `workspacesdk`**: default constants live in
`codersdk/workspacesdk/` so both agent and server reference them without
cross-layer imports
- **`skillMetaFile` persistence**: stored on context-file parts via
`ContextFileSkillMetaFile` and restored on subsequent chat turns so
custom values survive across turns
- **Working dir dedup**: `slices.Contains` guard prevents reading the
same instruction file from both `InstructionsDirs` and the working
directory
- **MCP server dedup**: first-occurrence-wins dedup prevents leaking
duplicate connections from overlapping config files
- **ResolvePath safety**: returns `""` for relative paths when `baseDir`
is empty, so `ResolvePaths` filters them out
### Files changed
| File | Change |
|---|---|
| `agent/agentcontextconfig/` | New package — path resolution + HTTP
endpoint |
| `codersdk/workspacesdk/agentconn.go` | `ContextConfigResponse` type,
default constants, client method |
| `agent/agent.go` + `agent/api.go` | Wire up endpoint, pass config to
MCP |
| `agent/x/agentmcp/manager.go` | Accept `[]string` MCP config paths,
dedup by name |
| `coderd/x/chatd/chatd.go` | Fetch config, thread through, named
returns |
| `coderd/x/chatd/instruction.go` | Accept configurable dir + file name,
`skillMetaFileFromParts` |
| `coderd/x/chatd/chattool/skill.go` | Accept configurable dirs + meta
file |
| `codersdk/chats.go` | `ContextFileSkillMetaFile` field for persistence
|
### Test coverage
- `TestConfig` (4 cases): defaults, custom env vars, whitespace
trimming, comma-separated dirs
- `TestResolvePath` / `TestResolvePaths`: including empty baseDir edge
case
- `TestPersistInstructionFilesFallbackOnOlderAgent`: backward-compat
path when `ContextConfig` returns 404
- `TestChatMessagePartVariantTags`: updated exclusion list for new
internal field
### Backward compatibility
Older agents return 404 for the new endpoint. `chatd` catches this and
falls back to today's defaults via `readHomeInstructionFile` (using
`LSRelativityHome`). Existing workspaces work with no changes.
</details>
## Problem
Subagent chats were receiving git context (branch, remote origin, PR
status) from their parent or sibling chats' git operations. When a git
operation triggers external auth, the workspace agent sends `chat_id`
identifying which chat initiated it — but this was broken at two levels:
1. **Agent side:** `CODER_CHAT_ID` was never injected into process
environments. `chatd` sets `Coder-Chat-Id` HTTP headers and the
agent extracts them for process isolation, but never propagated
`CODER_CHAT_ID` to `cmd.Env`. So `gitaskpass` always sent an empty
`chat_id`.
2. **Server side:** `workspaceAgentsExternalAuth` ignored the `chat_id`
query param. `MarkStale` broadcast git context to **all** chats on
the workspace via `filterChatsByWorkspaceID`.
## Fix
- Inject `CODER_CHAT_ID` into `cmd.Env` in `agentproc` when the chat
ID is known, so `gitaskpass` can read and forward it.
- Read `chat_id` from query params in `workspaceAgentsExternalAuth`
and thread it through `chatGitRef`.
- Refactor `MarkStale` to accept a `MarkStaleParams` struct. When
`ChatID` is provided, target only that specific chat. When empty
(legacy agents, non-chat git operations), fall back to the existing
workspace-wide broadcast.
- Extract `markStaleSingle` helper to deduplicate the upsert+publish
logic.
<details><summary>Investigation notes</summary>
### Data flow before fix
```
chatd → sets Coder-Chat-Id header on agent conn
agent → extracts chatID, stores on process struct
agent → does NOT set CODER_CHAT_ID in cmd.Env ← gap 1
gitaskpass → reads CODER_CHAT_ID (always empty), sends chat_id=""
server handler → ignores chat_id query param ← gap 2
MarkStale → broadcasts to ALL workspace chats
```
### Data flow after fix
```
chatd → sets Coder-Chat-Id header on agent conn
agent → extracts chatID, stores on process struct
agent → sets CODER_CHAT_ID in cmd.Env
gitaskpass → reads CODER_CHAT_ID, sends chat_id=<uuid>
server handler → reads chat_id, passes to MarkStale
MarkStale → targets only that specific chat
```
</details>
`TestServer_X11_EvictionLRU` hangs forever when the developer's login
shell is `fish`. This is the only test in the repo that breaks on fish,
and it meant I couldn't run `make test` or similar without it blocking
indefinitely.
The test uses `sess.Shell()` to start interactive shell sessions, which
causes the SSH server to run the user's login shell directly (`fish
-l`). Fish buffers all piped stdin to EOF before executing any of it, so
the test's `echo ready-0\n` write never gets processed — fish sits
waiting for the pipe to close, and the test sits waiting for the echo
response.
The fix is a one-line change: `sess.Shell()` → `sess.Start("sh")`. The
test is exercising X11 LRU eviction, not shell behavior, so using `sh`
explicitly is both correct and shell-agnostic. The DISPLAY environment
variable is set identically either way since the x11-req handler runs
before `sessionStart`.
Coder's chat (chatd) can now discover and use MCP servers configured in
a workspace's `.mcp.json` file. This brings project-specific tooling
(GitHub, databases, docs servers, etc.) into the chat without any manual
configuration.
## How it works
The workspace agent reads `.mcp.json` from the workspace directory (same
format Claude Code uses), connects to the declared MCP servers —
spawning child processes for stdio servers and connecting over the
network for HTTP/SSE — and caches their tool lists. Two new agent HTTP
endpoints expose this:
- `GET /api/v0/mcp/tools` returns the cached tool list (supports
`?refresh=true`)
- `POST /api/v0/mcp/call-tool` proxies calls to the correct server
On each chat turn, chatd calls `ListMCPTools` through the existing
`AgentConn` tailnet connection, wraps each tool as a
`fantasy.AgentTool`, and adds them to the LLM's tool set alongside
built-in and admin-configured MCP tools. Tool names are prefixed with
the server name (`github__create_issue`) to avoid collisions.
Failed server connections are logged and skipped — they never block the
agent or break the chat. Child stdio processes are terminated on agent
shutdown.
## Changes
- **Commit 1**: Remove 17 unnecessary `//nolint` directives:
- `//nolint:varnamelen` — linter not active
- `//nolint:unused` on exported `SlimUnsupported`
- `//nolint:govet` in `coderd/httpmw/csrf` — no longer fires
- `//nolint:revive` on functions refactored since the nolint was added
- `//nolint:paralleltest` citing Go 1.22 loop variable capture
(obsolete)
- Bare `//nolint` narrowed to specific `//nolint:gocritic` with
justification
- **Commit 2**: Fix root causes behind 5 dangerous nolint suppressions:
- Add `MinVersion: tls.VersionTLS12` to TLS client config (removes
`gosec` G402)
- Delete trivial unexported wrappers `apiKey()`/`normalizeProvider()` in
chatprovider (removes `revive` confusing-naming)
- Add doc comments to `StartWithAssert` and `Router` (removes `revive`
exported)
- Rename unused parameters to `_` in integration test helpers
> 🤖 This PR was created using Coder Agents and reviewed by me.
- Move `agent/agentdesktop/` to `agent/x/agentdesktop/` to signal
experimental/unstable status
- Update import paths in `agent/agent.go` and `api_test.go`
> 🤖 This mechanical refactor was performed by an agent. I made sure it
didn't change anything it wasn't supposed to.
When edit_files receives multiple files, each file was processed
independently: read, compute edits, write. If file B failed, file A
was already written to disk. The caller got an error but had no way
to know which files were modified.
Split editFile into prepareFileEdit (read + compute, no side
effects) and a write phase. The handler runs all preparations
first and writes only if every file's edits succeed.
A write-phase failure (e.g. disk full) can still leave earlier
files committed. True cross-file atomicity would require
filesystem transactions. The prepare phase catches the common
failure modes: bad paths, search misses, permission errors.
replace_all in fuzzy mode (passes 2 and 3 of fuzzyReplace) only
replaced the first match. seekLines returned the first match,
spliceLines replaced one range, and there was no loop.
Extract fuzzy pass logic into fuzzyReplaceLines which:
- Returns a 3-tuple (result, matched, error) for clean caller flow
- When replaceAll is true, collects all non-overlapping matches
then applies replacements from last to first to preserve indices
- When replaceAll is false with multiple matches, returns an error
Add test cases for replace_all with fuzzy trailing whitespace and
fuzzy indent matching.
Both write_file and edit_files use atomic writes (write to temp
file, then rename). Since rename operates on directory entries, it
replaces symlinks with regular files instead of writing through
the link to the target.
Add resolveSymlink() that uses afero.Lstater/LinkReader to resolve
symlink chains (up to 10 levels) before the atomic write. Both
writeFile and editFile resolve the path before any filesystem
operations, matching the behavior of 'echo content > symlink'.
Gracefully no-ops on filesystems that don't support symlinks (e.g.
MemMapFs used in existing tests).
The slices package provides type-safe generic replacements for the
old typed sort convenience functions. The codebase already uses
slices.Sort in 43 call sites; this finishes the migration for the
remaining 29.
- sort.Strings(x) -> slices.Sort(x)
- sort.Float64s(x) -> slices.Sort(x)
- sort.StringsAreSorted(x) -> slices.IsSorted(x)
This PR changes agents desktop resolution from 1366x768 to 1920x1080.
Anthropic requires the that the resolution of desktop screenshots fits
in 1,150,000 total pixels, so we downscale screenshots to 1280x720
before sending them to the LLM provider.
Resolution scaling was already implemented, but our code didn't exercise
it. The resolution bump showed that there were some bugs in the scaling
logic - this PR fixes these bugs too.
Add Unwrap() to StatusWriter so http.ResponseController.SetWriteDeadline
can reach the underlying net.Conn through the middleware wrapper. Without
this, the agent's 20s WriteTimeout killed blocking process output
connections.
Also add 30s headroom to the write deadline in handleProcessOutput so
the response can be written after a full-duration blocking wait.
On the tool layer, waitForProcess and the process_output tool now try a
non-blocking snapshot on any error, not just context timeout. Transport
errors (like the WriteTimeout EOF) previously returned with no process
ID and no recovery path. Now if the process finished, the result is
returned transparently. If still running, the error includes the process
ID and tells the agent to use process_output.
Mathias Fredriksson