Skip to content

feat(#6684): add --gitlab-url flag to repos install - #7087

Merged
ggallen merged 8 commits into
mainfrom
agent/6684-gitlab-url-flag
Sep 9, 2026
Merged

feat(#6684): add --gitlab-url flag to repos install#7087
ggallen merged 8 commits into
mainfrom
agent/6684-gitlab-url-flag

Conversation

@fullsend-ai-coder

Copy link
Copy Markdown
Contributor

Summary

Add a --gitlab-url flag to fullsend repos install that sets gitlab.url in the manifest when bootstrapping GitLab repos. This enables first-time GitLab onboarding in a single command without needing a prior set-default step.

Previously, bootstrapping a new manifest with GitLab repos required running fullsend repos set-default gitlab.url <url> first, because Validate() rejects manifests where gitlab.url is empty but GitLab repos are present. With this change, the URL can be provided inline via the new --gitlab-url flag.

Changes

  • Add gitlabURL field to reposInstallConfig and register --gitlab-url flag
  • Validate the URL early (HTTPS scheme, no path/query/fragment) using the existing RejectExtraneousURLParts helper
  • Set manifest.GitLab.URL and persist it after AddToManifest but before Converge validation
  • Export RejectExtraneousURLParts from the repos package (was unexported rejectExtraneousURLParts)
  • Update docs/cli/repos.md flags table and GitLab install example

Testing

  • Flag registration test (TestReposInstallCmd_GitLabURLFlag)
  • Bootstrap flow test (TestRunReposInstall_GitLabURLBootstrap)
  • Override test (TestRunReposInstall_GitLabURLOverridesExisting)
  • URL validation tests (TestRunReposInstall_GitLabURLValidation)
  • Linting passes
  • All repos-related tests pass
  • Existing internal/repos tests pass

Closes #6684

Post-script verification

  • Branch is not main/master (agent/6684-gitlab-url-flag)
  • Secret scan passed (gitleaks — bccd9e815a09ae063447740473df37908efe17ac..HEAD)
  • PR body secret scan passed (gitleaks — no-git)

Add a --gitlab-url flag to `fullsend repos install` that sets
gitlab.url in the manifest when bootstrapping GitLab repos.
This enables first-time GitLab onboarding in a single command
without a prior set-default step.

Previously, users had to run `fullsend repos set-default
gitlab.url <url>` or manually edit repos.yaml before installing
GitLab repos, because Validate() rejects manifests where
gitlab.url is empty but GitLab repos are present.

Changes:
- Add gitlabURL field to reposInstallConfig struct
- Register --gitlab-url flag in newReposInstallCmd()
- Validate the URL early (HTTPS scheme, no path/query/fragment)
  reusing the existing RejectExtraneousURLParts helper
- Set manifest.GitLab.URL and persist via SetDefault after
  AddToManifest but before Converge validation
- Export RejectExtraneousURLParts for use outside the repos
  package (was unexported rejectExtraneousURLParts)
- Add tests for flag registration, bootstrap flow, URL override,
  and URL validation (non-HTTPS, invalid, extraneous path)
- Update docs/cli/repos.md flags table and GitLab example

Closes #6684
@fullsend-ai-coder
fullsend-ai-coder Bot requested a review from a team as a code owner September 8, 2026 01:07
@fullsend-ai-coder fullsend-ai-coder Bot added the ready-for-review Triggers review agent dispatch label Sep 8, 2026
@fullsend-ai-review

fullsend-ai-review Bot commented Sep 8, 2026

Copy link
Copy Markdown

🤖 Finished Review · ✅ Success · Started 1:09 AM UTC · Completed 1:28 AM UTC

Commit: 9e029bd · View workflow run →

Runtime: claude · Model: opus → claude-opus-4-6 · Effort: high · Cost: $5.33

@codecov

codecov Bot commented Sep 8, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 87.50000% with 4 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
internal/cli/repos.go 85.71% 3 Missing and 1 partial ⚠️

📢 Thoughts on this report? Let us know!

@fullsend-ai-review fullsend-ai-review Bot added the risk/moderate PR risk: moderate label Sep 8, 2026
@fullsend-ai-review

fullsend-ai-review Bot commented Sep 8, 2026

Copy link
Copy Markdown

Risk Assessment: moderate (2/5)

Details

Tier 1 signals are identical to the prior assessment (9 files, medium blast radius, bot author, 0.22 test ratio, no protected or security-sensitive paths); Tier 2 shows the same elevated churn and fix-commit counts concentrated in high-traffic docs and CLI files as the prior run (sub-score 2.86); Tier 3 confirms tight scope alignment with a strictly additive new flag; composite 0.50x1.50+0.30x2.86+0.20x1.50=1.91 rounds to 2 (moderate), matching the prior score with no change in signal characteristics.

Previous run

Risk Assessment: moderate (2/5)

Details

Tier 1 signals are identical to the prior assessment (9 files, medium blast radius, bot author, 0.22 test ratio, no protected or security-sensitive paths); Tier 2 shows elevated churn and fix-commit counts concentrated in high-traffic docs and CLI files consistent with the prior run; Tier 3 confirms tight scope alignment with a strictly additive change; composite 0.50x1.50+0.30x2.86+0.20x1.50=1.91 rounds to 2 (moderate), matching the prior score with no change in signal characteristics.

Previous run (2)

Risk Assessment: moderate (2/5)

Details

Tier 1 signals are identical to the prior assessment (9 files, medium blast radius, bot author, no protected or security-sensitive paths); Tier 2 confirms active CLI-area churn with zero reverts; Tier 3 confirms tight issue-to-PR scope alignment; composite 1.79 rounds to 2 (moderate), matching the prior score with no change in signal characteristics.

Previous run (3)

Risk Assessment: moderate (2/5)

Details

Tier 1 signals essentially unchanged from prior (9 files, medium blast radius, bot author, no protected or security-sensitive paths); Tier 2 confirms active CLI area churn with zero actual reverts; Tier 3 confirms tight issue-to-PR scope alignment; composite 1.78 rounds to moderate (2).

Previous run (4)

Risk Assessment: moderate (2/5)

Details

Tier 1 signals unchanged from prior (8 files, medium blast, bot author, no protected paths); Tier 2 confirms moderate churn on active CLI area with zero reverts; Tier 3 confirms tight issue-to-PR scope alignment; composite 1.74 rounds to moderate (2).

Previous run (5)

Risk Assessment: moderate (2/5)

Details

Tier 1 signals are unchanged from the prior assessment (7 files, medium blast radius, bot author, no protected paths or security-sensitive files, 0.14 test ratio); Tier 2 confirms known-characteristic high churn on active CLI and docs files but finds no actual revert commits and low change coupling; composite lands at 1.79, rounding to moderate (2) — the PR is a narrow, well-tested single-flag addition that squarely matches the issue scope.

Previous run (6)

Risk Assessment: moderate (2/5)

Details

Tier 1 signals are unchanged from the prior assessment (same file count, no protected paths, no dependency changes, bot author); composite lands at 2.16 (0.50×1.5 + 0.30×3.7 + 0.20×1.5), rounding to moderate (2) — the high Tier 2 churn on these CLI/manifest files is a known characteristic rather than a PR-specific risk, and the change itself remains a narrow, well-tested single-flag addition.

Previous run (7)

Risk Assessment: moderate (2/5)

Details

Well-scoped single-flag CLI addition with tests and docs, authored by a bot on frequently churned but low-security files; git history churn inflates Tier 2 but the PR itself is narrow and low-risk, landing at moderate (2).

@fullsend-ai-review

fullsend-ai-review Bot commented Sep 8, 2026

Copy link
Copy Markdown

Review

Findings

Low

  • [missing-feature-documentation] docs/guides/getting-started/operations.md:29 — The GitLab subsection under "Updating configuration values" does not mention the new --gitlab-url flag. The primary documentation surfaces (docs/cli/repos.md, docs/guides/getting-started/repo-management.md, and docs/guides/dev/cli-internals.md) were updated in this PR. Note: the operations guide's GitLab subsection focuses on CI/CD variable convergence rather than forge URL configuration, so this may be intentionally out of scope.
    Remediation: Consider adding a note about --gitlab-url to the GitLab subsection if the section's scope expands to cover forge URL configuration.
Previous run

Review

Findings

Medium

  • [logic-error] internal/cli/repos.go:635 — The --gitlab-url flag documents "implies --forge=gitlab when no forge is specified," but this implication relies on EnsurePlatform(ForgeGitLab) at line 600 creating an empty GitLab section that the forge-inference block at lines 629–646 will detect. This only works when the manifest has no existing GitHub repos. When a manifest already has GitHub repos (ghHasRepos=true, glHasRepos=false), the inference at line 635 picks ForgeGitHub, causing the new repo to be added to the GitHub section instead of GitLab — contradicting the documented behavior. The test TestRunReposInstall_GitLabURLImpliesForge does not catch this because it starts from an empty manifest.
    Remediation: After line 628 (forgeName := opts.forge), add: if opts.gitlabURL != "" && forgeName == "" { forgeName = repos.ForgeGitLab }. Update the test to include a case with an existing GitHub-only manifest.

Low

  • [missing-feature-documentation] docs/guides/getting-started/operations.md:29 — The "GitLab" subsection under "Updating configuration values" does not mention the new --gitlab-url flag. The primary documentation surfaces (docs/cli/repos.md and docs/guides/getting-started/repo-management.md) were updated, but this operations guide omits the new flag.
    Remediation: Consider adding a note about --gitlab-url to the GitLab subsection.

Next steps:

  • /fs-fix — agent addresses review findings automatically
  • /fs-fix <your instruction> — agent fixes with your specific guidance
  • Push commits directly — review re-runs automatically on push
  • /fs-fix-stop — disable automatic fix runs for this PR
Previous run (2)

Review

Findings

Low

  • [naming-convention] internal/cli/repos.go:1289 — The new helper announceGitlabURLDryRun uses lowercase 'l' in 'Gitlab', inconsistent with every other GitLab-related unexported function in this package: setupGitLabBotToken, cleanupGitLabBotToken, healGitLabResourceGroups, getGitLabToken, generateGitLabForgeProfile, resolveGitLabTrackerToken, setupGitLabPipelineSchedules, cleanupGitLabPipelineSchedules. The comment at line 1285 repeats the same casing.
    Remediation: Rename to announceGitLabURLDryRun (capital L) and update both call sites at lines 722 and 738.

  • [architectural-coherence] internal/cli/repos.go:596 — The implicit forge inference triggered by --gitlab-url is only documented in a code comment and a test (TestRunReposInstall_GitLabURLImpliesForge), not in user-facing docs. When --gitlab-url is provided without --forge, EnsurePlatform(repos.ForgeGitLab) creates the GitLab manifest section as a side effect, causing the downstream forge-inference block to select gitlab. Users who provide --gitlab-url while omitting --forge discover this behavior only through observation, not through the flag's help text or the CLI reference page.
    Remediation: Update the --gitlab-url flag description to note that it implies --forge=gitlab when no forge is specified.

  • [architectural-coherence] internal/cli/repos.go:735 — There is no cross-flag consistency check between --forge and --gitlab-url. When --gitlab-url is combined with --forge=github, the code sets manifest.GitLab.URL in-memory (line 598) and AddToManifest serializes the entire in-memory manifest including the GitLab section. If the manifest already contains GitLab repos, the persist block also writes the URL via SetDefault. If no GitLab repos exist (bootstrap), the GitLab section with just the URL is still written by AddToManifest, and the warning "flag had no effect" is printed even though the URL was persisted. No validation guards this contradictory flag combination.
    Remediation: Add an early validation check: if opts.gitlabURL != "" and opts.forge == repos.ForgeGitHub { return error }.


Next steps:

  • /fs-fix — agent addresses review findings automatically
  • /fs-fix <your instruction> — agent fixes with your specific guidance
  • Push commits directly — review re-runs automatically on push
  • /fs-fix-stop — disable automatic fix runs for this PR
Previous run (3)

Review

Findings

Low

  • [edge-case] internal/cli/repos.go:636 — The EnsurePlatform(ForgeGitLab) call at line 594 creates a manifest.GitLab section as a side effect before the forge-inference logic at lines 633–638. When a user bootstraps with --gitlab-url but omits --forge, this causes the forge to be silently inferred as gitlab (because manifest.GitLab != nil && manifest.GitHub == nil), bypassing the --forge is required error at line 642. While the practical effect is reasonable (providing --gitlab-url implies GitLab intent), the inference is accidental rather than intentional. No test covers the --gitlab-url without --forge bootstrap path, so this behavior could regress or surprise users who expect an explicit error when --forge is omitted.

  • [internal-inconsistency] docs/problems/gitlab-implementation.md:369 — The Phase 4 description still reads "Add forge: github or forge: gitlab to config.yaml" and "Support forge-specific settings (GitLab instance URL for self-hosted)." The PR updates the Config Schema Changes section in the same file to show gitlab.url in repos.yaml, but leaves Phase 4 describing the original plan. A reader following both sections will encounter contradictory descriptions of where GitLab URL configuration lives.
    Remediation: Add a parenthetical note to Phase 4's bullet or update the Phase 4 bullets to describe what was actually implemented.

  • [naming-convention] internal/cli/repos.go:1283 — The doc comment on announceGitlabURLDryRun uses British English: "Centralises the message and guard." American English is the default convention for Go projects.
    Remediation: Change "Centralises" to "Centralizes."


Next steps:

  • /fs-fix — agent addresses review findings automatically
  • /fs-fix <your instruction> — agent fixes with your specific guidance
  • Push commits directly — review re-runs automatically on push
  • /fs-fix-stop — disable automatic fix runs for this PR
Previous run (4)

Review

Findings

Medium

  • [logic-error] internal/cli/repos.go:583 — The forge client factory is created by newForgeClientFactory(opts.gitlabToken, manifest), which captures manifest.GitLab.URL at creation time. The --gitlab-url handler that sets manifest.GitLab.URL runs ~130 lines later (after Phase 0 adds repos to the manifest). The forgeClientFactory stores gitlabURL as a plain string copy (forge_client.go:101) and its forgeURL method (line 143–146) returns this captured copy, not a live reference. During converge, clients.ConfigFor("gitlab") creates a GitLab client with the stale/empty URL, causing it to fall back to env vars or default to gitlab.com. On a bootstrap run without FULLSEND_GITLAB_URL set, converge will target the wrong server. A re-run succeeds because the manifest on disk now carries the correct URL.
    Remediation: Either (a) move the --gitlab-url in-memory assignment (manifest.GitLab.URL = opts.gitlabURL) to before the newForgeClientFactory call (requires ensuring the GitLab platform section exists, e.g. via manifest.EnsurePlatform(repos.ForgeGitLab) when --gitlab-url is set), or (b) give the forgeClientFactory a method to update its stored URL and call it after the --gitlab-url handler runs.

Low

  • [test-inadequate] internal/cli/repos_test.go — All new --gitlab-url tests use testClient (fake client via newSingleClientFactory), which bypasses the real forgeClientFactory and its URL resolution entirely. The stale-URL ordering bug cannot be caught by these tests. See also: [logic-error] finding at internal/cli/repos.go:583.

  • [stale-reference] docs/problems/gitlab-implementation.md:494 — The "Config Schema Changes" section shows gitlab_instance_url as the planned config key for a self-hosted GitLab instance URL, but the implementation uses gitlab.url in the manifest. This PR implements the flag using gitlab.url, making the gitlab_instance_url identifier in this design document stale.
    Remediation: Update the code block at line 491–495 to replace gitlab_instance_url with gitlab.url, or add a note that the final implementation diverged from the original design.


Next steps:

  • /fs-fix — agent addresses review findings automatically
  • /fs-fix <your instruction> — agent fixes with your specific guidance
  • Push commits directly — review re-runs automatically on push
  • /fs-fix-stop — disable automatic fix runs for this PR
Previous run (5)

Review

Findings

Low

  • [code organization] internal/cli/repos.go:712 — The Would set gitlab.url=%s in manifest message string is duplicated across two independent code paths: the dry-run early-return block (line 712) and the main handler (line 729). The two sites apply the guard differently — line 712 has no manifest.GitLab != nil check while line 729 does — so the surrounding logic diverges. Any future wording change or conditional adjustment must be made in both places.
    Remediation: Extract the gitlabURL dry-run announcement into a shared helper so the message and its guard are expressed exactly once.

  • [stale-reference] docs/problems/gitlab-implementation.md:485 — The Install Command Changes section lists "Add --gitlab-url for self-hosted GitLab instances" as a future action item. This PR implements the flag, so the entry now describes completed work as pending.


Next steps:

  • /fs-fix — agent addresses review findings automatically
  • /fs-fix <your instruction> — agent fixes with your specific guidance
  • Push commits directly — review re-runs automatically on push
  • /fs-fix-stop — disable automatic fix runs for this PR
Previous run (6)

Review

Findings

Medium

  • [dry-run preview incomplete] internal/cli/repos.go:714 — In the bootstrap dry-run case (new manifest + new repos + --gitlab-url + --dry-run), the function returns early at line 714 before reaching the --gitlab-url block at line 720. The user sees "Install complete: 1 to add ..." but gets no indication that gitlab.url would be set. Additionally, even if the early return were bypassed, AddToManifest does not call EnsurePlatform in dry-run mode, so manifest.GitLab would remain nil and the code would fall through to the misleading warning "no GitLab repos are in the manifest." This means the primary use case described in issue feat(cli): add --gitlab-url flag to repos install for GitLab bootstrapping #6684 (bootstrapping a new manifest with --gitlab-url) produces an incomplete dry-run preview.
    Remediation: Move the --gitlab-url dry-run preview message into the early-return block (lines 710–715), or emit the preview before the early return when opts.gitlabURL is set and the forge is GitLab.

Low

  • [test-adequacy] internal/cli/repos_test.go:2106TestRunReposInstall_GitLabURLDryRun tests dry-run with an existing manifest that already has a gitlab: section, but does not test the bootstrap dry-run case (new manifest + --forge gitlab + --gitlab-url + --dry-run). This is the gap that allows the medium-severity finding above to pass undetected.
    Remediation: Add a test that bootstraps a new manifest with --forge gitlab, --gitlab-url, and --dry-run=true, and verify that the function communicates the URL would be set.

  • [missing-doc] docs/guides/dev/cli-internals.md:78 — The tree-structured flag listing for repos install does not include the new --gitlab-url flag. The listing was already missing other GitLab-specific flags (--gitlab-bot-token) and several others, so this is consistent with pre-existing selective incompleteness rather than a regression caused by this PR.
    Remediation: Add --gitlab-url <url> and --gitlab-bot-token <token> entries to the repos install flag tree in cli-internals.md.

  • [code organization] internal/cli/repos.go:723 — The opts.gitlabURL != "" guard is repeated in both the if and else if conditions. The idiomatic Go pattern is a single outer if opts.gitlabURL != "" block with an inner if manifest.GitLab != nil / else branch.


Next steps:

  • /fs-fix — agent addresses review findings automatically
  • /fs-fix <your instruction> — agent fixes with your specific guidance
  • Push commits directly — review re-runs automatically on push
  • /fs-fix-stop — disable automatic fix runs for this PR
Previous run (7)

Review

Findings

Medium

  • [dry-run contract violation] internal/cli/repos.go:723 — The --gitlab-url block writes to the manifest via repos.SetDefault() without checking opts.dryRun. When run with --dry-run against an existing manifest that already has a gitlab: section, the URL is written to disk despite --dry-run promising preview-only behavior. Every other write in runReposInstall is guarded by !opts.dryRun (e.g., the fullsend_ref writeback and per-repo manifest writeback). The bootstrap path avoids the bug because AddToManifest returns early in dry-run mode before calling EnsurePlatform, so manifest.GitLab remains nil and the condition short-circuits. But the override-existing-URL path reaches this block in dry-run mode and writes.
    Remediation: Guard the SetDefault call and in-memory mutation with if !opts.dryRun, and optionally print a preview message in the dry-run case: printer.StepDone(fmt.Sprintf("Would set gitlab.url=%s in manifest", opts.gitlabURL)).

Low

  • [silent-no-op] internal/cli/repos.go:723--gitlab-url is silently ignored when manifest.GitLab is nil (e.g., when passed with --forge github or against a GitHub-only manifest with no repoFilter). No warning or error is returned to the user, so their intent to set the GitLab URL goes unacknowledged.
    Remediation: Add an else branch when opts.gitlabURL != "" and manifest.GitLab == nil that returns an error or calls printer.StepWarn to inform the user the flag had no effect.

  • [test-adequacy] internal/cli/repos_test.go:2049 — No test covers --dry-run + --gitlab-url. TestRunReposInstall_GitLabURLOverridesExisting tests the override path but not with dryRun: true, so the dry-run contract violation is not caught by any test.
    Remediation: Add a test with dryRun: true and gitlabURL set against an existing GitLab manifest, asserting the manifest on disk is unchanged.

  • [missing-doc] docs/guides/getting-started/repo-management.md:96 — The GitLab URL section describes the env-var resolution chain (FULLSEND_GITLAB_URLGITLAB_API_URLCI_SERVER_URL) but does not mention the new --gitlab-url flag as another way to set gitlab.url in the manifest during bootstrapping.
    Remediation: Add a note that --gitlab-url can be passed to repos install to set gitlab.url during bootstrapping.


Next steps:

  • /fs-fix — agent addresses review findings automatically
  • /fs-fix <your instruction> — agent fixes with your specific guidance
  • Push commits directly — review re-runs automatically on push
  • /fs-fix-stop — disable automatic fix runs for this PR

fullsend-ai-review[bot]

This comment was marked as outdated.

@fullsend-ai-coder

fullsend-ai-coder Bot commented Sep 8, 2026

Copy link
Copy Markdown
Contributor Author

🤖 Finished Fix · ✅ Success · Started 1:30 AM UTC · Completed 1:42 AM UTC

Commit: 9e029bd · View workflow run →

Runtime: claude · Model: opus → claude-opus-4-6 · Effort: high · Cost: $1.84

- Guard --gitlab-url SetDefault with dryRun check to prevent writing
  to the manifest during dry-run mode
- Warn when --gitlab-url is provided but no GitLab repos are in the
  manifest (silent-no-op)
- Add TestRunReposInstall_GitLabURLDryRun to cover the dry-run +
  --gitlab-url path
- Document --gitlab-url in the GitLab URL section of
  docs/guides/getting-started/repo-management.md

Addresses #7087
@fullsend-ai-coder

Copy link
Copy Markdown
Contributor Author

🔧 Fix agent — iteration 1 (bot-triggered)

Addressed all four review findings: guarded --gitlab-url manifest write with dry-run check, added warning for silent no-op when no GitLab repos exist, added dry-run test coverage, and documented the flag in the getting-started guide

Fixed (4):

  1. dry-run contract violation (internal/cli/repos.go): Guarded the --gitlab-url SetDefault call and in-memory mutation with opts.dryRun check; in dry-run mode, prints a preview message instead of writing to the manifest
  2. silent-no-op (internal/cli/repos.go): Added an else branch that calls printer.StepWarn when --gitlab-url is provided but manifest.GitLab is nil, informing the user the flag had no effect
  3. test-adequacy (internal/cli/repos_test.go): Added TestRunReposInstall_GitLabURLDryRun that sets dryRun: true with an existing GitLab manifest and asserts the URL on disk is unchanged after the run
  4. missing-doc (docs/guides/getting-started/repo-management.md): Added a note in the GitLab URL section mentioning that --gitlab-url can be passed to fullsend repos install to set gitlab.url during bootstrapping

Tests: passed

Next steps:

  • /fs-review — request a re-review of the changes
  • /fs-fix <your instruction> — run another fix pass with specific guidance
  • Push commits directly — review re-runs automatically on push
    Updated by fullsend fix agent

@fullsend-ai-review

fullsend-ai-review Bot commented Sep 8, 2026

Copy link
Copy Markdown

🤖 Finished Review · ✅ Success · Started 1:43 AM UTC · Completed 1:58 AM UTC

Commit: e9a7322 · View workflow run →

Runtime: claude · Model: opus → claude-opus-4-6 · Effort: high · Cost: $3.81

fullsend-ai-review[bot]

This comment was marked as outdated.

fullsend-ai-review[bot]

This comment was marked as outdated.

@fullsend-ai-coder

fullsend-ai-coder Bot commented Sep 8, 2026

Copy link
Copy Markdown
Contributor Author

🤖 Finished Fix · ✅ Success · Started 2:00 AM UTC · Completed 2:10 AM UTC

Commit: e9a7322 · View workflow run →

Runtime: claude · Model: opus → claude-opus-4-6 · Effort: high · Cost: $1.72

The dry-run bootstrap path (new manifest + --forge gitlab + --gitlab-url)
returned early before the --gitlab-url block, so users saw no preview of
the URL being set. Emit the preview message inside the early-return block.

Also restructure the --gitlab-url conditional to use a single outer guard
instead of repeating the check, add a bootstrap dry-run test, and document
--gitlab-url and --gitlab-bot-token in the cli-internals flag tree.

Addresses #7087
@fullsend-ai-coder

Copy link
Copy Markdown
Contributor Author

🔧 Fix agent — iteration 2 (bot-triggered)

Addressed all four review findings: added dry-run preview for --gitlab-url in the bootstrap early-return path, added a bootstrap dry-run test, documented --gitlab-url and --gitlab-bot-token in cli-internals.md flag tree, and restructured the duplicated guard into idiomatic nested conditionals

Fixed (4):

  1. dry-run preview incomplete (internal/cli/repos.go): Emitted the dry-run preview message for --gitlab-url inside the early-return block (line 710) so the bootstrap dry-run case shows 'Would set gitlab.url=...' before returning
  2. test-adequacy (internal/cli/repos_test.go): Added TestRunReposInstall_GitLabURLBootstrapDryRun that bootstraps a new manifest with --forge gitlab, --gitlab-url, and --dry-run, and verifies the function succeeds without writing the manifest to disk
  3. missing-doc (docs/guides/dev/cli-internals.md): Added --gitlab-url and --gitlab-bot-token entries to the repos install flag tree in cli-internals.md
  4. code organization (internal/cli/repos.go): Restructured the --gitlab-url conditional to use a single outer if opts.gitlabURL != '' block with nested if manifest.GitLab != nil / else branches, eliminating the duplicated guard

Tests: passed

Decision points
  • Place dry-run preview inside early-return block rather than moving the --gitlab-url block before the early return (alternatives: Emit preview inside early-return block, Move entire --gitlab-url block before early return, Remove the early return entirely; rationale: Adding the preview inside the early-return block is the smallest change that fixes the issue. Moving or removing the early return would affect other code paths unnecessarily.)

Next steps:

  • /fs-review — request a re-review of the changes
  • /fs-fix <your instruction> — run another fix pass with specific guidance
  • Push commits directly — review re-runs automatically on push
    Updated by fullsend fix agent

@fullsend-ai-review

fullsend-ai-review Bot commented Sep 8, 2026

Copy link
Copy Markdown

🤖 Finished Review · ✅ Success · Started 2:12 AM UTC · Completed 2:27 AM UTC

Commit: b770452 · View workflow run →

Runtime: claude · Model: opus → claude-opus-4-6 · Effort: high · Cost: $4.38

fullsend-ai-review[bot]

This comment was marked as outdated.

@fullsend-ai-coder

fullsend-ai-coder Bot commented Sep 8, 2026

Copy link
Copy Markdown
Contributor Author

🤖 Finished Fix · ✅ Success · Started 2:29 AM UTC · Completed 2:38 AM UTC

Commit: b770452 · View workflow run →

Runtime: claude · Model: opus → claude-opus-4-6 · Effort: high · Cost: $1.51

…rence

Extract the --gitlab-url dry-run announcement into a shared
announceGitlabURLDryRun helper so the message text and guard are
defined exactly once, eliminating divergent duplication between the
early-return path and the main --gitlab-url handler.

Mark the --gitlab-url action item in gitlab-implementation.md as
implemented now that this PR delivers the feature.

Note: pre-commit hooks were not run. pre-commit could not complete
(infrastructure failure — sandbox cannot fetch remote hook repos),
and the hooks were run directly instead (gofmt, go vet, go build).

Addresses #7087
@fullsend-ai-coder

Copy link
Copy Markdown
Contributor Author

🔧 Fix agent — iteration 3 (bot-triggered)

Addressed both review findings: (1) deduplicated the dry-run message by extracting announceGitlabURLDryRun helper, (2) marked the --gitlab-url action item as implemented in gitlab-implementation.md

Fixed (2):

  1. Duplicated dry-run message string across two code paths with divergent guards (internal/cli/repos.go): Extracted the --gitlab-url dry-run announcement into a shared announceGitlabURLDryRun helper function so the message text and guard (gitlabURL != "") are defined exactly once, eliminating duplication between the early-return path (line 712) and the main --gitlab-url handler (line 729)
  2. Stale reference lists --gitlab-url as a future action item (docs/problems/gitlab-implementation.md): Annotated the --gitlab-url action item in the Install Command Changes section as implemented with a reference to PR feat(#6684): add --gitlab-url flag to repos install #7087

Tests: passed

Strategy change: Iteration 3 (at escalation threshold): previous iterations addressed functional bugs (dry-run contract violation, bootstrap preview). This iteration's findings are organizational (code duplication, stale docs) requiring a different approach — extracting shared code rather than adding new conditional branches.

Decision points
  • Helper function vs constant for deduplication (alternatives: Extract a helper function encapsulating both guard and message, Extract only a format-string constant; rationale: A helper function addresses the reviewer's request to express the message and its guard exactly once. A constant would only deduplicate the string, not the guard logic.)

Next steps:

  • /fs-review — request a re-review of the changes
  • /fs-fix <your instruction> — run another fix pass with specific guidance
  • Push commits directly — review re-runs automatically on push
    Updated by fullsend fix agent

@fullsend-ai-review

fullsend-ai-review Bot commented Sep 8, 2026

Copy link
Copy Markdown

🤖 Finished Review · ✅ Success · Started 2:40 AM UTC · Completed 3:01 AM UTC

Commit: 9c9b3dd · View workflow run →

Runtime: claude · Model: opus → claude-opus-4-6 · Effort: high · Cost: $5.16

fullsend-ai-review[bot]

This comment was marked as outdated.

When --gitlab-url was provided without --forge on a manifest that
already contained GitHub repos, the forge-inference logic picked
ForgeGitHub (because ghHasRepos was true and glHasRepos was false),
causing the new repo to land in the wrong platform section.

Add an explicit guard before the general inference block so that
--gitlab-url always implies ForgeGitLab. Update the test to cover
the mixed-manifest case with existing GitHub repos.

Note: pre-commit hooks were not run. pre-commit could not complete
(infrastructure failure), and the hooks were run directly (gofmt,
go vet) instead.

Addresses #7087
@fullsend-ai-coder

Copy link
Copy Markdown
Contributor Author

🔧 Fix agent — iteration 7 (human-triggered)

Fixed the remaining logic bug where --gitlab-url with a mixed GitHub/GitLab manifest would incorrectly infer ForgeGitHub. Added explicit forge inference guard and test coverage for the mixed-manifest case. Four previously-addressed findings confirmed as already fixed.

Fixed (3):

  1. --gitlab-url forge inference fails with mixed-manifest (existing GitHub repos) (internal/cli/repos.go): Added explicit guard before the general forge-inference block: when --gitlab-url is set and --forge is not, forgeName is set to ForgeGitLab immediately. This prevents manifests with existing GitHub repos from pulling the new repo into the wrong platform section.
  2. No test covers --gitlab-url without --forge on mixed manifest (internal/cli/repos_test.go): Refactored TestRunReposInstall_GitLabURLImpliesForge into subtests and added a 'manifest with existing GitHub repos' case that verifies the new repo lands in the GitLab section while preserving existing GitHub repos.
  3. EnsurePlatform comment implies forge inference is intentionally accidental (internal/cli/repos.go): Updated the comment on the EnsurePlatform block to clarify that forge inference is now explicit (via the new guard), and EnsurePlatform is only for setting the URL.

Disagreed (6):

  1. dry-run preview incomplete: Already fixed in a prior iteration. announceGitLabURLDryRun is called before the early return at line 725. The bootstrap dry-run case (new manifest + --gitlab-url + --dry-run) correctly shows the preview message.
  2. Factory captures stale/empty GitLab URL: Already fixed in a prior iteration. The in-memory assignment (manifest.GitLab.URL = opts.gitlabURL) happens at line 601, before factory creation at line 608.
  3. Config Schema Changes section shows gitlab_instance_url: Already fixed in a prior iteration. The Config Schema Changes section now shows gitlab.url in repos.yaml with a note about the divergence from the original design.
  4. Phase 4 description inconsistent with implementation: Already fixed in a prior iteration. Phase 4 already has a parenthetical note: '(implementation uses gitlab.url in the repos manifest — see Config Schema Changes below)'.
  5. Rebase and squash commits: Agent constraints prohibit amending existing commits or running destructive git operations. A human can squash via the GitHub merge strategy (squash-and-merge) or manually rebase.
  6. Reply to and resolve all conversations: Agent constraints prohibit posting comments on PRs. The post-script handles PR comment posting based on this structured output.

Tests: passed

Strategy change: Iteration 7 (above threshold 3): focused exclusively on the one remaining unfixed finding (forge inference with mixed manifests) rather than re-examining all findings equally, since prior iterations had already addressed the others.

Decision points
  • Where to place the --gitlab-url forge inference guard (alternatives: Before the general inference block (chosen), Inside the inference block as an additional condition, As a separate post-inference override; rationale: Placing the guard before the general inference block is cleanest: it short-circuits entirely when --gitlab-url is set, making the intent explicit and avoiding any ambiguity with the existing inference logic.)

Next steps:

  • /fs-review — request a re-review of the changes
  • /fs-fix <your instruction> — run another fix pass with specific guidance
  • Push commits directly — review re-runs automatically on push
    Updated by fullsend fix agent

@fullsend-ai-review

fullsend-ai-review Bot commented Sep 9, 2026

Copy link
Copy Markdown

🤖 Finished Review · ✅ Success · Started 1:33 AM UTC · Completed 1:49 AM UTC

Commit: 8e75d55 · View workflow run →

Runtime: claude · Model: opus → claude-opus-4-6 · Effort: high · Cost: $4.49

@fullsend-ai-review
fullsend-ai-review Bot dismissed stale reviews from themself September 9, 2026 01:49

Superseded by updated review

@fullsend-ai-review fullsend-ai-review Bot added the ready-for-merge All reviewers approved — ready to merge label Sep 9, 2026
@ggallen
ggallen enabled auto-merge September 9, 2026 12:00
@ggallen
ggallen added this pull request to the merge queue Sep 9, 2026
@github-merge-queue
github-merge-queue Bot removed this pull request from the merge queue due to failed status checks Sep 9, 2026
@ggallen
ggallen added this pull request to the merge queue Sep 9, 2026
@github-merge-queue
github-merge-queue Bot removed this pull request from the merge queue due to failed status checks Sep 9, 2026
@ggallen
ggallen added this pull request to the merge queue Sep 9, 2026
@github-merge-queue
github-merge-queue Bot removed this pull request from the merge queue due to failed status checks Sep 9, 2026
@ggallen
ggallen added this pull request to the merge queue Sep 9, 2026
Merged via the queue into main with commit fd9c57f Sep 9, 2026
54 checks passed
@ggallen
ggallen deleted the agent/6684-gitlab-url-flag branch September 9, 2026 13:37
@fullsend-ai-retro

fullsend-ai-retro Bot commented Sep 9, 2026

Copy link
Copy Markdown

🤖 Finished Retro · ✅ Success · Started 1:38 PM UTC · Completed 1:53 PM UTC

Commit: 8e75d55 · View workflow run →

Runtime: claude · Model: opus → claude-opus-4-6 · Effort: high · Cost: $3.28

@fullsend-ai-retro

Copy link
Copy Markdown

Retro: PR #7087--gitlab-url flag for repos install

Timeline

Stage Time (UTC) Cost Outcome
Triage Sep 7 21:09–21:14 $1.14 Good summary, missed dry-run/forge-interaction contracts
Code Sep 8 00:50–01:07 $5.46 PR created, 7 files, 4 tests
Review×8 + Fix×7 Sep 8 01:09 – Sep 9 01:49 $52.63 5 bot iterations hit cap; 2 human /fs-fix interventions
Human approval + merge Sep 9 11:58–13:37 Merged with 8 commits
Total ~37 hours wall clock $59.23

This was a well-scoped single-flag CLI addition (314 lines added) that required 8 review/fix iterations before approval — hitting the bot iteration cap twice and requiring human intervention. The review agent found genuine bugs (dry-run contract violation, stale URL ordering, forge inference logic error) but also surfaced new low-severity findings each iteration (naming conventions, code organization, British English spelling), preventing convergence.

Key observations

  1. Review-fix loop did not converge within the bot cap. Each of the first 5 bot-triggered reviews surfaced new low-severity findings. Reviews 1–2 found medium bugs (dry-run handling); Reviews 3–5 found only low issues (code organization, doc staleness, spelling). The review agent never approved despite all medium findings being fixed — new low-severity findings kept triggering CHANGES_REQUESTED.

  2. The most impactful bug was caught late. The forge-inference logic error (--gitlab-url with a mixed GitHub/GitLab manifest incorrectly infers ForgeGitHub) was present in the initial code but wasn't flagged until Review 6. By then, 5 iterations had been spent on lower-impact issues.

  3. Fix agent falsely claimed a finding was "already fixed." In Fix 6, the fix agent marked 7 of 10 findings as "already fixed in prior iterations" — but the forge inference bug was not actually fixed, as Review 7 re-raised the identical finding. Fix 7 finally addressed it correctly.

  4. Code agent didn't account for existing CLI contracts. The initial implementation didn't handle --dry-run for the new mutation path, didn't validate --forge/--gitlab-url cross-flag consistency, and had a factory-ordering issue where newForgeClientFactory captured a stale URL. These accounted for 4+ review findings and at least 2 fix iterations.

Evidence for existing open issues

Proposals filed

  1. Code agent should verify new CLI code paths respect existing behavioral contracts
  2. Fix agent should verify "already fixed" claims against current code state

Proposals filed

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

needs-human Agent loop needs human intervention ready-for-merge All reviewers approved — ready to merge ready-for-review Triggers review agent dispatch risk/moderate PR risk: moderate

Projects

None yet

Development

Successfully merging this pull request may close these issues.

feat(cli): add --gitlab-url flag to repos install for GitLab bootstrapping

1 participant