feat(cli): wire --max-depth and --max-resources flags for transitive resolution by ggallen · Pull Request #2022 · fullsend-ai/fullsend

ggallen · 2026-06-08T13:30:22Z

Summary

Phase 2, PR 3 of ADR-0038 (Universal Harness Access). Wires the transitive dependency resolver (merged in #1923) into the CLI run flow.

Adds --max-depth flag (default 10) to control transitive dependency resolution depth; --max-depth 0 disables transitive resolution entirely
Adds --max-resources flag (default 50) to cap total remote resources fetched per harness
Passes both values through ResolveOpts to the resolver; previously MaxDepth defaulted to Go's zero value (0), which disabled transitive resolution — now it defaults to DefaultMaxDepth (10)

Depends on: #1923 (merged)

Changes

File	Action
`internal/cli/run.go`	Add `--max-depth` / `--max-resources` flags, thread through `runAgent()` signature, pass to `ResolveOpts`
`internal/cli/run_test.go`	Add flag registration tests for both new flags

Test plan

go test ./internal/cli/... — all tests pass (including new flag registration tests)
go test ./internal/resolve/... — all 28 Phase 1 + Phase 2 tests pass unchanged
go vet ./... — clean
make lint — clean
Review: flag defaults match resolve.DefaultMaxDepth (10) and resolve.DefaultMaxResources (50)
Review: MaxDepth=0 still disables transitive resolution (Phase 1 behavior preserved)

🤖 Generated with Claude Code

github-actions · 2026-06-08T13:31:56Z

Site preview

Preview: https://317e80e2-site.fullsend-ai.workers.dev

Commit: ec59225a689ec59f6dd59ea8e69e4fd32ff4a323

ggallen · 2026-06-08T14:21:07Z

/fs-review

ggallen · 2026-06-08T14:27:34Z

/fs-review

ggallen · 2026-06-08T14:59:24Z

/fs-review

fullsend-ai-review · 2026-06-08T15:01:24Z

🤖 Review · Started 3:01 PM UTC
Commit: d0ac11b · View workflow run →

fullsend-ai-review · 2026-06-08T15:08:46Z

Review

Findings

Info

[missing-authorization] No linked issue for this PR. The PR body references ADR-0038 (accepted), a Phase 2 implementation plan, and prior merged PR feat: add transitive dependency resolution to URL resolver #1923, which provide design authorization context. A tracking issue would improve traceability for this work stream.
[naming-convention] internal/cli/run.go — The new resolveFlags struct uses a different suffix than the existing statusOpts pattern. This is a defensible choice since resolve.ResolveOpts already exists as the downstream type, and resolveFlags distinguishes CLI-layer concerns from resolver options.
[prior-finding-resolved] internal/cli/run_test.go — The prior low-severity finding about missing --max-depth 0 boundary test is resolved. TestRunCommand_AcceptsZeroMaxDepth was added and correctly validates that maxDepth=0 is accepted (not rejected by validation). The TestRunCommand_RejectsNegativeMaxResources test also addresses the prior gap for negative max-resources values.

Previous run

Review

Findings

Low

[edge-case] internal/cli/run_test.go — No CLI-level test for the --max-depth 0 boundary value, which has the special semantic of disabling transitive resolution entirely. The downstream resolve package covers this case (TestResolveHarness_ZeroMaxDepthDisablesTransitive), so the behavior is verified at the library level, but a CLI integration test confirming maxDepth=0 is accepted and propagated would strengthen coverage of this important boundary.

Info

[missing-authorization] No linked issue for this PR. The PR body references ADR-0038 (accepted), a Phase 2 implementation plan, and prior merged PR feat: add transitive dependency resolution to URL resolver #1923, which provide design authorization context. A tracking issue would improve traceability for this work stream.
[naming-convention] internal/cli/run.go — The new resolveFlags struct uses a different suffix than the existing statusOpts pattern. This is a defensible choice since resolve.ResolveOpts already exists as the downstream type, and resolveFlags distinguishes CLI-layer concerns from resolver options.

Previous run (2)

Review

Findings

Low

[test-adequacy] internal/cli/run_test.go — The validation tests cover --max-depth -1 (rejected) and --max-resources 0 (rejected), but do not test --max-resources with a negative value (e.g., -1). The validation maxResources < 1 catches it, but an explicit boundary test would guard against regression if the check were accidentally narrowed to == 0.
Remediation: Add a test case with --max-resources -1 asserting the error contains --max-resources must be >= 1.

Info

[prior-finding-resolved] internal/cli/run.go:56 — Both prior low-severity findings (parameter-ordering and flag-declaration-grouping) are resolved. The new resolveFlags struct groups offline, maxDepth, and maxResources exactly as recommended, reducing the runAgent parameter count and following the established statusOpts pattern.
[scope-alignment] internal/cli/run.go — No linked issue, but the change traces to ADR-0038 Phase 2 and docs/plans/universal-harness-access-phase2.md (PR 3 specification). Authorization is implicit in the ADR/plan chain.

Previous run (3)

Review

Findings

Low

[parameter-ordering] internal/cli/run.go:112 — The runAgent function signature now has 15 positional parameters. While the new maxDepth and maxResources parameters are placed logically (after offline, before sOpts), the growing parameter count increases the risk of argument transposition.
Remediation: Consider consolidating related parameters into an options struct in a follow-up (e.g., a resolveFlags struct grouping offline, maxDepth, maxResources).
[flag-declaration-grouping] internal/cli/run.go:74 — The new maxDepth and maxResources variables are conceptually related to offline (all control remote resource fetching) but are declared separately. For consistency with existing patterns (statusOpts struct), consider grouping them in a follow-up.

Info

[prior-finding-resolved] internal/cli/run_test.go:103 — The prior low-severity finding about missing validation tests is now resolved. TestRunCommand_RejectsNegativeMaxDepth and TestRunCommand_RejectsZeroMaxResources exercise the runAgent validation logic with invalid flag values and assert the expected error messages.
[scope-alignment] internal/cli/run.go — No linked issue, but the change traces to ADR-0038 Phase 2 and docs/plans/universal-harness-access-phase2.md (PR 3 specification). Authorization is implicit in the ADR/plan chain.

Previous run (4)

Review

Findings

Medium

[missing-flag-documentation] docs/guides/dev/cli-internals.md:34 — The CLI command tree (lines 34–45) lists every fullsend run flag but is missing the new --max-depth and --max-resources flags added in this PR.
Remediation: Add entries after the --offline line: --max-depth <int> (maximum dependency depth for transitive resolution, 0 disables) and --max-resources <int> (maximum total remote resources per harness).

Low

[edge-case] internal/cli/run.go:99 — The --max-depth flag help text documents that 0 disables transitive resolution, but negative values are silently treated as the default (10) by resolve.go. A user passing --max-depth -1 gets depth 10 rather than an error. Unlikely in practice but an undocumented behavior asymmetry.
[missing-flag-documentation] docs/guides/user/running-agents-locally.md — The user guide documents other optional fullsend run flags but does not mention --max-depth or --max-resources. Consider adding a brief note for users managing harnesses with URL-referenced transitive skill dependencies.

Info

[scope-alignment] internal/cli/run.go — No linked issue, but the change traces to ADR-0038 Phase 2 and docs/plans/universal-harness-access-phase2.md (PR 3 specification at lines 256–280). Authorization is implicit in the ADR/plan chain.

Previous run

Review

Findings

Low

[edge-case] internal/cli/run_test.go — No CLI-level test for the --max-depth 0 boundary value, which has the special semantic of disabling transitive resolution entirely. The downstream resolve package covers this case (TestResolveHarness_ZeroMaxDepthDisablesTransitive), so the behavior is verified at the library level, but a CLI integration test confirming maxDepth=0 is accepted and propagated would strengthen coverage of this important boundary.

Info

[missing-authorization] No linked issue for this PR. The PR body references ADR-0038 (accepted), a Phase 2 implementation plan, and prior merged PR feat: add transitive dependency resolution to URL resolver #1923, which provide design authorization context. A tracking issue would improve traceability for this work stream.
[naming-convention] internal/cli/run.go — The new resolveFlags struct uses a different suffix than the existing statusOpts pattern. This is a defensible choice since resolve.ResolveOpts already exists as the downstream type, and resolveFlags distinguishes CLI-layer concerns from resolver options.

Previous run (2)

Review

Findings

Low

[test-adequacy] internal/cli/run_test.go — The validation tests cover --max-depth -1 (rejected) and --max-resources 0 (rejected), but do not test --max-resources with a negative value (e.g., -1). The validation maxResources < 1 catches it, but an explicit boundary test would guard against regression if the check were accidentally narrowed to == 0.
Remediation: Add a test case with --max-resources -1 asserting the error contains --max-resources must be >= 1.

Info

[prior-finding-resolved] internal/cli/run.go:56 — Both prior low-severity findings (parameter-ordering and flag-declaration-grouping) are resolved. The new resolveFlags struct groups offline, maxDepth, and maxResources exactly as recommended, reducing the runAgent parameter count and following the established statusOpts pattern.
[scope-alignment] internal/cli/run.go — No linked issue, but the change traces to ADR-0038 Phase 2 and docs/plans/universal-harness-access-phase2.md (PR 3 specification). Authorization is implicit in the ADR/plan chain.

Previous run (3)

Review

Findings

Low

[parameter-ordering] internal/cli/run.go:112 — The runAgent function signature now has 15 positional parameters. While the new maxDepth and maxResources parameters are placed logically (after offline, before sOpts), the growing parameter count increases the risk of argument transposition.
Remediation: Consider consolidating related parameters into an options struct in a follow-up (e.g., a resolveFlags struct grouping offline, maxDepth, maxResources).
[flag-declaration-grouping] internal/cli/run.go:74 — The new maxDepth and maxResources variables are conceptually related to offline (all control remote resource fetching) but are declared separately. For consistency with existing patterns (statusOpts struct), consider grouping them in a follow-up.

Info

[prior-finding-resolved] internal/cli/run_test.go:103 — The prior low-severity finding about missing validation tests is now resolved. TestRunCommand_RejectsNegativeMaxDepth and TestRunCommand_RejectsZeroMaxResources exercise the runAgent validation logic with invalid flag values and assert the expected error messages.
[scope-alignment] internal/cli/run.go — No linked issue, but the change traces to ADR-0038 Phase 2 and docs/plans/universal-harness-access-phase2.md (PR 3 specification). Authorization is implicit in the ADR/plan chain.

Previous run (4)

Review

Findings

Low

[test-adequacy] internal/cli/run_test.go:89 — The new tests verify flag existence and default values but do not exercise the validation logic in runAgent that rejects maxDepth < 0 or maxResources < 1. A regression removing CLI-level validation would silently fall back to the resolver's internal sentinel behavior (negative maxDepth → DefaultMaxDepth) rather than surfacing an error to the user.
Remediation: Add test cases that invoke the command with invalid flag values (e.g. --max-depth -1, --max-resources 0) and assert the expected error messages.
[parameter-ordering] internal/cli/run.go:109 — The runAgent function signature now has 15 positional parameters. While the new maxDepth and maxResources parameters are placed logically (after offline, before sOpts), the growing parameter count is a code smell that increases the risk of argument transposition.
Remediation: Consider consolidating related parameters into an options struct in a follow-up (e.g., a resolveFlags struct grouping offline, maxDepth, maxResources).

Info

[prior-finding-resolved] internal/cli/run.go:117 — The prior low-severity finding about --max-depth -1 being silently treated as the default (10) is now resolved. The CLI validates maxDepth < 0 and returns an explicit error before the value reaches the resolver.
[scope-alignment] internal/cli/run.go — No linked issue, but the change traces to ADR-0038 Phase 2 and docs/plans/universal-harness-access-phase2.md (PR 3 specification). Authorization is implicit in the ADR/plan chain.

Previous run (5)

Review

Findings

Medium

[missing-flag-documentation] docs/guides/dev/cli-internals.md:34 — The CLI command tree (lines 34–45) lists every fullsend run flag but is missing the new --max-depth and --max-resources flags added in this PR.
Remediation: Add entries after the --offline line: --max-depth <int> (maximum dependency depth for transitive resolution, 0 disables) and --max-resources <int> (maximum total remote resources per harness).

Low

[edge-case] internal/cli/run.go:99 — The --max-depth flag help text documents that 0 disables transitive resolution, but negative values are silently treated as the default (10) by resolve.go. A user passing --max-depth -1 gets depth 10 rather than an error. Unlikely in practice but an undocumented behavior asymmetry.
[missing-flag-documentation] docs/guides/user/running-agents-locally.md — The user guide documents other optional fullsend run flags but does not mention --max-depth or --max-resources. Consider adding a brief note for users managing harnesses with URL-referenced transitive skill dependencies.

Info

[scope-alignment] internal/cli/run.go — No linked issue, but the change traces to ADR-0038 Phase 2 and docs/plans/universal-harness-access-phase2.md (PR 3 specification at lines 256–280). Authorization is implicit in the ADR/plan chain.

fullsend-ai-review · 2026-06-08T15:08:52Z

🤖 Finished Review · ✅ Success · Started 3:01 PM UTC · Completed 3:08 PM UTC
Commit: d0ac11b · View workflow run →

fullsend-ai-review · 2026-06-08T15:31:22Z

🤖 Finished Review · ✅ Success · Started 3:31 PM UTC · Completed 3:38 PM UTC
Commit: d0ac11b · View workflow run →

fullsend-ai-review · 2026-06-08T17:35:04Z

🤖 Finished Review · ✅ Success · Started 5:35 PM UTC · Completed 5:43 PM UTC
Commit: d0ac11b · View workflow run →

fullsend-ai-review · 2026-06-08T17:53:32Z

🤖 Finished Review · ✅ Success · Started 5:53 PM UTC · Completed 6:00 PM UTC
Commit: d0ac11b · View workflow run →

fullsend-ai-review · 2026-06-08T18:04:42Z

🤖 Finished Review · ✅ Success · Started 6:04 PM UTC · Completed 6:11 PM UTC
Commit: d0ac11b · View workflow run →

…resolution Phase 2, PR 3 of ADR-0038. Connects the transitive dependency resolver (merged in fullsend-ai#1923) to the CLI by adding flags and passing them through ResolveOpts. Previously MaxDepth defaulted to 0 (disabled); now it defaults to 10, enabling transitive resolution for URL-referenced skills. - Add --max-depth (default 10) and --max-resources (default 50) flags - Group offline, maxDepth, maxResources into resolveFlags struct - Validate flag values: reject negative --max-depth and --max-resources < 1 - Add boundary tests: --max-depth 0 accepted, -1 rejected; --max-resources 0 and -1 rejected - Document both flags in cli-internals.md and running-agents-locally.md Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> Signed-off-by: Greg Allen <gallen@redhat.com>

fullsend-ai-review · 2026-06-08T18:19:50Z

🤖 Finished Review · ✅ Success · Started 6:19 PM UTC · Completed 6:27 PM UTC
Commit: d0ac11b · View workflow run →

ralphbean

LGTM.

fullsend-ai-retro · 2026-06-08T20:37:24Z

🤖 Retro · Started 8:37 PM UTC
Commit: d0ac11b · View workflow run →

fullsend-ai-retro · 2026-06-08T20:52:00Z

Retro: PR #2022 — `feat(cli): wire --max-depth and --max-resources flags`

Timeline: Human-authored PR by ggallen, opened 2026-06-08T13:30Z, merged 20:35Z (7 hours). 4 files changed. 9 review workflow runs total: 3 failed, 6 succeeded. 6 review rounds before all findings resolved. Final human approval by ralphbean.

Key findings:

CLI version resolution served dev-vendored build after v0.15.0 release (net-new). The reusable review workflow was updated to pass --status-repo flags, and v0.15.0 (which added those flags) was released at 14:14 UTC. However, the latest version resolution mechanism continued serving a dev-vendored build for ~45 minutes after the release. Runs at 14:21 and 14:27 UTC still got the old binary, causing 3 consecutive failures. The author had to manually retry /fs-review three times. Proposing one issue for this.
Whack-a-mole low-severity findings across review rounds (already tracked). Each of the 6 review rounds surfaced 1 new Low-severity finding (test-adequacy, parameter-ordering, flag-grouping, boundary tests). The bot correctly tracked resolutions with [prior-finding-resolved] tags, but the pattern of discovering findings incrementally extended the review cycle unnecessarily. Skipping proposal — extensively covered by existing issues: Review agent surfaces new low-severity findings across multiple approved passes #1367 (new low findings across passes), Review agent should catch all findings in the first pass to reduce rework cycles #1582 (catch all findings in first pass), Review agent should deduplicate findings across iterations on the same PR #1013 (deduplicate findings), Pass prior review findings to follow-up reviews for targeted verification #1552 (pass prior findings to follow-up reviews).
9 review runs for a 4-file PR (already tracked). Every push auto-triggered a new review run. Skipping proposal — covered by Review agent accumulates redundant internal iterations on clean diffs #1370 (redundant iterations on clean diffs) and Reduce unnecessary re-review runs on large PRs #1390 (reduce unnecessary re-reviews).
Failed runs produced no author-visible feedback (already tracked). When the first 3 review runs failed due to the CLI error, ggallen received no notification — they had to check Actions logs manually. Skipping proposal — covered by Failed review agent runs should update the status comment on the PR #837 (failed review runs should update status comment).

What went well: The review bot's finding-resolution tracking worked correctly across all 6 rounds. Each resolved finding was properly marked with [prior-finding-resolved]. The final round had only Info-level findings remaining. The bot never re-raised a Low+ finding that had already been fixed.

Proposals filed

CLI latest-version resolution can serve stale dev-vendored builds after a new release (in fullsend-ai/fullsend)

fullsend-ai-retro · 2026-06-08T20:52:01Z

🤖 Finished Retro · ✅ Success · Started 8:37 PM UTC · Completed 8:52 PM UTC
Commit: d0ac11b · View workflow run →

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

fullsend-ai-review Bot added the requires-manual-review Review requires human judgment label Jun 8, 2026

ggallen mentioned this pull request Jun 8, 2026

fix(statuscomment): use start comment author instead of GetAuthenticatedUser in analyzeTimeline #2027

Closed

ggallen force-pushed the feat/adr0038-phase2-cli-wiring branch from 6998b4e to 7c92e20 Compare June 8, 2026 15:29

github-actions Bot deployed to site-preview June 8, 2026 15:31 View deployment