Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -0,0 +1,158 @@
# 260914 — Regression audit and release

## Why this unit exists

`dev` is 43 commits ahead of both `main` and `preview`. Most of that arrived
today, across two delivery rounds plus a separate cost-guard session, and the
lanes were deliberately run in parallel. Each pull request was reviewed and each
reached green hosted CI at its own head. That is not the same as the merged tree
being right, because a lane only ever saw `dev` as it stood when the lane
branched.

So the question this unit answers is narrow and specific: **did any two merges
that touched the same file disagree with each other once both were on `dev`?**
Only after that is answered does the tree get promoted.

## The actual regression surface

Six source files were touched by three separate merges in this delta, and twelve
more by two. Those, not the diff size, are where a cross-merge regression can
live.

| File | Merges that touched it |
|---|---|
| `src/codex/routing.ts` | the cache-safe quota rebind, the cache-affinity default, the transient-hold fix |
| `src/server/responses/core.ts` | the control-strip scoping, the forward-identity sanitation, the terminal-refusal and reasoning-blob work |
| `src/web-search/passthrough-bridge.ts` | the destination assessment, the backend model binding, the mixed-tool leg |
| `src/chat/inbound.ts` | inbound image normalization, tool-result image carry, lossy-conversion refusal |
| `src/server/chat-native.ts` | same three chat-path merges |
| `src/server/chat-completions.ts` | same three chat-path merges |

Two-merge files worth naming because they cross lane boundaries:
`src/config.ts` and `src/types/config.ts` (the version-line bump and the
auto-refresh schema), `src/providers/registry.ts`, `src/adapters/anthropic.ts`,
`src/cli/connect.ts` and `src/cli/dispatch.ts`.

The routing file is the one to worry about most. Three separate sessions changed
account-binding behavior there in sequence, and one of them made cache affinity
the default, which changes the branch the other two are reached through. That is
exactly the shape of a regression that every individual CI run can be green for.

## How the audit runs

Reviewer subagents, one per contended file group, each reading the merged state
on `dev` rather than any single pull request's diff. The question put to each is
whether the merged result is coherent, not whether each change was correct on its
own. A finding is either fixed before promotion or written down here.

Alongside that, hosted Cross-platform CI must be green at the exact `dev` tip
SHA that gets promoted — not at a lane head, and not at an earlier tip.

## Promotion and release path

Promotion is a pull request from a release branch into `preview` and then into
`main`, matching how 2.54.0 was promoted. Both branches carry rulesets requiring
a pull request, so no direct push is attempted at any point.

`dev` already carries the 2.55.0 version line, opened ahead of the 2.54.0
release, so the stable release is 2.55.0 and the preview is the matching
preview stamp.

The npm release itself is dispatched through the Release workflow with an
explicit `expected-sha`, so a branch that moves between verification and
dispatch fails the publish instead of shipping an unaudited commit.

### One deliberate deviation, stated plainly

`scripts/release.ts` is the release authority, and its step 1 preflight runs a
branch and clean-tree guard, version-availability and channel-forward checks, a
dependency audit, a typecheck, the full test suite and a privacy scan locally
before it will bump anything. This unit does not run that preflight, because the
standing rule for this work is that no local suite runs and hosted exact-head CI
is the proof of record.

An audit of this plan corrected three things about that substitution, and the
corrections matter more than the original claim did.

The typecheck and privacy scan really are covered: the CI `gates` job runs
`bun x tsc --noEmit` and `bun run privacy:scan` directly. But the suite is
**not** run in the same grouping. The preflight copies CI's isolation policy, not
its shard layout: CI runs four Linux shards through
`scripts/ci/run-bun-test-batches.sh` with the worker-heavy files pulled into
dedicated jobs, plus two macOS shards and an unsharded macOS control job, while
the preflight runs one `bun test --isolate tests` with path-ignores and then
seven isolated files one at a time. Same files, different partitioning. CI is the
broader of the two, since it adds the macOS matrix the preflight never runs.

`audit:high` is not uncovered either — `release.yml` runs it as a publish step,
and so are the branch match and the unused-version, unused-tag and
unused-GitHub-release checks, which run in `validate-dispatch` and
`Preflight release metadata`. Only two things are genuinely script-only: the
clean-working-tree guard, and `assertChannelVersionMovesForward`, which reads
the live npm dist-tags and refuses a channel that would move backwards. Both are
checked by hand before each dispatch.

Everything the script does after step 1 is performed the same way, with one real
difference: the script bumps, commits and **pushes directly** to `main` or
`preview` using a release deploy key. This unit does not push to a protected
branch at all. The version line moves inside the promotion pull request, and the
merge commit becomes the release SHA.

### The version-line ordering, which the audit caught twice

The first draft said `dev` already carries 2.55.0 so 2.55.0 is what ships. That
is backwards, and `release.yml` would have refused the publish. Its **Require
dev to be ready for this release** step runs `version-line.ts assert-ahead`
against `origin/dev:package.json`, and equal versions fail. `dev` is opened at
the NEXT version before a release, not at the version being released — exactly
what commit `866367a6ff` in this very delta did when it opened `dev` at 2.55.0
ahead of shipping 2.54.0.

The second draft still said one product tree goes to both branches. It cannot.
`release.yml` requires `package.json` to **equal** the dispatched version, and a
`preview` dispatch must carry a prerelease version. So `preview` and `main`
carry two different version lines over the same product tree, which is what
2.54.0 did: `main` at `2.54.0`, `preview` at `2.54.0-preview.20260914`.

The order, then:

1. Promote the audited `dev` tree to `preview` through a pull request whose
branch **rewrites `package.json` to `2.55.0-preview.<stamp>`**.
2. Publish that preview from `preview`. No dev move is needed first, because a
stable 2.55.0 on `dev` already outranks the prerelease.
3. Promote the same audited tree to `main` through a pull request that leaves
`package.json` at `2.55.0`.
4. Move `dev` to 2.56.0 by dispatching `dev-version-bump` with
`intended-version=2.55.0` — the input is the version about to be released,
and the workflow opens a pull request rather than pushing — then merge it.
5. Only then publish stable `2.55.0` from `main`.

Dispatch inputs for both publishes are `version`, `tag`, `expected-sha` and
`dry-run`. `expected-sha` must be the full 40-character SHA and must equal the
branch head at dispatch time, which is what makes a branch that moved fail the
publish instead of shipping something unaudited. Each publish is dispatched once
as a dry run and then re-dispatched with `dry-run=false`.

Two prerequisites at the release SHA, both easy to get wrong:

- `release.yml` accepts only a successful **push-event** `ci.yml` run on
`main`/`preview` for that commit. A green pull-request run at the same SHA is
refused, so the run that counts is the one the merge itself triggers.
- Service lifecycle must also be green there, because `package.json` is a
service-lifecycle trigger path.

## Acceptance criteria

1. Every file touched by more than one merge in the delta is reviewed for
cross-merge interaction, with each finding fixed before promotion or recorded.
2. The exact `dev` tip SHA being promoted has green hosted Cross-platform CI.
3. `preview` carries the promoted tree and a preview npm release is published,
with the workflow run and resulting dist-tag recorded.
4. `main` carries the promoted tree and the stable npm release is published,
with the workflow run, dist-tag and git tag recorded.

## What would make this fail

Promoting on the strength of thirteen green lane runs. Every one of those was
green against a different `dev`. The only CI result that says anything about
what users will install is the one at the tip being promoted.
Original file line number Diff line number Diff line change
@@ -0,0 +1,67 @@
# 010 — Cross-merge audit findings

Five reviewers read the merged state on `dev` rather than any single pull
request's diff, one per contended file group. Four groups came back clean. One
found a real regression, and it is fixed on this branch.

## Clean

**Account routing** — the highest-risk group, and the one the audit was really
for. Three sessions changed account-binding behavior in sequence. The reviewer
established first that they were sequential rather than parallel: the cache-safe
replacement landed first, the cache-affinity default was rebased on top of it and
updated the earlier tests explicitly, and the transient-hold change describes
itself as a follow-up from reviewing the merged commit. Then it verified the
thing that actually mattered — that the cache-safe replacement is still reachable
now that affinity defaults on — by tracing that a fully spent account remains
selectable, so the rebind branch is still entered. It also confirmed the
transient-hold path and the quota-rebind path are mutually exclusive, since one
requires a soft-avoided account and the other requires a selectable one.

**Responses and Anthropic** — the ChatGPT control strip still applies on the code
paths the later merges added, because recovery and 401 replay both rebuild
through the same adapter. Opaque-blob recovery and forward-identity sanitation
compose rather than collide: an identity mismatch strips the blob before the
first send, so there is nothing left for recovery to act on.

**Chat image pipeline** — three merges on one pipeline, and the question was
whether the final-boundary refusal can now reject something the earlier
normalization deliberately produced. It cannot: the normalizer only emits Chat
image objects carrying a URL, and the refusal only matches audio, file, document
and file-id-only inputs. Those sets do not intersect.

**Config schema and CLI** — the auto-refresh section still degrades to off when
absent, and the terminal-escaping wrapper still wraps the diagnostics that the
runtime-discovery change now produces. That second one is worth noting because
those two changes are exactly the pair whose test-file conflict was resolved by
hand during round 1.

## Finding, fixed here

`src/web-search/passthrough-bridge.ts` — a mixed leg whose upstream terminal was
`response.failed` released its withheld client-executed tool call.

The merge that added mixed-tool leg termination reordered the decision so the
failed/incomplete terminal is checked before the client-executed-call case, and
routed both terminals to the same `endWithoutSearch` branch, which calls
`flushHeldCalls()`. Ten lines above it, the failure path documents the opposite
rule in as many words: releasing a tool call Codex would start executing is
exactly what must not happen. Before that reordering, a mixed leg with a failed
terminal went to the failure path and dropped the held call.

The two terminals are not interchangeable. `response.incomplete` leaves a turn
the client can still act on, so handing its call back is right. `response.failed`
does not, and releasing the call there starts work inside a dead turn.

The fix splits them on that distinction rather than reverting the reordering: the
decision now carries whether held calls may be released, true only for
`incomplete`, and the emit path drops them otherwise. The hosted cell still
closes in both cases, which is what the reordering was for in the first place.

A regression test sits directly beside the existing incomplete-terminal test, as
its sibling, asserting that no function call and no call id reach the client on a
failed terminal.

This was an intra-commit defect rather than a two-lane collision. The audit found
it anyway, because reading the merged file against its own documented invariants
is the same activity either way.
Original file line number Diff line number Diff line change
@@ -0,0 +1,78 @@
# 020 — Release runbook for 2.55.0

The exact sequence, in order, with the check that gates each step. Every command
below was derived from `scripts/release.ts`, `release.yml` and
`dev-version-bump.yml` rather than from memory, and audited against them.

## 0. Precondition

The cross-merge audit is clean or its findings are fixed and merged into `dev`.

## 1. Preview promotion

Branch from the audited `dev` tree. Rewrite `package.json` to
`2.55.0-preview.<YYYYMMDD>`. Open a pull request into `preview`.

`preview` is protected and requires a pull request, so the version line moves
inside the promotion rather than through a direct push.

Merge it. **The merge commit is the release SHA.**

## 2. Preview publish gate

At that merge SHA, both must be green before dispatch:

- `ci.yml` from the **push** event on `preview` — a pull-request run at the
same SHA does not satisfy the gate
- `service-lifecycle.yml`, because `package.json` is a trigger path for it

Then check by hand the two things only the script would have checked: the working
tree is clean, and the npm dist-tag for `preview` moves forward rather than back.

## 3. Preview dispatch

Dry run first, then the real one:

`gh workflow run release.yml --ref preview -f version=2.55.0-preview.<stamp> -f tag=preview -f expected-sha=<40-char merge sha> -f dry-run=true`

Watch it, then repeat with `dry-run=false`. The dry run exercises the real
release commit, which is the point of running it at all.

## 4. Main promotion

Branch from the same audited `dev` tree, leaving `package.json` at `2.55.0`.
Open a pull request into `main` and merge it. That merge commit is the stable
release SHA.

## 5. Move dev to 2.56.0

`gh workflow run dev-version-bump.yml -f intended-version=2.55.0`

The input is the version about to be released; the workflow computes 2.56.0 from
it and opens a pull request into `dev`. Merge that pull request.

This has to land **before** the stable publish, because `release.yml` asserts
`origin/dev:package.json` is strictly ahead of the version being released, and
equal versions fail.

## 6. Stable publish

Same gate as step 2, at the `main` merge SHA: push-event `ci.yml` on `main`,
plus Service lifecycle. Then:

`gh workflow run release.yml --ref main -f version=2.55.0 -f tag=latest -f expected-sha=<40-char merge sha> -f dry-run=true`

then the same with `dry-run=false`.

## 7. Record

The workflow run ids for both publishes, the resulting npm dist-tags, and the git
tag the workflow creates after publish.

## The failure this ordering prevents

Publishing stable 2.55.0 while `dev` still says 2.55.0 does not fail cleanly at
dispatch — it fails after the promotion pull requests have already merged, with
the tree public and the version line stuck. The repository has repaired that state
by hand four times, which is why `dev-version-bump.yml` exists at all. Doing the
dev move before the publish is the whole point of the workflow.
31 changes: 29 additions & 2 deletions src/web-search/passthrough-bridge.ts
Original file line number Diff line number Diff line change
Expand Up @@ -444,6 +444,12 @@ interface LegDecision {
searches: InterceptedSearchCall[];
message?: string;
code?: string;
/**
* Whether an endWithoutSearch leg may hand its withheld client-executed calls back.
* Only `response.incomplete` may: the client can still act on that turn. A
* `response.failed` terminal must not, for the same reason the fail path drops them.
*/
releaseHeldCalls?: boolean;
}

/** One client-executed call event held until the leg's fate is known. */
Expand Down Expand Up @@ -667,14 +673,33 @@ class BridgeStreamState {
return blocks;
}

/**
* Discard the withheld client-executed calls without emitting them. Used when the turn is
* ending in a state the client cannot act on, where releasing the call would start work
* under a turn that is already over.
*/
dropHeldCalls(): void {
this.heldCalls = [];
}

/** Decide what the leg's terminal means once the whole leg has been read. */
decide(remainingLegs: number): LegDecision {
if (this.searches.length === 0) return { kind: "end", searches: [] };
const terminalType = this.terminalPayload?.type;
if (terminalType === "response.failed" || terminalType === "response.incomplete") {
// The upstream terminal already ended this leg, so running the intercepted searches now
// would bill a search for a dead turn. The opened cells are closed unanswered instead.
return { kind: "endWithoutSearch", searches: this.searches };
//
// The two terminals differ in what happens to a withheld client-executed call, and
// lumping them together released one under a failed turn. `response.incomplete` leaves a
// turn the client can still act on, so its held call goes back. `response.failed` does
// not, and handing Codex a tool call to start executing inside a dead turn is the exact
// thing the fail path below refuses to do.
Comment on lines +693 to +697

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 Update the owned web-search runtime contract

This changes the documented streaming contract by making failed and incomplete terminals handle withheld client calls differently, but structure/INDEX.md maps src/web-search/ to structure/runtime.md and this commit leaves that document unchanged. Its current hosted-search section still groups both terminal types together without recording this new invariant, so update the owned document alongside the implementation.

AGENTS.md reference: src/AGENTS.md:L10-L11

Useful? React with 👍 / 👎.

return {
kind: "endWithoutSearch",
searches: this.searches,
releaseHeldCalls: terminalType === "response.incomplete",
};
}
if (this.sawClientExecutedCall) {
// The client's own call is unanswered, so this leg cannot continue upstream: the
Expand Down Expand Up @@ -1011,7 +1036,9 @@ async function* bridgeStreamBlocks(
error: "the upstream turn ended before the web search could run",
}));
}
yield* emit(state.flushHeldCalls());
// Only an incomplete terminal hands the withheld call back; a failed one drops it.
if (decision.releaseHeldCalls) yield* emit(state.flushHeldCalls());
else state.dropHeldCalls();
yield* emit(state.terminalFrames());
Comment on lines +1040 to 1042

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 Remove dropped calls from oversized terminal snapshots

When a response has already retained 500 output items, retain marks retainedItemsComplete false. In this failed mixed-leg path, dropHeldCalls() then clears only the queued SSE events, while terminalFrames() skips rebuilding the terminal snapshot and relays the upstream response.failed.response.output verbatim—including the supposedly withheld client function call. The original defect therefore persists for large responses; remove dropped calls from the terminal payload regardless of the retention cap, and cover this boundary in the regression test.

Useful? React with 👍 / 👎.

return;
}
Expand Down
Loading
Loading