feat(compat): PR-gated v2 drop-in compat testing (bootstrap)#77
Draft
phramz wants to merge 18 commits into
Draft
feat(compat): PR-gated v2 drop-in compat testing (bootstrap)#77phramz wants to merge 18 commits into
phramz wants to merge 18 commits into
Conversation
Completes the "diffs against goldens" step the 2026-04-23 local-CI unification spec anticipated but never implemented. Goldens captured once on ubuntu-24.04 x86_64 / PHP 8.4, committed to test/compat/testdata/golden/v2/. Per-PR compat-diff runs inside the existing noble/amd64/8.4 pipeline cell; deviations fail the cell and a compat-report job posts (or deletes) a sticky PR comment that surfaces the on-demand golden-refresh escape hatch. A weekly workflow opens a drift PR when v2 or the Ondrej PPA shifts. No separate compat workflow, no v2 install at PR time.
Extend testCellOpts with CompatMatrix/GoldenDir/DeviationsPath flags, wire them into parseTestCellFlags, add the shouldRunCompat gate and compat-diff step inside runFixture (step 6, after invariants), add the RW deviations mount in buildCellMounts, and thread all three new flags from runCell into the inner test-cell command.
GitHub's workflow_dispatch requires the workflow file to already exist on the default branch before it can be dispatched from any ref. That's a chicken-and-egg for the initial capture of the feature branch's goldens: we can't dispatch against feat/pr-gated-v2-compat because the file isn't on main yet. Adding a pull_request trigger scoped to this file's own path sidesteps the restriction (pull_request workflows run from the PR's head, not from main). It also keeps this safety net permanently: any future edit to the capture pipeline re-exercises it on its own PR before landing on main.
git diff --quiet ignores untracked files, so on the initial capture (where every JSON is new to the tree) it returned clean and the drift-PR step was silently skipped. Switch to git status --porcelain, which surfaces adds, modifications, and deletions uniformly. Observed on the PR #77 bootstrap run (#24897651795): 8 goldens were staged into the tree but drift=false fired because none had been tracked previously.
On pull_request triggers github.ref_name is the synthetic <pr-number>/merge ref — not a real branch — so peter-evans/create-pull-request fails with "couldn't find remote ref" when it tries to rebase onto it. github.head_ref holds the PR's source branch, which is what we actually want. Observed on PR #77 bootstrap run (#24897788494) after the drift- detection fix: 8 goldens committed cleanly, then the PR-open step errored on git fetch 77/merge.
The JSONs under test/compat/testdata/golden/v2/ are captured from shivammathur/setup-php@<sha> via test/compat/probe.sh. probe.sh emits compact single-line arrays; prettier wants one element per line. Every weekly refresh would otherwise force a follow-up reformat commit and cause lint churn on both the feature PR and every drift PR. Goldens are frozen captured data, not source — same rationale as gitignoring generated artifacts.
buildCellMounts was mounting test/compat/testdata/golden/ as /golden, so captured files at .../golden/v2/bare.json landed at /golden/v2/ bare.json inside the container. But runFixture looks for /golden/ <fixture>.json (no v2/ subpath), causing every opted-in fixture to fail with "golden missing". Mount the v2 subdirectory directly so the container path matches what the consumer expects. Preserves the version namespace on the host side (room for future golden/v3/ if the upstream pin ever bumps in a non-backward-compatible way). Observed on PR #77 ci run #24898111265: all 8 compat fixtures failed with "golden missing for fixture X at /golden/X.json: run \`gh workflow run compat-golden-refresh.yml\`" despite goldens being present on the branch.
jq's JSON-string parser rejects \` (backtick preceded by backslash) as
an invalid escape sequence — only the standard JSON escapes (", \, /,
b, f, n, r, t, u) are valid. Using literal backticks inside concatenated
string segments produces the same markdown output without tripping the
lexer.
Observed on PR #77 ci run #24898366328: compat-report's "Render comment
body" step exited with "jq: 8 compile errors" and the 8 per-fixture
deviations never reached the sticky comment.
Contributor
|
…rker From the final holistic review on #77: 1. Add a concurrency group to compat-golden-refresh.yml so a Monday cron firing while a PR-trigger run is in flight doesn't race on the shared chore/compat-golden-refresh branch. cancel-in-progress is false because the workflow opens a PR — cancelling mid-run could leave it half-pushed. 2. Update the spec to match what the compat-report job actually does: on a red→green transition it REWRITES the sticky comment body to the "✅ Compat-diff cleared" marker (preserving audit trail) rather than DELETING the comment. The implementation chose the replace path deliberately; the spec text is now consistent.
Contributor
Author
Final review — appliedLanded the two pre-merge items from the holistic review in af7ecc5:
Deferred items (tracked as GitHub issues after you review)The holistic review flagged four follow-ups that aren't merge-blocking. Recommending filing issues for each rather than landing them in this PR:
And the substantive follow-up flagged when the PR went canonical-cell-red:
Shall I file these as GitHub issues, or do you want to review and file yourself? |
This was referenced Apr 25, 2026
Contributor
Author
|
Five tracking issues filed for the deferred follow-ups:
Infrastructure side of this PR is complete; the canonical-cell red is the gate doing its job. Ready for your call on whether to merge as-is and chase #79 in a follow-up, or hold this PR until #79 lands so the cell is green at merge time. |
6 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Wires the "diffs against goldens" step the 2026-04-23 local-CI-unification spec anticipated but PR 3 didn't fully implement. Every piece around it existed (
phpup compat-diffsubcommand,probe.sh,docs/compat-matrix.mdallowlist,test/compat/testdata/location allocated); this PR delivers the actual comparison wiring + the refresh/report infrastructure.compat: trueYAML flagphpup internal test-cellcallscompatdiff.DiffFilesper opted-in fixture when running in the canonicalnoble/x86_64/8.4cell; non-canonical cells skipphpup internal golden-capture+make compat-refresh-goldensfor the maintainer side.github/workflows/compat-golden-refresh.yml— weekly cron + workflow_dispatch + (bootstrap) pull_request trigger on itself; opens a drift PR when v2/PPA bytes shiftcompat-reportjob inci.yml— posts a sticky PR comment listing deviations with the on-demand refresh escape hatch; updates the sticky to "cleared" when a red PR goes greenDesign reference:
docs/superpowers/specs/2026-04-24-pr-gated-v2-compat-design.mdBootstrap done
Goldens captured via the self-triggering
pull_requesthook and merged into this branch in935e739 chore: refresh v2 compat goldens. Weekly cron +gh workflow run compat-golden-refresh.ymlescape hatch are live for future refreshes.CI state — canonical cell is RED by design
The
noble/x86_64/8.4pipeline cell fails on real, pre-existing drift between our action and v2 — exactly what the gate is designed to surface. The compat-report sticky comment on this PR enumerates ~12 ini keys per fixture where v2 emits stock upstream defaults and our bundles currently don't:expose_php,log_errors,max_input_time,opcache.{enable,enable_cli,jit,jit_buffer_size,memory_consumption,revalidate_freq,validate_timestamps},session.{gc_maxlifetime,save_handler}, plusxdebug.{mode,start_with_request}oncoverage: xdebug.None of these are regressions introduced by this PR — they're gaps in our
internal/composelayer that predate the gate. The PR demonstrates the machinery works: the gate fires, the artifact is produced, the comment renders, the escape hatch is documented.Blocking merge until maintainer decides per-deviation:
php.ini-production+jit.ini+xdebug.inioverlays)docs/compat-matrix.mdwithreason:explaining each accepted divergenceI deliberately did NOT land blanket
kind: ignoreallowlist entries in this PR — they'd mask the real drift the gate is correctly catching, which is scope creep beyond "build the compat infrastructure."Test plan
make checkclean)shouldRunCompatgate unit-tested for non-canonical skip + canonical pass + canonical fail pathscompatdiff.DiffFilesAPI tested for no-deviation, reports-deviation, invalid-probecompat-golden-refreshworkflow opens a drift PR on this branch (PR chore: refresh v2 compat goldens #78, fast-forward merged)compat-reportjob posts a correctly-formatted sticky PR comment on canonical-cell failure (see this PR's comments)compat-report"cleared" path verified once drift is resolved (follow-up PR)Files of note
docs/superpowers/specs/2026-04-24-pr-gated-v2-compat-design.md— designinternal/testsuite/testcell.go:runFixture— step 6 compat-diff gateinternal/testsuite/goldencapture.go—phpup internal golden-captureinternal/compatdiff/public.go—DiffFiles/Deviationexportsinternal/testsuite/deviations.go— per-cell artifact writer.github/workflows/compat-golden-refresh.yml— refresh workflow.github/workflows/ci.yml—compat-reportjob + canonical-cell artifact uploadtest/compat/testdata/golden/v2/*.json— 8 captured v2 probes