Skip to content

fix(scaffold): place vendored per-repo reusable workflows under .github/workflows/ - #2545

Merged
ifireball merged 2 commits into
mainfrom
agent/2544-fix-vendored-per-repo-workflow-paths
Jun 28, 2026
Merged

fix(scaffold): place vendored per-repo reusable workflows under .github/workflows/#2545
ifireball merged 2 commits into
mainfrom
agent/2544-fix-vendored-per-repo-workflow-paths

Conversation

@fullsend-ai-coder

Copy link
Copy Markdown
Contributor

GitHub Actions requires local reusable workflow references (paths starting with ./) to resolve under .github/workflows/. Vendored per-repo installs were placing reusable workflows under .fullsend/.github/workflows/ and generating uses: lines pointing there, causing GitHub to reject the workflows.

Fix all three path-generation sites in render.go (reusableWorkflowUses, reusableDispatchUses, and RenderDispatchPerRepoStagePaths) to always produce ./.github/workflows/ paths for vendored installs. Update CollectVendoredAssets to write reusable workflows directly under .github/workflows/ regardless of the per-repo prefix. Update enumerateVendoredPaths so path enumeration and cleanup also use the correct location.


Closes #2544

Post-script verification

  • Branch is not main/master (agent/2544-fix-vendored-per-repo-workflow-paths)
  • Secret scan passed (gitleaks — 0ca2613578a60c30ed7e1ba67d5ea1486fbe4723..HEAD)
  • Pre-commit hooks passed (authoritative run on runner)
  • Tests ran inside sandbox

…ub/workflows/

GitHub Actions requires local reusable workflow references (paths
starting with ./) to resolve under .github/workflows/. Vendored
per-repo installs were placing reusable workflows under
.fullsend/.github/workflows/ and generating uses: lines pointing
there, causing GitHub to reject the workflows.

Fix all three path-generation sites in render.go
(reusableWorkflowUses, reusableDispatchUses, and
RenderDispatchPerRepoStagePaths) to always produce
./.github/workflows/ paths for vendored installs. Update
CollectVendoredAssets to write reusable workflows directly under
.github/workflows/ regardless of the per-repo prefix. Update
enumerateVendoredPaths so path enumeration and cleanup also use the
correct location.

Closes #2544
@github-actions

github-actions Bot commented Jun 23, 2026

Copy link
Copy Markdown

E2E tests are running

Authorization passed for this commit. See the E2E Tests workflow for results.

@github-actions

github-actions Bot commented Jun 23, 2026

Copy link
Copy Markdown

Site preview

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

Commit: b9458226f18c76e16cb7d701c3e74166f51350d0

@fullsend-ai-review

fullsend-ai-review Bot commented Jun 23, 2026

Copy link
Copy Markdown

🤖 Finished Review · ✅ Success · Started 9:19 AM UTC · Completed 9:32 AM UTC
Commit: 69d0571 · View workflow run →

@codecov

codecov Bot commented Jun 23, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

@fullsend-ai-review

fullsend-ai-review Bot commented Jun 23, 2026

Copy link
Copy Markdown

Looks good to me


Labels: PR fixes a bug in vendored per-repo workflow path generation.

Previous run

Review

Findings

Medium

  • [unused parameter / dead code] internal/scaffold/vendormanifest.go:158 — After this change, enumerateVendoredPaths(workflowPrefix string) no longer uses the workflowPrefix parameter for reusable workflow paths — it only uses defaultsVendoredPrefix for defaults-infra paths. The parameter is now dead, meaning ManagedVendoredContentPaths(".fullsend/") and ManagedVendoredContentPaths("") return identical results.
    Remediation: Remove the workflowPrefix parameter from enumerateVendoredPaths (and update callers), or document that it is intentionally unused now.

  • [missing test coverage for new code path] internal/scaffold/vendormanifest.go:197 — The diff adds legacy cleanup paths in enumerateLegacyFlatVendoredPaths when workflowPrefix != "", emitting both .github/workflows/<name> and .fullsend/.github/workflows/<name> for cleanup. However, TestLegacyFlatVendoredPaths only tests with workflowPrefix="", so the new branch is untested.
    Remediation: Add a test case that calls LegacyFlatVendoredPaths(".fullsend/") and asserts the result contains both .github/workflows/reusable-triage.yml and .fullsend/.github/workflows/reusable-triage.yml.

  • [stale-identifier-reference] docs/ADRs/0033-per-repo-installation-mode.md:238 — ADR-0033 describes the pre-change layout where per-repo reusable workflows lived under .fullsend/.github/workflows/. After this PR, per-repo reusable workflows are under .github/workflows/ instead. While the mint's job_workflow_ref validation pattern 3 already covers the new path, the ADR's prose description is now outdated and could mislead readers.
    Remediation: Update ADR-0033 to note that per-repo vendored reusable workflows now live under .github/workflows/ rather than .fullsend/.github/workflows/.

Low

  • [unused parameter] internal/scaffold/vendorcontent.go:16CollectVendoredAssets(root, workflowPrefix string) now uses workflowPrefix only as a boolean flag (workflowPrefix == ".fullsend/") since the Path field no longer incorporates it. The parameter semantics have shifted from "path prefix" to "mode selector".

  • [test-assertion-consistency] internal/scaffold/vendorcontent_test.go:46 — Test assertion message uses "must" while the pre-existing test used "should". Minor style inconsistency.


Labels: PR fixes vendored per-repo workflow path generation in internal/scaffold/

@ifireball

Copy link
Copy Markdown
Member

/fs-fix tend to review finding and get coverage to 80% or above

@fullsend-ai-coder

fullsend-ai-coder Bot commented Jun 24, 2026

Copy link
Copy Markdown
Contributor Author

🤖 Finished Fix · ✅ Success · Started 10:06 AM UTC · Completed 10:13 AM UTC
Commit: 6bd5f3a · View workflow run →

@ifireball ifireball added the ok-to-test Allow e2e CI to run after maintainer review (must be re-applied after each push) label Jun 24, 2026
Remove unused workflowPrefix parameter from enumerateVendoredPaths —
after the reusable-workflow path fix, this function always produces
.github/workflows/ paths regardless of install mode.

Add TestLegacyFlatVendoredPaths_PerRepoPrefix to cover the new
legacy-cleanup branch in enumerateLegacyFlatVendoredPaths that emits
both canonical and legacy per-repo paths when workflowPrefix is set.

Addresses review feedback on #2545
@fullsend-ai-coder

Copy link
Copy Markdown
Contributor Author

🔧 Fix agent — iteration 1 (human-triggered)

Addressed 2 of 5 review findings: removed dead workflowPrefix parameter from enumerateVendoredPaths (medium) and added test for legacy cleanup paths with per-repo prefix to cover the untested branch (medium, fixes patch coverage gap). Disagreed with 3 findings: ADR-0033 update is out of scope per ADR immutability policy, CollectVendoredAssets parameter rename is cosmetic, and assertion style is a minor nit.

Fixed (2):

  1. unused parameter / dead code in enumerateVendoredPaths (internal/scaffold/vendormanifest.go): Removed the unused workflowPrefix parameter from enumerateVendoredPaths and updated all callers (ResolveVendoredCleanupPaths, ManagedVendoredContentPaths, and two test call sites).
  2. missing test coverage for legacy cleanup paths with per-repo prefix (internal/scaffold/vendormanifest_test.go): Added TestLegacyFlatVendoredPaths_PerRepoPrefix that calls LegacyFlatVendoredPaths(".fullsend/") and asserts the result contains both .github/workflows/reusable-triage.yml and .fullsend/.github/workflows/reusable-triage.yml, covering the previously untested workflowPrefix != "" branch.

Disagreed (3):

  1. stale-identifier-reference in ADR-0033: ADR-0033 is an accepted Architecture Decision Record on main. Per AGENTS.md, accepted ADRs are immutable point-in-time records — updating the prose is out of scope for this bug-fix PR. A minor annotation or cross-reference could be added in a separate PR.
  2. unused parameter semantics in CollectVendoredAssets: Low-severity finding. The workflowPrefix parameter still serves as a mode selector for RenderDispatchPerRepoStagePaths; renaming or refactoring the public API is out of scope for this PR.
  3. test-assertion-consistency (must vs should): Low-severity style nit. The 'must' wording accurately reflects the GitHub Actions requirement. Not worth a diff line.

Tests: passed

Updated by fullsend fix agent

@github-actions github-actions Bot removed the ok-to-test Allow e2e CI to run after maintainer review (must be re-applied after each push) label Jun 24, 2026
@fullsend-ai-review

fullsend-ai-review Bot commented Jun 24, 2026

Copy link
Copy Markdown

🤖 Finished Review · ✅ Success · Started 10:16 AM UTC · Completed 10:30 AM UTC
Commit: b945822 · View workflow run →

@fullsend-ai-review fullsend-ai-review Bot added ready-for-merge All reviewers approved — ready to merge type/bug Confirmed defect in existing behavior and removed requires-manual-review Review requires human judgment labels Jun 24, 2026
@ifireball ifireball added the ok-to-test Allow e2e CI to run after maintainer review (must be re-applied after each push) label Jun 24, 2026
@ifireball
ifireball added this pull request to the merge queue Jun 28, 2026
Merged via the queue into main with commit 120ff3d Jun 28, 2026
28 checks passed
@ifireball
ifireball deleted the agent/2544-fix-vendored-per-repo-workflow-paths branch June 28, 2026 06:19
@fullsend-ai-retro

fullsend-ai-retro Bot commented Jun 28, 2026

Copy link
Copy Markdown

🤖 Finished Retro · ✅ Success · Started 6:24 AM UTC · Completed 6:40 AM UTC
Commit: b945822 · View workflow run →

@fullsend-ai-retro

Copy link
Copy Markdown

Retro: PR #2545 — fix(scaffold): place vendored per-repo reusable workflows under .github/workflows/

Overall assessment: This workflow went well. The full triage → code → review → fix → re-review pipeline completed successfully with only one fix iteration. The code agent produced a clean, well-tested bug fix in ~15 minutes, and the fix agent made sound judgment calls when disagreeing with 3 of 5 review findings.

Timeline

Stage Time Duration Notes
Triage Jun 23 08:49 ~11 min Labeled ready-to-code
Code Jun 23 09:00 ~15 min Created PR #2545
Review Jun 23 09:15 ~17 min 3 medium + 2 low findings
Human /fs-fix Jun 24 10:03 24.5h gap
Fix Jun 24 10:03 ~10 min Addressed 2/5, disagreed with 3
Re-review Jun 24 10:13 ~17 min Approved
Human approval Jun 24 13:53
Merged Jun 28 06:20 3.5 day gap to merge

What went well

  • Fast issue-to-PR pipeline (~26 min from issue to PR)
  • Fix agent showed good judgment: correctly declined to modify an accepted ADR (per AGENTS.md immutability rules), correctly scoped out a cosmetic parameter rename, and reasonably rejected a style nit
  • Review agent accepted valid disagreements on re-review rather than insisting
  • Test coverage was solid: Codecov confirmed all modified lines were covered

Improvement areas (mostly already tracked)

  • Auto-fix dispatch for bot PRs: The 24.5h delay waiting for a human /fs-fix is the largest time sink. Already tracked in #2596.
  • Review agent flagging ADR modifications: The review suggested updating ADR-0033, which contradicts AGENTS.md's immutability rule. Already partially covered by #1068 (governance-only findings) and #2678 (deprioritizing doc findings).
  • Code agent leaving dead parameters: The workflowPrefix parameter became unused after the code agent's changes but wasn't removed until the fix iteration. This is a minor rework driver but hard to systematically address.

No new proposals filed — all identified improvements are covered by existing open issues.

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

Labels

component/install CLI install and app setup ok-to-test Allow e2e CI to run after maintainer review (must be re-applied after each push) ready-for-merge All reviewers approved — ready to merge type/bug Confirmed defect in existing behavior

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Vendored per-repo shim uses invalid local path for reusable workflows

1 participant