proposal: auto-review primitive for skills

[JoshSalomon]

Summary

Adds a proposal for an opt-in auto-review mechanism as a new primitive for skills. Any skill can include a review.md alongside SKILL.md to enable automatic worker→reviewer→fix loops.

Decision

Approach A (container-internal loop) has been selected for implementation.

The review loop runs inside the container entrypoint. The orchestrator gets near-real-time visibility via file-based polling of per-cycle JSON files written to the shared workspace mount.

Key Design Points

• Opt-in: review.md alongside SKILL.md — no file means no review, current behavior preserved
• Any skill: works for planning skills (PRD, spec) and execution skills (implementation, CI fix)
• Separate reviewer agent: avoids confirmation bias — reviewer is a different agent than the worker
• Per-skill retry config: max_retries in review.md frontmatter, global default fallback
• File-based observability: container writes per-cycle JSON files, orchestrator polls for near-real-time Prometheus metrics and Jira comments
• Final sweep: orchestrator reads all cycle files after container exit to catch the approval verdict
• Cycle file persistence: files copied before workspace teardown

Files Changed

• proposals/auto-review-primitive.md — full proposal (Approach A accepted, Approach B retained for reference)
• proposals/README.md — index entry added

🤖 Generated with Claude Code

[eshulman2]

I agree option A is the better option, lets go with this one. please modify the proposal to reflect option A is the chosen one and that option B was considered but wasn't chosen.

[JoshSalomon]

The proposal was updated to reflect the selected approach (option A - in container loop)

[JoshSalomon]

@eshulman2
There is one thing we did not fully think about. The design suggests that the review is 'best effort' - meaning if it fails, the process continues successfully. After some thought, I am not sure this is always correct. My suggestion (amendment to the proposal):
1 - default stays as-is; if the review fails, the process continues without errors (possibly warnings somewhere).
2 - the review.md creator can specify in the file (in the front matter) that this review is blocking (something like block_on_failure: true ) and, in this case, the step will block if the review did not pass.
WDYT? If you agree, I will update this in the proposal and in the code.

[JoshSalomon]

@eshulman2 There is one thing we did not fully think about. The design suggests that the review is 'best effort' - meaning if it fails, the process continues successfully. After some thought, I am not sure this is always correct. My suggestion (amendment to the proposal): 1 - default stays as-is; if the review fails, the process continues without errors (possibly warnings somewhere). 2 - the review.md creator can specify in the file (in the front matter) that this review is blocking (something like block_on_failure: true ) and, in this case, the step will block if the review did not pass. WDYT? If you agree, I will update this in the proposal and in the code.

@eshulman2
I changed the proposal (see addendum from line 336) - now the flow gathers information when the reviewd faile (still 'REJECTED' after all the retries), and adds this information to the PR, so the PR reviewer can see if some of the reviews failed along the process.