Correctness sub-agent should verify deployment-context feasibility for infrastructure changes

[fullsend-ai-retro]

What happened

PR #58 added ~1,000 lines of pre-commit tool resolver/installer scripts to the scripts/ directory, claiming they would be found via SCRIPT_DIR-relative lookups from existing pre/post scripts. The review agent (run 28949303400) reviewed the PR twice and produced 9 code-quality findings (GHA command injection, tar path traversal, GOARCH default, pyyaml supply chain, etc.) but gave a comment (non-blocking) verdict — accepting the PR's premise that copying companion files would make SCRIPT_DIR-relative lookups work.

Two days later, human reviewer waynesun09 identified that the premise was fundamentally wrong: the fullsend CLI fetches each pre/post script individually into a content-addressed cache (/home/runner/work/.fullsend/.fullsend/.fullsend-cache/resources/sha256/.../content), so SCRIPT_DIR resolves to the cache directory, not a repo checkout. Companion scripts are never fetched alongside the entrypoint. The PR was closed without merge after 12 days.

What could go better

The correctness sub-agent's "runtime mechanism checklist" instructs it to "trace the full path from producer to consumer and verify the mechanism will function at runtime." In practice, this was interpreted as tracing mechanisms within the diff (flags, guards, API contracts). It did not extend to tracing the deployment delivery path — how the files in this PR actually reach their consumers at runtime.

For infrastructure/tooling PRs that add files intended to be consumed by a build or deployment system, the critical question is not just "is this code correct?" but "will these files be reachable at the point of consumption?" In this case, a few tool calls (reading the harness config to see how scripts are fetched, or reading the pre/post scripts to see what SCRIPT_DIR resolves to) would have revealed the cache-based execution model.

Confidence: HIGH that the gap exists — the sub-agent definition contains no instruction to verify deployment-context feasibility. MEDIUM confidence that adding such guidance would reliably catch this class of issue, since it requires the sub-agent to reason about deployment models it may not have full context for. The documented runtime isolation constraint (tracked in agents#136) would also help — if AGENTS.md described the cache-based execution model, the sub-agent would have that context available.

This is distinct from fullsend#1472 (checking implementation vs. issue-stated preferred approach — the issue here didn't specify an approach preference), fullsend#3639 (approach proportionality — the approach wasn't disproportionate, it was infeasible), and agents#299 (code agent root-cause verification — this targets the review agent).

Proposed change

Expand the correctness sub-agent's runtime mechanism checklist in skills/pr-review/sub-agents/correctness.md to include deployment-context feasibility verification. Specifically, add guidance along these lines:

After the existing "Runtime mechanism checklist" items, add:

Deployment-context feasibility: When a PR adds files that are consumed by a deployment system (CI workflows, harness scripts, build artifacts, companion scripts), verify the delivery mechanism:

• How does the consuming system locate these files at runtime? (Direct path? Content-addressed cache? Package install?)
• Will the files be present at the expected path when the consumer runs? Read the harness config or deployment workflow to confirm.
• If the PR assumes filesystem co-location (sibling scripts, relative imports), verify that the deployment model preserves directory structure rather than fetching files individually.
• If the delivery path is unclear, emit a medium-severity finding asking the author to confirm the runtime reachability of the added files.

This targets skills/pr-review/sub-agents/correctness.md in the fullsend-ai/agents repo. The change complements agents#136 (documenting the constraint in AGENTS.md) — both are needed, since the sub-agent guidance ensures the review agent actively checks feasibility even for repos where the constraint is not yet documented.

Validation criteria

1. The correctness sub-agent definition includes deployment-context feasibility guidance.
2. On the next 3 infrastructure/tooling PRs that add files consumed by deployment systems, the review agent's correctness findings show evidence of deployment-path reasoning (visible in the JSONL trace or review output).
3. Regression test: re-run the review agent against PR fix(scripts): add pre-commit tool resolver and installer #58's commit 12e84d2 and verify it produces a finding questioning whether the added scripts will be reachable from the content-addressed cache execution path.
4. No false positives on PRs that add files consumed directly from the repo checkout (e.g., source code, config files read at build time).

---

Generated by retro agent from #58

[fullsend-ai-triage]

🤖 Finished Triage · ✅ Success · Started 10:07 PM UTC · Completed 10:11 PM UTC
Commit: 17eed93 · View workflow run →

[fullsend-ai-triage]

Triage Summary

Severity: Medium · Category: Feature (new sub-agent guidance)

Problem

The correctness sub-agent's runtime mechanism checklist traces mechanisms within the diff but does not verify whether files added by a PR will be reachable at their point of consumption in the deployment system. On PR #58, ~1,000 lines of companion scripts were added assuming SCRIPT_DIR-relative co-location, but the harness fetches scripts individually into a content-addressed cache. The sub-agent accepted this premise; a human caught the fundamental infeasibility 2 days later.

Root Cause

skills/pr-review/sub-agents/correctness.md has no guidance for deployment-context feasibility verification. The checklist covers producer-to-consumer tracing, format expectations, and failure paths — but not physical file reachability in deployment systems.

Recommended Fix

Add a "Deployment-context feasibility" block after the existing runtime mechanism checklist items in correctness.md. The proposed wording in the issue body is well-suited — it covers: identifying the delivery mechanism, verifying file presence at expected paths, flagging co-location assumptions, and emitting medium-severity findings when delivery path is unclear.

Related Issues

• Review correctness sub-agent should evaluate deployment-context compatibility when methods delegate to underlying APIs #228 — Similar but distinct: covers API delegation semantics compatibility (method-level), not file delivery/reachability (infrastructure-level). Both target correctness.md but address different gap classes.
• Document post-script runtime isolation constraint in AGENTS.md #136 — Complementary: documents the runtime isolation constraint in AGENTS.md. Both are needed — Document post-script runtime isolation constraint in AGENTS.md #136 provides context, this issue ensures the sub-agent actively checks feasibility even for repos where the constraint is undocumented.

Proposed Test Case

Regression: Re-run review agent against PR #58 commit 12e84d2
  Expected: Correctness sub-agent emits a finding questioning whether
  added scripts are reachable from the content-addressed cache path.
  Actual (current): No finding about script reachability.

False-positive check: Run against PRs adding source code or config files
  consumed directly from repo checkout.
  Expected: No deployment-feasibility findings.

Forward validation: Next 3 infrastructure/tooling PRs should show
  deployment-path reasoning in correctness findings (visible in JSONL
  trace or review output).

---

Labels: This is a feature enhancement to the review agent's correctness sub-agent. Applying 'enhancement' for feature category and 'review-agent' for the affected agent.