diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 58a3e5929..74f4b560b 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -814,9 +814,10 @@ jobs: # own lane behind the rollup; the tri-event + same-repo-head guards stay # on the steps as defense-in-depth. The job-level `if:` ANDs the # dispatch-aware gate guard with the forge affected gate: on a PR the job - # runs only when setup found the forge surface affected (forge_affected — - # the go/internal/forge/** or ci.yml change), and always on push/schedule - # and on a base-re-point re-trigger. The + # runs only when setup found the forge contract surface affected + # (forge_affected — a go/internal/forge/** change, fixtures included; NOT a + # bare ci.yml edit, RIG-2909), and always on push/schedule and on a + # base-re-point re-trigger. The # in-step forge_affected detection + tri-event/same-repo-head guards stay as # defense-in-depth. No privileged container. if: >- @@ -887,12 +888,18 @@ jobs: id: forge_affected working-directory: go # PRs only. The live-contract oracle below is expensive (it drives a real - # GitHub + Linear testbed over the network) and secret-bearing, so on a PR - # it runs ONLY when the PR actually changes the forge surface it guards — - # the same affected posture the one-job gate takes everywhere else. The - # surface is two path sets: the forge package the suite exercises - # (go/internal/forge/**) and this workflow file itself (a change to the - # oracle's own wiring must re-run the oracle to prove the wiring). + # GitHub + Linear testbed over the network) and secret-bearing, and it is + # the EXTRA verification layered on top of the untagged golden-replay + # battery (leg 1) that already asserts the client contract on every PR + # with zero network. So on a PR it runs ONLY when the PR changes the forge + # CONTRACT SURFACE it guards: the forge package the suite exercises, + # including its committed testdata fixtures (go/internal/forge/**). + # Deliberately NOT keyed on this workflow file: a docs/CI-only PR touching + # ci.yml must not run the whole live oracle (RIG-2909 — that over-trigger + # flaked unrelated PRs on a third-party API blip). Oracle-wiring changes + # are re-verified by the push/schedule full sweep, which runs it + # unconditionally. This mirrors the ci-matrix generator's FORGE_PATH_RE; + # the two must stay in step. # # GitHub exposes no changed-paths primitive to a step `if:`, so — exactly # as the dogfood job resolves image_affected into $GITHUB_OUTPUT for its @@ -919,7 +926,7 @@ jobs: fi changed=$(git diff --name-only "$base...HEAD") if printf '%s\n' "$changed" \ - | grep -qE '^(go/internal/forge/|\.github/workflows/ci\.yml$)'; then + | grep -qE '^go/internal/forge/'; then echo "forge_affected=true" >>"$GITHUB_OUTPUT" else echo "forge_affected=false" >>"$GITHUB_OUTPUT" diff --git a/go/internal/forge/livegithub_test.go b/go/internal/forge/livegithub_test.go index a4cf2e4d4..049c75959 100644 --- a/go/internal/forge/livegithub_test.go +++ b/go/internal/forge/livegithub_test.go @@ -33,6 +33,7 @@ import ( "errors" "fmt" "io" + "net" "net/http" "os" "path/filepath" @@ -474,10 +475,12 @@ func TestLiveLinearCreateIssue(t *testing.T) { ln := liveLinear(ts) f := liveFixture(t, providerLinear, "create_issue") - got, err := ln.CreateIssue(ctx, team, CreateIssue{ - Title: "compass-live-issue-" + newRunID(), - Body: f.Request.Input.body(), - Labels: f.Request.Input.labels(), + got, err := createWithBackoff(ctx, func() (Issue, error) { + return ln.CreateIssue(ctx, team, CreateIssue{ + Title: "compass-live-issue-" + newRunID(), + Body: f.Request.Input.body(), + Labels: f.Request.Input.labels(), + }) }) if err != nil { t.Fatalf("CreateIssue: %v", err) @@ -497,7 +500,9 @@ func TestLiveLinearCommentOnIssue(t *testing.T) { ctx := context.Background() ln := liveLinear(ts) - issue, err := ln.CreateIssue(ctx, team, CreateIssue{Title: "compass-live-comment-" + newRunID()}) + issue, err := createWithBackoff(ctx, func() (Issue, error) { + return ln.CreateIssue(ctx, team, CreateIssue{Title: "compass-live-comment-" + newRunID()}) + }) if err != nil { t.Fatalf("CreateIssue (setup): %v", err) } @@ -525,8 +530,10 @@ func TestLiveLinearGetIssue(t *testing.T) { ln := liveLinear(ts) f := liveFixture(t, providerLinear, "get_issue") - issue, err := ln.CreateIssue(ctx, team, CreateIssue{ - Title: "compass-live-get-" + newRunID(), + issue, err := createWithBackoff(ctx, func() (Issue, error) { + return ln.CreateIssue(ctx, team, CreateIssue{ + Title: "compass-live-get-" + newRunID(), + }) }) if err != nil { t.Fatalf("CreateIssue (setup): %v", err) @@ -552,8 +559,10 @@ func TestLiveLinearListIssues(t *testing.T) { ctx := context.Background() ln := liveLinear(ts) - setup, err := ln.CreateIssue(ctx, team, CreateIssue{ - Title: "compass-live-list-" + newRunID(), + setup, err := createWithBackoff(ctx, func() (Issue, error) { + return ln.CreateIssue(ctx, team, CreateIssue{ + Title: "compass-live-list-" + newRunID(), + }) }) if err != nil { t.Fatalf("CreateIssue (setup): %v", err) @@ -615,18 +624,30 @@ func (r fixtureResponse) firstWant(t *testing.T) json.RawMessage { return rows[0] } -// --- rate-limit backoff ------------------------------------------------------ +// --- transient-condition backoff --------------------------------------------- -// createWithBackoff wraps ANY GitHub content-creating call with a single -// bounded backoff on GitHub's SECONDARY rate limit (403 abuse-detection on -// rapid content creation — issue/PR/comment creates and the H3 branch-seed -// commit all trip it). This is real live-API timing behavior on the network -// path: it never executes on the skip path, and it is a bounded one-shot -// ctx-aware backoff, NOT a retry loop masking a bug (rule://no-retries). +// createWithBackoff wraps ANY live content-creating call (GitHub or Linear) with +// a single bounded backoff on a TRANSIENT live-API condition, then re-issues once. +// Two conditions qualify, both real live-API timing behavior on the network path, +// neither a code bug the retry would mask (rule://no-retries): +// +// - GitHub's SECONDARY rate limit (403 abuse-detection on rapid content +// creation — issue/PR/comment creates and the H3 branch-seed commit trip it); +// it clears in seconds. +// - A transient network timeout — the HTTP client's deadline elapsing while +// awaiting response headers (a third-party latency/availability blip against +// api.github.com / api.linear.app). RIG-2909: a single such blip on a Linear +// setup create was failing the whole forge-oracle gate on unrelated PRs. +// +// It is a bounded ONE-SHOT ctx-aware backoff, not a retry loop: exactly one +// re-issue, and if the condition persists the second attempt's error propagates +// and the test fails loud (a genuine outage or a real bug is not papered over). +// It never executes on the skip path (no credentials -> the caller t.Skips first). func createWithBackoff[T any](ctx context.Context, create func() (T, error)) (T, error) { got, err := create() - if isSecondaryRateLimit(err) { - // Back off once, then re-issue — GitHub's secondary limit clears quickly. + if isSecondaryRateLimit(err) || isTransientNetworkTimeout(err) { + // Back off once, then re-issue — the secondary limit clears quickly and a + // transient header timeout is gone by the next attempt. select { case <-ctx.Done(): var zero T @@ -693,6 +714,25 @@ func isSecondaryRateLimit(err error) bool { return strings.Contains(strings.ToLower(se.Message), "secondary rate limit") } +// isTransientNetworkTimeout reports whether err is a transient network timeout: +// the HTTP client's deadline elapsing while awaiting response headers (the +// `Post "...": context deadline exceeded (Client.Timeout exceeded while awaiting +// headers)` shape). The client wraps this as a *url.Error whose Timeout() is +// true and which wraps context.DeadlineExceeded; the provider then wraps that as +// `do request: %w`, so both errors.As(net.Error) and errors.Is(DeadlineExceeded) +// see through the chain. This is a third-party latency/availability blip, not a +// bug in our client — exactly the class createWithBackoff re-issues once. +func isTransientNetworkTimeout(err error) bool { + if err == nil { + return false + } + var netErr net.Error + if errors.As(err, &netErr) && netErr.Timeout() { + return true + } + return errors.Is(err, context.DeadlineExceeded) +} + // --- teardown (test-side REST; the Provider interface has no close/delete) ---- // closeGitHubIssue closes an issue via REST (GitHub cannot delete issues). A @@ -1085,7 +1125,7 @@ func linearUpdateSpecs() []captureSpec { ctx := context.Background() ln := recordingLinear(ts, rt) in := CreateIssue{Title: "compass-live-issue-" + newRunID(), Body: "stamped body"} - got, err := ln.CreateIssue(ctx, team, in) + got, err := createWithBackoff(ctx, func() (Issue, error) { return ln.CreateIssue(ctx, team, in) }) if err != nil { t.Fatalf("CreateIssue: %v", err) } @@ -1099,7 +1139,9 @@ func linearUpdateSpecs() []captureSpec { ts, team := requireLinear(t) ctx := context.Background() setup := setupLinear(ts) - issue, err := setup.CreateIssue(ctx, team, CreateIssue{Title: "compass-live-get-" + newRunID(), Body: "raw body"}) + issue, err := createWithBackoff(ctx, func() (Issue, error) { + return setup.CreateIssue(ctx, team, CreateIssue{Title: "compass-live-get-" + newRunID(), Body: "raw body"}) + }) if err != nil { t.Fatalf("CreateIssue (setup): %v", err) } @@ -1116,7 +1158,9 @@ func linearUpdateSpecs() []captureSpec { ts, team := requireLinear(t) ctx := context.Background() setup := setupLinear(ts) - issue, err := setup.CreateIssue(ctx, team, CreateIssue{Title: "compass-live-list-" + newRunID(), Body: "raw body"}) + issue, err := createWithBackoff(ctx, func() (Issue, error) { + return setup.CreateIssue(ctx, team, CreateIssue{Title: "compass-live-list-" + newRunID(), Body: "raw body"}) + }) if err != nil { t.Fatalf("CreateIssue (setup): %v", err) } @@ -1135,7 +1179,9 @@ func linearUpdateSpecs() []captureSpec { ts, team := requireLinear(t) ctx := context.Background() setup := setupLinear(ts) - issue, err := setup.CreateIssue(ctx, team, CreateIssue{Title: "compass-live-comment-" + newRunID()}) + issue, err := createWithBackoff(ctx, func() (Issue, error) { + return setup.CreateIssue(ctx, team, CreateIssue{Title: "compass-live-comment-" + newRunID()}) + }) if err != nil { t.Fatalf("CreateIssue (setup): %v", err) } diff --git a/tools/ci-matrix/index.test.ts b/tools/ci-matrix/index.test.ts index 011e3d60e..bd1d373ee 100644 --- a/tools/ci-matrix/index.test.ts +++ b/tools/ci-matrix/index.test.ts @@ -241,16 +241,32 @@ describe("flags — pgtest / microvm / forge / gtk4 rules", () => { ).toBe(true); }); - test("forgeAffected on the ci.yml self-edit path", () => { + test("forgeAffected on a testdata fixture change (still the forge surface)", () => { expect( generate( prInput({ affectedIds: [], - changedPaths: [".github/workflows/ci.yml"], + changedPaths: ["go/internal/forge/testdata/linear/create_issue.json"], }), ).forgeAffected, ).toBe(true); - // A different workflow file must NOT trigger forge. + }); + + test("forgeAffected NOT triggered by a ci.yml-only PR (RIG-2909)", () => { + // The live oracle is the expensive extra verification on top of the + // untagged golden-replay battery; a PR that only touches this workflow + // file (or any other CI/docs-only change) must not run it — that + // over-trigger flaked unrelated PRs on a Linear API blip. Oracle-wiring + // changes are re-verified by the push/schedule full sweep instead. + expect( + generate( + prInput({ + affectedIds: [], + changedPaths: [".github/workflows/ci.yml"], + }), + ).forgeAffected, + ).toBe(false); + // A different workflow file must not trigger forge either. expect( generate( prInput({ diff --git a/tools/ci-matrix/index.ts b/tools/ci-matrix/index.ts index 90539d979..846fbe728 100755 --- a/tools/ci-matrix/index.ts +++ b/tools/ci-matrix/index.ts @@ -68,9 +68,19 @@ const CI_GROUP_PREFIX = "ci-group."; const PGTEST_PROJECT = "compass-go"; const GUEST_IMAGE_PROJECT = "compass-guest-image"; -/** forge trigger: changed path under go/internal/forge/ OR ci.yml itself. */ -const FORGE_PATH_RE = - /^(?:go\/internal\/forge\/|\.github\/workflows\/ci\.yml$)/; +/** + * forge trigger (PR): a changed path under go/internal/forge/ — the forge + * CONTRACT SURFACE the live oracle re-verifies (client code AND its committed + * testdata fixtures). Deliberately NOT keyed on ci.yml: the live oracle is the + * expensive, third-party-dependent EXTRA verification on top of the untagged + * golden-replay battery (leg 1) that already asserts the client contract on + * every PR with zero network, so it should not fire on unrelated PRs that merely + * touch this workflow file (RIG-2909: a docs/CI-only PR editing ci.yml was + * running the whole live oracle and flaking on a Linear API blip). Oracle-wiring + * changes are still covered: every push to main and every schedule full-sweeps + * the oracle unconditionally (isFullSweep below). + */ +const FORGE_PATH_RE = /^go\/internal\/forge\//; /** * gtk4 trigger: any changed path under go/cmd/compass-app/, OR one of the * shared GTK closure inputs. The e2e lane is the ONLY CI lane that compiles the