From dacc3d7533f1b627a84e92c5dabdaede35bfda8b Mon Sep 17 00:00:00 2001 From: fullsend-code <278716306+fullsend-ai-coder[bot]@users.noreply.github.com> Date: Thu, 27 Aug 2026 21:37:40 +0000 Subject: [PATCH] ci(#6701): wait for permission propagation after pool repo recreate After a pool repo is deleted and recreated during behaviour test allocation, the GitHub App installation's permission graph for the new repo ID is a separate consistency domain from repo visibility. awaitCreation only waits until GetRepo stops returning 404. Without a permission wait, dispatch's GetCollaboratorPermission can 404, causing the actor to be treated as RoleNone and the harness matrix to be empty. Add awaitPermissionPropagation to ensure.go, called after ensureRepoExists in doEnsure. It polls GetCollaboratorPermission for the authenticated bot user with the same bounded exponential backoff (resetMaxAttempts=5, doubling delay) that awaitCreation and awaitDeletion use. When the forge client does not implement GitHubExtensions (e.g. GitLab), the check is skipped. Unit tests cover: immediate success, retry-until-confirmed, timeout after max attempts, non-NotFound error propagation, context cancellation, and skip when GitHubExtensions is absent. Closes #6701 --- pkg/behaviourtest/drivers/install/ensure.go | 60 +++++++ .../drivers/install/ensure_test.go | 166 ++++++++++++++++++ 2 files changed, 226 insertions(+) diff --git a/pkg/behaviourtest/drivers/install/ensure.go b/pkg/behaviourtest/drivers/install/ensure.go index 9f22d0018e..85d17e0140 100644 --- a/pkg/behaviourtest/drivers/install/ensure.go +++ b/pkg/behaviourtest/drivers/install/ensure.go @@ -151,6 +151,14 @@ func (e *repoEnsurer) doEnsure(ctx context.Context, org, repoName string) error return err } + // Step 2b: wait for the App installation's permission graph to + // settle for the new repo ID. GitHub's permission consistency is + // a separate domain from repo visibility; dispatch's + // GetCollaboratorPermission can 404 until propagation completes. + if err := e.awaitPermissionPropagation(ctx, org, repoName, target); err != nil { + return err + } + // Step 3: the repo is always freshly created (step 1 deleted any // prior version), so fullsend is never pre-installed. Run the full // install flow and settle for Actions readiness. @@ -317,6 +325,58 @@ func (e *repoEnsurer) awaitCreation(ctx context.Context, org, repoName, target s return fmt.Errorf("repo %s not visible after %d attempts following creation", target, resetMaxAttempts) } +// awaitPermissionPropagation polls GetCollaboratorPermission with +// exponential backoff until the authenticated user's permission is +// visible on the newly created repo. After a repo is deleted and +// recreated, the GitHub App installation's permission graph for the +// new repo ID is a separate consistency domain from repo visibility. +// Without this wait, dispatch's collaborator-permission lookup can +// 404, causing the actor to be treated as unauthorized (RoleNone) +// and the harness matrix to be empty. +// +// When the forge client does not implement GitHubExtensions (e.g., a +// GitLab backend), the check is skipped — permission propagation is a +// GitHub-specific concern. +func (e *repoEnsurer) awaitPermissionPropagation(ctx context.Context, org, repoName, target string) error { + gh, ok := e.client.(forge.GitHubExtensions) + if !ok { + e.logf("[ensure] %s: client does not support permission checks, skipping", target) + return nil + } + + login, err := e.client.GetAuthenticatedUser(ctx) + if err != nil { + return fmt.Errorf("resolving bot login for permission wait on %s: %w", target, err) + } + + e.logf("[ensure] waiting for %s permission to propagate for %s", target, login) + delay := resetRetryDelay + for attempt := 1; attempt <= resetMaxAttempts; attempt++ { + if err := ctx.Err(); err != nil { + return fmt.Errorf("context cancelled while waiting for %s permission: %w", target, err) + } + role, permErr := gh.GetCollaboratorPermission(ctx, org, repoName, login) + if permErr == nil { + e.logf("[ensure] %s permission confirmed for %s: role=%s (attempt %d)", target, login, role, attempt) + return nil + } + if !forge.IsNotFound(permErr) { + return fmt.Errorf("checking permission on %s for %s: %w", target, login, permErr) + } + if attempt < resetMaxAttempts { + e.logf("[ensure] %s permission not yet visible for %s, attempt %d/%d — backing off %v", + target, login, attempt, resetMaxAttempts, delay) + select { + case <-ctx.Done(): + return fmt.Errorf("context cancelled while waiting for %s permission: %w", target, ctx.Err()) + case <-time.After(delay): + } + delay *= 2 + } + } + return fmt.Errorf("permission for %s on %s not visible after %d attempts", login, target, resetMaxAttempts) +} + // installFullsend runs inference provision (when a GCP project is // configured) and fullsend github setup for the target repo. func (e *repoEnsurer) installFullsend(_ context.Context, _, _, target string) error { diff --git a/pkg/behaviourtest/drivers/install/ensure_test.go b/pkg/behaviourtest/drivers/install/ensure_test.go index ee86459894..510f0a48ef 100644 --- a/pkg/behaviourtest/drivers/install/ensure_test.go +++ b/pkg/behaviourtest/drivers/install/ensure_test.go @@ -134,6 +134,14 @@ type stubClient struct { // When nil and installed is true, GetWorkflow returns a valid Workflow. getWorkflowErr error getWorkflowCalled atomic.Int32 + + // authenticatedUser is the login returned by GetAuthenticatedUser. + // Defaults to "test-bot[bot]" when empty. + authenticatedUser string + + // collaboratorRole is the role returned by GetCollaboratorPermission. + // Defaults to "admin" when empty (immediate success). + collaboratorRole string } func (s *stubClient) GetRepo(_ context.Context, _, repo string) (*forge.Repository, error) { @@ -202,6 +210,32 @@ func (s *stubClient) GetWorkflow(_ context.Context, _, _, _ string) (*forge.Work return &forge.Workflow{ID: 1, Name: "fullsend", Path: ".github/workflows/fullsend.yaml", State: "active"}, nil } +func (s *stubClient) GetAuthenticatedUser(_ context.Context) (string, error) { + if s.authenticatedUser != "" { + return s.authenticatedUser, nil + } + return "test-bot[bot]", nil +} + +// GitHubExtensions methods — needed so the type assertion in +// awaitPermissionPropagation succeeds for stubClient. + +func (s *stubClient) ListOrgInstallations(_ context.Context, _ string) ([]forge.Installation, error) { + return nil, nil +} + +func (s *stubClient) GetAppClientID(_ context.Context, _ string) (string, error) { + return "", forge.ErrNotFound +} + +func (s *stubClient) GetCollaboratorPermission(_ context.Context, _, _, _ string) (string, error) { + role := s.collaboratorRole + if role == "" { + role = "admin" + } + return role, nil +} + func TestNewRepoEnsurer_ReturnsNonNil(t *testing.T) { sc := &stubClient{} e := newRepoEnsurer(e2etest.EnvConfig{}, sc, "tok", "/bin/true", t.Logf) @@ -1017,3 +1051,135 @@ func TestAwaitCreation_ContextCancellation(t *testing.T) { require.Error(t, err) assert.Contains(t, err.Error(), "context cancelled") } + +// --- awaitPermissionPropagation unit tests --- + +// countingPermissionClient returns different GetCollaboratorPermission +// results after a configurable number of calls. It also implements +// GitHubExtensions so the type assertion in awaitPermissionPropagation +// succeeds. +type countingPermissionClient struct { + forge.Client + mu sync.Mutex + permCalls int + switchAfter int // after this many calls, switch to switchedRole + initialErr error // returned for calls 1..switchAfter + switchedRole string // returned for calls switchAfter+1.. + switchedErr error // returned for calls switchAfter+1.. + authenticatedAs string // login returned by GetAuthenticatedUser +} + +func (c *countingPermissionClient) GetAuthenticatedUser(_ context.Context) (string, error) { + if c.authenticatedAs != "" { + return c.authenticatedAs, nil + } + return "test-bot[bot]", nil +} + +func (c *countingPermissionClient) ListOrgInstallations(_ context.Context, _ string) ([]forge.Installation, error) { + return nil, nil +} + +func (c *countingPermissionClient) GetAppClientID(_ context.Context, _ string) (string, error) { + return "", forge.ErrNotFound +} + +func (c *countingPermissionClient) GetCollaboratorPermission(_ context.Context, _, _, _ string) (string, error) { + c.mu.Lock() + defer c.mu.Unlock() + c.permCalls++ + if c.permCalls > c.switchAfter { + return c.switchedRole, c.switchedErr + } + return "", c.initialErr +} + +func TestAwaitPermissionPropagation_ImmediateSuccess(t *testing.T) { + speedUpResetRetries(t) + client := &countingPermissionClient{ + switchAfter: 0, + switchedRole: "write", + } + e := &repoEnsurer{client: client, logf: t.Logf} + + err := e.awaitPermissionPropagation(context.Background(), "org", "repo", "org/repo") + require.NoError(t, err) + assert.Equal(t, 1, client.permCalls, "should confirm on first poll") +} + +func TestAwaitPermissionPropagation_RetriesUntilConfirmed(t *testing.T) { + speedUpResetRetries(t) + client := &countingPermissionClient{ + switchAfter: 3, + initialErr: forge.ErrNotFound, // 404 for 3 calls + switchedRole: "write", // then found + } + e := &repoEnsurer{client: client, logf: t.Logf} + + err := e.awaitPermissionPropagation(context.Background(), "org", "repo", "org/repo") + require.NoError(t, err) + assert.Equal(t, 4, client.permCalls, "should retry until permission visible") +} + +func TestAwaitPermissionPropagation_FailsAfterMaxAttempts(t *testing.T) { + speedUpResetRetries(t) + client := &countingPermissionClient{ + switchAfter: resetMaxAttempts + 1, // never switches + initialErr: forge.ErrNotFound, // always 404 + } + e := &repoEnsurer{client: client, logf: t.Logf} + + err := e.awaitPermissionPropagation(context.Background(), "org", "repo", "org/repo") + require.Error(t, err, "should error when permission never becomes visible") + assert.Contains(t, err.Error(), "not visible after") + assert.Contains(t, err.Error(), "test-bot[bot]") +} + +func TestAwaitPermissionPropagation_PropagatesNonNotFoundError(t *testing.T) { + speedUpResetRetries(t) + client := &countingPermissionClient{ + switchAfter: 0, + initialErr: assert.AnError, + switchedErr: assert.AnError, + } + e := &repoEnsurer{client: client, logf: t.Logf} + + err := e.awaitPermissionPropagation(context.Background(), "org", "repo", "org/repo") + require.Error(t, err) + assert.Contains(t, err.Error(), "checking permission") +} + +func TestAwaitPermissionPropagation_ContextCancellation(t *testing.T) { + speedUpResetRetries(t) + client := &countingPermissionClient{ + switchAfter: resetMaxAttempts + 1, + initialErr: forge.ErrNotFound, + } + e := &repoEnsurer{client: client, logf: t.Logf} + + ctx, cancel := context.WithCancel(context.Background()) + cancel() // cancel immediately + + err := e.awaitPermissionPropagation(ctx, "org", "repo", "org/repo") + require.Error(t, err) + assert.Contains(t, err.Error(), "context cancelled") +} + +// noGHExtClient satisfies forge.Client but does NOT implement +// GitHubExtensions. Used to verify awaitPermissionPropagation skips +// the check for non-GitHub forges. +type noGHExtClient struct { + forge.Client +} + +func (c *noGHExtClient) GetAuthenticatedUser(_ context.Context) (string, error) { + return "bot", nil +} + +func TestAwaitPermissionPropagation_SkipsWhenNoGitHubExtensions(t *testing.T) { + client := &noGHExtClient{} + e := &repoEnsurer{client: client, logf: t.Logf} + + err := e.awaitPermissionPropagation(context.Background(), "org", "repo", "org/repo") + require.NoError(t, err, "should skip gracefully when client lacks GitHubExtensions") +}