From 9fc3b3d23cc81e9bb159bb67f000ead4abde565b Mon Sep 17 00:00:00 2001 From: Joseph Schorr Date: Wed, 30 Sep 2026 23:45:38 -0400 Subject: [PATCH] Fix the stale-read flake in TestRunnerAuthz_ReadonlyAllow MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The test wrote a relationship and immediately checked it through a cacheless Checker. With no ZedToken cache there is no freshness floor, so CheckToolCall runs the check at MinimizeLatency — which is free to serve a quantized snapshot predating the write this same test just made (the consistency notes in CheckToolCall spell this out). The just-granted reader is then invisible and the expected allow comes back as a deny. This is not hypothetical: the test failed 5 of 30 local runs and took down two consecutive CI integration jobs (this branch's base and PR one of its descendants) with the identical denial message. The deny sibling never flakes because a stale read still denies, and the ZedTokenFreshness sibling already floors its check. Seed a ZedTokenCache with the WriteRelationships response token so the check runs AtLeastAsFresh at the write's revision — the same pattern TestRunnerAuthz_ZedTokenFreshness uses. What this test claims is the readonly allow path, not floorless freshness. 60 of 60 local runs pass with the floor in place. --- pkg/authz/spicedb/toolcheck/integration_test.go | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/pkg/authz/spicedb/toolcheck/integration_test.go b/pkg/authz/spicedb/toolcheck/integration_test.go index 6d1247d8..a376dbbd 100644 --- a/pkg/authz/spicedb/toolcheck/integration_test.go +++ b/pkg/authz/spicedb/toolcheck/integration_test.go @@ -92,7 +92,7 @@ func TestRunnerAuthz_ReadonlyAllow(t *testing.T) { defer cancel() // Grant alice reader on repo:foo. - _, err := perm.WriteRelationships(ctx, &v1.WriteRelationshipsRequest{ + wresp, err := perm.WriteRelationships(ctx, &v1.WriteRelationshipsRequest{ Updates: []*v1.RelationshipUpdate{{ Operation: v1.RelationshipUpdate_OPERATION_TOUCH, Relationship: &v1.Relationship{ @@ -104,7 +104,16 @@ func TestRunnerAuthz_ReadonlyAllow(t *testing.T) { }) require.NoError(t, err, "WriteRelationships") - res := toolcheck.Checker{Cli: cli}.CheckToolCall(ctx, + // Floor the check at the write above. A cacheless Checker checks at + // MinimizeLatency, which is free to serve a quantized snapshot that + // predates the write (see the consistency notes in CheckToolCall) — + // correct in production, but here it intermittently turns this allow + // into a deny. What this test claims is the readonly allow path, not + // floorless freshness, so pin the revision like production callers do. + cache := toolcheck.NewZedTokenCache() + cache.Set("github_repo", "foo", wresp.WrittenAt.Token) + + res := toolcheck.Checker{Cli: cli, Cache: cache}.CheckToolCall(ctx, authz.Permission{ StateImpact: authz.Readonly, Check: &authz.PermissionCheck{