ci(#6025): provision test actor permissions in pool org setup

[fullsend-ai-coder]

Summary

Extend the pool org setup script to idempotently provision test actor permissions (write/triage/outsider) and document the permission model in the e2e testing guide.

Related Issue

Changes

• Add fstest-write, fstest-triage, fstest-outsider user variables to hack/setup-new-e2e-org.sh
• Add section 3b: check/create org membership for write and triage actors, with PAT-based auto-accept (TEST_ACTOR_WRITE_PAT, TEST_ACTOR_TRIAGE_PAT) and manual fallback; verify outsider has no membership
• Add section 3c: grant push/triage collaborator permissions on all non-fork test-repo* repos; verify outsider has no collaborator access
• Update docs/guides/dev/e2e-testing.md pool org provisioning checklist (add item 3, renumber)
• Add "Test actor permissions" subsection documenting the permission model table, PAT env vars, and link to test(e2e): create write, triage, and outsider test accounts with PATs and stored credentials #6024

Testing

• shellcheck passes on hack/setup-new-e2e-org.sh
• Secret scan passes
• Script follows existing idempotent patterns (membership check → invite → accept → verify)
• Manual: run updated script against a pool org to verify grants

---

Closes #6025

Post-script verification

• Branch is not main/master (agent/6025-pool-org-test-actors)
• Secret scan passed (gitleaks — 743f99d88b883f61b19f09d00b723f7a3f8ae4f8..HEAD)
• PR body secret scan passed (gitleaks — no-git)

[fullsend-ai-review]

🤖 Finished Review · ✅ Success · Started 11:02 AM UTC · Completed 11:19 AM UTC

Commit: 2568747 · View workflow run →

[github-actions]

Site preview

Preview: https://0e1a2aac-site.fullsend-ai.workers.dev

Commit: d080aae038b5c1e224285a406eafd6faf0daf3e0

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

[fullsend-ai-review]

Review

Findings

Low

• [error-handling] hack/setup-new-e2e-org.sh — When the gh api --paginate call to list org repos fails (auth issue, network error), the script prints a WARNING with the error and sets base_repos to empty string, which then triggers the "No base test-repo* repos found" message. This secondary message is misleading when repos exist but the API call failed. The WARNING line immediately above does surface the actual error. Consider using a distinct message when the API call failed (e.g., "Skipping collaborator grants due to API error above").

• [fail-open] hack/setup-new-e2e-org.sh — The outsider non-membership check (org membership and repo collaborator) logs a WARNING but does not exit on failure. If fstest-outsider accidentally has org membership or collaborator access, the script continues, which could produce false-positive e2e test results. This is a test fidelity concern rather than a privilege escalation, and the idempotent design makes a hard failure less appropriate here.

Previous run

Review

Findings

Low

• [error-handling] hack/setup-new-e2e-org.sh — When the gh api --paginate call to list org repos fails entirely (auth issue, network error, insufficient permissions), || true makes base_repos empty and the script prints "No base test-repo* repos found. Skipping collaborator grants" — misleading when repos exist but the API call failed. The operator may not realize collaborator grants were skipped due to an API failure rather than genuinely missing repos.

• [error-messaging] hack/setup-new-e2e-org.sh — The new FAIL: message prefix for non-fatal grant failures is not seen in the existing script, which uses ERROR: (always followed by exit 1), MISSING:, and SKIP:. The semantic distinction (non-fatal batch failure vs hard exit) is valid, but the convention is undocumented — consider aligning or documenting the intent.

• [terminology-consistency] docs/guides/dev/e2e-testing.md — "Elevated access uses direct collaborator overrides (no teams)" — the term "overrides" is technically accurate (GitHub direct collaborator permissions override team-based permissions) but may confuse readers unfamiliar with GitHub's permission model. Consider "direct collaborator grants (not team membership)" for clarity.

---

Labels: PR modifies e2e pool org setup script and e2e testing documentation

[ifireball]

/fs-fix

[fullsend-ai-coder]

🤖 Finished Fix · ✅ Success · Started 6:01 AM UTC · Completed 6:08 AM UTC

Commit: 2568747 · View workflow run →

[fullsend-ai-coder]

🔧 Fix agent — iteration 1 (human-triggered)

Addressed all 3 review findings: surfaced API errors instead of masking them, aligned message prefixes with script conventions, and clarified docs terminology.

Fixed (3):

1. API failure masked by || true in repo listing (hack/setup-new-e2e-org.sh): Replaced fail-open || true with if ! pattern that captures stderr and warns the operator when the gh api --paginate call fails, distinguishing API errors from genuinely empty results
2. FAIL: prefix inconsistent with script conventions (hack/setup-new-e2e-org.sh): Changed FAIL: to WARNING: for non-fatal collaborator grant failures, aligning with the existing convention where ERROR: is fatal (with exit 1) and WARNING: is non-fatal
3. overrides terminology confusing in docs (docs/guides/dev/e2e-testing.md): Changed 'direct collaborator overrides (no teams)' to 'direct collaborator grants (not team membership)' for clarity

Tests: passed

Next steps:

• /fs-review — request a re-review of the changes
• /fs-fix <your instruction> — run another fix pass with specific guidance
• Push commits directly — review re-runs automatically on push
  _{Updated by fullsend fix agent}

[fullsend-ai-review]

🤖 Finished Review · ✅ Success · Started 6:09 AM UTC · Completed 6:22 AM UTC

Commit: d080aae · View workflow run →

[fullsend-ai-retro]

🤖 Finished Retro · ✅ Success · Started 5:44 AM UTC · Completed 5:58 AM UTC

Commit: d080aae · View workflow run →

[fullsend-ai-retro]

Retro: PR #6032 — provision test actor permissions in pool org setup

Verdict: Workflow went well. The full agent pipeline (triage → code → review → fix → re-review → merge) executed smoothly with one fix cycle. The code agent produced working code in a single pass, the review agent found 3 genuine low-severity issues, the fix agent addressed all 3 in one pass, and the re-review confirmed the fixes while correctly accepting 2 residual polish-level items. The human reviewer approved without needing to add anything. No new proposals are warranted — the improvement opportunities identified are already tracked by existing open issues.

Timeline

1. Aug 10, 07:58 — Triage agent auto-analyzed issue #6025, identified blocker on test(e2e): create write, triage, and outsider test accounts with PATs and stored credentials #6024
2. Aug 10, 09:23 — Human re-triggered triage after blocker resolved; triage agent updated plan
3. Aug 10, 10:51 — Human triggered /fs-code; code agent created PR #6032 in ~10 min
4. Aug 10, 11:19 — Review agent approved with 3 low-severity findings (error-handling, message-prefix, terminology)
5. Aug 11, 06:00 — Human triggered /fs-fix; fix agent addressed all 3 findings in ~8 min
6. Aug 11, 06:22 — Re-review approved with 2 residual low findings (correctly accepted as polish-level)
7. Aug 12, 05:41 — Human approved and merged

What worked well

• Review quality was high. All 3 findings were genuine issues: || true masking API errors, inconsistent FAIL: prefix, and unclear docs terminology. No false positives.
• Fix agent was effective. Addressed all 3 findings correctly in a single pass — no rework needed.
• Re-review calibration was good. The 2 residual findings were correctly classified as acceptable polish items, not blockers.
• Human-agent alignment. The human approved without comments, indicating agreement with the agent's assessment.

Improvement opportunities (all tracked by existing issues)

Code agent didn't read shell scripting conventions. The code agent modified hack/setup-new-e2e-org.sh but did not follow the AGENTS.md conditional pointer to read docs/contributing/shell-scripting.md (which covers the exact || true anti-pattern and the if ! capture alternative). Two of three review findings would have been avoided. This is a 5-step inference chain (CLAUDE.md → AGENTS.md → conditional table → self-classify task → read guide) where the agent missed one link.

• Evidence supports fullsend#596 (code agent does not apply AGENTS.md instructions) — the agent skipped the conditional guide despite clearly writing shell scripts
• Evidence supports fullsend#2414 (code agent should study sibling files for established patterns) — the FAIL: prefix issue stems from not scanning existing message conventions in the target file
• Evidence supports fullsend#4021 (review agent approved || true after grep) — the code agent used || true on a repo-listing call, exactly the pattern that guide warns against

Review findings posted as top-level comments, not inline. All review findings were in issue-level comments rather than inline on the diff. This is likely related to the 422 stale-diff fallback tracked in agents#760 (filed Aug 11) and fullsend#6039 (investigate 100% inline comment posting failure).