Skip to content

proposal: auto-review primitive for skills#60

Open
JoshSalomon wants to merge 4 commits into
forge-sdlc:mainfrom
JoshSalomon:auto-review
Open

proposal: auto-review primitive for skills#60
JoshSalomon wants to merge 4 commits into
forge-sdlc:mainfrom
JoshSalomon:auto-review

Conversation

@JoshSalomon

@JoshSalomon JoshSalomon commented May 28, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

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

JoshSalomon and others added 3 commits June 1, 2026 13:02
@JoshSalomon @claude
proposal: add auto-review primitive for skills
0ede9c2 
Introduces an opt-in review.md file alongside SKILL.md that enables
automatic worker→reviewer→fix loops for any skill. Includes two
architectural approaches for maintainer discussion, with file-based
observability for near-real-time metrics and Jira visibility.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@JoshSalomon @claude
devtools: add webhook replay script
fb9e4d8 
Interactive script to send Jira webhook payloads to a local Forge
instance. Substitutes ticket IDs, and for revision/question payloads
fetches the latest comment from Jira via REST API so feedback is
always current.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@JoshSalomon @claude
devtools: fetch real issue metadata from Jira in webhook script
46956d4 
The test payloads hardcode issuetype as "Feature", which bypasses
parent routing for Epics/Tasks. The script now fetches the real
issue type, labels, summary, and status from Jira and injects them
into the payload, so child ticket events route correctly.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
eshulman2
eshulman2 reviewed Jun 11, 2026
View reviewed changes

@eshulman2 eshulman2 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 @claude
proposal: mark Approach A as selected for auto-review primitive
bdee983 
Container-internal loop with file-based observability selected for
implementation. Approach B retained for reference. Added implementation
note about final file sweep and cycle file persistence.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@JoshSalomon

JoshSalomon commented Jun 29, 2026

Copy link
Copy Markdown
Contributor Author

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

@JoshSalomon

JoshSalomon commented Jul 2, 2026

Copy link
Copy Markdown
Contributor Author

@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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@eshulman2 eshulman2 eshulman2 left review comments

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@JoshSalomon @eshulman2