docs(devlog): bug-PR zero-remaining campaign roadmap + executable review gate

devlog/_plan/260829_bugpr_zero_remaining/000_plan.md

@@ -0,0 +1,136 @@
+# 260829 — Bug-PR zero-remaining campaign
+
+Objective: no open pull request labeled `bug` remains on `lidge-jun/opencodex`. Every
+bug PR reaches a terminal disposition backed by evidence, the 429 OAuth rotation work
+(#2807) lands on `dev`, matching issues close with cross-references, and important
+bug issues with no PR get a reimplementation merged.
+
+## Constraints
+
+- Every change travels through a PR targeting `dev`. No direct push to `dev`.
+- CI-first evidence. The repository's own `ci`, `test N/4`, `macos`, `hygiene`, and
+  `gates` (privacy) checks on the exact head SHA are the primary proof. The full local
+  `bun run test` suite is not run; when a suite is genuinely needed it runs remotely
+  via `ssh lidge` + `ocx-run`.
+- Commits and pushes use `--no-verify`.
+- `--admin` merge is available (the operator holds admin) but never substitutes for the
+  `MAINTAINERS.md` non-author security review on authentication, credential, OAuth,
+  workflow, release, or dependency surfaces.
+- Unfixed security reproduction detail stays in gitignored `.tmp/`, never `devlog/`.
+- Unrelated dirty worktrees and the 10 existing stashes are preserved untouched.
+
+## Live triage (2026-08-29, four parallel Sol-high lanes)
+
+**Sixteen** open PRs carry the `bug` label. The first triage pass found fourteen; the A-gate
+audit found that #2744 had been missed, and #2836 (this campaign's own keystone PR) is also
+`bug`-labeled. The inventory is re-queried at the start of every work-phase and again at
+closeout, because the set moves while the campaign runs — see
+`001_audit_round1_synthesis.md`. Every row below is live evidence from
+`gh pr view`/`gh pr checks`/`gh api compare` at triage time.
+
+| PR | author | head SHA | mergeable | behind dev | real gates | security surface | matching issue |
+|----|--------|----------|-----------|-----------|------------|------------------|----------------|
+| #2835 | lidge-jun | 0dc8704531 | MERGEABLE/BLOCKED | 0 | 6 pass / 1 fail | no | none |
+| #2828 | luvs01 | 019c792607 | MERGEABLE/BLOCKED (draft) | 0 | 1 pass / 9 never started | yes (grok inject credential fields) | #2830 partially |
+| #2822 | luvs01 | 450b1bc60c | MERGEABLE/UNSTABLE | 1 | 6 pass / 1 fail | no | none |
+| #2821 | luvs01 | d21ad61d51 | MERGEABLE/UNSTABLE | 11 | 6 pass / 3 fail | no | none |
+| #2812 | gaoran1209 | 220a9048ed | MERGEABLE/BLOCKED | 77 | 5 pass / 3 fail | no | #2810 |
+| #2807 | lidge-jun | 1c61a7e8cd | CONFLICTING/DIRTY | 80 | 9 pass / 0 fail | yes (OAuth core) | none (Closes #2745 is a PR) |
+| #2799 | adtumk | e9a7bb7bb0 | MERGEABLE/CLEAN | 94 | 7 pass / 0 fail | no | none |
+| #2798 | olddonkey | 856ad72d41 | MERGEABLE/CLEAN | 97 | 8 pass / 0 fail | no | none |
+| #2797 | rrmlima | edaa044f28 | MERGEABLE/BLOCKED (draft) | 97 | 6 pass / 1 fail | yes (doctor reads env_key) | #2713 (partial) |
+| #2796 | rrmlima | 2328c16c76 | MERGEABLE/BLOCKED (draft) | 97 | 6 pass / 3 fail | yes (client fingerprint) | #2717 |
+| #2793 | smileBeda | 3a6e600eda | MERGEABLE/BLOCKED (draft) | 97 | 4 pass / 3 fail, unsponsored_surface | yes (78 files, auth core) | #2718 |
+| #2785 | DevonGithub | 107f2cbb28 | MERGEABLE/UNSTABLE | 97 | 5 pass / 1 fail | no | none |
+| #2638 | luvs01 | c8556f3703 | MERGEABLE/BLOCKED | 13 | 4 pass / 4 fail | yes (auth-context, routing) | none |
+| #2497 | MarcTCruz | 86a49e8525 | CONFLICTING/DIRTY (draft) | 496 | 6 pass / 2 fail | yes (20 files, auth core) | #2221 |
+| #2744 | yxr1995-maker | 1d8e35462a | CONFLICTING/DIRTY (draft) | 140 | CHANGES_REQUESTED | yes (core.ts + package.json) | none |
+| #2836 | lidge-jun | befcac3e10 | MERGEABLE (wp8 keystone) | 0 | 23 pass / 0 fail, macOS queued | package.json (maintainer-authored) | n/a |
+
+#2638 and #2828 moved after triage: both are now zero commits behind at rewritten heads
+(`375e6f8fb8`, `019c792607`), so their recorded reviews no longer describe their current
+diffs. They are handled by wp9, not by the reimplementation lane.
+
+## The keystone: `dev` trails its own published channel
+
+`test 2/4`, `test 3/4`, `test 4/4`, and `macos` fail on #2835, #2822, #2821, #2796,
+#2797, and #2785 with one shared assertion, not with anything those PRs changed:
+
+```
+release version line > the in-tree version is never behind a released one
+package.json version 2.35.0 is BEHIND the highest release tag v2.36.0-preview.20260829
+```
+
+Live state at triage:
+
+| ref | package.json version |
+|-----|----------------------|
+| `dev` | 2.35.0 |
+| `main` | 2.35.0 |
+| `preview` | 2.36.0-preview.20260829 |
+| tag `v2.36.0-preview.20260829` | 2.36.0-preview.20260829 |
+
+The preview bump was cut on the prerelease train and never came back to `dev`, which is
+the exact failure mode `tests/release-version-line.test.ts` was written for — its own
+header documents the previous occurrence (repaired by hand in `32529c2b2`, when `dev`
+said 2.24.2 against a published 2.26.0).
+
+Consequence for this campaign: rebasing a stale bug PR onto `dev` does **not** turn its
+CI green, because the failure is inherited from the base. The version line is therefore
+work-phase wp8 and runs FIRST; every later lane rebases onto the repaired `dev`.
+
+## Merge lanes
+
+- **wp8 keystone** — repair `dev`'s version line so inherited red turns green.
+- **wp2 Lane A** — approved and CI-clean: #2799, #2798. Rebase onto the repaired `dev`,
+  confirm exact-head green, merge. #2798 is **security-gated**: `src/lib/destination-policy.ts`
+  decides whether an OAuth bearer may be sent to an overridden destination, which
+  `MAINTAINERS.md` covers under "other security-boundary changes" even though the hygiene
+  gate's restricted-path list does not name it. Its approval must be re-earned on the
+  rebased head.
+- **wp3 Lane B** — approved but stale or inherited-red: #2822, #2821, #2785. Same
+  treatment; patch integrity proven with `git patch-id --stable` and `git range-diff`.
+- **wp4 Lane C** — #2807, the 429 OAuth rotation work. Conflicting, 80 behind, and
+  carrying one live reviewer blocker (a rotated bearer can still be paired with the
+  previous account's accepted origin). Reimplement on current `dev` with an executable
+  A→429→B regression, then non-author security review.
+- **wp5 Lane D** — reimplementation lane for PRs whose intent is right but whose branch
+  cannot land as-is: #2812 (reviewer rejected the equivalence assumption), #2796, #2797,
+  #2835 (host-identity disclosure in devlog), #2793, #2497, #2744. #2828 and #2638 were
+  moved OUT of this lane into wp9 after the audit found their reviews bound to superseded
+  heads. Every member that touches `src/server/responses/core.ts` (#2497, #2793, #2744) runs
+  AFTER wp4 and re-verifies against the accumulated file.
+- **wp9 re-audit** — #2638 and #2828 at their current heads, sequenced after wp4 because
+  #2638 touches `src/server/responses/core.ts`.
+- **wp6 Lane E** — PR-less bug issues worth reimplementing, chosen in wp1 from the 16
+  open `bug` issues.
+- **wp7 closeout** — issue closure with cross-references and the zero-open-bug-PR proof.
+
+## Verifier reality check (PLAN-VERIFIER-REAL-01)
+
+Corrected after the A-gate audit ran each command rather than trusting the plan.
+
+- **Bootstrap first:** this worktree had no `node_modules`, which made `bun x tsc --noEmit`
+  exit 1 with `TS2688: Cannot find type definition file for 'bun-types'` — an environment
+  gap, not a type error. After `bun install`, `bun x tsc --noEmit` exits 0. A fresh rebase
+  worktree needs `bun install` before its tests mean anything (a missing install surfaced as
+  `Cannot find module 'zod/v4'` on #2799's rebase).
+- `gh pr checks <N> --repo lidge-jun/opencodex` — exists, ran during triage, reads the
+  exact PR head. This is the campaign's primary verifier.
+- `bun x tsc --noEmit` and focused `bun test tests/<file>` — exist in `package.json`
+  (`typecheck`); used for local implementation loops only, after the bootstrap above.
+- `scripts/ci/assert-mergeable-review.sh <N>` — the executable pre-merge review gate added
+  by this campaign. Fails closed unless an `APPROVED` review is bound to the exact current
+  head, authored by someone other than the PR author, and listed as a current maintainer in
+  `MAINTAINERS.md`. Proven non-vacuous: exit 0 on #2798 (real exact-head approval from
+  Ingwannu), exit 1 on #2836 and #2812.
+- `bun run skill:surface:check` — the read-only verifier. `bun run skill:surface` is a
+  GENERATOR (`scripts/generate-ocx-skill-surface.ts` calls `writeFileSync`) and must never
+  be cited as a gate.
+- `bun run test` (full suite) — deliberately NOT used locally per the operator's
+  instruction; the remote equivalent is `ssh lidge 'export PATH=$HOME/bin:$PATH; ocx-run
+  <name> <workdir> <timeout> <command...>'`, confirmed present at
+  `/home/lidgeai/bin/ocx-run`. Remote evidence counts only when the recorded workdir is
+  proven to sit at the exact head SHA and the child command exercises the change; a bare
+  `rc=0` from an unrelated directory proves nothing.
+- `bun run privacy:scan` — runs inside the `gates` check on every PR head.

devlog/_plan/260829_bugpr_zero_remaining/001_audit_round1_synthesis.md

@@ -0,0 +1,150 @@
+# 001 — A-gate audit round 1: synthesis and plan amendments
+
+An independent Sol-high reviewer audited the roadmap against live repository state and
+returned `VERDICT: FAIL` with 8 blockers. Each is recorded below with its disposition.
+Two were verified independently before acceptance, because a reviewer claim is evidence to
+check, not a verdict to copy.
+
+## B1 (Critical) — inventory was stale: 16 bug PRs, not 14. ACCEPTED
+
+Live query returns 16: the 14 triaged, plus **#2744** (missed) and **#2836** (the keystone
+PR this campaign created, auto-labeled `bug`).
+
+#2744 `Recover encrypted agent tasks on the combo path before failing closed`
+(yxr1995-maker, draft, CONFLICTING/DIRTY, head `1d8e35462a`) changes 4 files:
+`package.json`, `src/server/responses/core.ts`, and two agent-task-recovery tests.
+
+Amendment: #2744 joins wp5. #2836 is wp8's own PR and needs no lane. The inventory is
+re-queried at the start of every work-phase and again at closeout, because the set moves
+while the campaign runs — this campaign itself proved that by adding a member.
+
+## B2 (Critical) — wp8 omitted review of a restricted surface. ACCEPTED WITH CORRECTION
+
+The reviewer is right that `package.json` is a restricted path
+(`.github/scripts/pr-sponsored-surface.cjs`, under `// Dependency surfaces.`) and that
+wp8's accept criteria did not mention review.
+
+The reviewer's implied conclusion — that #2836 would be hygiene-blocked — is WRONG, and the
+live gate says so: `hygiene = pass` on #2836. The reason is in the same file:
+`assessSponsoredSurface` returns `[]` immediately when `authorHasPushPermission` is true,
+because a maintainer's own change carries its own sponsorship. #2836 is authored by
+`lidge-jun`, who has admin.
+
+What survives is the governance point, and it is the stronger one: `MAINTAINERS.md` still
+requires a non-author approval, and `gh pr view 2836 --json reviewDecision` returns
+`REVIEW_REQUIRED` with no reviews. Self-approval is forbidden.
+
+Amendment to wp8 accept criteria: a fifth criterion — the merge requires a non-author
+**maintainer** approval bound to the exact head.
+
+**Withdrawn in round 2.** The first version of this amendment allowed "or an explicit
+recorded operator decision to admin-merge". The reviewer correctly identified that as the
+very bypass B3 exists to close, and it is withdrawn: an alternative that permits skipping
+the approval is not a gate. If the approval cannot be obtained, wp8 reports BLOCKED and the
+operator decides — the campaign does not pre-authorize the bypass on their behalf.
+
+## B3 (Critical) — `--admin` bypasses the approval gate; the guard was prose-only. ACCEPTED
+
+Live `dev` ruleset: `required_approving_review_count: 1`,
+`require_code_owner_review: true`, `dismiss_stale_reviews_on_push: false`, and
+`current_user_can_bypass: pull_requests_only`. So an admin merge genuinely can bypass the
+approval requirement, and GitHub cannot tell a security review from any approval.
+
+Amendment — an executable, fail-closed pre-merge check for EVERY merge in this campaign.
+Round 2 rejected the first version of this amendment because it only PRINTED reviews (a
+command that exits 0 on an empty list is not a gate) and because it checked only
+`user != author` when `MAINTAINERS.md` requires a *maintainer*. Both points are correct and
+are now fixed in code rather than in prose: `scripts/ci/assert-mergeable-review.sh`.
+
+It exits nonzero unless one review is simultaneously `APPROVED`, bound to the exact current
+`headRefOid`, authored by someone other than the PR author, and authored by an account the
+script parses out of the `## Current maintainers` table in `MAINTAINERS.md` — so the gate
+cannot drift from the policy document it enforces. Merges then use
+`--match-head-commit <SHA>`.
+
+Proven non-vacuous against live PRs:
+
+```
+$ bash scripts/ci/assert-mergeable-review.sh 2798
+OK: #2798 approved at head 856ad72d414f27556729d70ed077e04494bb7336 by maintainer Ingwannu (author olddonkey)
+EXIT=0
+
+$ bash scripts/ci/assert-mergeable-review.sh 2836
+FAIL: #2836 has no maintainer approval bound to head befcac3e10ac175f9aa8de65a799abd0b5e8f7aa
+  maintainer roster: Ingwannu lidge-jun
+EXIT=1
+
+$ bash scripts/ci/assert-mergeable-review.sh 2812
+FAIL: #2812 has no maintainer approval bound to head 220a9048edc9e6715c0c4cf7f1388e26a016293e
+EXIT=1
+```
+
+Residual limitation, stated rather than hidden: GitHub cannot mark an approval as
+specifically a *security* review, so for security-boundary PRs the reviewer's own words are
+read to confirm the approval addressed the security surface. That is a human judgment the
+script cannot make, and pretending otherwise would be the same error as the prose guard.
+
+## B4 (High) — #2638 and #2828 were assigned from stale review evidence. ACCEPTED
+
+Both moved since triage. Live: #2638 head `375e6f8fb8`, ahead 6 / behind 0,
+`CHANGES_REQUESTED` (bound to the older `c8556f3703`). #2828 head `019c792607`, ahead 5 /
+behind 0, no longer draft.
+
+Amendment: both leave the reimplementation lane and enter a current-head re-audit lane
+(wp9). Discarding an author's branch because of a finding already fixed on a newer head
+would be both wasteful and unfair to the contributor. Reimplementation stays available if
+the current head still fails review.
+
+## B5 (High) — rebasing destroys the exact-head-approved premise. ACCEPTED
+
+`dismiss_stale_reviews_on_push: false` means GitHub will happily keep an approval that no
+longer describes the code. The plan leaned on approvals granted to pre-rebase heads.
+
+Amendment: after any rebase, the approval is re-earned on the new head (B3's check enforces
+it mechanically). The reviewer's falsification work is recorded as supporting the plan: it
+inspected the failing logs of #2822, #2821, #2796, #2835, #2797, and #2785 and found ONLY
+the `release version line` assertion — no unrelated failure. The inheritance thesis stands,
+now independently confirmed and additionally proven by #2836's own `test 2/4 = pass`.
+
+## B6 (High) — verifier claims overstated. ACCEPTED, CAUSE CORRECTED
+
+The reviewer found `bun x tsc --noEmit` exiting 1 with
+`error TS2688: Cannot find type definition file for 'bun-types'`. Verified: the cause was
+that this worktree had no `node_modules` at all. After `bun install` (103 packages),
+`bun x tsc --noEmit` exits 0. So it is a usable verifier once dependencies exist — the
+plan's omission was the bootstrap step, not the command.
+
+Accepted without reservation: `bun run skill:surface` WRITES its target and is a generator;
+the verifier is `bun run skill:surface:check`. Also accepted: `ocx-run` evidence is
+meaningless unless the remote workdir is proven to be at the exact head SHA and the child
+command actually exercises the change. Both are now required in the evidence format.
+
+## B7 (High) — missed cross-lane collisions on `src/server/responses/core.ts`. ACCEPTED
+
+The plan named only the `destination-policy.ts` collision. Live intersections:
+`src/server/responses/core.ts` is touched by #2807 (wp4), #2497, #2638, #2793, and #2744.
+`package.json` is touched by wp8 and #2744.
+
+Amendment: wp4 (#2807) is serialized BEFORE every other core-touching member, and each
+later core-touching member re-verifies against the accumulated `core.ts` rather than
+against the tree it was written on.
+
+## B8 (Critical) — #2798 called "no security surface". ACCEPTED
+
+The hygiene gate's restricted list does clear all five wp2/wp3 members
+(`restricted=NONE` for #2799, #2798, #2822, #2821, #2785), so there is no gate
+misclassification. But `src/lib/destination-policy.ts` decides whether an OAuth bearer may
+be sent to an overridden destination, and `MAINTAINERS.md` covers "other security-boundary
+changes", not just the mechanical list. Calling it non-security was wrong.
+
+Amendment: #2798 is security-gated in wp2 and needs a fresh exact-head security review
+after rebase. #2812 (wp5), which edits the same file, inherits that classification.
+
+## Residual disagreement
+
+None outstanding. B2's gate mechanics were corrected and B6's root cause was corrected;
+both underlying blockers were accepted rather than rebutted.
+
+## New work-phase
+
+wp9 — current-head re-audit lane for #2638 and #2828.