From 9d28489abbeab2de7cb5efae81faca78d2e4719d Mon Sep 17 00:00:00 2001 From: "blinkagent[bot]" <237617714+blinkagent[bot]@users.noreply.github.com> Date: Sat, 30 May 2026 13:05:51 +0500 Subject: [PATCH 01/23] chore(provisioner/terraform): preserve existing AWS_SDK_UA_APP_ID (#24606) Co-authored-by: blink-so[bot] <211532188+blink-so[bot]@users.noreply.github.com> Co-authored-by: Atif Ali --- provisioner/terraform/provision.go | 2 +- provisioner/terraform/safeenv.go | 36 +++++++++++++++ .../terraform/safeenv_internal_test.go | 44 +++++++++++++++++++ 3 files changed, 81 insertions(+), 1 deletion(-) create mode 100644 provisioner/terraform/safeenv_internal_test.go diff --git a/provisioner/terraform/provision.go b/provisioner/terraform/provision.go index 592bc3c9cc..90c96403bc 100644 --- a/provisioner/terraform/provision.go +++ b/provisioner/terraform/provision.go @@ -381,7 +381,7 @@ func provisionEnv( "CODER_WORKSPACE_BUILD_ID="+metadata.GetWorkspaceBuildId(), "CODER_TASK_ID="+metadata.GetTaskId(), "CODER_TASK_PROMPT="+metadata.GetTaskPrompt(), - "AWS_SDK_UA_APP_ID=APN_1.1/pc_cdfmjwn8i6u8l9fwz8h82e4w3$", + awsSDKUserAgentEnv(safeEnvironValue(env, awsSDKUserAgentEnvKey)), ) if metadata.GetPrebuiltWorkspaceBuildStage().IsPrebuild() { env = append(env, provider.IsPrebuildEnvironmentVariable()+"=true") diff --git a/provisioner/terraform/safeenv.go b/provisioner/terraform/safeenv.go index 4da2fc32cd..a42a899bc8 100644 --- a/provisioner/terraform/safeenv.go +++ b/provisioner/terraform/safeenv.go @@ -53,3 +53,39 @@ func safeEnviron() []string { } return strippedEnv } + +// safeEnvironValue returns the value of the named variable in the given +// `KEY=VALUE` environment slice, or an empty string if it is not present. +func safeEnvironValue(env []string, name string) string { + prefix := name + "=" + for _, e := range env { + if strings.HasPrefix(e, prefix) { + return strings.TrimPrefix(e, prefix) + } + } + return "" +} + +const ( + awsSDKUserAgentEnvKey = "AWS_SDK_UA_APP_ID" + // awsSDKUserAgentCoder is Coder's AWS Partner Revenue Measurement + // User-Agent string. The `APN_1.1/pc_$` format and the + // space-delimited append behavior below follow AWS's guidance: + // https://docs.aws.amazon.com/PRM/latest/aws-prm-onboarding-guide/automated-user-agent.html + awsSDKUserAgentCoder = "APN_1.1/pc_cdfmjwn8i6u8l9fwz8h82e4w3$" +) + +// awsSDKUserAgentEnv returns the AWS_SDK_UA_APP_ID value to pass to the +// Terraform subprocess. If the caller's environment already configures an +// Application ID (e.g. an operator who is also an AWS Partner and wants +// their own revenue attribution), Coder's value is appended with a space +// delimiter so both attributions are preserved. Otherwise Coder's value is +// used on its own. +// +// See: https://docs.aws.amazon.com/PRM/latest/aws-prm-onboarding-guide/automated-user-agent.html +func awsSDKUserAgentEnv(existing string) string { + if existing == "" { + return awsSDKUserAgentEnvKey + "=" + awsSDKUserAgentCoder + } + return awsSDKUserAgentEnvKey + "=" + existing + " " + awsSDKUserAgentCoder +} diff --git a/provisioner/terraform/safeenv_internal_test.go b/provisioner/terraform/safeenv_internal_test.go new file mode 100644 index 0000000000..1863f8fee1 --- /dev/null +++ b/provisioner/terraform/safeenv_internal_test.go @@ -0,0 +1,44 @@ +package terraform + +import ( + "testing" + + "github.com/stretchr/testify/require" +) + +func TestSafeEnvironValue(t *testing.T) { + t.Parallel() + + env := []string{ + "FOO=bar", + "AWS_SDK_UA_APP_ID=my-existing-id", + "BAZ=qux", + } + require.Equal(t, "my-existing-id", safeEnvironValue(env, "AWS_SDK_UA_APP_ID")) + require.Equal(t, "bar", safeEnvironValue(env, "FOO")) + require.Equal(t, "", safeEnvironValue(env, "MISSING")) +} + +func TestAWSSDKUserAgentEnv(t *testing.T) { + t.Parallel() + + t.Run("NoExisting", func(t *testing.T) { + t.Parallel() + require.Equal(t, + "AWS_SDK_UA_APP_ID=APN_1.1/pc_cdfmjwn8i6u8l9fwz8h82e4w3$", + awsSDKUserAgentEnv(""), + ) + }) + + t.Run("AppendToExisting", func(t *testing.T) { + t.Parallel() + // When the operator is themselves an AWS Partner and has set their own + // Application ID, we append Coder's with a space delimiter so both + // attributions are preserved. See: + // https://docs.aws.amazon.com/PRM/latest/aws-prm-onboarding-guide/automated-user-agent.html + require.Equal(t, + "AWS_SDK_UA_APP_ID=EXISTING_APP_ID APN_1.1/pc_cdfmjwn8i6u8l9fwz8h82e4w3$", + awsSDKUserAgentEnv("EXISTING_APP_ID"), + ) + }) +} From 8bec65a56aff25c9778e9acee117bc898b62d4bd Mon Sep 17 00:00:00 2001 From: 35C4n0r <70096901+35C4n0r@users.noreply.github.com> Date: Sat, 30 May 2026 13:44:01 +0530 Subject: [PATCH 02/23] chore(dogfood): remove tasks bits from coder and vscode-coder templates (#25479) Co-authored-by: Atif Ali --- dogfood/coder/main.tf | 5 -- dogfood/vscode-coder/main.tf | 106 ++++++----------------------------- 2 files changed, 17 insertions(+), 94 deletions(-) diff --git a/dogfood/coder/main.tf b/dogfood/coder/main.tf index aad65c886f..ad576e543f 100644 --- a/dogfood/coder/main.tf +++ b/dogfood/coder/main.tf @@ -277,7 +277,6 @@ data "coder_external_auth" "github" { data "coder_workspace" "me" {} data "coder_workspace_owner" "me" {} -data "coder_task" "me" {} data "coder_workspace_tags" "tags" { tags = { "cluster" : "dogfood-v2" @@ -991,10 +990,6 @@ resource "coder_metadata" "container_info" { key = "region" value = data.coder_parameter.region.option[index(data.coder_parameter.region.option.*.value, data.coder_parameter.region.value)].name } - item { - key = "ai_task" - value = data.coder_task.me.enabled ? "yes" : "no" - } } resource "coder_script" "boundary_config_setup" { diff --git a/dogfood/vscode-coder/main.tf b/dogfood/vscode-coder/main.tf index eece70b548..791136979f 100644 --- a/dogfood/vscode-coder/main.tf +++ b/dogfood/vscode-coder/main.tf @@ -204,7 +204,6 @@ data "coder_external_auth" "github" { data "coder_workspace" "me" {} data "coder_workspace_owner" "me" {} -data "coder_task" "me" {} data "coder_workspace_tags" "tags" { tags = { "cluster" : "dogfood-v2" @@ -541,99 +540,28 @@ resource "coder_metadata" "container_info" { key = "region" value = data.coder_parameter.region.option[index(data.coder_parameter.region.option.*.value, data.coder_parameter.region.value)].name } - item { - key = "ai_task" - value = data.coder_task.me.enabled ? "yes" : "no" - } -} - -# --- AI task support --- - -locals { - claude_system_prompt = <<-EOT - -- Framing -- - You are a helpful coding assistant working on the coder/vscode-coder - VS Code extension. Aim to autonomously investigate and solve issues - the user gives you and test your work, whenever possible. - - Avoid shortcuts like mocking tests. When you get stuck, you can ask - the user but opt for autonomy. - - -- Tool Selection -- - - Built-in tools for everything: - (file operations, git commands, builds & installs, one-off shell commands) - - -- Testing -- - Integration tests launch a real VS Code instance and require a - virtual framebuffer. Run them headlessly with: - xvfb-run -a pnpm test:integration - This matches how CI runs them. Unit tests do not need xvfb-run: - pnpm test - - -- Workflow -- - When starting new work: - 1. If given a GitHub issue URL, use the `gh` CLI to read the full - issue details with `gh issue view `. - 2. Create a feature branch for the work using a descriptive name - based on the issue or task. - Example: `git checkout -b fix/issue-123-ssh-retry` - 3. Proceed with implementation following the AGENTS.md guidelines. - - -- Context -- - This is the coder/vscode-coder VS Code extension. It is a real-world - production extension used by developers to connect to Coder workspaces. - Be sure to read AGENTS.md before making any changes. - EOT } module "claude-code" { - count = data.coder_task.me.enabled ? data.coder_workspace.me.start_count : 0 - source = "dev.registry.coder.com/coder/claude-code/coder" - version = "4.9.2" - enable_boundary = true - agent_id = coder_agent.dev.id - workdir = local.repo_dir - claude_code_version = "latest" - model = "opus" - order = 999 - claude_api_key = data.coder_parameter.use_ai_bridge.value ? data.coder_workspace_owner.me.session_token : var.anthropic_api_key - agentapi_version = "latest" - system_prompt = local.claude_system_prompt - ai_prompt = data.coder_task.me.prompt + count = data.coder_workspace.me.start_count + source = "dev.registry.coder.com/coder/claude-code/coder" + version = "5.2.0" + enable_ai_gateway = data.coder_parameter.use_ai_bridge.value + anthropic_api_key = data.coder_parameter.use_ai_bridge.value ? "" : var.anthropic_api_key + agent_id = coder_agent.dev.id + workdir = local.repo_dir } -resource "coder_ai_task" "task" { - count = data.coder_task.me.enabled ? data.coder_workspace.me.start_count : 0 - app_id = module.claude-code[count.index].task_app_id -} - -resource "coder_app" "watch" { - count = data.coder_task.me.enabled ? data.coder_workspace.me.start_count : 0 +resource "coder_app" "claude" { agent_id = coder_agent.dev.id - slug = "watch" - display_name = "pnpm watch" - icon = "${data.coder_workspace.me.access_url}/icon/code.svg" - command = "screen -x pnpm_watch" - share = "authenticated" - open_in = "tab" - order = 0 -} - -resource "coder_script" "watch" { - count = data.coder_task.me.enabled ? data.coder_workspace.me.start_count : 0 - display_name = "pnpm watch" - agent_id = coder_agent.dev.id - run_on_start = true - start_blocks_login = false - icon = "${data.coder_workspace.me.access_url}/icon/code.svg" - script = <<-EOT - #!/usr/bin/env bash - set -eux -o pipefail - - trap 'coder exp sync complete pnpm-watch' EXIT - coder exp sync want pnpm-watch install-deps - coder exp sync start pnpm-watch - - cd "${local.repo_dir}" && screen -dmS pnpm_watch /bin/sh -c 'while true; do pnpm watch; echo "pnpm watch exited with code $? restarting in 10s"; sleep 10; done' + slug = "claude" + display_name = "Claude Code" + icon = "/icon/claude.svg" + open_in = "slim-window" + command = <<-EOT + #!/bin/bash + set -e + cd "${local.repo_dir}" + exec tmux new-session -A -s claude claude EOT } From 6f5220202d3b2ba32ab9825c77310b1ccc4f5269 Mon Sep 17 00:00:00 2001 From: Atif Ali Date: Sun, 31 May 2026 10:03:34 +0500 Subject: [PATCH 03/23] fix(site/src/modules/resources): clarify agent log download button label (#25641) --- site/src/modules/resources/DownloadAgentLogsButton.stories.tsx | 2 +- site/src/modules/resources/DownloadAgentLogsButton.tsx | 2 +- site/src/modules/resources/DownloadSelectedAgentLogsButton.tsx | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/site/src/modules/resources/DownloadAgentLogsButton.stories.tsx b/site/src/modules/resources/DownloadAgentLogsButton.stories.tsx index 08f3f60e5d..b2c9114a46 100644 --- a/site/src/modules/resources/DownloadAgentLogsButton.stories.tsx +++ b/site/src/modules/resources/DownloadAgentLogsButton.stories.tsx @@ -34,7 +34,7 @@ export const ClickOnDownload: Story = { play: async ({ canvasElement, args }) => { const canvas = within(canvasElement); await userEvent.click( - canvas.getByRole("button", { name: "Download logs" }), + canvas.getByRole("button", { name: "Download agent logs" }), ); await waitFor(() => expect(args.download).toHaveBeenCalledWith( diff --git a/site/src/modules/resources/DownloadAgentLogsButton.tsx b/site/src/modules/resources/DownloadAgentLogsButton.tsx index 6850578897..fbc676d9d9 100644 --- a/site/src/modules/resources/DownloadAgentLogsButton.tsx +++ b/site/src/modules/resources/DownloadAgentLogsButton.tsx @@ -58,7 +58,7 @@ export const DownloadAgentLogsButton: FC = ({ }} > - {isDownloading ? "Downloading..." : "Download logs"} + {isDownloading ? "Downloading..." : "Download agent logs"} ); }; diff --git a/site/src/modules/resources/DownloadSelectedAgentLogsButton.tsx b/site/src/modules/resources/DownloadSelectedAgentLogsButton.tsx index 7aab6fd56c..16509cb568 100644 --- a/site/src/modules/resources/DownloadSelectedAgentLogsButton.tsx +++ b/site/src/modules/resources/DownloadSelectedAgentLogsButton.tsx @@ -63,7 +63,7 @@ export const DownloadSelectedAgentLogsButton: FC< > - {isDownloading ? "Downloading..." : "Download logs"} + {isDownloading ? "Downloading..." : "Download agent logs"} From 9c111a2be2ed87490083a1224e86f04cb7bcc6ba Mon Sep 17 00:00:00 2001 From: Dean Sheather Date: Sun, 31 May 2026 23:36:05 +1000 Subject: [PATCH 04/23] chore: disable release freezing on dev.coder.com (#25881) --- scripts/should_deploy.sh | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/scripts/should_deploy.sh b/scripts/should_deploy.sh index 6259f9e109..003828b411 100755 --- a/scripts/should_deploy.sh +++ b/scripts/should_deploy.sh @@ -17,6 +17,20 @@ deploy_branch=main # branch names. branch_name=$(git branch --show-current) +# Short circuit: we no longer deploy release branches to dogfood, and instead +# test them on the stable deployment. +# TODO: once we're happy with the new deployment process, we can remove this +# script and the related github workflow stuff. +if [[ "$branch_name" == "main" ]]; then + log "VERDICT: DEPLOY" + echo "DEPLOY" # stdout + exit 0 +else + log "VERDICT: NOOP" + echo "NOOP" # stdout + exit 0 +fi + if [[ "$branch_name" != "main" && ! "$branch_name" =~ ^release/[0-9]+\.[0-9]+$ ]]; then error "Current branch '$branch_name' is not a supported branch name for dogfood, must be 'main' or 'release/x.y'" fi From 76d3181aba6c2be0e06b816265e5228a0d7b7685 Mon Sep 17 00:00:00 2001 From: Ethan <39577870+ethanndickson@users.noreply.github.com> Date: Mon, 1 Jun 2026 13:42:37 +1000 Subject: [PATCH 05/23] ci(.github/workflows): bump action-linkspector to v1.5.2 (#25882) The `check-docs` job has been failing on every PR touching `docs/**` since 2026-05-29. `umbrelladocs/action-linkspector` runs linkspector under puppeteer, which expects an exact Chrome build (e.g. `148.0.7778.97`) in `/home/runner/.cache/puppeteer`. When that build isn't present on the hosted runner, linkspector crashes with `Could not find Chrome` and reviewdog then fails parsing the empty rdjson output with `proto: syntax error`. The pinned `v1.4.1` of the action was installing linkspector `0.4.7`, whose puppeteer requires `148.0.7778.97`; that build is no longer in the runner cache. Upstream `v1.5.2` upgrades linkspector to `0.5.3` and adds Chromium fallback logic, but on `ubuntu-22.04` x86_64 none of its new code paths fire (the AppArmor branch is gated on `lsb_release -rs == "24.04"`, the system-Chromium branch on aarch64 or missing 24.04 sysctl), so the bump alone leaves the same Chrome error in place. This PR: - Bumps the action to `v1.5.2` (linkspector `0.5.3`). - Sets `PUPPETEER_EXECUTABLE_PATH=/usr/bin/google-chrome` on the action step. The hosted `ubuntu-22.04` image ships Google Chrome at that path. `v1.5.2`'s `script.sh` short-circuits Chromium setup when this env is set, so puppeteer skips the cache lookup and uses the runner binary directly. End-to-end verified by temporarily perturbing `docs/**` on this branch so the workflow's `pull_request` trigger would fire: https://github.com/coder/coder/actions/runs/26732938434. `check-docs` ran linkspector against `docs/**` for ~2m30s and exited 0, with no `Could not find Chrome` or reviewdog parse errors in the log. That perturbation has been removed from the branch. Refs UmbrellaDocs/action-linkspector#62, UmbrellaDocs/action-linkspector#61 --- .github/workflows/weekly-docs.yaml | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/.github/workflows/weekly-docs.yaml b/.github/workflows/weekly-docs.yaml index 0d34ef1f43..505c8522de 100644 --- a/.github/workflows/weekly-docs.yaml +++ b/.github/workflows/weekly-docs.yaml @@ -55,9 +55,14 @@ jobs: mkdir -p "$(pnpm store path --silent)" - name: Check Markdown links - uses: umbrelladocs/action-linkspector@37c85bcde51b30bf929936502bac6bfb7e8f0a4d # v1.4.1 + uses: umbrelladocs/action-linkspector@036f295d12b67b0c4b445bc83db0538afb78db69 # v1.5.2 id: markdown-link-check # checks all markdown files from /docs including all subfolders + env: + # Use the runner-provided Chrome instead of letting linkspector's + # puppeteer download a specific version that may not match the + # runner's puppeteer cache. See: https://github.com/UmbrellaDocs/action-linkspector/issues/62 + PUPPETEER_EXECUTABLE_PATH: /usr/bin/google-chrome with: reporter: github-pr-review config_file: ".github/.linkspector.yml" From 8b7e040105c10e9a5bad5689da607c1f32f9c9e7 Mon Sep 17 00:00:00 2001 From: Mathias Fredriksson Date: Mon, 1 Jun 2026 12:42:09 +0300 Subject: [PATCH 06/23] fix(coderd/x/chatd/chatloop): discourage doctrine in compaction summaries (#25850) Two additions to the compaction summary prompt: 1. Error specificity: the "errors encountered" bullet now instructs the model to keep error notes specific (name the file, the error, the fix) and not generalize from a specific failure to a blanket tool-avoidance rule. This addresses the doctrine crystallization pattern where a single tool failure gets promoted to a standing "avoid tool X" rule that persists across compactions and model swaps. 2. Reproducibility: a new closing sentence instructs the model to reference reproducible content by path, command, or URL rather than inlining it. Content without a stable reproducer is still preserved inline with a brief summary. This targets summary bloat from inlined code blocks (worst case: 34k chars, 76 code blocks reproducing repo content verbatim). Refs CODAGT-331 --- coderd/x/chatd/chatloop/compaction.go | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/coderd/x/chatd/chatloop/compaction.go b/coderd/x/chatd/chatloop/compaction.go index 503eff51bc..b267f17e2a 100644 --- a/coderd/x/chatd/chatloop/compaction.go +++ b/coderd/x/chatd/chatloop/compaction.go @@ -35,14 +35,22 @@ const ( "- Key decisions made and their rationale\n" + "- Concrete technical details: file paths, function names, " + "commands, APIs, and configurations\n" + - "- Errors encountered and how they were resolved\n" + + "- Errors encountered and how they were resolved. Keep error " + + "notes specific: name the file, the error, and the fix. Do not " + + "generalize from a specific failure to a blanket tool-avoidance " + + "rule (e.g. \"tool X is unreliable\" or \"always use Y instead " + + "of Z\")\n" + "- Current state of the work: what is DONE, what is IN PROGRESS, " + "and what REMAINS to be done\n" + "- The specific action the assistant was performing or about to " + "perform when this summary was triggered\n\n" + "Be dense and factual. Every sentence should convey essential " + "context for continuation. Do not include pleasantries or " + - "conversational filler." + "conversational filler. For content that can be reproduced " + + "(repo files, command output, API responses), reference how to " + + "obtain it (file path, command, URL) rather than inlining the " + + "full content. Include brief inline summaries when the content " + + "itself would exceed a few lines." defaultCompactionSystemSummaryPrefix = "The following is a summary of " + "the earlier conversation. The assistant was actively working when " + "the context was compacted. Continue the work described below:" From 6ecf804896f41e25344d263abd8955a5918f5b05 Mon Sep 17 00:00:00 2001 From: Mathias Fredriksson Date: Mon, 1 Jun 2026 13:58:57 +0300 Subject: [PATCH 07/23] test(cli): eliminate race in PausedDuringWaitForReady test (#25858) The PausedDuringWaitForReady and WaitsForWorkingAppState tests flaked because the quartz resetTrap was released immediately after catching ticker.Reset (line 174), allowing client.TaskByID (line 175) to race with the subsequent DB mutation (pauseTask / PatchAppStatus). Fix: keep the resetTrap open across both poll iterations. On the first poll, release the trap so the goroutine sees the initial state and continues. On the second poll, hold the goroutine frozen at ticker.Reset while mutating state. Then release; client.TaskByID deterministically sees the mutated state. No race because the goroutine cannot execute client.TaskByID while trapped. Closes CODAGT-482 --- cli/task_send_test.go | 56 +++++++++++++++++++++++++++---------------- 1 file changed, 35 insertions(+), 21 deletions(-) diff --git a/cli/task_send_test.go b/cli/task_send_test.go index c90cb335cc..1590bcab29 100644 --- a/cli/task_send_test.go +++ b/cli/task_send_test.go @@ -237,7 +237,10 @@ func Test_TaskSend(t *testing.T) { t.Parallel() // Given: An initializing task (workspace running, no agent - // connected). + // connected). Close the agent, pause, then resume so the + // workspace is started but no agent is connected. The + // command enters waitForTaskIdle directly (initializing + // path), where we verify it handles an external pause. setupCtx := testutil.Context(t, testutil.WaitLong) setup := setupCLITaskTest(setupCtx, t, nil) @@ -246,8 +249,6 @@ func Test_TaskSend(t *testing.T) { resumeTask(setupCtx, t, setup.userClient, setup.task) // Set up mock clock and traps before starting the command. - // Without a mock clock the poll can race with the stop build - // and see a transient 'unknown' status instead of 'paused'. mClock := quartz.NewMock(t) tickTrap := mClock.Trap().NewTicker("task_send", "poll") resetTrap := mClock.Trap().TickerReset("task_send", "poll") @@ -271,23 +272,28 @@ func Test_TaskSend(t *testing.T) { tickCall.MustRelease(ctx) tickTrap.Close() - // Fire the immediate first poll (time.Nanosecond initial interval). - // This poll sees 'initializing' because no agent is connected. + // Fire the first poll. The goroutine calls ticker.Reset + // which the trap catches, freezing the goroutine BEFORE + // client.TaskByID runs. Release it so the first poll + // sees 'initializing' and continues. mClock.Advance(time.Nanosecond).MustWait(ctx) - - // Wait for Reset (confirms first poll completed). resetCall := resetTrap.MustWait(ctx) resetCall.MustRelease(ctx) - resetTrap.Close() - // Pause the task while waitForTaskIdle is polling. Since - // no agent is connected, the task stays initializing until - // we pause it, at which point the status becomes paused. + // Fire the second poll. The goroutine is again frozen at + // ticker.Reset by the trap. + mClock.Advance(5 * time.Second).MustWait(ctx) + resetCall = resetTrap.MustWait(ctx) + + // While the goroutine is frozen (before client.TaskByID), + // pause the task. The stop build completes, so the DB has + // (stop, succeeded) = 'paused'. pauseTask(ctx, t, setup.userClient, setup.task) - // Fire second poll at the regular 5s interval. The stop - // build has completed, so the poll sees 'paused'. - mClock.Advance(5 * time.Second).MustWait(ctx) + // Release the trap. The goroutine unfreezes and + // client.TaskByID deterministically sees 'paused'. + resetCall.MustRelease(ctx) + resetTrap.Close() // Then: The command should fail because the task was paused. err := w.Wait() @@ -328,23 +334,31 @@ func Test_TaskSend(t *testing.T) { tickCall.MustRelease(ctx) tickTrap.Close() - // Fire the immediate first poll (time.Nanosecond initial interval). + // Fire the first poll. The goroutine calls ticker.Reset + // which the trap catches, freezing the goroutine BEFORE + // client.TaskByID runs. Release it so the first poll + // sees "working" and continues. mClock.Advance(time.Nanosecond).MustWait(ctx) - - // Wait for Reset (confirms first poll completed and saw "working"). resetCall := resetTrap.MustWait(ctx) resetCall.MustRelease(ctx) - resetTrap.Close() - // Transition the app back to idle so waitForTaskIdle proceeds. + // Fire the second poll. The goroutine is again frozen + // at ticker.Reset by the trap. + mClock.Advance(5 * time.Second).MustWait(ctx) + resetCall = resetTrap.MustWait(ctx) + + // While the goroutine is frozen (before client.TaskByID), + // transition the app to idle. require.NoError(t, agentClient.PatchAppStatus(ctx, agentsdk.PatchAppStatus{ AppSlug: "task-sidebar", State: codersdk.WorkspaceAppStatusStateIdle, Message: "ready", })) - // Fire second poll at the regular 5s interval. - mClock.Advance(5 * time.Second).MustWait(ctx) + // Release the trap. The goroutine unfreezes and + // client.TaskByID deterministically sees "idle". + resetCall.MustRelease(ctx) + resetTrap.Close() // Then: The command should complete successfully. require.NoError(t, w.Wait()) From 1fcb4002d7c7a0a13b2c7cad00b1eff5ecbf9381 Mon Sep 17 00:00:00 2001 From: Ethan <39577870+ethanndickson@users.noreply.github.com> Date: Mon, 1 Jun 2026 21:25:29 +1000 Subject: [PATCH 08/23] fix: show execute tool errors (#25886) Execute tool failures that only return an `error` field, such as stopped-workspace connection failures, were rendered as a generic failed command without showing the backend detail. Normalize execute results into transcript blocks so shell output and tool errors both render in the *expanded* command transcript, and add Storybook coverage for connection errors plus output-with-error cases. image edit: i've dropped the red on the danger icon, though it was pre-existing. no point alerting the user to an error the model will handle. Closes CODAGT-530 --- .../tools/ExecuteTool.stories.tsx | 126 +++++++++++++----- .../ChatElements/tools/ExecuteTool.tsx | 27 ++-- .../components/ChatElements/tools/Tool.tsx | 7 +- .../ChatElements/tools/toolVisibility.test.ts | 41 +++++- .../ChatElements/tools/toolVisibility.ts | 19 ++- 5 files changed, 171 insertions(+), 49 deletions(-) diff --git a/site/src/pages/AgentsPage/components/ChatElements/tools/ExecuteTool.stories.tsx b/site/src/pages/AgentsPage/components/ChatElements/tools/ExecuteTool.stories.tsx index d9b469ff9c..54a47fe050 100644 --- a/site/src/pages/AgentsPage/components/ChatElements/tools/ExecuteTool.stories.tsx +++ b/site/src/pages/AgentsPage/components/ChatElements/tools/ExecuteTool.stories.tsx @@ -1,10 +1,13 @@ import type { Meta, StoryObj } from "@storybook/react-vite"; -import { userEvent, within } from "storybook/test"; +import { expect, userEvent, within } from "storybook/test"; import { ExecuteTool } from "./ExecuteTool"; const longCommand = "find /home/coder/project/src -type f -name '*.ts' -not -path '*/node_modules/*' -not -path '*/.git/*' | xargs grep -l 'deprecated' | sort | head -50"; +const stoppedWorkspaceError = + "workspace has no running agent: the workspace is likely stopped. Use the start_workspace tool to start it"; + const meta: Meta = { title: "components/ai-elements/tool/ExecuteTool", component: ExecuteTool, @@ -18,7 +21,7 @@ const meta: Meta = { args: { status: "completed", isError: false, - output: "", + transcriptBlocks: [], }, }; export default meta; @@ -28,7 +31,7 @@ type Story = StoryObj; export const ShortCommand: Story = { args: { command: "git status", - output: "", + transcriptBlocks: [], }, play: async ({ canvasElement }) => { const canvas = within(canvasElement); @@ -43,7 +46,7 @@ export const RunningWithoutCommand: Story = { args: { command: "", status: "running", - output: "", + transcriptBlocks: [], }, }; @@ -57,7 +60,7 @@ export const LongCommand: Story = { ], args: { command: longCommand, - output: "", + transcriptBlocks: [], }, play: async ({ canvasElement }) => { const canvas = within(canvasElement); @@ -72,14 +75,19 @@ export const LongCommand: Story = { export const LongCommandWithOutput: Story = { args: { command: longCommand, - output: [ - "src/api/legacyClient.ts", - "src/components/OldTable/OldTable.tsx", - "src/hooks/useObsoleteAuth.ts", - "src/pages/SettingsPage/DeprecatedPanel.tsx", - "src/utils/formatDate.ts", - "src/utils/legacyHelpers.ts", - ].join("\n"), + transcriptBlocks: [ + { + kind: "output", + text: [ + "src/api/legacyClient.ts", + "src/components/OldTable/OldTable.tsx", + "src/hooks/useObsoleteAuth.ts", + "src/pages/SettingsPage/DeprecatedPanel.tsx", + "src/utils/formatDate.ts", + "src/utils/legacyHelpers.ts", + ].join("\n"), + }, + ], }, }; @@ -87,19 +95,24 @@ export const LongCommandWithOutput: Story = { export const WithOutput: Story = { args: { command: "docker ps --format 'table {{.Names}}\t{{.Status}}\t{{.Ports}}'", - output: [ - "NAMES STATUS PORTS", - "coder-gateway Up 3 hours 0.0.0.0:3000->3000/tcp", - "coder-database Up 3 hours 0.0.0.0:5432->5432/tcp", - "coder-provisioner Up 3 hours", - "redis-cache Up 3 hours 0.0.0.0:6379->6379/tcp", - "nginx-proxy Up 2 hours 0.0.0.0:80->80/tcp, 0.0.0.0:443->443/tcp", - "prometheus Up 2 hours 0.0.0.0:9090->9090/tcp", - "grafana Up 2 hours 0.0.0.0:3001->3001/tcp", - "jaeger Up 1 hour 0.0.0.0:16686->16686/tcp", - "otel-collector Up 1 hour 0.0.0.0:4317->4317/tcp", - "loki Up 1 hour 0.0.0.0:3100->3100/tcp", - ].join("\n"), + transcriptBlocks: [ + { + kind: "output", + text: [ + "NAMES STATUS PORTS", + "coder-gateway Up 3 hours 0.0.0.0:3000->3000/tcp", + "coder-database Up 3 hours 0.0.0.0:5432->5432/tcp", + "coder-provisioner Up 3 hours", + "redis-cache Up 3 hours 0.0.0.0:6379->6379/tcp", + "nginx-proxy Up 2 hours 0.0.0.0:80->80/tcp, 0.0.0.0:443->443/tcp", + "prometheus Up 2 hours 0.0.0.0:9090->9090/tcp", + "grafana Up 2 hours 0.0.0.0:3001->3001/tcp", + "jaeger Up 1 hour 0.0.0.0:16686->16686/tcp", + "otel-collector Up 1 hour 0.0.0.0:4317->4317/tcp", + "loki Up 1 hour 0.0.0.0:3100->3100/tcp", + ].join("\n"), + }, + ], }, }; @@ -108,8 +121,12 @@ export const Running: Story = { args: { command: "go test -race -count=1 ./coderd/...", status: "running", - output: - "=== RUN TestWorkspaceAgent\n--- PASS: TestWorkspaceAgent (0.42s)", + transcriptBlocks: [ + { + kind: "output", + text: "=== RUN TestWorkspaceAgent\n--- PASS: TestWorkspaceAgent (0.42s)", + }, + ], }, }; @@ -119,11 +136,54 @@ export const ErrorOutput: Story = { command: "make build", status: "completed", isError: true, - output: [ - "coderd/workspaces.go:142:6: cannot use ws (variable of type *database.Workspace) as database.Store value in argument to api.Authorize", - "coderd/workspaces.go:155:19: ws.OwnerID undefined (type *database.Workspace has no field or method OwnerID)", - "make: *** [build] Error 1", - ].join("\n"), + transcriptBlocks: [ + { + kind: "output", + text: [ + "coderd/workspaces.go:142:6: cannot use ws (variable of type *database.Workspace) as database.Store value in argument to api.Authorize", + "coderd/workspaces.go:155:19: ws.OwnerID undefined (type *database.Workspace has no field or method OwnerID)", + "make: *** [build] Error 1", + ].join("\n"), + }, + ], + }, +}; + +/** A connection error renders its details inside the expanded transcript. */ +export const ConnectionError: Story = { + args: { + command: "ls -la", + status: "error", + isError: true, + shellToolDisplayMode: "auto", + transcriptBlocks: [{ kind: "error", text: stoppedWorkspaceError }], + }, + play: async ({ canvasElement }) => { + const canvas = within(canvasElement); + const errorMessage = await canvas.findByText( + /workspace has no running agent/i, + ); + await expect(errorMessage).toBeVisible(); + }, +}; + +/** A timed-out command can return partial output plus an execute error. */ +export const OutputWithError: Story = { + args: { + command: "go test ./...", + status: "completed", + isBackgrounded: true, + transcriptBlocks: [ + { + kind: "output", + text: [ + "=== RUN TestWorkspaceAgent", + "--- PASS: TestWorkspaceAgent (0.42s)", + "=== RUN TestWorkspaceBuild", + ].join("\n"), + }, + { kind: "error", text: "command timed out after 10s" }, + ], }, }; diff --git a/site/src/pages/AgentsPage/components/ChatElements/tools/ExecuteTool.tsx b/site/src/pages/AgentsPage/components/ChatElements/tools/ExecuteTool.tsx index 6f0a3b18f0..70f2c9fddf 100644 --- a/site/src/pages/AgentsPage/components/ChatElements/tools/ExecuteTool.tsx +++ b/site/src/pages/AgentsPage/components/ChatElements/tools/ExecuteTool.tsx @@ -27,6 +27,7 @@ import { resolveAgentDisplayState, } from "./displayMode"; import { ToolIcon } from "./ToolIcon"; +import type { ExecuteTranscriptBlock } from "./toolVisibility"; import { formatShellDurationMs, sanitizeExecuteModelIntent, @@ -37,7 +38,7 @@ import { type ExecuteToolProps = { command: string; - output: string; + transcriptBlocks: readonly ExecuteTranscriptBlock[]; status: ToolStatus; isError: boolean; durationMs?: number; @@ -53,8 +54,9 @@ type ExecuteToolInnerProps = ExecuteToolProps & { }; export const ExecuteTool: React.FC = (props) => { + const hasTranscriptBlocks = props.transcriptBlocks.length > 0; const autoDisplayState: AgentDisplayState = - props.output.length > 0 || + hasTranscriptBlocks || props.status === "running" || props.isBackgrounded || !!props.killedBySignal @@ -75,7 +77,7 @@ export const ExecuteTool: React.FC = (props) => { const ExecuteToolInner: React.FC = ({ command, - output, + transcriptBlocks, status, isError, durationMs, @@ -127,7 +129,7 @@ const ExecuteToolInner: React.FC = ({ @@ -168,7 +170,7 @@ const ExecuteToolInner: React.FC = ({ {outputOpen && ( )} @@ -217,9 +219,9 @@ const ShellCommandLine: React.FC<{ const ShellTranscriptBody: React.FC<{ command: string; - output: string; + transcriptBlocks: readonly ExecuteTranscriptBlock[]; isError: boolean; -}> = ({ command, output, isError }) => { +}> = ({ command, transcriptBlocks, isError }) => { return ( {" "} {command} - {output.length > 0 && ( + {transcriptBlocks.map((block) => (
-						{output}
+						{block.text}
 					
- )} + ))}
); diff --git a/site/src/pages/AgentsPage/components/ChatElements/tools/Tool.tsx b/site/src/pages/AgentsPage/components/ChatElements/tools/Tool.tsx index 82c18f6fe7..5fce31cff8 100644 --- a/site/src/pages/AgentsPage/components/ChatElements/tools/Tool.tsx +++ b/site/src/pages/AgentsPage/components/ChatElements/tools/Tool.tsx @@ -231,12 +231,15 @@ const ExecuteRenderer: FC = ({ shellToolDisplayMode, }) => { const data = getExecuteRenderData(args, result); + const outputBlock = data.transcriptBlocks.find( + (block) => block.kind === "output", + ); if (data.authenticateURL) { return ( @@ -245,7 +248,7 @@ const ExecuteRenderer: FC = ({ return ( { describe("getExecuteRenderData", () => { it("parses execute output and auth metadata from result payloads", () => { @@ -18,13 +21,49 @@ describe("toolVisibility", () => { ), ).toEqual({ command: "git fetch origin", - output: "fetched", + transcriptBlocks: [{ kind: "output", text: "fetched" }], durationMs: 47200, isBackgrounded: true, authenticateURL: "https://example.com/auth", providerLabel: "GitHub", }); }); + + it("normalizes execute error results into transcript blocks", () => { + const data = getExecuteRenderData( + { command: "ls -la" }, + { error: stoppedWorkspaceError }, + ); + + expect(data.command).toBe("ls -la"); + expect(data.transcriptBlocks).toEqual([ + { kind: "error", text: stoppedWorkspaceError }, + ]); + expect( + data.transcriptBlocks.map((block) => block.text).join("\n"), + ).toContain("workspace has no running agent"); + }); + + it("keeps output before error when both fields exist", () => { + expect( + getExecuteRenderData( + { command: "make build" }, + { output: " compiling ", error: " failed " }, + ).transcriptBlocks, + ).toEqual([ + { kind: "output", text: "compiling" }, + { kind: "error", text: "failed" }, + ]); + }); + + it("uses message as an error fallback when error is blank", () => { + expect( + getExecuteRenderData( + { command: "coder login" }, + { error: " ", message: " auth required " }, + ).transcriptBlocks, + ).toEqual([{ kind: "error", text: "auth required" }]); + }); }); describe("shouldRenderTool", () => { diff --git a/site/src/pages/AgentsPage/components/ChatElements/tools/toolVisibility.ts b/site/src/pages/AgentsPage/components/ChatElements/tools/toolVisibility.ts index daaca9e269..32c17772fd 100644 --- a/site/src/pages/AgentsPage/components/ChatElements/tools/toolVisibility.ts +++ b/site/src/pages/AgentsPage/components/ChatElements/tools/toolVisibility.ts @@ -8,9 +8,14 @@ import { toProviderLabel, } from "./utils"; +export type ExecuteTranscriptBlock = { + kind: "output" | "error"; + text: string; +}; + type ExecuteRenderData = { command: string; - output: string; + transcriptBlocks: ExecuteTranscriptBlock[]; durationMs?: number; isBackgrounded: boolean; authenticateURL: string; @@ -29,6 +34,16 @@ export const getExecuteRenderData = ( const command = parsedArgs ? asString(parsedArgs.command) : ""; const rec = asRecord(result); const output = rec ? asString(rec.output).trim() : ""; + const error = rec ? asString(rec.error).trim() : ""; + const fallbackMessage = rec && !error ? asString(rec.message).trim() : ""; + const errorText = error || fallbackMessage; + const transcriptBlocks: ExecuteTranscriptBlock[] = []; + if (output) { + transcriptBlocks.push({ kind: "output", text: output }); + } + if (errorText) { + transcriptBlocks.push({ kind: "error", text: errorText }); + } const durationMs = rec ? (asNumber(rec.wall_duration_ms, { parseString: true }) ?? asNumber(rec.duration_ms, { parseString: true })) @@ -47,7 +62,7 @@ export const getExecuteRenderData = ( return { command, - output, + transcriptBlocks, durationMs, isBackgrounded, authenticateURL, From ca337915ccc1cf645e4624ee632c2f980c62551d Mon Sep 17 00:00:00 2001 From: Nick Vigilante Date: Mon, 1 Jun 2026 08:47:29 -0400 Subject: [PATCH 09/23] docs: fix broken and naked relative links (#25825) Several relative links in the docs pointed at pages that no longer exist or rendered incorrectly on coder.com. Fixes: - `start/first-template.md`: IDE links repointed from the removed `../ides.md` / `../ides/web-ides.md` to their current homes under `user-guides/workspace-access/`. - `tutorials/example-guide.md`: contributing link repointed to `../about/contributing/documentation.md`. - `about/contributing/backend.md`: the `migrations/testdata/fixtures` and `full_dumps` references (and the `000024_example.up.sql` example) used relative paths that escape `docs/` and render as bogus `/docs/coderd/...` routes on the site. Normalized to the canonical `github.com/coder/coder/(blob|tree)/main/...` form already used by ~120 other source links in the docs. - Normalized extensionless directory links (`ai-coder/ai-gateway`, `user-guides/workspace-access`, `install`) to their `/index.md` targets for consistency with the rest of the docs. This class of bug is invisible to the local doc checks (`make lint/markdown` / `pnpm check-docs` only run markdownlint + table formatting); only CI's Linkspector job validates link targets. Found via a relative-link audit while investigating the docs preview on #25816. Source-link version-awareness (so older docs versions don't all point at `main`) is tracked separately in DOCS-268 and will be handled in the coder.com render layer. Linear: DOCS-278 Co-authored-by: Claude Opus 4.8 (1M context) --- docs/about/contributing/backend.md | 6 +++--- docs/admin/infrastructure/architecture.md | 2 +- docs/ai-coder/tasks-core-principles.md | 2 +- docs/ai-coder/tasks.md | 2 +- docs/install/upgrade.md | 2 +- docs/start/first-template.md | 4 ++-- docs/tutorials/example-guide.md | 2 +- 7 files changed, 10 insertions(+), 10 deletions(-) diff --git a/docs/about/contributing/backend.md b/docs/about/contributing/backend.md index bc159fe580..c568d53cd7 100644 --- a/docs/about/contributing/backend.md +++ b/docs/about/contributing/backend.md @@ -169,9 +169,9 @@ There are two types of fixtures that are used to test that migrations don't break existing Coder deployments: * Partial fixtures - [`migrations/testdata/fixtures`](../../../coderd/database/migrations/testdata/fixtures) + [`migrations/testdata/fixtures`](https://github.com/coder/coder/tree/main/coderd/database/migrations/testdata/fixtures) * Full database dumps - [`migrations/testdata/full_dumps`](../../../coderd/database/migrations/testdata/full_dumps) + [`migrations/testdata/full_dumps`](https://github.com/coder/coder/tree/main/coderd/database/migrations/testdata/full_dumps) Both types behave like database migrations (they also [`migrate`](https://github.com/golang-migrate/migrate)). Their behavior mirrors @@ -194,7 +194,7 @@ To add a new partial fixture, run the following command: ``` Then add some queries to insert data and commit the file to the repo. See -[`000024_example.up.sql`](../../../coderd/database/migrations/testdata/fixtures/000024_example.up.sql) +[`000024_example.up.sql`](https://github.com/coder/coder/blob/main/coderd/database/migrations/testdata/fixtures/000024_example.up.sql) for an example. To create a full dump, run a fully fledged Coder deployment and use it to diff --git a/docs/admin/infrastructure/architecture.md b/docs/admin/infrastructure/architecture.md index c409feeba9..7576712ef3 100644 --- a/docs/admin/infrastructure/architecture.md +++ b/docs/admin/infrastructure/architecture.md @@ -142,7 +142,7 @@ as OpenAI and Anthropic. Users authenticate through Coder instead of managing se provider API keys. All prompts, token usage, and tool invocations are recorded for compliance and cost tracking. -Learn more: [AI Gateway](../../ai-coder/ai-gateway) +Learn more: [AI Gateway](../../ai-coder/ai-gateway/index.md) ### Agent Firewall diff --git a/docs/ai-coder/tasks-core-principles.md b/docs/ai-coder/tasks-core-principles.md index c172d33907..771680cb8f 100644 --- a/docs/ai-coder/tasks-core-principles.md +++ b/docs/ai-coder/tasks-core-principles.md @@ -17,7 +17,7 @@ Coder Tasks is Coder's platform for managing coding agents. With Coder Tasks, yo ![Tasks UI](../images/guides/ai-agents/tasks-ui.png)Coder Tasks Dashboard view to see all available tasks. -Coder Tasks allows you and your organization to build and automate workflows to fully leverage AI. Tasks operate through Coder Workspaces. We support interacting with an agent through the Task UI and CLI. Some Tasks can also be accessed through the Coder Workspace IDE; see [connect via an IDE](../user-guides/workspace-access). +Coder Tasks allows you and your organization to build and automate workflows to fully leverage AI. Tasks operate through Coder Workspaces. We support interacting with an agent through the Task UI and CLI. Some Tasks can also be accessed through the Coder Workspace IDE; see [connect via an IDE](../user-guides/workspace-access/index.md). ## Why Use Tasks? diff --git a/docs/ai-coder/tasks.md b/docs/ai-coder/tasks.md index a39292f570..aedf76f9fa 100644 --- a/docs/ai-coder/tasks.md +++ b/docs/ai-coder/tasks.md @@ -11,7 +11,7 @@ Coder Tasks is an interface for running & managing coding agents such as Claude ![Tasks UI](../images/guides/ai-agents/tasks-ui.png) -Coder Tasks is best for cases where the IDE is secondary, such as prototyping or running long-running background jobs. However, tasks run inside full workspaces so developers can [connect via an IDE](../user-guides/workspace-access) to take a task to completion. +Coder Tasks is best for cases where the IDE is secondary, such as prototyping or running long-running background jobs. However, tasks run inside full workspaces so developers can [connect via an IDE](../user-guides/workspace-access/index.md) to take a task to completion. You can also interact with Coder Tasks from your IDE. The [Coder extension for VS Code](https://marketplace.visualstudio.com/items?itemName=coder.coder-remote) (and compatible forks like Cursor) enables you to create, monitor, and manage Tasks directly from the IDE, eliminating the need to context-switch to a browser. After logging in, you get access to a dedicated Tasks view in the sidebar that lets you select a template, configure parameters, prompt an agent, and track task status or download logs. Your tasks run in Coder workspaces with access to your repos, credentials, and internal network. diff --git a/docs/install/upgrade.md b/docs/install/upgrade.md index 2559217edc..8c4282202d 100644 --- a/docs/install/upgrade.md +++ b/docs/install/upgrade.md @@ -12,7 +12,7 @@ For upgrade recommendations and troubleshooting, see ## Reinstall Coder to upgrade To upgrade your Coder server, reinstall Coder using your original method -of [install](../install). +of [install](../install/index.md). ### Coder install script diff --git a/docs/start/first-template.md b/docs/start/first-template.md index 3b9d49fc59..ba7a2a802c 100644 --- a/docs/start/first-template.md +++ b/docs/start/first-template.md @@ -67,7 +67,7 @@ This starter template lets you connect to your workspace in a few ways: - VS Code Desktop: Loads your workspace into [VS Code Desktop](https://code.visualstudio.com/Download) installed on your local computer. -- code-server: Opens [browser-based VS Code](../ides/web-ides.md) with your +- code-server: Opens [browser-based VS Code](../user-guides/workspace-access/web-ides.md) with your workspace. - Terminal: Opens a browser-based terminal with a shell in the workspace's Docker instance. @@ -77,7 +77,7 @@ This starter template lets you connect to your workspace in a few ways: > [!TIP] > You can edit the template to let developers connect to a workspace in -> [a few more ways](../ides.md). +> [a few more ways](../user-guides/workspace-access/index.md). When you're done, you can stop the workspace. --> diff --git a/docs/tutorials/example-guide.md b/docs/tutorials/example-guide.md index 71d5ff15cd..5ede1a7344 100644 --- a/docs/tutorials/example-guide.md +++ b/docs/tutorials/example-guide.md @@ -16,7 +16,7 @@ repository. ## Content -Defer to our [Contributing/Documentation](../contributing/documentation.md) page +Defer to our [Contributing/Documentation](../about/contributing/documentation.md) page for rules on technical writing. ### Adding Photos From 61a9c4a61d8823c46a4a30401045face91c0f3ea Mon Sep 17 00:00:00 2001 From: Nick Vigilante Date: Mon, 1 Jun 2026 09:04:14 -0400 Subject: [PATCH 10/23] chore: Style fixes and nits across the AI Governance docs (#25793) - Add the "AI Governance Add-On" label across all pages - Use a generic `coder.example.com` URL across examples - Fix a few typos - Remove mentions of command access as a feature of AI Gov Fixes DOCS-262 --------- Co-authored-by: Danny Kopping --- docs/ai-coder/agent-firewall/index.md | 8 ++-- docs/ai-coder/ai-governance.md | 8 ++-- docs/ai-coder/usage-data-reporting.md | 4 +- docs/manifest.json | 61 ++++++++++++++++++--------- 4 files changed, 49 insertions(+), 32 deletions(-) diff --git a/docs/ai-coder/agent-firewall/index.md b/docs/ai-coder/agent-firewall/index.md index d5d2921097..8fe5192756 100644 --- a/docs/ai-coder/agent-firewall/index.md +++ b/docs/ai-coder/agent-firewall/index.md @@ -48,8 +48,8 @@ In your Terraform module, enable Agent Firewall with minimal configuration: ```tf module "claude-code" { - source = "dev.registry.coder.com/coder/claude-code/coder" - version = "4.7.0" + source = "registry.coder.com/coder/claude-code/coder" + version = "5.2.0" enable_boundary = true } ``` @@ -59,7 +59,7 @@ Claude Code module, use the following minimal configuration: ```yaml allowlist: - - "domain=dev.coder.com" # Required - use your Coder deployment domain + - "domain=coder.example.com" # Required - use your Coder deployment domain - "domain=api.anthropic.com" # Required - API endpoint for Claude - "domain=statsig.anthropic.com" # Required - Feature flags and analytics - "domain=claude.ai" # Recommended - WebFetch/WebSearch features @@ -225,5 +225,5 @@ such as Grafana Loki. Example of an allowed request (assuming stderr): ```console -2026-01-16 00:11:40.564 [info] coderd.agentrpc: boundary_request owner=joe workspace_name=some-task-c88d agent_name=dev decision=allow workspace_id=f2bd4e9f-7e27-49fc-961e-be4d1c2aa987 http_method=GET http_url=https://dev.coder.com event_time=2026-01-16T00:11:39.388607657Z matched_rule=domain=dev.coder.com request_id=9f30d667-1fc9-47ba-b9e5-8eac46e0abef trace=478b2b45577307c4fd1bcfc64fad6ffb span=9ece4bc70c311edb +2026-01-16 00:11:40.564 [info] coderd.agentrpc: boundary_request owner=joe workspace_name=some-task-c88d agent_name=dev decision=allow workspace_id=f2bd4e9f-7e27-49fc-961e-be4d1c2aa987 http_method=GET http_url=https://coder.example.com event_time=2026-01-16T00:11:39.388607657Z matched_rule=domain=coder.example.com request_id=9f30d667-1fc9-47ba-b9e5-8eac46e0abef trace=478b2b45577307c4fd1bcfc64fad6ffb span=9ece4bc70c311edb ``` diff --git a/docs/ai-coder/ai-governance.md b/docs/ai-coder/ai-governance.md index 1581a972c8..ce786ea53e 100644 --- a/docs/ai-coder/ai-governance.md +++ b/docs/ai-coder/ai-governance.md @@ -51,12 +51,10 @@ being used across the organization. AI Gateway provides audit trails of prompts, token usage, and tool invocations, giving administrators insight into AI adoption patterns and potential issues. -### Restricting agent network and command access +### Restricting agent network access -AI agents can make arbitrary network requests, potentially accessing -unauthorized services or exfiltrating data. They can also execute destructive -commands within a workspace. Agent Firewall enforces process-level policies -that restrict which domains agents can reach and what actions they can perform, +AI agents can make arbitrary network requests, potentially accessing unauthorized services or exfiltrating data. +Agent Firewall enforces process-level policies that restrict which domains agents can reach and what actions they can perform, preventing unintended data exposure and destructive operations like `rm -rf`. ### Centralizing API key management diff --git a/docs/ai-coder/usage-data-reporting.md b/docs/ai-coder/usage-data-reporting.md index 9d8fe08bfa..21c1e42d47 100644 --- a/docs/ai-coder/usage-data-reporting.md +++ b/docs/ai-coder/usage-data-reporting.md @@ -5,7 +5,7 @@ The [AI Governance Add-On](./ai-governance.md) requires reporting usage data to - number of agent workspace builds consumed - number of AI Governance seats consumed -No user-identifiable information or additional metrics are sent to Tallyman. This information is also shared with [Metronome](https://metronome.com), a Stripe product and Coder partner for usage-based and reporting. +No user-identifiable information or additional metrics are sent to Tallyman. This information is also shared with [Metronome](https://metronome.com), a Stripe product and Coder partner for usage-based billing and reporting. To send usage data, your Coder deployment must be able to make outbound HTTPS requests to `https://tallyman-prod.coder.com`. Usage data is sent approximately every 17 minutes and can be monitored via `coderd` logs. @@ -17,7 +17,7 @@ Example of a successful request (requires debug logging enabled [`CODER_LOG_FILT Example of a request payload: -```sh +```txt POST /api/v1/events/ingest HTTP/1.1 Host: tallyman-prod.coder.com Content-Type: application/json diff --git a/docs/manifest.json b/docs/manifest.json index cbbeceefbe..6f2fbf33ac 100644 --- a/docs/manifest.json +++ b/docs/manifest.json @@ -1144,85 +1144,99 @@ { "title": "Setup", "description": "How to set up and configure AI Gateway", - "path": "./ai-coder/ai-gateway/setup.md" + "path": "./ai-coder/ai-gateway/setup.md", + "state": ["ai governance add-on"] }, { "title": "Authentication", "description": "Learn how to authenticate against AI Gateway", - "path": "./ai-coder/ai-gateway/auth.md" + "path": "./ai-coder/ai-gateway/auth.md", + "state": ["ai governance add-on"] }, { "title": "Client Configuration", "description": "How to configure your AI coding tools to use AI Gateway", "path": "./ai-coder/ai-gateway/clients/index.md", + "state": ["ai governance add-on"], "children": [ { "title": "Coder Agents", "description": "Route Coder Agents traffic through AI Gateway", - "path": "./ai-coder/ai-gateway/clients/coder-agents.md" + "path": "./ai-coder/ai-gateway/clients/coder-agents.md", + "state": ["ai governance add-on"] }, { "title": "Claude Code", "description": "Configure Claude Code to use AI Gateway", - "path": "./ai-coder/ai-gateway/clients/claude-code.md" + "path": "./ai-coder/ai-gateway/clients/claude-code.md", + "state": ["ai governance add-on"] }, { "title": "Codex", "description": "Configure Codex to use AI Gateway", - "path": "./ai-coder/ai-gateway/clients/codex.md" + "path": "./ai-coder/ai-gateway/clients/codex.md", + "state": ["ai governance add-on"] }, { "title": "Mux", "description": "Configure Mux to use AI Gateway", - "path": "./ai-coder/ai-gateway/clients/mux.md" + "path": "./ai-coder/ai-gateway/clients/mux.md", + "state": ["ai governance add-on"] }, { "title": "OpenCode", "description": "Configure OpenCode to use AI Gateway", - "path": "./ai-coder/ai-gateway/clients/opencode.md" + "path": "./ai-coder/ai-gateway/clients/opencode.md", + "state": ["ai governance add-on"] }, { "title": "Factory", "description": "Configure Factory to use AI Gateway", - "path": "./ai-coder/ai-gateway/clients/factory.md" + "path": "./ai-coder/ai-gateway/clients/factory.md", + "state": ["ai governance add-on"] }, { "title": "Cline", "description": "Configure Cline to use AI Gateway", - "path": "./ai-coder/ai-gateway/clients/cline.md" + "path": "./ai-coder/ai-gateway/clients/cline.md", + "state": ["ai governance add-on"] }, { "title": "Kilo Code", "description": "Configure Kilo Code to use AI Gateway", - "path": "./ai-coder/ai-gateway/clients/kilo-code.md" + "path": "./ai-coder/ai-gateway/clients/kilo-code.md", + "state": ["ai governance add-on"] }, { "title": "VS Code", "description": "Configure VS Code to use AI Gateway", - "path": "./ai-coder/ai-gateway/clients/vscode.md" + "path": "./ai-coder/ai-gateway/clients/vscode.md", + "state": ["ai governance add-on"] }, { "title": "JetBrains", "description": "Configure JetBrains IDEs to use AI Gateway", - "path": "./ai-coder/ai-gateway/clients/jetbrains.md" + "path": "./ai-coder/ai-gateway/clients/jetbrains.md", + "state": ["ai governance add-on"] }, { "title": "Zed", "description": "Configure Zed to use AI Gateway", - "path": "./ai-coder/ai-gateway/clients/zed.md" + "path": "./ai-coder/ai-gateway/clients/zed.md", + "state": ["ai governance add-on"] }, { "title": "GitHub Copilot", "description": "Configure GitHub Copilot to use AI Gateway via AI Gateway Proxy", - "path": "./ai-coder/ai-gateway/clients/copilot.md" + "path": "./ai-coder/ai-gateway/clients/copilot.md", + "state": ["ai governance add-on"] } ] }, { "title": "MCP Tools Injection", "description": "How to configure MCP servers for tools injection through AI Gateway", - "path": "./ai-coder/ai-gateway/mcp.md", - "state": ["early access"] + "path": "./ai-coder/ai-gateway/mcp.md" }, { "title": "AI Gateway Proxy", @@ -1233,31 +1247,36 @@ { "title": "Setup", "description": "How to set up and configure AI Gateway Proxy", - "path": "./ai-coder/ai-gateway/ai-gateway-proxy/setup.md" + "path": "./ai-coder/ai-gateway/ai-gateway-proxy/setup.md", + "state": ["ai governance add-on"] } ] }, { "title": "Auditing AI Sessions", "description": "How to audit AI sessions", - "path": "./ai-coder/ai-gateway/audit.md" + "path": "./ai-coder/ai-gateway/audit.md", + "state": ["ai governance add-on"] }, { "title": "Monitoring", "description": "How to monitor AI Gateway", - "path": "./ai-coder/ai-gateway/monitoring.md" + "path": "./ai-coder/ai-gateway/monitoring.md", + "state": ["ai governance add-on"] }, { "title": "Reference", "description": "Technical reference for AI Gateway", - "path": "./ai-coder/ai-gateway/reference.md" + "path": "./ai-coder/ai-gateway/reference.md", + "state": ["ai governance add-on"] } ] }, { "title": "Usage Data Reporting", "description": "Configure AI usage data reporting", - "path": "./ai-coder/usage-data-reporting.md" + "path": "./ai-coder/usage-data-reporting.md", + "state": ["ai governance add-on"] } ] }, From c8555e2163ff2ac8eff2e2e6751c59a9fe4ff2ca Mon Sep 17 00:00:00 2001 From: Danny Kopping Date: Mon, 1 Jun 2026 15:15:47 +0200 Subject: [PATCH 11/23] fix: deprecate ai provider seeding env config (#25854) Environment variables used to configure AI Gateway providers are now deprecated, and we need to reflect this as such. --- cli/server.go | 35 +++++++++- cli/server_aibridge_internal_test.go | 64 +++++++++++++++++++ cli/testdata/coder_server_--help.golden | 62 ++++++++++++------ cli/testdata/server-config.yaml.golden | 34 +++++++--- codersdk/deployment.go | 51 ++++++++------- docs/reference/cli/server.md | 20 +++--- .../cli/testdata/coder_server_--help.golden | 62 ++++++++++++------ 7 files changed, 244 insertions(+), 84 deletions(-) diff --git a/cli/server.go b/cli/server.go index b2fa89fd3b..e8b8768eea 100644 --- a/cli/server.go +++ b/cli/server.go @@ -2979,6 +2979,11 @@ func parseExternalAuthProvidersFromEnv(prefix string, environ []string) ([]coder return providers, nil } +const ( + aiGatewayProviderEnvPrefix = "CODER_AI_GATEWAY_PROVIDER_" + aiBridgeProviderEnvPrefix = "CODER_AIBRIDGE_PROVIDER_" +) + // ReadAIProvidersFromEnv parses CODER_AI_GATEWAY_PROVIDER__ // environment variables into a slice of AIProviderConfig. // Deprecated alias env vars with the CODER_AIBRIDGE_PROVIDER__ @@ -2986,16 +2991,22 @@ func parseExternalAuthProvidersFromEnv(prefix string, environ []string) ([]coder // // This follows the same indexed pattern as ReadExternalAuthProvidersFromEnv. func ReadAIProvidersFromEnv(logger slog.Logger, environ []string) ([]codersdk.AIProviderConfig, error) { - providers, err := readAIProvidersForPrefix(logger, environ, "CODER_AIBRIDGE_PROVIDER_") + providers, err := readAIProvidersForPrefix(logger, environ, aiBridgeProviderEnvPrefix) if err != nil { return nil, err } - gatewayProviders, err := readAIProvidersForPrefix(logger, environ, "CODER_AI_GATEWAY_PROVIDER_") + gatewayProviders, err := readAIProvidersForPrefix(logger, environ, aiGatewayProviderEnvPrefix) if err != nil { return nil, err } if len(providers) > 0 && len(gatewayProviders) > 0 { - return nil, xerrors.New("cannot mix CODER_AIBRIDGE_PROVIDER_* and CODER_AI_GATEWAY_PROVIDER_* environment variables, please consolidate onto CODER_AI_GATEWAY_PROVIDER_*") + return nil, xerrors.Errorf("cannot mix %s* and %s* environment variables, please consolidate onto %s*", aiBridgeProviderEnvPrefix, aiGatewayProviderEnvPrefix, aiGatewayProviderEnvPrefix) + } + var activePrefix string + if len(providers) > 0 { + activePrefix = aiBridgeProviderEnvPrefix + } else if len(gatewayProviders) > 0 { + activePrefix = aiGatewayProviderEnvPrefix } providers = append(providers, gatewayProviders...) @@ -3077,9 +3088,27 @@ func ReadAIProvidersFromEnv(logger slog.Logger, environ []string) ([]codersdk.AI names[p.Name] = i } + warnIfAIProvidersConfiguredFromEnv(context.Background(), logger, activePrefix, providers) + return providers, nil } +func warnIfAIProvidersConfiguredFromEnv(ctx context.Context, logger slog.Logger, prefix string, providers []codersdk.AIProviderConfig) { + if len(providers) == 0 { + return + } + + if prefix == "" { + return + } + + logger.Warn(ctx, + "ai provider environment variables are deprecated for provider management and only seed provider configuration at startup", + slog.F("env_prefix", prefix), + slog.F("replacement", "Manage AI Providers from the Coder UI or HTTP API."), + ) +} + // readAIProvidersForPrefix parses provider env vars under a single // indexed prefix (e.g. CODER_AI_GATEWAY_PROVIDER_) into a slice of // AIProviderConfig. Per-field syntax errors and unknown keys are diff --git a/cli/server_aibridge_internal_test.go b/cli/server_aibridge_internal_test.go index a91e5b51d2..fce45aa674 100644 --- a/cli/server_aibridge_internal_test.go +++ b/cli/server_aibridge_internal_test.go @@ -1,6 +1,7 @@ package cli import ( + "context" "database/sql" "encoding/json" "fmt" @@ -575,6 +576,58 @@ func TestValidateLegacyAIBridgeConfig(t *testing.T) { } } +func TestWarnIfAIProvidersConfiguredFromEnv(t *testing.T) { + t.Parallel() + + t.Run("NoProviders", func(t *testing.T) { + t.Parallel() + + sink := testutil.NewFakeSink(t) + warnIfAIProvidersConfiguredFromEnv(context.Background(), sink.Logger(), aiGatewayProviderEnvPrefix, nil) + + require.Empty(t, sink.Entries()) + }) + + t.Run("EmptyPrefix", func(t *testing.T) { + t.Parallel() + + sink := testutil.NewFakeSink(t) + warnIfAIProvidersConfiguredFromEnv(context.Background(), sink.Logger(), "", []codersdk.AIProviderConfig{{Type: "openai", Name: "openai"}}) + + require.Empty(t, sink.Entries()) + }) + + t.Run("AIGatewayPrefix", func(t *testing.T) { + t.Parallel() + + sink := testutil.NewFakeSink(t) + warnIfAIProvidersConfiguredFromEnv(context.Background(), sink.Logger(), aiGatewayProviderEnvPrefix, []codersdk.AIProviderConfig{{Type: "openai", Name: "openai"}}) + + entries := sink.Entries(func(e slog.SinkEntry) bool { + return e.Message == "ai provider environment variables are deprecated for provider management and only seed provider configuration at startup" + }) + require.Len(t, entries, 1) + require.Len(t, entries[0].Fields, 2) + assertFieldValue(t, entries[0].Fields, "env_prefix", aiGatewayProviderEnvPrefix) + assertFieldValue(t, entries[0].Fields, "replacement", "Manage AI Providers from the Coder UI or HTTP API.") + }) + + t.Run("AIBridgePrefix", func(t *testing.T) { + t.Parallel() + + sink := testutil.NewFakeSink(t) + warnIfAIProvidersConfiguredFromEnv(context.Background(), sink.Logger(), aiBridgeProviderEnvPrefix, []codersdk.AIProviderConfig{{Type: "openai", Name: "openai"}}) + + entries := sink.Entries(func(e slog.SinkEntry) bool { + return e.Message == "ai provider environment variables are deprecated for provider management and only seed provider configuration at startup" + }) + require.Len(t, entries, 1) + require.Len(t, entries[0].Fields, 2) + assertFieldValue(t, entries[0].Fields, "env_prefix", aiBridgeProviderEnvPrefix) + assertFieldValue(t, entries[0].Fields, "replacement", "Manage AI Providers from the Coder UI or HTTP API.") + }) +} + func TestBuildAIProviderFromRowSetsAPIDumpDir(t *testing.T) { t.Parallel() @@ -721,3 +774,14 @@ func mustMarshalSettings(s codersdk.AIProviderSettings) sql.NullString { } return sql.NullString{String: string(data), Valid: true} } + +func assertFieldValue(t *testing.T, fields slog.Map, name string, expected interface{}) { + t.Helper() + for _, f := range fields { + if f.Name == name { + assert.Equal(t, expected, f.Value) + return + } + } + t.Errorf("field %q not found", name) +} diff --git a/cli/testdata/coder_server_--help.golden b/cli/testdata/coder_server_--help.golden index 4bcf9efda9..53ccf77f7c 100644 --- a/cli/testdata/coder_server_--help.golden +++ b/cli/testdata/coder_server_--help.golden @@ -124,35 +124,55 @@ AI GATEWAY OPTIONS: disabled, only centralized key authentication is permitted. --ai-gateway-anthropic-base-url string, $CODER_AI_GATEWAY_ANTHROPIC_BASE_URL (default: https://api.anthropic.com/) - The base URL of the Anthropic API. + Deprecated: manage AI Providers from the Coder UI or HTTP API. If set, + this option seeds provider configuration at startup only exactly once. + It will not be used in service runtime. The base URL of the Anthropic + API. --ai-gateway-anthropic-key string, $CODER_AI_GATEWAY_ANTHROPIC_KEY - The key to authenticate against the Anthropic API. + Deprecated: manage AI Providers from the Coder UI or HTTP API. If set, + this option seeds provider configuration at startup only exactly once. + It will not be used in service runtime. The key to authenticate + against the Anthropic API. --ai-gateway-bedrock-access-key string, $CODER_AI_GATEWAY_BEDROCK_ACCESS_KEY - The access key to authenticate against the AWS Bedrock API. - - --ai-gateway-bedrock-access-key-secret string, $CODER_AI_GATEWAY_BEDROCK_ACCESS_KEY_SECRET - The access key secret to use with the access key to authenticate + Deprecated: manage AI Providers from the Coder UI or HTTP API. If set, + this option seeds provider configuration at startup only exactly once. + It will not be used in service runtime. The access key to authenticate against the AWS Bedrock API. + --ai-gateway-bedrock-access-key-secret string, $CODER_AI_GATEWAY_BEDROCK_ACCESS_KEY_SECRET + Deprecated: manage AI Providers from the Coder UI or HTTP API. If set, + this option seeds provider configuration at startup only exactly once. + It will not be used in service runtime. The access key secret to use + with the access key to authenticate against the AWS Bedrock API. + --ai-gateway-bedrock-base-url string, $CODER_AI_GATEWAY_BEDROCK_BASE_URL - The base URL to use for the AWS Bedrock API. Use this setting to - specify an exact URL to use. Takes precedence over - CODER_AI_GATEWAY_BEDROCK_REGION. + Deprecated: manage AI Providers from the Coder UI or HTTP API. If set, + this option seeds provider configuration at startup only exactly once. + It will not be used in service runtime. The base URL to use for the + AWS Bedrock API. Use this setting to specify an exact URL to use. + Takes precedence over CODER_AI_GATEWAY_BEDROCK_REGION. --ai-gateway-bedrock-model string, $CODER_AI_GATEWAY_BEDROCK_MODEL (default: global.anthropic.claude-sonnet-4-5-20250929-v1:0) - The model to use when making requests to the AWS Bedrock API. + Deprecated: manage AI Providers from the Coder UI or HTTP API. If set, + this option seeds provider configuration at startup only exactly once. + It will not be used in service runtime. The model to use when making + requests to the AWS Bedrock API. --ai-gateway-bedrock-region string, $CODER_AI_GATEWAY_BEDROCK_REGION - The AWS Bedrock API region to use. Constructs a base URL to use for - the AWS Bedrock API in the form of - 'https://bedrock-runtime..amazonaws.com'. + Deprecated: manage AI Providers from the Coder UI or HTTP API. If set, + this option seeds provider configuration at startup only exactly once. + It will not be used in service runtime. The AWS Bedrock API region to + use. Constructs a base URL to use for the AWS Bedrock API in the form + of 'https://bedrock-runtime..amazonaws.com'. --ai-gateway-bedrock-small-fastmodel string, $CODER_AI_GATEWAY_BEDROCK_SMALL_FAST_MODEL (default: global.anthropic.claude-haiku-4-5-20251001-v1:0) - The small fast model to use when making requests to the AWS Bedrock - API. Claude Code uses Haiku-class models to perform background tasks. - See + Deprecated: manage AI Providers from the Coder UI or HTTP API. If set, + this option seeds provider configuration at startup only exactly once. + It will not be used in service runtime. The small fast model to use + when making requests to the AWS Bedrock API. Claude Code uses + Haiku-class models to perform background tasks. See https://docs.claude.com/en/docs/claude-code/settings#environment-variables. --ai-gateway-circuit-breaker-enabled bool, $CODER_AI_GATEWAY_CIRCUIT_BREAKER_ENABLED (default: false) @@ -171,10 +191,16 @@ AI GATEWAY OPTIONS: to disable (unlimited). --ai-gateway-openai-base-url string, $CODER_AI_GATEWAY_OPENAI_BASE_URL (default: https://api.openai.com/v1/) - The base URL of the OpenAI API. + Deprecated: manage AI Providers from the Coder UI or HTTP API. If set, + this option seeds provider configuration at startup only exactly once. + It will not be used in service runtime. The base URL of the OpenAI + API. --ai-gateway-openai-key string, $CODER_AI_GATEWAY_OPENAI_KEY - The key to authenticate against the OpenAI API. + Deprecated: manage AI Providers from the Coder UI or HTTP API. If set, + this option seeds provider configuration at startup only exactly once. + It will not be used in service runtime. The key to authenticate + against the OpenAI API. --ai-gateway-rate-limit int, $CODER_AI_GATEWAY_RATE_LIMIT (default: 0) Maximum number of AI Gateway requests per second per replica. Set to 0 diff --git a/cli/testdata/server-config.yaml.golden b/cli/testdata/server-config.yaml.golden index 5bc02e0ae6..613b639553 100644 --- a/cli/testdata/server-config.yaml.golden +++ b/cli/testdata/server-config.yaml.golden @@ -874,25 +874,41 @@ ai_gateway: # Whether to start an in-memory AI Gateway instance. # (default: true, type: bool) enabled: true - # The base URL of the OpenAI API. + # Deprecated: manage AI Providers from the Coder UI or HTTP API. If set, this + # option seeds provider configuration at startup only exactly once. It will not be + # used in service runtime. The base URL of the OpenAI API. # (default: https://api.openai.com/v1/, type: string) openai_base_url: https://api.openai.com/v1/ - # The base URL of the Anthropic API. + # Deprecated: manage AI Providers from the Coder UI or HTTP API. If set, this + # option seeds provider configuration at startup only exactly once. It will not be + # used in service runtime. The base URL of the Anthropic API. # (default: https://api.anthropic.com/, type: string) anthropic_base_url: https://api.anthropic.com/ - # The base URL to use for the AWS Bedrock API. Use this setting to specify an - # exact URL to use. Takes precedence over CODER_AI_GATEWAY_BEDROCK_REGION. + # Deprecated: manage AI Providers from the Coder UI or HTTP API. If set, this + # option seeds provider configuration at startup only exactly once. It will not be + # used in service runtime. The base URL to use for the AWS Bedrock API. Use this + # setting to specify an exact URL to use. Takes precedence over + # CODER_AI_GATEWAY_BEDROCK_REGION. # (default: , type: string) bedrock_base_url: "" - # The AWS Bedrock API region to use. Constructs a base URL to use for the AWS - # Bedrock API in the form of 'https://bedrock-runtime..amazonaws.com'. + # Deprecated: manage AI Providers from the Coder UI or HTTP API. If set, this + # option seeds provider configuration at startup only exactly once. It will not be + # used in service runtime. The AWS Bedrock API region to use. Constructs a base + # URL to use for the AWS Bedrock API in the form of + # 'https://bedrock-runtime..amazonaws.com'. # (default: , type: string) bedrock_region: "" - # The model to use when making requests to the AWS Bedrock API. + # Deprecated: manage AI Providers from the Coder UI or HTTP API. If set, this + # option seeds provider configuration at startup only exactly once. It will not be + # used in service runtime. The model to use when making requests to the AWS + # Bedrock API. # (default: global.anthropic.claude-sonnet-4-5-20250929-v1:0, type: string) bedrock_model: global.anthropic.claude-sonnet-4-5-20250929-v1:0 - # The small fast model to use when making requests to the AWS Bedrock API. Claude - # Code uses Haiku-class models to perform background tasks. See + # Deprecated: manage AI Providers from the Coder UI or HTTP API. If set, this + # option seeds provider configuration at startup only exactly once. It will not be + # used in service runtime. The small fast model to use when making requests to the + # AWS Bedrock API. Claude Code uses Haiku-class models to perform background + # tasks. See # https://docs.claude.com/en/docs/claude-code/settings#environment-variables. # (default: global.anthropic.claude-haiku-4-5-20251001-v1:0, type: string) bedrock_small_fast_model: global.anthropic.claude-haiku-4-5-20251001-v1:0 diff --git a/codersdk/deployment.go b/codersdk/deployment.go index 3fb36c587f..8164222831 100644 --- a/codersdk/deployment.go +++ b/codersdk/deployment.go @@ -1700,6 +1700,7 @@ func (c *DeploymentValues) Options() serpent.OptionSet { } // AI Gateway options + aiGatewayProviderSeedingDeprecated := "Deprecated: manage AI Providers from the Coder UI or HTTP API. If set, this option seeds provider configuration at startup only exactly once. It will not be used in service runtime. " aiGatewayEnabled := serpent.Option{ Name: "AI Gateway Enabled", Description: "Whether to start an in-memory AI Gateway instance.", @@ -1712,7 +1713,7 @@ func (c *DeploymentValues) Options() serpent.OptionSet { } aiGatewayOpenAIBaseURL := serpent.Option{ Name: "AI Gateway OpenAI Base URL", - Description: "The base URL of the OpenAI API.", + Description: aiGatewayProviderSeedingDeprecated + "The base URL of the OpenAI API.", Flag: "ai-gateway-openai-base-url", Env: "CODER_AI_GATEWAY_OPENAI_BASE_URL", Value: &c.AI.BridgeConfig.LegacyOpenAI.BaseURL, @@ -1722,7 +1723,7 @@ func (c *DeploymentValues) Options() serpent.OptionSet { } aiGatewayOpenAIKey := serpent.Option{ Name: "AI Gateway OpenAI Key", - Description: "The key to authenticate against the OpenAI API.", + Description: aiGatewayProviderSeedingDeprecated + "The key to authenticate against the OpenAI API.", Flag: "ai-gateway-openai-key", Env: "CODER_AI_GATEWAY_OPENAI_KEY", Value: &c.AI.BridgeConfig.LegacyOpenAI.Key, @@ -1732,7 +1733,7 @@ func (c *DeploymentValues) Options() serpent.OptionSet { } aiGatewayAnthropicBaseURL := serpent.Option{ Name: "AI Gateway Anthropic Base URL", - Description: "The base URL of the Anthropic API.", + Description: aiGatewayProviderSeedingDeprecated + "The base URL of the Anthropic API.", Flag: "ai-gateway-anthropic-base-url", Env: "CODER_AI_GATEWAY_ANTHROPIC_BASE_URL", Value: &c.AI.BridgeConfig.LegacyAnthropic.BaseURL, @@ -1742,7 +1743,7 @@ func (c *DeploymentValues) Options() serpent.OptionSet { } aiGatewayAnthropicKey := serpent.Option{ Name: "AI Gateway Anthropic Key", - Description: "The key to authenticate against the Anthropic API.", + Description: aiGatewayProviderSeedingDeprecated + "The key to authenticate against the Anthropic API.", Flag: "ai-gateway-anthropic-key", Env: "CODER_AI_GATEWAY_ANTHROPIC_KEY", Value: &c.AI.BridgeConfig.LegacyAnthropic.Key, @@ -1751,30 +1752,28 @@ func (c *DeploymentValues) Options() serpent.OptionSet { Annotations: serpent.Annotations{}.Mark(annotationSecretKey, "true"), } aiGatewayBedrockBaseURL := serpent.Option{ - Name: "AI Gateway Bedrock Base URL", - Description: "The base URL to use for the AWS Bedrock API. Use this setting to specify an exact URL to use. Takes precedence " + - "over CODER_AI_GATEWAY_BEDROCK_REGION.", - Flag: "ai-gateway-bedrock-base-url", - Env: "CODER_AI_GATEWAY_BEDROCK_BASE_URL", - Value: &c.AI.BridgeConfig.LegacyBedrock.BaseURL, - Default: "", - Group: &deploymentGroupAIGateway, - YAML: "bedrock_base_url", + Name: "AI Gateway Bedrock Base URL", + Description: aiGatewayProviderSeedingDeprecated + "The base URL to use for the AWS Bedrock API. Use this setting to specify an exact URL to use. Takes precedence over CODER_AI_GATEWAY_BEDROCK_REGION.", + Flag: "ai-gateway-bedrock-base-url", + Env: "CODER_AI_GATEWAY_BEDROCK_BASE_URL", + Value: &c.AI.BridgeConfig.LegacyBedrock.BaseURL, + Default: "", + Group: &deploymentGroupAIGateway, + YAML: "bedrock_base_url", } aiGatewayBedrockRegion := serpent.Option{ - Name: "AI Gateway Bedrock Region", - Description: "The AWS Bedrock API region to use. Constructs a base URL to use for the AWS Bedrock API in the form of " + - "'https://bedrock-runtime..amazonaws.com'.", - Flag: "ai-gateway-bedrock-region", - Env: "CODER_AI_GATEWAY_BEDROCK_REGION", - Value: &c.AI.BridgeConfig.LegacyBedrock.Region, - Default: "", - Group: &deploymentGroupAIGateway, - YAML: "bedrock_region", + Name: "AI Gateway Bedrock Region", + Description: aiGatewayProviderSeedingDeprecated + "The AWS Bedrock API region to use. Constructs a base URL to use for the AWS Bedrock API in the form of 'https://bedrock-runtime..amazonaws.com'.", + Flag: "ai-gateway-bedrock-region", + Env: "CODER_AI_GATEWAY_BEDROCK_REGION", + Value: &c.AI.BridgeConfig.LegacyBedrock.Region, + Default: "", + Group: &deploymentGroupAIGateway, + YAML: "bedrock_region", } aiGatewayBedrockAccessKey := serpent.Option{ Name: "AI Gateway Bedrock Access Key", - Description: "The access key to authenticate against the AWS Bedrock API.", + Description: aiGatewayProviderSeedingDeprecated + "The access key to authenticate against the AWS Bedrock API.", Flag: "ai-gateway-bedrock-access-key", Env: "CODER_AI_GATEWAY_BEDROCK_ACCESS_KEY", Value: &c.AI.BridgeConfig.LegacyBedrock.AccessKey, @@ -1784,7 +1783,7 @@ func (c *DeploymentValues) Options() serpent.OptionSet { } aiGatewayBedrockAccessKeySecret := serpent.Option{ Name: "AI Gateway Bedrock Access Key Secret", - Description: "The access key secret to use with the access key to authenticate against the AWS Bedrock API.", + Description: aiGatewayProviderSeedingDeprecated + "The access key secret to use with the access key to authenticate against the AWS Bedrock API.", Flag: "ai-gateway-bedrock-access-key-secret", Env: "CODER_AI_GATEWAY_BEDROCK_ACCESS_KEY_SECRET", Value: &c.AI.BridgeConfig.LegacyBedrock.AccessKeySecret, @@ -1794,7 +1793,7 @@ func (c *DeploymentValues) Options() serpent.OptionSet { } aiGatewayBedrockModel := serpent.Option{ Name: "AI Gateway Bedrock Model", - Description: "The model to use when making requests to the AWS Bedrock API.", + Description: aiGatewayProviderSeedingDeprecated + "The model to use when making requests to the AWS Bedrock API.", Flag: "ai-gateway-bedrock-model", Env: "CODER_AI_GATEWAY_BEDROCK_MODEL", Value: &c.AI.BridgeConfig.LegacyBedrock.Model, @@ -1804,7 +1803,7 @@ func (c *DeploymentValues) Options() serpent.OptionSet { } aiGatewayBedrockSmallFastModel := serpent.Option{ Name: "AI Gateway Bedrock Small Fast Model", - Description: "The small fast model to use when making requests to the AWS Bedrock API. Claude Code uses Haiku-class models to perform background tasks. See https://docs.claude.com/en/docs/claude-code/settings#environment-variables.", + Description: aiGatewayProviderSeedingDeprecated + "The small fast model to use when making requests to the AWS Bedrock API. Claude Code uses Haiku-class models to perform background tasks. See https://docs.claude.com/en/docs/claude-code/settings#environment-variables.", Flag: "ai-gateway-bedrock-small-fastmodel", Env: "CODER_AI_GATEWAY_BEDROCK_SMALL_FAST_MODEL", Value: &c.AI.BridgeConfig.LegacyBedrock.SmallFastModel, diff --git a/docs/reference/cli/server.md b/docs/reference/cli/server.md index de694faa79..2de88e4960 100644 --- a/docs/reference/cli/server.md +++ b/docs/reference/cli/server.md @@ -1743,7 +1743,7 @@ Whether to start an in-memory AI Gateway instance. | YAML | ai_gateway.openai_base_url | | Default | https://api.openai.com/v1/ | -The base URL of the OpenAI API. +Deprecated: manage AI Providers from the Coder UI or HTTP API. If set, this option seeds provider configuration at startup only exactly once. It will not be used in service runtime. The base URL of the OpenAI API. ### --ai-gateway-openai-key @@ -1752,7 +1752,7 @@ The base URL of the OpenAI API. | Type | string | | Environment | $CODER_AI_GATEWAY_OPENAI_KEY | -The key to authenticate against the OpenAI API. +Deprecated: manage AI Providers from the Coder UI or HTTP API. If set, this option seeds provider configuration at startup only exactly once. It will not be used in service runtime. The key to authenticate against the OpenAI API. ### --ai-gateway-anthropic-base-url @@ -1763,7 +1763,7 @@ The key to authenticate against the OpenAI API. | YAML | ai_gateway.anthropic_base_url | | Default | https://api.anthropic.com/ | -The base URL of the Anthropic API. +Deprecated: manage AI Providers from the Coder UI or HTTP API. If set, this option seeds provider configuration at startup only exactly once. It will not be used in service runtime. The base URL of the Anthropic API. ### --ai-gateway-anthropic-key @@ -1772,7 +1772,7 @@ The base URL of the Anthropic API. | Type | string | | Environment | $CODER_AI_GATEWAY_ANTHROPIC_KEY | -The key to authenticate against the Anthropic API. +Deprecated: manage AI Providers from the Coder UI or HTTP API. If set, this option seeds provider configuration at startup only exactly once. It will not be used in service runtime. The key to authenticate against the Anthropic API. ### --ai-gateway-bedrock-base-url @@ -1782,7 +1782,7 @@ The key to authenticate against the Anthropic API. | Environment | $CODER_AI_GATEWAY_BEDROCK_BASE_URL | | YAML | ai_gateway.bedrock_base_url | -The base URL to use for the AWS Bedrock API. Use this setting to specify an exact URL to use. Takes precedence over CODER_AI_GATEWAY_BEDROCK_REGION. +Deprecated: manage AI Providers from the Coder UI or HTTP API. If set, this option seeds provider configuration at startup only exactly once. It will not be used in service runtime. The base URL to use for the AWS Bedrock API. Use this setting to specify an exact URL to use. Takes precedence over CODER_AI_GATEWAY_BEDROCK_REGION. ### --ai-gateway-bedrock-region @@ -1792,7 +1792,7 @@ The base URL to use for the AWS Bedrock API. Use this setting to specify an exac | Environment | $CODER_AI_GATEWAY_BEDROCK_REGION | | YAML | ai_gateway.bedrock_region | -The AWS Bedrock API region to use. Constructs a base URL to use for the AWS Bedrock API in the form of 'https://bedrock-runtime..amazonaws.com'. +Deprecated: manage AI Providers from the Coder UI or HTTP API. If set, this option seeds provider configuration at startup only exactly once. It will not be used in service runtime. The AWS Bedrock API region to use. Constructs a base URL to use for the AWS Bedrock API in the form of 'https://bedrock-runtime..amazonaws.com'. ### --ai-gateway-bedrock-access-key @@ -1801,7 +1801,7 @@ The AWS Bedrock API region to use. Constructs a base URL to use for the AWS Bedr | Type | string | | Environment | $CODER_AI_GATEWAY_BEDROCK_ACCESS_KEY | -The access key to authenticate against the AWS Bedrock API. +Deprecated: manage AI Providers from the Coder UI or HTTP API. If set, this option seeds provider configuration at startup only exactly once. It will not be used in service runtime. The access key to authenticate against the AWS Bedrock API. ### --ai-gateway-bedrock-access-key-secret @@ -1810,7 +1810,7 @@ The access key to authenticate against the AWS Bedrock API. | Type | string | | Environment | $CODER_AI_GATEWAY_BEDROCK_ACCESS_KEY_SECRET | -The access key secret to use with the access key to authenticate against the AWS Bedrock API. +Deprecated: manage AI Providers from the Coder UI or HTTP API. If set, this option seeds provider configuration at startup only exactly once. It will not be used in service runtime. The access key secret to use with the access key to authenticate against the AWS Bedrock API. ### --ai-gateway-bedrock-model @@ -1821,7 +1821,7 @@ The access key secret to use with the access key to authenticate against the AWS | YAML | ai_gateway.bedrock_model | | Default | global.anthropic.claude-sonnet-4-5-20250929-v1:0 | -The model to use when making requests to the AWS Bedrock API. +Deprecated: manage AI Providers from the Coder UI or HTTP API. If set, this option seeds provider configuration at startup only exactly once. It will not be used in service runtime. The model to use when making requests to the AWS Bedrock API. ### --ai-gateway-bedrock-small-fastmodel @@ -1832,7 +1832,7 @@ The model to use when making requests to the AWS Bedrock API. | YAML | ai_gateway.bedrock_small_fast_model | | Default | global.anthropic.claude-haiku-4-5-20251001-v1:0 | -The small fast model to use when making requests to the AWS Bedrock API. Claude Code uses Haiku-class models to perform background tasks. See https://docs.claude.com/en/docs/claude-code/settings#environment-variables. +Deprecated: manage AI Providers from the Coder UI or HTTP API. If set, this option seeds provider configuration at startup only exactly once. It will not be used in service runtime. The small fast model to use when making requests to the AWS Bedrock API. Claude Code uses Haiku-class models to perform background tasks. See https://docs.claude.com/en/docs/claude-code/settings#environment-variables. ### --ai-gateway-retention diff --git a/enterprise/cli/testdata/coder_server_--help.golden b/enterprise/cli/testdata/coder_server_--help.golden index a9062a426f..addd3dc256 100644 --- a/enterprise/cli/testdata/coder_server_--help.golden +++ b/enterprise/cli/testdata/coder_server_--help.golden @@ -125,35 +125,55 @@ AI GATEWAY OPTIONS: disabled, only centralized key authentication is permitted. --ai-gateway-anthropic-base-url string, $CODER_AI_GATEWAY_ANTHROPIC_BASE_URL (default: https://api.anthropic.com/) - The base URL of the Anthropic API. + Deprecated: manage AI Providers from the Coder UI or HTTP API. If set, + this option seeds provider configuration at startup only exactly once. + It will not be used in service runtime. The base URL of the Anthropic + API. --ai-gateway-anthropic-key string, $CODER_AI_GATEWAY_ANTHROPIC_KEY - The key to authenticate against the Anthropic API. + Deprecated: manage AI Providers from the Coder UI or HTTP API. If set, + this option seeds provider configuration at startup only exactly once. + It will not be used in service runtime. The key to authenticate + against the Anthropic API. --ai-gateway-bedrock-access-key string, $CODER_AI_GATEWAY_BEDROCK_ACCESS_KEY - The access key to authenticate against the AWS Bedrock API. - - --ai-gateway-bedrock-access-key-secret string, $CODER_AI_GATEWAY_BEDROCK_ACCESS_KEY_SECRET - The access key secret to use with the access key to authenticate + Deprecated: manage AI Providers from the Coder UI or HTTP API. If set, + this option seeds provider configuration at startup only exactly once. + It will not be used in service runtime. The access key to authenticate against the AWS Bedrock API. + --ai-gateway-bedrock-access-key-secret string, $CODER_AI_GATEWAY_BEDROCK_ACCESS_KEY_SECRET + Deprecated: manage AI Providers from the Coder UI or HTTP API. If set, + this option seeds provider configuration at startup only exactly once. + It will not be used in service runtime. The access key secret to use + with the access key to authenticate against the AWS Bedrock API. + --ai-gateway-bedrock-base-url string, $CODER_AI_GATEWAY_BEDROCK_BASE_URL - The base URL to use for the AWS Bedrock API. Use this setting to - specify an exact URL to use. Takes precedence over - CODER_AI_GATEWAY_BEDROCK_REGION. + Deprecated: manage AI Providers from the Coder UI or HTTP API. If set, + this option seeds provider configuration at startup only exactly once. + It will not be used in service runtime. The base URL to use for the + AWS Bedrock API. Use this setting to specify an exact URL to use. + Takes precedence over CODER_AI_GATEWAY_BEDROCK_REGION. --ai-gateway-bedrock-model string, $CODER_AI_GATEWAY_BEDROCK_MODEL (default: global.anthropic.claude-sonnet-4-5-20250929-v1:0) - The model to use when making requests to the AWS Bedrock API. + Deprecated: manage AI Providers from the Coder UI or HTTP API. If set, + this option seeds provider configuration at startup only exactly once. + It will not be used in service runtime. The model to use when making + requests to the AWS Bedrock API. --ai-gateway-bedrock-region string, $CODER_AI_GATEWAY_BEDROCK_REGION - The AWS Bedrock API region to use. Constructs a base URL to use for - the AWS Bedrock API in the form of - 'https://bedrock-runtime..amazonaws.com'. + Deprecated: manage AI Providers from the Coder UI or HTTP API. If set, + this option seeds provider configuration at startup only exactly once. + It will not be used in service runtime. The AWS Bedrock API region to + use. Constructs a base URL to use for the AWS Bedrock API in the form + of 'https://bedrock-runtime..amazonaws.com'. --ai-gateway-bedrock-small-fastmodel string, $CODER_AI_GATEWAY_BEDROCK_SMALL_FAST_MODEL (default: global.anthropic.claude-haiku-4-5-20251001-v1:0) - The small fast model to use when making requests to the AWS Bedrock - API. Claude Code uses Haiku-class models to perform background tasks. - See + Deprecated: manage AI Providers from the Coder UI or HTTP API. If set, + this option seeds provider configuration at startup only exactly once. + It will not be used in service runtime. The small fast model to use + when making requests to the AWS Bedrock API. Claude Code uses + Haiku-class models to perform background tasks. See https://docs.claude.com/en/docs/claude-code/settings#environment-variables. --ai-gateway-circuit-breaker-enabled bool, $CODER_AI_GATEWAY_CIRCUIT_BREAKER_ENABLED (default: false) @@ -172,10 +192,16 @@ AI GATEWAY OPTIONS: to disable (unlimited). --ai-gateway-openai-base-url string, $CODER_AI_GATEWAY_OPENAI_BASE_URL (default: https://api.openai.com/v1/) - The base URL of the OpenAI API. + Deprecated: manage AI Providers from the Coder UI or HTTP API. If set, + this option seeds provider configuration at startup only exactly once. + It will not be used in service runtime. The base URL of the OpenAI + API. --ai-gateway-openai-key string, $CODER_AI_GATEWAY_OPENAI_KEY - The key to authenticate against the OpenAI API. + Deprecated: manage AI Providers from the Coder UI or HTTP API. If set, + this option seeds provider configuration at startup only exactly once. + It will not be used in service runtime. The key to authenticate + against the OpenAI API. --ai-gateway-rate-limit int, $CODER_AI_GATEWAY_RATE_LIMIT (default: 0) Maximum number of AI Gateway requests per second per replica. Set to 0 From 82752844bc96c68ca5f8f9ca251a95c1aec624cc Mon Sep 17 00:00:00 2001 From: Mathias Fredriksson Date: Mon, 1 Jun 2026 16:17:29 +0300 Subject: [PATCH 12/23] fix: isolate MCP HTTP transports from DefaultTransport in tests (#25821) 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. Closes coder/internal#1016 Closes PLAT-291 --- agent/x/agentmcp/manager.go | 20 ++-- agent/x/agentmcp/mcphttpclient.go | 25 ++++ aibridge/mcp/mcphttpclient.go | 25 ++++ aibridge/mcp/proxy_streamable_http.go | 11 ++ coderd/mcp/mcp_e2e_test.go | 137 +++++++++++++++++----- coderd/x/chatd/mcpclient/mcpclient.go | 33 ++---- coderd/x/chatd/mcpclient/mcphttpclient.go | 25 ++++ 7 files changed, 218 insertions(+), 58 deletions(-) create mode 100644 agent/x/agentmcp/mcphttpclient.go create mode 100644 aibridge/mcp/mcphttpclient.go create mode 100644 coderd/x/chatd/mcpclient/mcphttpclient.go diff --git a/agent/x/agentmcp/manager.go b/agent/x/agentmcp/manager.go index 23cba06c18..cd6a705151 100644 --- a/agent/x/agentmcp/manager.go +++ b/agent/x/agentmcp/manager.go @@ -975,15 +975,19 @@ func (m *Manager) createTransport(ctx context.Context, cfg ServerConfig) (transp }), ), nil case "http", "": - return transport.NewStreamableHTTP( - cfg.URL, - transport.WithHTTPHeaders(cfg.Headers), - ) + var opts []transport.StreamableHTTPCOption + opts = append(opts, transport.WithHTTPHeaders(cfg.Headers)) + if c := mcpHTTPClient(); c != nil { + opts = append(opts, transport.WithHTTPBasicClient(c)) + } + return transport.NewStreamableHTTP(cfg.URL, opts...) case "sse": - return transport.NewSSE( - cfg.URL, - transport.WithHeaders(cfg.Headers), - ) + var sseOpts []transport.ClientOption + sseOpts = append(sseOpts, transport.WithHeaders(cfg.Headers)) + if c := mcpHTTPClient(); c != nil { + sseOpts = append(sseOpts, transport.WithHTTPClient(c)) + } + return transport.NewSSE(cfg.URL, sseOpts...) default: return nil, xerrors.Errorf("unsupported transport %q", cfg.Transport) } diff --git a/agent/x/agentmcp/mcphttpclient.go b/agent/x/agentmcp/mcphttpclient.go new file mode 100644 index 0000000000..7099c44281 --- /dev/null +++ b/agent/x/agentmcp/mcphttpclient.go @@ -0,0 +1,25 @@ +package agentmcp + +import ( + "net/http" + "testing" +) + +// mcpHTTPClient returns an isolated *http.Client when running +// inside tests, or nil for production. During tests, +// httptest.Server.Close() calls +// http.DefaultTransport.CloseIdleConnections(), which disrupts +// any MCP client sharing that transport. When DefaultTransport +// is a *http.Transport it is cloned; otherwise a minimal +// transport with ProxyFromEnvironment is created as a fallback. +func mcpHTTPClient() *http.Client { + if !testing.Testing() { + return nil + } + if dt, ok := http.DefaultTransport.(*http.Transport); ok { + return &http.Client{Transport: dt.Clone()} + } + return &http.Client{Transport: &http.Transport{ + Proxy: http.ProxyFromEnvironment, + }} +} diff --git a/aibridge/mcp/mcphttpclient.go b/aibridge/mcp/mcphttpclient.go new file mode 100644 index 0000000000..1685bcf795 --- /dev/null +++ b/aibridge/mcp/mcphttpclient.go @@ -0,0 +1,25 @@ +package mcp + +import ( + "net/http" + "testing" +) + +// mcpHTTPClient returns an isolated *http.Client when running +// inside tests, or nil for production. During tests, +// httptest.Server.Close() calls +// http.DefaultTransport.CloseIdleConnections(), which disrupts +// any MCP client sharing that transport. When DefaultTransport +// is a *http.Transport it is cloned; otherwise a minimal +// transport with ProxyFromEnvironment is created as a fallback. +func mcpHTTPClient() *http.Client { + if !testing.Testing() { + return nil + } + if dt, ok := http.DefaultTransport.(*http.Transport); ok { + return &http.Client{Transport: dt.Clone()} + } + return &http.Client{Transport: &http.Transport{ + Proxy: http.ProxyFromEnvironment, + }} +} diff --git a/aibridge/mcp/proxy_streamable_http.go b/aibridge/mcp/proxy_streamable_http.go index 132c03965a..108a710d19 100644 --- a/aibridge/mcp/proxy_streamable_http.go +++ b/aibridge/mcp/proxy_streamable_http.go @@ -39,6 +39,17 @@ func NewStreamableHTTPServerProxy(serverName, serverURL string, headers map[stri opts = append(opts, transport.WithHTTPHeaders(headers)) } + // Prepend an isolated HTTP client when running in tests so + // httptest.Server.Close() does not disrupt this proxy's + // connections via http.DefaultTransport.CloseIdleConnections(). + // Caller-provided WithHTTPBasicClient in opts overrides this + // (last-wins). + if c := mcpHTTPClient(); c != nil { + opts = append([]transport.StreamableHTTPCOption{ + transport.WithHTTPBasicClient(c), + }, opts...) + } + mcpClient, err := client.NewStreamableHttpClient(serverURL, opts...) if err != nil { return nil, xerrors.Errorf("create streamable http client: %w", err) diff --git a/coderd/mcp/mcp_e2e_test.go b/coderd/mcp/mcp_e2e_test.go index 5b374e36b8..633c68582a 100644 --- a/coderd/mcp/mcp_e2e_test.go +++ b/coderd/mcp/mcp_e2e_test.go @@ -12,6 +12,7 @@ import ( "os" "path/filepath" "strings" + "sync/atomic" "testing" "github.com/google/uuid" @@ -57,11 +58,10 @@ func TestMCPHTTP_E2E_ClientIntegration(t *testing.T) { mcpURL := api.AccessURL.String() + mcpserver.MCPEndpoint // Configure client with authentication headers using RFC 6750 Bearer token - mcpClient, err := mcpclient.NewStreamableHttpClient(mcpURL, + mcpClient := newIsolatedMCPClient(t, mcpURL, transport.WithHTTPHeaders(map[string]string{ "Authorization": "Bearer " + coderClient.SessionToken(), })) - require.NoError(t, err) defer func() { if closeErr := mcpClient.Close(); closeErr != nil { t.Logf("Failed to close MCP client: %v", closeErr) @@ -72,7 +72,7 @@ func TestMCPHTTP_E2E_ClientIntegration(t *testing.T) { defer cancel() // Start client - err = mcpClient.Start(ctx) + err := mcpClient.Start(ctx) require.NoError(t, err) // Initialize connection @@ -190,8 +190,7 @@ func TestMCPHTTP_E2E_UnauthenticatedAccess(t *testing.T) { require.Equal(t, http.StatusUnauthorized, resp.StatusCode, "Should get HTTP 401 for unauthenticated access") // Also test with MCP client to ensure it handles the error gracefully - mcpClient, err := mcpclient.NewStreamableHttpClient(mcpURL) - require.NoError(t, err, "Should be able to create MCP client without authentication") + mcpClient := newIsolatedMCPClient(t, mcpURL) defer func() { if closeErr := mcpClient.Close(); closeErr != nil { t.Logf("Failed to close MCP client: %v", closeErr) @@ -245,11 +244,10 @@ func TestMCPHTTP_E2E_ToolWithWorkspace(t *testing.T) { coderdtest.NewWorkspaceAgentWaiter(t, coderClient, r.Workspace.ID).Wait() mcpURL := api.AccessURL.String() + mcpserver.MCPEndpoint - mcpClient, err := mcpclient.NewStreamableHttpClient(mcpURL, + mcpClient := newIsolatedMCPClient(t, mcpURL, transport.WithHTTPHeaders(map[string]string{ "Authorization": "Bearer " + coderClient.SessionToken(), })) - require.NoError(t, err) defer func() { if closeErr := mcpClient.Close(); closeErr != nil { t.Logf("Failed to close MCP client: %v", closeErr) @@ -260,7 +258,7 @@ func TestMCPHTTP_E2E_ToolWithWorkspace(t *testing.T) { defer cancel() require.NoError(t, mcpClient.Start(ctx)) - _, err = mcpClient.Initialize(ctx, mcp.InitializeRequest{ + _, err := mcpClient.Initialize(ctx, mcp.InitializeRequest{ Params: mcp.InitializeParams{ ProtocolVersion: mcp.LATEST_PROTOCOL_VERSION, ClientInfo: mcp.Implementation{ @@ -307,11 +305,10 @@ func TestMCPHTTP_E2E_ErrorHandling(t *testing.T) { // Create MCP client mcpURL := api.AccessURL.String() + mcpserver.MCPEndpoint - mcpClient, err := mcpclient.NewStreamableHttpClient(mcpURL, + mcpClient := newIsolatedMCPClient(t, mcpURL, transport.WithHTTPHeaders(map[string]string{ "Authorization": "Bearer " + coderClient.SessionToken(), })) - require.NoError(t, err) defer func() { if closeErr := mcpClient.Close(); closeErr != nil { t.Logf("Failed to close MCP client: %v", closeErr) @@ -322,7 +319,7 @@ func TestMCPHTTP_E2E_ErrorHandling(t *testing.T) { defer cancel() // Start and initialize client - err = mcpClient.Start(ctx) + err := mcpClient.Start(ctx) require.NoError(t, err) initReq := mcp.InitializeRequest{ @@ -366,11 +363,10 @@ func TestMCPHTTP_E2E_ConcurrentRequests(t *testing.T) { // Create MCP client mcpURL := api.AccessURL.String() + mcpserver.MCPEndpoint - mcpClient, err := mcpclient.NewStreamableHttpClient(mcpURL, + mcpClient := newIsolatedMCPClient(t, mcpURL, transport.WithHTTPHeaders(map[string]string{ "Authorization": "Bearer " + coderClient.SessionToken(), })) - require.NoError(t, err) defer func() { if closeErr := mcpClient.Close(); closeErr != nil { t.Logf("Failed to close MCP client: %v", closeErr) @@ -381,7 +377,7 @@ func TestMCPHTTP_E2E_ConcurrentRequests(t *testing.T) { defer cancel() // Start and initialize client - err = mcpClient.Start(ctx) + err := mcpClient.Start(ctx) require.NoError(t, err) initReq := mcp.InitializeRequest{ @@ -520,11 +516,10 @@ func TestMCPHTTP_E2E_OAuth2_EndToEnd(t *testing.T) { sessionToken := coderClient.SessionToken() mcpURL := api.AccessURL.String() + mcpserver.MCPEndpoint - mcpClient, err := mcpclient.NewStreamableHttpClient(mcpURL, + mcpClient := newIsolatedMCPClient(t, mcpURL, transport.WithHTTPHeaders(map[string]string{ "Authorization": "Bearer " + sessionToken, })) - require.NoError(t, err) defer func() { if closeErr := mcpClient.Close(); closeErr != nil { t.Logf("Failed to close MCP client: %v", closeErr) @@ -669,11 +664,10 @@ func TestMCPHTTP_E2E_OAuth2_EndToEnd(t *testing.T) { // Step 3: Use access token to authenticate with MCP endpoint mcpURL := api.AccessURL.String() + mcpserver.MCPEndpoint - mcpClient, err := mcpclient.NewStreamableHttpClient(mcpURL, + mcpClient := newIsolatedMCPClient(t, mcpURL, transport.WithHTTPHeaders(map[string]string{ "Authorization": "Bearer " + accessToken, })) - require.NoError(t, err) defer func() { if closeErr := mcpClient.Close(); closeErr != nil { t.Logf("Failed to close MCP client: %v", closeErr) @@ -762,11 +756,10 @@ func TestMCPHTTP_E2E_OAuth2_EndToEnd(t *testing.T) { t.Logf("Successfully refreshed token: %s...", newAccessToken[:10]) // Step 5: Use new access token to create another MCP connection - newMcpClient, err := mcpclient.NewStreamableHttpClient(mcpURL, + newMcpClient := newIsolatedMCPClient(t, mcpURL, transport.WithHTTPHeaders(map[string]string{ "Authorization": "Bearer " + newAccessToken, })) - require.NoError(t, err) defer func() { if closeErr := newMcpClient.Close(); closeErr != nil { t.Logf("Failed to close new MCP client: %v", closeErr) @@ -990,11 +983,10 @@ func TestMCPHTTP_E2E_OAuth2_EndToEnd(t *testing.T) { t.Logf("Successfully obtained access token: %s...", accessToken[:10]) // Step 5: Use access token to get user information via MCP - mcpClient, err := mcpclient.NewStreamableHttpClient(mcpURL, + mcpClient := newIsolatedMCPClient(t, mcpURL, transport.WithHTTPHeaders(map[string]string{ "Authorization": "Bearer " + accessToken, })) - require.NoError(t, err) defer func() { if closeErr := mcpClient.Close(); closeErr != nil { t.Logf("Failed to close MCP client: %v", closeErr) @@ -1088,11 +1080,10 @@ func TestMCPHTTP_E2E_OAuth2_EndToEnd(t *testing.T) { t.Logf("Successfully refreshed token: %s...", newAccessToken[:10]) // Step 7: Use refreshed token to get user information again via MCP - newMcpClient, err := mcpclient.NewStreamableHttpClient(mcpURL, + newMcpClient := newIsolatedMCPClient(t, mcpURL, transport.WithHTTPHeaders(map[string]string{ "Authorization": "Bearer " + newAccessToken, })) - require.NoError(t, err) defer func() { if closeErr := newMcpClient.Close(); closeErr != nil { t.Logf("Failed to close new MCP client: %v", closeErr) @@ -1268,11 +1259,10 @@ func TestMCPHTTP_E2E_ChatGPTEndpoint(t *testing.T) { mcpURL := api.AccessURL.String() + mcpserver.MCPEndpoint + "?toolset=chatgpt" // Configure client with authentication headers using RFC 6750 Bearer token - mcpClient, err := mcpclient.NewStreamableHttpClient(mcpURL, + mcpClient := newIsolatedMCPClient(t, mcpURL, transport.WithHTTPHeaders(map[string]string{ "Authorization": "Bearer " + coderClient.SessionToken(), })) - require.NoError(t, err) t.Cleanup(func() { if closeErr := mcpClient.Close(); closeErr != nil { t.Logf("Failed to close MCP client: %v", closeErr) @@ -1283,7 +1273,7 @@ func TestMCPHTTP_E2E_ChatGPTEndpoint(t *testing.T) { defer cancel() // Start client - err = mcpClient.Start(ctx) + err := mcpClient.Start(ctx) require.NoError(t, err) // Initialize connection @@ -1433,11 +1423,10 @@ func TestMCPHTTP_E2E_WorkspaceSSHAuthz(t *testing.T) { // Connect with the template-admin user. mcpURL := api.AccessURL.String() + mcpserver.MCPEndpoint - mcpClient, err := mcpclient.NewStreamableHttpClient(mcpURL, + mcpClient := newIsolatedMCPClient(t, mcpURL, transport.WithHTTPHeaders(map[string]string{ "Authorization": "Bearer " + tmplAdminClient.SessionToken(), })) - require.NoError(t, err) defer func() { _ = mcpClient.Close() }() @@ -1446,7 +1435,7 @@ func TestMCPHTTP_E2E_WorkspaceSSHAuthz(t *testing.T) { defer cancel() require.NoError(t, mcpClient.Start(ctx)) - _, err = mcpClient.Initialize(ctx, mcp.InitializeRequest{ + _, err := mcpClient.Initialize(ctx, mcp.InitializeRequest{ Params: mcp.InitializeParams{ ProtocolVersion: mcp.LATEST_PROTOCOL_VERSION, ClientInfo: mcp.Implementation{ @@ -1489,3 +1478,91 @@ func mustParseURL(t *testing.T, rawURL string) *url.URL { require.NoError(t, err, "Failed to parse URL %q", rawURL) return u } + +// newIsolatedMCPClient creates a streamable HTTP MCP client that uses +// an isolated http.Transport cloned from http.DefaultTransport. +// This prevents httptest.Server.Close() (which calls +// http.DefaultTransport.CloseIdleConnections()) from disrupting the +// client's connections during parallel tests. +func newIsolatedMCPClient(t *testing.T, mcpURL string, opts ...transport.StreamableHTTPCOption) *mcpclient.Client { + t.Helper() + isolated := coderdtest.NewIsolatedHTTPClient(nil) + opts = append([]transport.StreamableHTTPCOption{transport.WithHTTPBasicClient(isolated)}, opts...) + client, err := mcpclient.NewStreamableHttpClient(mcpURL, opts...) + require.NoError(t, err) + return client +} + +// sentinelTransport wraps an http.RoundTripper and counts how many +// requests flow through it. Used as a test sentinel to verify +// whether a client is (or is not) using http.DefaultTransport. +type sentinelTransport struct { + inner http.RoundTripper + hits atomic.Int64 +} + +func (s *sentinelTransport) RoundTrip(req *http.Request) (*http.Response, error) { + s.hits.Add(1) + return s.inner.RoundTrip(req) +} + +// TestMCPHTTP_E2E_TransportIsolation verifies that the +// newIsolatedMCPClient helper creates clients that do NOT route +// requests through http.DefaultTransport, while raw +// mcpclient.NewStreamableHttpClient (without explicit +// WithHTTPBasicClient) does use it. +// +//nolint:paralleltest // Mutates http.DefaultTransport. +func TestMCPHTTP_E2E_TransportIsolation(t *testing.T) { + // Replace DefaultTransport with a counting sentinel. + original := http.DefaultTransport + sentinel := &sentinelTransport{inner: original} + http.DefaultTransport = sentinel + t.Cleanup(func() { http.DefaultTransport = original }) + + coderClient, closer, api := coderdtest.NewWithAPI(t, nil) + t.Cleanup(func() { closer.Close() }) + _ = coderdtest.CreateFirstUser(t, coderClient) + + mcpURL := api.AccessURL.String() + mcpserver.MCPEndpoint + authOpt := transport.WithHTTPHeaders(map[string]string{ + "Authorization": "Bearer " + coderClient.SessionToken(), + }) + + ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitLong) + defer cancel() + + initReq := mcp.InitializeRequest{ + Params: mcp.InitializeParams{ + ProtocolVersion: mcp.LATEST_PROTOCOL_VERSION, + ClientInfo: mcp.Implementation{Name: "sentinel-test", Version: "1.0.0"}, + }, + } + + t.Run("RawClientUsesDefaultTransport", func(t *testing.T) { + sentinel.hits.Store(0) + rawClient, err := mcpclient.NewStreamableHttpClient(mcpURL, authOpt) + require.NoError(t, err) + defer func() { _ = rawClient.Close() }() + + require.NoError(t, rawClient.Start(ctx)) + _, err = rawClient.Initialize(ctx, initReq) + require.NoError(t, err) + + require.Greater(t, sentinel.hits.Load(), int64(0), + "raw client should route requests through http.DefaultTransport") + }) + + t.Run("IsolatedClientBypassesDefaultTransport", func(t *testing.T) { + sentinel.hits.Store(0) + isoClient := newIsolatedMCPClient(t, mcpURL, authOpt) + defer func() { _ = isoClient.Close() }() + + require.NoError(t, isoClient.Start(ctx)) + _, err := isoClient.Initialize(ctx, initReq) + require.NoError(t, err) + + require.Equal(t, int64(0), sentinel.hits.Load(), + "isolated client must NOT route requests through http.DefaultTransport") + }) +} diff --git a/coderd/x/chatd/mcpclient/mcpclient.go b/coderd/x/chatd/mcpclient/mcpclient.go index 16ef5ed9fd..cb7e0322c2 100644 --- a/coderd/x/chatd/mcpclient/mcpclient.go +++ b/coderd/x/chatd/mcpclient/mcpclient.go @@ -285,31 +285,24 @@ func createTransport( cfg database.MCPServerConfig, headers map[string]string, ) (transport.Interface, error) { - // Each connection gets its own HTTP client with a dedicated - // transport so that httptest.Server.Close() (which calls - // CloseIdleConnections on http.DefaultTransport) does not - // disrupt unrelated connections during parallel tests. - var httpClient *http.Client - if dt, ok := http.DefaultTransport.(*http.Transport); ok { - httpClient = &http.Client{Transport: dt.Clone()} - } else { - httpClient = &http.Client{} - } + httpClient := mcpHTTPClient() switch cfg.Transport { case "sse": - return transport.NewSSE( - cfg.Url, - transport.WithHeaders(headers), - transport.WithHTTPClient(httpClient), - ) + var opts []transport.ClientOption + opts = append(opts, transport.WithHeaders(headers)) + if httpClient != nil { + opts = append(opts, transport.WithHTTPClient(httpClient)) + } + return transport.NewSSE(cfg.Url, opts...) case "", "streamable_http": // Default to streamable HTTP, the newer transport. - return transport.NewStreamableHTTP( - cfg.Url, - transport.WithHTTPHeaders(headers), - transport.WithHTTPBasicClient(httpClient), - ) + var opts []transport.StreamableHTTPCOption + opts = append(opts, transport.WithHTTPHeaders(headers)) + if httpClient != nil { + opts = append(opts, transport.WithHTTPBasicClient(httpClient)) + } + return transport.NewStreamableHTTP(cfg.Url, opts...) default: return nil, xerrors.Errorf( "unsupported transport %q", cfg.Transport, diff --git a/coderd/x/chatd/mcpclient/mcphttpclient.go b/coderd/x/chatd/mcpclient/mcphttpclient.go new file mode 100644 index 0000000000..c34ff59262 --- /dev/null +++ b/coderd/x/chatd/mcpclient/mcphttpclient.go @@ -0,0 +1,25 @@ +package mcpclient + +import ( + "net/http" + "testing" +) + +// mcpHTTPClient returns an isolated *http.Client when running +// inside tests, or nil for production. During tests, +// httptest.Server.Close() calls +// http.DefaultTransport.CloseIdleConnections(), which disrupts +// any MCP client sharing that transport. When DefaultTransport +// is a *http.Transport it is cloned; otherwise a minimal +// transport with ProxyFromEnvironment is created as a fallback. +func mcpHTTPClient() *http.Client { + if !testing.Testing() { + return nil + } + if dt, ok := http.DefaultTransport.(*http.Transport); ok { + return &http.Client{Transport: dt.Clone()} + } + return &http.Client{Transport: &http.Transport{ + Proxy: http.ProxyFromEnvironment, + }} +} From a85462bd4973efd96e2d43761266ccc8c3ebc60f Mon Sep 17 00:00:00 2001 From: Danny Kopping Date: Mon, 1 Jun 2026 15:26:37 +0200 Subject: [PATCH 13/23] feat: support adding GitHub Copilot AI provider via UI (#25888) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Copilot is the only AI provider type that could not be added through the `/ai/settings` UI. The aibridge runtime and the env-var seeding path already supported it, but the runtime CRUD API rejected `type=copilot` and the UI omitted it entirely. The root cause is that Copilot's auth model (a per-request GitHub OAuth token, with no pre-shared key) does not fit the credential-centric add-provider flow that every other provider uses. ## Backend Allow `type=copilot` in `CreateAIProviderRequest.Validate()`, and reject `api_keys` for Copilot on both create (validation) and update (handler sentinel), mirroring the existing Bedrock guards. Copilot carries no stored credential. ## Frontend Add Copilot to the provider type picker (with the `github-copilot.svg` icon) and give the form a credential-free branch: name, display name, and a free-text endpoint defaulting to `https://api.business.githubcopilot.com`, with copy explaining that authentication happens via the user's GitHub token at request time. Copilot maps to the distinct `copilot` wire type rather than collapsing to `openai`, and the edit flow recovers it correctly. The endpoint stays required with a business-tier default; users on the individual or enterprise endpoints edit the field. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --- coderd/ai_providers.go | 16 +++++ coderd/ai_providers_test.go | 69 +++++++++++++++++++ codersdk/aiproviders.go | 12 +++- enterprise/aibridgeproxyd/aibridgeproxyd.go | 35 +++++++++- .../aibridgeproxyd_internal_test.go | 68 ++++++++++++++++++ site/src/api/typesGenerated.ts | 5 +- .../AddProviderPageView.stories.tsx | 23 +++++++ .../AddProviderPage/AddProviderPageView.tsx | 7 +- .../UpdateProviderPageView.stories.tsx | 16 +++++ .../UpdateProviderPageView.tsx | 12 ++-- .../components/ProviderForm.stories.tsx | 32 +++++++++ .../ProvidersPage/components/ProviderForm.tsx | 61 +++++++++++++--- .../components/ProviderIcon.stories.tsx | 6 ++ .../ProvidersPage/components/ProviderIcon.tsx | 4 ++ .../components/addableProviderTypes.ts | 1 + .../components/providerFormApiMap.test.ts | 63 +++++++++++++++++ .../components/providerFormApiMap.ts | 49 +++++++++---- site/src/testHelpers/entities.ts | 14 ++++ 18 files changed, 457 insertions(+), 36 deletions(-) create mode 100644 enterprise/aibridgeproxyd/aibridgeproxyd_internal_test.go diff --git a/coderd/ai_providers.go b/coderd/ai_providers.go index 78ca50ecc9..0637822592 100644 --- a/coderd/ai_providers.go +++ b/coderd/ai_providers.go @@ -340,6 +340,10 @@ func (api *API) aiProvidersUpdate(rw http.ResponseWriter, r *http.Request) { return errBedrockRejectsAPIKeys } + if req.APIKeys != nil && old.Type == database.AiProviderTypeCopilot && len(*req.APIKeys) > 0 { + return errCopilotRejectsAPIKeys + } + displayName := old.DisplayName if req.DisplayName != nil { // Empty string clears the column. @@ -383,6 +387,12 @@ func (api *API) aiProvidersUpdate(rw http.ResponseWriter, r *http.Request) { }) return } + if errors.Is(err, errCopilotRejectsAPIKeys) { + httpapi.Write(ctx, rw, http.StatusBadRequest, codersdk.Response{ + Message: "Copilot providers do not accept api_keys; they authenticate via request-time GitHub OAuth tokens.", + }) + return + } if errors.Is(err, errAIProviderBedrockTypeMismatch) { httpapi.Write(ctx, rw, http.StatusBadRequest, codersdk.Response{ Message: "Bedrock settings are only valid for type=anthropic or type=bedrock.", @@ -483,6 +493,12 @@ func (api *API) publishAIProvidersChanged(ctx context.Context) { // Bedrock-typed provider; the outer handler translates it into a 400. var errBedrockRejectsAPIKeys = xerrors.New("bedrock providers do not accept api_keys") +// errCopilotRejectsAPIKeys is the sentinel returned from inside the +// update transaction when a caller attempts to attach api_keys to a +// Copilot-typed provider; the outer handler translates it into a 400. +// Copilot authenticates via request-time GitHub OAuth tokens. +var errCopilotRejectsAPIKeys = xerrors.New("copilot providers do not accept api_keys") + // errAIProviderBedrockTypeMismatch is the sentinel returned from // inside the update transaction when the post-merge settings carry a // Bedrock block but the provider is not anthropic- or bedrock-typed; diff --git a/coderd/ai_providers_test.go b/coderd/ai_providers_test.go index c020f90c26..e4f4f27a06 100644 --- a/coderd/ai_providers_test.go +++ b/coderd/ai_providers_test.go @@ -889,6 +889,75 @@ func TestAIProvidersKeyManagement(t *testing.T) { require.Contains(t, sdkErr.Message, "Bedrock providers do not accept api_keys") }) + t.Run("CopilotCreateWithoutKeys", func(t *testing.T) { + t.Parallel() + client := coderdtest.New(t, nil) + _ = coderdtest.CreateFirstUser(t, client) + ctx := testutil.Context(t, testutil.WaitLong) + + //nolint:gocritic // Owner role is the audience for this endpoint. + provider, err := client.CreateAIProvider(ctx, codersdk.CreateAIProviderRequest{ + Type: codersdk.AIProviderTypeCopilot, + Name: "keys-copilot", + Enabled: true, + BaseURL: "https://api.business.githubcopilot.com", + }) + require.NoError(t, err) + require.Equal(t, codersdk.AIProviderTypeCopilot, provider.Type) + require.Empty(t, provider.APIKeys) + }) + + t.Run("CopilotRejectsCreateWithKeys", func(t *testing.T) { + t.Parallel() + client := coderdtest.New(t, nil) + _ = coderdtest.CreateFirstUser(t, client) + ctx := testutil.Context(t, testutil.WaitLong) + + //nolint:gocritic // Owner role is the audience for this endpoint. + _, err := client.CreateAIProvider(ctx, codersdk.CreateAIProviderRequest{ + Type: codersdk.AIProviderTypeCopilot, + Name: "keys-copilot-create", + Enabled: true, + BaseURL: "https://api.business.githubcopilot.com", + APIKeys: []string{"sk-should-be-rejected"}, //nolint:gosec // test fixture, not a real credential + }) + require.Error(t, err) + var sdkErr *codersdk.Error + require.ErrorAs(t, err, &sdkErr) + require.Equal(t, http.StatusBadRequest, sdkErr.StatusCode()) + require.Len(t, sdkErr.Validations, 1) + require.Equal(t, "api_keys", sdkErr.Validations[0].Field) + require.Contains(t, sdkErr.Validations[0].Detail, "type=copilot does not accept api_keys") + }) + + t.Run("CopilotRejectsUpdateWithKeys", func(t *testing.T) { + t.Parallel() + client := coderdtest.New(t, nil) + _ = coderdtest.CreateFirstUser(t, client) + ctx := testutil.Context(t, testutil.WaitLong) + + //nolint:gocritic // Owner role is the audience for this endpoint. + provider, err := client.CreateAIProvider(ctx, codersdk.CreateAIProviderRequest{ + Type: codersdk.AIProviderTypeCopilot, + Name: "keys-copilot-update", + Enabled: true, + BaseURL: "https://api.business.githubcopilot.com", + }) + require.NoError(t, err) + + rejected := []codersdk.AIProviderKeyMutation{ + {APIKey: ptr.Ref("sk-copilot-no")}, //nolint:gosec // test fixture, not a real credential + } + _, err = client.UpdateAIProvider(ctx, provider.Name, codersdk.UpdateAIProviderRequest{ + APIKeys: &rejected, + }) + require.Error(t, err) + var sdkErr *codersdk.Error + require.ErrorAs(t, err, &sdkErr) + require.Equal(t, http.StatusBadRequest, sdkErr.StatusCode()) + require.Contains(t, sdkErr.Message, "Copilot providers do not accept api_keys") + }) + t.Run("EmptyKeyRejected", func(t *testing.T) { t.Parallel() client := coderdtest.New(t, nil) diff --git a/codersdk/aiproviders.go b/codersdk/aiproviders.go index fe34b9c03c..7b513340bc 100644 --- a/codersdk/aiproviders.go +++ b/codersdk/aiproviders.go @@ -188,8 +188,9 @@ type AIProviderKey struct { // CreateAIProviderRequest is the payload for creating a new AI // provider. Name and Type are required. APIKeys carries the plaintext -// keys for OpenAI/Anthropic providers; Bedrock providers authenticate -// via Settings and must omit APIKeys. +// keys for OpenAI/Anthropic providers; Bedrock and Copilot providers +// must omit APIKeys (Bedrock authenticates via Settings, Copilot via +// request-time GitHub OAuth tokens). type CreateAIProviderRequest struct { Type AIProviderType `json:"type"` Name string `json:"name"` @@ -209,6 +210,7 @@ func (req CreateAIProviderRequest) Validate() []ValidationError { AIProviderTypeAnthropic, AIProviderTypeAzure, AIProviderTypeBedrock, + AIProviderTypeCopilot, AIProviderTypeGoogle, AIProviderTypeOpenAICompat, AIProviderTypeOpenrouter, @@ -244,6 +246,12 @@ func (req CreateAIProviderRequest) Validate() []ValidationError { Detail: "type=bedrock does not accept api_keys", }) } + if req.Type == AIProviderTypeCopilot && len(req.APIKeys) > 0 { + validations = append(validations, ValidationError{ + Field: "api_keys", + Detail: "type=copilot does not accept api_keys", + }) + } return validations } diff --git a/enterprise/aibridgeproxyd/aibridgeproxyd.go b/enterprise/aibridgeproxyd/aibridgeproxyd.go index cfcb2071c4..438d7c46b7 100644 --- a/enterprise/aibridgeproxyd/aibridgeproxyd.go +++ b/enterprise/aibridgeproxyd/aibridgeproxyd.go @@ -1076,9 +1076,11 @@ func (s *Server) handleResponse(resp *http.Response, ctx *goproxy.ProxyCtx) *htt switch { case resp.StatusCode >= http.StatusInternalServerError: - logger.Error(s.ctx, "received error response from aibridged") + logger.Error(s.ctx, "received error response from aibridged", + slog.F("response_body", s.readErrorBodyForLog(resp, logger))) case resp.StatusCode >= http.StatusBadRequest: - logger.Warn(s.ctx, "received error response from aibridged") + logger.Warn(s.ctx, "received error response from aibridged", + slog.F("response_body", s.readErrorBodyForLog(resp, logger))) default: logger.Debug(s.ctx, "received response from aibridged") } @@ -1101,6 +1103,35 @@ func (s *Server) handleResponse(resp *http.Response, ctx *goproxy.ProxyCtx) *htt return resp } +// maxLoggedErrorBodyBytes bounds how much of an aibridged error response +// body is rendered into a log line, so a large upstream error payload +// cannot blow up log volume. +const maxLoggedErrorBodyBytes = 16 << 10 // 16 KiB + +// readErrorBodyForLog reads resp.Body for diagnostic logging and restores +// it with an equivalent reader, so the proxy still forwards the body +// downstream and the response dumper can read it again. The returned +// string is truncated to maxLoggedErrorBodyBytes; the restored body is +// always complete. +func (s *Server) readErrorBodyForLog(resp *http.Response, logger slog.Logger) string { + if resp.Body == nil { + return "" + } + body, err := io.ReadAll(resp.Body) + _ = resp.Body.Close() + // Restore the full body even on a read error: the proxy and dumper + // downstream still expect a readable body, and a partial body is + // better than a nil one. + resp.Body = io.NopCloser(bytes.NewReader(body)) + if err != nil { + logger.Warn(s.ctx, "failed to read aibridged error response body", slog.Error(err)) + } + if len(body) > maxLoggedErrorBodyBytes { + return string(body[:maxLoggedErrorBodyBytes]) + "...(truncated)" + } + return string(body) +} + // Handler returns an HTTP handler for the AI Bridge Proxy's HTTP endpoints. // This is separate from the proxy server itself and is used by coderd to // serve endpoints like the CA certificate. diff --git a/enterprise/aibridgeproxyd/aibridgeproxyd_internal_test.go b/enterprise/aibridgeproxyd/aibridgeproxyd_internal_test.go new file mode 100644 index 0000000000..397ed1cf3e --- /dev/null +++ b/enterprise/aibridgeproxyd/aibridgeproxyd_internal_test.go @@ -0,0 +1,68 @@ +package aibridgeproxyd + +import ( + "bytes" + "io" + "net/http" + "strings" + "testing" + + "github.com/stretchr/testify/require" + + "cdr.dev/slog/v3/sloggers/slogtest" +) + +// TestReadErrorBodyForLog verifies that reading an aibridged error +// response body for logging leaves the body intact for downstream +// consumers (the proxy forwards it, and the response dumper reads it +// again), and that the logged rendering is capped. +func TestReadErrorBodyForLog(t *testing.T) { + t.Parallel() + + newResponse := func(body string) *http.Response { + return &http.Response{ + StatusCode: http.StatusBadRequest, + Body: io.NopCloser(strings.NewReader(body)), + } + } + + t.Run("ReturnsBodyAndRestores", func(t *testing.T) { + t.Parallel() + s := &Server{ctx: t.Context(), logger: slogtest.Make(t, nil)} + resp := newResponse(`{"error":"bad request"}`) + + got := s.readErrorBodyForLog(resp, s.logger) + require.Equal(t, `{"error":"bad request"}`, got) + + // The body must still be readable in full for the proxy and the + // response dumper. + restored, err := io.ReadAll(resp.Body) + require.NoError(t, err) + require.Equal(t, `{"error":"bad request"}`, string(restored)) + }) + + t.Run("TruncatesLargeBodyButRestoresFull", func(t *testing.T) { + t.Parallel() + s := &Server{ctx: t.Context(), logger: slogtest.Make(t, nil)} + full := bytes.Repeat([]byte("a"), maxLoggedErrorBodyBytes+512) + resp := newResponse(string(full)) + + got := s.readErrorBodyForLog(resp, s.logger) + require.Len(t, got, maxLoggedErrorBodyBytes+len("...(truncated)")) + require.True(t, strings.HasSuffix(got, "...(truncated)")) + + // Truncation only affects the log string; the restored body is + // the complete payload. + restored, err := io.ReadAll(resp.Body) + require.NoError(t, err) + require.Equal(t, full, restored) + }) + + t.Run("NilBody", func(t *testing.T) { + t.Parallel() + s := &Server{ctx: t.Context(), logger: slogtest.Make(t, nil)} + resp := &http.Response{StatusCode: http.StatusInternalServerError, Body: nil} + + require.Equal(t, "", s.readErrorBodyForLog(resp, s.logger)) + }) +} diff --git a/site/src/api/typesGenerated.ts b/site/src/api/typesGenerated.ts index c4fae7a3d0..a42e1489ff 100644 --- a/site/src/api/typesGenerated.ts +++ b/site/src/api/typesGenerated.ts @@ -3240,8 +3240,9 @@ export interface ConvertLoginRequest { /** * CreateAIProviderRequest is the payload for creating a new AI * provider. Name and Type are required. APIKeys carries the plaintext - * keys for OpenAI/Anthropic providers; Bedrock providers authenticate - * via Settings and must omit APIKeys. + * keys for OpenAI/Anthropic providers; Bedrock and Copilot providers + * must omit APIKeys (Bedrock authenticates via Settings, Copilot via + * request-time GitHub OAuth tokens). */ export interface CreateAIProviderRequest { readonly type: AIProviderType; diff --git a/site/src/pages/AISettingsPage/ProvidersPage/AddProviderPage/AddProviderPageView.stories.tsx b/site/src/pages/AISettingsPage/ProvidersPage/AddProviderPage/AddProviderPageView.stories.tsx index f7bbab9761..176f8fb9ed 100644 --- a/site/src/pages/AISettingsPage/ProvidersPage/AddProviderPage/AddProviderPageView.stories.tsx +++ b/site/src/pages/AISettingsPage/ProvidersPage/AddProviderPage/AddProviderPageView.stories.tsx @@ -1,4 +1,5 @@ import type { Meta, StoryObj } from "@storybook/react-vite"; +import { within } from "storybook/test"; import { reactRouterParameters } from "storybook-addon-remix-react-router"; import { withToaster } from "#/testHelpers/storybook"; import { addableProviders } from "../components/addableProviderTypes"; @@ -26,16 +27,38 @@ export const AddAnthropic: Story = { args: { provider: addableProviders.find((p) => p.value === "anthropic")!, }, + play: async ({ canvasElement }) => { + const canvas = within(canvasElement); + await canvas.findByText("Add an Anthropic provider"); + }, }; export const AddOpenAI: Story = { args: { provider: addableProviders.find((p) => p.value === "openai")!, }, + play: async ({ canvasElement }) => { + const canvas = within(canvasElement); + await canvas.findByText("Add an OpenAI provider"); + }, }; export const AddBedrock: Story = { args: { provider: addableProviders.find((p) => p.value === "bedrock")!, }, + play: async ({ canvasElement }) => { + const canvas = within(canvasElement); + await canvas.findByText("Add an AWS Bedrock provider"); + }, +}; + +export const AddCopilot: Story = { + args: { + provider: addableProviders.find((p) => p.value === "copilot")!, + }, + play: async ({ canvasElement }) => { + const canvas = within(canvasElement); + await canvas.findByText("Add a GitHub Copilot provider"); + }, }; diff --git a/site/src/pages/AISettingsPage/ProvidersPage/AddProviderPage/AddProviderPageView.tsx b/site/src/pages/AISettingsPage/ProvidersPage/AddProviderPage/AddProviderPageView.tsx index 6f21156077..dbdb31b36a 100644 --- a/site/src/pages/AISettingsPage/ProvidersPage/AddProviderPage/AddProviderPageView.tsx +++ b/site/src/pages/AISettingsPage/ProvidersPage/AddProviderPage/AddProviderPageView.tsx @@ -16,6 +16,9 @@ interface AddProviderPageViewProps { provider: AddableProvider; } +const indefiniteArticle = (word: string): string => + /^[aeiou]/i.test(word) ? "an" : "a"; + const AddProviderPageView: React.FC = ({ provider, }) => { @@ -38,7 +41,9 @@ const AddProviderPageView: React.FC = ({ size="lg" src={getProviderIcon(provider.value)} /> - {`Add a ${provider.label} provider`} + {`Add ${indefiniteArticle( + provider.label, + )} ${provider.label} provider`}

Configure connection details and credentials. diff --git a/site/src/pages/AISettingsPage/ProvidersPage/UpdateProviderPage/UpdateProviderPageView.stories.tsx b/site/src/pages/AISettingsPage/ProvidersPage/UpdateProviderPage/UpdateProviderPageView.stories.tsx index 15713428f9..4d212d2b14 100644 --- a/site/src/pages/AISettingsPage/ProvidersPage/UpdateProviderPage/UpdateProviderPageView.stories.tsx +++ b/site/src/pages/AISettingsPage/ProvidersPage/UpdateProviderPage/UpdateProviderPageView.stories.tsx @@ -5,6 +5,7 @@ import type { AIProvider } from "#/api/typesGenerated"; import { MockAIProviderAnthropic, MockAIProviderBedrock, + MockAIProviderCopilot, MockAIProviderOpenAI, } from "#/testHelpers/entities"; import { withToaster } from "#/testHelpers/storybook"; @@ -53,6 +54,21 @@ export const Bedrock: Story = { }, }; +// Copilot has no stored credential, so the edit form renders no API key +// field and keeps the immutable name disabled. +export const Copilot: Story = { + parameters: { + reactRouter: routingFor(`/ai/settings/${MockAIProviderCopilot.name}`), + ...seed(MockAIProviderCopilot), + }, + play: async ({ canvasElement }) => { + const canvas = within(canvasElement); + const name = await canvas.findByLabelText(/^name/i); + expect(name).toBeDisabled(); + expect(canvas.queryByLabelText(/api key/i)).not.toBeInTheDocument(); + }, +}; + // No seeded query: the page renders the loader while useQuery fetches. export const Loading: Story = { parameters: { diff --git a/site/src/pages/AISettingsPage/ProvidersPage/UpdateProviderPage/UpdateProviderPageView.tsx b/site/src/pages/AISettingsPage/ProvidersPage/UpdateProviderPage/UpdateProviderPageView.tsx index dbeb16003d..9991c3b8f6 100644 --- a/site/src/pages/AISettingsPage/ProvidersPage/UpdateProviderPage/UpdateProviderPageView.tsx +++ b/site/src/pages/AISettingsPage/ProvidersPage/UpdateProviderPage/UpdateProviderPageView.tsx @@ -43,8 +43,12 @@ const UpdateProviderPageView: React.FC = () => { }); const provider = providerQuery.data; - const providerIsOpenAiAnthropic = - provider !== undefined && !isBedrockProvider(provider); + // Copilot has no stored credential, and Bedrock keeps its secrets in + // settings, so only the remaining types surface the api_keys UI. + const providerUsesApiKeys = + provider !== undefined && + !isBedrockProvider(provider) && + provider.type !== "copilot"; const updateMutation = useMutation( updateAIProviderMutation(queryClient, providerId ?? ""), @@ -109,8 +113,8 @@ const UpdateProviderPageView: React.FC = () => { } const openAiAnthropicSavedApiKey = - providerIsOpenAiAnthropic && provider.api_keys.length > 0; - const openAiAnthropicMaskedApiKey = providerIsOpenAiAnthropic + providerUsesApiKeys && provider.api_keys.length > 0; + const openAiAnthropicMaskedApiKey = providerUsesApiKeys ? provider.api_keys[0]?.masked : undefined; diff --git a/site/src/pages/AISettingsPage/ProvidersPage/components/ProviderForm.stories.tsx b/site/src/pages/AISettingsPage/ProvidersPage/components/ProviderForm.stories.tsx index 8fda28fae2..181a786a32 100644 --- a/site/src/pages/AISettingsPage/ProvidersPage/components/ProviderForm.stories.tsx +++ b/site/src/pages/AISettingsPage/ProvidersPage/components/ProviderForm.stories.tsx @@ -64,6 +64,38 @@ export const EditBedrockKeepCredentials: Story = { }, }; +export const AddCopilot: Story = { + args: { + // The real add flow passes only the type; the form fills name and + // endpoint from the copilot defaults. + initialValues: { type: "copilot" }, + }, + play: async ({ canvasElement }) => { + const canvas = within(canvasElement); + await canvas.findByLabelText(/endpoint/i); + expect(canvas.queryByLabelText(/api key/i)).not.toBeInTheDocument(); + }, +}; + +export const EditCopilot: Story = { + args: { + editing: true, + initialValues: { + type: "copilot", + name: "copilot", + displayName: "GitHub Copilot", + baseUrl: "https://api.business.githubcopilot.com", + enabled: true, + }, + }, + play: async ({ canvasElement }) => { + const canvas = within(canvasElement); + const name = await canvas.findByLabelText(/^name/i); + expect(name).toBeDisabled(); + expect(canvas.queryByLabelText(/api key/i)).not.toBeInTheDocument(); + }, +}; + export const EditProvider: Story = { args: { editing: true, diff --git a/site/src/pages/AISettingsPage/ProvidersPage/components/ProviderForm.tsx b/site/src/pages/AISettingsPage/ProvidersPage/components/ProviderForm.tsx index 0a609de2ac..7468d39b86 100644 --- a/site/src/pages/AISettingsPage/ProvidersPage/components/ProviderForm.tsx +++ b/site/src/pages/AISettingsPage/ProvidersPage/components/ProviderForm.tsx @@ -81,6 +81,10 @@ const providerDefaults: Partial< name: "azure", baseUrl: "https://YOUR-RESOURCE.openai.azure.com/openai/v1", }, + copilot: { + name: "copilot", + baseUrl: "https://api.business.githubcopilot.com", + }, google: { name: "google", baseUrl: "https://generativelanguage.googleapis.com/v1beta/openai/", @@ -164,6 +168,20 @@ const makeBedrockSchema = (editing: boolean) => enabled: Yup.boolean(), }); +const makeCopilotSchema = (editing: boolean) => + Yup.object({ + type: Yup.string() + .oneOf(["copilot"] as const) + .required(), + name: makeNameSchema(editing), + displayName: makeDisplayNameSchema(editing), + baseUrl: Yup.string() + .url("Endpoint must be a valid URL") + .matches(HTTP_SCHEME_REGEX, "Endpoint must use http or https.") + .required("Endpoint is required"), + enabled: Yup.boolean(), + }); + const getProviderFormSchema = (editing: boolean) => Yup.lazy((value: { type?: AIProviderType } | undefined) => { switch (value?.type) { @@ -177,6 +195,8 @@ const getProviderFormSchema = (editing: boolean) => return makeOpenAiAnthropicSchema(editing); case "bedrock": return makeBedrockSchema(editing); + case "copilot": + return makeCopilotSchema(editing); default: return Yup.object({ type: Yup.string() @@ -185,6 +205,7 @@ const getProviderFormSchema = (editing: boolean) => "anthropic", "bedrock", "azure", + "copilot", "google", "openai-compat", "openrouter", @@ -319,18 +340,38 @@ export const ProviderForm: FC = ({ required field={getFieldHelpers("baseUrl")} label="Endpoint" - description="The base URL where the provider's API is hosted." + description={ + typeSelectValue === "copilot" ? ( + <> + The base URL for your Copilot tier:{" "} + https://api.individual.githubcopilot.com,{" "} + https://api.business.githubcopilot.com, or{" "} + https://api.enterprise.githubcopilot.com. + + ) : ( + "The base URL where the provider's API is hosted." + ) + } className="w-full" placeholder={baseUrlPlaceholder(form.values.type)} /> - handleCredentialFocus("apiKey")} - autoComplete="new-password" - placeholder={apiKeyPlaceholder(form.values.type)} - /> + {typeSelectValue === "copilot" ? ( +

+ Copilot authenticates with each user's GitHub OAuth token at + request time, so there is no API key to configure here. This + requires a GitHub external authentication provider to be + configured. +

+ ) : ( + handleCredentialFocus("apiKey")} + autoComplete="new-password" + placeholder={apiKeyPlaceholder(form.values.type)} + /> + )} )} @@ -409,7 +450,7 @@ export const ProviderForm: FC = ({