-
Notifications
You must be signed in to change notification settings - Fork 0
Review 5631
#5631 ci: route visual gh-pages writers through publisher by cixzhang (bucket: the maintainer)
311ba8cdc3c0f3e94207d86bc151559e8902758a — every claim below was verified at this commit, whose sole parent is approved #5629 head 88195bb8e6765f7fd10fd5abf7521e09d157bc42.
LOOP VERSION: 1.9.1 AUDIT RUBRIC: 1.14.1
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.
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
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)
- Publish immutable PR visual evidence through the shared queue and refresh the state checkout afterward.
- Archive visual acceptance records through the same publisher while preserving trusted identity inputs.
- Promote accepted post-merge pixels only while holding the shared baseline turn.
- 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.
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.
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.
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.
No CSS, tokens, theme targets, rendered elements, or style contracts change.
VERDICT: clear
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.
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 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: not triggered — no proven visual defect
No rendered element, interaction, role, accessible string, locale formatting, or direction-sensitive behavior changes.
VERDICT: clear
| 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)
-
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 -
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
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:144now enters the shared wait path, but itsacceptjob grants noactions: 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]
None — both defects cross a workflow permission block and publication-owner boundary.
- 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 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
- 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.
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.
- 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: readexceptvisual-acceptance.yml’sacceptjob. - Roll-forward probe: run 100 acquired
.astryx-gh-pages/publication-queue/holder.jsonwhile run 101 simultaneously acquiredvisual-gate/publication-queue/holder.jsonagainst 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
ghbinary.
Posted as drafted after one critic pass; the public review is 107 words.
GitHub review: https://github.com/facebook/astryx/pull/5631#pullrequestreview-5048724688
#5631 ci: route visual gh-pages writers through publisher by cixzhang (bucket: the maintainer)
d64f87286431e6003d6b5da1df379706f09844e9 — every claim below was verified at this commit, whose sole parent is approved #5629 head 88195bb8e6765f7fd10fd5abf7521e09d157bc42.
LOOP VERSION: 1.9.1 AUDIT RUBRIC: 1.14.1
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.
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
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)
- Grant the migrated acceptance job
actions: readand bind every shared-publisher caller’s permission contract in the workflow test. - 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.
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.
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.
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.
No CSS, token, target, rendered element, or style contract changes.
VERDICT: clear
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.
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 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: not triggered — no proven visual defect
The delta adds no rendered element, interaction, role, accessible string, locale formatting, or direction-sensitive behavior.
VERDICT: clear
| 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)
- 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
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]
None — the finding crosses the global shared holder and the legacy baseline holder.
- Focused Vitest is 2 files / 25 tests green at the exact head; actionlint, both Node syntax checks, and
pnpm check:repoare 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 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
- 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.
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.
- 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.
Posted as drafted after one critic pass; the public review is 113 words.
GitHub review: https://github.com/facebook/astryx/pull/5631#pullrequestreview-5048893503
#5633 is not code-review-safe to start while this parent remains semantically blocked; it was not inspected or touched in this run.
#5631 ci: route visual gh-pages writers through publisher by cixzhang (bucket: the maintainer)
7c03d407e6c6e6c3044a4209f76134d7cea8ec84 — every claim below was verified at this exact head, whose sole parent is approved/green #5629 head 88195bb8e6765f7fd10fd5abf7521e09d157bc42.
LOOP VERSION: 1.9.1 AUDIT RUBRIC: 1.14.1
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.
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
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)
- 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.
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.
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.
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.
No CSS, token, target, rendered element, or style contract changes.
VERDICT: clear
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.
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 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: not triggered — no proven visual defect
No rendered element, interaction, role, accessible string, locale formatting, or direction-sensitive behavior changes.
VERDICT: clear
| 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)
- 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
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]
None — the finding is the cross-queue acquisition sequence itself.
- The exact
d64f8728→7c03d407delta 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, andpnpm check:repoare 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 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:repoin 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
- 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.
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.
- 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:
d64f8728reproduces 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 releasefinallyuntil that wait returns at lines 1512–1523. - Permissions/security:
actions: readremains 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 fakegh. - Checks: 2 files / 25 tests passed; actionlint passed all six relevant workflows; three Node syntax checks passed; Prettier,
git diff --check, andpnpm check:repopassed.
Posted as drafted after one critic pass; the public review is under the 150-word request-changes cap.
GitHub review: https://github.com/facebook/astryx/pull/5631#pullrequestreview-5049088174
#5633 is not code-review-safe to start while this parent remains semantically blocked; it was not inspected or touched in this run.
#5631 ci: route visual gh-pages writers through publisher by cixzhang (bucket: the maintainer)
89c9f042ddc4d481271fb1be9f7ad21cb7743c54 — every claim below was verified at this exact head, whose sole parent is approved/green #5629 head 88195bb8e6765f7fd10fd5abf7521e09d157bc42.
LOOP VERSION: 1.9.1 AUDIT RUBRIC: 1.14.1
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.
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
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)
- 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.
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.
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.
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.
No CSS, token, target, rendered element, or style contract changes.
VERDICT: clear
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.
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 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: not triggered — no proven visual defect
No rendered element, interaction, role, accessible string, locale formatting, or direction-sensitive behavior changes.
VERDICT: clear
| 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)
- 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
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]
None — the finding is the cross-queue ticket-order model, not one local statement.
- 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, andpnpm check:repopass. - 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 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
- 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.
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.
- 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: readremains 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, andpnpm check:repopassed.
Posted as drafted after one critic pass.
GitHub review: https://github.com/facebook/astryx/pull/5631#pullrequestreview-5049260806
#5633 is not code-review-safe to start while this parent remains semantically blocked; it was not inspected or touched in this run.