Skip to content

fix(#7378): remove migrated repos from per-org config - #7385

Merged
ggallen merged 1 commit into
mainfrom
agent/7378-remove-migrated-org-entries
Sep 16, 2026
Merged

ggallen merged 1 commit into
mainfrom
agent/7378-remove-migrated-org-entries

Conversation

@fullsend-ai-coder

Copy link
Copy Markdown
Contributor

Summary

fullsend repos migrate now deletes successfully migrated repository entries from the source <org>/.fullsend/config.yaml instead of leaving them as enabled: false. That leftover disabled entry was enough for still-running per-org repo-maintenance.yml reconcile to open an offboard PR that would delete the newly installed per-repo shim.

Related Issue

#7378

Changes

  • Add DeleteRepo to config.OrgConfigWriter so callers can remove a repository entry without mutating RepoMap().
  • Migration cleanup (Step 4) deletes still-enabled source entries for freshly migrated repos and for selected repos already detected as per-repo installed (recovery after a source-config write failure).
  • Failed, unselected, and pre-existing disabled entries — plus unrelated org config — are left unchanged. Dry runs do not write the source config.
  • Update CLI help, docs/cli/repos.md, and the repo-management guide to describe removal rather than disabling.

Testing

  • go test -race -count=1 -coverprofile=coverage.out ./internal/config/ ./internal/repos/ — pass (DeleteRepo 100%, Migrate 91.1%).
  • go test -race -count=1 ./internal/cli/ -run 'TestRunReposMigrate|TestReposCmd' — pass.
  • go vet ./internal/config/ ./internal/repos/ ./internal/cli/ — pass.
  • Direct hook execution (gofmt, gitleaks, lychee, lint-docs-links) — pass.
  • pre-commit run could not fetch remote hook repos (HTTP 403); golangci-lint is not installed in this sandbox.

Checklist

  • PR title follows Conventional Commits (fix(#7378): ...)
  • Autonomous agent commit (no DCO sign-off)
  • Changes are limited to source-config removal, tests, and docs for this bug

Closes #7378

Post-script verification

  • Branch is not main/master (agent/7378-remove-migrated-org-entries)
  • Secret scan passed (gitleaks — 6aa078bc7dc2a5f0dcf2aea8e78e604e380cb2ff..HEAD)
  • PR body secret scan passed (gitleaks — no-git)

repos migrate previously set enabled: false on successful
migrations. Legacy repo-maintenance reconcile then treated those
entries as offboard candidates and could open a PR deleting the
newly installed per-repo shim.

Delete the source repository entry instead, and add DeleteRepo to
OrgConfigWriter so callers do not mutate RepoMap() directly.
Failed, unselected, and pre-existing disabled entries stay in
place. A rerun still removes still-enabled entries for repos
already detected as per-repo installed.

Note: pre-commit could not fetch remote hook repos (HTTP 403).
gofmt, go vet, gitleaks, lychee, and lint-docs-links were run
directly. golangci-lint is not installed in this sandbox.

Closes #7378
@fullsend-ai-coder
fullsend-ai-coder Bot requested a review from a team as a code owner September 16, 2026 19:53
@fullsend-ai-coder fullsend-ai-coder Bot added the ready-for-review Triggers review agent dispatch label Sep 16, 2026
@fullsend-ai-review

fullsend-ai-review Bot commented Sep 16, 2026

Copy link
Copy Markdown

🤖 Finished Review · ✅ Success · Started 7:55 PM UTC · Completed 8:07 PM UTC

Commit: e326828 · View workflow run →

Runtime: pi · Model: sonnet → claude-sonnet-5 · Effort: high · Cost: $4.27

@github-actions

Copy link
Copy Markdown

Site preview

Preview: https://c527e36c-site.fullsend-ai.workers.dev

Commit: e326828444765dbfbd605b5e4b0e8977ec2b985d

@codecov

codecov Bot commented Sep 16, 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

Copy link
Copy Markdown

Looks good to me

@fullsend-ai-review fullsend-ai-review Bot added the ready-for-merge All reviewers approved — ready to merge label Sep 16, 2026
@ggallen
ggallen enabled auto-merge September 16, 2026 20:23
@ggallen
ggallen added this pull request to the merge queue Sep 16, 2026
Merged via the queue into main with commit 200a184 Sep 16, 2026
69 of 73 checks passed
@ggallen
ggallen deleted the agent/7378-remove-migrated-org-entries branch September 16, 2026 21:04
@fullsend-ai-retro

fullsend-ai-retro Bot commented Sep 16, 2026

Copy link
Copy Markdown

🤖 Finished Retro · ✅ Success · Started 9:06 PM UTC · Completed 9:14 PM UTC

Commit: e326828 · View workflow run →

Runtime: claude · Model: sonnet → claude-sonnet-5 · Effort: high · Cost: $1.75

@fullsend-ai-retro

Copy link
Copy Markdown

PR #7385 is the correct, successful resolution of issue #7378. The first attempt, PR #7382, implemented only the guard-variable fix the issue originally hypothesized; maintainer ggallen closed it and rewrote the issue's acceptance criteria after determining the real fix required deleting (not disabling) migrated repo entries from the source org config. That saga (wrong initial diagnosis, ~$17 spent on the first triage/code/review cycle, plus a fix-agent run that kept working for minutes after the PR was closed and pushed a commit to a dead branch) was already thoroughly analyzed by this session's own retro on PR #7382, which matched it to already-open issues #2251 and #3158/#2812 (triage/code should verify governing ADRs, not just the issue's stated hypothesis), #2284 (re-evaluate in-flight PRs when a human questions the linked issue), the in-flight-cancellation cluster (#2388, #1870, #6725, #4070, #5817), and the inline-comment 422 cluster (#6039, #7346). All remain open; I re-verified none were closed since and am not re-filing. This second pass (re-triage, correct implementation adding OrgConfigWriter.DeleteRepo and switching migrate.go Step 4 from disable-to delete, review approval, human approval, merge-queue merge) ran cleanly with no new gaps in that class.

One new, distinct finding: the 'risk-assessment' review sub-agent (ADR 0089, implemented as a pi runtime persona in internal/runtime/pi_personas.go) never posted its sticky Risk Assessment: N/5 comment on PR #7385, even though it's designed to run as a pre-pass on every PR independent of the approve/request-changes verdict (confirmed via a clean counterexample, #7374: merged with a plain 'Looks good to me' verdict, still got a risk-assessment comment). Across the last 20 fullsend-ai-coder PRs, 3 (#7385, #7356, #7312) silently lack this comment with no visible error - a ~15% miss rate. #7385 is arguably a higher-blast-radius change (deletes config entries) than its predecessor #7382, which did get scored moderate (2/5), making the omission more than cosmetic. No existing open issue is scoped to this specific sub-agent/comment; closest analogs (#4265, #5355, #6050, #7144) cover silent dispatch loss more generally. Filing one proposal for this.

Proposals filed

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

repos migrate leaves migrated repos as enabled: false, triggering offboard PRs that delete their per-repo shim

1 participant