Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
27 changes: 17 additions & 10 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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: >-
Expand Down Expand Up @@ -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
Expand All @@ -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"
Expand Down
90 changes: 68 additions & 22 deletions go/internal/forge/livegithub_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ import (
"errors"
"fmt"
"io"
"net"
"net/http"
"os"
"path/filepath"
Expand Down Expand Up @@ -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)
Expand All @@ -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)
}
Expand Down Expand Up @@ -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)
Expand All @@ -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)
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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)
}
Expand All @@ -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 <!--owner--> body"})
issue, err := createWithBackoff(ctx, func() (Issue, error) {
return setup.CreateIssue(ctx, team, CreateIssue{Title: "compass-live-get-" + newRunID(), Body: "raw <!--owner--> body"})
})
if err != nil {
t.Fatalf("CreateIssue (setup): %v", err)
}
Expand All @@ -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)
}
Expand All @@ -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)
}
Expand Down
22 changes: 19 additions & 3 deletions tools/ci-matrix/index.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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({
Expand Down
16 changes: 13 additions & 3 deletions tools/ci-matrix/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Loading