Skip to content

chore: backport post-2.37 merged PRs to release/2.37 - #28683

Merged
mtojek merged 9 commits into
release/2.37from
mike/backport-2.37
Aug 27, 2026
Merged

chore: backport post-2.37 merged PRs to release/2.37#28683
mtojek merged 9 commits into
release/2.37from
mike/backport-2.37

Conversation

@ibetitsmike

@ibetitsmike ibetitsmike commented Aug 26, 2026

Copy link
Copy Markdown
Collaborator

Batch backport of ibetitsmike's PRs that merged into main after the release/2.37 branch was cut (merge-base c275327fb72, 2026-08-24).

Included (9 cherry-picks, in order)

PR Title
#28470 fix(coderd/x): surface Gemini malformed-function-call stream deaths as retryable errors
#28471 fix: apply MCP server selection when editing a chat message
#28460 refactor: consolidate viewport hooks and remove defineProperty matchMedia stub
#28476 feat(coderd/x/chatd/chattool): improve find_tools relevance and model guidance
#28400 fix(coderd/x/chatd/mcpclient): enforce MCP connect budget and unblock session cleanup
#28589 fix(coderd/x/chatd): truncate overlong generated chat titles instead of rejecting them
#28496 feat: mount chat API routes under /api/v2
#28497 feat: promote codersdk chat API methods to Client
#28498 feat(site): use /api/v2 chat API paths

All picks applied cleanly with git cherry-pick -x onto release/2.37.

Excluded and follow-up backports

The remaining post-cut PRs are cherry-picked onto this PR as a GitHub stack (#28692) that merges bottom-up, so the dependency order is enforced by the PR bases:

Stack order Original PR Backport PR Notes
1 #28462 (repair failing Storybook stories) #28687 One fixture adaptation: 2.37's OrganizationModelsContextValue has no organizations field
2 #28186 (enable Coder Agents for organization members) #28688 One story-assertion conflict resolved to the picked regex; ships no migration by design
3 #28593 (allow sharing MCP servers with users and groups) #28686 Clean pick on top of #28688, plus the 1-line test fix #28657 (expect v2 MCP ACL path)
4 #28659 (point MCP server ACL msw handlers at v2 paths) #28689 Clean pick on top of #28686

Intentionally skipped: #28587 (restore agents-access cleanup migration). Release branches cannot take a migration unless its sequence number matches main with no gap.

Validation

  • go build ./... and test-binary compilation for ./coderd/... ./codersdk/... ./enterprise/... ./site pass
  • pnpm check and pnpm lint:types pass
  • Targeted Vitest on all touched test files: unit 281/281 pass; Storybook 180/181 pass. The single failure (AgentChatPage.stories.tsx > Queued For Capacity After Polling) reproduces identically on the untouched release/2.37 baseline, so it is pre-existing and unrelated.

Xum acted on Mike's behalf.

…s retryable errors (#28470)

## Problem

When Gemini's OpenAI-compatible endpoint rejects a model-generated
function call server-side, it ends the SSE stream cleanly with the
nonstandard finish reason `function_call_filter:
MALFORMED_FUNCTION_CALL` after streaming only thought summaries; the
rejected call never reaches the wire. chatd treated this as a normal
completion: fantasy maps the unrecognized finish reason to `unknown`,
the reasoning block never closes (the only non-thought delta is the
`</thought>` marker, which the transport seam strips to an empty string,
and the openaicompat hook only ends reasoning on a non-empty content
delta), so the step accumulates no content and the generation loop
finishes the turn as complete. The user sees the model think for ~40
seconds and then nothing: no assistant message, no `last_error`, chat
status `waiting`. Observed twice in a row in production on
gemini-3.7-flash, with "Resume" reproducing it identically.

## Fix

Two independent layers:

- `coderd/x/googleopenai`: the stream rewrite now converts any chunk
whose `finish_reason` starts with `function_call_filter` into an
OpenAI-style SSE `{"error": ...}` event embedding the raw reason.
openai-go turns error-bearing events into stream errors, so the failure
rides the existing stream-error path instead of ending the stream
cleanly.
- `coderd/x/chatd/chaterror`: classifies that injected error as
retryable (kind `generic`, provider `google`) with a clear user-facing
message, so the existing generation retry machinery re-runs the step and
persists a `last_error` if retries exhaust.
- `coderd/x/chatd/chatloop`: provider-agnostic guard: a step that
produced no user-visible content and no tool calls under a finish reason
of `unknown`, `error`, `other`, or `tool-calls` (a tool-calls finish
that delivered zero calls) now returns a retryable error instead of
silently completing the turn. `stop` and `length` finishes keep their
existing semantics.

Tests cover the seam rewrite (live-capture SSE shape plus standard
finish reason passthrough), the new classification, and the chatloop
guard (error cases plus preserved stop, length, text, and tool-call
behavior). Each layer was red-green verified independently.

Remote dogfood UAT ran against this exact commit: normal reasoning and
tool-call chats on a real model complete cleanly with no spurious guard
errors and no retry loops. The Google-side failure itself is not
deterministically triggerable against live Gemini and is owned by the
unit tests.

> Xum acted on Mike's (@ibetitsmike) behalf.

<!-- xum-attribution: model=claude-opus, thinking=enabled -->

(cherry picked from commit a48aedd)
## Problem

Editing a user message in an Agent chat silently dropped the MCP server
selection. The composer renders the MCP picker in edit mode and toggles
update local client state, so the picker displayed the new selection,
but the edit request omitted `mcp_server_ids` at every layer (frontend
request builder, `codersdk.EditChatMessageRequest`, the `PATCH
/chats/{chat}/messages/{message}` handler, and `chatd.EditMessage`). The
chat's persisted selection never changed and the regenerated turn ran
without the newly enabled MCP tools. Two dogfood users hit this within
hours; there is no error anywhere and the UI shows the opposite of the
server state.

## Changes

- `codersdk`: add `MCPServerIDs *[]uuid.UUID` to
`EditChatMessageRequest`, mirroring `CreateChatMessageRequest` (nil
preserves the current selection).
- `chatd`: extract the send path's MCP update block into one shared
`applyRequestedMCPServerIDs` helper (explore-subagent snapshot
immutability guard plus Force On enforcement, Cure53 CDM-02-010) and
call it from both `SendMessage` and `EditMessage`, so enforcement cannot
drift between the two paths.
- `coderd`: extract the send handler's request validation (dedupe,
enabled-in-organization check, persisted-ID exemption) into
`normalizeRequestedChatMCPServerIDs` and wire it into the edit handler,
which now threads the selection into `EditMessageOptions`.
- Frontend: the edit request now includes `mcp_server_ids:
[...effectiveMCPServerIds]`, exactly like the send path, making the
picker's displayed state real.
- `make gen` artifacts (swagger, API docs, `typesGenerated.ts`).

## Tests

Each layer is covered and was proven with independent red toggles
(removing one layer's wiring fails only that layer's tests):

- chatd (`TestEditMessage_MCPServerIDs`): edit applies a provided
selection, nil preserves it, an emptied list cannot remove a `force_on`
server, and explore subagent chats keep the spawn-time snapshot.
- API (`TestPatchChatMessage/MCPServerIDsApplied`,
`MCPServerIDsInvalidRejected`): persistence via the endpoint, omission
preserves, unknown IDs get the same 400 as the send path.
- Storybook (`EditAppliesMCPServerSelection`): toggling a server on
during an edit puts it in the edit request payload.

Note: the `AgentChatPage.stories.tsx` story "Queued For Capacity After
Polling" fails locally on current main as well (verified against the
main baseline with this branch's changes reverted); it is unrelated to
this diff.

Remote dogfood UAT ran on the exact head and passed, including proof
that the regenerated turn actually gains the newly enabled MCP server's
tools.

> 🤖 Xum acted on Mike's (@ibetitsmike) behalf. • Model:
`anthropic:claude-fable-5`
<!-- xum-attribution: model=anthropic:claude-fable-5 -->

(cherry picked from commit b6d7653)
…edia stub (#28460)

Follow-up to review feedback on #28387.

That PR added three hooks for one feature (`useIsBelowLgViewport`,
`useIsBelowMdViewport`, and the local single-use
`useRightPanelNarrowSuppression`) and stubbed `window.matchMedia` in
tests with `Object.defineProperty`.

- Replace the two single-purpose viewport hooks with one generic
`useMediaQuery(query)`. Callers pass the shared Tailwind-aligned query
constants from `utils/mobile.ts`. The one new hook replaces the two
deleted ones (net -1).
- Inline `useRightPanelNarrowSuppression` into `AgentChatPage`, its only
consumer, and drop its `renderHook` unit suite. The behavior stays
covered by the narrow-viewport stories; the widening-restore case moved
into the `NarrowingSuppressesExpandedPanel` play function.
- Rework `testHelpers/matchMedia.ts` to install the stub with `spyOn`
from `storybook/test` instead of `Object.defineProperty`. The helper is
story-only now (stories run in real Chromium; jsdom has no
`matchMedia`), so any future unit test needing a stub should use
`vi.stubGlobal` directly.
- Encode the feedback in the canonical FE contract so it gets caught
during development: `.claude/docs/FRONTEND_PATTERNS.md` now bans new
React hooks when an existing hook, a plain function, or component state
suffices (FE3) and bans replacing browser globals with
`Object.defineProperty` in tests or stories (FE9: `vi.stubGlobal` /
`spyOn`), and notes that `renderHook` suites for stateful UI hooks
belong in the consuming component's story (FE1). The `frontend-review`
skill checklist flags all three. `site/AGENTS.md` is unchanged since it
already defers to the patterns doc.

Validation: `pnpm check`, `pnpm format:check`, `pnpm lint` (biome,
types, knip, circular deps, compiler check), `AgentChatPage.test.ts` (70
passed), story runs for `AgentChatPage.stories.tsx` and
`WorkspacePill.stories.tsx` (49 passed in Chromium).

> Xum acted on Mike's behalf (@ibetitsmike).

<!-- xum-attribution: model=claude-opus-4-6 thinking=high -->

(cherry picked from commit 3c32408)
… guidance (#28476)

Broad `find_tools` queries such as "linear issues" saturated the hard
20-match cap on every call: a large server's name token-matches every
one of its tools (+8 name, +1 server), so ranking was decided by a term
carrying no discriminating information and the model got 20 activations
per search regardless of intent.

Three changes, mirroring what makes xum's `tool_catalog_search` behave
well:

- **Lower default with a `limit` argument.** Keyword matches default to
10 per call; the new optional `limit` raises that up to the existing
hard cap of 20 (non-positive values fall back to the default). Exact
`names` are explicit activation requests and bypass the limit up to the
hard cap, so naming 15 tools still activates all 15.
- **Coverage-first ranking.** Keyword matches sort by distinct query
terms matched before raw score, so a query like "linear issues" ranks
tools matching both terms above the dozens matching only the server
name.
- **Model guidance.** `queries` and `names` now carry schema
descriptions (capability keywords and what they are matched against;
exact-name activation), and the tool description opens by explaining
what deferral means, that matches become callable on the next step, and
that a `"server: terms"` prefix scopes a query to one server.

An earlier revision also inferred a server scope from unprefixed query
words ("linear issues" behaving like "linear: issues"). Review kept
surfacing edge cases in that inference, and coverage-first ranking
already resolves the original saturation complaint, so it was dropped in
favor of the explicit prefix.

The hard cap stays at 20 so the persisted result keeps fitting under the
generic tool-result truncation budget that protects activation-recovery
JSON. Budget and reservation accounting are unchanged; the frontend
renderer ignores unknown argument fields, so no `site/` change is
needed.

> [!NOTE]
> Xum acted on @ibetitsmike's behalf in this pull request.
<!-- xum-attribution: model=claude-opus-4-6 thinking=high -->

(cherry picked from commit e244cac)
… session cleanup (#28400)

## Problem

During the Aug 19 dev.coder.com incident, chats stalled for up to ~15
minutes per turn. chatd reconnects to every configured MCP server on
every generation step, and one configured server
(`registry.coder.com/mcp`) was black-holing requests from the
deployment's egress IPs: TCP/requests were silently dropped, and each
connect attempt hung until the kernel gave up (~125s), far past the
nominal 10s connect budget.

The budget does not hold because go-sdk v1.7.0 detaches the context
inside `StreamableClientTransport.Connect`, and its error paths block on
HTTP work bound to that detached context. Reproduced in a test: a bare
`Connect` with a 2s deadline against a black-holed server returns after
**12s**. Step-end session cleanup (`session.Close()`) runs synchronously
in the generation loop and blocks on the same detached requests, so a
server that wedges mid-turn stalls every step boundary too.

## Changes

- **Enforce the connect budget externally** (`connectOne`): run
`Connect` + `ListTools` in a goroutine and select on the budget context.
On timeout, abandon the goroutine and leave a reaper that drains its
late result and closes any session that still materialized, so nothing
leaks and the caller returns within the budget.
- **Stop using bare `http.DefaultTransport`**: MCP traffic now rides a
cloned transport with a 5s dial timeout (converts SYN black-holes into
fast errors) and a 60s `ResponseHeaderTimeout`. `http.Client.Timeout`
stays unset so long-lived SSE streams are unaffected.
- **Close sessions in detached goroutines during cleanup**
(`ConnectAll`'s cleanup func): the sessions are discarded regardless,
and a wedged `Close` (DELETE bound to the SDK's detached context) must
not stall the generation loop at step boundaries.

One deliberate deviation from the incident plan: `ResponseHeaderTimeout`
is 60s (matching `toolCallTimeout`) instead of ~10s. The same HTTP
client serves tool-call POSTs, and a JSON-response MCP server sends no
headers until the tool finishes, so a 10s bound would falsely kill
legitimate 10-60s tools that fit today's tool-call budget. The real 10s
connect budget is enforced by the external select, not the transport.

## Tests

- Acceptance: with a black-holed server plus a healthy one configured,
`ConnectAll` returns within the budget and the healthy server's tools
are present; closing the black-holed connections makes the reaper exit
(red without the fix: 1s budget took 11s).
- A slow-but-alive server (300ms/request) still connects.
- A server that only responds after the budget: `ConnectAll` returns
promptly, the late result is reaped.
- Cleanup returns promptly while the session-teardown DELETE is wedged
server-side, and the teardown still happens in the background (red
without the fix: cleanup blocked 60s).
- Transport guard test pinning the dial/response-header bounds.

`go test ./coderd/x/chatd/...` passes (including goleak in `chatd`).

Part of the MCP connect-stall incident follow-up; observability (connect
durations in logs/debug runs) comes in a stacked follow-up PR.

> 🤖 Mux authored this PR on Mike's behalf.

<!-- mux-attribution: model=anthropic:claude-opus-4-6 thinking=high -->

(cherry picked from commit cc958b7)
…of rejecting them (#28589)

Chat title generation asks the model for a title in 2-8 words and then
hard-rejects any response longer than 8 words
(`validateGeneratedTitle`). Quickgen pins temperature for repeatable
output, so a model that overshoots the budget for a given conversation
overshoots on every retry: the rename dialog's Generate button returns a
deterministic 500 (`generate manual title: generated title exceeded 8
words`) no matter how often it is clicked, and the automatic
first-message path silently leaves the chat on its fallback title. On
dev.coder.com this rejection fires 1-3 times a day; one chat took 13
consecutive manual failures on 2026-08-25 (14:22-14:27 UTC).

Truncate the normalized title to the 8-word budget in
`normalizeTitleOutput` instead of rejecting it, and keep the empty-title
validation. Both the automatic and manual title paths share this
normalization, so both are fixed.

> Xum (AI agent) authored this change and PR on Mike's behalf.

(cherry picked from commit 60b0313)
## Stack context

This is the base of a 3-PR stack promoting the chat API from
`/api/experimental` to `/api/v2`: server compatibility mounts (this PR),
codersdk promotion (#28497), and frontend path updates (#28498).

## Summary

Double-mount the stable chat and MCP handlers under `/api/v2` while
retaining the existing experimental routes for the one-release
compatibility window decided in CODAGT-921. CODAGT-922 tracks removing
the compatibility mounts.

The shared route builders preserve existing authentication and
middleware behavior. Experiment-gated, debug, tombstone, and legacy
default-organization model routes remain experimental-only. Signed file
URLs, external OAuth callback URLs, and mixed-version replica relays
also remain on the experimental prefix during the transition.

Update Swagger and the generated API reference for the promoted routes,
including the workspace lookup and a runnable raw-body chat file upload
example. Retain internal endpoints outside the published reference,
share chat-file rate limits across both prefixes, enable CORS for the v2
MCP routes, and cover dual mounts plus exclusions with compatibility
tests. Remote dogfood UAT passed for the promoted chat, model, MCP, and
file flows.

> [!NOTE]
> Xum acted on Mike's behalf in this pull request.
<!-- xum-attribution: model=claude-fable-5 thinking=high -->

(cherry picked from commit 351bb14)
## Stack context

This is the second PR in the 3-PR chat API promotion stack: server
compatibility mounts (#28496), codersdk promotion (this PR), and
frontend path updates (#28498).

## Summary

Move the promoted chat and MCP SDK methods from `ExperimentalClient` to
`Client` and update them to use `/api/v2`. Methods for routes that
remain experimental stay on `ExperimentalClient`.

Update in-repository callers and generated types. The multi-replica chat
stream relay dials `/api/v2` directly: mixed-version replica sets are
not a supported upgrade path, so no experimental-path fallback is kept
(per review). Remote dogfood UAT passed for the composed stack.

> [!NOTE]
> Xum acted on Mike's behalf in this pull request.
<!-- xum-attribution: model=claude-fable-5 thinking=high -->

(cherry picked from commit 973d5a4)
## Stack context

This is the final PR in the 3-PR chat API promotion stack: server
compatibility mounts (#28496), codersdk promotion (#28497), and frontend
path updates (this PR).

## Summary

Switch promoted frontend chat and MCP REST and WebSocket calls to
`/api/v2`. Debug runs, virtual desktop streaming, advisor, and
computer-use provider routes remain on `/api/experimental` because those
surfaces were not promoted.

Update the matching tests, stories, helpers, and end-to-end route
expectations. Remote dogfood UAT passed with chat traffic verified on
the v2 routes.

> [!NOTE]
> Xum acted on Mike's behalf in this pull request.
<!-- xum-attribution: model=claude-fable-5 thinking=high -->

(cherry picked from commit 7394d2c)
@ibetitsmike ibetitsmike added the cherry-pick/v2.37 Cherry-pick PR targeting release/2.37 label Aug 26, 2026
@github-actions

Copy link
Copy Markdown
Contributor

👋 Hey @ibetitsmike!

This PR is targeting the release/2.37 release branch, but its title does not start with fix: or fix(scope):.

Only bug fixes should be cherry-picked to release branches. If this is a bug fix, please update the PR title to match the conventional commit format:

fix: description of the bug fix
fix(scope): description of the bug fix

If this is not a bug fix, it likely should not target a release branch.

@github-actions

Copy link
Copy Markdown
Contributor

Docs preview

Check off each page once it's been reviewed. If a page changes in a later push, its checkbox clears automatically so it gets a fresh look. Pages not yet wired into the docs navigation aren't listed here.

@coderagents

coderagents Bot commented Aug 26, 2026

Copy link
Copy Markdown
Contributor

Documentation Check

This PR promotes the chat/MCP-config API surface from /api/experimental/* to /api/v2/* and correctly updates most affected docs (models.md, platform-controls/mcp-servers.md, platform-controls/organizations.md, tasks-to-chats-migration.md). One page referencing promoted endpoints was missed.

Updates Needed

  • docs/ai-coder/agents/platform-controls/advisor.md - Update the API paths (lines ~49 and ~51) to /api/v2. The advisor config and model-override endpoints were promoted in this PR (GET/PUT /api/v2/chats/config/advisor and PUT /api/v2/organizations/{organization}/chats/model-overrides/advisor), but this page still points at /api/experimental/..., making it inconsistent with the other docs updated here.

Note

docs/ai-coder/mcp-server.md (/api/experimental/mcp/http) and docs/reference/api/chats.md (/api/experimental/chats/{chat}/stream/desktop) were intentionally left unchanged and are correct: those routes were not promoted and remain experimental-only.


Automated review via Coder Agents

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 29dd39ad51

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

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".

We make use of a relay mechanism when there are multiple coderd replicas. If a client connects to the stream endpoint on replica A, but the chat worker that owns the chat is on replica B, the endpoint will connect to replica B and relay streaming message parts.

There exists a `GET /api/experimental/chats/{chat}/stream/parts` endpoint that is responsible exclusively for streaming message parts. That endpoint talks to the chat worker on the same replica to obtain the message parts and relay them to the client.
There exists a `GET /api/v2/chats/{chat}/stream/parts` endpoint that is responsible exclusively for streaming message parts. That endpoint talks to the chat worker on the same replica to obtain the message parts and relay them to the client.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Leave a TODO instead of editing chatd architecture

This directly rewrites the chatd architecture document, but the repository guardrail requires changes affecting documented chatd architecture to leave TODOs in the affected sections and defer the actual documentation update to the human PR author. Revert these prose substitutions and add the required TODOs instead.

AGENTS.md reference: AGENTS.md:L65-L65

Useful? React with 👍 / 👎.

Comment on lines +332 to +334
go func() {
if late := <-resCh; late.session != nil {
_ = late.session.Close()

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Bound abandoned MCP response bodies

When an MCP endpoint sends response headers but never completes its initialization or ListTools response body, ResponseHeaderTimeout no longer applies and the SDK's detached request does not observe connectCtx. This timeout branch returns while both the connect goroutine and this reaper remain blocked forever on resCh; because ConnectAll runs again on later generation steps, one misbehaving server can accumulate goroutines and open connections. Add an operation-specific body deadline or cancellation mechanism before abandoning the request.

Useful? React with 👍 / 👎.

@mtojek
mtojek merged commit f7e068e into release/2.37 Aug 27, 2026
69 of 70 checks passed
@mtojek
mtojek deleted the mike/backport-2.37 branch August 27, 2026 06:55
@github-actions github-actions Bot locked and limited conversation to collaborators Aug 27, 2026
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

cherry-pick/v2.37 Cherry-pick PR targeting release/2.37

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants