fix(#2017): unenroll repos during uninstall via repo-maintenance workflow by fullsend-ai-coder[bot] · Pull Request #2020 · fullsend-ai/fullsend

fullsend-ai-coder · 2026-06-08T13:12:45Z

Previously, EnrollmentLayer.Uninstall() was a no-op — enrolled repos kept their shim workflows and .fullsend references after running fullsend admin uninstall. This left stale agent configuration in repos when users later reinstalled with a different set of agents.

The fix makes two changes:

runUninstall() now extracts enabled repos from config.yaml and
passes them as disabledRepos to the EnrollmentLayer.
EnrollmentLayer.Uninstall() now updates config.yaml to mark all
repos as disabled, then dispatches the repo-maintenance workflow
to create unenrollment PRs that remove shim workflows from each
enrolled repo. Errors are non-fatal — the uninstall continues and
the user is informed of any repos needing manual cleanup.

The unenrollment runs before ConfigRepoLayer deletes the .fullsend repo (layers uninstall in reverse order), so the workflow is still available to dispatch.

Note: pre-commit could not run in the sandbox (shellcheck hook failed to install due to network restrictions). The post-script runs pre-commit authoritatively on the runner.

Closes #2017

Post-script verification

Branch is not main/master (agent/2017-uninstall-cleanup-enrollment)
Secret scan passed (gitleaks — 5faa79f06ad8dbac4b7f08aa7a6ff79772c74552..HEAD)
Pre-commit hooks passed (authoritative run on runner)
Tests ran inside sandbox

…flow Previously, EnrollmentLayer.Uninstall() was a no-op — enrolled repos kept their shim workflows and .fullsend references after running fullsend admin uninstall. This left stale agent configuration in repos when users later reinstalled with a different set of agents. The fix makes two changes: 1. runUninstall() now extracts enabled repos from config.yaml and passes them as disabledRepos to the EnrollmentLayer. 2. EnrollmentLayer.Uninstall() now updates config.yaml to mark all repos as disabled, then dispatches the repo-maintenance workflow to create unenrollment PRs that remove shim workflows from each enrolled repo. Errors are non-fatal — the uninstall continues and the user is informed of any repos needing manual cleanup. The unenrollment runs before ConfigRepoLayer deletes the .fullsend repo (layers uninstall in reverse order), so the workflow is still available to dispatch. Note: pre-commit could not run in the sandbox (shellcheck hook failed to install due to network restrictions). The post-script runs pre-commit authoritatively on the runner. Closes #2017

github-actions · 2026-06-08T13:14:20Z

Site preview

Preview: https://74789b30-site.fullsend-ai.workers.dev

Commit: 3336cb69b3cdd0041fe8510d4bdf60357613e49f

ralphbean

I think this needs a small change before we merge. See inline comments.

Replace the manual loop over disabledRepos with a call to the existing reportReconciliationPRs method, which already iterates both enabledRepos and disabledRepos with the correct PR titles. This avoids duplicating the title string that must match UNENROLL_PR_TITLE in reconcile-repos.sh. Assisted-by: Claude Opus 4.6 <noreply@anthropic.com> Signed-off-by: Ralph Bean <rbean@redhat.com>

fullsend-ai-review · 2026-06-10T21:09:23Z

🤖 Review · Started 9:09 PM UTC
Commit: 25c69ce · View workflow run →

The comment claimed only ConfigRepoLayer matters for uninstall since other layers are no-ops. This is no longer true now that EnrollmentLayer.Uninstall() does real work. Assisted-by: Claude Opus 4.6 <noreply@anthropic.com> Signed-off-by: Ralph Bean <rbean@redhat.com>

ralphbean

LGTM. Pushed 25c69ce and 3336cb6 to address the earlier feedback.

fullsend-ai-review · 2026-06-10T21:18:06Z

🤖 Finished Review · ✅ Success · Started 9:18 PM UTC · Completed 9:30 PM UTC
Commit: 3336cb6 · View workflow run →

fullsend-ai-review · 2026-06-10T21:30:36Z

Review

Findings

High

[architectural-violation] internal/cli/admin.go:1647 — The change passes enrolledRepos as the disabledRepos parameter to NewEnrollmentLayer during uninstall, but the approved design specification (docs/superpowers/specs/2026-04-18-enrollment-reconciliation-design.md, line 69) explicitly states that runUninstall should pass nil for disabledRepos. The design spec should be updated before or alongside the implementation change.
Remediation: Update the design spec to document the rationale for passing enrolled repos during uninstall, then implement the code change. Alternatively, follow the existing spec and pass nil.
[scope-creep] internal/layers/enrollment.go — The approved design spec (line 62) states "Uninstall — Stays a no-op" and the alternatives-considered section (line 100) explicitly rejected "Go-side unenrollment in EnrollmentLayer." This PR implements the rejected approach. Note: the PR's approach still dispatches the workflow rather than doing Go-side unenrollment directly, which partially addresses the rejection reason.
Remediation: Either revert EnrollmentLayer.Uninstall() to remain a no-op, or update the design spec first to document why the original decision should be reversed.

Medium

[race condition / ordering] internal/layers/enrollment.go:237 — If awaitWorkflowRun times out (~3 minutes), the method logs a warning and returns nil. The workflow may still be running when ConfigRepoLayer.Uninstall subsequently deletes the .fullsend repo, killing the in-progress workflow run and preventing unenrollment PRs from being created.
[intent-misalignment] internal/cli/admin.go:1640 — The variable enrolledRepos is extracted via EnabledRepos() (repos with enabled: true) but passed as the disabledRepos parameter. The semantic inversion is not documented. See also: [naming-consistency] finding at this location.
[coherence-drift] internal/layers/enrollment.go — The Uninstall method always returns nil, swallowing all errors as warnings. This bypasses Stack.UninstallAll()'s error-collection mechanism. The user is informed via StepWarn but the error is not propagated to the caller.

Low

[logic error] internal/layers/enrollment.go:211 — Minor TOCTOU: Uninstall disables all repos from a fresh config.yaml read, but reportReconciliationPRs searches repos in l.disabledRepos (populated at construction time). The window is negligible in practice.
[error handling] internal/layers/enrollment.go:220 — When CreateOrUpdateFile fails, the method returns nil after a warning. The repos are not actually disabled in the persisted config. Consistent with the non-fatal error philosophy.
[test adequacy] internal/layers/enrollment_test.go:158 — No test covers the case where enabledRepos is non-empty but disabledRepos is empty (layer constructed with enabled repos but no disabled repos during uninstall).
[comment-consistency] internal/layers/enrollment.go:55 — The old // no-op inline comment on the OpUninstall case was removed in the diff, but verify no stale reference remains.
[comment-staleness] internal/cli/admin.go:1694 — Removed comment was factually correct before this PR; removal is appropriate given the change.

Info

[naming-consistency] internal/cli/admin.go:1641 — The codebase consistently uses enabledRepos; the new variable enrolledRepos deviates from this convention.
[comment-location] internal/cli/admin.go:1702 — No replacement comment explains why EnrollmentLayer now receives enrolledRepos during uninstall.
[import-organization] internal/layers/enrollment.go:7 — New config import is correctly placed among internal imports.
[error-message-style] internal/layers/enrollment.go — Error messages use lowercase and StepWarn consistently. No returned errors to format with %w.

fullsend-ai-review

See the review comment for full details.

fullsend-ai-review · 2026-06-10T21:30:37Z

 		if parsedCfg, parseErr := config.ParseOrgConfig(cfgData); parseErr == nil {
 			for _, agent := range parsedCfg.Agents {
 				agentSlugs = append(agentSlugs, agent.Slug)
 			}


[high] architectural-violation

The change passes enrolledRepos as the disabledRepos parameter to NewEnrollmentLayer during uninstall, but the approved design specification (docs/superpowers/specs/2026-04-18-enrollment-reconciliation-design.md, line 69) explicitly states that runUninstall should pass nil for disabledRepos. The design spec should be updated before or alongside the implementation change.

Suggested fix: Update the design spec to document the rationale for passing enrolled repos during uninstall, then implement the code change. Alternatively, follow the existing spec and pass nil.

fullsend-ai-review · 2026-06-10T21:30:37Z

+// ConfigRepoLayer deletes the .fullsend repo (layers uninstall in
+// reverse order), so the workflow is still available to dispatch.
+//
+// Errors during unenrollment are non-fatal — the user is informed but


[high] scope-creep

The approved design spec (line 62) states Uninstall stays a no-op and the alternatives-considered section (line 100) explicitly rejected Go-side unenrollment in EnrollmentLayer. This PR implements the rejected approach. The approach still dispatches the workflow rather than doing Go-side unenrollment directly, which partially addresses the rejection reason.

Suggested fix: Either revert EnrollmentLayer.Uninstall() to remain a no-op, or update the design spec first to document why the original decision should be reversed.

fullsend-ai-review · 2026-06-10T21:30:37Z

+	l.ui.StepDone("Disabled all repos in config")
+
+	// Dispatch repo-maintenance to create unenrollment PRs.
+	dispatchTime := time.Now().UTC().Add(-30 * time.Second)


[medium] race condition / ordering

If awaitWorkflowRun times out (~3 minutes), the method logs a warning and returns nil. The workflow may still be running when ConfigRepoLayer.Uninstall subsequently deletes the .fullsend repo, killing the in-progress workflow run and preventing unenrollment PRs from being created.

fullsend-ai-review · 2026-06-10T21:30:37Z

@@ -1638,13 +1638,15 @@ func runUninstall(ctx context.Context, client forge.Client, printer *ui.Printer,
 	// apps that block reinstallation (PEM keys are one-shot).
 	var agentSlugs []string
 	var configMode string


[medium] intent-misalignment

The variable enrolledRepos is extracted via EnabledRepos() (repos with enabled: true) but passed as the disabledRepos parameter. The semantic inversion is not documented.

fullsend-ai-review · 2026-06-10T21:30:37Z

+// ConfigRepoLayer deletes the .fullsend repo (layers uninstall in
+// reverse order), so the workflow is still available to dispatch.
+//
+// Errors during unenrollment are non-fatal — the user is informed but


[medium] coherence-drift

The Uninstall method always returns nil, swallowing all errors as warnings. This bypasses Stack.UninstallAll() error-collection mechanism. The user is informed via StepWarn but the error is not propagated to the caller.

fullsend-ai-review · 2026-06-10T21:30:38Z

 }

-func TestEnrollmentLayer_Uninstall_Noop(t *testing.T) {
+func TestEnrollmentLayer_Uninstall_NoRepos(t *testing.T) {


[low] test adequacy

No test covers the case where enabledRepos is non-empty but disabledRepos is empty (layer constructed with enabled repos but no disabled repos during uninstall).

fullsend-ai-retro · 2026-06-12T16:26:10Z

🤖 Finished Retro · ✅ Success · Started 4:26 PM UTC · Completed 4:34 PM UTC
Commit: 3336cb6 · View workflow run →

fullsend-ai-retro · 2026-06-12T16:34:18Z

Retro: PR #2020 — `fix(#2017): unenroll repos during uninstall via repo-maintenance workflow`

Timeline

Jun 8 13:12 — Code agent creates PR fix(#2017): unenroll repos during uninstall via repo-maintenance workflow #2020 from issue Uninstall doesn't remove .fullsend #2017 (3 files, +198/-12)
Jun 8 18:32 — Human reviewer (ralphbean) requests changes: suggests reusing reportReconciliationPRs() instead of duplicating logic, notes missing test coverage for error paths
~2 day gap — No fix agent activity observed; human resolves feedback themselves
Jun 10 21:17 — Human pushes fix commits (25c69ce, 3336cb6) and approves
Jun 10 21:18–21:30 — Review bot runs on the new commits, issues CHANGES_REQUESTED with 2 high, 3 medium findings
Jun 12 16:21 — PR merged despite bot's CHANGES_REQUESTED

Key observations

Code agent quality: The human had to fix the code themselves rather than the fix agent handling it. The primary feedback (reuse existing function) is a standard code-quality concern the code agent could have caught.
Review bot timing: The bot ran after the human already approved, producing a CHANGES_REQUESTED that contradicted the human's approval. This is covered by existing issue #1922.
Review bot false positives on design-spec deviation: The bot's intent-coherence sub-agent flagged two high-severity findings (scope-creep and architectural-violation) because the implementation deviated from a design spec that said "Uninstall stays a no-op." However, issue Uninstall doesn't remove .fullsend #2017 explicitly authorized this change. The sub-agent treated the design spec as higher authority than the linked issue. This is a distinct gap from #1273 (which covers PR description context, not linked issue vs. design spec conflicts).
Fix agent gap: The fix agent didn't visibly respond to the human's CHANGES_REQUESTED review. Existing issues #1125 and #551 cover fix agent dispatch mechanics.
CHANGES_REQUESTED override: The bot's verdict was overridden by the human merge. Covered by #1922 and #2029.

Proposals filed

1 new proposal about intent-coherence sub-agent authority hierarchy. Other findings are already covered by existing open issues.

Proposals filed

Intent-coherence sub-agent should resolve linked-issue vs design-spec conflicts in favor of the issue (in fullsend-ai/fullsend)

github-actions Bot deployed to site-preview June 8, 2026 13:14 View deployment

ralphbean requested changes Jun 8, 2026

View reviewed changes

Comment thread internal/layers/enrollment.go

Comment thread internal/layers/enrollment_test.go

github-actions Bot deployed to site-preview June 10, 2026 21:09 View deployment

ralphbean approved these changes Jun 10, 2026

View reviewed changes

github-actions Bot deployed to site-preview June 10, 2026 21:17 View deployment

fullsend-ai-review Bot suggested changes Jun 10, 2026

View reviewed changes

fullsend-ai-retro Bot mentioned this pull request Jun 11, 2026

fix(#2143): add pre-flight GitHub API connectivity check in sandbox #2144

Merged

4 tasks

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

Merged via the queue into main with commit a2652ea Jun 12, 2026
12 checks passed

ralphbean deleted the agent/2017-uninstall-cleanup-enrollment branch June 12, 2026 16:21

fullsend-ai-retro Bot mentioned this pull request Jun 12, 2026

Intent-coherence sub-agent should resolve linked-issue vs design-spec conflicts in favor of the issue #2245

Closed

fullsend-ai-coder Bot mentioned this pull request Jun 12, 2026

fix(#2245): add conflict-resolution guidance to intent-coherence sub-agent #2246

Closed

4 tasks

Uh oh!

Conversation

fullsend-ai-coder Bot commented Jun 8, 2026

Post-script verification

Uh oh!

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

Uh oh!

Site preview

Uh oh!

ralphbean left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

fullsend-ai-review Bot commented Jun 10, 2026

Uh oh!

ralphbean left a comment

Choose a reason for hiding this comment

Uh oh!

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

Uh oh!

Uh oh!

fullsend-ai-review Bot commented Jun 10, 2026

Review

Findings

High

Medium

Low

Info

Uh oh!

fullsend-ai-review Bot left a comment

Choose a reason for hiding this comment

Uh oh!

fullsend-ai-review Bot Jun 10, 2026

Choose a reason for hiding this comment

Uh oh!

fullsend-ai-review Bot Jun 10, 2026

Choose a reason for hiding this comment

Uh oh!

fullsend-ai-review Bot Jun 10, 2026

Choose a reason for hiding this comment

Uh oh!

fullsend-ai-review Bot Jun 10, 2026

Choose a reason for hiding this comment

Uh oh!

fullsend-ai-review Bot Jun 10, 2026

Choose a reason for hiding this comment

Uh oh!

fullsend-ai-review Bot Jun 10, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

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

Uh oh!

Uh oh!

fullsend-ai-retro Bot commented Jun 12, 2026

Retro: PR #2020 — fix(#2017): unenroll repos during uninstall via repo-maintenance workflow

Timeline

Key observations

Proposals filed

Proposals filed

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

github-actions Bot commented Jun 8, 2026 •

edited

Loading

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

edited

Loading

fullsend-ai-retro Bot commented Jun 12, 2026 •

edited

Loading

Retro: PR #2020 — `fix(#2017): unenroll repos during uninstall via repo-maintenance workflow`