feat: reusable workflows (ADR 31), centralized routing (ADR 34), layered content (ADR 35)

[waynesun09]

Implementation of ADR 31 (reusable workflows), ADR 34 (centralized routing), and ADR 35 (layered content resolution) for the fullsend agent pipeline.

Summary

ADR 31 — Reusable workflows for action-installed distribution

• Publish 5 reusable workflows (workflow_call) and a root composite action from fullsend-ai/fullsend, enabling org .fullsend repos to delegate agent pipeline logic upstream instead of maintaining full scaffold copies
• Convert scaffold agent workflows from 80-289 line full implementations to ~30-55 line thin callers that use workflow_call + explicit secrets: passthrough
• Extract 3 shared composite actions (mint-token, validate-enrollment, setup-gcp) to .github/actions/ for use by reusable workflows
• Consolidate duplicate fullsend action to single root action.yml (was duplicated at .github/actions/fullsend/ and scaffold copy)
• Remove CLI version-pinning from WorkflowsLayer since thin callers no longer contain CLI download steps — version is now controlled by fullsend_version input to reusable workflows

ADR 34 — Centralized event routing via dispatch.yml

• Move event-to-stage routing from the per-target-repo shim (8 jobs, ~210 lines) into dispatch.yml in the .fullsend repo
• Shim shrinks to 2 jobs (~80 lines): a universal dispatch forwarder via workflow_call and a stop-fix handler
• New "Determine stage" step in dispatch.yml routes events to stages using shell case logic with env: blocks (no expression injection)
• Adding a new stage requires only a case branch in dispatch.yml — zero changes to enrolled repos
• No backwards compatibility needed: shim and dispatch.yml ship together atomically via fullsend admin install

ADR 35 — Layered content resolution

• Upstream defaults (agents, skills, schemas, harness, policies, scripts, env) are no longer copied into .fullsend repos by the scaffold
• Reusable workflows sparse-checkout upstream defaults from fullsend-ai/fullsend@v0 at runtime and layer org overrides from customized/ on top
• Composite actions are referenced directly from upstream (fullsend-ai/fullsend/.github/actions/*@v0), not installed into .fullsend
• Scaffold output shrinks from ~68 files to ~23 org-specific files (customized/ gitkeeps, thin callers, config, env files)

New files (repo root, published upstream)

File	Description
action.yml	Published composite action (install CLI, run agent, upload artifacts) — single source, replaces duplicate at .github/actions/fullsend/
.github/actions/mint-token/action.yml	OIDC token mint via fullsend mint service
.github/actions/validate-enrollment/action.yml	Config.yaml enrollment validation
.github/actions/setup-gcp/action.yml	GCP WIF auth + credential masking
.github/workflows/reusable-{triage,code,review,fix,retro}.yml	Reusable agent pipelines
docs/ADRs/0035-layered-content-resolution.md	ADR 35 decision record

Modified files

File	Change
internal/scaffold/.../workflows/{triage,code,review,fix,retro}.yml	Reduced to thin callers (~30-55 lines)
internal/scaffold/.../workflows/dispatch.yml	Added centralized routing logic with event_action input
internal/scaffold/.../workflows/{prioritize,prioritize-scheduler,repo-maintenance}.yml	Updated to use upstream action refs (fullsend-ai/fullsend/.github/actions/*@v0) and workspace layering for scripts
internal/scaffold/.../templates/shim-workflow-call.yaml	Simplified from 8 jobs to 2 jobs (~80 lines)
internal/scaffold/.../customized/{agents,skills,schemas,harness,policies,scripts,env}/.gitkeep	Override directories for org customization
internal/scaffold/scaffold.go, scaffold_test.go	Updated tests for thin callers, routing logic, shim structure, slim scaffold output
internal/layers/workflows.go	Removed version-pinning logic
internal/cli/admin.go	Updated NewWorkflowsLayer call sites
internal/mint/main.go, main_test.go	Accept fullsend-ai/fullsend/ prefix in job_workflow_ref for cross-org workflow_call

Deleted files

File	Reason
.github/actions/fullsend/action.yml	Consolidated to root action.yml
internal/scaffold/fullsend-repo/.github/actions/fullsend/action.yml	Scaffold copy of above, also consolidated

Architecture

target-repo shim (2 jobs, ~80 lines)
  └─ workflow_call ──> .fullsend/dispatch.yml
                          │  "Determine stage" routing
                          │  OIDC mint + config checks
                          │
          ┌───────────────┼───────────────────────┐
          ▼               ▼                       ▼
  .fullsend/code.yml   review.yml          triage.yml
  (~30-55 lines, thin callers via workflow_call)
          │
  uses: fullsend-ai/fullsend/.github/workflows/reusable-code.yml@v0
          │                    ┌─ sparse-checkout upstream defaults (ADR 35)
          │                    ├─ layer customized/ overrides on top
          │                    └─ clean up .defaults directory
          │
  uses: fullsend-ai/fullsend@v0  ← published root composite action

ADR 31 properties:

• vars.* mapped to explicit workflow_call inputs (vars don't cross the boundary)
• Secrets pass via explicit secrets: blocks (not secrets: inherit — doesn't work cross-org)
• 1 level of workflow_call nesting (limit is 4)
• OIDC job_workflow_ref reports the reusable workflow's repo — mint accepts both {org}/.fullsend/ and fullsend-ai/fullsend/ prefixes

ADR 34 routing:

• Shim forwards event_action (not stage) to dispatch.yml
• Routing uses github.event.* context via env: blocks — no expression injection
• Kill switch and role gating in dispatch.yml
• Fork PR detection for fix stage
• Author association checks for /fix and /retro commands
• Stage-to-role mapping: retro and prioritize map to fullsend App role (no dedicated PEM)

ADR 35 layering:

• Reusable workflows sparse-checkout fullsend-ai/fullsend@v0 at runtime for upstream defaults
• Org overrides in customized/ are layered on top
• Non-agent workflows (repo-maintenance, prioritize) use workspace layering for scripts
• Composite actions referenced directly from upstream, not installed into .fullsend

Test plan

• go test ./internal/... — all 19 packages pass
• go vet ./internal/... — clean
• Review squads (security, correctness, test coverage, YAML consistency, parity) — multiple rounds, clean
• Manual: fullsend admin install against nonflux org — 5 apps reused, 25 scaffold files written (down from 77), mint health check passed, enrollment completed (2 repos)
• Scaffold verification: thin callers are 41-66 lines with 1 reusable ref each; dispatch.yml has routing step; customized/ override directories all present with .gitkeep
• Mint redeployment: --force-mint-deploy required when upgrading from PR feat: OIDC token mint dispatch — deprecate PAT, migrate to OIDC #503 — the mint's job_workflow_ref validation must accept fullsend-ai/fullsend/ prefix for cross-org workflow_call. Without redeployment, reusable workflows get 403 from the old mint.
• End-to-end dispatch chain (triage on nonflux/integration-service): shim → dispatch.yml routing → triage.yml thin caller → reusable-triage.yml@v0 → sparse-checkout upstream defaults → mint-token action → GCP auth → agent setup — all green. Agent run fails due to openshell gateway incompatibility (Investigate standalone openshell-gateway with Docker/Podman drivers across target environments #780), unrelated to this PR.
• Rebase on main after direct-WIF merge (PR feat: switch GCP auth to direct WIF #856) — resolved conflicts, aligned reusable workflows and setup-gcp with WIF-only auth (removed gcp_auth_mode, FULLSEND_GCP_WIF_SA_EMAIL, FULLSEND_GCP_SA_KEY_JSON)
• Dogfood: replace fullsend-ai/.fullsend workflows with thin callers, trigger dispatch chain
• Test remaining stages end-to-end: code, review, fix, retro, stop-fix
• Verify routing negative cases: bot filtering, /code on PR, unauthorized /fix, fork PR, kill switch

Issues found and fixed during testing

1. /health endpoint (GCF routing) — CLI polled /healthz which GCF Functions Framework v1.9.0 doesn't route to the handler. Fixed: changed to /health in gcf.go.
2. Missing permissions blocks in thin callers — org default workflow permissions are read; workflow_call caps the called workflow's permissions to the caller's top-level permissions: block. Without explicit permissions, reusable workflows got none for write scopes. Fixed: added permissions: blocks to all thin caller templates.
3. secrets: inherit doesn't work cross-org — nonflux/.fullsend calling fullsend-ai/fullsend is cross-org; secrets: inherit only works within the same org/enterprise. Fixed: reusable workflows declare explicit secrets: inputs; thin callers pass each secret individually.
4. uses: ./ resolves to caller's checkout — in workflow_call, uses: ./ resolves to the workspace root (caller's .fullsend checkout), not the reusable workflow's repo. Fixed with upstream action refs (fullsend-ai/fullsend/.github/actions/*@v0).
5. Cross-org job_workflow_ref OIDC validation — mint validated job_workflow_ref starts with {org}/.fullsend/, but cross-repo workflow_call reports fullsend-ai/fullsend/.github/workflows/reusable-*.yml@.... Fixed: updated mint to also accept fullsend-ai/fullsend/ prefix.
6. App manifest permissions drift — nonflux-coder/nonflux-review Apps created with issues: read, but mint requests issues: write. Fixed: upgraded manifests in types.go (33920c8). Existing Apps need manual permission update.
7. fullsend composite action not at repo root — reusable workflows reference fullsend-ai/fullsend@v0, consolidated the action to root action.yml (removed duplicates at .github/actions/fullsend/ and scaffold copy).
8. Non-agent workflows used local action refs — repo-maintenance.yml, prioritize.yml, prioritize-scheduler.yml used ./.github/actions/mint-token (local ref), but ADR 35 no longer installs composite actions into .fullsend. Fixed: updated to upstream action refs and added workspace layering for scripts.
9. retro/prioritize stages blocked by dispatch role check — dispatch.yml maps stage→role but only handled code → coder. retro and prioritize share the fullsend App. Fixed: added retro|prioritize → fullsend mapping and updated reusable-retro.yml to mint with role: fullsend.
10. Mint 403 after --skip-mint-deploy — upgrading from PR feat: OIDC token mint dispatch — deprecate PAT, migrate to OIDC #503 to PR feat: reusable workflows (ADR 31), centralized routing (ADR 34), layered content (ADR 35) #792 with --skip-mint-deploy leaves the old mint code deployed, which rejects fullsend-ai/fullsend/ in job_workflow_ref. Must use --force-mint-deploy on first install after this PR.
11. Stale direct-WIF references after rebase — after rebasing on main (which merged PR feat: switch GCP auth to direct WIF #856 direct-WIF), reusable workflows and setup-gcp still had gcp_auth_mode, FULLSEND_GCP_WIF_SA_EMAIL, FULLSEND_GCP_SA_KEY_JSON. Fixed: aligned all reusable workflows and setup-gcp with WIF-only auth.

[github-actions]

fullsend review is working on this — view logs

[github-actions]

Site preview

Preview: https://20965d3a-site.fullsend-ai.workers.dev

Commit: 1cabedaf066e82115bb304b119e5ebdf0ae12195

[github-actions]

fullsend review is working on this — view logs

[github-actions]

fullsend review is working on this — view logs

[fullsend-ai-review]

Review: #792

Head SHA: 1cabeda
Timestamp: 2026-05-13T00:00:00Z
Outcome: comment-only

Summary

This PR implements a major architectural change spanning ADRs 31, 34, and 35 — converting scaffold workflows from full implementations to thin callers that delegate to reusable workflows, centralizing event routing in dispatch.yml, and resolving upstream defaults via sparse checkout at runtime. The security posture is strong: all ${{ }} expressions touching user-controlled data in run: blocks are properly routed through env: bindings (no expression injection found), secrets are masked with ::add-mask::, OIDC tokens are validated server-side with fail-closed semantics, and fork PRs are blocked with defense-in-depth at both dispatch and reusable workflow layers. The Go code changes are sound — the mint's job_workflow_ref prefix check is safe because it derives the expected prefix from the already-validated claims.RepositoryOwner. The normative SPEC deletions are appropriate given the architectural shift. No critical or high findings; six medium findings are worth discussion but do not block merge.

Findings

Critical

None.

High

None.

Medium

1. [injection-defense] shim-workflow-call.yaml (dispatch-review condition) — The review dispatch triggers on all pull_request_target events except closed but has no fork PR guard, unlike the fix dispatch which explicitly blocks fork PRs. While review agents have contents: read (lower risk than contents: write), a fork PR could still trigger review agent compute and token minting. Consider adding && github.event.pull_request.head.repo.full_name == github.event.pull_request.base.repo.full_name to match the pattern in fix.
   Remediation: Add fork check to review dispatch condition or add a fork guard step in reusable-review.yml.

2. [correctness] shim-workflow-call.yaml:36-38 — The shim collapsed eight per-stage concurrency groups into a single fullsend-dispatch-$N group with cancel-in-progress: false. If a triage run is in progress for issue docs: add agent infrastructure problem document #5 and a /code command arrives for the same issue, code dispatch will queue behind triage rather than running in parallel. Previously these were independent per-stage groups.
   Remediation: Clarify if this is intentional. If not, consider preserving per-stage groups at the shim level.

3. [injection-defense] dispatch.yml (routing step) — The has_label() helper splits label names on comma via join(github.event.issue.labels.*.name, ','). A label containing a comma in its name (e.g., needs-info,type/feature) would cause incorrect CSV parsing, potentially bypassing or falsely triggering routing logic. Requires label-creation permissions, so attack surface is limited.
   Remediation: Use a delimiter unlikely to appear in label names (newline) or check label membership via jq from $GITHUB_EVENT_PATH.

4. [platform-security] All reusable workflows (sparse checkout step) — Upstream defaults are fetched via ref: v0, a mutable tag. If fullsend-ai/fullsend's v0 tag is moved (authorized release or compromised credential), all downstream agent runs immediately pick up new content including agent definitions, skills, policies, and scripts that execute with org-level tokens. Mutable major-version tags are standard GHA convention, but the risk is elevated given the scope of content fetched.
   Remediation: Document the trust assumption explicitly. Consider SHA-pinned references or signed tag verification for the sparse-checkout step.

5. [content-security] retro.yml / reusable-retro.yml — RETRO_COMMENT is set from fromJSON(inputs.event_payload).comment.body (user-controlled, truncated to 4096 chars by dispatch.yml). While properly in an env: binding (not in run: interpolation), the raw comment text is passed to the fullsend action. Verify the agent/action never interpolates $RETRO_COMMENT into shell commands.
   Remediation: Consider applying the same structured extraction/validation pattern used in reusable-fix.yml.

6. [correctness] Reusable workflow concurrency groups — fromJSON(inputs.event_payload).issue.number is used in concurrency keys. If event_payload has malformed JSON or the field is missing, fromJSON() returns empty, causing unrelated runs to collide on the same concurrency group. Dispatch.yml validates the payload before dispatching, so this is defense-in-depth.
   Remediation: Add a fallback value (e.g., || github.run_id) in concurrency group expressions.

Low

1. [correctness] internal/mint/main_test.go — No test for cross-org job_workflow_ref confusion (e.g., org-a token with org-b/.fullsend/... ref). The code handles this correctly since the prefix derives from claims.RepositoryOwner, but an explicit test would protect the invariant.
   Remediation: Add a test where repository_owner is org-a but job_workflow_ref references org-b/.fullsend/....

2. [correctness] reusable-retro.yml:97 — Mint requests role: retro but dispatch.yml maps retro stage to fullsend role. If the mint validates role against App names, verify retro is a recognized role, or align to role: fullsend.

3. [style] code.yml:28, fix.yml:47 — Both request packages: read permission without visible usage. Add a comment explaining why it's needed or remove it.

4. [correctness] dispatch.yml → reusable-fix.yml — Comment body truncated to 4096 chars in the event payload. If a /fix instruction exceeds this, it will be silently truncated. Document the limit.

Info

1. [injection-defense] All reusable workflows and dispatch.yml demonstrate exemplary expression injection hygiene — every user-controlled ${{ }} value is bound to an env: variable, never interpolated in run: blocks. The random heredoc delimiter in reusable-fix.yml for GITHUB_OUTPUT prevents delimiter injection. The dispatch.yml reads from $GITHUB_EVENT_PATH instead of toJSON(github.event) in shell context.

2. [platform-security] Fork PR blocking is implemented at two layers (dispatch routing + reusable workflow), both fail-closed when fork status cannot be determined.

3. [platform-security] Mint's job_workflow_ref validation is correctly hardened: prefix derived from validated claims.RepositoryOwner, .github/workflows/ path required, optional allowlist is fail-closed (empty list denies all).

4. [correctness] The WorkflowsLayer simplification cleanly removes version-pinning logic in favor of the fullsend_version input to reusable workflows. No important functionality was lost.

Footer

Outcome: comment-only
This review applies to SHA 1cabedaf066e82115bb304b119e5ebdf0ae12195. Any push to the PR head clears this review and requires a new evaluation.

Previous run

Review: #792

Head SHA: af0bf5b
Timestamp: 2026-05-13T00:00:00Z
Outcome: comment-only

Summary

This PR implements three ADRs (31, 34, 35) converting the fullsend agent pipeline from full scaffold copies to thin callers delegating to upstream reusable workflows, centralizing event routing in dispatch.yml, and introducing layered content resolution. The security posture has improved: fork PR detection now fails closed (was fail-open), enrollment validation is inlined to prevent override via customized scripts, and routing logic is centralized in the .fullsend repo context rather than the target repo shim. The code is well-structured, tests are updated comprehensively, and the mint validation correctly extends to accept upstream workflow refs while maintaining the .github/workflows/ path constraint. A few minor observations are noted below.

Findings

Medium

• [Style/conventions] reusable-{code,fix,review,retro,triage}.yml — The "Prepare workspace (upstream defaults + org overrides)" step (~20 lines of bash) is copy-pasted verbatim across all 5 reusable workflows. Extracting this into a composite action (e.g., .github/actions/prepare-workspace/action.yml) would reduce duplication and make the layering logic easier to maintain. This is a maintainability concern, not a correctness issue.
  Remediation: Extract the workspace preparation logic into a shared composite action.

Low

• [Correctness] internal/scaffold/scaffold_test.go:TestCustomizedDirsInstalled — The test asserts that 6 of the 7 customized/ .gitkeep files are installed but omits customized/env/.gitkeep. The file exists in the scaffold (internal/scaffold/fullsend-repo/customized/env/.gitkeep) and will be installed correctly (it's not in a skipped directory), but the test doesn't verify it.
  Remediation: Add "customized/env/.gitkeep": false to the expected map.

• [Style/conventions] internal/scaffold/fullsend-repo/.github/workflows/dispatch.yml — The command extraction echo "${COMMENT_BODY}" | head -1 | awk '{print $1}' could behave unexpectedly with edge-case comment bodies (e.g., bodies starting with -e or -n). Using printf '%s\n' "${COMMENT_BODY}" would be more robust.
  Remediation: Replace echo "${COMMENT_BODY}" with printf '%s\n' "${COMMENT_BODY}".

Info

• [Platform security] internal/mint/main.go — The hardcoded fullsend-ai/fullsend/ upstream prefix in prevalidateOIDCToken is a deliberate architectural decision for cross-org workflow_call OIDC validation. The existing .github/workflows/ path constraint and ALLOWED_WORKFLOW_FILES check still apply, so this does not meaningfully widen the attack surface. Well-tested with positive and negative cases.

• [Platform security] Fork PR detection — The fix stage's fork PR detection changed from fail-open (|| echo "false") to fail-closed (|| true with empty-string blocking). This is a security improvement over the prior behavior.

• [Correctness] Normative spec deletion — Four docs/normative/ SPEC files are deleted, with ADRs updated to point to the Go implementation as the source of truth. This is consistent with the project's design philosophy ("this is a design exploration, not a spec") and reduces spec-vs-implementation drift risk.

Footer

Outcome: comment-only
This review applies to SHA af0bf5bf0082923d71d5c97e799698e6fc820408. Any push to the PR head clears this review and requires a new evaluation.

Previous run (2)

Review: #792

Head SHA: b1dbc1d
Timestamp: 2026-05-13T00:00:00Z
Outcome: approve

Summary

This PR implements three coordinated ADRs (31, 34, 35) to restructure the fullsend agent pipeline from full-scaffold deployment to a reusable-workflow architecture with runtime layering. The security model is sound: dispatch routing correctly uses env: blocks to prevent expression injection, fork PR detection fails closed, OIDC token validation properly extends to upstream workflow refs while maintaining .github/workflows/ path restrictions, and enrollment validation is inlined to prevent override via customized/scripts/. The architectural changes are well-motivated, test coverage is comprehensive, and the changes are internally consistent across all 47 files.

Findings

Medium

• [correctness] docs/normative/admin-install/v1/ — Five normative v1 specification files (ADR 0011–0014 SPECs and the config schema) are deleted without replacement or explicit mention in the PR description. These specs documented the install contract for config.yaml shape, repo file layout, enrollment flow, and credential surface. Their deletion leaves no normative reference for the v1-to-v2 transition. Consider either retaining them with a deprecation notice or documenting their supersession in a new spec.

Low

• [correctness] shim-workflow-call.yaml — The post-run-link job was removed, eliminating the "fullsend X is working on this" comment that notified users when agents started. No replacement notification mechanism is visible in this PR. If this is intentional (e.g., deferred to a follow-up), consider noting it.

• [style] reusable-{code,fix,review,retro,triage}.yml — The "Prepare workspace (upstream defaults + org overrides)" step is duplicated verbatim across all 5 reusable workflows (~20 lines each). Consider extracting to a shared composite action to reduce maintenance surface and ensure consistency when the layering logic evolves.

Info

• [platform-security] internal/mint/main.go — The hardcoded fullsend-ai/fullsend/ prefix in OIDC job_workflow_ref validation is an acceptable trade-off for cross-org workflow_call support. The .github/workflows/ path check and optional ALLOWED_WORKFLOW_FILES guard still apply, limiting the attack surface to legitimate workflow files in the upstream repo.

• [platform-security] internal/scaffold/fullsend-repo/.github/actions/validate-enrollment/action.yml — Enrollment validation logic is now inlined rather than delegating to validate-source-repo.sh. This is a security improvement: it prevents org overrides in customized/scripts/ from altering the enrollment gate. The test explicitly asserts this (assert.NotContains(t, s, "validate-source-repo.sh")).

• [injection-defense] dispatch.yml routing — All GitHub context values (github.event.comment.body, github.event.action, label names, user logins) are assigned to environment variables via env: blocks and consumed as shell variables, never interpolated in GitHub Actions expressions. Event payload forwarding uses jq on $GITHUB_EVENT_PATH with field truncation (body: .body[:4096]). No expression injection vectors found.

• [correctness] reusable-fix.yml fork PR detection — The new implementation fails closed when fork status cannot be determined (if [[ -z "${IS_FORK}" ]]; then exit 1), which is more secure than the prior version that defaulted to "false" on API failure.

• [correctness] .github/workflows/release.yml — Tag pattern tightened from v* to v[0-9]+.[0-9]+.[0-9]+*, preventing non-semver tags (e.g., v-test, version-bump) from triggering releases.

Footer

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

---

Protected paths detected — this PR modifies files under one or more
protected paths. The review agent cannot approve PRs that touch these paths.
A human reviewer must approve this PR.

Protected files in this PR:

• .github/actions/mint-token/action.yml
• .github/actions/setup-gcp/action.yml
• .github/actions/validate-enrollment/action.yml
• .github/workflows/notify-adr-slack.yml
• .github/workflows/release.yml
• .github/workflows/reusable-code.yml
• .github/workflows/reusable-fix.yml
• .github/workflows/reusable-retro.yml
• .github/workflows/reusable-review.yml
• .github/workflows/reusable-triage.yml

Previous run (3)

Review: #792

Head SHA: 2a336a9
Timestamp: 2026-05-13T00:00:00Z
Outcome: comment-only

Summary

This is a large, well-structured PR implementing three ADRs (31, 34, 35) that fundamentally reshapes the agent pipeline architecture. The security-critical changes — OIDC mint validation expansion, centralized dispatch routing, fork PR blocking, and expression injection prevention — are all handled correctly. The transition from per-stage shim jobs to centralized routing in dispatch.yml is sound, with proper fail-closed behavior and defense-in-depth (fork checks at both dispatch and reusable workflow layers). The layered content resolution design cleanly separates upstream defaults from org customizations. A few non-blocking observations are noted below.

Findings

Medium

• [style/conventions] .github/workflows/reusable-{code,fix,review,retro,triage}.yml — The "Prepare workspace (upstream defaults + org overrides)" step is ~20 lines of identical shell duplicated across all 5 reusable workflows, and partially duplicated again in prioritize.yml and repo-maintenance.yml (7 copies total). If the layering logic changes (e.g., adding a new directory category), all copies must be updated in lockstep.
  Remediation: Consider extracting workspace preparation into a composite action (e.g., .github/actions/prepare-workspace/action.yml) that all reusable workflows and non-agent workflows can reference. This would reduce the 7 copies to 1 authoritative implementation.

Low

• [correctness] internal/scaffold/fullsend-repo/.github/workflows/dispatch.yml — The has_label helper splits label names on comma (IFS=',' read -ra labels), which would misparse a label whose name contains a literal comma. GitHub label names rarely contain commas, but the function silently misbehaves if they do.
  Remediation: Document this assumption or switch to a JSON-based label check (e.g., pass labels as a JSON array and use jq for membership testing).

• [correctness] internal/scaffold/fullsend-repo/.github/workflows/dispatch.yml — The COMMENT_BODY env var receives the full github.event.comment.body for command extraction. While safely in an env: block (no expression injection), a comment whose first word coincidentally matches a command prefix (e.g., a user writes /triage is broken) would trigger dispatch. This matches the previous behavior (the old shim used startsWith) so it's not a regression, but worth noting.

Info

• [style/conventions] docs/ADRs/0035-layered-content-resolution.md — Status is "Proposed" but this PR implements the decision. Consider updating to "Accepted" to reflect that the implementation is shipping.

• [correctness] The retro role changed from role: retro (old thin workflow) to role: fullsend (reusable workflow). This is intentional per the ADR 31 update ("retro and prioritize share the fullsend App") and the dispatch.yml routing (retro|prioritize → fullsend). The change is consistent across all three touchpoints (dispatch role mapping, reusable workflow mint step, ADR documentation).

• [platform-security] The mint validation change (internal/mint/main.go) correctly adds fullsend-ai/fullsend/ as an accepted job_workflow_ref prefix for cross-org workflow_call. The existing .github/workflows/ path check and optional ALLOWED_WORKFLOW_FILES allowlist still apply after the prefix match, preventing arbitrary paths from minting tokens. Test coverage includes both the positive case (upstream workflow ref accepted) and the negative case (upstream non-workflow path rejected).

• [platform-security] Fork PR blocking has proper defense-in-depth: (1) dispatch.yml routing checks PR_HEAD_REPO == PR_BASE_REPO for pull_request_review events, (2) dispatch.yml has a dedicated "Block fork PRs for fix stage" step using the API for issue_comment events, and (3) reusable-fix.yml independently performs fork detection with fail-closed semantics. Three layers of protection.

Footer

Outcome: comment-only
This review applies to SHA 2a336a96d40c98881ffcc4c29ffe2e90c2eaf78c. Any push to the PR head clears this review and requires a new evaluation.

Previous run (4)

Review: #792

Head SHA: c9cc35c
Timestamp: 2026-05-13T00:00:00Z
Outcome: request-changes

Summary

This is a large, well-structured PR implementing three ADRs (31, 34, 35) to move from full-copy scaffold workflows to thin callers backed by upstream reusable workflows. The architectural direction is sound and the security posture is generally strong — inputs are routed through env: blocks (no expression injection), event payloads are sanitized via jq from $GITHUB_EVENT_PATH, fork PRs are blocked, and the mint token validator is correctly extended for cross-org workflow_call refs. However, there is one high-severity security finding in the mint validation change that must be addressed, and several medium-severity correctness and security issues.

Findings

High

• [Platform security] internal/mint/main.go — The upstream prefix fullsend-ai/fullsend/ is hardcoded, not derived from configuration. If the upstream repo is ever forked or the org name changes, the hardcoded string becomes a security bypass vector or a silent failure. More critically, the current implementation accepts any workflow ref starting with fullsend-ai/fullsend/ — an attacker who can create a workflow in a public fork of fullsend-ai/fullsend and get it referenced via workflow_call could potentially mint tokens. The existing .github/workflows/ path check mitigates this somewhat, but the trust boundary should be explicit: the upstream prefix should be loaded from an environment variable (e.g., UPSTREAM_REPO) rather than hardcoded, to maintain the security model's configurability and to make the trust decision auditable.
  Remediation: Make the upstream repo prefix configurable via environment variable (e.g., UPSTREAM_REPO=fullsend-ai/fullsend). Validate that the env var is set at startup. This aligns with the existing pattern of ALLOWED_ORGS being configurable.

Medium

• [Correctness] internal/scaffold/fullsend-repo/.github/workflows/dispatch.yml (routing step) — The COMMENT_BODY env var is set from ${{ github.event.comment.body }} and then parsed with head -1 | awk '{print $1}' to extract the command. This is safe from expression injection (uses env:), but the COMMENT_BODY variable is set unconditionally for all event types including issues, pull_request_target, and pull_request_review where github.event.comment is null. When null, ${{ github.event.comment.body }} evaluates to empty string in GHA, so this is functionally correct but semantically noisy — every non-comment event sets 7+ comment-related env vars to empty strings.
  Remediation: No functional fix required, but consider documenting this is intentional (the routing shell already handles empty values correctly via case matching).

• [Correctness] .github/workflows/reusable-retro.yml:117 — The expression ${{ fromJSON(inputs.event_payload).pull_request.html_url || fromJSON(inputs.event_payload).issue.html_url }} uses || which is a GHA expression operator that works, but fromJSON(inputs.event_payload).comment.body || '' }} on the next line uses || '' which evaluates to empty string if comment.body is falsy. If event_payload does not contain a comment key at all, fromJSON() will error. The dispatch.yml sanitizes the payload to always include comment: null when no comment exists, and fromJSON of null with .body access would produce empty string in GHA — but this is fragile.
  Remediation: Use the null-coalescing pattern: ${{ fromJSON(inputs.event_payload).comment.body }} and let GHA default to empty string, or use a shell step with jq for safer extraction.

• [Correctness] internal/scaffold/fullsend-repo/.github/workflows/dispatch.yml — The cancel-in-progress: false on the shim's dispatch job means that if two events fire rapidly for the same issue/PR (e.g., issue opened + immediate comment), both dispatch jobs run concurrently. The thin callers have per-stage concurrency groups with cancel-in-progress: true, so the stage-level cancellation still works, but two concurrent dispatch runs could trigger two gh workflow run calls for the same stage before the first thin caller starts — resulting in two workflow runs where only one is needed. The old shim had per-stage concurrency groups at the shim level that prevented this.
  Remediation: Document this as a known trade-off of the unified dispatch job. The thin caller concurrency groups provide the primary protection. Alternatively, consider adding a short delay or using per-stage concurrency at the thin caller level (which is already present).

• [Style/conventions] internal/scaffold/fullsend-repo/.github/workflows/dispatch.yml — The routing step is ~160 lines of bash in a single run: block. This is the most complex single step in the codebase and would benefit from being extracted into a script file for testability. However, since ADR 35 means scripts are provided at runtime via layering, and this workflow runs before workspace preparation, inlining is the pragmatically correct choice. Consider adding a comment explaining why the logic is inlined rather than in a script.

• [Correctness] internal/scaffold/fullsend-repo/templates/shim-workflow-call.yaml — The shim's bot filter github.event.comment.user.type != 'Bot' is the only filter on the dispatch job. In the old shim, each per-stage job had its own if: condition that precisely matched the relevant events. The new universal dispatch job fires for every non-bot event the shim receives (issues opened/edited/labeled, pull_request_target opened/synchronize/ready_for_review/closed, pull_request_review submitted, issue_comment created). This means the dispatch workflow (which mints an OIDC token) runs for events that will ultimately route to no stage (e.g., a pull_request_target labeled event). This is a minor efficiency concern — the mint runs but the routing step returns empty and subsequent steps are skipped.
  Remediation: Consider adding common no-op filters to the shim's if: condition to avoid minting tokens for events that are guaranteed to not route to any stage.

Low

• [Correctness] internal/scaffold/fullsend-repo/.github/workflows/dispatch.yml — The Block fork PRs for fix stage step uses github.event.issue.pull_request in its if: condition, but for pull_request_review events that trigger the fix stage, the event object has github.event.pull_request, not github.event.issue.pull_request. The fork check in the routing step handles pull_request_review correctly (checks PR_HEAD_REPO == PR_BASE_REPO), so the dedicated fork-blocking step is redundant for that path — it only fires for issue_comment triggered /fix commands. This is not a bug (the routing step already blocks forks), but the if: condition is misleading.
  Remediation: Add a comment clarifying this step only applies to issue_comment-triggered fix dispatches.

• [Style/conventions] docs/ADRs/0035-layered-content-resolution.md — ADR status is "Proposed" but the PR implements it. Should be "Accepted" if this PR constitutes acceptance.
  Remediation: Update status to "Accepted" or document the acceptance process.

Info

• [Intent alignment] The PR removes the post-run-link job from the shim, which previously posted a "fullsend X is working on this" comment with a link to the shim run. This is a user-visible behavior change not called out in the PR description. Users will no longer get an immediate feedback comment when an agent is dispatched.

• [Intent alignment] The PR changes the retro workflow's mint role from retro to fullsend. This aligns with the dispatch.yml routing (retro|prioritize → fullsend), but means the retro agent now gets a broader token than before. The PR description mentions this but the security implication (broader token scope) should be explicitly acknowledged.

• [Injection defense] PR body inspected for injection patterns — none found. The PR body contains only descriptive content, architecture diagrams, and test plans. No prompt injection, non-rendering Unicode, or instruction-like directives detected.

Footer

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

Previous run (5)

Review: #792

Head SHA: 264dfa5
Timestamp: 2026-05-13T00:00:00Z
Outcome: comment-only

Summary

This PR implements three ADRs (31, 34, 35) that restructure the fullsend agent pipeline from duplicated full workflows into thin callers backed by upstream reusable workflows, centralized event routing in dispatch.yml, and runtime content layering. The architecture is sound and the security-critical changes — mint OIDC validation widening, expression injection prevention via env blocks, fork PR defense-in-depth — are well-executed. No critical or high findings. Several medium and info-level observations regarding spec/test gaps and maintenance burden are noted below.

Findings

Medium

• [Correctness] docs/normative/admin-install/v1/adr-0012-fullsend-repo-files/SPEC.md — The normative spec still lists paths that are no longer installed by the scaffold after this PR: agents/triage.md, env/gcp-vertex.env, env/triage.env, harness/triage.yaml, policies/triage.yaml, scripts/validate-triage.sh, scripts/reconcile-repos.sh, .github/scripts/setup-agent-env.sh. These files are now in layeredDirs or upstreamOnlyDirs and are skipped by WalkFullsendRepo, but the spec claims they "SHALL exist on the default branch after a complete admin install." Only .github/actions/fullsend/action.yml was removed from the table. The spec is now inaccurate — tools or tests relying on it will have wrong expectations.
  Remediation: Update the SPEC.md table to remove layered/upstream-only paths, or add a new section documenting which paths are now provided at runtime vs. installed by scaffold.

• [Correctness] internal/scaffold/scaffold_test.go:TestCustomizedDirsInstalled — The test verifies customized/{agents,skills,schemas,harness,policies,scripts}/.gitkeep but omits customized/env/.gitkeep, which is created by this PR (the file exists in the scaffold). The env/ directory is in layeredDirs, so customized/env/.gitkeep should be verified as installed.
  Remediation: Add "customized/env/.gitkeep": false to the expected map.

Low

• [Style/conventions] .github/workflows/reusable-{code,fix,review,retro,triage}.yml — The "Prepare workspace (upstream defaults + org overrides)" step is a ~20-line shell block copy-pasted identically across all 5 reusable workflows. Any change to the layering logic requires updating all 5. Consider extracting this into a composite action or a script in the upstream repo to reduce maintenance burden.
  Remediation: Extract the workspace preparation logic into a composite action (e.g., .github/actions/prepare-workspace/action.yml) or a shell script referenced by all reusable workflows.

Info

• [Correctness] internal/scaffold/fullsend-repo/templates/shim-workflow-call.yaml — The post-run-link job was removed from the shim. This job previously posted a comment linking to the agent run logs on the issue/PR. No replacement is provided. This is a user-facing feature regression — users will no longer see "fullsend X is working on this — view logs" comments from the shim.

• [Platform security] internal/mint/main.go — The upstream prefix fullsend-ai/fullsend/ is hardcoded. If the upstream repo is renamed, forked to a different org, or if additional upstream repos are added, the mint code must be updated. This is acceptable for now but worth noting as technical debt. The security impact is mitigated by the fact that repository_owner is still validated against ALLOWED_ORGS before the prefix check.

• [Correctness] internal/scaffold/fullsend-repo/templates/shim-workflow-call.yaml — The dispatch job uses cancel-in-progress: false, unlike the previous per-stage jobs which used cancel-in-progress: true. This means concurrent dispatches for the same issue/PR will queue rather than cancel. This is correct because per-stage cancellation is now handled by the thin callers' concurrency groups, but it means the dispatch step itself may queue behind a slow dispatch for the same item.

• [Injection defense] PR body and commit messages — Scanned for prompt injection patterns, non-rendering Unicode, and instruction-like directives. None detected. The PR body is descriptive and factual.

Footer

Outcome: comment-only
This review applies to SHA 264dfa582b8855b1c1b13bd6c642f54b5218751a. Any push to the PR head clears this review and requires a new evaluation.

Previous run (6)

Review: automated review

Outcome: failure
Reason: stale-head

The review agent reviewed commit 64e098883f6506f3be5c6ac77a60fc46fff8047a but the PR HEAD is now 264dfa582b8855b1c1b13bd6c642f54b5218751a. This review was discarded to avoid approving unreviewed code.

Previous run (7)

Review: #792

Head SHA: 6d611f5
Timestamp: 2026-05-12T00:00:00Z
Outcome: request-changes

Summary

This PR implements three ADRs (31, 34, 35) to convert agent workflows into thin callers delegating to upstream reusable workflows, centralize event routing in dispatch.yml, and implement layered content resolution. The architecture is sound and the security model is well-considered — expression injection prevention via env: blocks, fork PR fail-closed checks, enrollment validation inlining, and OIDC scope widening with proper workflow-path validation. However, there is a high-severity version tag inconsistency where prioritize.yml and repo-maintenance.yml reference ref: v1 for sparse checkout while all reusable workflows and action references use @v0, which will cause runtime failures when the v1 tag does not exist or points to different content. ADR 35 text also references @v1, creating documentation drift from the implementation.

Findings

High

• [Correctness] internal/scaffold/fullsend-repo/.github/workflows/prioritize.yml:36 and internal/scaffold/fullsend-repo/.github/workflows/repo-maintenance.yml:37 — Version tag mismatch: these workflows use ref: v1 for sparse checkout of upstream defaults, while all reusable workflows use ref: v0 and all action references use @v0. If v1 does not exist or points to different code than v0, these workflows will fail or behave inconsistently at runtime. ADR 35 (docs/ADRs/0035-layered-content-resolution.md) also references fullsend-ai/fullsend@v1, creating documentation-implementation drift.
  Remediation: Align all version tag references to the same tag (v0 or v1). Update prioritize.yml line 36, repo-maintenance.yml line 37, and ADR 35 text (lines mentioning @v1) to match the tag used by reusable workflows.

• [Correctness] internal/scaffold/fullsend-repo/.github/workflows/prioritize.yml — prioritize.yml does its own workspace layering with a different sparse-checkout path pattern (internal/scaffold/fullsend-repo/agents/, internal/scaffold/fullsend-repo/skills/, etc. plus .github/scripts/) compared to reusable workflows which checkout internal/scaffold/fullsend-repo/ as a whole. This means prioritize.yml sparse-checkouts .github/scripts/ from the upstream repo root (line ~3019), while reusable workflows copy setup-agent-env.sh from ${SRC}/.github/scripts/. If .github/scripts/ at the repo root differs from what's under internal/scaffold/fullsend-repo/.github/scripts/, prioritize gets different scripts than agent workflows.
  Remediation: Verify that .github/scripts/ in the upstream repo root and internal/scaffold/fullsend-repo/.github/scripts/ are kept in sync, or standardize the sparse-checkout pattern across all workflows.

Medium

• [Platform security] internal/mint/main.go:108-116 — The OIDC job_workflow_ref validation now accepts the hardcoded fullsend-ai/fullsend/ prefix in addition to {org}/.fullsend/. This widens the trust boundary to allow any workflow in the upstream repo (that passes the existing .github/workflows/ path check) to mint tokens for any allowed org. The existing workflow-file path validation mitigates arbitrary code execution, but if a malicious workflow were added to fullsend-ai/fullsend/.github/workflows/, it could mint tokens for any org in ALLOWED_ORGS. This is an accepted trade-off for cross-org workflow_call, but the hardcoded string should be documented.
  Remediation: Add a code comment explaining that fullsend-ai/fullsend is the canonical upstream repo and that adding workflows to it requires the same security scrutiny as modifying the mint itself. Consider making the upstream prefix configurable via environment variable for future flexibility.

• [Style/conventions] Multiple reusable workflows — The "Prepare workspace (upstream defaults + org overrides)" shell block (~20 lines) is copy-pasted identically across all 5 reusable workflows (reusable-{code,fix,review,retro,triage}.yml). This creates a maintenance burden where workspace-prep changes must be applied in 5 places, plus the slightly different versions in prioritize.yml and repo-maintenance.yml.
  Remediation: Extract the workspace preparation logic into a shared composite action (e.g., .github/actions/prepare-workspace/action.yml) or a shell script checked out from upstream. Note: composite actions cannot contain uses: actions/checkout, so a script-based approach may be necessary.

• [Correctness] docs/ADRs/0035-layered-content-resolution.md and docs/architecture.md — ADR 35 references fullsend-ai/fullsend@v1 for sparse checkout (lines 1668, 1727 in the diff), but the actual implementation uses @v0. The architecture.md update also references @v1. Documentation should match the implementation.
  Remediation: Update ADR 35 and architecture.md to reference @v0 consistently.

Low

• [Correctness] internal/scaffold/scaffold_test.go TestCustomizedDirsInstalled — The test checks for 6 customized/ gitkeep files but the scaffold includes 7 (missing customized/env/.gitkeep). This is a test coverage gap.
  Remediation: Add "customized/env/.gitkeep": false to the expected map in TestCustomizedDirsInstalled.

• [Style/conventions] internal/scaffold/fullsend-repo/templates/shim-workflow-call.yaml — The dispatch job uses cancel-in-progress: false, which means if two events fire in quick succession for the same issue/PR, both dispatch jobs run to completion and both will trigger downstream workflows. The thin callers have their own per-stage concurrency groups with cancel-in-progress: true, so this is safe but worth documenting why false is used at the dispatch level.
  Remediation: Add a comment explaining that dispatch must not cancel in-progress because it routes to different stages — cancellation is handled per-stage by thin callers.

Info

• [Correctness] The retro role changed from retro to fullsend in the reusable workflow. The dispatch.yml routing correctly maps retro|prioritize → fullsend for the role check. This is consistent but is a behavioral change — ensure ROLE_APP_IDS in the mint environment includes the fullsend role mapping for orgs that previously only had retro.

• [Platform security] The validate-enrollment action was changed from calling scripts/validate-source-repo.sh to inlining the validation logic. This is a security improvement — it prevents customized/scripts/ overrides from bypassing enrollment validation.

PR-Specific Checks

PR body injection defense: No prompt injection patterns or non-rendering Unicode detected in the PR description, commit messages, or body text. The PR body contains only legitimate technical documentation.

Scope authorization: No linked issue. The PR implements three previously accepted/proposed ADRs (31, 34, 35). The scope is large but coherent — all changes serve the reusable-workflows architecture. The fullsend-no-fix label is present, which is appropriate for a PR of this scope.

Footer

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

Previous run (8)

Review: #792

Head SHA: ea67fbf
Timestamp: 2026-05-12T00:00:00Z
Outcome: comment-only

Summary

This is a well-structured PR implementing three ADRs (30/31, 33/34, 34/35) that converts agent workflows to thin callers delegating to upstream reusable workflows, centralizes event routing in dispatch.yml, and introduces layered content resolution. The security posture is preserved or improved: expression injection is mitigated by using env blocks in dispatch routing, validate-enrollment is correctly inlined to prevent script override attacks, fork PR detection fails closed, and the mint OIDC validation properly extends to accept the upstream fullsend-ai/fullsend/ workflow ref prefix with existing .github/workflows/ path constraints. Two medium findings relate to a tag version inconsistency and a concurrency behavior change that should be verified before merge.

Findings

Medium

• [Correctness] prioritize.yml:39, repo-maintenance.yml:35, ADR 0035, architecture.md — Tag inconsistency: v0 vs v1 for upstream sparse-checkout ref. The five reusable workflows (reusable-{code,fix,retro,review,triage}.yml) and all composite action references use @v0 / ref: v0. However, prioritize.yml and repo-maintenance.yml use ref: v1 for their sparse-checkout of upstream defaults. ADR 35 and docs/architecture.md also reference @v1. If the v1 tag does not exist (or points to different content than v0), the prioritize and repo-maintenance workflows will fail at runtime.
  Remediation: Align all sparse-checkout ref: values and action @ tags to the same version (v0 or v1). Update ADR 35 and architecture.md to match.

• [Correctness] shim-workflow-call.yaml:36 — Shim concurrency group changed from per-stage cancel-in-progress to single non-cancelling group. The old shim had per-stage concurrency groups (triage-${{ }}, code-${{ }}, etc.) each with cancel-in-progress: true. The new shim uses a single fullsend-dispatch-${{ }} group with cancel-in-progress: false. While per-stage cancellation is handled by thin callers, dispatch jobs themselves will queue rather than cancel. For a rapid sequence of events on the same issue (e.g., issue opened then labeled ready-to-code), multiple dispatch jobs will run sequentially instead of the later one cancelling the earlier. This could increase Actions minute consumption and cause unnecessary dispatches.
  Remediation: Consider whether cancel-in-progress: true is safe for the unified dispatch group, or document why false is intentional (e.g., to avoid dropping fix dispatches when a review arrives).

Low

• [Style/conventions] .github/actions/fullsend/action.yml, action.yml — Near-duplicate action files with ~120 lines of shared CLI/OpenShell install logic. The root action.yml (public composite action) and .github/actions/fullsend/action.yml (internal, used by reusable workflows) share the same install logic but differ in fullsend-dir/target-repo input handling. Over time these will drift independently.
  Remediation: Consider having one delegate to the other, or extracting shared install logic into a script.

Info

• [Intent alignment] internal/mint/main.go — The mint now accepts both {org}/.fullsend/ and fullsend-ai/fullsend/ as valid job_workflow_ref prefixes. This is correctly constrained: the .github/workflows/ path check still applies after prefix stripping, and ALLOWED_WORKFLOW_FILES filtering (if configured) is applied. The hardcoded fullsend-ai/fullsend/ prefix is acceptable since this is the canonical upstream repo.

• [Platform security] validate-enrollment/action.yml — Enrollment validation logic was previously delegated to validate-source-repo.sh (overridable via customized/scripts/). It is now inlined in the composite action, preventing override. This is a security improvement.

• [Correctness] dispatch.yml — All event context used in routing flows through env: blocks, not GitHub Actions expressions in if: conditions. This eliminates expression injection risk from attacker-controlled fields. The routing logic correctly uses shell case statements with quoted variables.

• [Correctness] Tests are comprehensive: new tests for upstream workflow ref acceptance, upstream non-workflow path rejection, layered directory filtering, customized directory inclusion, and the simplified shim structure.

Footer

Outcome: comment-only
This review applies to SHA ea67fbff12f72868d5801a2fc4828b336c4ff4d1. Any push to the PR head clears this review and requires a new evaluation.

Previous run (9)

Review: #792

Head SHA: 72db671
Timestamp: 2026-05-12T00:00:00Z
Outcome: request-changes

Summary

This PR implements three ADRs (30/31, 33/34, 34/35) to move from full-copy scaffold workflows to thin callers backed by upstream reusable workflows, centralize event routing in dispatch.yml, and add runtime layered content resolution. The architectural direction is sound — scaffold shrinks from ~82 to ~24 files, infrastructure patches ship once upstream, and expression injection risks are eliminated by moving routing logic from GHA expressions to shell case with env: blocks. The mint validation update to accept upstream job_workflow_ref is well-tested. However, there is a version tag inconsistency (v0 vs v1) across two scaffold workflows that will cause runtime failures, and a near-duplicate action.yml creates maintenance risk.

Findings

High

• [Correctness] internal/scaffold/fullsend-repo/.github/workflows/prioritize.yml:10 — Upstream defaults checkout uses ref: v1 but all reusable workflows and action references throughout the PR use ref: v0 (and @v0). The same mismatch exists in repo-maintenance.yml:37. At runtime, if only v0 is tagged (which is what thin callers and reusable workflows reference), these two workflows will fail with a "reference not found" error on the actions/checkout step.
  Remediation: Change ref: v1 to ref: v0 in both prioritize.yml and repo-maintenance.yml, or ensure the intended tag exists and update all references to be consistent.

Medium

• [Correctness] docs/ADRs/0035-layered-content-resolution.md:59 — ADR 35 states "sparse-checkouts upstream defaults from fullsend-ai/fullsend@v1 at runtime" but the reusable workflows implement @v0. The ADR text should match the implemented tag to avoid confusion during future maintenance.
  Remediation: Update ADR 35 to reference @v0 consistently.

• [Style/conventions] action.yml and .github/actions/fullsend/action.yml — Two nearly-identical 170+ line composite action files exist. Root action.yml adds fullsend-dir and target-repo inputs with different Run fullsend step logic; .github/actions/fullsend/action.yml hardcodes paths. The rest (CLI install, OpenShell install, retry logic, upload) is duplicated verbatim. A bug fix in one will need to be manually replicated in the other.
  Remediation: Consider having the .github/actions/fullsend/action.yml delegate to root action.yml with default inputs, or extract shared logic into a script.

• [Platform security] internal/mint/main.go — The upstream repo prefix "fullsend-ai/fullsend/" is hardcoded. If the upstream repo is renamed, this requires a code change and mint redeployment. This is a minor operational risk, not a vulnerability (OIDC org validation and ALLOWED_ORGS already prevent abuse).
  Remediation: Consider making the upstream prefix configurable via environment variable alongside ALLOWED_ORGS.

Low

• [Correctness] internal/scaffold/fullsend-repo/.github/workflows/prioritize.yml — The sparse-checkout pattern in prioritize.yml includes .github/scripts/ but the reusable workflows do not (they copy setup-agent-env.sh from the scaffold subtree instead). The layering approach is subtly different between prioritize.yml and the reusable workflows, which may cause confusion.
  Remediation: Document the two layering patterns or unify them.

Info

• [Platform security] reusable-fix.yml — Fork PR detection improved from fail-open (|| echo "false") to fail-closed (|| true + empty check + exit 1). Good security hardening.

• [Platform security] validate-enrollment/action.yml — Enrollment validation inlined instead of calling validate-source-repo.sh, preventing org overrides in customized/scripts/ from bypassing enrollment checks. Test explicitly verifies the script reference is absent.

• [Injection defense] dispatch.yml — Event routing uses env: blocks exclusively to pass event context to shell, eliminating expression injection vectors that existed in the old shim's if: conditions. Comment body is read only via awk '{print $1}' for command extraction.

• [Correctness] Retro agent role changed from retro to fullsend (shared with prioritize). dispatch.yml retro|prioritize case branch maps both to fullsend role. Consistent with the PR description.

• [Correctness] Shim dispatch concurrency changed from per-stage groups with cancel-in-progress: true to single fullsend-dispatch-* group with cancel-in-progress: false. Per-stage cancellation is now handled by thin callers' own concurrency groups, so this is correct.

Footer

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

Previous run (10)

Review: #792

Head SHA: 896eb0e
Timestamp: 2026-05-12T00:00:00Z
Outcome: comment-only

Summary

This PR implements three coordinated architectural changes: reusable workflows (ADR 31), centralized event routing, and layered content resolution (ADR 35). The overall design is sound — the security model is preserved through defense-in-depth (fork PR blocking at both dispatch and reusable workflow layers, OIDC validation expanded to accept upstream workflow refs with the existing .github/workflows/ path check still enforced), injection-safe routing via env: blocks, and inlined enrollment validation that prevents override via customized/scripts/. The code changes are well-tested with comprehensive unit test updates. A few non-blocking observations follow.

Findings

Medium

• [Correctness] docs/ADRs/ — ADR numbering is inconsistent between the PR title (ADR 31, 34, 35) and body (ADR 30, 33, 34). The actual ADR files are 0031 and 0035. The centralized routing decision (called ADR 33 in the body, ADR 34 in the title) has no corresponding ADR document in this PR.
  Remediation: Reconcile the numbering and consider adding the centralized routing ADR as a separate document (or note it as a future follow-up).

• [Correctness] internal/scaffold/fullsend-repo/.github/workflows/dispatch.yml — The dispatch job in the shim uses cancel-in-progress: false, which means rapidly-firing events for the same issue/PR will queue multiple dispatch runs rather than canceling stale ones. The thin callers handle per-stage cancellation, but redundant dispatch runs will still consume Actions minutes executing routing logic, minting OIDC tokens, and checking kill switches before discovering no workflow to dispatch (or dispatching a run that the thin caller's concurrency group then cancels).
  Remediation: Consider whether cancel-in-progress: true on the dispatch concurrency group is acceptable, since the latest event's routing decision supersedes earlier ones. If not, document the tradeoff.

Low

• [Style/conventions] internal/scaffold/scaffold.go — The executableFiles map still lists 18 scripts/* entries, but WalkFullsendRepo now skips all scripts/ paths. These entries are dead code for the install path (only exercised via WalkFullsendRepoAll in tests). Not a bug, but could confuse future maintainers into thinking scripts are still installed with executable mode.
  Remediation: Add a comment to executableFiles clarifying these are used only for WalkFullsendRepoAll / test validation, not for scaffold installation.

• [Correctness] .github/workflows/reusable-retro.yml:line 80 — RETRO_COMMENT: ${{ fromJSON(inputs.event_payload).comment.body || '' }} passes user-controlled comment content through an expression into an env: value. While this is injection-safe (env context, not run: interpolation), the comment body has already been truncated to 4096 bytes in dispatch.yml's jq payload construction. If the retro agent relies on the full comment body, the truncation could silently drop content.
  Remediation: Document the 4096-byte truncation in the reusable workflow or event_payload input description so downstream agents are aware of the limit.

Info

• [Platform security] internal/mint/main.go — The hardcoded fullsend-ai/fullsend/ prefix for upstream workflow ref validation creates a coupling to the upstream repo name. If the repo were ever renamed or forked for distribution, the mint validation would need updating. This is an acceptable tradeoff given the current architecture.

• [Correctness] Behavioral change in retro role: the old retro.yml thin caller used role: retro, the new reusable retro workflow uses role: fullsend. This changes which GitHub App identity the retro agent operates under. The PR body documents this ("retro and prioritize share the fullsend App") and dispatch.yml's role check maps retro|prioritize → fullsend, so this is consistent — just worth noting for existing deployments that may have configured the retro role separately.

Footer

Outcome: comment-only
This review applies to SHA 896eb0e34e3f4f0865643c7f1c4cec3e44636695. Any push to the PR head clears this review and requires a new evaluation.

Previous run (11)

Review: #792

Head SHA: 48c6d6d
Timestamp: 2026-05-12T00:00:00Z
Outcome: request-changes

Summary

This PR implements three ADRs (30/33/34 — titled 31/34/35 in the PR) to move from full-copy scaffold workflows to thin callers backed by reusable workflows, centralized event routing in dispatch.yml, and layered content resolution via runtime sparse-checkout. The architecture is sound, security controls are well-designed (expression-injection-safe routing, inlined enrollment validation, defense-in-depth fork-PR blocking, OIDC mint expansion with proper tests), and tests are comprehensive. However, all reusable workflows and scaffold workflows sparse-checkout upstream defaults at ref: v1 while every action and reusable workflow reference uses @v0. This version tag mismatch will cause runtime failures if the tags point to different commits (or if v1 doesn't exist yet), and must be resolved before merge.

Findings

High

• [correctness] reusable-code.yml:14, reusable-fix.yml:18, reusable-retro.yml:14, reusable-review.yml:14, reusable-triage.yml:14, internal/scaffold/fullsend-repo/.github/workflows/prioritize.yml:39, internal/scaffold/fullsend-repo/.github/workflows/repo-maintenance.yml:38 — Version tag mismatch between sparse-checkout ref: v1 and all action/workflow references using @v0. All 5 reusable workflows checkout upstream defaults with ref: v1 but reference composite actions at @v0. The thin callers in the scaffold also use @v0 for reusable workflow refs. The ADR text was updated from v1→v0. If these tags resolve to different commits (or v1 doesn't exist), the "Checkout upstream defaults" step will either fail or pull mismatched content.
  Remediation: Unify all fullsend-ai/fullsend version references to the same tag. Either change all ref: v1 to ref: v0, or change all @v0 to @v1. The ADR already says @v0, so changing the sparse-checkout ref: to v0 across all 7 files is the most consistent fix.

Medium

• [correctness] action.yml and .github/actions/fullsend/action.yml — Near-duplicate composite actions at repo root and under .github/actions/fullsend/. The root action adds fullsend-dir and target-repo inputs with fallback logic, while the nested version hardcodes paths. The install step, OpenShell install, retry logic, and validation dependencies are copy-pasted. Changes to one will not automatically propagate to the other.
  Remediation: Consider having the nested action delegate to the root action (or vice versa), or extract shared logic into a single action with inputs that cover both use cases.

• [correctness] internal/scaffold/fullsend-repo/.github/workflows/dispatch.yml:165 — The dispatch routing for the review stage triggers on pull_request_target events with actions opened|synchronize|ready_for_review, but the old shim triggered on all non-closed pull_request_target events. Verify that restricting to opened|synchronize|ready_for_review is the intended behavior change (draft PRs were previously reviewed on every non-closed event).

• [platform-security] internal/mint/main.go:574-586 — The OIDC job_workflow_ref validation now accepts the hardcoded prefix fullsend-ai/fullsend/ in addition to {org}/.fullsend/. This is necessary for cross-org workflow_call, and the subsequent .github/workflows/ path check and ALLOWED_WORKFLOW_FILES check still apply. Tests cover both acceptance and rejection paths. The security boundary holds, but this broadening should be documented in the mint's operational runbook — operators need to know that tokens can now be minted by workflows in fullsend-ai/fullsend, not just the org's .fullsend repo.

Low

• [style] reusable-{code,fix,retro,review,triage}.yml — The "Prepare workspace" step (~15 lines of shell) is copy-pasted identically across all 5 reusable workflows. If the layering logic changes (e.g., adding a new directory), all 5 files need updating.
  Remediation: Extract the workspace preparation into a composite action (e.g., .github/actions/prepare-workspace/action.yml).

Info

• [correctness] The retro stage changed from minting with role: retro to role: fullsend, and the dispatch routing maps retro|prioritize → fullsend. The PR body explains this is because retro and prioritize share the fullsend App (no dedicated PEM). This is an intentional scope change — the fullsend role token may have broader permissions than a dedicated retro role would. Verify this matches the intended permission model.

• [intent-alignment] The PR title says "ADR 31, ADR 34, ADR 35" but the body and code reference ADR 30, 33, 34. The actual ADR files modified/added are 0031-reusable-workflows-for-action-installed-distribution.md and 0035-layered-content-resolution.md. Minor numbering confusion in the title vs. body, but the code is internally consistent.

• [injection-defense] The dispatch routing correctly uses env: blocks to pass all untrusted inputs (COMMENT_BODY, EVENT_ACTION, user logins, labels) into the shell step, avoiding GitHub Actions expression injection. The /fix instruction extraction in reusable-fix.yml uses a random heredoc delimiter to prevent GITHUB_OUTPUT injection. No injection patterns found in PR body or commit messages.

• [correctness] The validate-enrollment composite action was correctly inlined (replacing the call to validate-source-repo.sh) to prevent a security bypass where a malicious customized/scripts/validate-source-repo.sh could override the enrollment check. Good security improvement.

Footer

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

[fullsend-ai-review]

See the review comment for full details.

[github-actions]

fullsend fix is working on this — view logs

[fullsend-ai-coder]

🔧 Fix agent — iteration 1 (bot-triggered)

Addressed 3 of 6 review findings with code fixes (build verification, reopened PR routing, jsonschema pinning). Disagreed with 3 (OpenShell SHA pin needs external lookup, source_repo validation and cancel-in-progress documentation are info-level).

Fixed (3):

1. Go build verification (internal/layers/workflows.go): Verified go build ./... passes on the PR branch — no merge conflict with PR fix: prevent dev build versions from being pinned in action.yml #790. No code change needed.
2. Missing reopened action in pull_request_target routing (internal/scaffold/fullsend-repo/.github/workflows/dispatch.yml): Added reopened to the pull_request_target case pattern so reopened PRs trigger review, matching the old shim behavior. Updated scaffold test assertion accordingly.
3. jsonschema version range not pinned (action.yml): Pinned jsonschema to exact version 4.23.0 instead of open range >=4.18.0. Hash pinning not added because pip install in GitHub Actions composite actions doesn't support --hash verification without a requirements file, and adding a requirements file is a larger refactor.

Disagreed (3):

1. OpenShell tag-based pinning should use commit SHA: Switching from tag to commit SHA requires looking up the correct SHA from the NVIDIA/OpenShell repo, which is not accessible from this sandbox. The tag pin to v0.0.36 is the current pattern used across the codebase. This hardening can be done in a follow-up PR with verified SHA values.
2. source_repo format validation removed from dispatch.yml: Info-level finding. The reviewer acknowledges source_repo now comes from github.repository (GitHub-controlled) making the validation redundant. Re-adding defense-in-depth validation is a style choice better handled in a follow-up if desired.
3. Document cancel-in-progress: false design choice: Info-level finding. Adding documentation comments is out of scope for this fix pass. The reviewer confirmed the behavior is correct.

Tests: passed

Decision points

• Did not add pip hash pinning for jsonschema (alternatives: Add --hash flag to pip install, Create requirements.txt with hashes, Pin exact version only; rationale: Hash pinning with pip install inline requires --require-hashes which forces all dependencies to also have hashes. A requirements.txt refactor is out of scope. Exact version pin is a meaningful improvement.)

_{Updated by fullsend fix agent}

[github-actions]

fullsend review is working on this — view logs

[github-actions]

fullsend review is working on this — view logs

[github-actions]

fullsend review is working on this — view logs

[github-actions]

fullsend review is working on this — view logs

[github-actions]

fullsend review is working on this — view logs

[github-actions]

fullsend review is working on this — view logs

[github-actions]

fullsend review is working on this — view logs

[github-actions]

fullsend review is working on this — view logs

[github-actions]

fullsend review is working on this — view logs

[github-actions]

fullsend review is working on this — view logs

[github-actions]

fullsend review is working on this — view logs

[fullsend-ai-review]

See the review comment for full details.

[github-actions]

fullsend review is working on this — view logs

[github-actions]

fullsend review is working on this — view logs

[github-actions]

fullsend review is working on this — view logs

[ralphbean]

Review Summary

This is a well-motivated architectural improvement implementing three ADRs (31, 34, 35) that dramatically reduces per-org maintenance burden. The security posture is strong — expression injection avoidance, explicit secret passing, fork PR blocking, and OIDC validation are all correctly implemented.

Since our last review pass (against b1dbc1d), three issues have been fixed:

• ✅ SPEC.md for ADR 0012 removed (was normatively incorrect)
• ✅ Fork PR || true checks now fail closed with explicit empty-string guards
• ✅ e2e verifyInstalled updated correctly for ADR 35

4 items require changes:

1. Mint token validation fails open when ALLOWED_WORKFLOW_FILES is unset — should fail closed
2. Retro agent role escalated from retro to fullsend without justification
3. ADR 0035 miscategorizes env/ as org-only when it's actually a layered directory
4. Test missing customized/env/.gitkeep assertion

4 items noted for follow-up:

• Duplicated "Prepare workspace" step across 5 reusable workflows (extract to composite action)
• No test verifying LAYERED_DIRS consistency between Go and YAML
• post-run-link feedback removed (restore inside fullsend run with correct app identity)
• No automated tests for dispatch routing logic (~100 lines of bash)

[ralphbean]

[moderate] (noted, deferred) This "Prepare workspace" step is byte-for-byte identical across all 5 reusable workflows (reusable-code, reusable-fix, reusable-review, reusable-triage, reusable-retro), with a slightly different variant in prioritize.yml. If the layering logic or LAYERED_DIRS changes, all 6 must be updated in lockstep. Consider extracting to a composite action (.github/actions/prepare-workspace/action.yml) consistent with the existing mint-token, setup-gcp, and validate-enrollment actions.

[ralphbean]

[moderate] (noted, deferred) The layeredDirs list here must stay in sync with LAYERED_DIRS in all 5 reusable workflows. Currently both list agents skills schemas harness policies scripts env, but there's no automated test to catch drift. Consider adding a test that parses LAYERED_DIRS from a reusable workflow YAML and compares it against this Go slice.

[ralphbean]

[moderate] (noted, deferred) The old shim's post-run-link job ("fullsend X is working on this — view logs") is removed. This is user-visible — users will no longer get feedback that an agent was dispatched. Plan is to restore this from inside fullsend run so it uses the correct app identity token (not github-actions[bot]) and can provide a better link.

[ralphbean]

[moderate] (noted, deferred) The routing logic here (~100 lines of bash: command parsing, label matching, author association, bot filtering, fork detection, auto-triage) has no automated test coverage. Any regression silently breaks all dispatch. The old shim used GHA if: expressions which were at least visible in the workflow graph. Consider adding a shell-script test that exercises this routing with various event/input combinations.

[github-actions]

fullsend review is working on this — view logs

[ralphbean]

Review Summary

All 4 required changes from the previous review have been addressed in 1cabedaf:

• ✅ ALLOWED_WORKFLOW_FILES now fails closed (unset/empty denies all)
• ✅ Retro role reverted from fullsend to retro
• ✅ ADR 0035 env/ moved from Org-only to Upstream defaults
• ✅ customized/env/.gitkeep assertion added to scaffold test

The security posture is strong: expression injection avoidance via env: blocks, explicit secret passing (not secrets: inherit), fork PR fail-closed detection, persist-credentials: false on all target-repo checkouts in reusable workflows, and OIDC job_workflow_ref validation with a fail-closed allowlist.

2 items noted for follow-up (not blocking):

1. dispatch.yml checkout missing persist-credentials: false on minted token checkout
2. e2e verifyInstalled missing retro.yml and AGENTS.md assertions

[ralphbean]

[moderate] (noted, deferred) This checkout uses the minted OIDC token but doesn't set persist-credentials: false. The token stays in the git credential helper for the rest of the job. While dispatch.yml only runs trusted shell scripts (not LLM-driven), the reusable workflows correctly set persist-credentials: false on all their target-repo checkouts. For consistency and defense-in-depth, consider adding persist-credentials: false here too.

[ralphbean]

[moderate] (noted, deferred) This list is missing .github/workflows/retro.yml and AGENTS.md — both are in the scaffold, not in any layered or upstream-only directory, so WalkFullsendRepo will install them. Without assertions here, regressions on those files won't be caught by e2e tests.