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
136 changes: 136 additions & 0 deletions devlog/_plan/260911_lane_dispatch_round/100_wp3_plan.md
Original file line number Diff line number Diff line change
@@ -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/<branch>`.
- Pull request: `gh pr list -R lidge-jun/opencodex --head <branch> --state open --json`
`number,headRefOid,reviewDecision,mergeStateStatus`. Use `list`, never `view`: `gh pr view <branch>`
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 <headRefOid> --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/<branch>` 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] <pr>` 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 <headRefOid>`, 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 <pr>`, 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.

52 changes: 52 additions & 0 deletions devlog/_plan/260911_lane_dispatch_round/110_tracking_refresh.md
Original file line number Diff line number Diff line change
@@ -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 <branch>`
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 <pr>`. #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.

Loading