From 7eeb4c832f7e3b02eacdf368b6e6f834dcb0ecd9 Mon Sep 17 00:00:00 2001 From: mintaka Date: Fri, 11 Sep 2026 11:13:09 -0400 Subject: [PATCH 1/2] test(forge): pin the one-shared-Linear-TokenSource invariant (RIG-3135) The forge Linear notify lane and write coordinate MUST ride exactly one linearagent.TokenSource (DEC-4). Linear revokes a client-credentials app's tokens when its scope set changes, and the mint singleflight coalesces only WITHIN an instance, so a second source is two mints racing one credential -- each revoking the other's live token. That invariant was held by code inspection only: a refactor building a source per site passed CI. Unlike the GitHub lanes, which share a whole *forge.GitHub and are pinned by TestForgeLanesShareOneBudgetGate, the two Linear sinks each build their OWN *forge.Linear. Only the source inside is shared, so client identity proves nothing and forge.Linear.token is unexported. Adds a read-only TokenSourceForTest accessor -- the smallest seam that makes it observable, matching the existing forgeNotifyLane.reader recorded-for-tests pattern, and no injectable OAuth endpoint or pgtest e2e as the issue had assumed. The test reads the source each BUILDER threaded into the client it produced, never a handle the test holds, and asserts the two clients are distinct objects so the pair is not trivially true. Mutation-proved against the exact regression, twice: minting a separate source at the write coordinate fails the write arm, and at the notify lane fails the notify arm. Refs RIG-3090. Co-authored-by: Matt Wilkinson --- go/internal/forge/linear.go | 8 ++++ go/server/serve_forge_test.go | 75 +++++++++++++++++++++++++++++++++++ 2 files changed, 83 insertions(+) diff --git a/go/internal/forge/linear.go b/go/internal/forge/linear.go index 5a3b22363..352621310 100644 --- a/go/internal/forge/linear.go +++ b/go/internal/forge/linear.go @@ -144,6 +144,14 @@ var _ Provider = (*Linear)(nil) // --- Provider: exported methods ---------------------------------------------- +// TokenSourceForTest exposes the TokenSource this client was built over, so a +// server-package test can assert two independently-built Linear clients ride +// ONE shared source (DEC-4's one-instance rule). Production reads it never: +// Linear revokes a client-credentials app's tokens on a scope-set change, and +// the mint singleflight coalesces only WITHIN an instance, so a second source +// is a live credential hazard no other signal catches (RIG-3135). +func (l *Linear) TokenSourceForTest() TokenSource { return l.token } + // Name identifies this provider. func (l *Linear) Name() string { return "linear" } diff --git a/go/server/serve_forge_test.go b/go/server/serve_forge_test.go index 419f4cd71..76072828c 100644 --- a/go/server/serve_forge_test.go +++ b/go/server/serve_forge_test.go @@ -21,6 +21,8 @@ import ( "testing" "time" + compassv1 "github.com/RigelBuild/compass/go/gen/compass/v1" + "github.com/RigelBuild/compass/go/internal/forge" "github.com/RigelBuild/compass/go/internal/linearagent" "github.com/RigelBuild/compass/go/internal/secrets" ) @@ -634,3 +636,76 @@ func TestLinearWebhookWiringResolvesFromTheServerKeyspace(t *testing.T) { t.Fatal("server fake carries the user secret; the fixtures overlap") } } + +// TestForgeLinearLanesShareOneTokenSource proves the notify lane and the write +// coordinate ride ONE shared *linearagent.TokenSource (DEC-4's one-instance +// rule, RIG-3135). Unlike the GitHub lanes, which share a whole *forge.GitHub, +// the two Linear sinks each build their OWN *forge.Linear — so the only shared +// object is the source inside, and pointer identity on the client would prove +// nothing. The test therefore reads the source each BUILDER threaded into the +// client it produced (notifyLane.reader and the registry's resolved author), +// never a handle the test holds: a builder that minted its own source would +// satisfy every existing test and fail only this one. +// +// Why one instance is load-bearing: Linear revokes a client-credentials app's +// tokens when its scope set changes, and the mint singleflight coalesces only +// WITHIN an instance. Two sources means two independent mints racing one +// credential — each revoking the other's live token. +func TestForgeLinearLanesShareOneTokenSource(t *testing.T) { + ctx := context.Background() // test root + tokens := linearagent.NewTokenSource("cid", "csecret", nil, "") + if tokens == nil { + t.Fatal("NewTokenSource returned nil, want a source to thread") + } + + // (1) The notify lane's Linear reader must wrap the source it was handed. + notifyLane := buildLinearNotifyLane(nil, nil, tokens, slog.Default()) + if notifyLane == nil { + t.Fatal("buildLinearNotifyLane returned nil, want an assembled lane") + } + notifyLinear, ok := notifyLane.reader.(*forge.Linear) + if !ok { + t.Fatalf("notify lane reader is %T, want *forge.Linear", notifyLane.reader) + } + if notifyLinear.TokenSourceForTest() != forge.TokenSource(tokens) { + t.Fatal("notify lane's Linear client rides a different TokenSource than the one passed in") + } + + // (2) The write coordinate must wrap the SAME source. Reaching it means + // driving the real builder: a reviewer-app key the fake resolver satisfies, + // and a non-nil primary client (both are fail-fast gates ahead of the Linear + // registration). + const reviewerKey = "REVIEWER_APP_KEY" + cfg := ServeConfig{Forge: ForgeConfig{ + Host: "github.com", + App: ForgeAppConfig{AppID: 42, InstallationID: 7, AppPrivateKeySecret: "APP_KEY"}, + ReviewerApp: ForgeAppConfig{AppID: 43, InstallationID: 8, AppPrivateKeySecret: reviewerKey}, + }} + resolver := &fakeResolver{resolved: []secrets.ResolvedSecret{ + {Name: serverSecretName(reviewerKey), Value: "key"}, + }} + primary := forge.NewGitHub(forge.GitHubConfig{Host: "github.com", Token: staticTokenSource{}}) + + svc, err := buildForgeWriteService(ctx, cfg, nil, nil, resolver, primary, tokens, slog.Default()) + if err != nil { + t.Fatalf("buildForgeWriteService: %v", err) + } + resolved, ok := svc.providers.resolve(&compassv1.ForgeRef{Provider: compassv1.ForgeProvider_FORGE_PROVIDER_LINEAR}) + if !ok { + t.Fatal("no Linear write coordinate registered with a configured token source") + } + writeLinear, ok := resolved.author.(*forge.Linear) + if !ok { + t.Fatalf("Linear coordinate author is %T, want *forge.Linear", resolved.author) + } + if writeLinear.TokenSourceForTest() != forge.TokenSource(tokens) { + t.Fatal("Linear write coordinate rides a different TokenSource than the notify lane") + } + + // (3) The two clients are genuinely distinct objects, so (1) and (2) are two + // independent reads of one shared source -- not the same client twice, which + // would make the pair trivially true. + if notifyLinear == writeLinear { + t.Fatal("notify and write clients are the same *forge.Linear; the shared-source assertions prove nothing") + } +} From 031e67e47b69ce8001a5e4840f53fe083eed24c3 Mon Sep 17 00:00:00 2001 From: mintaka Date: Fri, 11 Sep 2026 11:40:16 -0400 Subject: [PATCH 2/2] test(forge): ground the one-instance rationale and close the GitHub author hop (RIG-3135) Review found the comments asserting a mechanism the frozen record marks unverified, which is the same defect I shipped one PR ago. I wrote that two TokenSource instances each revoke the other's live token. The documented trigger is a scope-set CHANGE (tokenScope const comment in internal/linearagent/client.go), and tokenScope is pinned, so both sources mint IDENTICAL scope and that trigger cannot fire here. What the record actually says is weaker and is the real reason: 'whether concurrent same-scope mints from independent instances coexist is unverified -- one shared instance removes the question entirely' (docs/designs/server/compass-forge-app-credentials/design.md, T4). Stating an unverified hazard as certain invites a future reader to 'fix' the sharing once they discover same-scope mints do not in fact cross-revoke. Also corrects the citation: the one-instance directive is T4's task text, not DEC-4 (which is the clean-cutover/no-PAT-fallback decision). Drops a dead guard -- NewTokenSource cannot return nil -- and records why the nil store/hub/board args are safe, mirroring the sibling budget test's note. Closes RIG-3135's optional secondary bullet with a fourth arm: the GitHub author role IS the primaryClient the builder was passed. Mutation-proved like the others -- registering a freshly-built client as author fails that arm alone. Co-authored-by: Matt Wilkinson --- go/internal/forge/linear.go | 9 +++++---- go/server/serve_forge_test.go | 33 ++++++++++++++++++++++++--------- 2 files changed, 29 insertions(+), 13 deletions(-) diff --git a/go/internal/forge/linear.go b/go/internal/forge/linear.go index 352621310..b8f78fde3 100644 --- a/go/internal/forge/linear.go +++ b/go/internal/forge/linear.go @@ -146,10 +146,11 @@ var _ Provider = (*Linear)(nil) // TokenSourceForTest exposes the TokenSource this client was built over, so a // server-package test can assert two independently-built Linear clients ride -// ONE shared source (DEC-4's one-instance rule). Production reads it never: -// Linear revokes a client-credentials app's tokens on a scope-set change, and -// the mint singleflight coalesces only WITHIN an instance, so a second source -// is a live credential hazard no other signal catches (RIG-3135). +// ONE shared source (the one-instance rule, DEC-4). Production reads it never. +// The mint singleflight coalesces only WITHIN an instance, so a second source +// mints independently against the same app; whether concurrent same-scope +// mints from independent instances coexist is unverified, and sharing one +// instance removes the question (RIG-3135). func (l *Linear) TokenSourceForTest() TokenSource { return l.token } // Name identifies this provider. diff --git a/go/server/serve_forge_test.go b/go/server/serve_forge_test.go index 76072828c..b8c3e4a9f 100644 --- a/go/server/serve_forge_test.go +++ b/go/server/serve_forge_test.go @@ -638,8 +638,9 @@ func TestLinearWebhookWiringResolvesFromTheServerKeyspace(t *testing.T) { } // TestForgeLinearLanesShareOneTokenSource proves the notify lane and the write -// coordinate ride ONE shared *linearagent.TokenSource (DEC-4's one-instance -// rule, RIG-3135). Unlike the GitHub lanes, which share a whole *forge.GitHub, +// coordinate ride ONE shared *linearagent.TokenSource (the one-instance rule, +// RIG-3135; the directive is the compass-forge-app-credentials T4 task text). +// Unlike the GitHub lanes, which share a whole *forge.GitHub, // the two Linear sinks each build their OWN *forge.Linear — so the only shared // object is the source inside, and pointer identity on the client would prove // nothing. The test therefore reads the source each BUILDER threaded into the @@ -647,16 +648,18 @@ func TestLinearWebhookWiringResolvesFromTheServerKeyspace(t *testing.T) { // never a handle the test holds: a builder that minted its own source would // satisfy every existing test and fail only this one. // -// Why one instance is load-bearing: Linear revokes a client-credentials app's -// tokens when its scope set changes, and the mint singleflight coalesces only -// WITHIN an instance. Two sources means two independent mints racing one -// credential — each revoking the other's live token. +// Why one instance is load-bearing: the mint singleflight coalesces only +// WITHIN an instance, so two sources mint independently against the same app. +// The record (compass-forge-app-credentials T4) marks same-scope coexistence +// UNVERIFIED rather than harmless, and one instance removes the question. The +// documented revocation trigger is a scope-set CHANGE, which cannot fire here: +// tokenScope is a pinned const, so both sources mint identical scope. +// +// Nil store/hub/board are safe: both builders only stash them into structs and +// adapters, and this test never starts the arms/reconcilers that read them. func TestForgeLinearLanesShareOneTokenSource(t *testing.T) { ctx := context.Background() // test root tokens := linearagent.NewTokenSource("cid", "csecret", nil, "") - if tokens == nil { - t.Fatal("NewTokenSource returned nil, want a source to thread") - } // (1) The notify lane's Linear reader must wrap the source it was handed. notifyLane := buildLinearNotifyLane(nil, nil, tokens, slog.Default()) @@ -708,4 +711,16 @@ func TestForgeLinearLanesShareOneTokenSource(t *testing.T) { if notifyLinear == writeLinear { t.Fatal("notify and write clients are the same *forge.Linear; the shared-source assertions prove nothing") } + + // (4) The GitHub author role is the primaryClient this builder was PASSED, + // not one it minted. RIG-3135's secondary bullet: the budget test calls + // registerGitHubForgeCoordinate directly, so this pass-through was the one + // hop covered by inspection alone. + ghResolved, ok := svc.providers.resolve(nil) + if !ok { + t.Fatal("no default GitHub write coordinate registered") + } + if ghResolved.author != forge.Provider(primary) { + t.Fatal("GitHub coordinate author is not the primaryClient passed to buildForgeWriteService") + } }