feat(#5271): include private repos in repos.yaml glob expansion - #5275
Conversation
|
🤖 Finished Review · ❌ Failure · Started 10:30 PM UTC · Completed 10:44 PM UTC |
Site previewPreview: https://9720b177-site.fullsend-ai.workers.dev Commit: |
|
Looks good to me Previous runReviewFindingsLow
Labels: PR adds includePrivate parameter to forge.Client.ListOrgRepos interface and updates all callers across CLI admin, dispatch, repos, and forge implementations — a feature touching the install/enrollment flow. Previous run (2)ReviewFindingsMedium
|
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
|
/fs-fix fix the review issues in #5275 (comment). |
|
🤖 Finished Fix · ✅ Success · Started 10:51 PM UTC · Completed 10:57 PM UTC |
🔧 Fix agent — iteration 1 (human-triggered)Fixed both stale-doc findings by updating docs/plans/repos-management.md to reflect the implemented includePrivate parameter on ListOrgRepos. Both sections now use present tense and describe the actual implementation, consistent with the note block at lines 114-118 which was already updated by the original PR. Fixed (2):
Tests: passed Updated by fullsend fix agent |
E2E tests did not runE2E tests run automatically for org/repo members and collaborators on pull requests. For other contributors, a maintainer must add the See E2E testing guide for details. |
1 similar comment
E2E tests did not runE2E tests run automatically for org/repo members and collaborators on pull requests. For other contributors, a maintainer must add the See E2E testing guide for details. |
|
/fs-fix Rebase and address the review issues, both in-line and on the sticky comment. Don't forget to resolve the issues. |
|
🤖 Finished Fix · ❌ Failure · Started 12:34 AM UTC · Completed 12:41 AM UTC |
|
/fs-fix Rebase and address the review issues, both in-line and on the sticky comment. Don't forget to resolve the issues. |
|
🤖 Finished Fix · ✅ Success · Started 4:22 PM UTC · Completed 4:32 PM UTC |
🔧 Fix agent — iteration 2 (human-triggered)Rebased branch onto origin/main and fixed build breakage caused by the GitLab forge client (added to main after original branch) not implementing the updated ListOrgRepos(ctx, org, includePrivate) interface. Both stale-doc findings from the review bot were already addressed in iteration 1. All tests pass. Fixed (6):
Tests: passed Decision points
Updated by fullsend fix agent |
…er rebase The rebase onto main pulled in the GitLab forge client which still had the old ListOrgRepos(ctx, org) signature. Updated to accept the includePrivate bool parameter and apply the same filtering logic as the GitHub client: archived and forked repos are always excluded; private repos are excluded only when includePrivate is false. Addresses review feedback on #5275
21e6429 to
512b771
Compare
|
/fs-review |
|
🤖 Finished Review · ✅ Success · Started 4:59 PM UTC · Completed 5:14 PM UTC |
|
/fs-fix Rebase and address the review issues, both in-line and on the sticky comment. Don't forget to resolve the issues. |
|
🤖 Finished Fix · ❌ Failure · Started 5:18 PM UTC · Completed 5:26 PM UTC |
|
The fix agent completed, but the post-fix script failed before finishing. Workflow run: https://github.com/fullsend-ai/.fullsend/actions/runs/29852268133 Details: |
|
/fs-fix Rebase and address the review issues, both in-line and on the sticky comment. Don't forget to resolve the issues. CRITICAL: DO NOT TRY TO SIGN THE COMMIT! |
|
🤖 Finished Fix · ❌ Failure · Started 5:39 PM UTC · Completed 5:46 PM UTC |
|
The fix agent completed, but the post-fix script failed before finishing. Workflow run: https://github.com/fullsend-ai/.fullsend/actions/runs/29853794757 Details: |
ListOrgRepos now accepts an includePrivate parameter. ExpandGlobs passes includePrivate=true because repos.yaml manifests operate in per-repo mode, where agents run on the target repo and public log exposure does not apply. All existing per-org callers pass false to preserve the original exclusion. Archived and forked repos remain excluded regardless of the flag. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> Signed-off-by: Greg Allen <gallen@redhat.com>
512b771 to
2e19492
Compare
ggallen
left a comment
There was a problem hiding this comment.
Reviewed the changes. The includePrivate parameter addition to ListOrgRepos is clean:
- Interface extended correctly with all callers updated
- Per-org callers pass
false(preserving existing behavior), per-repo glob expansion passestrue - GitLab implementation added after rebase correctly mirrors the GitHub pattern
- Tests cover both include/exclude paths
- Stale doc references fixed
- Coverage threshold met (92% patch)
LGTM.
|
/fs-review |
|
🤖 Finished Review · ✅ Success · Started 7:32 PM UTC · Completed 7:47 PM UTC |
|
🤖 Finished Retro · ✅ Success · Started 8:05 PM UTC · Completed 8:32 PM UTC |
Retro: PR #5275 — include private repos in repos.yaml glob expansionOutcome: Merged after 11 agent runs (1 triage, 1 code, 3 review, 5 fix, 1 retro). The code agent created the PR in 23 minutes from issue creation with a clean 16-file implementation including tests. The primary workflow friction was a post-fix script bug that caused 3 unnecessary fix failures and forced the human to manually squash-merge. Review qualityGood. The review agent correctly identified stale-doc findings in plan documents. The challenger correctly removed false positives — notably a security finding where the doc comment had actually been updated in the diff, and a duplicate finding from overlapping sub-agents. The final review approved with zero findings, aligning with the human reviewer's independent assessment. No human-review findings were missed by the agent. Rework analysis5 fix runs total (2 succeeded, 3 failed). All 3 failures shared the same root cause: the This same bug was independently discovered by a retro on PR #5273 on the same day, confirming it is a systemic issue affecting any fix run that includes a rebase. Existing issues with new evidence
No new proposalsAll identified improvement opportunities are covered by existing open issues. The workflow otherwise performed well — fast code generation, accurate review findings, effective challenger adjudication, and good alignment between agent and human review. |
Summary
includePrivateparameter toforge.Client.ListOrgReposso callers can opt into listing private reposManifest.ExpandGlobs()now passesincludePrivate=truebecauserepos.yamlmanifests operate in per-repo mode where agents run on the target repo itself — the per-org rationale for excluding private repos (public config repo, public workflow logs) does not applyfalseto preserve the original exclusion behaviorRelated Issue
Closes #5271
Changes
Interface & implementations
internal/forge/forge.go: ExtendedListOrgRepossignature withincludePrivate boolparameter; updated doc commentinternal/forge/github/github.go: Split the private-repo filter into a conditional check based on the new parameterinternal/forge/fake.go: UpdatedFakeClient.ListOrgReposto respect the new parameterCallers
internal/repos/manifest.go:ExpandGlobs()passesincludePrivate=true(repos.yaml = per-repo mode)internal/cli/admin.go(7 call sites): passfalse(per-org mode)internal/cli/github.go: passfalseinternal/layers/dispatch.go: passfalseinternal/repos/init.go: passfalseTests
internal/forge/fake_test.go: AddedTestFakeClient_ListOrgRepos_IncludePrivateinternal/forge/github/github_test.go: AddedTestListOrgRepos_IncludePrivateinternal/repos/manifest_test.go: AddedTestExpandGlobs_IncludesPrivateRepos; updated existingTestExpandGlobsto verify private repos are now includedDocs
docs/plans/repos-management.md: Updated limitation note to reflect the implemented designTesting
go build ./...compiles cleanlygo vet ./...passesscan-secretspasses on all changed filesgo test ./internal/forge/... ./internal/repos/...)make go-testpasses (one pre-existing sandbox network failure ininternal/fetchunrelated to this change)Closes #5271
Post-script verification
agent/5271-glob-include-private)b84696fd80a59eb24190aad8fdf6d42f6d8f39bc..HEAD)