Review 5631

Review 5631 — route visual gh-pages writers through publisher

PR

#5631 ci: route visual gh-pages writers through publisher by cixzhang (bucket: the maintainer)

HEAD REVIEWED

311ba8cdc3c0f3e94207d86bc151559e8902758a — every claim below was verified at this commit, whose sole parent is approved #5629 head 88195bb8e6765f7fd10fd5abf7521e09d157bc42.

VERSIONS

LOOP VERSION: 1.9.1 AUDIT RUBRIC: 1.14.1

LANE

LANE: full WHY: this is a privileged workflow migration spanning four publication paths, two queue implementations, cross-run liveness, retries, immutable evidence, and a stacked successor contract.

PROBLEM

WHY 1: Independent workflows can mutate overlapping gh-pages paths concurrently, and Actions concurrency does not preserve every pending writer. WHY 2: A lost evidence, acceptance, or baseline write leaves maintainers without the durable review state needed to accept or release visual changes. WHY 3: Visual review is only trustworthy when every accepted artifact remains immutable and every baseline mutation has one serialized owner.

USER-FACING PROBLEM: A maintainer handling overlapping visual workflows can lose evidence, acceptance, or baseline state even though each individual workflow reports a locally valid publication attempt. PROBLEM SEVERITY: broken task — missing evidence or split publication ownership can prevent visual acceptance or publish an unreviewed baseline state. NEW FEATURE CASE: not a new product feature — this extends the repository-owned publication subsystem approved in #5629 to the visual workflow class. EARLY STOP: clear — the repository owner authored the migration and the prior review established the shared-publisher need and model.

VERDICT: clear

SOLUTION

Visual evidence, acceptance records, accepted baseline promotion, and manual baseline promotion are moved from independent shell publishers onto the shared FIFO publisher. Each operation re-clones the latest branch state on a rejected push, preserves unrelated paths, and keeps immutable records write-once. The workflows continue to resolve trusted identities before privileged publication.

SOLUTION (4 migration decisions · 529 added runtime/script lines of 869 additions)

1. Publish immutable PR visual evidence through the shared queue and refresh the state checkout afterward.
2. Archive visual acceptance records through the same publisher while preserving trusted identity inputs.
3. Promote accepted post-merge pixels only while holding the shared baseline turn.
4. Move manual baseline promotion onto the same turn and retry model.

BURDEN: high — four privileged mutation adapters, one shared queue, two retained executable queue state machines, five-attempt publication retries, and 22 focused tests across publisher and workflow contracts. BURDEN MATCH: disproportionate at this head — the migration intends to contract publication ownership, but it leaves the retired baseline lock executable and independently writable.

VERDICT: BLOCKS — the migration does not yet leave one publication-order authority.

ARCHITECTURE

OWNER: the shared gh-pages publisher should own every publication turn and branch mutation. TIER 1: the publisher accepted in #5629, with GitHub Actions run state as the liveness authority. TIER 2: none. SEAMS: immutable evidence, acceptance archive, accepted baseline promotion, manual baseline promotion, stale-run pruning, rejected-push retry, and roll-forward from the former baseline lock. BEHAVIOR UNIT: pure utility/module — publisher operations are exported and driven against local bare remotes; workflow contracts are checked as text. COMPLEXITY BUDGET: one publication-order owner, one durable queue/holder representation, four migrated operation adapters, and no second writable lock protocol. ACTUAL BURDEN: 7 changed files; 529 added publisher lines; 1 shared publisher plus 1 retained executable baseline-lock implementation; 2 durable queue roots; 4 migrated workflows; 22 focused tests. BURDEN TREND: parent 88195bb8 → current 311ba8cd: grew by four migration adapters, while publication-order representations remain at two instead of contracting to one. RESET TRIGGER: not triggered — this is the first model-level finding on this PR, and the contraction direction is derivable: one queue must govern both old and new runs during rollout.

domain fact	one authoritative writable source	generated / immutable projections	other writable copies
publication order and holder	.astryx-gh-pages/publication-queue	run id/scope in logs	visual-gate/publication-queue remains writable through the old executable — BLOCKS
workflow-run liveness	GitHub Actions run state	immutable run id in each ticket	none; the migrated acceptance job lacks permission to read it — BLOCKS
visual evidence identity	immutable pr/<pr>/visual/<head>/<run>/<attempt> path	report URLs and status projection	none
acceptance/baseline identity	immutable acceptance record plus recaptured merge manifest	projected status and decision log	none

seam	driven result
immutable evidence repeat	identical bytes are idempotent; different bytes refuse in the local integration test
acceptance archive	acceptance record and reviewed image archive publish through the shared queue in the local integration test
accepted baseline	accepted pixels promote only after a shared-turn assertion in the local integration test
active shared-queue blocker	waitForPublicationTurn() calls the Actions run endpoint; visual-acceptance.yml’s migrated accept job grants no actions: read
old/new rollout	a local bare-remote probe acquired a shared-queue holder for run 100 and an old baseline-queue holder for run 101 simultaneously
successor writers	preview deploy, redeploy, cleanup, and vibe screenshot writers remain outside the shared queue; the next migration must preserve pr/<n>/visual/** and use the one authority rather than add another queue

VERDICT: BLOCKS — two independent queue roots can both grant a publication turn, and one newly migrated caller cannot query the shared queue’s liveness authority.

IMPACT

An acceptance command that overlaps another queued publisher reaches the shared wait path without Actions read permission and exits before recording the decision. During roll-forward, an already-running old baseline workflow and a new workflow can each acquire a different holder and mutate the same branch without seeing the other. NEW FEATURE IMPACT: maintainers should gain one durable publication order for every visual artifact; this head centralizes the happy path but does not yet make contention or rollout single-owner.

VERDICT: BLOCKS — the defining overlap and migration states remain unsafe.

API

No package or consumer API changes. The repository-internal publisher CLI adds enqueue, wait, release, immutable-path, visual-acceptance-record, visual-baseline-accepted, and visual-baseline-manual; the visual workflows are their only new callers.

	change	public?	class	doc’d?	verdict
+	shared queue lifecycle commands with explicit scope	no — repository workflow contract	inherited publisher protocol	workflow call sites/tests	clear
+	immutable evidence and acceptance publication commands	no — repository workflow contract	scoped immutable writers	workflow call sites/tests	clear
+	accepted/manual baseline publication commands	no — repository workflow contract	baseline writers	workflow call sites/tests	clear

OSSIFICATION: no installed consumer can depend on these commands, but repository automation can; the retained old executable is therefore a competing internal contract until removed or made a one-way compatibility boundary.

VERDICT: BLOCKS — publication-order API ownership remains split between two executable CLIs.

THEMING

No CSS, tokens, theme targets, rendered elements, or style contracts change.

VERDICT: clear

BREAKING

BEHAVIOR: intended behavior changes by routing four writers through one queue; overlapping visual acceptance currently fails without Actions read permission, and old/new rollout can grant two holders. API: no consumer API change. VISUAL: no rendered UI change. THEME: no theme change.

VERDICT: BLOCKS — the migrated contention behavior is not operational in every caller and is not single-owner across rollout.

PERFORMANCE & RESOURCES

EFFECTS: zero. RENDER: no application render path changes. LISTENERS/OBSERVERS: none. LAYOUT: none. BUNDLE: no dependency or package bundle change; pngjs was already declared. CI cost: each migrated publication adds queue clones/mutations and may wait up to 75 minutes; retry loops are bounded and re-clone current branch state.

VERDICT: clear

VISUAL EVIDENCE

VISUAL CHECK: not applicable WHY: all seven changed files are workflow YAML, Node publication scripts, or tests; no UI, CSS, rendered markup, story, or component behavior changes.

VERDICT: clear

REMEDY SEARCH

REMEDY SEARCH: not triggered — no proven visual defect

A11Y & I18N

No rendered element, interaction, role, accessible string, locale formatting, or direction-sensitive behavior changes.

VERDICT: clear

JUDGEMENT

slot	verdict
PROBLEM	clear
SOLUTION	BLOCKS — migration leaves two publication-order implementations
ARCHITECTURE	BLOCKS — two queue roots can grant ownership; one caller lacks liveness permission
IMPACT	BLOCKS — acceptance overlap and rollout remain unsafe
API	BLOCKS — two executable internal queue contracts remain
THEMING	clear
BREAKING	BLOCKS — migrated contention behavior is incomplete
PERFORMANCE	clear
VISUAL	clear — not applicable
A11Y & I18N	clear — not applicable

GOAL: partly met — local integration tests prove immutable evidence, acceptance archive, accepted baseline promotion, preservation, and retry foundations, but a static permission probe and a driven dual-queue probe prove two contention paths remain broken. DISPOSITION: missing actions: read in the migrated acceptance job → blocks now; independently writable old/new queue roots during rollout → blocks now; absent exact-head repository CI on this stacked draft → report, but focused local evidence settles the two findings. ADVICE: bounded outcome criteria — every shared-publisher caller must retain the permission needed to inspect blocking runs, and old plus new workflow versions must never be able to hold publication ownership simultaneously; lock both contracts with focused tests. AUTHOR CAN PROCEED: yes — add the missing permission and make rollout converge on one authoritative queue, with a regression test that fails when old and new holders can coexist. WORST OUTCOME: “An old and a new publisher can each believe it owns baseline publication and push conflicting visual state.” → request changes.

JUDGEMENT NEEDED: none — both findings are reproducible correctness defects with bounded acceptance criteria.

request changes (self-authored fallback COMMENT)

1. The migrated visual-acceptance job calls the shared wait path without Actions read permission. → when an acceptance overlaps any active publisher, it exits before recording the maintainer’s decision · .github/workflows/visual-acceptance.yml:144

2. The migration leaves the former baseline lock executable against a different queue root. → a pre-merge run and post-merge run can both hold publication ownership and race the same baseline · .github/scripts/visual-gate/lib/baseline-publication-lock.mjs:15

REVIEW

Semantic verdict: request changes

Thanks for moving the visual writers behind the shared publisher. Two concurrency gaps still leave the migration unsafe:

• visual-acceptance.yml:144 now enters the shared wait path, but its accept job grants no actions: read; an acceptance overlapping any publisher exits before recording.
• The executable former baseline lock still writes a separate queue. A local roll-forward probe acquired old and new holders simultaneously, so pre-merge and post-merge runs can each believe they own baseline publication.

Could we retain the required permission for every queued caller, leave one authoritative queue across rollout, and cover both contracts before stacking the next migration?

[Reviewed by Robohands]

INLINE (0–3, only when genuinely line-anchored)

None — both defects cross a workflow permission block and publication-owner boundary.

EVIDENCE I DID NOT SPEND

• The four new publisher operations preserve unrelated sparse paths and rebuild their commit from a fresh clone after a rejected push.
• The acceptance reason and visual identities remain argument-vector data; no new shell interpolation, dependency, package hook, or off-registry resolution was added.
• Preview deploy/redeploy currently replace pr/<n> wholesale and therefore remain part of the next migration’s preservation contract; this is inherited behavior and is not charged to this delta.

TIME

TIME total 10m

• setup/rules: 3m — fresh loop 1.9.1, audit 1.14.1, prior round, feedback, untrusted-code preflight, and exact-head setup
• worktree/install: 1m — dedicated detached checkout and lockfile-matched install (3.9s combined)
• build/server: 0m — not applicable
• browser/a11y: 0m — non-rendering change
• focused tests: 2m — publisher/workflow Vitest, syntax, and actionlint
• code/history: 2m — permissions, state ownership, retry/idempotency, stack base, successor seam, and two local probes
• critique/wiki: 2m — presentation, critic pass, delivery, record/index update
• CI wait: 0m — no repository Actions run exists for this stacked exact head
• waste: 1m — one unavailable search binary and one mirror-hash command were retried with portable tools

WHAT I COULD NOT VERIFY

• A live overlapping GitHub Actions publication was not dispatched because that would mutate shared gh-pages; both failure paths were verified with static workflow contracts and local bare remotes.
• Full repository CI has not run for this stacked draft head.

Current CI

At exact head 311ba8cdc3c0f3e94207d86bc151559e8902758a, GitHub reports Meta CLA and Vercel Preview Comments green plus the Vercel status green. Posting the review triggered Review signal run 33150530911, whose two jobs were skipped; no substantive repository CI/test workflow has run because the draft is stacked on the open #5629 branch.

Focused evidence

• Vitest: 2 files, 22/22 tests passed in 84.68s.
• Syntax: all three changed Node files pass node --check.
• Actionlint 1.7.12: all four changed workflows pass with the repository’s documented runner/shellcheck ignores.
• Permission contract: every new shared-publisher caller has actions: read except visual-acceptance.yml’s accept job.
• Roll-forward probe: run 100 acquired .astryx-gh-pages/publication-queue/holder.json while run 101 simultaneously acquired visual-gate/publication-queue/holder.json against the same local bare remote.
• Security preflight: all seven changed workflow/script/test files were read before checkout or execution; no dependency, package hook, lockfile, or executable-mode change; focused tests use local bare remotes and a fake gh binary.

What changed before posting

Posted as drafted after one critic pass; the public review is 107 words.

Delivery

GitHub review: https://github.com/facebook/astryx/pull/5631#pullrequestreview-5048724688

Round 2 — d64f8728

PR

#5631 ci: route visual gh-pages writers through publisher by cixzhang (bucket: the maintainer)

HEAD REVIEWED

d64f87286431e6003d6b5da1df379706f09844e9 — every claim below was verified at this commit, whose sole parent is approved #5629 head 88195bb8e6765f7fd10fd5abf7521e09d157bc42.

VERSIONS

LOOP VERSION: 1.9.1 AUDIT RUBRIC: 1.14.1

LANE

LANE: full WHY: this is a delta re-review of two prior blockers in a privileged branch-publication migration; the source-of-truth remedy adds a rollout bridge and must be checked across every publication scope.

PROBLEM

WHY 1: Independent workflow generations can mutate gh-pages while consulting different publication queues. WHY 2: Two runs that both believe they own publication can race one branch update, fail one workflow, or overwrite evidence that a maintainer relies on. WHY 3: Visual acceptance and release decisions are trustworthy only when publication ownership remains singular across deployment roll-forward.

USER-FACING PROBLEM: A maintainer running workflows across the migration boundary can receive two simultaneous publication owners instead of one durable order for every artifact. PROBLEM SEVERITY: broken task — one of the supposedly serialized publications can race or fail under overlap. NEW FEATURE CASE: not a new product feature — this extends the approved repository publication subsystem to visual writers. EARLY STOP: clear — the owner-authored migration and approved parent establish the need; this round judges whether the prior concurrency acceptance criteria are met.

VERDICT: clear

SOLUTION

Current baseline publishers write both a shared ticket and a compatibility ticket that the previous baseline workflow understands, then acquire the old holder before the shared holder. The acceptance job also gains the Actions access required to inspect a blocking run. The compatibility path is applied only to the baseline scope, so other current shared publishers do not observe an old baseline holder.

SOLUTION (2 delta decisions · 363 runtime lines plus 130 test/config lines)

1. Grant the migrated acceptance job actions: read and bind every shared-publisher caller’s permission contract in the workflow test.
2. Duplicate baseline queue lifecycle into the shared publisher so new baseline writers acquire both old and new holders during rollout.

BURDEN: high for the delta — a second queue parser, mutator, blocker scan, holder claim, wait loop, and release path are copied into the shared publisher. BURDEN MATCH: disproportionate at this head — the added compatibility state machine prevents baseline-vs-baseline dual ownership but does not cover the other shared publication scopes that can run while an old baseline holder exists.

The prior review asked for actions: read on the acceptance job and one publication owner across old/new rollout. The permission ask is satisfied. The baseline-specific case is satisfied, but the broader ownership ask remains open.

VERDICT: BLOCKS — the rollout bridge leaves current non-baseline scopes outside the old-holder boundary.

ARCHITECTURE

OWNER: the shared gh-pages publisher owns branch publication order across every scope. TIER 1: the approved #5629 publisher and its global run-id FIFO. TIER 2: none. SEAMS: old baseline workflow → current baseline writer, old baseline workflow → current report/site/evidence/acceptance writer, stale-run cleanup, rejected-push retry, and release after partial failure. BEHAVIOR UNIT: pure utility/module — both production CLIs were driven against one local bare remote with a fake run-status endpoint. COMPLEXITY BUDGET: one authoritative current queue; at most one temporary compatibility projection spanning every current writer while old runs can exist; one active holder across both roots. ACTUAL BURDEN: 7 PR files; 892 added publisher lines overall; 363 publisher lines added in this round; 1 shared publisher plus the retained executable old lock; 2 writable queue roots; 4 migrated visual workflows; 25 focused tests. BURDEN TREND: 311ba8cd → d64f8728: grew by 363 runtime lines and a complete legacy queue state machine while the stated migration problem stayed the same. RESET TRIGGER: triggered — the remedy for split publication authority adds a second writable compatibility representation and scope guard, yet a second model-level dual-holder path remains.

domain fact	one authoritative writable source	generated / immutable projections	other writable copies
current publication order	.astryx-gh-pages/publication-queue	run id/scope in logs	legacy queue is still writable by old runs and by current baseline runs
rollout compatibility	current publisher should project every current writer into the old holder boundary until old runs drain	old ticket/holder with the same run id	current non-baseline writers do not project or observe that boundary — BLOCKS
workflow liveness	GitHub Actions run state	immutable run id in tickets	none

seam	driven result
acceptance permission	current workflow has actions: read; the contract test fails against 311ba8cd and passes at this head
old baseline run 899 → current baseline run 901	current baseline wait refuses behind the old holder; the added regression fails against the prior publisher and passes here
old baseline run 899 → current report writer 900	both actual production CLIs acquire different holder files simultaneously against one remote — BLOCKS
token/argument boundary	bridge paths are constants, run/repository identity is validated, and subprocess calls use argument vectors rather than a shell

VERDICT: BLOCKS — publication order still has two active authorities for old-baseline/current-nonbaseline overlap; contraction requires every current shared scope to observe the compatibility holder while old runs can exist, or a proven disjoint-lane model replacing the global-owner claim.

IMPACT

The acceptance permission failure is fixed, and a current baseline writer no longer passes an active old baseline ticket. But a current report/site/evidence/acceptance writer can still acquire the shared holder while that old baseline run owns the legacy holder, so maintainers can still see concurrent branch publication during roll-forward.

VERDICT: BLOCKS — one reachable migration state still violates the single-owner promise.

API

No package or installed-consumer API changes. The repository-internal queue contract now adds a legacy compatibility adapter limited to visual-gate/baseline; actions: read is the minimum permission required by the existing run-status lookup.

OSSIFICATION: installed consumers cannot depend on this surface, but repository automation does. The compatibility adapter remains a second internal contract until the rollout boundary covers every writer and is later removed.

VERDICT: note — internal compatibility surface grew; the correctness block is owned by ARCHITECTURE.

THEMING

No CSS, token, target, rendered element, or style contract changes.

VERDICT: clear

BREAKING

BEHAVIOR: baseline-vs-baseline rollout now serializes, but old-baseline/current-nonbaseline rollout can still dual-hold and race branch publication. API: no consumer API change. VISUAL: no rendered UI change. THEME: no theme change.

VERDICT: BLOCKS — the migration’s defining single-owner behavior remains incomplete across publication scopes.

PERFORMANCE & RESOURCES

EFFECTS: zero. RENDER: no application render path changes. LISTENERS/OBSERVERS: none. LAYOUT: none. BUNDLE: no package or dependency change. CI cost: each current baseline run now performs queue mutation and polling against both roots; no measured application-runtime degradation is claimed.

VERDICT: clear

VISUAL EVIDENCE

VISUAL CHECK: not applicable WHY: the delta changes workflow permissions, Node publication coordination, and tests; no UI, CSS, markup, story, or rendered component behavior changes.

VERDICT: clear

REMEDY SEARCH

REMEDY SEARCH: not triggered — no proven visual defect

A11Y & I18N

The delta adds no rendered element, interaction, role, accessible string, locale formatting, or direction-sensitive behavior.

VERDICT: clear

JUDGEMENT

slot	verdict
PROBLEM	clear
SOLUTION	BLOCKS — compatibility applies only to baseline scope
ARCHITECTURE	BLOCKS — old baseline and current non-baseline writers can dual-hold
IMPACT	BLOCKS — concurrent publication remains reachable during rollout
API	note — internal compatibility contract grew
THEMING	clear
BREAKING	BLOCKS — single-owner migration is incomplete
PERFORMANCE	clear
VISUAL	clear — not applicable
A11Y & I18N	clear — not applicable

GOAL: partly met — actions: read is present and its negative control fails on the prior head; the baseline-specific bridge test also fails on the prior head and passes here. A separate probe using the actual old lock and current publisher proves run 899 can hold visual-gate/publication-queue/holder.json while a current visual-gate/reports run 900 holds .astryx-gh-pages/publication-queue/holder.json. DISPOSITION: missing acceptance permission → resolved; baseline-vs-baseline dual-holder → resolved; old-baseline/current-nonbaseline dual-holder → blocks now; absent exact-head substantive GitHub CI → landing gate, not a code finding. ADVICE: bounded outcome criteria — while any old baseline run can exist, every current shared-publisher scope must observe the old holder boundary, or the system must prove and test genuinely disjoint authorities instead of claiming one global owner. AUTHOR CAN PROCEED: yes — extend the rollout contract across all current scopes and add a production-old-lock/current-nonbaseline regression that fails when both holder files exist simultaneously. WORST OUTCOME: “An old baseline publisher and a current report/site publisher can both believe they own gh-pages and push concurrently.” → request changes.

JUDGEMENT NEEDED: none — this is a reproduced concurrency defect against the already-approved global publication-owner contract.

request changes (self-authored fallback COMMENT)

1. The compatibility bridge is guarded to the baseline scope while the shared queue represents all publication scopes. → during rollout, an active old baseline run and a current report/site/evidence/acceptance run can acquire different holders and mutate the same branch concurrently · .github/scripts/lib/gh-pages-publisher.mjs:803

REVIEW

Semantic verdict: request changes

Thanks for closing the permission path — actions: read is present and the negative control fails on the prior head.

The rollout bridge still only consults the legacy queue for visual-gate/baseline. I drove the production old lock at run 899 and a current visual-gate/reports writer at run 900 against one bare remote; both acquired different holders simultaneously. The new baseline-only regression passes, but the scope guard leaves every other shared publisher outside the bridge, so old and new publication ownership still overlap during rollout.

Could we make every current shared publisher observe the legacy holder while old runs can exist, then cover that cross-scope production case?

[Reviewed by Robohands]

INLINE (0–3, only when genuinely line-anchored)

None — the finding crosses the global shared holder and the legacy baseline holder.

EVIDENCE I DID NOT SPEND

• Focused Vitest is 2 files / 25 tests green at the exact head; actionlint, both Node syntax checks, and pnpm check:repo are green.
• The added baseline bridge regression and workflow permission regression both fail under 311ba8cd, confirming they detect the two prior cases rather than merely passing.
• Security preflight found no dependency, lockfile, package-hook, executable-mode, shell-evaluation, or user-controlled queue-path expansion in the delta.

TIME

TIME total 10m

• setup/rules: 3m — fresh loop 1.9.1, audit 1.14.1, parent Round 2, prior #5631 review, full untrusted diff, and exact-head setup
• worktree/install: 1m — dedicated detached worktree and lockfile-matched install
• build/server: 0m — not applicable
• browser/a11y: 0m — non-rendering change
• focused tests: 1m — 25/25 Vitest, actionlint, syntax, and check:repo
• code/history: 3m — old→new delta, permissions, bridge state machine, current-main check, two negative controls, and actual old-lock/current-publisher probes
• critique/wiki: 2m — presentation, critic pass, delivery, and record/index update
• CI wait: 0m — delivery triggered one Review signal run; both jobs skipped, so no substantive repository CI ran
• waste: 1m — large rule/rubric reads exceeded one output window and were repeated in smaller chunks

WHAT I COULD NOT VERIFY

• A live GitHub Actions overlap was not dispatched because it would mutate shared gh-pages; the same production CLIs and schemas were driven against an isolated bare remote instead.
• Full repository CI has not run for this stacked draft head.

Current CI and landing gate

At exact head d64f87286431e6003d6b5da1df379706f09844e9, GitHub reports Meta CLA and Vercel Preview Comments green plus the Vercel status green. Delivery triggered one Review signal run, but both jobs skipped; no substantive repository CI/test workflow has run. That absent inherited CI is a separate landing gate: after #5629 lands and this PR targets main, substantive exact-head GitHub CI must run before landing; it is not being used as a code blocker or as evidence of correctness here.

Focused evidence

• Vitest: 2 files, 25/25 tests passed in 38.31s.
• Negative controls: the bridge regression and permission regression both fail against prior head 311ba8cd.
• Production rollout probe: old baseline run 899 and current report run 900 acquired legacy/shared holders simultaneously; current baseline run 901 correctly refused behind old run 899.
• actionlint: all six relevant workflows passed with the repository’s documented ignores.
• node --check: both publisher entrypoints passed.
• pnpm check:repo: sync, boundaries, changesets, demo media, executable bits, CLI structure, use-client, portable scripts, i18n catalog, and CLDR weekdays all passed.
• Security radius: permission growth is read-only Actions access; queue paths are constants; repository/run/scope identity is validated; subprocesses use argument vectors; tests use isolated bare remotes and a fake gh.

What changed before posting

Posted as drafted after one critic pass; the public review is 113 words.

Delivery

GitHub review: https://github.com/facebook/astryx/pull/5631#pullrequestreview-5048893503

Successor stack safety

#5633 is not code-review-safe to start while this parent remains semantically blocked; it was not inspected or touched in this run.

Round 3 — 7c03d407

PR

#5631 ci: route visual gh-pages writers through publisher by cixzhang (bucket: the maintainer)

HEAD REVIEWED

7c03d407e6c6e6c3044a4209f76134d7cea8ec84 — every claim below was verified at this exact head, whose sole parent is approved/green #5629 head 88195bb8e6765f7fd10fd5abf7521e09d157bc42.

VERSIONS

LOOP VERSION: 1.9.1 AUDIT RUBRIC: 1.14.1

LANE

LANE: full WHY: this is a third-round re-review of a privileged two-queue rollout bridge; the prior architecture blocker and cross-workflow ordering require the full concurrency and security radius.

PROBLEM

WHY 1: Independent workflow generations can mutate gh-pages while consulting different publication authorities. WHY 2: A lost writer or wait cycle leaves maintainers without the evidence, report, acceptance, or baseline state the workflow was meant to publish. WHY 3: Visual and release decisions are trustworthy only when every current and legacy writer can make progress under one publication order.

USER-FACING PROBLEM: A maintainer running overlapping publication workflows across the rollout boundary can lose a publication or wait until timeout even though each workflow joined a queue. PROBLEM SEVERITY: broken task — one or both queued publications can fail to complete. NEW FEATURE CASE: not a product feature — this extends the repository-owned publisher approved in #5629 across the visual writer class. EARLY STOP: clear — the owner-authored migration and approved parent establish the need; this round judges the prior concurrency acceptance criteria.

VERDICT: clear

SOLUTION

Every current publisher now creates a compatibility ticket in the former baseline queue as well as a scoped ticket in the shared queue. It waits for the legacy authority first, then claims the shared authority before publishing. This closes the named old-baseline/current-report split when tickets are already present, but the sequential claims can invert if an older current run arrives between them.

SOLUTION (1 delta decision · 18 added / 22 removed runtime lines)

1. Remove the baseline-scope guard so every current publisher enqueues, waits on, and releases the legacy compatibility authority.

BURDEN: high overall, low delta — the delta removes scope guards, but every publication still spans two mutable ticket sets, two holders, two wait loops, and two release paths. BURDEN MATCH: disproportionate — the temporary projection now covers the intended scopes, but its non-atomic acquisition can prevent progress under a reachable late-arrival ordering.

The Round 1 actions: read finding remains fixed. Round 2 asked every current scope to observe the legacy holder and to cover old-baseline/current-report overlap; both named cases are now covered. The remaining block is the same global-owner contract under a different interleaving, not an unrelated new nit.

VERDICT: BLOCKS — the rollout bridge can form a two-queue wait cycle.

ARCHITECTURE

OWNER: the shared gh-pages publisher should own current publication order; the former baseline queue is a temporary compatibility gate for old runs. TIER 1: the publisher approved in #5629, with GitHub Actions run state as liveness authority. TIER 2: none. SEAMS: old baseline → current writer, current writer → old baseline, every current scope, late lower-run ticket arrival, stale-run cleanup, partial enqueue/release, and rejected-push retry. BEHAVIOR UNIT: pure utility/module — the production current publisher and production old baseline CLI were driven against isolated bare remotes. COMPLEXITY BUDGET: one current holder authority plus one one-way temporary legacy compatibility gate, with no acquisition inversion or second independent ordering decision. ACTUAL BURDEN: 7 PR files; 1,353 additions / 205 deletions over the approved parent; 2 writable queue roots and holders; 2 wait/claim state machines; 4 migrated visual workflows plus the parent’s site/report writers; 25 focused tests. BURDEN TREND: first reviewed 311ba8cd → d64f8728 grew a second queue state machine; d64f8728 → 7c03d407 shrank 4 runtime lines by removing scope guards, but retained both writable authorities and exposed a late-arrival inversion. RESET TRIGGER: triggered — the second model-level ordering defect persists after the prior dual-authority remedy; the model still needs one monotonic acquisition order rather than another scope guard.

domain fact	one authoritative writable source	generated / immutable projections	other writable copies
current publication order	shared queue tickets and holder	run id/scope in logs	legacy tickets and holder are a temporary writable projection; acquisition order can diverge — BLOCKS
rollout compatibility	current publisher projects each run into the legacy queue	same run id in both ticket sets	old runs may still write only the legacy queue
workflow liveness	GitHub Actions run state	immutable run id in both tickets	none; all current callers retain the required Actions permission

seam	driven result
old baseline run 899 → current report run 900	production old lock holds legacy; current report times out before claiming shared — pass
no legacy blocker → current report run 900	report owns shared {runId:900, scope:visual-gate/reports} and legacy {runId:900} holders — pass
every current scope	whole-tree, reports, evidence, acceptances, and baseline each acquire matching shared/legacy holders — pass
prior head negative control	d64f8728 reproduces legacy holder 899 plus shared report holder 900 simultaneously — pass
late older current run	after run 900 owns legacy, run 899 enqueues; 900 waits on shared ticket 899 while 899 waits on legacy holder 900 — BLOCKS
token/argument boundary	bridge paths are constants; repository/run/scope identities are validated; subprocesses use argument vectors; permission growth is read-only Actions access — pass

VERDICT: BLOCKS — two mutable authorities are acquired in a sequence that can invert; current publication needs a monotonic owner before consulting the legacy compatibility gate.

IMPACT

The requested old-blocker and dual-holder cases are fixed. But if an older current workflow reaches publication during the gap between a newer run’s legacy and shared claims, both runs wait on the other authority; neither publishes, and the normal withPublicationTurn() release finally is never reached because the wait fails first.

VERDICT: BLOCKS — a reachable cross-workflow ordering loses progress instead of serializing it.

API

No installed-consumer API changes. The repository-internal queue lifecycle now projects every scope into the legacy ticket/holder protocol.

OSSIFICATION: the compatibility protocol is temporary repository automation, but while active it must have a single acquisition invariant that every current caller shares.

VERDICT: note — internal compatibility reach is broader; the correctness block belongs to ARCHITECTURE.

THEMING

No CSS, token, target, rendered element, or style contract changes.

VERDICT: clear

BREAKING

BEHAVIOR: the named old/current holder split is fixed, but late lower-run arrival can deadlock two current publishers. API: no consumer API change. VISUAL: no rendered UI change. THEME: no theme change.

VERDICT: BLOCKS — the migration’s serialization contract still has a reachable non-progress state.

PERFORMANCE & RESOURCES

EFFECTS: zero. RENDER: no application render path changes. LISTENERS/OBSERVERS: none. LAYOUT: none. BUNDLE: no dependency or package bundle change. CI cost: every current publication now mutates and polls both queue roots; loops and retries remain bounded. No application-runtime degradation is claimed.

VERDICT: clear

VISUAL EVIDENCE

VISUAL CHECK: not applicable WHY: the exact delta changes only Node queue coordination and its tests; the full PR changes workflow YAML, Node publication code, and tests, with no UI, CSS, markup, story, or rendered component behavior.

VERDICT: clear

REMEDY SEARCH

REMEDY SEARCH: not triggered — no proven visual defect

A11Y & I18N

No rendered element, interaction, role, accessible string, locale formatting, or direction-sensitive behavior changes.

VERDICT: clear

JUDGEMENT

slot	verdict
PROBLEM	clear
SOLUTION	BLOCKS — sequential dual-authority acquisition can invert
ARCHITECTURE	BLOCKS — late older ticket creates a wait cycle
IMPACT	BLOCKS — both publications can time out
API	note — temporary internal compatibility reach broadened
THEMING	clear
BREAKING	BLOCKS — serialization can lose progress
PERFORMANCE	clear
VISUAL	clear — not applicable
A11Y & I18N	clear — not applicable

GOAL: partly met — exact-head production probes prove the requested old-blocker and same-run dual-holder cases for all five scopes, and the prior head reproduces its dual-holder bug. A separate exact-head production probe then proves a late lower-run ticket creates the two-sided wait cycle. DISPOSITION: missing actions: read → resolved; old-baseline/current-report dual holder → resolved; current/current late-arrival deadlock → blocks now; absent substantive exact-head GitHub Actions CI → post-parent-landing landing gate, not the code finding. ADVICE: bounded outcome criteria — current publishers must acquire the two authorities in an order that cannot invert when a lower run id arrives between claims, with this exact interleaving locked by a regression. AUTHOR CAN PROCEED: yes — preserve the two passing bridge cases, eliminate the wait cycle, and add the late-arrival test. WORST OUTCOME: “Both queued publications wait on the other authority until timeout, so neither artifact is published.” → request changes.

JUDGEMENT NEEDED: none — this is a reproduced concurrency defect against the approved global publication-owner contract.

request changes (self-authored fallback COMMENT)

1. The current publisher waits for the legacy holder before claiming the shared holder. → if an older current run enqueues between those claims, each run waits on the other queue and neither publishes · .github/scripts/lib/gh-pages-publisher.mjs:801

REVIEW

Semantic verdict: request changes

Thanks — actions: read remains, an old baseline ticket blocks a new report writer, and an unblocked report owns both holders.

One ordering hole remains. If newer run 900 acquires the legacy holder after seeing no older shared ticket, then older run 899 enqueues before 900 claims the shared holder, the two-step acquisition deadlocks: 900 waits on 899’s shared ticket while 899 waits on 900’s legacy holder. I reproduced both timeouts with the production CLIs against one bare remote.

Could we make current publishers acquire the two authorities in one monotonic order, and add this late-arrival regression?

[Reviewed by Robohands]

INLINE (0–3, only when genuinely line-anchored)

None — the finding is the cross-queue acquisition sequence itself.

EVIDENCE I DID NOT SPEND

• The exact d64f8728 → 7c03d407 delta is 18 additions / 22 removals in the publisher plus 5/5 test-line edits; all scope guards around the legacy bridge were removed.
• Focused Vitest, actionlint, three syntax checks, Prettier, git diff --check, and pnpm check:repo are green.
• No dependency, lockfile, package hook, executable mode, shell-evaluation, user-controlled queue path, or credential boundary changed; all changed CI code was read before execution.

TIME

TIME total 11m

• setup/rules: 4m — fresh loop 1.9.1, audit 1.14.1, public kit/loop/rubric, #5629 Round 2, both prior #5631 rounds, exact head, and untrusted-code preflight
• worktree/install: 1m — dedicated detached checkout; lockfile-matched install completed in 63.5s
• build/server: 0m — not applicable
• browser/a11y: 0m — non-rendering change
• focused tests: 1m — 25/25 Vitest in 49.7s, actionlint/syntax in 0.7s, check:repo in 5.0s, Prettier in 1.3s (run concurrently where independent)
• code/history: 3m — exact delta, permission/scope census, required cases, prior-head negative control, all-scope bridge probe, late-arrival interleaving, and current-main invalidation check
• critique/wiki/delivery: 2m — presentation, critic pass, exact-head recheck, fallback COMMENT, record/index update, and readback
• CI wait: 0m — no substantive Actions run exists at this stacked head
• waste: 1m — one probe invocation used relative paths and was rerun with absolute paths

WHAT I COULD NOT VERIFY

• A live GitHub Actions overlap was not dispatched because it would mutate shared gh-pages; the production CLIs and schemas were driven against isolated bare remotes instead.
• Full repository CI has not run for this stacked exact head.

Current CI and landing gate

At exact head 7c03d407e6c6e6c3044a4209f76134d7cea8ec84, GitHub reports only Meta CLA and Vercel checks/status green; there are zero Actions workflow runs for this head. That absence is not the code verdict. After #5629 lands and this PR targets main, substantive exact-head GitHub CI must run and pass before landing.

Focused evidence

• Required blocker case: production old run 899 owns the legacy holder; exact-head report run 900 creates its legacy ticket but no shared holder and times out behind 899.
• Required no-blocker case: report run 900 owns both the scoped shared holder and legacy holder; the same result holds for whole-tree, evidence, acceptance, and baseline scopes.
• Negative control: d64f8728 reproduces legacy holder 899 plus shared report holder 900 simultaneously.
• Additional concurrency radius: exact-head run 900 owns legacy, then lower run 899 arrives; 900 times out behind shared ticket 899 and 899 times out behind legacy holder 900. Static confirmation is the legacy-wait-before-shared-claim sequence at lines 801–819; withPublicationTurn() does not enter its release finally until that wait returns at lines 1512–1523.
• Permissions/security: actions: read remains on every queued publisher job; queue paths are constants; repository/run/scope identities are validated; subprocesses use argument vectors; tests use isolated bare remotes and a fake gh.
• Checks: 2 files / 25 tests passed; actionlint passed all six relevant workflows; three Node syntax checks passed; Prettier, git diff --check, and pnpm check:repo passed.

What changed before posting

Posted as drafted after one critic pass; the public review is under the 150-word request-changes cap.

Delivery

GitHub review: https://github.com/facebook/astryx/pull/5631#pullrequestreview-5049088174

Successor stack safety

#5633 is not code-review-safe to start while this parent remains semantically blocked; it was not inspected or touched in this run.

Round 4 — 89c9f042

PR

#5631 ci: route visual gh-pages writers through publisher by cixzhang (bucket: the maintainer)

HEAD REVIEWED

89c9f042ddc4d481271fb1be9f7ad21cb7743c54 — every claim below was verified at this exact head, whose sole parent is approved/green #5629 head 88195bb8e6765f7fd10fd5abf7521e09d157bc42.

VERSIONS

LOOP VERSION: 1.9.1 AUDIT RUBRIC: 1.14.1

LANE

LANE: full WHY: this is a fourth-round re-review of a privileged two-queue rollout bridge; the prior deadlock, every publisher scope, terminal cleanup, permissions, and test specificity require the full concurrency and security radius.

PROBLEM

WHY 1: Independent workflow generations can mutate gh-pages while consulting two publication authorities during rollout. WHY 2: A wait cycle prevents both queued runs from publishing evidence, reports, acceptance, or baseline state. WHY 3: Visual and release decisions are trustworthy only when every legacy/current and current/current overlap makes progress under one publication order.

USER-FACING PROBLEM: A maintainer running overlapping current publication workflows can have both jobs wait until timeout even though each joined both queues. PROBLEM SEVERITY: broken task — neither queued publication completes. NEW FEATURE CASE: not a product feature — this extends the repository-owned publisher approved in #5629 across the visual writer class. EARLY STOP: clear — the owner-authored migration and approved parent establish the need; this round judges whether the prior concurrency acceptance criteria are met.

VERDICT: clear

SOLUTION

Current publishers now claim the shared authority before waiting for the legacy bridge, and release the legacy authority before the shared one. This lets a late legacy-only run finish without waiting on the current queue. Current runs still enqueue tickets into both FIFO orderings before either acquisition, so a late lower-id current run can be first in the legacy queue while a newer run already holds the shared authority.

SOLUTION (1 delta decision · 9 added / 9 removed runtime lines; 123 added / 9 removed test lines)

1. Reverse current publisher acquisition to shared-then-legacy and release to legacy-then-shared; add late legacy, simultaneous bridge, and completed-blocker tests.

BURDEN: high overall, flat in the delta — every publication still spans two mutable ticket sets, two holders, two wait loops, and two release paths. BURDEN MATCH: disproportionate — swapping holder order fixes legacy-only overlap but preserves two independently ordered current-run ticket queues and the same wait-cycle class.

The Round 1 permission finding remains fixed. Round 2’s every-scope bridge is preserved. Round 3 asked for a monotonic order covering the late lower-run interleaving; the new test covers a late legacy-only run, not a late current publisher, so that ask remains open.

VERDICT: BLOCKS — current publishers can still form a two-queue wait cycle.

ARCHITECTURE

OWNER: the shared gh-pages publisher should own current publication order; the former baseline queue should only exclude legacy-only runs during rollout. TIER 1: the publisher approved in #5629, with GitHub Actions run state as liveness authority. TIER 2: none. SEAMS: legacy-only baseline → current writer, current writer → current writer, every current scope, late lower-run arrival, terminal cleanup, partial enqueue/release, and rejected-push retry. BEHAVIOR UNIT: pure utility/module — both production queue CLIs were driven against isolated bare remotes. COMPLEXITY BUDGET: one authoritative current FIFO plus one temporary legacy exclusion boundary that cannot independently reorder current publishers. ACTUAL BURDEN: 7 changed files; 1,472 additions / 210 deletions over the approved parent; 2 writable FIFO ticket roots and holders; 2 wait/claim state machines; 4 migrated visual workflows plus parent site/report writers; 27 focused tests. BURDEN TREND: first reviewed 311ba8cd → d64f8728 added the legacy state machine; d64f8728 → 7c03d407 broadened it; 7c03d407 → 89c9f042 moved holder acquisition/release order but left both writable FIFO orderings intact. RESET TRIGGER: triggered — this is the third model-level ordering failure after local remedies; the model still needs one authority to order current publishers rather than another interleaving guard.

domain fact	one authoritative writable source	generated / immutable projections	other writable copies
current publication order	shared queue tickets and holder	run id/scope in logs	legacy tickets independently order every current run — BLOCKS
rollout exclusion	legacy holder excludes old baseline publishers	same current run id may bridge both holders after shared acquisition	legacy FIFO can prioritize a current run that does not own shared — BLOCKS
workflow liveness	GitHub Actions run state	immutable run id in both tickets	none; every current caller retains Actions read permission

seam	driven result
active legacy-only run 899 → current run 900	all five scopes hold shared 900 while legacy 899 publishes; current 900 does not return acquired; after old release, both holders become 900 and reverse release clears all four state files — pass
late legacy-only run 899 after current shared 900	old production lock acquires/releases legacy; current 900 then acquires legacy and proceeds — pass
late current run 899 after current shared 900	exact-head current 900 times out behind legacy ticket 899 while exact-head current 899 times out behind shared holder 900; all four tickets remain and no legacy holder exists — BLOCKS
terminal legacy blocker	focused exact-head test prunes completed run 899 and then acquires both holders for 900 — pass
permissions	every queued publisher job has actions: read and the required contents permission; promotion retains actions: write — pass
test specificity	the new late-arrival test invokes the legacy-only CLI for 899; no test drives two current publishers through the late-arrival seam — BLOCKS

VERDICT: BLOCKS — holder acquisition is ordered, but pre-acquisition tickets still let the two FIFO authorities disagree; one current ordering authority must govern the rollout bridge.

IMPACT

The requested legacy-only blocker, same-run bridge, terminal cleanup, permissions, and reverse release cases pass. But if a lower-id current workflow enqueues after a newer current workflow owns shared and before it owns legacy, the newer waits on the lower legacy ticket while the lower waits on the newer shared holder; both remain active until the 75-minute timeout. NEW FEATURE IMPACT: not a new feature — the migration should serialize repository publication without losing progress.

VERDICT: BLOCKS — a reachable current/current overlap still prevents both publications.

API

No installed-consumer API changes. The repository-internal queue lifecycle keeps the shared and legacy ticket protocols.

	change	public?	class	doc’d?	verdict
~	shared holder is acquired before the legacy holder	no — repository workflow contract	rollout ordering	focused tests	finding
~	legacy holder is released before the shared holder	no — repository workflow contract	reverse release	focused tests	clear

OSSIFICATION: repository automation depends on both ticket protocols during rollout; the current queue must remain the only authority that orders current publishers.

VERDICT: note — internal ordering changed; the correctness block belongs to ARCHITECTURE.

THEMING

No CSS, token, target, rendered element, or style contract changes.

VERDICT: clear

BREAKING

BEHAVIOR: legacy/current overlap now progresses, but current/current late lower-run overlap can still deadlock. API: no consumer API change. VISUAL: no rendered UI change. THEME: no theme change.

VERDICT: BLOCKS — the migration’s serialization contract still has a reachable non-progress state.

PERFORMANCE & RESOURCES

EFFECTS: zero. RENDER: no application render path changes. LISTENERS/OBSERVERS: none. LAYOUT: none. BUNDLE: no dependency or package change. CI cost: every current publication mutates and polls both queue roots; wait and retry loops remain bounded. No application-runtime degradation is claimed.

VERDICT: clear

VISUAL EVIDENCE

VISUAL CHECK: not applicable WHY: the exact delta changes only Node queue coordination and tests; the full PR changes workflow YAML, Node publication code, and tests, with no UI, CSS, markup, story, or rendered component behavior.

VERDICT: clear

REMEDY SEARCH

REMEDY SEARCH: not triggered — no proven visual defect

A11Y & I18N

No rendered element, interaction, role, accessible string, locale formatting, or direction-sensitive behavior changes.

VERDICT: clear

JUDGEMENT

slot	verdict
PROBLEM	clear
SOLUTION	BLOCKS — two current FIFO orderings can still disagree
ARCHITECTURE	BLOCKS — late current ticket creates a wait cycle
IMPACT	BLOCKS — both publications time out
API	note — internal ordering changed
THEMING	clear
BREAKING	BLOCKS — serialization can lose progress
PERFORMANCE	clear
VISUAL	clear — not applicable
A11Y & I18N	clear — not applicable

GOAL: partly met — exact-head production probes prove legacy/current serialization, matching dual-holder acquisition, reverse release, terminal cleanup, and permissions; a separate exact-head production probe proves the current/current late-arrival cycle remains. DISPOSITION: Round 1 permission finding → resolved; Round 2 every-scope legacy bridge → resolved; Round 3 current/current late-arrival cycle → still blocks; absent substantive exact-head GitHub Actions CI → post-parent-landing landing gate, separate from the code verdict. ADVICE: bounded outcome criteria — one authority must determine order for current publishers across both queue representations, and a late lower-id current run after a newer shared claim must complete without either active run waiting on the other. AUTHOR CAN PROCEED: yes — preserve legacy-only rollout exclusion and reverse release, eliminate the current/current cycle, and add this exact regression. WORST OUTCOME: “Both current publication jobs wait on the other queue until timeout, so neither artifact is published.” → request changes.

JUDGEMENT NEEDED: none — this is a reproduced concurrency defect against the approved global publication-owner contract.

request changes (self-authored fallback COMMENT)

1. Current publishers reserve positions in two independently ordered FIFOs before taking the shared-then-legacy holders. → a late lower-id current run can make both active workflows wait on each other until timeout · .github/scripts/lib/gh-pages-publisher.mjs:752

REVIEW

Semantic verdict: request changes

Thanks — shared-first/legacy-second lets a late legacy-only run finish, and reverse release is clean. The current/current case still deadlocks: at waitForPublicationTurn(), after run 900 owns shared, a late lower run 899 adds tickets to both queues; 900 waits on 899’s legacy ticket while 899 waits on 900’s shared holder. I reproduced both exact-head timeouts with the production queue code. The added regression drives the old lock for 899, so it misses this class.

Could we make one authority determine order across both current/current and legacy/current rollout, and cover this current/current interleaving?

[Reviewed by Robohands]

INLINE (0–3, only when genuinely line-anchored)

None — the finding is the cross-queue ticket-order model, not one local statement.

EVIDENCE I DID NOT SPEND

• Focused exact-head Vitest is 2 files / 27 tests green; the missing current/current interleaving is outside those 27 cases.
• Exact-head actionlint, four Node syntax checks, Prettier, git diff --check, and pnpm check:repo pass.
• Security preflight found no dependency, lockfile, package-hook, executable-mode, shell-evaluation, user-controlled queue path, credential expansion, or public-repo private residue in the delta.

TIME

TIME total 17m

• setup/rules: 4m — fresh loop 1.9.1, fresh official audit 1.14.1, all three prior rounds, exact-head setup, ownership, PR intent, and untrusted-code preflight
• worktree/install: 6m — dedicated detached worktree; APFS dependency clone was unexpectedly slow, then pnpm reconciled the lockfile-matched snapshot
• build/server: 0m — not applicable
• browser/a11y: 0m — non-rendering change
• focused tests: 2m — 27/27 Vitest plus actionlint, syntax, Prettier, diff-check, and check:repo
• code/history: 3m — exact delta, all-scope legacy/current probe, current/current late-arrival probe, terminal handling, permissions, security, and current-main invalidation check
• critique/wiki/delivery: 2m — presentation, critic pass, exact-head recheck, fallback COMMENT, record/index update, and readback
• CI wait: 0m — no substantive Actions run exists at this stacked head
• waste: 6m — the dependency snapshot copy took six minutes before the focused checks could start

WHAT I COULD NOT VERIFY

• A live GitHub Actions overlap was not dispatched because that would mutate shared gh-pages; both production queue CLIs and schemas were driven against isolated bare remotes instead.
• Full repository CI has not run for this stacked exact head.

Current CI and landing gate

At exact head 89c9f042ddc4d481271fb1be9f7ad21cb7743c54, Meta CLA, Vercel Preview Comments, and the Vercel status are green. Delivery triggered Review signal run 33155046851, whose two jobs were skipped; no substantive repository CI/test workflow has run. That absence is not the code verdict. After #5629 lands and this PR targets main, substantive exact-head GitHub CI must run and pass before landing.

Focused evidence

• Current/current late arrival: exact-head run 900 owns shared, then exact-head lower run 899 enqueues; 900 times out behind legacy 899 and 899 times out behind shared 900, with all four tickets left and no legacy holder.
• Legacy/current all-scope matrix: whole-tree, reports, evidence, acceptances, and baseline each blocked current 900 behind active old 899 without returning acquired; after old release, both holders matched 900 and reverse release cleared both tickets and holders.
• Terminal handling: the exact-head completed-blocker regression prunes legacy 899 and acquires both holders for 900.
• Permissions: actions: read remains on every queued publisher job; each retains the required contents permission, and promotion retains Actions write.
• Checks: 2 files / 27 tests passed in 106.50s; actionlint 1.7.12 passed all six relevant workflows with the documented custom-runner ignore; four Node syntax checks, Prettier, git diff --check, and pnpm check:repo passed.

What changed before posting

Posted as drafted after one critic pass.

Delivery

GitHub review: https://github.com/facebook/astryx/pull/5631#pullrequestreview-5049260806

Successor stack safety

#5633 is not code-review-safe to start while this parent remains semantically blocked; it was not inspected or touched in this run.

Round 5 — d8738a1b

PR

#5631 ci: route visual gh-pages writers through publisher by cixzhang (bucket: the maintainer)

HEAD REVIEWED

d8738a1bcaa99e27e56e9c465ba74c46083046cf — every claim below was verified at this exact head, whose sole parent is approved/green #5629 head 88195bb8e6765f7fd10fd5abf7521e09d157bc42.

VERSIONS

LOOP VERSION: 1.9.1 AUDIT RUBRIC: 1.14.1

LANE

LANE: full WHY: this is a fifth-round re-review of a privileged two-queue rollout bridge with a standing current/current deadlock finding; atomicity, every publisher scope, both arrival orders, simultaneous claims, terminal cleanup, push retry, permissions, and regression specificity require the full concurrency and security radius.

PROBLEM

WHY 1: During rollout, old and current workflows can mutate gh-pages while using two queue representations. WHY 2: If current runs can reserve or hold those representations independently, two jobs can wait on one another or both believe they own publication. WHY 3: Visual and release decisions are trustworthy only when every legacy/current and current/current overlap has one owner and makes progress.

USER-FACING PROBLEM: A maintainer running overlapping publication workflows can otherwise wait until timeout for evidence, reports, acceptance, or baseline state that never publishes. PROBLEM SEVERITY: broken task — both queued publications can fail to complete. NEW FEATURE CASE: not a product feature — this completes the repository-owned publication migration approved in #5629. EARLY STOP: clear — the approved parent and four prior rounds establish the need; this round judges the exact prior concurrency acceptance criteria.

VERDICT: clear

SOLUTION

Every current publisher creates both queue tickets in one branch update. It reads both queues from one fresh checkout and writes both holders in one commit only if no earlier ticket or holder exists in either representation; release removes both tickets and both holders in the same way. Current publishers therefore have one atomic ownership decision, while the legacy representation remains a same-source rollout projection that can still exclude old baseline-only runs.

SOLUTION (1 delta decision · 282 added / 84 removed publisher lines; 10 added / 14 removed test lines)

1. Replace sequential queue lifecycle and holder acquisition with same-commit dual-queue enqueue, claim, and release, rechecking both orderings immediately before the claim.

BURDEN: high overall, medium in this delta — two temporary queue projections remain, but current enqueue/claim/release now have one mutation owner, one remote snapshot, one ref update, bounded push retries, and one focused module boundary. BURDEN MATCH: proportionate in runtime code — rollout compatibility requires observing the old queue, and atomic projection removes the independently writable current-run ordering that caused the prior deadlocks. The evidence burden is not met because the committed suite does not enforce the new atomic invariant.

The Round 4 review said a late lower current run made run 900 wait on its legacy ticket while run 899 waited on run 900’s shared holder, and asked for one current ordering authority plus that exact current/current regression. The implementation now satisfies the ordering ask. The checked-in tests still exercise a legacy-only run 899, not a second current publisher, so the regression ask remains open.

VERDICT: BLOCKS — the runtime model is corrected, but the required regression contract does not fail when atomic dual-holder claim is replaced by sequential claims.

ARCHITECTURE

OWNER: the shared gh-pages publisher owns every current publication turn; the former baseline queue is a temporary same-commit projection for old-run exclusion. TIER 1: the global run-id FIFO and GitHub Actions run status accepted in #5629. TIER 2: none. SEAMS: legacy-only/current overlap, current/current overlap in both arrival orders, all five current scopes, simultaneous enqueue/claim, terminal holder cleanup, rejected-push retry, and release after publication failure. BEHAVIOR UNIT: pure utility/module — exported queue operations were driven against isolated bare remotes with a fake run-status endpoint and forced non-fast-forward injection. COMPLEXITY BUDGET: one current ordering authority; one ref update per enqueue/claim/release; one temporary generated legacy projection; no partial current holder; focused tests for every ordering transition. ACTUAL BURDEN: 7 changed files; 1,716 additions / 260 deletions over the approved parent; 1 shared publisher; 2 durable queue roots updated together for current runs; 4 migrated visual workflows plus parent site/report writers; 12 publisher integration tests and 15 workflow-contract tests. BURDEN TREND: first reviewed 311ba8cd → d64f8728 added a second state machine; 7c03d407 and 89c9f042 changed scope and holder order without removing independent current ordering; 89c9f042 → d8738a1b grew 198 net publisher lines but contracted current ownership to one dual-queue transaction. RESET TRIGGER: runtime reset satisfied — the current model has one atomic claim instead of another interleaving guard; verification remains incomplete because the test model still permits sequential claims.

domain fact	one authoritative writable source	generated / immutable projections	other writable copies
current run order	shared run-id ticket order	matching legacy ticket written/removed in the same commit	none in current code
current publication ownership	one dual-queue claim mutation	matching holder JSON in both roots from the same commit	none in current code
rollout exclusion	active legacy-only ticket/holder	current run’s legacy projection	old workflow writes only the legacy root by design
workflow liveness	GitHub Actions run status	immutable run id in both tickets	none; every caller has the required Actions permission

seam	driven result
active legacy-only run → current writer	all five scopes leave shared unheld, then acquire matching holders after old release
lower current arrives after higher claim	lower waits on the single dual holder; after release it acquires both and completes
lower current arrives before higher claim	higher cannot claim; lower acquires/releases first, then higher completes
simultaneous enqueue/claim	exactly one run owns both holders; the second completes after release
terminal current dual holder	successor prunes both tickets and both holders, then acquires both
forced non-fast-forward during claim	retry preserves the competing commit and creates both holders in one later commit
committed regression strength	sequential shared-then-legacy claim mutant passes all 27 focused tests — BLOCKS

VERDICT: BLOCKS — the architecture is correct in production code, but the checked-in behavior boundary does not lock the atomic invariant that resolved the prior model-level failure.

IMPACT

At this head, overlapping current publishers complete in one run-id order without dual holders, deadlock, livelock, or starvation from later higher run ids; old baseline-only runs remain excluded through the compatibility root. A future refactor can nevertheless restore sequential acquisition and its 75-minute wait cycle while every committed focused test stays green. NEW FEATURE IMPACT: not a new feature — maintainers gain reliable serialized publication only if the atomic ownership invariant remains enforced.

VERDICT: BLOCKS — current behavior is correct, but the repository does not yet prevent the exact prior deadlock class from returning unnoticed.

API

No installed-consumer API changes. The repository-internal queue lifecycle changes from separate shared/legacy operations to atomic dual-queue enqueue, claim, and release.

	change	public?	class	doc’d?	verdict
~	current ticket and holder lifecycle becomes one dual-queue transaction	no — repository workflow contract	rollout serialization	production names plus tests	clear in code; regression gap

OSSIFICATION: repository automation depends on the atomic invariant during rollout; the legacy representation remains temporary and source-identified rather than becoming a second current-run authority.

VERDICT: note — internal coordination changed; the blocking verification gap belongs to ARCHITECTURE.

THEMING

No CSS, token, target, rendered element, or style contract changes.

VERDICT: clear

BREAKING

BEHAVIOR: intended — the prior current/current deadlock is removed; both arrival orders and simultaneous claims progress in isolated production-code probes. API: no installed-consumer API change. VISUAL: no rendered UI change; all changed paths are workflow YAML, Node publication code, or tests. THEME: no theme change.

VERDICT: clear

PERFORMANCE & RESOURCES

EFFECTS: zero. RENDER: no application render path changes. LISTENERS/OBSERVERS: none. LAYOUT: none. BUNDLE: no dependency or package change. CI cost: current enqueue/claim/release each clone both sparse queue roots together; retries are bounded to ten attempts with linear backoff, and no application runtime changes.

VERDICT: clear

VISUAL EVIDENCE

VISUAL CHECK: not applicable WHY: the exact delta changes only Node queue coordination and its tests; the full PR changes workflow YAML, Node publication code, and tests, with no UI, CSS, DOM, story, or rendered component path.

VERDICT: clear

REMEDY SEARCH

REMEDY SEARCH: not triggered — no proven visual defect

A11Y & I18N

No rendered element, interaction, role, accessible string, locale formatting, or direction-sensitive behavior changes.

VERDICT: clear

JUDGEMENT

slot	verdict
PROBLEM	clear
SOLUTION	BLOCKS — required current/current regression remains absent
ARCHITECTURE	BLOCKS — sequential claim mutant passes all focused tests
IMPACT	BLOCKS — prior deadlock class can return under green tests
API	note — internal transaction contract changed
THEMING	clear
BREAKING	clear — runtime deadlock is fixed
PERFORMANCE	clear
VISUAL	clear — not applicable
A11Y & I18N	clear — not applicable

GOAL: partly met — the exact production code passes independent all-scope, both-order, simultaneous-claim, terminal-cleanup, and forced-retry probes with same-commit holders and no partial ownership. The committed 27-test suite also passes after replacing that atomic claim with separate shared-then-legacy claims, so the prior review’s regression requirement is not met. DISPOSITION: Round 4 current/current deadlock → resolved in runtime code; missing current/current/simultaneous/retry regression → blocks now; exact-head substantive GitHub Actions CI absent on the stacked draft → post-parent-landing landing gate, not the code finding. ADVICE: bounded outcome criteria — check in a production-code race that places a lower current run between a higher run’s observation and claim, plus simultaneous claim and forced dual-queue push-retry coverage; the tests must fail if holders can come from separate commits or either root can be held alone. AUTHOR CAN PROCEED: yes — the runtime design is settled; add the missing focused integration coverage without changing the queue model. WORST OUTCOME: “A future queue change can restore the 75-minute current/current deadlock while every focused test stays green.” → request changes.

JUDGEMENT NEEDED: none — this is a required regression-evidence gap for a reproduced correctness failure, not a new owner or API decision.

request changes (self-authored fallback COMMENT)

1. The late-arrival regression drives a legacy-only run instead of two current publishers, and no test races claims or dual-queue retry. → the prior current/current deadlock model can return without making the focused suite red · .github/scripts/lib/gh-pages-publisher.test.mjs:516

REVIEW

Semantic verdict: request changes

Thanks — the prior 900/899 deadlock is fixed in the implementation. I verified one same-commit dual-holder claim across all five scopes, both current-run orderings, simultaneous claims, terminal cleanup, and a forced push retry.

The checked-in regression still does not protect that fix. lets a late older legacy run finish… drives the legacy-only CLI for 899; it never starts a second current publisher. I replaced the atomic claim with separate shared-then-legacy claims and all 27 focused tests still passed, so the prior wait-cycle model can return under green tests. There is also no simultaneous-claim or dual-queue retry test.

Could we check in the current/current late-arrival race plus simultaneous/retry coverage before this lands?

[Reviewed by Robohands]

INLINE (0–3, only when genuinely line-anchored)

None — the gap is the missing cross-run test boundary, not one incorrect assertion.

EVIDENCE I DID NOT SPEND

• Exact code uses one fresh dual-queue checkout, rechecks both blocker sets inside the claim mutation, writes both holder files before one commit, and retries from a new clone after a rejected push.
• All queued publisher jobs retain actions: read; each has the required contents permission, and promotion retains actions: write.
• Security preflight found no dependency, lockfile, package-hook, executable-mode, shell-evaluation, user-controlled queue path, credential expansion, or public-repo private residue in the delta.

TIME

TIME total 25m

• setup/rules: 5m — fresh loop 1.9.1, fresh official audit 1.14.1, all four prior rounds, exact-head metadata, untrusted-code preflight, and parent relation
• worktree/install: 1m — dedicated detached checkout and lockfile-matched dependency clone/reconcile
• build/server: 0m — not applicable
• browser/a11y: 0m — non-rendering change
• focused tests: 3m — exact and mutant Vitest, actionlint, syntax, Prettier, diff-check, and check:repo
• code/history/probes: 11m — exact delta, atomicity proof, all-scope matrix, both current orderings, simultaneous claims, terminal cleanup, forced push retry, permissions, and current-main invalidation
• critique/wiki/delivery: 5m — presentation, critic pass, exact-head recheck, fallback COMMENT, record/index update, and readback
• CI wait: 0m — no substantive Actions run exists at this stacked head
• waste: 6m — the first terminal-cleanup probe used the wrong fake-CLI argument, waited on an active status, and was cancelled before the corrected clean rerun

WHAT I COULD NOT VERIFY

• A live GitHub Actions overlap was not dispatched because it would mutate shared gh-pages; the production queue module and schemas were instead driven against isolated bare remotes.
• Full repository CI has not run for this stacked exact head.

Current CI and landing gate

At exact head d8738a1bcaa99e27e56e9c465ba74c46083046cf, Meta CLA, Vercel Preview Comments, and the Vercel status are green; GitHub reports zero Actions workflow runs for the head. That absence is separate from the code verdict. After #5629 lands and this PR targets main, substantive exact-head GitHub CI must run and pass before landing.

Focused evidence

• Runtime atomicity: each current enqueue, holder claim, and release updates both queue roots in one commit; the five-scope matrix observed no partial holder.
• Current/current orderings: lower 899 arriving after higher 900’s claim waits on the same dual holder and completes after release; lower tickets arriving first prevent 900 from claiming, so 899 completes first.
• Simultaneous claims: concurrent enqueue/claim produced exactly one dual holder, then the waiting run completed after release; no dual holder, deadlock, livelock, or starvation appeared.
• Terminal/retry: a completed current dual holder was removed from both roots before the successor claimed; a forced non-fast-forward preserved the competing commit, retried, and created both holders together.
• Legacy/current and scopes: an active old holder excluded each of whole-tree, reports, evidence, acceptances, and baseline; after old release, each acquired and cleaned both roots.
• Permissions: every queued publisher job has actions: read; required contents access is unchanged, and promotion keeps actions: write.
• Checks: exact-head Vitest 2 files / 27 tests passed in 74.78s; actionlint 1.7.12 passed all six relevant workflows; five Node syntax checks, Prettier, git diff --check, and pnpm check:repo passed.
• Negative control: a sequential shared-then-legacy claim mutant passed the same 27 tests in 33.46s, proving the committed suite does not enforce the atomic claim or prior current/current interleaving.

What changed before posting

Posted as drafted after one critic pass.

Delivery

GitHub review: https://github.com/facebook/astryx/pull/5631#pullrequestreview-5049580767

Successor stack safety

#5633 is not code-review-safe to start while this parent remains semantically blocked; it was not inspected or touched in this run.

Round 6 — 14bccbdb

PR

#5631 ci: route visual gh-pages writers through publisher by cixzhang (bucket: the maintainer)

HEAD REVIEWED

14bccbdb581acb893207a8b587b5aafb42037b6d — every claim below was verified at this exact head, whose sole parent is approved/green #5629 head 88195bb8e6765f7fd10fd5abf7521e09d157bc42.

VERSIONS

LOOP VERSION: 1.9.1 AUDIT RUBRIC: 1.14.3

LANE

LANE: full WHY: this is a sixth-round re-review of a privileged two-queue rollout bridge with an explicit mutation-kill requirement for claim, release, ordering, retry, cleanup, permissions, and every publisher scope.

PROBLEM

WHY 1: During rollout, old and current workflows can mutate gh-pages while two queue representations coexist. WHY 2: If a queue lifecycle transition becomes non-atomic or stops rechecking current order, a publisher can hold only one representation or wait until timeout. WHY 3: Visual and release decisions are trustworthy only when the checked-in suite prevents those exact ownership regressions from returning under green CI.

USER-FACING PROBLEM: A maintainer running overlapping publication workflows can otherwise wait until timeout for evidence, reports, acceptance, or baseline state while the focused suite remains green. PROBLEM SEVERITY: broken task — a publication can fail to complete or temporarily expose split ownership. NEW FEATURE CASE: not a product feature — this completes the repository-owned publication migration approved in #5629. EARLY STOP: clear — the approved parent and five prior rounds establish the need; this round judges the exact mutation-kill acceptance criteria.

VERDICT: clear

SOLUTION

The runtime queue model is unchanged: current publishers create both tickets, recheck both orderings, and claim or release both holders in one branch update. This delta adds a claim-push fault-injection seam plus commit-history assertions, a simultaneous-current claim case, and an interrupted-push retry case. Those additions enforce atomic claim but do not yet enforce every required queue transition.

SOLUTION (1 delta decision · 5 production lines plus 118 added / 17 removed test lines)

1. Expose the dual-claim push boundary to tests and assert that both holder files are always created by the same commit, including simultaneous claim and rejected-push retry.

BURDEN: medium in the delta — one internal test hook, three history helpers, and two new integration cases over the existing high-burden rollout bridge. BURDEN MATCH: proportionate in production code; incomplete in verification because three required lifecycle mutations still survive the checked-in suite.

The Round 5 review said the implementation was correct but a sequential claim mutant passed all tests, and required current/current, simultaneous-claim, retry, release, late-order, and terminal-cleanup enforcement. The sequential claim mutant now fails 3 of 14 publisher tests. Sequential release, removal of the claim-time blocker recheck, and terminal legacy-holder leakage still leave all 14 green.

VERDICT: BLOCKS — the delta closes atomic-claim enforcement but not the full required regression boundary.

ARCHITECTURE

OWNER: the shared gh-pages publisher owns every current publication turn; the former baseline queue is a temporary same-commit projection for old-run exclusion. TIER 1: the global run-id FIFO and GitHub Actions run status accepted in #5629. TIER 2: none. SEAMS: active legacy-only holder across all five current scopes, both current-run arrival orders, simultaneous current claims, rejected claim push, terminal legacy holder cleanup, and atomic claim/release history. BEHAVIOR UNIT: pure utility/module — exported queue operations were driven against isolated bare remotes with a fake run-status endpoint and forced non-fast-forward injection. COMPLEXITY BUDGET: one current ordering authority; one dual-queue transaction per enqueue/claim/release; one temporary generated legacy projection; no partial holder or ticket history; one checked regression per ordering transition. ACTUAL BURDEN: 7 PR files; 1,822 additions / 260 deletions over the approved parent; exact delta 2 files and 123 additions / 17 deletions; 2 durable queue roots updated together for current runs; 4 migrated visual workflows plus parent site/report writers; 14 publisher integration tests and 15 workflow-contract tests. BURDEN TREND: first reviewed 311ba8cd → d8738a1b contracted runtime ownership to one dual-queue transaction; d8738a1b → 14bccbdb leaves that model unchanged and strengthens claim tests, but not release, claim-time reorder, or terminal-holder cleanup tests. RESET TRIGGER: runtime reset remains satisfied — no new model-level code issue. Verification is still incomplete because the checked-in boundary does not enforce three transitions the settled model requires.

domain fact	one authoritative writable source	generated / immutable projections	other writable copies
current run order	shared run-id ticket order	matching legacy ticket written/removed in the same commit	none in current code
current publication ownership	one dual-queue claim/release mutation	matching holder JSON in both roots from the same commit	none in current code
rollout exclusion	active legacy-only ticket/holder	current run’s legacy projection	old workflow writes only the legacy root by design
workflow liveness	GitHub Actions run status	immutable run id in both tickets	none; every caller has the required Actions permission

seam	driven result
active legacy holder → every current scope	whole-tree, reports, evidence, acceptances, and baseline wait without a shared holder; after old release each claims and releases both roots atomically — pass
lower current enqueued before higher claim	lower claims/releases first, then higher completes — pass
lower current arrives after higher claim	lower waits on the existing dual holder, then completes after release — pass
simultaneous current claims	exactly one run owns both holders; the other waits — pass
forced non-fast-forward during claim	competing commit is preserved; retry creates both holders in one later commit — pass
terminal legacy ticket + holder	both old files are removed before the current run acquires both holders — pass
permissions	all six queued publisher jobs retain actions: read and required contents access; promotion retains actions: write — pass
sequential claim mutant	3 of 14 publisher tests fail: late legacy, simultaneous current, and interrupted retry — pass
sequential release mutant	14 of 14 publisher tests pass — BLOCKS
claim-time blocker-recheck removal	14 of 14 publisher tests pass — BLOCKS
terminal legacy-holder leak	14 of 14 publisher tests pass — BLOCKS

VERDICT: BLOCKS — production ownership is correct, but the checked-in behavior unit does not enforce atomic release, the second late-order window, or terminal legacy-holder cleanup.

IMPACT

At this exact head, every requested production-code scenario completes with one owner and no partial holder or ticket history. The remaining gap is durable verification: a future refactor can split release, stop rechecking a lower current arrival, or strand a terminal legacy holder while all focused publisher tests remain green. NEW FEATURE IMPACT: not a new feature — maintainers gain reliable serialized publication only if the full ownership lifecycle remains enforced.

VERDICT: BLOCKS — three prior failure classes can return without CI detecting them.

API

No installed-consumer API changes. The repository-internal test seam adds optional beforeClaimPush forwarding from waitForPublicationTurn() into the dual-queue mutation.

	change	public?	class	doc’d?	verdict
+	waitForPublicationTurn({beforeClaimPush}) internal fault-injection callback	no — repository test contract	existing publisher retry hooks	integration tests	clear

OSSIFICATION: installed consumers cannot depend on this surface; repository automation ignores it, and tests use it only to inject a rejected push.

VERDICT: clear

THEMING

No CSS, token, target, rendered element, or style contract changes.

VERDICT: clear

BREAKING

BEHAVIOR: no runtime queue behavior changed; the exact code passes both current arrival orders, simultaneous claims, retry, atomic release, terminal cleanup, and all-scope legacy exclusion. API: no installed-consumer API change. VISUAL: no rendered UI change; all changed paths are Node publication code and tests. THEME: no theme change.

VERDICT: clear

PERFORMANCE & RESOURCES

EFFECTS: zero. RENDER: no application render path changes. LISTENERS/OBSERVERS: none. LAYOUT: none. BUNDLE: no dependency or package change. CI cost: two additional local-bare-remote integration tests increase the focused publisher suite; no application-runtime cost changes.

VERDICT: clear

VISUAL EVIDENCE

VISUAL CHECK: not applicable WHY: the exact delta changes only a Node queue test seam and Node tests; the full PR changes workflow YAML, Node publication code, and tests, with no UI, CSS, DOM, story, or rendered component path.

VERDICT: clear

REMEDY SEARCH

REMEDY SEARCH: not triggered — no proven visual defect

A11Y & I18N

No rendered element, interaction, role, accessible string, locale formatting, or direction-sensitive behavior changes.

VERDICT: clear

JUDGEMENT

slot	verdict
PROBLEM	clear
SOLUTION	BLOCKS — full mutation-kill contract is incomplete
ARCHITECTURE	BLOCKS — three required lifecycle mutants survive
IMPACT	BLOCKS — prior failures can return under green tests
API	clear — internal test hook only
THEMING	clear
BREAKING	clear — exact runtime code is correct
PERFORMANCE	clear
VISUAL	clear — not applicable
A11Y & I18N	clear — not applicable

GOAL: partly met — the exact 29-test suite is green and the sequential-claim mutant now fails 3 publisher tests. Independent exact-head probes pass 10/10 across all five scopes, both current arrival orders, simultaneous claims, rejected-push retry, atomic release, and terminal legacy cleanup. Three required mutants still pass 14/14 publisher tests. DISPOSITION: Round 5 sequential-claim mutant → resolved; atomic release enforcement → blocks now; claim-time late-current reorder enforcement → blocks now; terminal legacy-holder cleanup enforcement → blocks now; absent substantive exact-head GitHub Actions CI → post-parent-landing landing gate, separate from the code verdict. ADVICE: bounded outcome criteria — add checked-in cases that fail when release is split across commits, when a lower current ticket appears between observation and claim without a fresh blocker check, and when terminal legacy cleanup leaves its holder behind. AUTHOR CAN PROCEED: yes — production behavior is settled; add the three missing mutation-killing regressions without changing the queue model. WORST OUTCOME: “A future queue refactor can restore partial ownership or a publication timeout while every focused test stays green.” → request changes.

JUDGEMENT NEEDED: none — this is required regression enforcement for reproduced correctness failures, not a new owner, API, or design decision.

request changes (self-authored fallback COMMENT)

1. Checked-in tests enforce atomic claim but not the rest of the dual-queue lifecycle. → a future publisher change can strand a maintainer’s publication while focused CI stays green · .github/scripts/lib/gh-pages-publisher.test.mjs:631

REVIEW

Semantic verdict: request changes

Thanks — simultaneous/retry coverage now kills the sequential-claim mutant (3/14 failures), and exact code passes the full matrix.

The checked-in boundary still misses three required regressions: sequential release, a terminal legacy holder left behind, and a lower current run inserted between observation and claim all keep 14/14 publisher tests green. A future refactor could therefore restore partial ownership or a timeout without CI noticing.

Could we make those three mutants fail before this lands?

[Reviewed by Robohands]

INLINE (0–3, only when genuinely line-anchored)

None — the finding spans release, claim-time ordering, and terminal cleanup test boundaries.

EVIDENCE I DID NOT SPEND

• Exact production code rechecks both blocker sets inside the fresh dual-queue claim mutation, writes both holders before one commit, releases both roots in one commit, and retries from a fresh clone.
• The new beforeClaimPush hook is test-only, optional, and forwards no credentials or user-controlled command text.
• Current main has not changed any PR file since the approved parent.

TIME

TIME total 18m

• setup/rules: 4m — fresh loop 1.9.1, official audit 1.14.3, all five prior rounds, exact metadata, parent relation, and untrusted-code preflight
• worktree/install: 1m — dedicated detached checkout and lockfile-matched dependency clone/reconcile
• build/server: 0m — not applicable
• browser/a11y: 0m — non-rendering change
• focused tests: 2m — 29/29 Vitest, actionlint, five syntax checks, Prettier, diff-check, and check:repo
• code/history/probes: 8m — exact delta, four mutants, all-scope matrix, both current orders, simultaneous claim, retry, atomic release, terminal cleanup, permissions, current-main check, and exact CI census
• critique/wiki/delivery: 3m — presentation, critic pass, exact-head recheck, fallback COMMENT, record/index update, and readback
• CI wait: 0m — no substantive Actions run exists at this stacked head
• waste: 1m — long rubric output needed smaller follow-up reads after truncation

WHAT I COULD NOT VERIFY

• A live GitHub Actions overlap was not dispatched because it would mutate shared gh-pages; the production queue module and schemas were driven against isolated bare remotes instead.
• Full repository CI has not run for this stacked exact head.

Current CI and landing gate

At exact head 14bccbdb581acb893207a8b587b5aafb42037b6d, Meta CLA, Vercel Preview Comments, and the Vercel status are green; GitHub reports zero Actions workflow runs for the head. That absence is separate from the code verdict. After #5629 lands and this PR targets main, substantive exact-head GitHub CI must run and pass before landing.

Focused evidence

• Exact checks: 2 files / 29 tests passed in 58.31s; actionlint 1.7.12 passed all six relevant workflows; five Node syntax checks, Prettier, git diff --check, and pnpm check:repo passed.
• Independent runtime matrix: 10/10 uncommitted production-code probes passed across all five scopes, both current arrival orders, simultaneous claim, rejected-push retry, atomic release history, and terminal legacy ticket+holder cleanup.
• Sequential claim mutation: 3 failed / 11 passed. The late-legacy, simultaneous-current, and interrupted-retry cases each reject separate holder commits or the missing retry hook.
• Surviving mutations: sequential release 14/14 passed; terminal legacy-holder leak 14/14 passed; claim-time blocker-recheck removal 14/14 passed.
• Permissions: all six queued publisher jobs retain actions: read and required contents access; post-merge promotion retains actions: write.
• Security preflight: all changed workflow/script/test files were read before checkout; no dependency, lockfile, package hook, executable mode, shell-evaluation, user-controlled queue path, credential expansion, or public-repo private residue was introduced.

What changed before posting

Posted as drafted after one critic pass.

Delivery

GitHub review: https://github.com/facebook/astryx/pull/5631#pullrequestreview-5049849762

Successor stack safety

#5633 is not code-review-safe to start while this parent remains semantically blocked; it was not inspected or touched in this run.

Round 7 — 29dafeba

PR

#5631 ci: route visual gh-pages writers through publisher by cixzhang (bucket: the maintainer)

HEAD REVIEWED

29dafeba0e6cef644c36bd9e99725baa09259dfe — every claim below was verified at this exact head. It has the single approved/green #5629 head 88195bb8e6765f7fd10fd5abf7521e09d157bc42 as its parent.

VERSIONS

LOOP VERSION: 1.9.1 AUDIT RUBRIC: 1.14.4

LANE

LANE: full WHY: this is a seventh-round re-review of a privileged two-queue rollout bridge with four explicit mutation-kill requirements, cross-run liveness, retry, release, cleanup, permissions, and a stacked CI gate.

PROBLEM

WHY 1: During rollout, old and current workflows can mutate gh-pages while two queue representations coexist. WHY 2: If a queue transition becomes non-atomic or stops rechecking current order, a publication can expose partial ownership or wait until timeout. WHY 3: Visual and release decisions are trustworthy only when the checked-in suite rejects each known ownership regression for the reason that made it unsafe.

USER-FACING PROBLEM: A maintainer running overlapping publication workflows can otherwise wait for evidence, reports, acceptance, or baseline state that never publishes while focused CI remains green. PROBLEM SEVERITY: broken task — publication can fail to complete or temporarily expose split ownership. NEW FEATURE CASE: not a product feature — this completes the repository-owned publication migration approved in #5629. EARLY STOP: clear — the approved parent and six prior rounds establish the need; this round judges the exact mutation-kill acceptance criteria.

VERDICT: clear

SOLUTION

The runtime queue model is unchanged from the previously verified head: current publishers create both tickets, recheck both orderings, and claim or release both holders in one branch update. The exact delta adds three focused regressions for atomic release, terminal legacy-holder cleanup, and a competing owner arriving between observation and the retried claim. Together with the existing simultaneous and retry cases, the checked-in boundary now rejects all four requested mutants.

SOLUTION (1 delta decision · 0 production lines; 123 added test lines)

1. Add three focused local-bare-remote regressions that complete the mutation boundary requested in Round 6.

BURDEN: medium in the delta — two fixture writers and three integration cases over the existing high-burden rollout bridge. BURDEN MATCH: proportionate — each test targets a previously demonstrated ownership or liveness regression, without changing production code or adding another state representation.

Round 6 said atomic runtime behavior was correct but sequential release, a leaked terminal legacy holder, and removal of the fresh claim-time recheck all survived; sequential claim was already only partially killed. At this head, each of those four mutations makes its named case red or non-terminating, and the clean suite remains green.

VERDICT: clear

ARCHITECTURE

OWNER: the shared gh-pages publisher owns every current publication turn; the former baseline queue is a temporary same-commit projection for old-run exclusion. TIER 1: the global run-id FIFO and GitHub Actions run status accepted in #5629. TIER 2: none. SEAMS: active legacy-only holder across all five current scopes, both current-run arrival orders, simultaneous current claims, rejected claim push, terminal legacy cleanup, atomic claim/release history, and permissions. BEHAVIOR UNIT: pure utility/module — exported queue operations and checked-in integration tests run against isolated bare remotes with a fake run-status endpoint. COMPLEXITY BUDGET: one current ordering authority; one dual-queue transaction per enqueue/claim/release; one temporary generated legacy projection; no partial holder or ticket history; one regression per known ordering transition. ACTUAL BURDEN: 7 full-PR files and 1,945 additions / 260 deletions over the approved parent; exact delta 1 test file and 123 additions; 2 queue roots updated together for current runs; 4 migrated visual workflows plus parent site/report writers; 17 publisher integration tests and 15 workflow-contract tests. BURDEN TREND: first reviewed 311ba8cd → d8738a1b contracted runtime ownership to one dual-queue transaction; d8738a1b → 14bccbdb strengthened atomic claim coverage; 14bccbdb → 29dafeba leaves runtime burden flat and adds only the three missing regression cases. RESET TRIGGER: not triggered — the runtime reset remains satisfied and this delta completes verification rather than adding another owner, representation, or guard.

domain fact	one authoritative writable source	generated / immutable projections	other writable copies
current run order	shared run-id ticket order	matching legacy ticket written/removed in the same commit	none in current code
current publication ownership	one dual-queue claim/release mutation	matching holder JSON in both roots from the same commit	none in current code
rollout exclusion	active legacy-only ticket/holder	current run’s legacy projection	old workflow writes only the legacy root by design
workflow liveness	GitHub Actions run status	immutable run id in both tickets	none; every caller has required Actions permission

seam	driven result
all current scopes	whole-tree, reports, evidence, acceptances, and baseline each acquire and release matching holders with no partial history — pass
lower current ticket before higher claim	lower run completes first; higher then acquires both holders — pass
lower current run after higher claim	lower waits on the existing dual holder, then completes after release — pass
simultaneous current claims	exactly one run owns both holders; the other waits — pass
interrupted claim push	competing commit is preserved; retry creates both holders together — pass
terminal legacy holder without ticket	exact code removes the stale holder and then claims both for the successor — pass
workflow permissions	six queued publisher jobs retain actions: read and required contents access; promotion retains actions: write — pass
sequential claim mutant	simultaneous-current test fails because shared and legacy holders come from different commits — killed
sequential release mutant	release-history test fails with {shared:true, legacy:false} — killed
terminal legacy-holder leak mutant	checked-in cleanup test cannot complete; isolated remote retains holder 899 and both run-900 tickets, and a bounded direct cleanup probe fails terminal legacy holder survived terminal cleanup — killed for the intended liveness defect
no fresh-tip recheck mutant	competing-holder test resolves instead of timing out behind run 899 — killed

VERDICT: clear

IMPACT

At this exact head, overlapping current publishers complete in one run-id order without dual holders, deadlock, livelock, or starvation from later higher run ids; old baseline-only runs remain excluded through the compatibility root. The checked-in suite now makes every requested ownership regression non-green. NEW FEATURE IMPACT: not a new feature — maintainers retain reliable serialized publication and CI now protects the full known ownership lifecycle.

VERDICT: clear

API

No installed-consumer or repository runtime API changes in this delta. Two test helpers model a legacy holder and a competing shared holder; three tests consume them.

	change	public?	class	doc’d?	verdict
+	test-only legacy-holder and competing-holder fixtures	no — test file only	local bare-remote fixtures	test names and assertions	clear

OSSIFICATION: no runtime surface changes and no installed consumer can depend on the test helpers.

VERDICT: clear

THEMING

No CSS, token, theme target, rendered element, or style contract changes.

VERDICT: clear

BREAKING

BEHAVIOR: no runtime queue behavior changed; exact code passes all scopes, both current arrival orders, simultaneous claims, retry, atomic release, terminal cleanup, and fresh-tip ownership. API: no installed-consumer or repository runtime API change. VISUAL: no rendered UI change; the exact delta is one Node test file. THEME: no theme change.

VERDICT: clear

PERFORMANCE & RESOURCES

EFFECTS: zero. RENDER: no application render path changes. LISTENERS/OBSERVERS: none. LAYOUT: none. BUNDLE: no dependency or package change. CI cost: three local-bare-remote integration tests increase the focused suite from 29 to 32 tests; no application-runtime cost changes.

VERDICT: clear

VISUAL EVIDENCE

VISUAL CHECK: not applicable WHY: the exact delta changes only a Node integration-test file; the full PR changes workflow YAML, Node publication code, and tests, with no UI, CSS, DOM, story, or rendered component path.

VERDICT: clear

REMEDY SEARCH

REMEDY SEARCH: not triggered — no proven visual defect

A11Y & I18N

No rendered element, interaction, role, accessible string, locale formatting, or direction-sensitive behavior changes.

VERDICT: clear

JUDGEMENT

slot	verdict
PROBLEM	clear
SOLUTION	clear — all four required mutants are non-green for their intended reason
ARCHITECTURE	clear — runtime and regression boundaries preserve one current owner
IMPACT	clear
API	clear — test-only helpers
THEMING	clear
BREAKING	clear
PERFORMANCE	clear
VISUAL	clear — not applicable
A11Y & I18N	clear — not applicable

GOAL: met — clean exact-head focused tests pass 32/32; the independent runtime matrix passes 10/10; sequential claim, sequential release, terminal legacy-holder leak, and missing fresh-tip recheck each become non-green for the ownership or liveness state they reintroduce. DISPOSITION: Round 6 sequential-release enforcement → resolved; terminal legacy-holder cleanup enforcement → resolved; fresh claim-time reorder enforcement → resolved; prior sequential-claim enforcement → retained; absent substantive exact-head GitHub Actions CI → post-parent-landing landing gate, separate from the code verdict. ADVICE: omitted — no code or evidence change remains from this review. AUTHOR CAN PROCEED: yes — code review is clear; after #5629 lands, this exact head still requires substantive inherited GitHub Actions CI before landing. WORST OUTCOME: none found at the reviewed head → approve.

JUDGEMENT NEEDED: none — the requested runtime, mutation, permission, and repository evidence is complete.

approve (self-authored fallback COMMENT; no merge or auto-merge)

REVIEW

Semantic verdict: approve

Thanks — the prior review’s four mutation gaps are closed: 32 tests and 10 runtime cases pass. Landing waits for #5629 and exact-head CI.

[Reviewed by Robohands]

INLINE (0–3, only when genuinely line-anchored)

None.

EVIDENCE I DID NOT SPEND

• The true 14bccbdb → 29dafeba delta is one test file and 123 added lines; production code and workflows are byte-identical.
• Exact-head actionlint passes six relevant workflows with only the documented custom-runner-label ignore; five Node syntax checks, Prettier, diff-check, and check:repo pass.
• Security preflight found no dependency, lockfile, package hook, executable-mode, shell-evaluation, user-controlled queue path, credential expansion, or public-repo private residue in the delta.

TIME

TIME total 20m

• setup/rules: 5m — loop 1.9.1, official audit 1.14.4, all six prior rounds, exact metadata, parent relation, and untrusted-code preflight
• worktree/install: 1m — dedicated detached checkout and lockfile-matched dependency clone/reconcile
• build/server: 0m — not applicable
• browser/a11y: 0m — non-rendering change
• focused tests: 4m — clean suite, four individual mutants, bounded leak confirmation, actionlint, syntax, formatting, diff-check, and check:repo
• code/history/probes: 6m — true delta, all-scope matrix, both current orders, simultaneous claim, retry, atomic release, terminal cleanup, permissions, current-main check, and exact CI census
• critique/wiki/delivery: 4m — presentation, critic pass, exact-head recheck, fallback COMMENT, record/index update, and readback
• CI wait: 0m — no substantive Actions run exists at this stacked head
• waste: 3m — the first terminal-holder mutant run required forced cleanup before a bounded direct probe isolated the stale holder

WHAT I COULD NOT VERIFY

• A live GitHub Actions overlap was not dispatched because that would mutate shared gh-pages; production queue code and schemas were driven against isolated bare remotes instead.
• Full repository CI has not run for this stacked exact head.

Current CI and landing gate

At exact head 29dafeba0e6cef644c36bd9e99725baa09259dfe, Meta CLA, Vercel Preview Comments, and Vercel are green; GitHub reports zero Actions workflow runs for the head. That absence is separate from the code verdict. After #5629 lands and this PR targets main, substantive exact-head GitHub CI must run and pass before landing.

Focused evidence

• Clean tests: 2 files / 32 tests passed in 44.29s.
• Runtime matrix: 10/10 passed in 19.41s across all five scopes, both current arrival orders, simultaneous claim, interrupted claim push, and terminal legacy cleanup.
• Sequential claim mutant: allows only one simultaneous current writer… fails because the holder commits differ.
• Sequential release mutant: releases both holders in one commit… fails with an observed partial state {shared:true, legacy:false}.
• Terminal legacy-holder leak mutant: the checked-in cleanup case does not complete; its isolated remote retains holder 899 and both run-900 tickets with no shared holder. A bounded direct cleanup assertion fails terminal legacy holder survived terminal cleanup: expected true to be false.
• Missing fresh-tip recheck mutant: rechecks a fresh tip… fails promise resolved "undefined" instead of rejecting, after the mutant overwrites the competing run-899 owner.
• Workflow and repository checks: actionlint passed 6 workflows; 5 Node syntax checks, Prettier, git diff --check, and pnpm check:repo passed.
• Parent/CI: exact sole parent is #5629 head 88195bb8; the parent has substantive green CI, while this stacked head has no Actions run.

What changed before posting

Posted as drafted after one critic pass; the public fallback comment is 29 words.

Delivery

GitHub review: https://github.com/facebook/astryx/pull/5631#pullrequestreview-5050031583

Successor stack safety

#5633 is code-review-safe to start from this parent verdict, but it is not landing-safe until #5629 lands and #5631 receives substantive exact-head CI. It was not inspected, started, or touched in this run.

Round 8 — ba4287ed restack revalidation

PR

#5631 ci: route visual gh-pages writers through publisher by cixzhang (bucket: the maintainer)

HEAD REVIEWED

ba4287ed93a0b082c699d10943fd7dac59950f85 — every claim below was verified at this exact head. Its sole parent and PR base are 677d68ea8f64bb0f9539cb499f4537bd98d2d7ca on main.

VERSIONS

LOOP VERSION: 1.9.1 AUDIT RUBRIC: 1.14.4

LANE

LANE: full WHY: this is an exact-head restack revalidation of a privileged two-queue rollout bridge after its parent landed; patch identity, every known ordering seam, four mutation guards, permissions, and newly available substantive CI all require direct evidence.

PROBLEM

WHY 1: During rollout, old and current workflows can mutate gh-pages while two queue representations coexist. WHY 2: If a queue transition becomes non-atomic or its integrated recovery test stops executing, publication can expose partial ownership or wait until timeout without CI protecting the path. WHY 3: Visual and release decisions are trustworthy only when the restack preserves the approved runtime behavior and exact-head CI executes every recovery/fail-closed contract.

USER-FACING PROBLEM: A maintainer running overlapping publication workflows can otherwise wait for evidence, reports, acceptance, or baseline state that never publishes while the affected recovery tests fail before checking that behavior. PROBLEM SEVERITY: broken task — publication can fail to complete, and the current exact-head gate cannot validate ten promotion recovery paths. NEW FEATURE CASE: not a product feature — this preserves the repository-owned publication migration approved at the prior head after its parent landed. EARLY STOP: clear — seven prior rounds established the need and approved head 29dafeba; this round independently checks the restack and the exact-head integration that could not run while stacked.

VERDICT: clear

SOLUTION

Current publishers still create both queue tickets, recheck both orderings, and claim or release both holders in one branch update. The restack moves that byte-identical patch onto main, so the repository’s full pull-request suite now runs. That suite exposes an omitted migration: the promotion workflow invokes the shared publisher through folded YAML, while its executable workflow harness still extracts the former literal shell block and only installs the old visual-gate scripts into its sandbox.

SOLUTION (0 restack semantic decisions · 0 changed PR-owned blobs; 1 newly exposed integration gap)

1. Preserve the approved publisher patch byte-for-byte while moving only its parent/base — satisfied.
2. Keep the landed executable promotion contract running after replacing its inline script with the shared publisher — not satisfied.

BURDEN: high overall, zero new in the restack — the full PR retains one current owner, a temporary legacy projection, four migrated visual workflows, and the focused concurrency matrix. BURDEN MATCH: disproportionate at the integrated-test seam — production ownership is unchanged and verified, but the migrated promotion step leaves ten recovery/fail-closed cases unable to execute.

Round 7 approved 29dafeba after all four mutation guards became non-green and held landing for parent completion plus exact-head CI. The restack preserves that runtime evidence, but the now-running CI gate supplies new directly caused evidence that reverses the landing verdict.

VERDICT: BLOCKS — the promotion workflow migration did not carry its executable contract test.

ARCHITECTURE

OWNER: the shared gh-pages publisher owns every current publication turn; the former baseline queue is a temporary same-commit projection for old-run exclusion. TIER 1: the global run-id FIFO and GitHub Actions run status accepted in the landed parent. TIER 2: none. SEAMS: restack patch identity, all five current scopes, both late lower-current orderings, simultaneous claims, rejected claim push, atomic release, terminal legacy cleanup, fresh-tip ownership, workflow permissions, executable promotion recovery, and exact-head CI. BEHAVIOR UNIT: pure utility/module — queue operations are directly tested against isolated bare remotes; the workflow-level promotion harness is intended to execute the checked-in step against its sandbox and currently cannot. COMPLEXITY BUDGET: one current ordering authority; one dual-queue transaction per enqueue/claim/release; one temporary generated legacy projection; no partial holder history; one executable regression per known ordering or promotion transition. ACTUAL BURDEN: 7 full-PR files and 1,945 additions / 260 deletions; 1 shared publisher; 2 queue roots updated together for current runs; 4 migrated visual workflows plus the landed parent’s site/report writers; 17 publisher integration tests and 15 workflow-contract tests; 12 promotion workflow tests, 10 of which fail before their scenario. BURDEN TREND: first reviewed 311ba8cd → approved 29dafeba contracted runtime ownership and completed the queue regression boundary; 29dafeba → ba4287ed is flat in PR bytes, while the first substantive exact-head suite exposes the previously unrun workflow-harness gap. RESET TRIGGER: not triggered — the queue model adds no owner or representation; the required contraction is to reconnect the existing executable workflow test to the migrated publisher, not add another protocol.

domain fact	one authoritative writable source	generated / immutable projections	other writable copies
current run order	shared run-id ticket order	matching legacy ticket written/removed in the same commit	none in current code
current publication ownership	one dual-queue claim/release mutation	matching holder JSON in both roots from the same commit	none in current code
rollout exclusion	active legacy-only ticket/holder	current run’s legacy projection	old workflow writes only the legacy root by design
workflow liveness	GitHub Actions run status	immutable run id in both tickets	none; every queued caller has Actions read permission
promotion behavior	checked-in workflow step	executable test extraction plus isolated sandbox	test projection still models the removed inline script — BLOCKS
restack semantics	exact parent→head patch	stable patch-id plus seven identical PR-owned blobs	none

seam	driven result
restack identity	old and new parent→head patch files are byte-identical; patch-id is 6a9447a1; all seven PR-owned blobs match — pass
all current scopes	whole-tree, reports, evidence, acceptances, and baseline each acquire and release matching holders with no partial history — pass
both late current orderings	lower-before-claim and lower-after-claim each serialize and complete — pass
simultaneous current claims	exactly one run owns both holders; the other completes after release — pass
interrupted claim push	competing commit is preserved; retry creates both holders together from a fresh tip — pass
terminal legacy holder without ticket	stale holder is removed; successor claims and releases both holders — pass
workflow permissions	all six queued publisher jobs retain actions: read and required contents access; promotion retains actions: write — pass
four targeted mutants	sequential claim, sequential release, terminal-holder leak, and missing fresh-tip recheck are each non-green for the intended ownership/liveness failure — pass
executable promotion workflow	exact head fails 10/12; the helper cannot extract run: >, then lacks the publisher in its sandbox and retains assertions for the removed inline script — BLOCKS
parent negative control	substituting only the parent workflow makes the same harness pass 12/12 — confirms the PR workflow migration caused the break

VERDICT: BLOCKS — queue ownership remains correct, but its promotion workflow no longer has a functioning integrated recovery boundary.

IMPACT

The restack itself changes no PR-owned bytes, and all queue runtime scenarios remain correct. However, maintainers cannot land this exact head: the full suite is red, and ten promotion recovery/fail-closed cases no longer execute the migrated command they are meant to protect. NEW FEATURE IMPACT: not a new feature — the migration remains reliable only when the executable promotion harness follows the publisher boundary.

VERDICT: BLOCKS — exact-head CI loses the promotion recovery contract and fails.

API

No installed-consumer or repository runtime API changes in the restack. The integration mismatch is between the existing workflow-test helper and the migrated repository command.

	change	public?	class	doc’d?	verdict
~	commit parent/base only; PR patch and seven owned blobs unchanged	no API change	restack metadata	patch/tree proof	clear
~	promotion step uses shared publisher, executable harness still models inline script	no — repository test contract	workflow integration	test names/assertions	finding

OSSIFICATION: no new runtime surface or meaning is introduced by the restack; the existing test projection must follow its single workflow source.

VERDICT: note — repository test contract is stale; the block is owned by SOLUTION/ARCHITECTURE.

THEMING

No CSS, token, theme target, rendered element, or style contract changes; the restack changes no PR-owned bytes.

VERDICT: clear

BREAKING

BEHAVIOR: queue runtime behavior is unchanged and passes; integrated promotion recovery behavior is unverified because 10/12 workflow tests fail before execution. API: no installed-consumer or repository runtime API change. VISUAL: no rendered UI change; all seven PR files are workflow YAML, Node publication code, or tests. THEME: no theme change.

VERDICT: BLOCKS — the repository’s promotion verification path is broken at this head.

PERFORMANCE & RESOURCES

EFFECTS: zero. RENDER: no application render path changes. LISTENERS/OBSERVERS: none. LAYOUT: none. BUNDLE: no dependency or package change. CI cost: unchanged from the approved head; the full test job runs for 17m12s and then fails deterministically in one workflow-test file.

VERDICT: clear

VISUAL EVIDENCE

VISUAL CHECK: not applicable WHY: the restack changes no PR-owned bytes, and the full PR contains only workflow YAML, Node publication code, and tests; no UI, CSS, DOM, story, or rendered component path changes.

VERDICT: clear

REMEDY SEARCH

REMEDY SEARCH: not triggered — no proven visual defect

A11Y & I18N

No rendered element, interaction, role, accessible string, locale formatting, or direction-sensitive behavior changes.

VERDICT: clear

JUDGEMENT

slot	verdict
PROBLEM	clear
SOLUTION	BLOCKS — promotion migration omitted its executable workflow harness
ARCHITECTURE	BLOCKS — integrated recovery boundary no longer reaches the publisher
IMPACT	BLOCKS — exact-head CI and ten recovery cases are non-green
API	note — stale repository test contract
THEMING	clear
BREAKING	BLOCKS — promotion verification path is broken
PERFORMANCE	clear
VISUAL	clear — not applicable
A11Y & I18N	clear — not applicable

GOAL: partly met — patch-id and all PR-owned blobs match the approved head; clean focused tests pass 32/32; the independent runtime matrix passes 10/10; all four targeted mutations are killed. Substantive exact-head CI runs, but its test job fails because 10 promotion workflow cases cannot execute the migrated publisher. DISPOSITION: prior queue-runtime approval → retained; parent-landing gate → resolved; executable promotion-harness migration → blocks now; exact-head CI green gate → blocks now. ADVICE: bounded outcome criteria — migrate visual-promotion-workflow.test.mjs so its sandbox executes the shared publisher command and its recovery/fail-closed assertions cover the new boundary; all 12 tests and full exact-head CI must pass. AUTHOR CAN PROCEED: yes — runtime ownership is settled; update the executable workflow harness without changing the queue model. WORST OUTCOME: “Ten promotion recovery/fail-closed paths stop executing while the exact-head gate is red.” → request changes.

JUDGEMENT NEEDED: none — this is a deterministic repository integration failure with bounded acceptance criteria.

request changes (self-authored fallback COMMENT; no merge or auto-merge)

1. The promotion workflow moved to the shared publisher without migrating its executable workflow harness. → maintainers lose ten passing recovery/fail-closed checks, and exact-head CI cannot clear · .github/workflows/visual-acceptance-promote.yml:344

REVIEW

Semantic verdict: request changes

Thanks — the restack preserves patch-id 6a9447a1 and the verified queue behavior. Exact-head CI now exposes one migration gap: the promotion step moved to the shared publisher, but visual-promotion-workflow.test.mjs still extracts only run: | and its sandbox does not install that publisher. All 10 recovery/fail-closed cases fail before exercising promotion; I reproduced this locally, while the parent workflow passes 12/12.

Could we migrate that executable workflow harness to the publisher, then get its 12 tests and exact-head CI green?

[Reviewed by Robohands]

INLINE (0–3, only when genuinely line-anchored)

None — the finding crosses the workflow command, parser, sandbox, and recovery assertions.

EVIDENCE I DID NOT SPEND

• The old and new parent→head patch files are byte-identical, not merely patch-id equivalent; the PR still contains exactly one intended commit whose sole parent is its main base.
• Five Node syntax checks, actionlint on six workflows, Prettier on all seven PR files, git diff --check, and pnpm check:repo pass.
• Security preflight read every changed workflow/script/test file before checkout or execution; no dependency, lockfile, package hook, executable-mode, shell-evaluation, user-controlled queue path, credential expansion, or public-repo private residue changed.

TIME

TIME total 37m

• setup/rules/security: 6m — fresh loop 1.9.1, official audit 1.14.4, all seven prior rounds, exact metadata, untrusted CI preflight, and dedicated worktree setup
• focused tests/runtime: 5m — clean 32-test suite, post-mutation clean rerun, and 10-case independent runtime matrix
• code/history/mutations: 6m — patch-id/tree/blob proof, parent/base ownership, atomic claim/release, both late orders, simultaneous claims, retry, cleanup, permissions, and four targeted mutants
• CI wait/investigation: 16m — substantive workflows retriggered after the automatic base retarget; failed full-test log, exact local reproduction, parser/sandbox control, and 12/12 parent negative control
• critique/wiki/delivery: 4m — presentation, critic pass, exact-head recheck, fallback COMMENT, record/index update, and readback
• waste: 2m — the first sequential-release mutant failed at staging before exposing partial history and was corrected; two timed-out mutant workers required targeted cleanup

WHAT I COULD NOT VERIFY

• A live overlapping GitHub Actions publication was not dispatched because that would mutate shared gh-pages; production queue code and schemas were driven against isolated bare remotes instead.

Current CI and landing gate

At exact head ba4287ed93a0b082c699d10943fd7dac59950f85, CI run 33221226226 ran after a close/reopen supplied the pull-request event omitted by the automatic base retarget. Build, sandbox, Storybook, docsite, fixture contrast, scope, theme layers, Lint, CLI Smoke Test, Internal Registry, Meta CLA, Vercel, and repository statuses pass or skip appropriately for this non-rendering change. The full test job fails: 625 files pass, 1 fails, and 2 skip; 13,076 tests pass, 10 fail, and 13 skip. All ten failures are in visual-promotion-workflow.test.mjs, before the migrated promotion scenarios execute. Landing is blocked.

Focused evidence

• Restack proof: approved 29dafeba and current ba4287ed have byte-identical parent→head patches and stable patch-id 6a9447a1c9d254ab9874a57504fbc4326cad608b; all seven PR-owned blobs are identical. The current head is one intended commit on parent/base 677d68ea.
• Clean tests: 2 files / 32 tests passed in 49.02s; a second clean rerun after restoring all mutants also passed 32/32.
• Runtime matrix: 10/10 passed in 24.82s across five scopes, both late-current orderings, simultaneous claims, interrupted claim push, atomic release history, and terminal legacy cleanup.
• Mutations: sequential claim fails on differing holder commits; sequential release fails with {shared:true, legacy:false}; terminal holder leakage does not complete within 20 seconds; missing fresh-tip recheck resolves instead of rejecting behind run 899.
• Promotion integration: exact head fails 10/12 locally exactly as CI does. Supporting folded run: > in the parser reaches a second failure—the sandbox has no .github/scripts/gh-pages-publisher.mjs, and the static assertion still expects the removed inline implementation. Replacing only the workflow with its parent version makes the unchanged harness pass 12/12.
• Workflow/repository checks: all six queued publisher jobs retain Actions read permission; actionlint passed six workflows; five Node syntax checks, Prettier, diff-check, and check:repo passed.

What changed before posting

Posted as drafted after one critic pass; the public fallback comment is under the 150-word request-changes cap.

Delivery

GitHub review: https://github.com/facebook/astryx/pull/5631#pullrequestreview-5055945360

Landing readiness

Not ready: the exact-head full test job is red. No merge or auto-merge was performed.

Round 9 — 13a2aa3f harness revalidation

PR

#5631 ci: route visual gh-pages writers through publisher by cixzhang (bucket: the maintainer)

HEAD REVIEWED

13a2aa3fb68029d835ebc3effea68c4d546e94b8 — every claim below was verified at this exact head. Its sole parent and PR base are 677d68ea8f64bb0f9539cb499f4537bd98d2d7ca on main.

VERSIONS

LOOP VERSION: 1.9.1 AUDIT RUBRIC: 1.14.4

LANE

LANE: full WHY: this is a ninth-round re-review of a privileged publication migration after exact-head CI exposed a broken executable recovery harness; the correction must preserve every prior concurrency, recovery, fail-closed, and mutation guarantee.

PROBLEM

WHY 1: The promotion workflow moved from an inline shell script to a folded shared-publisher command, but its executable harness only understood the removed literal block and did not install the publisher in its sandbox. WHY 2: Ten promotion recovery and refusal cases failed before they reached the code they were meant to protect, leaving the exact-head test gate red and behavior unverified. WHY 3: Maintainers can rely on visual acceptance and release only when CI executes the same trusted promotion boundary that production executes.

USER-FACING PROBLEM: A maintainer preparing a release could not use exact-head CI to tell whether promotion recovery and identity failures remained safe after the publisher migration. PROBLEM SEVERITY: broken task — the required landing gate was red and ten recovery/fail-closed scenarios did not execute. NEW FEATURE CASE: not a product feature — this repairs the test projection for the already-reviewed publisher migration. EARLY STOP: clear — Round 8 demonstrated the exact harness failure and gave bounded acceptance criteria.

VERDICT: clear

SOLUTION

The workflow test now reads either literal or folded shell from the checked-in step, gives its sandbox the same publisher entrypoint and helper tree that the workflow checkout supplies, and records the already-acquired publication turn. It then executes the exact migrated command rather than a copied predecessor. The existing recovery and refusal scenarios therefore exercise the shared publisher without changing production behavior.

SOLUTION (1 delta decision · 0 production lines; 109 added / 18 removed test lines)

1. Migrate the executable promotion harness to the folded shared-publisher boundary while preserving every existing behavioral scenario and adding a direct execution assertion.

BURDEN: low — one test file, one folded-scalar parser branch, sandbox links to existing scripts, queue fixture state, and one direct publisher-execution test. BURDEN MATCH: proportionate — the change repairs the exact integration seam that blocked CI and adds no runtime owner, workflow, state, or dependency.

Round 8 asked for the sandbox to execute the shared publisher and preserve all 12 recovery/fail-closed checks. This head does that and adds a thirteenth direct execution check.

VERDICT: clear

ARCHITECTURE

OWNER: the checked-in promotion workflow defines the command; the shared publisher owns promotion behavior; the harness executes that boundary. TIER 1: the shared gh-pages publisher and promotion identity/acceptance modules already reviewed in Rounds 1–8. TIER 2: none. SEAMS: folded YAML extraction, sandbox checkout shape, pre-held publication turn, cleanup-wins-first-push recovery, duplicate dispatch, seven identity/preimage refusal paths, queue rollout compatibility, and exact-head CI. BEHAVIOR UNIT: pure utility/module plus executable workflow integration — production modules run against isolated bare remotes and the workflow step runs from an isolated checkout-shaped sandbox. COMPLEXITY BUDGET: zero production decisions, owners, durable representations, or workflows; one harness adaptation that executes the existing source command. ACTUAL BURDEN: exact delta 1 test file, +109/−18; direct tests 5→6, assertion calls 31→37, fail-closed matrix unchanged at 7 cases; full PR remains 8 files and +2054/−278. BURDEN TREND: first reviewed 311ba8cd → approved runtime 29dafeba contracted current ownership to one dual-queue transaction; ba4287ed → 13a2aa3f leaves all runtime/workflow bytes flat and changes only their executable test projection. RESET TRIGGER: not triggered — this removes the Round 8 test-model mismatch without adding another runtime owner, representation, guard, or protocol.

domain fact	one authoritative writable source	generated / immutable projections	other writable copies
promotion command	visual-acceptance-promote.yml step	test extracts and executes that checked-in scalar	none
promotion behavior	lib/gh-pages-publisher.mjs	thin entrypoint plus sandbox links to the exact files	none
publication ownership	shared ticket/holder plus same-commit legacy projection	fixture snapshots a turn already acquired by run 900	none in the harness
acceptance and baseline identity	immutable acceptance record, current pointer, CI run, merge capture, and baseline preimage	seven adversarial fixture mutations	none

seam	driven result
exact delta	only visual-promotion-workflow.test.mjs changed; production and workflow files are byte-identical to ba4287ed — pass
folded workflow command	exact run: > text is folded and executed by /bin/bash from the sandbox root — pass
sandbox checkout	wrapper, implementation library, and visual-gate helpers are present; run id, runner temp, token, repository, merge/head, and held dual-queue state are supplied — pass
migrated publisher execution	new direct case exits 0; cleanup-race and idempotent recovery cases mutate/inspect the isolated remote — pass
fail-closed paths	stale pointer, superseded CI, wrong head, path/run mismatch, wrong merge capture, tampered archived pixels, and changed preimage all refuse — pass
prior harness negative control	substituting the ba4287ed harness against the migrated workflow passes 2/12 and fails 10/12 at the literal-block extractor — pass
assertion retention	no direct test was removed; the seven-case fail-closed block is byte-identical; one direct test and six assertions were added — pass
queue runtime matrix	five publication scopes, both current arrival orders, simultaneous claim, rejected-push retry, and terminal legacy cleanup pass 10/10 — pass
mutation boundary	sequential claim, sequential release, terminal-holder leak, and missing fresh-tip recheck are each non-green for their intended ownership/liveness failure — pass
exact-head CI	CI, Lint, CLI Smoke Test, Internal Registry, Vercel, Meta CLA, review-required, and visual-acceptance are green; non-rendering visual/a11y/RTL jobs skip as intended — pass

VERDICT: clear

IMPACT

Exact-head CI now executes the migrated publisher through all recovery and refusal paths, so a maintainer can distinguish a safe promotion from stale, superseded, tampered, or conflicting state before landing. The harness correction does not alter publication behavior. NEW FEATURE IMPACT: not a new feature — it restores trustworthy verification for the existing migration.

VERDICT: clear

API

No installed-consumer, repository runtime, publisher CLI, workflow input, output, permission, or state-schema API changes. The delta changes only test helpers and assertions.

	change	public?	class	doc’d?	verdict
~	test helper accepts both `run:	andrun: >`	no — test only	workflow-step extraction	test itself
+	sandbox publisher/checkpoint fixture and direct execution case	no — test only	executable workflow harness	test names/assertions	clear

OSSIFICATION: no runtime or consumer surface changes; the test projection follows existing workflow and module sources instead of creating a second implementation.

VERDICT: clear

THEMING

No CSS, tokens, theme targets, rendered elements, or style contracts change; the exact delta is one Node test file.

VERDICT: clear

BREAKING

BEHAVIOR: no production behavior changes; all previously approved queue, recovery, fail-closed, cleanup, and retry paths remain green. API: no installed-consumer or repository runtime API change. VISUAL: no rendered UI change. THEME: no theme change.

VERDICT: clear

PERFORMANCE & RESOURCES

EFFECTS: zero. RENDER: no application render path changes. LISTENERS/OBSERVERS: none. LAYOUT: none. BUNDLE: no dependency or shipped bundle change. CI cost: one direct integration case raises the promotion suite from 12 to 13 tests; local exact-13 and focused-45 runs completed in 26.65s and 42.44s respectively.

VERDICT: clear

VISUAL EVIDENCE

VISUAL CHECK: not applicable WHY: the exact delta changes only a Node workflow-test harness; the full PR changes workflow YAML, Node publication code, and tests, with no UI, CSS, DOM, story, or rendered component path.

VERDICT: clear

REMEDY SEARCH

REMEDY SEARCH: not triggered — no proven visual defect

A11Y & I18N

No rendered element, interaction, role, accessible string, locale formatting, or direction-sensitive behavior changes. Exact-head a11y and RTL jobs skip for the non-rendering path as intended.

VERDICT: clear

JUDGEMENT

slot	verdict
PROBLEM	clear
SOLUTION	clear — harness now executes the migrated boundary
ARCHITECTURE	clear — test projection follows the workflow and publisher sources
IMPACT	clear
API	clear — test only
THEMING	clear
BREAKING	clear — no production delta
PERFORMANCE	clear
VISUAL	clear — not applicable
A11Y & I18N	clear — not applicable

GOAL: met — exact promotion tests pass 13/13, focused tests pass 45/45, runtime probes pass 10/10, all four ownership/liveness mutants are killed, and exact-head CI passes 626 files / 13,103 tests with the promotion suite executing 13/13. DISPOSITION: Round 8 folded-command extraction and missing-sandbox-publisher finding → resolved; recovery/fail-closed execution → restored; prior runtime, terminal cleanup, queue atomicity, and four mutation guarantees → retained; exact-head CI gate → resolved. ADVICE: omitted — no code or evidence change remains from this review. AUTHOR CAN PROCEED: yes — the exact head is review- and CI-clean. WORST OUTCOME: none found at the reviewed head → approve.

JUDGEMENT NEEDED: none — this delta restores the settled executable test contract without changing product or repository runtime behavior.

approve (self-authored semantic COMMENT; no merge or auto-merge)

REVIEW

Semantic verdict: approve

Thanks — at 13a2aa3f, the harness executes the migrated publisher. Promotion 13/13, focused 45/45, runtime 10/10, all four regression mutants fail, and exact-head CI is green.

[Reviewed by Robohands]

INLINE (0–3, only when genuinely line-anchored)

None.

EVIDENCE I DID NOT SPEND

• The exact 7-case fail-closed test block is byte-identical between ba4287ed and 13a2aa3f; total assertion calls increase from 31 to 37.
• origin/main advanced to c6a8b21a, but no commit since the PR parent touches any of the eight PR files; the exact head remains one commit on its recorded base.
• Five Node syntax checks, Prettier over all eight PR files, git diff --check, filtered actionlint 1.7.12 over six workflows, and pnpm check:repo pass.

TIME

TIME total 25m

• setup/rules/security: 5m — fresh loop 1.9.1, official audit 1.14.4, all eight prior rounds, exact metadata, untrusted CI preflight, and isolated checkout/install
• focused tests/runtime: 5m — exact 13, focused 45, clean 17, independent 10-case runtime matrix, repository checks, and exact-head CI
• code/history/mutations: 8m — exact delta, assertion-retention proof, prior-harness negative control, four production mutants, current-main invalidation check, and CI log
• CI wait: 0m — exact-head CI completed green while local analysis and mutation checks ran
• critique/wiki/delivery: 5m — presentation, two critic passes, exact-head recheck, semantic COMMENT, record/index update, and readback
• waste: 2m — one unfiltered actionlint pass surfaced documented inherited ignores; the terminal-leak mutant needed a bounded direct probe after its focused test hung as intended

WHAT I COULD NOT VERIFY

• A live overlapping GitHub Actions publication was not dispatched because it would mutate shared gh-pages; the production queue modules and schemas were driven against isolated bare remotes instead.

Current CI and landing gate

At exact head 13a2aa3fb68029d835ebc3effea68c4d546e94b8, CI run 33222815512 succeeds. Its full test job passes 626 files / 13,103 tests with 2 files / 13 tests intentionally skipped; visual-promotion-workflow.test.mjs runs and passes all 13 cases. Build, sandbox, Storybook, docsite, fixture contrast, scope, theme layers, Lint, CLI Smoke Test, Internal Registry, Meta CLA, Vercel, review-required, and visual-acceptance are green; non-rendering a11y, RTL, and stable-visual jobs skip as intended.

Focused evidence

• Exact promotion suite: 1 file / 13 tests passed in 26.65s. It executes the real folded workflow command through the shared publisher, survives cleanup winning the first push race, remains idempotent, and refuses all seven stale/tampered/conflicting identities.
• Focused suite: 3 files / 45 tests passed in 42.44s; the restored publisher-only suite passes 17/17 after mutation testing.
• Harness negative control: the prior ba4287ed harness against the migrated workflow passes only 2/12 and fails the ten promotion scenarios at the obsolete run: | extractor.
• Assertion strength: every previous named test remains; the seven fail-closed fixture cases and their execution body are byte-identical; direct tests grow 5→6 and assertion calls 31→37.
• Independent runtime matrix: 10/10 pass across all five scopes, both current arrival orders, simultaneous claim, rejected-push retry, and terminal legacy-holder cleanup.
• Mutation guarantees: sequential claim exposes different holder commits; sequential release exposes {shared:true, legacy:false}; terminal-holder leakage times out with stale holder 899 and both successor tickets retained; removing the fresh-tip recheck resolves instead of refusing behind competing holder 899.
• Repository checks: actionlint 1.7.12 passes six workflows with the documented custom-runner and inherited ShellCheck ignores; five Node syntax checks, Prettier over all eight PR files, git diff --check, and pnpm check:repo pass.

What changed before posting

Posted after two critic passes; the public semantic comment is 25 words.

Delivery

GitHub review: https://github.com/facebook/astryx/pull/5631#pullrequestreview-5056051205

Landing readiness

Ready at exact head 13a2aa3fb68029d835ebc3effea68c4d546e94b8. Landing remains maintainer-controlled; no merge or auto-merge was performed.