fix(#1960): include disabled repos in repo-maintenance token scope by ralphbean · Pull Request #1962 · fullsend-ai/fullsend

ralphbean · 2026-06-05T18:27:41Z

Summary

Root cause fix: The repo-maintenance workflow only minted tokens for enabled repos. When the reconcile script tried to check/unenroll disabled repos, API calls failed silently (2>/dev/null), and the script treated them as "already unenrolled" — skipping removal PR creation entirely while reporting success.
Extract saveWorkflowRunDebugInfo so triage and unenrollment share the same log/artifact capture code
Capture repo-maintenance workflow logs unconditionally (not just on failure) so silent-skip problems leave a debug trail
Use ::warning:: annotations for failed runs, ::notice:: for successful ones
Add TestRepoMaintenanceTokenCoversAllRepos to prevent regression

Root cause

repo-maintenance.yml step "Extract enrolled repo names from config" only selected enabled == true repos:

REPOS=$(yq '[... | select(.value.enabled == true) | .key] | join(",")' config.yaml)

This meant the minted token had no permissions for disabled repos. The reconcile script's Phase 2 unenrollment tried gh api repos/$ORG/$REPO/contents/... on disabled repos, which returned 403 (swallowed by 2>/dev/null), causing line 530-533 to conclude "no shim on default branch" and skip the repo.

Fix

The step now collects both enabled and disabled repos for token minting:

ENABLED=$(yq '[... | select(.value.enabled == true) | .key] | join(",")' config.yaml)
DISABLED=$(yq '[... | select(.value.enabled == false) | .key] | join(",")' config.yaml)

Test plan

TestRepoMaintenanceTokenCoversAllRepos — asserts disabled repos are included
go vet -tags e2e ./e2e/admin/ passes
make lint passes
All scaffold tests pass
E2E run on this branch creates the removal PR successfully

Closes #1960

github-actions · 2026-06-05T18:29:06Z

Site preview

Preview: https://61d05b7a-site.fullsend-ai.workers.dev

Commit: e9ec1f2e70e702c489c5242da5ec838b7ca2f778

fullsend-ai-review · 2026-06-05T18:35:46Z

Review

Findings

Low

[annotation-format] e2e/admin/admin_test.go:439 — The saveWorkflowRunDebugInfo function produces malformed GitHub Actions annotations for the file-specific log line. The annotation variable already ends with :: (e.g., ::notice::), so fmt.Fprintf(os.Stderr, "%sfile=%s::...", annotation, ...) produces ::notice::file=path::message instead of the correct ::notice file=path::message. The file= parameter won't be parsed by GitHub Actions, so log file links won't render as clickable annotations.
Remediation: Split the annotation prefix so the file-parameter variant uses the bare command (e.g., ::notice / ::warning ) rather than reusing the ::notice:: form.
[edge-case] internal/scaffold/fullsend-repo/.github/workflows/repo-maintenance.yml:28 — The yq filter select(.value.enabled == true or .value.enabled == false) excludes repos where .enabled is missing, null, or a non-boolean value (e.g., the string "true"). This matches the reconcile script's behavior (which also uses == true / == false), so there is no producer-consumer mismatch. Worth noting for future config.yaml schema changes.

Info

[scope] The PR includes helper extraction (saveWorkflowRunDebugInfo) and unconditional repo-maintenance log capture beyond the minimal yq fix. Both are directly related to diagnosing the silent-failure class described in E2E: unenrollment phase fails — removal PR never appears for test-repo #1960 and are reasonable scope for a bug fix.
[test-integrity] The require.NotNil → if nil { t.Fatal } replacement is functionally equivalent (both call t.FailNow). The new form adds repo-maintenance run conclusion to the failure message, improving debuggability.
[permission-scope] Token scope expansion to include disabled repos is intentional and necessary — the reconcile script's Phase 2 performs write operations (content deletion, PR creation) on disabled repos during unenrollment.

Previous run

Review

Findings

Low

[edge-case] e2e/admin/admin_test.go:595 — The unenrollment debug-info capture takes runs[0] from ListWorkflowRuns without timestamp filtering, unlike the triage test (line 314) which filters by CreatedAt. Since the CLI's disable repos command already watches the repo-maintenance workflow to completion before returning, and each e2e org is isolated, runs[0] is reliably the correct run in practice. Adding a timestamp filter would improve robustness if the test structure changes in the future.

Previous run (2)

Review

Findings

Low

[edge-case] e2e/admin/admin_test.go:595 — The unenrollment debug-info capture takes runs[0] from ListWorkflowRuns without timestamp filtering, unlike the triage test (line 314) which filters by CreatedAt. Since the CLI’s disable repos command already watches the repo-maintenance workflow to completion before returning, and each e2e org is isolated, runs[0] is reliably the correct run in practice. Adding a timestamp filter would improve robustness if the test structure changes in the future. See also: [implementation-coherence] finding at this location.

Info

[annotation-style] e2e/admin/admin_test.go:429 — The new helper uses ::warning:: annotations where the original inline code used ::notice::. The PR description explicitly lists this as an intentional change. Warnings are appropriate for failed workflow runs that need investigation.
[doc-style] e2e/admin/admin_test.go:443 — The downloadRunArtifacts doc comment was shortened from 4 lines to 2, removing implementation details about subdirectory naming and artifact types. The shorter version is adequate but less discoverable for future maintainers.

fullsend-ai-review · 2026-06-05T18:35:47Z

+	// Find the most recent repo-maintenance run so we can capture debug info
+	// if it failed. The CLI already watched it to completion, so it should
+	// exist and be completed.
+	var repoMaintRun *forge.WorkflowRun


[low] edge-case

The unenrollment debug-info capture takes runs[0] from ListWorkflowRuns without timestamp filtering, unlike the triage test which filters by CreatedAt. Since the CLI already watches the workflow to completion and each e2e org is isolated, runs[0] is reliable in practice, but adding a timestamp filter would improve robustness.

Suggested fix: Capture time.Now() before runCLI and filter ListWorkflowRuns results by CreatedAt, matching the pattern used in the triage test.

fullsend-ai-review · 2026-06-05T20:17:41Z

@@ -584,6 +593,20 @@ func runUnenrollmentTest(t *testing.T, env *e2eEnv) {
 		"admin", "disable", "repos", env.org, testRepo, "--yolo")
 	t.Logf("Disable repos output:\n%s", output)



[low] edge-case

The unenrollment debug-info capture takes runs[0] from ListWorkflowRuns without timestamp filtering, unlike the triage test (line 314) which filters by CreatedAt. Since the CLI disable repos command already watches the repo-maintenance workflow to completion before returning, and each e2e org is isolated, runs[0] is reliably the correct run in practice. Adding a timestamp filter would improve robustness if the test structure changes in the future.

fullsend-ai-review · 2026-06-06T19:34:24Z

+	_ = os.MkdirAll(debugDir, 0o755)
+
+	logs, logErr := env.client.GetWorkflowRunLogs(ctx, env.org, forge.ConfigRepoName, run.ID)
+	if logErr != nil {


[low] annotation-format

saveWorkflowRunDebugInfo produces malformed GitHub Actions annotations for file-specific log lines. The annotation variable ends with :: (e.g. ::notice::), so the format string "%sfile=%s::..." produces ::notice::file=path::message instead of the correct ::notice file=path::message. The file= parameter will not be parsed by GitHub Actions.

Suggested fix: Split the annotation prefix so the file-parameter variant uses the bare command (e.g. ::notice / ::warning) rather than reusing the ::notice:: form.

fullsend-ai-review · 2026-06-06T19:34:25Z

-          REPOS=$(yq '[.repos | to_entries[] | select(.value.enabled == true) | .key] | join(",")' config.yaml)
-          if [[ -z "$REPOS" ]]; then
-            echo "::notice::No enabled repos found in config.yaml — nothing to do"
+          # Collect ALL repos (enabled + disabled) so the minted token has


[low] edge-case

The yq filter select(.value.enabled == true or .value.enabled == false) excludes repos where .enabled is missing, null, or a non-boolean value. This matches the reconcile script behavior so there is no mismatch, but is worth noting for future config.yaml schema changes.

Extract saveWorkflowRunDebugInfo from the triage phase so both triage and unenrollment use the same code to fetch workflow logs, download artifacts, and emit GitHub Actions annotations on failure. In the unenrollment phase, find the most recent repo-maintenance run after the CLI returns and capture its logs if it failed. This gives us visibility into why the removal PR never appears. Closes #1960 Signed-off-by: Ralph Bean <rbean@redhat.com> Assisted-by: Claude claude-opus-4-6 <noreply@anthropic.com> Signed-off-by: Ralph Bean <rbean@redhat.com>

The minted token only covered enabled repos, so the reconcile script couldn't access disabled repos during unenrollment. API calls failed silently (2>/dev/null), causing the script to treat them as "already unenrolled" and skip removal PR creation. Also capture repo-maintenance workflow logs unconditionally in the e2e test so silent-skip problems leave a debug trail even when the workflow reports success. Assisted-by: Claude claude-opus-4-6 <noreply@anthropic.com> Signed-off-by: Ralph Bean <rbean@redhat.com>

- Replace two yq queries + bash concatenation with a single query that selects both enabled and disabled repos - Return debugDir from saveWorkflowRunDebugInfo so callers can reference it in failure messages - Consolidate the two removal-PR failure paths into one with conditional repo-maintenance run context - Update test assertion to match the simplified yq query Assisted-by: Claude Opus 4.6 <noreply@anthropic.com> Signed-off-by: Ralph Bean <rbean@redhat.com>

…eters Split annotation prefix into annotationMsg (::notice::) for plain messages and annotationFile (::notice ) for file-parameter annotations. The previous code reused ::notice:: for both, producing malformed annotations like ::notice::file=path::message instead of the correct ::notice file=path::message. Assisted-by: Claude claude-opus-4-6 <noreply@anthropic.com> Signed-off-by: Ralph Bean <rbean@redhat.com>

fullsend-ai-review · 2026-06-07T13:33:24Z

Review skipped — this PR is already merged.

The /fs-review command only reviews open pull requests.

_{Posted by fullsend post-review check}

fullsend-ai-retro · 2026-06-07T13:36:01Z

Retro: PR #1962 — include disabled repos in repo-maintenance token scope

Workflow quality: Good. This was a human-authored PR by ralphbean (with Claude assistance) fixing a high-priority e2e bug (#1960). The review bot ran 3 times across different SHAs, approving each time with low-severity findings. Notably, the 3rd review caught a real annotation-format bug that the author fixed in a subsequent commit — a clear win for automated review.

Timeline: Issue filed → triage bot summary (6 min) → PR opened (35 min) → 3 review iterations over 2 days → human approval → merged. Total: ~43 hours.

No new proposals. The main inefficiency observed — the review bot re-posting the same runs[0] finding across reviews 1 and 2 — is already covered by multiple open issues (#1285, #1013, #956, #1500, #1513). The 3 review runs (one per push) are covered by #1418 and #1452. No novel improvement opportunities were identified beyond what's already tracked.

github-actions Bot deployed to site-preview June 5, 2026 18:29 View deployment

fullsend-ai-review Bot approved these changes Jun 5, 2026

View reviewed changes

fullsend-ai-review Bot added the ready-for-merge All reviewers approved — ready to merge label Jun 5, 2026

ralphbean mentioned this pull request Jun 5, 2026

fix(#1960): capture repo-maintenance workflow logs in e2e unenrollment test #1961

Closed

4 tasks

ralphbean changed the title ~~ci(e2e): capture repo-maintenance debug info on unenrollment failure~~ fix(#1960): include disabled repos in repo-maintenance token scope Jun 5, 2026

github-actions Bot deployed to site-preview June 5, 2026 20:08 View deployment

fullsend-ai-review Bot approved these changes Jun 5, 2026

View reviewed changes

fullsend-ai-review Bot added ready-for-merge All reviewers approved — ready to merge and removed ready-for-merge All reviewers approved — ready to merge labels Jun 5, 2026

github-actions Bot deployed to site-preview June 6, 2026 19:25 View deployment

fullsend-ai-review Bot approved these changes Jun 6, 2026

View reviewed changes

fullsend-ai-review Bot added ready-for-merge All reviewers approved — ready to merge and removed ready-for-merge All reviewers approved — ready to merge labels Jun 6, 2026

ralphbean added 4 commits June 7, 2026 09:22

ralphbean force-pushed the ci/e2e-unenroll-debug-logs branch from 055b969 to e9ec1f2 Compare June 7, 2026 13:22

ifireball approved these changes Jun 7, 2026

View reviewed changes

github-actions Bot deployed to site-preview June 7, 2026 13:24 View deployment

ralphbean added this pull request to the merge queue Jun 7, 2026

Merged via the queue into main with commit 64170dc Jun 7, 2026
10 checks passed

ralphbean deleted the ci/e2e-unenroll-debug-logs branch June 7, 2026 13:29

qodo-code-review Bot mentioned this pull request Jun 23, 2026

fix(install): skip enrollment in PR mode + e2e PR-based scaffold flow #2567

Merged

Navigation Menu

Uh oh!

fix(#1960): include disabled repos in repo-maintenance token scope - #1962

fix(#1960): include disabled repos in repo-maintenance token scope#1962
ralphbean merged 4 commits into
mainfrom
ci/e2e-unenroll-debug-logs

ralphbean commented Jun 5, 2026 •

edited

Loading

Uh oh!

github-actions Bot commented Jun 5, 2026 •

edited

Loading

Uh oh!

fullsend-ai-review Bot commented Jun 5, 2026 •

edited

Loading

Review

Findings

Low

Review

Findings

Low

Info

Uh oh!

fullsend-ai-review Bot Jun 5, 2026

Uh oh!

fullsend-ai-review Bot Jun 5, 2026

Uh oh!

fullsend-ai-review Bot Jun 6, 2026

Uh oh!

fullsend-ai-review Bot Jun 6, 2026

Uh oh!

Uh oh!

fullsend-ai-review Bot commented Jun 7, 2026

Uh oh!

fullsend-ai-retro Bot commented Jun 7, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

		@@ -584,6 +593,20 @@ func runUnenrollmentTest(t testing.T, env e2eEnv) {
		"admin", "disable", "repos", env.org, testRepo, "--yolo")
		t.Logf("Disable repos output:\n%s", output)

Uh oh!

Conversation

ralphbean commented Jun 5, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Root cause

Fix

Test plan

Uh oh!

github-actions Bot commented Jun 5, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Site preview

Uh oh!

fullsend-ai-review Bot commented Jun 5, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Review

Findings

Low

Info

Review

Findings

Low

Review

Findings

Low

Info

Uh oh!

fullsend-ai-review Bot Jun 5, 2026

Choose a reason for hiding this comment

Uh oh!

fullsend-ai-review Bot Jun 5, 2026

Choose a reason for hiding this comment

Uh oh!

fullsend-ai-review Bot Jun 6, 2026

Choose a reason for hiding this comment

Uh oh!

fullsend-ai-review Bot Jun 6, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

fullsend-ai-review Bot commented Jun 7, 2026

Uh oh!

fullsend-ai-retro Bot commented Jun 7, 2026

Retro: PR #1962 — include disabled repos in repo-maintenance token scope

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

ralphbean commented Jun 5, 2026 •

edited

Loading

github-actions Bot commented Jun 5, 2026 •

edited

Loading

fullsend-ai-review Bot commented Jun 5, 2026 •

edited

Loading