Repository navigation
🤖 deps: bump Coder SDK to v2.37.3 - #163
Conversation
|
@codex security review |
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
🛡️ Codex Security ReviewSecurity review completed. No security issues were found in this pull request. Reviewed commit: Only the user who started this review can view the report in Codex. ℹ️ About Codex security reviews in GitHubThis is an experimental Codex feature. Security reviews are triggered when:
Once complete, Codex will leave suggestions, or a comment if no findings are found. |
|
@codex review |
|
@codex security review |
|
Codex Review: Didn't find any major issues. Keep it up! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
🛡️ Codex Security ReviewSecurity review completed. No security issues were found in this pull request. Reviewed commit: Only the user who started this review can view the report in Codex. ℹ️ About Codex security reviews in GitHubThis is an experimental Codex feature. Security reviews are triggered when:
Once complete, Codex will leave suggestions, or a comment if no findings are found. |
62ae371 to
522fd78
Compare
|
@codex review |
|
@codex security review |
|
Codex Review: Didn't find any major issues. Hooray! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
🛡️ Codex Security ReviewSecurity review completed. No security issues were found in this pull request. Reviewed commit: Only the user who started this review can view the report in Codex. ℹ️ About Codex security reviews in GitHubThis is an experimental Codex feature. Security reviews are triggered when:
Once complete, Codex will leave suggestions, or a comment if no findings are found. |
Move github.com/coder/coder/v2 from v2.35.8 to v2.37.3 and re-vendor. The bump pulls github.com/modelcontextprotocol/go-sdk v1.4.1 -> v1.7.0, which no longer applies cross-origin protection by default; wrap the MCP HTTP handler with net/http CrossOriginProtection to keep rejecting cross-origin browser requests. Update the MCP docs and transport tests for the SDK's base-media-type Content-Type check and 4 MiB body limit. Refs #150 Signed-off-by: Thomas Kosiewski <tk@coder.com> --- _Generated with [`xum`](https://github.com/coder/xum) • Model: `anthropic:claude-opus-5-5` • Thinking: `high`_ Change-Id: I0598f1f3a6f5a2618c93c00fd09abeefc45784fa
… SDK A Coder SDK bump changes every aggregated-API call to the backend, but the pre-queue E2E path filter ignored vendor/, so the PR job only printed a skip notice. Route vendor/github.com/coder/coder/v2/** through full E2E. Refs #150 Signed-off-by: Thomas Kosiewski <tk@coder.com> --- _Generated with [`xum`](https://github.com/coder/xum) • Model: `anthropic:claude-opus-5-5` • Thinking: `high`_ Change-Id: Ie52fe1c46cffb9136f77187425bf2a8a0de66b61
… bump codersdk v2.37 returns *codersdk.Error with the 2xx status when a 2xx answer has no valid JSON body (for example a proxy or SSO page). Before the bump this was a plain decode error. The workspace start and stop POST treated it as uncertain and re-read Coder. With the bump the 2xx status fell through to MapCoderError and answered a definitive 500, although Coder had queued the build. Count a 2xx status as uncertain. This also covers a 200, which the SDK always returned as *codersdk.Error because the POST expects 201. Pin the CoderTemplateTest behavior too: a 2xx proxy page on create stays ConfirmingCreate (never CreateRejected), and on the readiness read it stays CoderUnavailable and never proves deletion. Refs #150 Signed-off-by: Thomas Kosiewski <tk@coder.com> --- _Generated with [`xum`](https://github.com/coder/xum) • Model: `anthropic:claude-opus-5-5` • Thinking: `high`_ Change-Id: I132860b872d346367059eee5ff240739551fc831
522fd78 to
48891a0
Compare
|
@codex review |
|
@codex security review |
|
Codex Review: Didn't find any major issues. Another round soon, please! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
🛡️ Codex Security ReviewSecurity review completed. No security issues were found in this pull request. Reviewed commit: Only the user who started this review can view the report in Codex. ℹ️ About Codex security reviews in GitHubThis is an experimental Codex feature. Security reviews are triggered when:
Once complete, Codex will leave suggestions, or a comment if no findings are found. |
#163) Change-Id: Idb95ec07d36db8f82ba80ead3a0bcdc6f6c233f8 Signed-off-by: Thomas Kosiewski <tk@coder.com>
1 participant
Summary
Bump
github.com/coder/coder/v2from v2.35.8 to v2.37.3 and re-vendor. The bump also movesgithub.com/modelcontextprotocol/go-sdkfrom v1.4.1 to v1.7.0. That SDK no longer applies cross-origin protection by default, so the MCP HTTP handler now wraps itself withnet/http'sCrossOriginProtectionto keep the existing behavior.The bump also changes one answer of the workspace start and stop subresources (#187). This PR fixes that, so they behave as before (see the table below).
Refs #150
Maintainer decision 2026-10-12: v2.37.3 ships as a stable-channel exception.
Background
The AI provider, chat model and MCP server APIs that the planned AI resources need live in Coder v2.37 (chats moved from
/api/experimentalto/api/v2). #150 and the AI CRDs (#154, #155, #156) need them.Version choice: every v2.37.x release is a Coder mainline release. The stable channel is still v2.36.6. #150 requires v2.37, so this takes the newest v2.37 patch, v2.37.3. Its changes over v2.37.2 are a tailnet panic fix, model catalog backports and
owner_idonCreateChatRequest. None of them touch workspaces, templates or provisioners.E2E backend pin: CI keeps
ghcr.io/coder/coder:v2.37.2@sha256:6ea19b67…(.github/workflows/ci.yaml). A v2.37.3 SDK against a v2.37.2 server is a patch-level skew inside one minor line. The existing identity and resourceVersion evidence is bound to 2.37.2, so the backend does not move in this PR.Implementation
go get github.com/coder/coder/v2@v2.37.3,go mod tidy,go mod vendor, on top of currentmain(rebased,vendor/regenerated, not hand-merged). Thegodirective (1.26.8) and all Kubernetes libraries (k8s.io/*, controller-runtime) are unchanged. Notable transitive moves: MCP go-sdk 1.4.1 → 1.7.0, prometheusclient_golang1.23.2 → 1.24.1, testify 1.11.1 → 1.12.0,terraform-provider-coder2.18.0 → 2.19.0. Thego.modversion moves are identical to the original (pre-rebase) commit.internal/app/mcpapp/http.go: wrap the streamable HTTP handler withhttp.NewCrossOriginProtection().Handler(...), which is the SDK's documented replacement for its deprecated option. The existingcross origin,opaque origin,cross site fetchandcross origin servicecases inTestHTTPTransportSecurityfail without this wrap.internal/app/mcpapp/http_test.go:application/json; charset=utf-8is now accepted, because the SDK matches the base media type. A charset parameter keeps the request non-simple for CORS, so browsers still preflight it. A new case pins the SDK's 4 MiB body limit (413).docs/how-to/mcp-server.md: request-rules table updated to match..github/workflows/ci.yaml: addvendor/github.com/coder/coder/v2/**to the pre-queuee2epath filter (🤖 ci: cover CoderWorkspace lifecycle in the Kind E2E job #113). Before this change, an SDK bump got only a skip notice in the PR's E2E job, even though the SDK carries every aggregated-API call to the backend.internal/aggregated/storage/workspace_transition.go:isUncertainStatusnow counts a 2xx status as uncertain (see the first table row). New cases inTestWorkspaceTransitionUncertainPostcover a 201 and a 200 proxy page.HTMLfault (atext/htmlbody with any status). New cases pin a 200 and a 201 proxy page on create (TestTemplateTestCreateResults) and a 200 proxy page on the readiness read (TestTemplateTestReadinessProxyPage).SDK behavior changes that affect existing resources
*codersdk.Errorwith the 2xx status. It used to be a plain decode error.Error()readsinvalid API response (status code N),MessagereadsReceived an HTML response instead of JSON from the Coder API.(or the empty/invalid JSON variant), andUnwrapexposes the cause.codersdk/client.go:521,:538-571,:664-680,:743workspace_transition.go:156re-reads Coder when the error is not a*codersdk.Erroror has an uncertain status. A 2xx proxy page used to take that path. With the bump it fell through toMapCoderErrorand answered a definitive 500, although Coder had queued the build.isUncertainStatusnow includes 2xx, so the answer is again the confirmedQueuedresult or the uncertain 504. CoderTemplateTest (#194-#213), no classification change. EverycoderStatus()branch matches only specific 4xx codes (401, 404, 406, 409, 410, 412, 429, 400) or the 4xx range, so a 2xx*codersdk.Errortakes the same branch as the old status-0 decode error: create staysConfirmingCreateand never sends a second request (codertemplatetest_create.go:59-93).createHadNoEffectdoes not match 2xx, so the marker stays. The readiness and confirm reads answerCoderUnavailableand never treat the answer as 410 deletion (codertemplatetest_readiness.go:36,codertemplatetest_confirm.go:44,:66). The status message changes from the raw decode error toCoder answered 2xx: Received an HTML response…. It holdsMessageonly, soDetail(content type, redirect URL) does not reach status. Everything else, no change.MapCoderError(internal/aggregated/coder/errors.go),template_promote.go:181(4xx only, else re-read), and the control plane and provisioner controllers branch on 4xx only, so 2xx falls through to the same InternalError or retry path as before.Content-Type: text/htmlis now an error. The old decoder ignored the header and decoded it.codersdk/client.go:544-547Template.AgentsAllowed(new field).UpdateTemplateMeta.AgentsAllowedandCreateTemplateRequest.AgentsAllowedare*bool,omitempty.codersdk/templates.go:71,:288;codersdk/organizations.go:235convert/template.gonever sets it, so the server keeps the current value on update and defaults it to true on create. This is the same as before, because the 2.37.2 server already applies those defaults. Not surfaced in the CR. CoderTemplateTest creates workspaces through the normal API as the operator user, not as a Coder Agent, so the flag does not gate it.CreateWorkspaceBuildRequest.OnSuccess(omitempty),WorkspaceFilter.IncludeAgentMetadata,WorkspaceAgent.Metadata(omitempty)codersdk/workspaces.go:142,:565;codersdk/workspaceagents.go:177codertemplatetest_delete.go:103) and the start/stop builds. CoderTemplateTest readiness reads agent status and lifecycle only, neverMetadata.PrebuildsSystemUserIDand the agent status, lifecycle, provisioner job status and transition constantscodersdk/prebuilds.go:13codertemplatetest_confirm.go:135) still recognizes claimed prebuilds by this user ID.codersdk/deployment.go:438agent_runtime_hoursis a usage-period feature. The control plane and provisioner readexternal_provisioner_daemons,multiple_organizationsandcustom_roles, which are unaffected.License.Trial()reads thetrialclaim (was misspelledtrail). New reserved-provisioner-key helpers.codersdk/licenses.go:86;codersdk/provisionerdaemons.go:426Content-Typematches the base media type, and request bodies are limited to 4 MiBgo-sdk/mcp/streamable.go:243,:388,:225Host) protection is unchanged.Validation
On the pushed commit
48891a0(treecffac8d), rebased ontomain41e4acb(#222: go1.26.9 andgolang.org/x/netv0.60.0): actionlint,make verify-vendor,make test,make test-integration,make build,make lint,make docs-check,GOFLAGS=-mod=vendor go tool govulncheck ./...(the CI command: 0 called vulnerabilities) andgo test -race -count=1 ./internal/controller/...all pass.Dependencies: I reset
go.modandgo.sumtomain, then rango get github.com/coder/coder/v2@v2.37.3 && go mod tidy && go mod vendor. Thegodirective stays 1.26.9 andx/netstays v0.60.0. Thego.modandgo.sumdiffs againstmainare line-for-line identical to the diffs of the previous head (522fd78against55e7f34), so the SDK bump moves the same modules as before on top of the #222 baseline.The new
TestWorkspaceTransitionUncertainPostrows fail on the v2.37.3 SDK without theisUncertainStatusfix (no confirming re-read). The 200 row also fails onmainwith the v2.35.8 SDK. That SDK already returned*codersdk.Errorfor a 200 on a POST that expects 201, so that gap predates this PR, and the fix also covers it.Known local-only failure:
hack/e2e-workspace-lifecycle_test.sh(part ofmake test-scripts) times out locally in itspassscenario. It fails the same way on unmodifiedmainon this host. The PR touches nohack/script. CI's run is authoritative.Generated with
xum• Model:anthropic:claude-opus-5-5• Thinking:high