fix(#297): add schema constraint rejecting approve with protected-path findings

[fullsend-ai-coder]

Summary

• Add a JSON Schema constraint (allOf if/then/not rule) in review-result.schema.json that rejects action: "approve" when any finding has category: "protected-path". This provides mechanical defense-in-depth — the harness validation now catches the violation before the post-script runs.
• Strengthen the protected-path section in skills/pr-review/SKILL.md to explicitly require a structured finding object (not just narrative text) and reference the schema enforcement. The agent now sees that omitting the finding is the only way an approval can slip through.
• Add 4 test cases in validate-output-schema-test.sh covering the new constraint: approve+protected-path rejected, comment+protected-path valid, request-changes+protected-path valid, approve without protected-path findings valid.

Context

The review agent detected protected-path modifications and generated narrative text saying it cannot approve, but still emitted action: "approve" in the structured output (observed on PR #195 and PR #37). The SKILL.md had rules against this, but they were narrative-only — no structured mechanism enforced them. This fix adds mechanical enforcement at the schema validation layer.

Test plan

• Schema validation test: action=approve + category=protected-path finding → rejected
• Schema validation test: action=comment + category=protected-path finding → valid
• Schema validation test: action=request-changes + category=protected-path finding → valid
• Schema validation test: action=approve + non-protected-path findings → valid
• All pre-existing tests continue to pass (10 pre-existing failures are infrastructure-related, not caused by this change)

---

Closes #297

Post-script verification

• Branch is not main/master (agent/297-enforce-protected-path-verdict)
• Secret scan passed (gitleaks — c65eba2a790ebfff571a2a00a06fd1ec3524e934..HEAD)
• PR body secret scan passed (gitleaks — no-git)
• Pre-commit hooks passed (authoritative run on runner)
• Tests ran inside sandbox

[rh-hemartin]

/fs-review

[fullsend-ai-review]

🤖 Finished Review · ✅ Success · Started 6:41 AM UTC · Completed 6:58 AM UTC
Commit: 7ada4e0 · View workflow run →

[fullsend-ai-review]

Review

Findings

Medium

• [protected-path] scripts/validate-output-schema-test.sh, skills/pr-review/SKILL.md — PR modifies files under protected paths (scripts/ and skills/). Human approval is always required for protected-path changes, regardless of context. The PR links to issue Review agent outputs approve despite detecting protected-path constraint #297 and explains the rationale for modifying these governance/infrastructure files.

Low

• [stale-enforcement-location] agents/code.md:76 — States "Protected paths are defined in post-review.sh" without noting that the schema now also enforces the approval constraint. The text is factually accurate (post-review.sh does define the path list) but incomplete regarding the enforcement mechanism.
• [stale-enforcement-location] agents/fix.md:105 — States "Protected-path enforcement lives in post-review.sh" without noting schema enforcement. Factually accurate but could note dual enforcement for completeness.

---

Labels: PR modifies review agent schema enforcement and skill documentation

[fullsend-ai-retro]

🤖 Finished Retro · ✅ Success · Started 7:23 AM UTC · Completed 7:35 AM UTC
Commit: 1c7b0a1 · View workflow run →

[fullsend-ai-retro]

Retro: PR #303 — Schema constraint for protected-path approval enforcement

Timeline

1. 2026-07-20 11:50 — Issue #297 filed by retro agent after analyzing PR fix(#193): apply outcome labels even when review submission fails #195, where the review agent detected protected paths but still emitted action: "approve".
2. 2026-07-20 11:51–11:56 — Triage agent ran, classified as HIGH severity governance bypass, labeled ready-to-code.
3. 2026-07-20 11:57–12:04 — Code agent ran, produced a clean single-commit fix: JSON Schema allOf constraint + SKILL.md update + 4 test cases (+57/−5 lines across 3 files).
4. 2026-07-20 12:04 — PR fix(#297): add schema constraint rejecting approve with protected-path findings #303 created. No automatic review dispatch triggered.
5. 2026-07-22 06:39 — Human (rh-hemartin) approved the PR, then triggered /fs-review 8 seconds later.
6. 2026-07-22 06:41–06:58 — Review agent ran (17 min, $4.08, 4 sub-agents: correctness/opus, intent-coherence/sonnet, style-conventions/sonnet, docs-currency/sonnet). Findings: 1 medium (protected-path), 2 low (stale enforcement references). Action: comment (correct for protected-path PRs).
7. 2026-07-22 07:20 — PR merged by rh-hemartin.

Assessment

This was a well-executed end-to-end autonomous workflow. The retro agent identified a real governance bug on PR #195, the triage agent classified it correctly with a detailed root-cause hypothesis, and the code agent produced a clean fix in a single attempt — zero rework. The review agent verified correctness (zero findings from the opus correctness sub-agent) and correctly applied the comment action given protected-path modifications.

The code agent's scope was appropriately limited to the 3 files the issue called for. The review agent's docs-currency sub-agent found two low-severity references in agents/code.md:76 and agents/fix.md:105 that describe enforcement as living in post-review.sh without noting the new schema enforcement. These references are factually correct (the post-script still enforces the downgrade) but incomplete — a reasonable observation that didn't warrant blocking.

Evidence supporting existing issues

• #351 (label mismatch ready-to-review vs ready-for-review): The pre-review script logged: "First automated review dispatch but PR already has 1 human review(s). The automated review may have missed the initial PR creation window." This confirms no automatic review was dispatched when the PR was created on July 20, causing a 2-day gap. The label mismatch tracked in Rename ready-to-review label to ready-for-review to match dispatch routing #351 is a plausible root cause — if the post-code script applied a label the dispatch routing doesn't recognize, the labeled event would not trigger review dispatch. The opened event on pull_request_target should have fired, but the dispatch routing may filter based on labels rather than just event type.

• #304 (ready-for-merge on human approval): The human approved, the review agent flagged protected paths, and the human had to manually merge. If the post-review script recognized the existing human approval as satisfying the protected-path requirement, it could have auto-applied a merge-ready signal.

Autonomy signal

The review agent's correctness sub-agent (opus) verified the JSON Schema allOf/not/contains logic was correct across all edge cases with zero false positives — a positive signal for review autonomy on schema and test changes. The style-conventions sub-agent's 3 initial findings were all correctly removed by the challenger as false positives. Since the PR touches protected paths (scripts/, skills/), human review is mandatory by design.

No new proposals — existing issues #351 and #304 cover the identified improvement opportunities.