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") +}