feat(repos): add mint verification pre-flight to repos upgrade - #5596
Conversation
|
🤖 Finished Review · ✅ Success · Started 3:17 PM UTC · Completed 3:31 PM UTC |
Site previewPreview: https://2b4b14fc-site.fullsend-ai.workers.dev Commit: |
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
ReviewAll prior review findings have been addressed. The plan document guidance ( FindingsLow
Previous runReviewAll prior review findings have been addressed: the CLI reference tree in FindingsMedium
Low
Previous run (2)ReviewAll four prior review findings have been addressed: the PR title now uses FindingsMedium
Low
Previous run (3)ReviewThe code changes are clean, well-tested, and follow established codebase patterns. The mint verification pre-flight reuses FindingsMedium
Low
ReviewThe code changes are clean, well-tested, and follow established codebase patterns exactly. The mint verification pre-flight reuses The documentation has not been updated to reflect the new behavior. FindingsMedium
Low
Labels: PR modifies repos upgrade command (install/upgrade area) and adds mint verification pre-flight Previous run (4)ReviewThe code changes are clean, well-tested, and follow established codebase patterns. The mint verification pre-flight reuses FindingsMedium
Low
Previous run (5)ReviewThe code changes are clean, well-tested, and follow established codebase patterns exactly. The mint verification pre-flight reuses The documentation has not been updated to reflect the new behavior. FindingsMedium
Low
Labels: PR modifies repos upgrade command (install/upgrade area) and adds mint verification pre-flight |
fullsend-ai-review
Bot
added
requires-manual-review
7d28622 to
1bb1201
Compare
|
/fs-review |
|
🤖 Review · |
1bb1201 to
4b6973c
Compare
|
/fs-review |
|
🤖 Finished Review · ✅ Success · Started 4:48 PM UTC · Completed 5:02 PM UTC |
![fullsend-ai-review[bot]](https://avatars.githubusercontent.com/in/3478501?s=60&v=4){kind=small}
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
ggallen
changed the title
4b6973c to
a4beacf
Compare
|
/fs-review |
|
🤖 Finished Review · ✅ Success · Started 5:06 PM UTC · Completed 5:23 PM UTC |
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
a4beacf to
eb2c5c5
Compare
|
/fs-review |
|
🤖 Finished Review · ✅ Success · Started 5:26 PM UTC · Completed 5:41 PM UTC |
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
Signed-off-by: Greg Allen <gallen@redhat.com> Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> Signed-off-by: Greg Allen <gallen@redhat.com>
eb2c5c5 to
3424b98
Compare
|
/fs-review |
|
🤖 Finished Review · ✅ Success · Started 5:46 PM UTC · Completed 5:59 PM UTC |
There was a problem hiding this comment.
Note: The following inline comments could not be posted on the diff (GitHub returned 422) and are included here instead:
docs/ADRs/0057-repos-management.md(file-level): Line 76 · [low] documentation-inconsistency
The command table in the Decision section describes repos upgrade as "Upgrade scaffold shim ref across repos" while other documentation tables (docs/cli/repos.md:21, docs/guides/getting-started/operations.md:88) were updated in this PR to "Verify mint deployment then upgrade scaffold shim ref across repos." A Note block was added after the table explaining the pre-flight behavior, but the table entry itself was not updated to match.
Suggested fix: Update the table entry to match the other tables, or note why the ADR Decision section table description was intentionally kept brief since the annotation below provides the detail.
fullsend-ai-review
Bot
added
ready-for-merge
|
🤖 Finished Retro · ✅ Success · Started 6:29 PM UTC · Completed 6:38 PM UTC |
Retro: PR #5596 — mint verification pre-flight for
|
| Time (UTC) | Event |
|---|---|
| 14:49 | ggallen files issue #5594 |
| 14:51–14:55 | Triage agent runs, reclassifies bug→feature, provides technical guidance (no docs scope) |
| 15:04 | ggallen adds ready-to-code |
| 15:05–15:16 | Code agent runs, produces PR #5596 — clean code, 4 new tests, zero documentation updates |
| 15:17–15:31 | Review 1 (auto): 3 medium docs-currency in repos.md |
| 16:29–16:45 | ggallen force-pushes twice to fix repos.md |
| 16:40–16:46 | Review 2: cancelled (force-push during run) |
| 16:48–17:02 | Review 3: 2 new medium (operations.md, PR title convention) + 2 new low (CLI help text, ADR) |
| 17:04 | ggallen force-pushes, renames PR title |
| 17:06–17:23 | Review 4: 1 new medium (cli-internals.md missing flag) + 1 new low (repos.md summary table) |
| 17:24 | ggallen force-pushes |
| 17:26–17:41 | Review 5: 1 new medium (plans doc stale) + 1 low (ADR edit approach wrong) |
| 17:44 | ggallen force-pushes (reverts ADR inline edit, adds annotation) |
| 17:46–17:59 | Review 6: APPROVED with 1 informational low |
| 18:04 | ggallen approves |
| 18:27 | PR merged |
Key observations
1. Code was clean from the start; all iteration was documentation. Every review noted the code was "clean, well-tested, and follows established codebase patterns." The 5 review rounds and ~2.5 hours of rework were entirely spent catching stale documentation across 7 locations.
2. Whack-a-mole review pattern. The review agent discovered documentation gaps in new files each round rather than surfacing all of them at once: repos.md → operations.md + ADR + CLI help text → cli-internals.md + summary table → plans/repos-management.md + ADR approach. A comprehensive first-pass scan would have reduced the cycle from 5 rounds to 1–2.
3. Triage omitted documentation scope entirely. The triage summary gave excellent technical guidance for the code change but listed zero documentation files. The code agent had no hint that repos upgrade was referenced in 5+ documentation locations.
4. No fix agent was dispatched. All 4 force-pushes were manual fixes by ggallen. The human spent ~2.5 hours in a tight review-fix-retrigger loop.
5. Autonomy readiness is high. The human reviewer made zero independent findings — every fix was a direct response to an agent-identified issue. The agent drove the entire substantive review. The human's role was purely supervisory: triggering re-reviews, confirming fixes, and merging.
Existing issues this retro provides evidence for
- #1733 (code agent proactive docs): This PR is a textbook case — 229 lines of feature code with zero doc updates, causing 5 review iterations. The code agent produced clean code but needed 5 rounds of review feedback to cover all 7 documentation locations.
- #1061 and #3932 (review agent single-pass docs scanning): The review agent found stale docs in
repos.mdfirst, thenoperations.mdnext round, thencli-internals.md, thenplans/repos-management.md— 4 files across 4 separate rounds. A single-pass grep-based scan when the first staleness is detected would have surfaced all 7 locations immediately. - #1616 (review agent flags missing CLI docs as medium): The review agent correctly flagged docs-currency issues as medium severity, which drove the fix cycle. This PR validates that policy.
Proposals filed
- Triage agent should enumerate documentation touchpoints for CLI changes (in
fullsend-ai/agents) - Add CLI-to-documentation cross-reference in contributing guide (in
fullsend-ai/fullsend)
Contributors updating CLI command behavior had no way to discover all documentation locations for a given command group — PR fullsend-ai#5596 needed 5 review iterations for this reason. Add docs/contributing/documentation.md mapping the 7 major command groups to their doc touchpoints, plus a general grep-based discovery rule for minor commands. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> Signed-off-by: Shai Revivo <srevivo@redhat.com>
1 participant
Summary
repos upgradenow runs a mint verification pre-flight step before upgrading any repos. If the mint deployment URL doesn't match the manifest, the upgrade fails with a clear error. Add--skip-mint-checkflag to bypass when needed.Related Issue
Closes #5594
Changes
skipMintCheckfield and--skip-mint-checkflag toreposUpgradeConfig(mirrorsrepos install)testProvisionerfield for test overrides (follows existing pattern)runReposUpgradebefore the upgrade loop: creates a provisioner, callsrepos.UpgradeMint, and fails early on mismatchrepos upgrade-mintremains unchanged as a standalone command--skip-mint-checkbypasses, mint pass then upgrade proceeds, mint check runs during dry-runskipMintCheck: trueto avoid live GCF callsTesting
go test -race ./internal/cli/...passes (all repos-related tests)go test -race ./internal/repos/...passesgo vet ./internal/cli/...passesgitlint --commit HEADpassesChecklist
!for breaking changes)Closes #5594
Post-script verification
agent/5594-upgrade-mint-preflight)63d6e214ad06b46b64bcf8b73926184a6fb3c53c..HEAD)