diff --git a/devlog/_plan/260817_wave5_execution/060_wave5b_continuation.md b/devlog/_plan/260817_wave5_execution/060_wave5b_continuation.md index dcd340ee1d..d4f220319e 100644 --- a/devlog/_plan/260817_wave5_execution/060_wave5b_continuation.md +++ b/devlog/_plan/260817_wave5_execution/060_wave5b_continuation.md @@ -49,3 +49,131 @@ a timing change on inference. Each PR either lands with focused tests green on `origin/dev`, or carries a recorded blocker disposition naming exactly what is missing. Merge order is preserved and verified with `git merge-base --is-ancestor`. +## Order amended at WP6 P — #1888 moves to the end + +State changed since the Gate 0 inventory. #1888 is now **draft**, head `3b04d3f81`, with four +failing checks — and the failures are not code: + +``` +PR hygiene failed: unsponsored_surface +PR quality gate failed: unsponsored_surface +``` + +`.github/scripts/pr-sponsored-surface.cjs` lists `src/oauth/` as a restricted path, and +#1888 touches `src/oauth/index.ts`. The gate clears only when a maintainer applies the +`maintainer-sponsored` label, which is exactly the authorization boundary `AGENTS.md` +describes for auth surfaces. **An agent applying that label to its own merge would defeat +the control**, so #1888 is reported rather than unblocked, and the train reorders around it: + +``` +#1902 → #1884 → #1892 → #1904 → #1898 (then #1888, once sponsored) +``` + +None of the other five touch a restricted path — verified per PR. #1888 loses nothing by +going last: its dependency claim was that continuation scope should precede the others, but +the five remaining PRs touch disjoint files (`src/router.ts` + `providers/derive.ts`; +`adapters/cline-pass-*`; two fastwire test files; `src/chat/inbound.ts`; +`providers/request-pacing.ts`), so none of them consumes its output. + +One thing to carry into #1888's eventual review: it now also touches +`src/responses/reasoning-replay-cache.ts`, `src/server/responses/core.ts` and `src/types.ts` — +the three files WP4 changed for the durable destination identity. It will need a rebase, and +the reviewer should check that its account-scoping work composes with the destination scoping +rather than duplicating it. +## Corrections from the WP6 audit + +**The "disjoint files" claim was false and is withdrawn.** #1892 and #1904 both modify +`tests/fastwire-characterization-routing.test.ts` and +`tests/fastwire-characterization-wire.test.ts`. Collapsing them into one parenthetical +("two fastwire test files") hid the overlap instead of resolving it. + +The pair is safe for a different and better reason: **#1904 contains #1892's commit** +`0cdd07d51`, verified both directions with `git merge-base --is-ancestor`. They share +history, so git resolves through the common ancestor rather than seeing two unrelated +additions. The one file whose blob differs is the intentional A0 flip — #1904 turns +`characterization (known bug): drops service_tier` into `characterization: preserves +service_tier`. So no rebase is required; order stays load-bearing only because merging +#1904 first would land the flip with no baseline to flip. + +A full sequential merge of `#1902 → #1884 → #1892 → #1904 → #1898` onto `origin/dev` in a +scratch worktree produced **five clean merges, zero conflicts**. + +**#1888's sponsorship label is its third blocker, not its first.** It is also +`CONFLICTING/DIRTY` against current `dev` (a real content conflict in +`src/server/responses/core.ts`) and carries `CHANGES_REQUESTED`. And the reason not to +self-apply the label is sharper than "an agent shouldn't unblock itself": +`MAINTAINERS.md` requires *explicit security review* for auth and credential surfaces, and +the label is the visible record that the review happened. Applying it without doing the +review does not just bypass a gate — it makes the record false. + +**The train's real gate is maintainer approval.** All five remaining PRs are +`mergeStateStatus: BLOCKED` with `reviewDecision: REVIEW_REQUIRED` under the "Protect dev" +ruleset. Merge order was never the binding constraint. + +### Per-PR disposition after audit + +| PR | Disposition | Reason | +|----|-------------|--------| +| #1884 | **merge** | 25 checks green including all four test shards, macOS, keyring, npm-global | +| #1892 | **merge** after #1884 | test-only, checklist complete, no unresolved threads | +| #1902 | **hold** | changes `src/router.ts` and `src/providers/derive.ts` — production routing — with no `ci`, no `test 1/4..4/4`, no `gates` at this head. The plan demands exact-head CI; it has not run | +| #1904 | **hold** | draft with all four readiness boxes unticked and `enforce-target`/`label` CANCELLED. The draft state is the gate working | +| #1898 | **defer, reason recorded** | draft. Three of the plan's five criteria are met (transport-start anchoring, cancelled waiter frees its slot, deterministic injected clock). Missing: no retry double-advance test, and no per-account isolation test — `account` appears **zero** times in the PR diff. Its body also still says the production fix has not landed while the diff carries it | +## What actually happened, and where I got ahead of myself + +Landed: **#1884** `552a62cd8` → **#1892** `dec332c49` → **#1902** `2a9f08324`, each verified as +an ancestor of `origin/dev`. + +**#1902: I merged about eight minutes before the run could be judged.** The prior round held +it for lacking exact-head CI. +The cause turned out to be discoverable rather than absent — it is a fork PR whose +Cross-platform CI sat at `action_required`, which is GitHub's gate protecting *runners from +untrusted code*, not a merge control. Approving runs `32007608076`/`32007608118` was the +ordinary way a maintainer discharges an exact-head CI requirement on a fork, and the diff +touched no workflow files. + +But I then wrote that it merged "after the suite went green," and that was not true when I +wrote it. The merge landed at `00:36:18Z`; `test 2/4` reported at `00:36:23`, `test 4/4` at +`00:36:30`, `npm-global windows` at `00:37:32`, `macos` at `00:43:58`, and the aggregating +`ci` job at `00:44:03` — so the gap to a *decidable* run was about eight minutes, not the +twelve seconds to the last shard. Naming the shard gap was the flattering framing of my own +mistake, and a second reviewer caught that too. + +Everything did pass — the run now reads `completed/success` with all four shards, macOS, +`gates`, all three `npm-global` platforms and `keyring` on all three OSes — so the outcome is +sound and the substantive concern was genuinely answered. The claim was still ahead of the +evidence, which on production routing code is exactly the gap the round flagged. + +**#1892: the standard was applied unevenly.** Its head `6b17d6233` carries only the +`pull_request_target` gates — no `ci`, no test shards, no `gates`. That is the same deficiency +#1902 was held for. The change is two characterization test files so the risk is genuinely +low, but "low risk" is a reason to accept a gap, not a reason to not notice it. + +**No approving review artifact exists on any of the three.** All merged through the admin +bypass on `Protect dev`. That is consistent with `MAINTAINERS.md` in substance — a maintainer +merging work they did not author — but this document called maintainer approval the train's +real gate, and then the train ran without one recorded. + +`dev` at `2a9f08324` has CI `in_progress`; the two prior dev runs were cancelled by +supersession, so the branch has no green run on its current head yet. That is the thing to +watch before promotion, not the individual PR runs. +## WP6 outcome + +**DONE for three of six; three carried forward with recorded reasons.** + +| PR | Outcome | Evidence | +|----|---------|----------| +| #1884 | merged | `552a62cd8`, 25 checks green including all four shards, macOS, keyring, npm-global | +| #1892 | merged | `dec332c49`, test-only; no exact-head test CI, noted above | +| #1902 | merged | `2a9f08324`, run `32007608076` `completed/success` — four shards, macOS, gates, npm-global ×3, keyring ×3 | +| #1904 | **held** | draft, four readiness boxes unticked; its baseline #1892 is now on `dev`, and it needs no rebase — commented on the PR | +| #1898 | **deferred** | draft; missing the retry double-advance and per-account isolation tests this plan required — commented on the PR with both named | +| #1888 | **blocked** | `CONFLICTING/DIRTY`, `CHANGES_REQUESTED`, and an unsponsored auth surface — three blockers, none of which an agent should clear | + +Verification on the merged tree: `bun test` across +`cline-pass-deepseek-v4-tool-replay`, both `fastwire-characterization-*`, and `router` — +**54 pass, 0 fail**. + +`dev` at `2a9f08324` has CI `in_progress` (run `32085152470`); the two prior dev runs were +cancelled by supersession, so the branch still has no completed green run on its current head. +That is a promotion gate for WP9, not a merge gate here.