feat: support --vendor-fullsend-binary in per-repo mode by waynesun09 · Pull Request #1011 · fullsend-ai/fullsend

waynesun09 · 2026-05-15T15:21:09Z

Summary

Removed --vendor-fullsend-binary from perOrgOnlyFlags so it works in both per-org and per-repo install modes
Parameterized VendorBinary(), VendorBinaryLayer, and vendorFullsendBinary() to accept an explicit repo target instead of hardcoding forge.ConfigRepoName
Added vendor binary wiring to runPerRepoInstall (including dry-run output)
Updated docs to remove "(per-org only)" from the flag description

Test plan

make go-test passes (cli and layers packages)
make go-vet clean
make lint passes
--vendor-fullsend-binary added to PerRepoAcceptsSharedFlags test table
Removed from PerRepoRejectsPerOrgFlags test table
VendorBinaryLayer tests updated with repo parameter

github-actions · 2026-05-15T15:21:26Z

fullsend review is working on this — view logs

github-actions · 2026-05-15T15:22:36Z

Site preview

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

Commit: 788f6ae257bcb99efda36f2d4935efab03096cd1

fullsend-ai-review · 2026-05-15T15:25:12Z

Review

Findings

No findings.

Previous run

Review

Findings

Low

[stale-doc] docs/guides/dev/cli-internals.md:114 — The Phase 5 diagram (lines 114-119), Mode Differences table (line 164), and Layer Stack section (lines 183-188) describe binary vendoring as per-org only. After this PR, per-repo mode also supports --vendor-fullsend-binary, writing to .fullsend/bin/fullsend in the target repo. The existing per-org descriptions are still correct, but omitting per-repo vendoring is incomplete and could mislead developers about the architecture.
Remediation: Update the Phase 5 diagram to add "vendor fullsend binary (optional)" under the per-repo section. Update the Mode Differences table row for Phase 5 to mention optional binary vendoring for per-repo. Add a note in the Layer Stack section that per-repo mode handles vendoring inline in runPerRepoInstall().

Previous run (2)

Review

Findings

Low

[docs-currency] docs/guides/dev/cli-internals.md:116 — The Phase 5 diagram lists "vendor fullsend binary" only under the per-org column, and the mode-differences table (line 164) describes per-org as "pushes workflows + binary" without a corresponding per-repo entry for vendoring. Now that per-repo supports --vendor-fullsend-binary, these sections are stale.
Remediation: Add "vendor fullsend binary" to the per-repo column in the Phase 5 diagram and update the table row to note per-repo vendoring support.

Previous run (3)

Review

Findings

No findings.

Previous run (4)

Review: #1011

Head SHA: 1c19e1f
Timestamp: 2026-05-15T00:00:00Z
Outcome: approve

Summary

This PR correctly parameterizes VendorBinary(), VendorBinaryLayer, and vendorFullsendBinary() to accept an explicit repo target instead of hardcoding forge.ConfigRepoName, enabling the --vendor-fullsend-binary flag in per-repo install mode. The change is well-scoped, consistent across all call sites, and properly tested. No correctness, security, or injection concerns were found.

Findings

Info

[style/conventions] docs/guides/admin/installation.md — The --skip-mint-check flag row was added to the shared flags table alongside --vendor-fullsend-binary. This is tangential to the PR's stated purpose but is a benign documentation improvement.

Footer

Outcome: approve
This review applies to SHA 1c19e1fc408e86fdb7595659975f0659de7b143b. Any push to the PR head clears this review and requires a new evaluation.

Previous run (5)

Review: #1011

Head SHA: 2822392
Timestamp: 2026-05-15T00:00:00Z
Outcome: request-changes

Summary

The change correctly parameterizes the vendor binary functions to accept an explicit repo target, enabling per-repo mode support. The per-org path preserves existing behavior by passing forge.ConfigRepoName. However, the e2e test in e2e/admin/admin_test.go still calls layers.VendorBinary with the old 4-argument signature, which will cause a compilation failure since the function now requires 5 arguments (owner and repo instead of just org).

Findings

High

[Correctness] e2e/admin/admin_test.go:549 — layers.VendorBinary(context.Background(), env.client, testOrg, tmpBinary.Name()) uses the old 4-parameter signature. After this PR, VendorBinary requires 5 parameters: (ctx, client, owner, repo, binaryPath). This will fail to compile.
Remediation: Update the call to pass the repo name as a separate argument, e.g. layers.VendorBinary(context.Background(), env.client, testOrg, forge.ConfigRepoName, tmpBinary.Name()) (or the appropriate target repo for the e2e test context).

Footer

Outcome: request-changes
This review applies to SHA 282239236774ec6dd1e88932cad12d769dae5af9. Any push to the PR head clears this review and requires a new evaluation.

Previous run (6)

Review: #1011

Head SHA: c9316ef
Timestamp: 2026-05-15T00:00:00Z
Outcome: request-changes

Summary

The change correctly parameterizes VendorBinary, VendorBinaryLayer, and vendorFullsendBinary to accept an explicit repo target, enabling --vendor-fullsend-binary in per-repo mode. The per-org path is preserved by passing forge.ConfigRepoName at the buildLayerStack call site. However, the e2e test at e2e/admin/admin_test.go:549 still calls layers.VendorBinary with the old 4-parameter signature and will fail to compile when the e2e build tag is active, breaking the e2e test suite.

Findings

High

[Correctness] e2e/admin/admin_test.go:549 — The VendorBinary function signature changed from (ctx, client, org, binaryPath) to (ctx, client, owner, repo, binaryPath), but this e2e test call site was not updated. This will cause a compilation error when running make e2e-test (build tag e2e). The PR's test plan only checked make go-test which excludes e2e tests.
Remediation: Update the call to layers.VendorBinary(context.Background(), env.client, testOrg, forge.ConfigRepoName, tmpBinary.Name()) (or the appropriate repo name for the e2e test context).

Info

[Style/conventions] internal/cli/admin.go — The per-repo path calls vendorFullsendBinary directly (line ~935 in the diff) rather than going through the layer stack like the per-org path does. This is a deliberate design choice (per-repo doesn't use buildLayerStack), but it means the cleanup-when-disabled behavior (removing stale vendored binaries) that VendorBinaryLayer.Install provides is not available in per-repo mode. This is acceptable for now since per-repo only vendors when the flag is explicitly set, but worth noting for future maintainers.

Footer

Outcome: request-changes
This review applies to SHA c9316ef321521d5e5be7dfcaadcbff4530834946. Any push to the PR head clears this review and requires a new evaluation.

Previous run (7)

Review: #1011

Head SHA: 88ac64d
Timestamp: 2026-05-15T00:00:00Z
Outcome: approve

Summary

Clean docs-only change that fixes an inaccurate description of --vendor-fullsend-binary (was incorrectly grouped with enrollment flags) and adds two missing flags (--skip-mint-check, --vendor-fullsend-binary) to the installation guide's flags table. All flag descriptions verified against the Go source in internal/cli/admin.go — defaults, behavior descriptions, and per-org-only annotations are accurate. No code changes, no security implications, no injection patterns detected in PR content.

Findings

No critical, high, medium, low, or info findings.

Footer

Outcome: approve
This review applies to SHA 88ac64d83e04de613ca7b74e8851754e92fd3297. Any push to the PR head clears this review and requires a new evaluation.

github-actions · 2026-05-15T15:35:27Z

fullsend review is working on this — view logs

fullsend-ai-review

See the review comment for full details.

github-actions · 2026-05-15T15:40:11Z

fullsend fix is working on this — view logs

github-actions · 2026-05-15T15:41:58Z

fullsend review is working on this — view logs

fullsend-ai-review

See the review comment for full details.

github-actions · 2026-05-15T15:46:40Z

fullsend fix is working on this — view logs

fullsend-ai-coder · 2026-05-15T15:53:06Z

🔧 Fix agent — iteration 1 (bot-triggered)

Addressed 1 of 1 review finding. Updated the e2e test call site to use the new VendorBinary(ctx, client, owner, repo, binaryPath) signature. Could not run Go compilation or tests due to Go version mismatch in sandbox (go.mod requires 1.26.0, sandbox has 1.24.13), but the fix is mechanical — adding forge.ConfigRepoName as the 4th argument to match the updated function signature.

Fixed (1):

e2e test uses old 4-param VendorBinary signature (e2e/admin/admin_test.go): Updated VendorBinary call at line 549 to pass owner and repo as separate arguments (testOrg, forge.ConfigRepoName) instead of just testOrg, matching the new 5-parameter function signature.

Tests: failed

_{Updated by fullsend fix agent}

Update VendorBinary call in e2e/admin/admin_test.go to pass owner and repo as separate arguments, matching the signature change from this PR. Addresses review feedback on #1011 Signed-off-by: fullsend-fix <fullsend-code@users.noreply.github.com>

github-actions · 2026-05-15T15:53:27Z

fullsend review is working on this — view logs

ralphbean

Two issues: vendored binary path and stale binary cleanup.

Update VendorBinary call in e2e/admin/admin_test.go to pass owner and repo as separate arguments, matching the signature change from this PR. Addresses review feedback on #1011 Signed-off-by: fullsend-fix <fullsend-code@users.noreply.github.com>

- Separate --vendor-fullsend-binary from enrollment flags in docs; it uploads a dev binary to the config repo, not enrollment-related - Add --skip-mint-check and --vendor-fullsend-binary to the flags table in installation.md - Clarify per-org-only flag descriptions in ADR 0033 Signed-off-by: Wayne Sun <gsun@redhat.com>

The flag was restricted to per-org mode only, but vendoring a development binary is useful in per-repo installs too. Parameterize VendorBinary, VendorBinaryLayer, and vendorFullsendBinary to accept an explicit repo target instead of hardcoding forge.ConfigRepoName. Signed-off-by: Wayne Sun <gsun@redhat.com>

Update VendorBinary call in e2e/admin/admin_test.go to pass owner and repo as separate arguments, matching the signature change from this PR. Addresses review feedback on #1011 Signed-off-by: fullsend-fix <fullsend-code@users.noreply.github.com>

Per-repo mode was uploading the vendored binary to bin/fullsend at the repo root, polluting the target repo with a top-level bin/ directory. Now per-repo uploads to .fullsend/bin/fullsend (matching per-repo config conventions), while per-org continues to use bin/fullsend in the .fullsend config repo. Also adds stale binary cleanup for per-repo mode — reinstalling without --vendor-fullsend-binary now removes any previously vendored binary, matching the cleanup behavior that VendorBinaryLayer provides for per-org mode. Updates action.yml to check both .fullsend/bin/fullsend (per-repo) and bin/fullsend (per-org) when detecting a vendored binary. Signed-off-by: Wayne Sun <gsun@redhat.com>

The layer's cleanup and analyze methods hardcoded VendoredBinaryPath (bin/fullsend) instead of selecting the path based on the target repo. Add a binaryPath() helper that returns the correct path for per-org vs per-repo mode, and add per-repo unit tests for cleanup, analyze, and VendorFunc delegation. Signed-off-by: Wayne Sun <gsun@redhat.com>

- Make --vendor-fullsend-binary flag and doc description mode-neutral (removes path that was only correct for per-repo) - Update VendorBinaryLayer type and Uninstall comments for dual-mode - Add cross-reference comment in per-repo cleanup noting the parallel with VendorBinaryLayer.Install - Standardize StepStart/StepFail/StepDone to lowercase in per-repo cleanup to match the layer's style - Fix e2e test log to use VendoredBinaryPath constant Signed-off-by: Wayne Sun <gsun@redhat.com>

Phase 5 diagram, mode differences table, and layer stack section described vendor binary as per-org only. Updated to show per-repo mode also supports optional binary vendoring via runPerRepoInstall(). Signed-off-by: Wayne Sun <gsun@redhat.com>

All findings addressed: vendor binary path fixed for per-repo mode, stale binary cleanup added inline, docs updated.

github-actions Bot deployed to site-preview May 15, 2026 15:22 View deployment

fullsend-ai-review Bot approved these changes May 15, 2026

View reviewed changes

waynesun09 changed the title ~~docs: fix --vendor-fullsend-binary description and add missing flags~~ feat: support --vendor-fullsend-binary in per-repo mode May 15, 2026

github-actions Bot deployed to site-preview May 15, 2026 15:36 View deployment

fullsend-ai-review Bot suggested changes May 15, 2026

View reviewed changes

waynesun09 force-pushed the fix-vendor-binary-flag branch from c9316ef to 2822392 Compare May 15, 2026 15:41

github-actions Bot deployed to site-preview May 15, 2026 15:43 View deployment

fullsend-ai-review Bot suggested changes May 15, 2026

View reviewed changes

fullsend-ai-review Bot approved these changes May 15, 2026

View reviewed changes

ralphbean previously requested changes May 15, 2026

View reviewed changes

Comment thread internal/cli/admin.go

Comment thread internal/cli/admin.go

rh-hemartin approved these changes May 18, 2026

View reviewed changes

waynesun09 force-pushed the fix-vendor-binary-flag branch from 1c19e1f to cc551ef Compare May 26, 2026 19:31

github-actions Bot deployed to site-preview May 26, 2026 19:33 View deployment

fullsend-ai-review Bot approved these changes May 26, 2026

View reviewed changes

fullsend-ai-review Bot added the ready-for-merge All reviewers approved — ready to merge label May 26, 2026

github-actions Bot deployed to site-preview May 26, 2026 19:45 View deployment

fullsend-ai-review Bot approved these changes May 26, 2026

View reviewed changes

fullsend-ai-review Bot added ready-for-merge All reviewers approved — ready to merge and removed ready-for-merge All reviewers approved — ready to merge labels May 26, 2026

github-actions Bot deployed to site-preview May 26, 2026 19:56 View deployment

fullsend-ai-review Bot approved these changes May 26, 2026

View reviewed changes

fullsend-ai-review Bot added ready-for-merge All reviewers approved — ready to merge and removed ready-for-merge All reviewers approved — ready to merge labels May 26, 2026

waynesun09 and others added 7 commits May 26, 2026 16:12

waynesun09 force-pushed the fix-vendor-binary-flag branch from 2631f21 to 788f6ae Compare May 26, 2026 20:13

github-actions Bot deployed to site-preview May 26, 2026 20:14 View deployment

fullsend-ai-review Bot approved these changes May 26, 2026

View reviewed changes

fullsend-ai-review Bot added ready-for-merge All reviewers approved — ready to merge and removed ready-for-merge All reviewers approved — ready to merge labels May 26, 2026

waynesun09 added this pull request to the merge queue May 26, 2026

Merged via the queue into main with commit ed22ab2 May 26, 2026
9 of 10 checks passed

waynesun09 deleted the fix-vendor-binary-flag branch May 26, 2026 21:13

This was referenced May 26, 2026

Review agent misses mode-specific behavioral implications in multi-mode features #1556

Open

Review-fix race condition causes duplicate review cycles and wasted tokens #1557

Open

Retro post-script 403 when commenting on pull requests #1558

Closed

qodo-code-review Bot mentioned this pull request Jul 6, 2026

feat(repos): add repos install command for bulk per-repo installation #3033

Merged

8 tasks

fullsend-ai-retro Bot mentioned this pull request Jul 6, 2026

Evidence for #3068: review agent fully covers human review on all dependency digest bumps in project-controller #3179

Closed

Uh oh!

Conversation

waynesun09 commented May 15, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Test plan

Uh oh!

github-actions Bot commented May 15, 2026

Uh oh!

github-actions Bot commented May 15, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Site preview

Uh oh!

fullsend-ai-review Bot commented May 15, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Review

Findings

Review

Findings

Low

Review

Findings

Low

Review

Findings

Review: #1011

Summary

Findings

Info

Footer

Review: #1011

Summary

Findings

High

Footer

Review: #1011

Summary

Findings

High

Info

Footer

Review: #1011

Summary

Findings

Footer

Uh oh!

github-actions Bot commented May 15, 2026

Uh oh!

fullsend-ai-review Bot left a comment

Choose a reason for hiding this comment

Uh oh!

github-actions Bot commented May 15, 2026

Uh oh!

github-actions Bot commented May 15, 2026

Uh oh!

fullsend-ai-review Bot left a comment

Choose a reason for hiding this comment

Uh oh!

github-actions Bot commented May 15, 2026

Uh oh!

fullsend-ai-coder Bot commented May 15, 2026

🔧 Fix agent — iteration 1 (bot-triggered)

Uh oh!

github-actions Bot commented May 15, 2026

Uh oh!

ralphbean left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

waynesun09 commented May 15, 2026 •

edited

Loading

github-actions Bot commented May 15, 2026 •

edited

Loading

fullsend-ai-review Bot commented May 15, 2026 •

edited

Loading