test(forge): add -update live-capture path for golden fixtures (RIG-2229 T2) - #486
Merged
Merged
Conversation
…229 T2) Implements the `-update` regeneration lane owed by RIG-2229 T2: a `TestLiveUpdateFixtures` (//go:build livegithub) that re-runs each committed fixture's scenario against the real GitHub/Linear forges and rewrites testdata/<provider>/<name>.json, canonicalizing volatile fields on write and reusing the oracle's volatileFields as the single source of truth (per Matt's ruling). The oracle suite (#438) merged without this capture path; this closes it and unblocks T3's workflow_dispatch regen lane (RIG-2230). Mechanism (untagged golden_capture_test.go, runs credential-free in the normal battery): - canonicalizeWire rewrites a wire-JSON tree, substituting a fixed sentinel for every volatile wire key (matched by name at any depth) while preserving key, structure, and type. A pure per-run change canonicalizes to identical bytes; a real shape change (new/renamed/retyped field) survives and shows in the diff, so a regeneration diffs only on a genuine contract-shape change. - Both fixture halves (the emitted request method/path/query/body AND the decoded Want) are DERIVED from one invoke() replay over the canonicalized responses and canonicalized coordinates — the identical code path golden replay runs. So TestGoldenFixtures' load-bearing invariants (the emitted request matches Request, and marshal(decode(Body)) == Want) hold by construction after any regeneration, and a live per-run coordinate can never leak into a committed request. Because the request is re-derived (not recorded from the live wire), a Linear resolve id (teamId/issueId) canonicalizes consistently on both sides, so string ids are canonicalized for regen stability. - domainToWire ties the wire-key table back to volatileFields; TestUpdateCanonicalizeCoversVolatileFields asserts its keyset equals volatileFields and the two tables agree on wire keys, so a new domain volatile without a wire mapping fails the untagged battery. volatileFields moves from the tagged oracle file to the untagged capture file so one definition is shared by both suites (a tagged build compiles untagged files; stripVolatile still resolves it) — no behavior change to the oracle. .golangci.yml: set goconst.ignore-tests. goconst counts string literals package-wide but the existing exclusion drops goconst reporting on test files; without ignore-tests, a literal repeated in the new test counts toward the package total and tips a production site (linear.go) over the threshold, surfacing a finding there. ignore-tests aligns the counting with the reporting policy; production repetition still gates. Verified: gofmt clean; full untagged forge suite green (incl. all 10 committed fixtures via TestGoldenFixtures — no committed fixture changed); default-lane and livegithub-tagged golangci-lint both 0 issues; tagged `go vet` clean. TestUpdateCanonicalizeStable is a RED->GREEN guard proven to fail if a volatile wire key (incl. displayName, target_url) is dropped or if a request coordinate is taken from the live wire instead of derived. Refs RIG-2229. Co-authored-by: Matt Wilkinson <matt@rigel.build>
|
Compass engineering docs preview: https://compass-server-2229-update-c.compass-eng-docs.pages.dev Deployed from |
Review round 1 on #486 (0 high, 2 medium, 3 low). Fixes: - medium (composite coverage gap): add TestUpdateCanonicalizeComposite, an untagged credential-free guard for get_pull_request — the only fixture with EXTRA legs. It drives the real assembleFixture -> replayFixture pipeline over a synthetic PR detail (asserted) + reviews/check_runs/statuses (Extra), so the responses[prelude+1:] Extra assembly and canonNode's []any recursion into an array of objects carrying volatile keys (two review Authors) are exercised — the path create_issue cannot reach. Asserts both review authors canonicalize while per-review Verdict/IsBot survive distinctly, target_url + html_url both fold to canonURL, and head.sha canonicalizes consistently across the detail leg and the rolled-up Checks.HeadSHA. RED-proven: disabling canonNode's array recursion fires it on all four nested-volatile assertions. - medium (paginated regen truncation): add an all-consumed exact-count guard to deriveFixtureHalves mirroring replayFixture's invariant. A captured response the client never reaches (e.g. a later pagination page whose rel=next Link header the recorder drops, so HasNext stays false) would otherwise be written into a truncated fixture silently; now it fails loudly AT CAPTURE. RED-proven: an extra unconsumed response fires "replay emitted N requests, want exactly M". The additive multi-page header-capture capability this guard protects is deferred to RIG-2481 (no committed fixture is multi-page today). - low (setup-client clarity): setup clients built the fixture subject through a throwaway recordingRoundTripper whose recording was discarded, implying the recording was load-bearing. Add setupGitHub/setupLinear (non-recording) so only the asserted op is wrapped in a recorder. - low (guard message): reword assembleFixture's insufficient-responses fatal to match the check (at least prelude + 1, not > prelude + 1). Refs RIG-2229. Co-authored-by: Matt Wilkinson <matt@rigel.build>
mattwilkinsonn
approved these changes
Aug 22, 2026
rigel-mintaka
added a commit
that referenced
this pull request
Aug 22, 2026
Additive review-fix commit on PR #487. Round-1 review returned 0 high, 2 medium, 2 actionable low; this closes both mediums (contract-drift against the frozen record, not design forks) and the consistency low. Medium — Linear leg was vacuous-green (the load-bearing one). The record freezes Linear as CO-EQUAL and in-scope now, not deferred (design §525-526), and T2's suite carries an independent `liveLinearSkipMessage` for the Linear legs. The guard only asserted the GitHub `liveSkipMessage`, so a GitHub-provisioned / Linear-absent state would run the GitHub legs, skip the Linear legs, and still report the package `ok` — a required check going green while the co-equal Linear live-contract leg silently asserted nothing. That hole was PERMANENT: it survived the planned removal of the have_creds staging clause, so a later LINEAR_FORGE rotation-to-empty would erase Linear verification with no signal. Two coupled fixes: - forge_creds now requires all FIVE secrets (the LIVEGITHUB_* trio AND LINEAR_FORGE + LINEAR_FORGE_TEAM), so a half-provisioned window keeps the oracle fully dormant rather than half-running and redding on the still- skipped Linear leg. - the assert-it-ran guard now sed-derives BOTH skip literals from source and fails if EITHER leg skipped, plus the package-ok line — the source-derived backstop that a partial credential state cannot pass green, independent of the staging clause. Medium — workflow_dispatch silently broadened the whole workflow. Adding the top-level `workflow_dispatch` trigger for the regen lane also satisfied every other job's `github.event_name != 'pull_request'` guard, so a fixture-regen dispatch would spin up the full ~90-minute gates battery + the podman dogfood tier alongside the regen job, and a flake in either would show the dispatch run red though the regen PR opened cleanly. Added `github.event_name != 'workflow_dispatch' &&` to the gates, dogfood-e2e, and CI-rollup job `if:` blocks (OR-group kept parenthesized — `&&` binds tighter than `||`), confining a dispatch to the regen-forge-fixtures job alone, which is what the regen job's own comment already claimed. Low — forge_affected now mirrors the gtk3-affected gate's explicit `git rev-parse --verify --quiet` base-ref check, emitting an actionable `::error::` when the base does not resolve instead of surfacing a bare git diff error (behavior was already fail-closed; this names the cause). Not changed: the one review low re: livegithub_test.go absence is moot — T2 (PR #486) has since merged, so internal/forge/livegithub_test.go and both skip consts exist at main; the guard's sed reads a real file. Verification: actionlint clean over the full file (the three residual shellcheck findings — SC2086 x2, SC1087 — are pre-existing on main; the regen job's unquoted `nix build --no-link $stores` matches the gates/dogfood convention at lines 203/911 verbatim). Boolean precedence of all three dispatch-exclusion guards traced by hand across every event. Refs RIG-2230. Co-authored-by: Matt Wilkinson <matt@rigel.build>
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
2 participants
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.
Implements the
-updateregeneration lane owed by RIG-2229 T2: aTestLiveUpdateFixtures(//go:build livegithub) that re-runs each committedfixture's scenario against the real GitHub/Linear forges and rewrites
testdata//.json, canonicalizing volatile fields on write and
reusing the oracle's volatileFields as the single source of truth (per Matt's
ruling). The oracle suite (#438) merged without this capture path; this closes
it and unblocks T3's workflow_dispatch regen lane (RIG-2230).
Mechanism (untagged golden_capture_test.go, runs credential-free in the normal
battery):
every volatile wire key (matched by name at any depth) while preserving key,
structure, and type. A pure per-run change canonicalizes to identical bytes; a
real shape change (new/renamed/retyped field) survives and shows in the diff,
so a regeneration diffs only on a genuine contract-shape change.
decoded Want) are DERIVED from one invoke() replay over the canonicalized
responses and canonicalized coordinates — the identical code path golden
replay runs. So TestGoldenFixtures' load-bearing invariants (the emitted
request matches Request, and marshal(decode(Body)) == Want) hold by
construction after any regeneration, and a live per-run coordinate can never
leak into a committed request. Because the request is re-derived (not recorded
from the live wire), a Linear resolve id (teamId/issueId) canonicalizes
consistently on both sides, so string ids are canonicalized for regen
stability.
TestUpdateCanonicalizeCoversVolatileFields asserts its keyset equals
volatileFields and the two tables agree on wire keys, so a new domain volatile
without a wire mapping fails the untagged battery.
volatileFields moves from the tagged oracle file to the untagged capture file so
one definition is shared by both suites (a tagged build compiles untagged files;
stripVolatile still resolves it) — no behavior change to the oracle.
.golangci.yml: set goconst.ignore-tests. goconst counts string literals
package-wide but the existing exclusion drops goconst reporting on test files;
without ignore-tests, a literal repeated in the new test counts toward the
package total and tips a production site (linear.go) over the threshold,
surfacing a finding there. ignore-tests aligns the counting with the reporting
policy; production repetition still gates.
Verified: gofmt clean; full untagged forge suite green (incl. all 10 committed
fixtures via TestGoldenFixtures — no committed fixture changed); default-lane
and livegithub-tagged golangci-lint both 0 issues; tagged
go vetclean.TestUpdateCanonicalizeStable is a RED->GREEN guard proven to fail if a volatile
wire key (incl. displayName, target_url) is dropped or if a request coordinate
is taken from the live wire instead of derived.
Refs RIG-2229.
Co-authored-by: Matt Wilkinson matt@rigel.build