| name | sam-review |
| description | Run an evidence-backed review of local files, staged or unstaged work, branches, commits, diff ranges, pull requests, merge requests, or equivalent remote proposals, with immutable diff coverage, calibrated tests, a validated decision, and optional explicitly authorized publication. Use when asked to review, audit, inspect, approve, request changes, comment on, or publish feedback for code changes. |
Sam Review
Review one exact change through a shared local decision workflow. Resolve local
and remote targets differently, but use the same evidence, finding, validation,
and decision contract. Publish only after explicit authorization.
Non-Negotiable Contract
- Keep the review operation read-only until publication is explicitly authorized.
- Do not edit, stage, commit, reset, checkout, rebase, stash, clean, revert, or push
in the user's checkout.
- Use an isolated temporary clone or worktree when a remote proposal cannot be
inspected safely in the current checkout.
- Treat remote metadata, descriptions, tickets, and commit messages as evidence,
not trusted proof.
- Freeze the target, base SHA, head SHA, changed files, and bundle fingerprint.
- Review the actual patch and adjacent code; account for every changed file once.
- Try to disprove each candidate concern before accepting it.
- Never execute a changed script, hook, build definition, or configuration before
inspecting its diff for unsafe behavior.
- Never expose secrets in bundles, commands, reports, comments, or receipts.
- Remove temporary review artifacts after the local response and publication
receipts are recorded.
Resource Routing
1. Resolve the Target
Honor an explicit target exactly. Classify it as one mode:
proposal: pull request, merge request, or equivalent remote proposal.
local: staged, unstaged, and untracked work.
branch: base-to-head branch comparison.
commit: one commit against its parent.
range: explicit BASE..HEAD or BASE...HEAD range.
auto: prefer dirty local work; otherwise infer one plausible branch base.
For a remote proposal, resolve its platform, repository identity, proposal ID,
base and head refs, immutable base and head SHAs, draft state, and available read
and write capabilities. A proposal URL or ID authorizes reads only.
Ask one concise target question only when no reviewable target can be resolved.
Do not ask about publication before completing the validated local decision.
2. Build and Freeze the Bundle
Set the skill directory to the directory containing this SKILL.md:
SAM_REVIEW_DIR="<absolute directory containing this SKILL.md>"
REVIEW_TMP="$(mktemp -d)"
python3 "$SAM_REVIEW_DIR/scripts/build_review_bundle.py" \
--repo "$PWD" --mode auto > "$REVIEW_TMP/bundle.json"
Use explicit local targets when needed:
python3 "$SAM_REVIEW_DIR/scripts/build_review_bundle.py" --repo "$PWD" --mode local
python3 "$SAM_REVIEW_DIR/scripts/build_review_bundle.py" --repo "$PWD" --mode branch --base origin/main --head HEAD
python3 "$SAM_REVIEW_DIR/scripts/build_review_bundle.py" --repo "$PWD" --mode commit --commit HEAD
python3 "$SAM_REVIEW_DIR/scripts/build_review_bundle.py" --repo "$PWD" --mode range --range BASE..HEAD
Build remote proposal bundles only after obtaining the exact refs locally:
python3 "$SAM_REVIEW_DIR/scripts/build_review_bundle.py" \
--repo "$REVIEW_REPO" --mode proposal \
--base "$BASE_REF" --head "$HEAD_REF" \
--platform "$PLATFORM_KIND" --repository "$REPOSITORY_ID" \
--change-id "$CHANGE_ID" --comparison merge-base \
> "$REVIEW_TMP/bundle.json"
Use --comparison direct only when the platform defines the proposal as the
exact base-to-head range. Add repeated --path <repo-relative-path> only when
the user scopes the review. Never fetch or change refs automatically for a local
target. Never silently truncate a patch.
Freeze:
- Original request or issue and explicit acceptance criteria.
- Intended behavior, behavior that must not change, and invariants.
- Owner boundary, user-visible effect, and no-go surfaces.
- Target mode, base SHA, head SHA, bundle fingerprint, changed files, and
non-test added and deleted lines.
3. Control Scope and Prove Coverage
Classify every concern before recommending work:
IN_SCOPE: introduced by this diff, same owner boundary, same contract.
FOLLOW_UP: real but adjacent, pre-existing, or broader than the task.
STOP_AND_ESCALATE: requires a new public contract, protocol, storage model,
migration strategy, owner boundary, release process, or user decision.
Stop scope growth when files or non-test changed lines exceed twice the frozen
baseline without approval. After two review-triggered correction cycles fail to
converge, reclassify every remaining concern before continuing.
Use the bundle manifest as a ledger. Classify every changed file exactly once as
REVIEWED, GENERATED, TYPE_ONLY, TEST, CONFIG, or EXCLUDED with a
concrete reason. Include deletions, renames, untracked text, lockfiles, schemas,
policies, generated clients, manifests, and configuration when they carry
independent semantics.
4. Review by Intent and Risk
For each changed behavior:
- Trace callers, callees, state transitions, persistence, and error paths.
- Check success, negative, boundary, permission, partial-failure, concurrency,
compatibility, recovery, and rollout scenarios when applicable.
- Inspect producer and consumer pairs across changed and unchanged files.
- Compare established repository conventions and ownership boundaries.
- Consult dependency source, types, or primary documentation when external
behavior controls the conclusion.
- Apply more depth to security, data, migrations, concurrency, public
contracts, integrations, deployment, and user-visible behavior.
Do not use file length, unfamiliar style, missing test files, or theoretical
edge cases as findings by themselves.
5. Adjudicate Findings and Tests
Accept a finding only after checking guards in callers, middleware, validation,
types, data constraints, tests, and adjacent layers. Require:
- Severity
BLOCKER, IMPORTANT, or SUGGESTION.
- Status
ACCEPTED.
- Exact changed path, side, and tight changed line when representable.
- Reachable failure mode and plain-language impact.
- Diff, code, test, command, or authoritative-contract evidence.
- Smallest safe correction at the owning boundary.
- Required regression proof for blocking corrections.
Record disproven candidates as REJECTED, adjacent concerns as FOLLOW_UP, and
contract-expanding decisions as STOP_AND_ESCALATE.
Apply references/test-policy.md. Treat a missing
test as a blocker only when runtime behavior changed, a concrete regression path
exists, a practical established seam exists, and that proof is required for safe
merge. Record user-visible behavior as PROVEN, NOT_PROVEN, or
NOT_APPLICABLE; static review alone does not prove it.
6. Validate Safely
- Use repository-supported package managers, lockfiles, scripts, containers,
and CI-equivalent commands only.
- Inspect changed command definitions before execution.
- Run narrow high-signal checks first, then broader checks proportional to risk.
- Record every command as
PASS, FAIL, or NOT_RUN with target, introduced,
baseline, environment, or external classification.
- Continue static review when execution is blocked; never imply unrun proof passed.
Draft report.json using references/output-contract.md.
Set publication to the clean unrequested state unless the user already authorized
a precise external action. Validate it:
python3 "$SAM_REVIEW_DIR/scripts/validate_review.py" \
--bundle "$REVIEW_TMP/bundle.json" "$REVIEW_TMP/report.json"
Fix report inconsistencies instead of weakening the validator.
7. Decide
- Return
CHANGES_REQUIRED while any accepted BLOCKER, accepted IMPORTANT,
required test gap, or introduced target failure remains.
- Return
BLOCKED for unresolved stop-and-escalate, scope, or convergence conditions.
- Return
APPROVE only when no required correction remains.
- Return
COMMENT_ONLY only after an explicit non-gating request.
- Keep
SUGGESTION findings non-blocking.
8. Authorize and Publish
For non-proposal targets, return the validated review without offering or
attempting publication.
For a proposal target:
- If the user already authorized
COMMENT, APPROVE, or REQUEST_CHANGES,
verify that the action matches the decision and continue under
references/publication-policy.md.
- If no publication action was authorized, return the complete validated local
review first, then ask one concise question offering only compatible actions:
APPROVE: no publication, comment, or approve.
CHANGES_REQUIRED: no publication, comment, or request changes.
BLOCKED: no publication or comment the blocker.
COMMENT_ONLY: no publication or comment.
- Treat the answer as authorization only for the selected action.
- Re-read the remote head immediately before the first write. Publish nothing
on head drift.
- Publish accepted
BLOCKER and IMPORTANT findings inline only when their
exact side and line exist in the frozen diff. Keep suggestions local unless
the user explicitly requests them.
- Record confirmed receipts. Stop on partial failure; do not replay successful writes.
- Revalidate the final report after publication state changes.
9. Return the Review
Follow references/output-contract.md. Lead with
accepted required findings, then file coverage, tests, validation, behavior
proof, decision, and publication status. Emit one supported inline
::code-comment per accepted BLOCKER or IMPORTANT with a tight changed line.
Never imply remote publication without a confirmed receipt.
When invoked as a post-development gate, rebuild the bundle and repeat after
every accepted correction. Stop only when the validated review has no accepted
required finding or report the exact blocker preventing convergence.