fix(ci): fall back to collaborator permission API in e2e/functional gate

[ralphbean]

Summary

• The author_association field in pull_request_target event payloads misreports org members with private membership visibility — returning CONTRIBUTOR or NONE instead of MEMBER. This blocked e2e and functional tests for maintainers like @maruiz93 on feat(sandbox): adopt provider-backed policy composition #2671.
• Adds a has_write_permission fallback using the collaborator permission API (repos/{owner}/{repo}/collaborators/{user}/permission), which resolves correctly regardless of visibility. Works with existing GITHUB_TOKEN permissions (contents: read).
• Follows the same pattern already adopted for agent dispatch authorization in feat(#1662): ADR 0054 — implement is_authorized on all agent dispatch paths #1688.

Test plan

• All 28 existing + new authorization tests pass (bash scripts/check-e2e-authorization-test.sh)
• Verify on feat(sandbox): adopt provider-backed policy composition #2671: re-run e2e gate after merge to confirm maruiz93 is authorized

🤖 Generated with Claude Code

[qodo-code-review]

PR Summary by Qodo

Fix e2e/functional CI gate by falling back to collaborator permission API
🐞 Bug fix 🧪 Tests ⚙️ Configuration changes 🕐 20-40 Minutes

Description

• Pass PR author login into the e2e/functional authorization gate for permission lookups.
• Authorize maintainers with private org membership via collaborator permission API (write+).
• Add regression tests covering collaborator fallback, failure modes, and ok-to-test behavior.

Diagram

graph TD
  A["pull_request_target (CI)"] --> B["e2e.yml + functional-tests.yml"] --> C["check-e2e-authorization action"] --> D["check-e2e-authorization.sh"] --> E["Fast path: author_association"]
  D --> F["Fallback: collaborator permission"] --> G[("GitHub REST API")]
  D --> H["Label path: ok-to-test freshness"]

Loading

High-Level Assessment

The following are alternative approaches to this PR:

1. Require org membership visibility / read:org-based checks

• ➕ Could directly verify org membership rather than repo permission
• ➖ Not compatible with default GITHUB_TOKEN permissions in pull_request_target
• ➖ Still brittle for private membership visibility and permission scoping

2. Static allowlist of maintainers/logins in the gate

• ➕ No external API calls; deterministic behavior
• ➖ High maintenance burden; easy to drift from real permissions
• ➖ Doesn’t generalize to new maintainers or repo collaborators

3. Use GitHub team/CODEOWNERS-based authorization

• ➕ More policy-driven; aligns with ownership models
• ➖ Requires additional API scopes and more complex resolution logic
• ➖ Harder to reason about for cross-org collaborators; more moving parts than needed

Recommendation: Keep the collaborator permission API fallback. It directly answers the gating question (does the PR author have write+ access to this repo?) and avoids the known unreliability of author_association for private org membership while staying within existing token permissions. The added tests meaningfully reduce regression risk.

Files changed (5) +112 / -5

Bug fix (1) +33 / -4

check-e2e-authorization.shAdd collaborator permission fallback for trusted author detection +33/-4

Add collaborator permission fallback for trusted author detection

• Documents the private-membership author_association misreporting issue and adds has_write_permission() using the collaborator permission API. When author_association is untrusted, the script now authorizes PRs if the API reports admin/maintain/write for the PR author login, otherwise continuing to the ok-to-test label path.

scripts/check-e2e-authorization.sh

Tests (1) +72 / -1

check-e2e-authorization-test.shMock collaborator permission API and add fallback authorization tests +72/-1

Mock collaborator permission API and add fallback authorization tests

• Extends the gh API mock to support /collaborators/{user}/permission responses driven by a temporary role file. Adds comprehensive test cases for write+ authorization, read denial, API failure fall-through to ok-to-test, and skipping fallback when PR_AUTHOR_LOGIN is missing.

scripts/check-e2e-authorization-test.sh

Other (3) +7 / -0

action.ymlAdd PR author login input and export to gate script +5/-0

Add PR author login input and export to gate script

• Introduces a new optional input for the PR author login and passes it through as PR_AUTHOR_LOGIN to the authorization script. This enables downstream permission-based fallback when author_association is unreliable.

.github/actions/check-e2e-authorization/action.yml

e2e.ymlWire PR author login into the e2e authorization gate +1/-0

Wire PR author login into the e2e authorization gate

• Adds github.event.pull_request.user.login as an input to the check-e2e-authorization composite action. This ensures the gate has the login needed for collaborator permission fallback.

.github/workflows/e2e.yml

functional-tests.ymlWire PR author login into the functional test authorization gate +1/-0

Wire PR author login into the functional test authorization gate

• Adds github.event.pull_request.user.login as an input to the check-e2e-authorization composite action used by the functional test workflow. Keeps functional gating behavior consistent with e2e gating.

.github/workflows/functional-tests.yml

[github-actions]

Site preview

Preview: https://6ae4b01b-site.fullsend-ai.workers.dev

Commit: 19d93825c6974a18d274ba145e65cd3ed65960a6

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

[fullsend-ai-review]

🤖 Finished Review · ✅ Success · Started 6:32 PM UTC · Completed 6:43 PM UTC
Commit: 19d9382 · View workflow run →

[qodo-code-review]

Code Review by Qodo

🐞 Bugs (1) 📘 Rule violations (0) 📜 Skill insights (0)

Context used

✅ Compliance rules (platform): 51 rules

✅ Skills: writing-user-docs, writing-adrs

1. Silent permission check failures 🐞 Bug ◔ Observability

Description

The new has_write_permission() path suppresses all stderr from the collaborator permission API (and
jq), so transient API errors or token/permission misconfigurations are silently treated as “no
permission” and can keep private org members blocked without any diagnostic signal in logs.

Code

scripts/check-e2e-authorization.sh[R61-68]

+has_write_permission() {
+  local username="${1:-}"
+  if [[ -z "${username}" ]]; then
+    return 1
+  fi
+  local perm_json role
+  perm_json=$(gh api "repos/${REPOSITORY}/collaborators/${username}/permission" 2>/dev/null) || return 1
+  role=$(jq -r '.role_name' <<<"${perm_json}") || return 1

Relevance

⭐⭐⭐ High

PR #1688 accepted stderr-capture + ::warning:: for collaborator permission API errors (fail-closed,
diagnosable).

PR-#1688

ⓘ Recommendations generated based on similar findings in past PRs

Evidence

The fallback currently discards stderr from the collaborator permission API and from jq parsing, so
failures cannot be distinguished from a legitimate non-write result. Elsewhere in the repo, the same
endpoint is wrapped with stderr capture + ::warning::, demonstrating an established pattern for
making authorization failures diagnosable without breaking flow control.

scripts/check-e2e-authorization.sh[28-48]
scripts/check-e2e-authorization.sh[61-73]
scripts/check-e2e-authorization.sh[89-99]
.github/workflows/reusable-dispatch.yml[111-136]
PR-#1688

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
`scripts/check-e2e-authorization.sh` adds `has_write_permission()` as a fallback authorization signal, but it redirects `gh api` stderr to `/dev/null` and the call site also redirects stderr. This makes failures of the collaborator permission API (rate limits, 403s, schema changes, jq parse errors) indistinguishable from a normal “not authorized” result, and the gate can continue to deny eligible private org members with no actionable logs.

### Issue Context
The repo already uses a more diagnosable pattern for the same permission endpoint in the dispatch workflow (captures stderr + emits `::warning::`), while still treating the permission check as a non-fatal boolean.

### Fix Focus Areas
- scripts/check-e2e-authorization.sh[58-73]
- scripts/check-e2e-authorization.sh[89-97]

### Suggested change (behavior-preserving)
1. Update `has_write_permission()` to:
  - Use `--jq '.role_name'` (or equivalent) to avoid a separate `jq` parse step.
  - Capture stderr to a temp file (guard `mktemp`), and on failure emit a `::warning::` **only for unexpected errors** (e.g., suppress warning for common “not a collaborator/404” cases to avoid log spam).
  - Clean up the temp file.
2. Remove the redundant `2>/dev/null` at the call site so warnings (when emitted) are actually visible.

This keeps the current fallback semantics (failure => return non-zero and continue to ok-to-test path) while making real outages/misconfigurations diagnosable.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

[fullsend-ai-review]

Review

Findings

High

• [protected-path] .github/, scripts/ — All 5 files modified in this PR are under protected paths. Protected files: .github/actions/check-e2e-authorization/action.yml, .github/workflows/e2e.yml, .github/workflows/functional-tests.yml, scripts/check-e2e-authorization-test.sh, scripts/check-e2e-authorization.sh. The PR references feat(sandbox): adopt provider-backed policy composition #2671 (another PR) and feat(#1662): ADR 0054 — implement is_authorized on all agent dispatch paths #1688 (prior art) but has no linked issue using Closes/Fixes syntax. Human approval is required for protected-path changes.
  Remediation: Create a GitHub issue documenting the author_association private membership bug and link it to this PR. Human review and approval is still required regardless.

---

Labels: PR modifies CI e2e/functional authorization gate scripts and workflows.

[fullsend-ai-review]

See the review comment for full details.

[ifireball]

nice that we can finally fix it.

[ralphbean]

Thanks for the reviews, all!

[fullsend-ai-retro]

🤖 Finished Retro · ❌ Failure · Started 5:29 PM UTC · Completed 5:36 PM UTC
Commit: 19d9382 · View workflow run →