feat(workflows): add fullsend_ai_ref for self-consistent version pinning#1278
Conversation
Site previewPreview: https://8f687e32-site.fullsend-ai.workers.dev Commit: |
ReviewFindingsMedium
Info
Previous runReviewFindingsMedium
Low
Info
Previous run (2)ReviewFindingsMedium
Low
Previous run (3)ReviewFindingsMedium
Low
Previous run (4)ReviewFindingsMedium
Low
|
fullsend-ai-review
Bot
added
the
requires-manual-review
fullsend-ai-review
Bot
added
requires-manual-review
|
/fs-review |
fullsend-ai-review
Bot
added
requires-manual-review
fullsend-ai-review
Bot
added
requires-manual-review
ralphbean
left a comment
ralphbean
left a comment
There was a problem hiding this comment.
A few things to sort out:
prioritize.yml / repo-maintenance.yml — these scaffold workflows (internal/scaffold/fullsend-repo/.github/workflows/) still use the old pattern: ref: v0, sparse-checkout, rm -rf .defaults, fullsend-ai/fullsend/.github/actions/*@v0. Worth bringing them into the new pattern for consistency.
docs/architecture.md:611 — the layer mapping table still says the agent runner uses fullsend-ai/fullsend@v0. Now that it's loaded from ./.defaults/, this line is stale.
waynesun09
left a comment
waynesun09
left a comment
There was a problem hiding this comment.
Review Squad Report — 7-agent review (2x claude-coder, 2x claude-researcher, 2x gemini, 1x cursor)
What this PR does
This PR replaces hardcoded @v0 remote action references with local action resolution via ./.defaults/. The mechanism:
- Caller pins to a tag (e.g.,
@v0.10.1) viauses: - Passes the same ref as
fullsend_ai_refinput - Stage workflows check out
fullsend-ai/fullsendat that ref into.defaults/ - All composite actions and the agent binary load from
./.defaults/(local filesystem) reusable-dispatch.ymluses relative./paths for stage workflow calls, which GitHub resolves at the same ref
Is this for local action resolution? Yes — the core change switches from remote action refs (fullsend-ai/fullsend/.github/actions/*@v0) to local action refs (uses: ./.defaults/.github/actions/*). This ensures all action code comes from the same checked-out ref rather than independently-resolved remote refs.
Will this break existing callers using v0?
Yes. fullsend_ai_ref is required: true with no default:. The moment v0 advances to include this change, every existing caller that hasn't been updated to pass fullsend_ai_ref will fail with "required input not provided." This affects all deployed per-org .fullsend stage workflows and per-repo shims.
Fix: Change to required: false with default: "v0" — one-line change per workflow, fully backward-compatible.
Findings summary
| Severity | Count | Key findings |
|---|---|---|
| CRITICAL | 1 | required: true breaks all existing installations (6/7 agents) |
| HIGH | 3 | .defaults/ retention exposes full repo (6/7); ref coupling footgun (6/7); prioritize.yml not updated (3/7) |
| MEDIUM | 4 | Full clone perf (7/7); no input validation (4/7); stale doc refs (2/7) |
| LOW | 3 | Testing guide clarity; quote style churn; misleading input descriptions |
Notable findings NOT in inline comments
- HIGH —
prioritize.yml,prioritize-scheduler.yml, andrepo-maintenance.ymlstill use the old pattern (hardcoded@v0, sparse-checkout,rm -rf .defaults, remote action refs). Creates inconsistency where 5 stages are updated but prioritize is not. Either update in this PR or track as a follow-up issue.
False positives removed (3)
Relative— GitHub resolves relative./workflow paths break cross-repo callsworkflow_callpaths within the called workflow's repository at the same ref. The design is correct.Sparse-checkout already includes composite action files— Actions are at.github/actions/in the repo root, not underinternal/scaffold/fullsend-repo/.Shell injection via—fullsend_ai_refworkflow_callinputs aren't user-controllable;actions/checkoutsanitizes the ref parameter. Downgraded to MEDIUM input validation concern.
| done | ||
| mkdir -p .github/scripts | ||
| cp "${SRC}/.github/scripts/setup-agent-env.sh" .github/scripts/setup-agent-env.sh | ||
| rm -rf .defaults |
There was a problem hiding this comment.
HIGH — .defaults/ directory retained during agent execution, exposing full repo source
The rm -rf .defaults cleanup was removed (necessary for local action refs), but with sparse-checkout also removed, the entire fullsend-ai/fullsend repo — Go source, internal tooling, CI config, e2e tests, infrastructure code — now persists in the workspace during agent execution. Previously only internal/scaffold/fullsend-repo/ was present.
Suggestion: Add a cleanup step with if: always() after the agent run step:
- name: Cleanup upstream checkout
if: always()
run: rm -rf .defaultsGitHub Actions resolves uses: action paths at step execution time (not eagerly for the whole job), but the composite action at .defaults/ needs to exist when its step runs. The cleanup should go immediately after the "Run code agent" step. If that's not safe, at minimum prune non-essential directories before agent execution:
find .defaults -mindepth 1 -maxdepth 1 \
! -name 'action.yml' ! -name '.github' \
-exec rm -rf {} +Same pattern applies to all 5 stage workflows. Flagged by 6/7 agents.
There was a problem hiding this comment.
Why should we delete the repository? It is publicly available.
| install_mode: per-repo | ||
| mint_url: ${{ vars.FULLSEND_MINT_URL }} | ||
| gcp_region: ${{ vars.FULLSEND_GCP_REGION }} | ||
| fullsend_ai_ref: v0 |
There was a problem hiding this comment.
HIGH — fullsend_ai_ref / uses: ref coupling is unenforced
These two values must stay in sync but are independent strings with no enforcement. A caller could update uses: ...@v0.10.1 but forget to update fullsend_ai_ref, causing split-brain: dispatch workflow code from one version, composite actions/binary from another.
Suggestion:
- Add an explicit comment here:
# Must match the @ref in the uses: line above - Consider a runtime validation step in the stage workflows comparing
inputs.fullsend_ai_refagainst the workflow ref (extractable fromGITHUB_WORKFLOW_REF) - The follow-up installer PR should generate both values from a single source to prevent drift
Flagged by 6/7 agents.
There was a problem hiding this comment.
- Ok
- As far as I could test
GITHUB_WORKFLOW_REFis the original workflow ref, not the reusable one. - It will when we change to static versions. Or we can add it and that it uses
v0, but I would do it on another PR.
These changes act at reusable level, since prioritize does not have a reusable workflow we can't use the pattern there. And that is a bigger change than I intend. |
fullsend-ai-review
Bot
added
requires-manual-review
…ng (ADR 41) Replace thin stage workflows with synchronous reusable workflow_call jobs in dispatch.yml, add fullsend_ai_ref pinning (fullsend-ai#1278), set FULLSEND_AI_REF on org install, and pin e2e CI to the PR branch ref. Co-authored-by: Cursor <cursoragent@cursor.com>
@rh-hemartin will the changes proposed in #1611 (basically moving org mode to reusable) resolve the issue for prioritize? |
i don't think that would solve the issue with prioritize. However the issue with prioritize is just that I'm not changing it. We are free to change it whenever so it conforms to these changes. Then caller of prioritize.yaml (prioritize-schedule.yaml) would pass the |
rh-hemartin
force-pushed
the
feat/version-pinning-workflows
branch
from
a9b7b92 to
d0a575c
Compare
…n pinning Add required `fullsend_ai_ref` input to reusable-dispatch and all stage workflows (triage, code, review, fix, retro). Each stage workflow now checks out the fullsend repo at `inputs.fullsend_ai_ref` instead of the hardcoded `v0` ref, then references actions and the agent itself via relative `./.defaults/` paths rather than `fullsend-ai/fullsend/...@v0`. Dispatch similarly switches stage workflow `uses:` to relative `./.github/workflows/reusable-*.yml` paths, so a caller pinned to e.g. `@v0.10.1` resolves all nested workflows at that same tag automatically. Renamed the dispatch workflow to "Dispatch (local)" to reflect this. Drop sparse-checkout (full clone required for relative action refs) and keep `.defaults/` in place after workspace prep so relative paths resolve. Scaffold callers and the per-repo shim template pass `fullsend_ai_ref: v0` as a stable default for enrolled repos. Signed-off-by: Hector Martinez <hemartin@redhat.com>
rh-hemartin
force-pushed
the
feat/version-pinning-workflows
branch
from
d0a575c to
716f73c
Compare
4 participants
Summary
Closes #1075 (partially — the installer-side SHA pinning is a follow-up PR).
fullsend_ai_refinput toreusable-dispatchand all five stage workflows (triage, code, review, fix, retro)fullsend-ai/fullsendatinputs.fullsend_ai_refinstead of the hardcodedv0ref, then loads all actions and the agent binary from the local.defaults/pathreusable-dispatchswitches its stageuses:to relative./.github/workflows/reusable-*.ymlpaths — a caller pinned to e.g.@v0.10.1automatically resolves all nested workflows at that same tagfullsend_ai_ref: v0as a stable default for enrolled reposdocs/guides/dev/testing-workflows.mdexplaining how to point an installed repo at a dev branch for end-to-end testingWhat's not covered (follow-up)
The installer (
fullsend admin install) still scaffoldsfullsend_ai_ref: v0as a literal string. A follow-up PR will make the installer substitute the actual release SHA at install time, completing the SHA-pinning story from #1075.Test plan
uses:andfullsend_ai_refto that branch, trigger/fs-triage— confirm the stage workflow loads actions and the agent binary from the branchfullsend_ai_refthrough correctly./stage workflow paths resolve when dispatch is called at a pinned tag🤖 Generated with Claude Code