Skip to content

ci(#6701): wait for permission propagation after pool repo recreate - #6709

Closed
fullsend-ai-coder[bot] wants to merge 1 commit into
mainfrom
agent/6701-await-permission-propagation
Closed

ci(#6701): wait for permission propagation after pool repo recreate#6709
fullsend-ai-coder[bot] wants to merge 1 commit into
mainfrom
agent/6701-await-permission-propagation

Conversation

@fullsend-ai-coder

Copy link
Copy Markdown
Contributor

Summary

After pool repos are deleted and recreated during behaviour test allocation (resetRepo), the GitHub App installation's permission graph for the new repo ID takes time to propagate — independently of repo visibility. awaitCreation only waits until GetRepo stops returning 404. Without a permission wait, dispatch's GetCollaboratorPermission 404s on the still-propagating repo, the actor is treated as RoleNone, and the harness matrix is empty.

This adds awaitPermissionPropagation to ensure.go, called between ensureRepoExists and installFullsend in doEnsure. It polls GetCollaboratorPermission for the authenticated bot user with the same bounded exponential backoff (resetMaxAttempts=5, doubling delay from resetRetryDelay) used by awaitCreation and awaitDeletion. When the forge client does not implement GitHubExtensions (e.g. GitLab), the check is skipped gracefully.

Related Issue

Closes #6701

Changes

  • Add awaitPermissionPropagation method on *repoEnsurer with exponential backoff polling of GetCollaboratorPermission
  • Call it in doEnsure after ensureRepoExists (step 2b) and before installFullsend
  • Add GetAuthenticatedUser and GitHubExtensions methods to stubClient test double so existing doEnsure tests pass through the new step transparently
  • Add 7 unit tests: immediate success, retry-until-confirmed, timeout, non-NotFound error propagation, context cancellation, skip-when-no-GitHubExtensions, plus a dedicated countingPermissionClient test double

Testing

  • All existing ensure_test.go tests pass (permission check is transparent via stubClient defaults)
  • 7 new awaitPermissionPropagation unit tests pass
  • 91.7% coverage on new function (above 80% threshold)
  • go vet passes
  • gofmt clean

Closes #6701

Post-script verification

  • Branch is not main/master (agent/6701-await-permission-propagation)
  • Secret scan passed (gitleaks — 96d46c802d4ad44bfea7989835dafb68eeb46195..HEAD)
  • PR body secret scan passed (gitleaks — no-git)

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
@fullsend-ai-coder
fullsend-ai-coder Bot requested a review from a team as a code owner August 27, 2026 21:39
@fullsend-ai-coder fullsend-ai-coder Bot added the ready-for-review Triggers review agent dispatch label Aug 27, 2026
@codecov

codecov Bot commented Aug 27, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

@fullsend-ai-review

fullsend-ai-review Bot commented Aug 27, 2026

Copy link
Copy Markdown

🤖 Finished Review · ✅ Success · Started 9:46 PM UTC · Completed 9:57 PM UTC

Commit: dacc3d7 · View workflow run →

Runtime: claude · Model: opus → claude-opus-4-6 · Effort: high · Cost: $3.93

@fullsend-ai-review fullsend-ai-review Bot added the risk/moderate PR risk: moderate label Aug 27, 2026
@fullsend-ai-review

Copy link
Copy Markdown

Risk Assessment: moderate (2/5)

Details

Small additive change (2 files, 226 lines, 50% test ratio) by a bot author with strong test coverage, but the target files show elevated fix/revert churn (8-10 fix/revert commits in 90 days), lifting the score from low to moderate.

@fullsend-ai-review

Copy link
Copy Markdown

Looks good to me

@fullsend-ai-retro

fullsend-ai-retro Bot commented Aug 29, 2026

Copy link
Copy Markdown

🤖 Finished Retro · ✅ Success · Started 5:57 PM UTC · Completed 6:11 PM UTC

Commit: dacc3d7 · View workflow run →

Runtime: claude · Model: opus → claude-opus-4-6 · Effort: high · Cost: $5.73

@fullsend-ai-retro

Copy link
Copy Markdown

Retro: PR #6709ci(#6701): wait for permission propagation after pool repo recreate

Outcome: Closed without merge. The code agent produced well-structured Go code that followed existing patterns exactly, but the approach was architecturally unsound — it polled permission propagation using the e2e installation token, which operates in a different credential context than dispatch's github.token. A human developer's parallel PR (#6703) made the same mistake, confirming this was a domain knowledge gap rather than a code quality issue.

Timeline

  1. Issue behaviourtest: wait for permission propagation after pool repo recreate (dispatch sees no role, matrix empty) #6701 created (Aug 27 21:19) — pool repo permission propagation race causes empty harness matrix
  2. Triage agent ran successfully (21:19–21:26), applied ready-to-code
  3. Human PR fix(#6701): wait for the actor's role on a recreated pool repo #6703 opened by waynesun09 (21:26) — installation-repository-listing approach
  4. Code agent dispatched simultaneously (21:26–21:40) — created PR ci(#6701): wait for permission propagation after pool repo recreate #6709 with GetCollaboratorPermission polling approach
  5. Review agent reviewed PR ci(#6701): wait for permission propagation after pool repo recreate #6709 (21:45–21:58, $3.93, opus) — ran 5 sub-agents (correctness, security, intent-coherence, style-conventions, docs-currency), all returned clean. Approved with zero findings.
  6. E2E testing on PR fix(#6701): wait for the actor's role on a recreated pool repo #6703 empirically disproved both approaches — the suite's installation token never resolves a role on freshly recreated repos (0/12 over 63 seconds)
  7. PR ci(#6701): wait for permission propagation after pool repo recreate #6709 closed without merge (Aug 29). PR fix(#6701): wait for the actor's role on a recreated pool repo #6703 converted to draft. Issue behaviourtest: wait for permission propagation after pool repo recreate (dispatch sees no role, matrix empty) #6701 labeled needs-design.

Review quality assessment

The review agent's 5 sub-agents all correctly verified code-level quality: patterns matched sibling functions, error handling was thorough, test coverage was complete, no security issues, scope aligned with the issue. The flaw was architectural — the code assumed the e2e bot token could observe the same permission propagation that dispatch's github.token uses. This is a domain-specific insight about credential context separation that neither the agent nor the human caught through code review alone; it required empirical E2E testing.

Parallel work (existing issues)

The code agent created PR #6709 while waynesun09 was simultaneously opening PR #6703 for the same issue. The code agent's existing-PR detection did not catch the human's concurrent PR. This is already tracked by open issues #1485 (reduce false positive rate in existing-PR check, priority/high) and #1655 (existing-PR check non-functional when GH_TOKEN missing, priority/high). This retro provides additional evidence: the code agent was dispatched at 21:26 and PR #6703 was opened at the same timestamp, so even with a functioning check the race window may be too narrow to detect.

Wasted cost

The code agent (~14 min) and review agent (~11 min, $3.93) both produced work on a PR that could never have been merged. Total wasted agent cost was approximately $5–6. The system's safeguards worked — the PR required human review and was not auto-merged — but the agent cycle was a dead end.

Proposals filed

dhshah13 pushed a commit to dhshah13/fullsend that referenced this pull request Sep 8, 2026
…tall pkg

The test suite's e2e installation token and dispatch's per-repo
GITHUB_TOKEN are distinct credential contexts with independent
permission propagation graphs. After pool repo recreation, the
suite can confirm the repo exists (GetRepo) but cannot observe
or predict when dispatch-side collaborator permissions are ready.
This was empirically validated in fullsend-ai#6701: both a human (PR fullsend-ai#6703)
and an agent (PR fullsend-ai#6709) independently attempted suite-side
GetCollaboratorPermission polling and both failed.

Add a package-level doc comment (doc.go) explaining the two-token
model, an inline note on awaitCreation warning against permission
polling, and a cross-reference in the behaviour-testing guide.

Closes fullsend-ai#6757
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ready-for-merge All reviewers approved — ready to merge ready-for-review Triggers review agent dispatch risk/moderate PR risk: moderate

Projects

None yet

Development

Successfully merging this pull request may close these issues.

behaviourtest: wait for permission propagation after pool repo recreate (dispatch sees no role, matrix empty)

2 participants