diff --git a/devlog/_plan/260911_lane_dispatch_round/100_wp3_plan.md b/devlog/_plan/260911_lane_dispatch_round/100_wp3_plan.md new file mode 100644 index 00000000000..c4c7c87959b --- /dev/null +++ b/devlog/_plan/260911_lane_dispatch_round/100_wp3_plan.md @@ -0,0 +1,136 @@ +# wp3 — tracking protocol (revision 3, after two wp3 audit rounds) + +The seven lane branches are published, so tracking has already begun: their remote heads are live +values today, before any thread pushes. What is missing is that the ledger must be regenerated by +commands whose flags are named. Audit round 2 of the round unit caught a hand-written ledger citing a +stale head; wp3 audit round 1 caught this plan trusting commands that do not return what it claimed; +wp3 audit round 2 caught the CI rule passing vacuously when no run exists at all. + +## Corrected command set + +- Remote head: `git ls-remote origin refs/heads/`. +- Pull request: `gh pr list -R lidge-jun/opencodex --head --state open --json` + `number,headRefOid,reviewDecision,mergeStateStatus`. Use `list`, never `view`: `gh pr view ` + exits 1 when no pull request exists, which breaks a `set -e` loop, while `list` returns `[]` and + exits 0. Treat `[]` as "no PR yet" and never let `jq '.[0].number'` print `null` into a table cell. +- Workflow runs: `gh run list -R lidge-jun/opencodex --commit --json` + `workflowName,conclusion,status,databaseId,headSha,event`. `--commit` binds the SHA; `--branch` does + not. Do not read `.[0]`: the list is newest-first across all workflows, so the first row is often a + cancelled hygiene run while the product workflow is still `action_required`. + +## How CI is judged + +Reduce the run list to the newest run per `workflowName` for that exact SHA, by highest +`databaseId`. Then: + +- An empty list is **not** green. It means no run exists for that commit, which is the state every + lane branch is in right now. Absence of evidence is recorded as `no CI`, never as success. +- `success` is the only passing conclusion. `cancelled`, `skipped`, `action_required`, `failure`, + `timed_out`, and an empty conclusion with `status` other than `completed` are all not-green. + `action_required` in particular is the normal state of a fork run awaiting maintainer approval. +- The workflows that must be present and `success` depend on what the pull request touches, because + `ci.yml` gates its product legs behind a `changes` filter. A pull request touching anything under + `src/`, `gui/`, `tests/`, or `scripts/` must show `Cross-platform CI` as `success` on that SHA; one + touching `gui/` must also show `React Doctor`; one touching the service paths must also show + `Service lifecycle`. `PR hygiene`, `PR Labeler`, and `Enforce PR target branch` are required on + every pull request. +- A documentation-only pull request legitimately shows the product legs as skipped, as #4217 and + #4220 did. That is recorded as "product legs skipped by the changes filter", not as green product + CI, so the distinction survives into the ledger. +- `mergeStateStatus` is not a CI verdict. It is frequently `UNKNOWN` on a perfectly mergeable pull + request, as it was on #4210, #4203, and on #4220 after it merged. + +## What this phase produces + +1. `110_tracking_refresh.md` — the refresh block written out with the flags above plus these reading + rules, so a later session cannot re-derive a softer version. +2. One executed snapshot with its capture time, proving the block runs against the live repository. + The snapshot says plainly that any lane pull request opened afterwards supersedes it. +3. The wp4 merge choreography below. + +## Merge choreography for wp4 + +Order: L5, L3, L4, L2, L6, L7, L1. L1 goes last because `core.ts` is contended by four open pull +requests and every other lane would rebase behind it; L5 goes first because it has the least +contention inside this round, though it is not collision-free in the repository at large — open draft +#3833 also edits `src/integrations/registry.ts`. Within a lane, a child lands only after its parent +and is retargeted from the parent branch to `dev` first. + +Gate for each merge, re-queried at merge time rather than read from the snapshot: + +1. `git ls-remote origin refs/heads/` equals the pull request's `headRefOid`. +2. CI on that exact SHA passes the rule above, including the presence requirement. +3. `scripts/ci/assert-mergeable-review.sh [--maintainer-integration] ` exits zero. That script, + not `reviewDecision`, is the review gate: it requires each maintainer's *latest* review bound to + the exact current head and fails closed on any API error, which a `reviewDecision` read does not. +4. A carried or reimplemented pull request has its `Co-authored-by` trailer present in a branch + commit, verified with `git log`, before the merge rather than after. +5. The base is still `dev`, re-read after the previous merge in the series. + +The merge itself is `gh pr merge --merge` or `--squash` — never rebase — with +`--match-head-commit `, so a push landing between the gate and the merge aborts the merge +instead of laundering stale evidence. When the maintainer-integration path is used, record the +decision and the exact-head evidence with it. + +Landing proof: `git fetch origin` and then ancestry of the merge commit, or tree equality when the +pull request was squashed, because a squash leaves the pull request head outside `origin/dev` +ancestry. The merge command's own output is never the proof. + +Test-layout conflicts: `scripts/test-layout/layout.json` and `tests/fixtures/test-layout-expected.json` +are resolved as a union of both sides' entries, never `--ours` or `--theirs`, since dropping a side +silently unregisters another lane's test file. + +## Acceptance + +- The refresh block runs with exit 0 today, when no lane has a pull request and no run exists, and + prints one line per lane with the remote head, `no PR`, and `no CI`. +- Every printed field comes from one of the three commands above. +- The snapshot carries its capture time and the superseding note. +- The choreography names the order, the five gate conditions, the head-pinned merge command, the + landing proof for both merge and squash, and the union rule for the two test-layout maps. + +## Out of scope + +No lane implementation, no lane pull request, no merge. Contributor pull requests outside this round +stay outside it: merging them is a review decision the operator has not delegated. + +## Local checks + +`NOT RUN`. The refresh block only reads. + + +## Amendments from wp3 audit round 3 + +The CI rule above was an allowlist, which is the wrong shape: it can only catch a failure it already +named. It is replaced by its inverse. + +- **Every** newest-per-`workflowName` run on the exact head must be `success`. A red workflow outside + any named set fails the lane; nothing passes by not being on a list. +- Only runs whose `event` is `pull_request` or `push` count toward the gate. A later + `workflow_dispatch` success must not outrank a failed `pull_request` run for the same workflow, + which is why `event` is fetched. +- Presence, corrected against the workflow files rather than from memory: `React Doctor` has no path + filter and runs on every pull request, so it must always be present. `Cross-platform CI` is gated by + the `changes` `ci` filter in `.github/workflows/ci.yml`, which covers more than `src/`, `gui/`, + `tests/`, and `scripts/` — it also includes `bin/**`, `docker/**`, `assets/**`, `package.json`, and + selected workflow files. `Service lifecycle` triggers on `src/service.ts`, `src/cli.ts`, + `src/cli/index.ts`, `src/lib/bun-runtime.ts`, `package.json`, `bun.lock`, and its own two workflow + files, which is why L4 in particular must read the filter rather than guess. +- The authoritative per-pull-request required set is `gh pr checks `, because GitHub already + resolved the filters for that head. `gh run list --commit` supplies the workflow-level conclusion + and the `event`; `gh pr checks` supplies the check-run granularity. Use both. +- "Product legs skipped" is a check-run fact, not a workflow fact: #4220 showed `Cross-platform CI` + and `React Doctor` as workflow-level `success` with the jobs skipped inside. Read it from + `gh pr checks`, never from `gh run list`. + +Two corrections to the merge gate: + +- `scripts/ci/assert-mergeable-review.sh` was described too loosely. Its default path requires one + non-author maintainer approval bound to the exact current head and no outstanding maintainer + `CHANGES_REQUESTED`; the `--maintainer-integration` path waives the second approval and is not + itself an approving review. +- Security review is a separate duty and this round triggers it: L3 owns + `src/server/management/oauth-account-routes.ts`, an authentication and credential surface, so + `MAINTAINERS.md` requires explicit security review before that lane lands. CI being green does not + discharge it. + diff --git a/devlog/_plan/260911_lane_dispatch_round/110_tracking_refresh.md b/devlog/_plan/260911_lane_dispatch_round/110_tracking_refresh.md new file mode 100644 index 00000000000..9919b150888 --- /dev/null +++ b/devlog/_plan/260911_lane_dispatch_round/110_tracking_refresh.md @@ -0,0 +1,52 @@ +# Tracking refresh + +Run this to regenerate the lane rows of `060_ledger.md`. Every field it prints comes from a command, +so the ledger cannot drift into narration the way it did before audit round 2 caught a stale head. + +```bash +set -uo pipefail +REPO=lidge-jun/opencodex +for b in codex/260911-l1-responses-core codex/260911-l2-catalog-provider codex/260911-l3-account-pool codex/260911-l4-service-cli codex/260911-l5-integrations-io codex/260911-l6-streaming-tools codex/260911-l7-docs; do + head=$(git ls-remote origin "refs/heads/$b" | cut -f1) + if [ -z "$head" ]; then echo "$b MISSING"; continue; fi + pr=$(gh pr list -R "$REPO" --head "$b" --state open --json number,headRefOid,reviewDecision,mergeStateStatus) + prtxt=$(printf '%s' "$pr" | jq -r 'if length==0 then "pr=none" else "pr=#\(.[0].number) prhead=\(.[0].headRefOid[0:9]) review=\(.[0].reviewDecision // "-") mergeState=\(.[0].mergeStateStatus // "-")" end') + runs=$(gh run list -R "$REPO" --commit "$head" --json workflowName,conclusion,status,databaseId,event) + ci=$(printf '%s' "$runs" | jq -r '[.[] | select(.event=="pull_request" or .event=="push")] | group_by(.workflowName) | map(max_by(.databaseId)) | if length==0 then "ci=no-run" else (map(select(.conclusion != "success")) | if length>0 then "ci=NOT-GREEN[" + (map(.workflowName + "=" + (if (.conclusion // "") == "" then .status else .conclusion end)) | join(";")) + "]" else "ci=green[" + ((. | length | tostring)) + " workflows]" end) end') + echo "$b head=${head:0:9} $prtxt $ci" +done +``` + +## Reading rules + +- `pr=none` means `gh pr list --head` returned `[]`. Use `list`, never `view`: `gh pr view ` + exits 1 when no pull request exists and would abort the loop. +- `ci=no-run` means no `pull_request` or `push` run exists for that exact head. It is **not** green. + Every lane reads that way right now, before any thread pushes. +- The verdict reduces to the newest run per `workflowName` by `databaseId`, and requires *every* one + of them to be `success`. It is not an allowlist: a red workflow nobody thought to name still fails + the lane. +- Only `pull_request` and `push` events count, so a later `workflow_dispatch` success cannot outrank a + failed run of the same workflow. +- `mergeState=UNKNOWN` is common and is not a mergeability verdict; it appeared on #4210, #4203, and + on #4220 after it merged. +- Which checks are skipped inside a green workflow is a check-run fact, readable only from + `gh pr checks `. #4220 showed `Cross-platform CI` and `React Doctor` as workflow-level `success` + with the product jobs skipped inside, which is correct for a documentation-only pull request and + must be recorded as such rather than as green product CI. + +## Snapshot — 2026-09-10T16:47Z + +```text +codex/260911-l1-responses-core head=29c342da7 pr=none ci=no-run +codex/260911-l2-catalog-provider head=b28f06ee0 pr=none ci=no-run +codex/260911-l3-account-pool head=6fd401636 pr=none ci=no-run +codex/260911-l4-service-cli head=f48cede91 pr=none ci=no-run +codex/260911-l5-integrations-io head=a7a92089c pr=none ci=no-run +codex/260911-l6-streaming-tools head=3760f81fd pr=none ci=no-run +codex/260911-l7-docs head=cd5dcd6a2 pr=none ci=no-run +``` + +Exit 0. Any lane pull request opened after that time supersedes this snapshot; re-run the block +rather than trusting it. +