GeezerVison

Co-authored-by: Ben Potter <[email protected]>

waitForTaskIdle used time.NewTicker(5s) which delays the first poll
by 5 seconds. Debugger tracing proved the failure mechanism: on slow
CI (Windows), the first poll at 5s sees "working" (idle patch has not
landed due to goroutine scheduling), needs poll #2 at 10s, but the
25s context expires before it fires.

Two changes:

1. Use r.clock.NewTicker (quartz) with time.Nanosecond initial
   interval and Reset(5s) for immediate first poll. Tests inject a
   mock clock via clitest.NewWithClock for deterministic control.
2. Rewrite WaitsForWorkingAppState test with quartz traps
   (NewTicker + TickerReset) for deterministic synchronization
   instead of racing goroutines. Fix PausedDuringWaitForReady
   sync point.

Closes https://linear.app/codercom/issue/DEVEX-381

#25648)

waitForTaskIdle used time.NewTicker(5s) which delays the first poll
by 5 seconds. Debugger tracing proved the failure mechanism: on slow
CI (Windows), the first poll at 5s sees "working" (idle patch has not
landed due to goroutine scheduling), needs poll #2 at 10s, but the
25s context expires before it fires.

Two changes:

1. Use r.clock.NewTicker (quartz) with time.Nanosecond initial
   interval and Reset(5s) for immediate first poll. Tests inject a
   mock clock via clitest.NewWithClock for deterministic control.
2. Rewrite WaitsForWorkingAppState test with quartz traps
   (NewTicker + TickerReset) for deterministic synchronization
   instead of racing goroutines. Fix PausedDuringWaitForReady
   sync point.

Closes DEVEX-381

…PR tab for demo

Hardcodes 'fix: increase icon sizes for wcag 2.2 compliance' (PR #2
from coder-ux-prototypes) as a second PR tab in the dropdown to
demonstrate multi-PR switching UI. The extra tab reuses the remote
diff content panel.

…#26296)

Backport of #26204 (commit b5ef700) to `release/2.34`

Replace RequestHost with httpmw.EffectiveHost, which honors
X-Forwarded-Host only when the original socket peer is a configured
trusted origin, otherwise falling back to the received Host header.

Cherry-pick was clean; tests pass on this branch.

> [!NOTE]
> Breaking change. See the original PR for the breaking-change details.

#26604)

Cherry-pick of #26313

Original PR: #26313 — fix(site): set external auth provider polling
status individually

Co-authored-by: Andrew Aquino <[email protected]>

…nges (#27059)

Closes CODAGT-592.

## Problem

The advisor tool sometimes fails with the opaque error `advisor produced
no text output`. Live reproduction against `claude-sonnet-4-6` showed
the cause: `BuildAdvisorMessages` forwards the parent conversation's raw
`tool_use`/`tool_result` blocks into the nested advisor call, which
defines no tools. The nested model imitates the forwarded pattern and
spends its turn committing to a tool call it cannot make (captured
reasoning from a failing run: "The user wants me to make another tool
call to the advisor about writing a poem about cucumbers."), so the step
ends with reasoning-only or empty content and no advice. Because each
chat step currently rebuilds the advisor runtime and snapshot
(CODAGT-593), the second advisor call in a run reliably sees the first
call's exchange, which is why the first call succeeds and later ones
fail.

## Fix

- `BuildAdvisorMessages` rewrites tool activity as plain-text notes:
assistant tool-call parts are removed and folded, together with their
matching result, into a single user-role note of the form `[The parent
agent ran the X tool with input {...}. Result: ...]`. No raw tool blocks
and no bare call lines reach the tool-less nested request. This also
removes the provider requirement that `tool_result` blocks pair with a
`tool_use`, so results orphaned by window truncation are kept as notes
instead of dropped.
- The `advisor produced no text output` error now appends the finish
reason and content-part kinds, e.g. `advisor produced no text output
(finish_reason=stop; parts: reasoning=1)`, so field reports distinguish
tool-call mimicry, reasoning-only turns, and truncation.

Validated live by driving the production `RunAdvisor` path against
`claude-sonnet-4-6` through the dev.coder.com AI gateway: the failing
scenario went from 3/3 errors to 6/6 genuine advice (with and without
extended thinking), with the control scenario unaffected.

Related: CODAGT-593 (per-step advisor runtime recreation, addressed
separately) and CODAGT-742 (advisor tool call design).

<details>
<summary>Investigation and validation details</summary>

### Reproduction

A CLI prototype constructed the exact conversation snapshot the
generation preparer hands the advisor tool and called the real
`chatadvisor.NewRuntime` / `Runtime.RunAdvisor` / `BuildAdvisorMessages`
/ `chatloop.GenerateAssistant` chain against live `claude-sonnet-4-6`,
with a stream-teeing model wrapper capturing what `runner.go` discards
(finish reason, part kinds, reasoning text).

| Scenario (snapshot contents) | Thinking | Before fix | After fix |
|---|---|---|---|
| control: call #1 state, no prior advisor exchange | on | 3/3 advice |
2/2 advice |
| repro: call #2 state, prior advisor `tool_use`/`tool_result` pair
forwarded | on | 3/3 `advisor produced no text output` | 3/3 genuine
advice |
| repro | off | 2/3 same error, 1/3 degenerate advice ("I'll ask the
advisor...") | 3/3 genuine advice |

Every failing response was a tiny thinking block, zero text, zero
tool-call stream parts, finish reason `stop`; the model's own reasoning
text showed it deciding to "make the second tool call" in a request with
`tools=0`. The refunded `remaining_uses: 1200` in the failing
tool-result JSON matches the original issue screenshot.

### Decision log

- Tool exchanges are folded into a single user-role note per call/result
pair. A first attempt rendered assistant-authored `[tool call:
name(input)]` text lines plus separate result messages; live runs then
returned the literal `[tool call: advisor(...)]` line as the advice 6/6
times. The bare assistant call line is itself an imitable pattern, so no
assistant-authored tool artifact may survive the handoff. The folded
user-role note produced 6/6 genuine advice.
- An assistant message that carried only tool calls is dropped entirely;
the folded notes preserve the information.
- `dropOrphanToolMessages` was removed: without raw tool blocks there is
no provider pairing constraint, and an orphaned result note retains
context value.
- A reasoning-budget-starvation hypothesis (thinking budget consuming
`MaxOutputTokens`) did not reproduce on `claude-sonnet-4-6`; the model
adapts thinking length to the cap. The enriched error would identify
such cases on other models via `finish_reason=length`.
- CODAGT-593 (persisting the advisor runtime across steps) is
intentionally not addressed here; it shrinks the priming window but the
handoff fix is what removes the failure mode.

</details>

---

*This PR was generated by Coder Agents on behalf of @ThomasK33 (Linear
agent session for CODAGT-592).*

…sidebar row (#27351)

## What

Clicking a selected module in the right-hand `SelectionSummary` sidebar
now jumps to the module settings step and scrolls that module's card
into view.

Addresses [DEVEX-587](https://linear.app/codercom/issue/DEVEX-587). This
is an isolated slice of #27077 (which bundles several unrelated
changes); only the "click a module to scroll to it" behavior is included
here.

## Changes

- `SelectionSummary`: gains an optional `onNavigateModule` prop. When
provided, each selected module row renders as an accessible `<button>`
(hover + focus-ring) labeled `Configure <name>`; otherwise rows stay
non-interactive.
- `ModuleSettingsStep`: each module card wrapper gets a stable
`id={module-config-<id>}` scroll anchor plus `scroll-mt-24` so the
sticky top nav does not cover the title.
- `TemplateBuilderPageView`: adds `navigateToModule`, which switches to
the module settings step and scrolls the target module into view once it
renders. If the module settings step is skipped (no configurable
variables), the click is a no-op.
- `SelectionSummary.stories`: adds a `NavigateModuleClick` interaction
story and updates `WithLongNameModule` to the new button semantics.

## Explicitly out of scope

The rest of #27077 is not included: gallery height/card clamp,
sensitive-var banner relocation, trash-icon removal wiring, the
scroll-past required-field highlight subsystem, and the broader
navigable-sidebar work (step labels, base-row navigation, back-stack
semantics).

## Testing

- `pnpm check` (biome) clean
- `pnpm lint:types` (tsc) clean
- `pnpm vitest run --project=unit src/pages/TemplateBuilder` — 46 pass
- `pnpm vitest run --project=storybook src/pages/TemplateBuilder` — 34
pass (incl. new `NavigateModuleClick`)

<details>
<summary>Implementation plan / decision log</summary>

### Goal

Open a new PR containing only the changes that satisfy DEVEX-587:
clicking a selected module in the right-hand `SelectionSummary` sidebar
should jump to the module settings step and scroll that module's card
into view.

### Base has moved since #27077

PR #27077 was cut against an older `main`. Today's `main` was
refactored:

- Steps are URL-driven; `steps.ts` already provides `StepId`, per-step
`group` (1/2/3), and `nearestVisible()`.
- `TemplateBuilderPageView` already has `navigateToStep(index: number)`
and a `useEffect` that resets window scroll on every `currentStep.id`
change.
- The sidebar no longer has a deselect ("X") button. The PR's
entanglement between "make row a nav button" and "move deselect to a
trash icon" therefore does not exist on current `main`, so module
navigation can be added without removing behavior and without pulling in
the trash-icon item.

So the isolated diff was written against current `main`, not reused
verbatim from the PR. It is smaller than the PR's own hunks and does not
include `maxReachedStep`, step-label navigation, or base-row navigation.

### Decisions

- Scope for this PR: module-row navigation only.
- Skipped-settings fallback: no-op. When no selected module exposes
configurable variables, the `module-settings` step is skipped and
clicking a module row does nothing (there is no card to scroll to).

### Scroll timing

`navigateToStep` triggers a window scroll reset via an existing effect
keyed on `currentStep.id`. To cooperate, `navigateToModule` stores the
target module id in a ref and a follow-up effect (declared after the
scroll-reset effect, so it runs second) calls `scrollIntoView` inside
`requestAnimationFrame` once `module-settings` has rendered. When
already on `module-settings`, it scrolls immediately.

### Follow-up (deferred): full navigable sidebar

Not part of this PR, documented for later. The remainder of #27077's
item #2, rebased onto current `main`:

- `onNavigateStep?: (stepId: StepId) => void` on `SelectionSummary`.
- Clickable step labels: `Base Template` -> `base-infra`, `Modules` ->
`module-select`, `Customizations` -> `customizations`.
- Clickable selected base-template row -> `base-parameters` (fall back
to `base-infra` when that step is skipped for the chosen base).
- Back-stack semantics via a `maxReachedStep` prop so steps at or below
the furthest-reached group stay `complete` and clickable even after
navigating backward, while strictly-higher groups render as inert
`upcoming`.
- `StepIndicator` and `BaseTemplateSelection` render as `<button>` when
a navigation handler is supplied, else stay inert.
- Stories: `NavigationClicks`, `BackwardNavigation`, and
`UpcomingStepsInert`.

</details>

---

Coder Agents generated, on behalf of @aqandrew.

…28153)

## What

Makes the remaining `SelectionSummary` sidebar elements clickable jump
targets on `/templates/new/builder`, continuing the work from #27351
(which made module rows navigable).

Clickable now:

| Sidebar element | Jumps to |
|---|---|
| `Base Template` label | `base-infra` |
| Selected base-template row | `base-parameters` (falls back to
`base-infra` when that step is skipped) |
| `Modules` label | `module-select` |
| Each module row | `module-settings` + scroll (already shipped in
#27351) |
| `Customizations` label | `customizations` |

## Back-stack behavior

The sidebar previously colored groups purely from the current step, so
jumping backward would grey out and disable steps you had already
reached. This adds a `maxReachedGroup` that never shrinks on backward
navigation:

- Groups at or below the furthest-reached group stay `complete` (green)
and clickable, like a browser back-stack.
- Groups strictly above render as `upcoming` and inert (no button, no
hover, not focusable).
- The connecting divider color keys off `maxReachedGroup`, not the
current step, so it stays green after navigating backward.

Clickability is gated on `maxReachedGroup` (you can only jump to steps
you have already reached).

## Changes

- `SelectionSummary.tsx`: new required `maxReachedStep` and
`onNavigateStep` props. Split the single `variant()` into
`indicatorVariant` (label circle), `dividerVariant` (connecting line),
and a `reachable()` gate. `StepIndicator` and `BaseTemplateSelection`
render as `<button>` (hover + focus ring, `aria-label`) when a reachable
handler is supplied, else stay inert.
- `TemplateBuilderPageView.tsx`: track `maxReachedGroup`; add
`navigateToStepId(stepId)` that resolves skipped steps via
`nearestVisible` (so `base-parameters` falls back to `base-infra`) and
mirrors the existing customizations reset when leaving that step. Wire
both new props into `SelectionSummary`.
- `SelectionSummary.stories.tsx`: add `onNavigateStep` to meta and
`maxReachedStep` to existing stories; add `NavigationClicks` (asserts
each label/base/module callback), `BackwardNavigation` (dividers stay
green), and `UpcomingStepsInert` (steps above max-reached are not
buttons).

## Out of scope

Everything else from #27077 stays out: gallery height, sensitive-var
banner relocation, trash-icon wiring, and the scroll-past required-field
subsystem.

## Testing

- `pnpm check` (biome) clean
- `pnpm lint:types` (tsc) clean
- `pnpm vitest run --project=storybook src/pages/TemplateBuilder` — 37
pass
- `pnpm vitest run --project=unit src/pages/TemplateBuilder` — 55 pass

<details>
<summary>Implementation plan / decision log</summary>

### Origin

This is the remainder of PR #27077's item #2 (navigable selection
summary), rebased onto current `main` after #27351 shipped the
module-row navigation.

### Why a `maxReachedGroup` back-stack

`furthestAllowedIndex(state)` on current `main` is all-or-nothing (0
without a base selected, otherwise the last step), so it cannot express
"how far the user has progressed" for the sidebar coloring. A monotonic
`maxReachedGroup` (bumped when the current group advances, never shrunk)
is needed to keep completed steps green and clickable after backward
navigation, matching #27077.

### Decisions

- Reachability gating: gate both coloring and clickability on
`maxReachedGroup` (only jump to steps already reached), rather than the
looser `furthestAllowedIndex` (which would let users skip required steps
once a base is chosen).
- Base-template row target: jump to `base-parameters` and let
`nearestVisible` fall back to `base-infra` when the base has no
parameters/prerequisites.
- Module rows: keep `onNavigateModule` passed directly (not re-gated on
reachability), since a module can only be selected after reaching group
2, so `reachable(2)` is always true when module rows render. This
preserves the earlier decision to keep the module row's handler required
with no inert branch.

</details>

---

Coder Agents generated, on behalf of @aqandrew.

## Summary

Deflakes the `adjust user theme preference` Playwright test
(`site/e2e/tests/users/userSettings.spec.ts`), tracked in DEVEX-415.

The test selected the Light theme and then hard-navigated with
`page.goto`
before the optimistic appearance update was persisted. The navigation
could
cancel the in-flight `PUT /api/v2/users/me/appearance`, so the reloaded
document embedded the stale `dark` preference and the final assertion
flaked.
`toPass` retries could not help because retrying the reload only
re-reads the
still-stale persisted state.

## Fix

Wait for the appearance form's save spinner to clear before the hard
reload,
mirroring how other settings tests wait for a visible save confirmation.
Asserting the optimistic light class first guarantees the spinner is
already
showing if a save started; a repeat run that is already light never
shows it,
so the test is idempotent and there are no direct API calls.

To give the test a UI signal, `Spinner` gets an opt-in `label` prop that
exposes it as a `role="status"` live region with an `aria-label`
(decorative
otherwise). `Loader` moves its label onto its own status container so it
keeps
a single status region.

## Changes

- `site/src/components/Spinner/Spinner.tsx`: opt-in `label` prop.
- `site/src/components/Loader/Loader.tsx`: label on its own status
region.
- `site/src/pages/UserSettingsPage/AppearancePage/AppearanceForm.tsx`:
label
  both appearance save spinners.
- `site/e2e/tests/users/userSettings.spec.ts`: wait for the save
spinner.

## Validation

- `biome check` and `tsc -p .` pass.
- Loader + AppearancePage unit tests and the affected storybook tests
pass.
- The e2e test passed 20/20 under
`pnpm playwright:test -g "adjust user theme preference" --repeat-each
20`.

<details>
<summary>Implementation plan & decision log</summary>

# DEVEX-415: Fix flake in "adjust user theme preference" e2e test

## Problem

Playwright test `site/e2e/tests/users/userSettings.spec.ts` → `adjust
user
theme preference` flakes. After selecting the Light theme and
hard-navigating
to `/`, the reloaded page sometimes stays `dark`, failing the final
assertion.

CI Flake Bot has recorded repeated recurrences on `main` (latest
2026-08-17,
runs `32004239187`, `31745261984`) even after PR #25183 added `toPass`
retries.

## Root cause (confirmed by reading the code)

The appearance update is optimistic and its persistence is not awaited
before
navigation:

- `updateAppearanceSettings` (`site/src/api/queries/users.ts`) has an
`onMutate` that optimistically writes the new theme into the React Query
  cache. The `<html>` class flips to `light` immediately, before the
  `PUT /api/v2/users/me/appearance` completes.
- `useQueuedAppearanceSubmit`
  (`site/src/pages/UserSettingsPage/AppearancePage/AppearancePage.tsx`)
serializes submits: if a request is in flight, the next is queued and
only
  fires after the first settles.
- A fresh `member` user has **empty** appearance settings.
`migrateLegacyPreference` (`site/src/theme/themeMode.ts`) maps empty
settings
to `{ mode: "single", theme: "dark" }` (`DEFAULT_THEME = "dark"`). So
the
"Theme mode" dropdown already starts on **Single theme** and `<html>`
starts
  `dark`. Selecting "Single theme" in the test is therefore a **no-op**
(`onChangeMode` early-returns when `mode === draft.mode`) and fires
**no**
  PUT. The only appearance PUT in the test is the one from clicking
"Light default" (`onSelectSingle("light")` → `theme_preference:
"light"`).
- The test's first `expectLightThemeClasses(page)` passes purely from
the
  optimistic cache. It then calls `page.goto("/")` almost immediately
  (~20 ms after the click). If PUT #2 (the light one) has not persisted
server-side, the new document loads the still-persisted `dark`
preference
from embedded metadata, and every retry of the post-navigation assertion
  sees `dark` for the full 10 s window.

`toPass` cannot help post-navigation because it only re-reads stale,
already
persisted state; it cannot make an unfinished/queued PUT complete.

## Implemented fix: wait for the saving spinner (UI signal) before
navigation

> Iteration history: (1) An earlier attempt waited on the appearance
`PUT` via
> a `waitForApiCall` helper, but that couples the test to a state
transition
> and is non-idempotent (a repeat run that is already light fires no
PUT, so
> the wait times out). CI retries reuse the same ephemeral server, so
this is a
> real hazard. (2) A second attempt confirmed persistence with
> `page.request.get(...)`, but that calls the API directly and stops
being a
> site test. Both were rejected.

The repo's non-flaky settings tests wait for a **visible save
confirmation**
(e.g. "settings updated successfully" toasts) before trusting the
result. The
appearance theme form has no toast; its only save-in-progress feedback
is the
`<Spinner>`. So the fix mirrors that pattern using the spinner:

1. Make the theme section's spinner identifiable via a new opt-in
`label` prop
   on `Spinner` (sets `role="status"` + `aria-label` only when provided;
   decorative otherwise). `Loader` moves its label onto its own `status`
   container so it keeps a single status region and its `getByLabelText`
   queries keep working.
2. In the test, after clicking "Light default", assert the optimistic
light
class, then wait for that spinner to be hidden before `page.goto("/")`.

Why this is correct and idempotent:

- React Query sets `isPending` before `onMutate` applies the optimistic
cache
update, so by the time the optimistic light class is visible the spinner
is
already showing if a save started. Waiting for it to clear guarantees
the PUT
  settled (and was not canceled by navigation) before the reload.
- A repeat run that is already light: clicking "Light default" is a
no-op radio
  change, no PUT fires, the spinner never shows, and `toBeHidden` passes
  immediately. The reload still shows light.
- No direct API calls: the test only observes site UI.

### Implemented changes

`Spinner` gains an opt-in `label` prop
(`site/src/components/Spinner/Spinner.tsx`):

```tsx
role={label ? "status" : undefined}
aria-label={label}
```

`Loader` carries the label on its own status container
(`site/src/components/Loader/Loader.tsx`), and the appearance save
spinners use
the new prop (`AppearanceForm.tsx`):

```tsx
<Spinner loading={isUpdating} size="sm" label="Saving theme preference" />
<Spinner loading={isUpdating} size="sm" label="Saving terminal font" />
```

`site/e2e/tests/users/userSettings.spec.ts`:

```ts
await expect(
	page.getByRole("combobox", { name: /theme mode/i }),
).toContainText("Single theme"); // precondition: single mode

const singleThemeGroup = page.getByRole("group", { name: "Theme" });
await expect(singleThemeGroup).toBeVisible();
await singleThemeGroup.getByText("Light default", { exact: true }).click();

await expectLightThemeClasses(page); // optimistic DOM => spinner showing if saving
await expect(
	page.getByRole("status", { name: "Saving theme preference" }),
).toBeHidden(); // save settled before the hard reload

await page.goto("/", { waitUntil: "domcontentloaded" });
await expectLightThemeClasses(page);
```

Validation: `biome check`, `tsc -p .` pass; Loader + AppearancePage unit
tests
and the affected storybook tests pass; the e2e test passed 20/20 under
`pnpm playwright:test -g "adjust user theme preference" --repeat-each
20`
(idempotent).

## Alternatives considered

- **Wait on the appearance `PUT` via a `waitForApiCall` helper**:
rejected. It
couples the test to a state transition and is non-idempotent, a repeat
run
that is already light fires no PUT so the wait times out (14/15 repeats
  failed). CI retries reuse the same ephemeral server, so this is a real
  hazard, not just a local-repeat artifact.
- **Confirm persistence with `page.request.get(...)`**: rejected. It
calls the
  API directly and stops being a site test.
- **Default `role="status"` on the shared `Spinner`**: rejected.
`Loader` wraps
  `Spinner` in its own `status` div, so a default would nest two status
regions and break `Loader.test.tsx`. The opt-in `label` prop avoids
this.
- **Add a networkidle wait or sleep before navigation**: rejected.
Violates
the repo guidance against `time.Sleep`-style timing hacks and is
inherently
  racy.
- **Fix product behavior instead of the test**: out of scope. Related
bug
DEVEX-94 ("Light Theme setting not respected until Appearance page
opened")
tracks product-side persistence/embedding behavior; this task is scoped
to
  stabilizing the e2e test.

## Files touched

- `site/src/components/Spinner/Spinner.tsx` (new opt-in `label` prop).
- `site/src/components/Loader/Loader.tsx` (label on its own status
region).
- `site/src/pages/UserSettingsPage/AppearancePage/AppearanceForm.tsx`
(use the
  `label` prop on both appearance save spinners).
- `site/e2e/tests/users/userSettings.spec.ts` (wait for the spinner).


</details>

---
This PR was created by Coder Agents on behalf of @jeremyruppel.

---------

Co-authored-by: Samuel Volin <[email protected]>