review-and-commit

name	review-and-commit
description	Streamlined two-phase workflow: review, then commit. Verdict gate between phases. Self-contained — execute the inlined steps directly, do NOT invoke other skills via the Skill tool.
disable-model-invocation	true

Task: Review and Commit

Overview

Two-phase command: first review current changes (QA + code review), then commit only if approved. A verdict gate between the phases ensures only approved changes get committed. Streamlined version: Phase 2 reuses diff from Phase 1, targeted doc sync, inline commit grouping.

Context

The user has completed a coding task and wants a single command to review and commit. This command inlines both workflows: 1. **Phase 1 — Review**: QA + code review, produces verdict. 2. **Phase 2 — Commit**: targeted doc sync, inline grouping, commit.

The gate logic prevents committing code that has critical issues.

Maintainer note (NOT for runtime): generated by scripts/generate-skill-composites.ts from framework/composites.yaml. Source-of-truth bookkeeping only.

Rules & Constraints

1. **No delegation**: Phase 1 and Phase 2 are FULLY INLINED below. Execute the steps directly. Do NOT invoke any review or commit skill via the Skill tool — that would re-enter without the composite's verdict gate and the workflow would silently exit after the review step. 2. **Two Phases**: Execute Phase 1 (review) fully before considering Phase 2 (commit). Never interleave. 3. **Gate Logic**: After Phase 1, check the verdict. Only **Approve** proceeds to Phase 2. **Request Changes** or **Needs Discussion** → output the review report and STOP. Do not commit. 4. **No partial commit**: If Phase 1 itself fails (errors, crashes), STOP — do not proceed to Phase 2. 5. **Transparency**: Output both review findings and commit results to the user. 6. **Session Scope**: Compare current `git status` with the git status snapshot from session start (available in system context). Files already modified/untracked at session start are outside the review and commit scope — note them but do not review or commit. Focus on changes made in the current session. If unsure which changes are yours, ask the user before staging.

Instructions

Phase 1 — Review

<step_by_step>

1. Empty Diff Guard — git diff --stat, git diff --cached --stat, git status --short. No changes → STOP. System gitStatus snapshot can be stale (hooks / parallel processes); if live status shows unauthored files clean per snapshot, ask user before staging.

2. Pre-flight Project Check

   • Pick the check/test command: AGENTS.md/CLAUDE.md declares it → manifest detection (deno.json → deno task check/test; package.json → check/lint/test script; Makefile check → make check; pyproject.toml → pytest/ruff check .; go.mod → go vet ./... && go test ./...) → else "No automated checks configured" in the report and JiT subset disables (Rule 10). Do NOT guess.
   • MUST NOT run a stack-specific command without its manifest. Any deno * creates deno.lock; npm * resolves deps; etc. Pre-flight artifacts (deno.lock, __pycache__/, node_modules/, .pytest_cache/) in the tree after verification are a bug.
   • 2a (current revision): run on working tree. Skip only if no code files changed since the last successful check in this session. On failure: report immediately as [critical] and continue review.
   • 2b (parent baseline — JiT): identify parent (unstaged/staged → HEAD; commit-range → <range-start>^). Prefer git worktree add <SCRATCH>/jit-parent-<sid> <parent-sha> (full runnable tree); use git show <parent-sha>:<path> fallback ONLY if worktree-add fails. BEFORE any JiT synthesis, run the SAME project test/check command from 2a inside the parent worktree to verify baseline is green. Fallback path OR red baseline → "JiT disabled — parent baseline unavailable/red" in Degradation Notes; review continues without the JiT subset.

3. Gather Context

   • First: resolve SRS, SDS, and tasks from AGENTS.md. If SRS or SDS exists and its current content is not already in your context — read the resolved file before proceeding. If a required role is missing, report it and continue only for review steps that do not depend on that role.
   • Create a review plan in the task management tool.
   • Collect the diff: git diff (unstaged), git diff --cached (staged), or git log --oneline <base>..HEAD + git diff <base>..HEAD for branch-based changes.
   • Untracked files: git diff does NOT show untracked files. Check git status output from step 1 — for each untracked file, read its content directly and include it in the review scope.
   • Read the original user request and the plan (task file under the resolved tasks role / task list).
   • Look for project conventions in config files (linter, formatter configs). Rely on conventions visible in the diff and surrounding code.
   • 3d (intent hints — JiT): collect intent-author hints for the JiT subset: git log -1 --pretty=%B <parent-sha>..HEAD (or commit messages of the range). Optionally gh pr view --json body IF the gh CLI is available AND the branch has a PR. If gh is missing or errors, proceed silently — PR body is a bonus.
   • 3e (intent inference — JiT): derive a list of ≤5 explicit intents for the diff in the form "the author tried to do X; invariants Y should hold". Pull from (a) the task file's DoD items, (b) commit messages from 3d, and (c) the diff hunks. If more than 5 candidates surface, merge related intents or drop the least-risky. Skip this sub-step if the JiT subset is disabled (Rule 10).

   Parallel Delegation (after gathering context):

   • Small diff shortcut: If git diff --stat shows < 50 changed lines, skip delegation — run all steps inline (overhead not justified).
   • Otherwise, delegate 2 independent tasks in parallel (via subagents, background tasks, or IDE-specific parallel execution — e.g., Task, Agent, parallel):
     • SA1: If pre-flight check (step 2a) already ran, skip SA1. Otherwise, run the project check command chosen via the same manifest-detection rule from step 2 (MUST NOT run stack-specific commands without the corresponding manifest). Delegate to a console/shell-capable agent (e.g., console-expert). Return pass/fail + full output.
     • SA2: Run hygiene grep scan on diff output — search for TODO, FIXME, HACK, XXX, console.log, temp_*, *.tmp, *.bak, hardcoded secrets patterns. Delegate to a console/shell-capable agent. Return findings list.
   • Fallback rule: If any delegated task fails or times out, the main agent performs that step inline. No hard dependency on delegation success.
   • Continue with steps 4, 6, 7, 8 (main agent review) while delegated tasks run.

4. QA: Task Completion

   • Map each requirement/plan item to concrete changes in the diff.
   • Flag requirements with no corresponding changes as [critical] Missing.
   • Flag plan items marked "done" but not present in diff as [critical] Phantom completion.
   • Check for regressions: do changed files break existing functionality?

4a. FR Coverage Audit (blocking gate — see Requirements Lifecycle in AGENTS.md)

• FRs in scope: (a) FR-* in the task file's implements:; (b) FR sections added/modified in the diff to SRS; (c) [REF:fr:<id>] SALP markers touched in the diff.
• Per FR: (1) SRS has **Acceptance:** with a runnable ref (test path::name, benchmark id, command, or manual — <reviewer>); missing/placeholder → [critical] no acceptance reference. (2) Run the evidence command (or deno run -A scripts/check-fr-coverage.ts FR-<ID>); non-zero / failing / manual without reviewer → [critical] acceptance fails. (3) FR claimed implemented but no [REF:fr:<id>] marker in changed source → [critical] missing code marker. (4) DoD [x] with no evidence run/cached pass → [critical] Phantom completion.
• Gate: blocking — verdict cannot be Approve while any FR-gate issue remains.

5. QA: Hygiene (use SA2 result if available; else inline)

   • SA2 done → dedupe its findings with own Code Review findings and merge.
   • SA2 failed/timed out or skipped (small diff) → perform inline:
   • Temp artifacts: New temp_*, *.tmp, *.bak, debug console.log/ print statements, hardcoded secrets or localhost URLs.
   • Unfinished markers: New TODO, FIXME, HACK, XXX introduced in this diff (distinguish from pre-existing ones).
   • Dead code: Commented-out blocks, unused imports/variables/functions added in this diff.
   • Deleted directories: If the diff deletes an entire skill, agent, or module directory (not just individual files), flag as [warning] Entire directory deleted — confirm intentional and ask the user to verify before proceeding.

6. Code Review: Design & Architecture

   • Responsibility: Does each changed file/module stay within its stated responsibility? Flag scope creep.
   • Coupling: Are new dependencies (imports, API calls) justified? Flag tight coupling or circular dependencies.
   • Abstraction: Is the level of abstraction appropriate? Flag over-engineering (unnecessary interfaces, premature generalization) and under-engineering (god-functions, duplicated logic).
   • Risk hypotheses (JiT side-channel): while reading each hunk, also accumulate ≤3 risk hypotheses per intent (from Step 3e) in the form "if the author, while trying to do X, had slipped on Y, the code would now fail at Z". Risks MUST be diff-specific — not generic code smells ("null deref", "unhandled exception") unless the diff directly exposes that risk. Skip this side-channel if the JiT subset is disabled.

7. Code Review: Implementation Quality

   • Naming: Are new identifiers (vars, funcs, types) clear and consistent with project conventions?
   • Error handling: Are errors handled explicitly? Flag swallowed exceptions, missing error paths, generic catch-all handlers.
   • Edge cases: Are boundary conditions (null, empty, overflow, concurrent access) handled?
   • Types & contracts: Are type signatures precise? Flag any, untyped parameters, missing return types (where project conventions require them).
   • Tests: Do new/changed behaviors have corresponding tests? Are existing tests updated for changed behavior?
   • Risk hypotheses (JiT side-channel): continue accumulating risks started in Step 6 (see Step 8a for the mutation taxonomy).

8. Code Review: Readability & Style

   • Consistency: Do changes follow the project's established patterns (file structure, naming, formatting)?
   • Comments: Are non-obvious decisions explained? Flag misleading or stale comments.
   • Complexity: Flag functions > 40 lines or cyclomatic complexity spikes introduced in this diff.
   • Clarity: Flag clarity sacrificed for brevity — nested ternaries, dense one-liners, overly compact expressions. Explicit code is preferred over clever short forms.

8a. Mutant + Catching Test Synthesis (JiT) (skip on pure-deletion diff or JiT-disabled flag)

• Generate ≤15 mutants total (≤5 intents × ≤3 risks × 1 mutant per risk), each modelling a concrete diff-specific failure mode. Typical mutations: comparator flip, removed guard, inverted return, off-by-one, swapped args, skipped branch.
• For each mutant, synthesize ONE ephemeral test that:
  1. Compiles / parses in the project's test language.
  2. Passes on the parent revision.
  3. Kills the mutant (fails when the mutation is applied to the diff-side code; passes on the current diff code if and only if the current code preserves the parent behaviour).
• Write tests to the session-id'd scratch directory (Rule 11). Never colocate next to the file under test in the main test tree.

8b. Dual-Run + Filter (JiT) (skip if Step 8a skipped)

• (a) parent: run the generated tests against the parent worktree from Step 2b. Any test that FAILS on the parent is an assumption leak — discard it.
• (b) diff: run the surviving tests against the diff revision. Any test that FAILS here is a Catching JiTTest — record it for the final report with file:line and the assertion output.
• (c) mutant kill-rate (optional): apply each mutant patch to the diff tree, re-run the matching test, record whether the mutant is killed. SKIP this sub-stage if a single invocation of the project's test command on the smallest scope exceeds 30 s — explicitly write Mutant kill-rate skipped — single test invocation exceeded 30 s threshold (recorded N s) in Degradation Notes so the lost signal is visible (not just an absent section).
• Filter ensemble, in order:
  1. Flaky — rerun each surviving test 3 times; if the result flips, discard.
  2. Assertion duplicates — two tests asserting the same thing on the same input.
  3. Zero-kill — passed on parent, passed on diff, killed no mutant.

9. Run Automated Checks (collect from step 2 and/or SA1)

   • Pre-flight 2a ran → use its result, do NOT re-run. SA1 broader check → merge.
   • Neither ran (no check command) → note "No automated checks configured" in the report; do not silently skip.

10. Final Report — verdict on first line. Include JiT sections (Intents, Catching Tests, Uncovered Risks, Degradation Notes) only when the JiT subset ran (or was disabled — Degradation Notes then explains why). Section order:

## Review: [Approve | Request Changes | Needs Discussion]
### Verdict (plain language) — 2–4 sentences a non-coder acts on: task complete? design sound? key risks? next step? Accept WITHOUT reading the diff. MUST come first.
### Intents (≤5)
### QA Findings — [severity] file:line — description
### Code Review Findings — [severity] file:line — description
### Catching Tests (pass on parent, fail on diff) — name, intent #, mutant killed?, failure, file:line
### Uncovered Risks — risk + reason no test (non-deterministic / I/O / etc.)
### Automated Checks — [pass|fail|skipped] command — summary
### Degradation Notes — which JiT step was skipped and why
### Summary — requirements X/Y; catching tests N; critical/warning/nit counts
### Diff (optional) — offer diff/details for optional inspection; verdict stands without it; never block (Model B). MUST close the report.

≥1 surviving catching test → verdict = Request Changes regardless of other findings. Rank findings top-5 by severity × uniqueness. No issues AND zero catching tests → "Changes look good. All requirements covered, no issues found, no behavioural regressions detected." (last clause only when JiT actually ran).

11. Ephemeral Dispose (JiT) (skip when no catching tests exist) — prompt: save <name> / save all / discard all. On save: propose destination beside file-under-test, confirm, git mv, stage. On discard all (default for timeout/ambiguous): delete entire scratch directory, leave no stray files.

</step_by_step>

Verdict Gate

After completing the review report above:

• Approve → DO NOT commit yet. Phase 2 below is MANDATORY: re-plan the todo list with Phase 2 steps and execute all of them in order. Committing before reaching Phase 2 step 6 (Reflect) is a workflow violation.
• Request Changes or Needs Discussion → output the full report and STOP. Do NOT commit.
• Phase 1 crashed or produced no verdict → report the error and STOP.

Phase 2 — Commit

<step_by_step>

1. Verify Unchanged State
   • The diff and file list are already in context from the prior phase. Do NOT re-read them.
   • Run only git status -s to confirm nothing changed between phases.
   • If new changes appeared (unexpected), report and STOP.
2. Documentation Sync (mandatory — do NOT skip)
   • Determine scope: look at the file paths from step 1. Classify the change:
     • Infra-only: ALL changed files are tests (*_test.*, *.test.*), CI (.github/), acceptance tests (acceptance-tests/), formatting, or dev-environment (.devcontainer/). → Skip doc sync. Output: Documentation sync: skipped — infra-only changes (tests/CI/acceptance-tests).
     • Product changes: anything else → proceed with doc sync below.
   • Find the mapping: check if ./AGENTS.md has a ## Documentation Map section. If yes → use the path→document mapping from there. If no → use the default mapping:
     • New/changed exported functions, classes, types → SDS (component section)
     • New feature, CLI command, skill, agent → SRS (new FR) + SDS (new component section)
     • Removed feature/component → remove from SRS + SDS
     • Changed behavior (fix that alters documented contract) → SDS (update description)
     • Renamed/moved modules → SDS (update paths and structure)
     • Config/build changes → SDS only if architecture section references them
     • README.md → update only for user-facing changes (new install steps, new features, changed API)
   • Sync each affected document:
     • For each changed file, identify which document section describes its component (using the mapping).
     • READ that specific section from the document.
     • COMPARE the section text with the actual code after your changes. Ask: "Does this section accurately describe the code as it is NOW?"
     • If inaccurate → update the section. If accurate → no change needed.
     • For new functionality with no corresponding section → add a new section.
     • For removed functionality → remove the section.
   • Gather change context for commit message and doc updates:
     1. Active task file: If the user referenced a task file in this session, resolve tasks from AGENTS.md and read that file there. Do NOT scan all task files.
     2. Session context: User messages explaining intent, decisions, requirements.
   • Apply Compression Rules to any doc updates:
     • Use combined extractive + abstractive summarization (preserve all facts, minimize words).
     • Compact formats: lists, YAML, Mermaid diagrams.
     • Concise language, abbreviations after first mention.
   • Execute Updates: Edit documents BEFORE proceeding to grouping.
3. Commit Grouping
   • Review the diff from step 1. Determine the primary business purpose.
   • Default: ALL changes → 1 commit. Only split if: a. Changes serve genuinely different, unrelated purposes (no causal link), OR b. The user explicitly requested a split.
   • Documentation describing a code change → same commit as that code.
   • Tests for a feature → same commit as that feature.
   • If splitting: use appropriate Conventional Commits types for each group.
   • Hunk-level splitting (within a single file) — ONLY when user explicitly requests it.
4. Commit Execution Loop
   • Iterate through the planned groups:
     1. Stage specific files for the group.
     2. Verify the staged content matches the group's intent.
     3. Task Status Lifecycle (FR-DOC-TASK-LIFECYCLE) — for each staged task file under the resolved tasks role with date: frontmatter (skip legacy flat-path), first check frontmatter status:. If it is superseded, require/keep superseded_by: and skip DoD derivation because the stale original DoD no longer maps to current reality. Otherwise count top-level - [ ]/- [x] items in ## Definition of Done. Derive status: K=0→"to do", 0<K<N→"in progress", K=N→"done" (warn if no DoD). Rewrite frontmatter and git add if it differs. Idempotent. Never downgrade done. Warn-only on parse errors.
     4. Commit with a Conventional Commits message (including any task-status frontmatter edit).
5. Task file Cleanup (only if a task file was used in step 2)
   • New-shape tasks (task files under the resolved tasks role with date: frontmatter): NEVER delete — persistent canonical records. Status auto-flip in step 4.3 is the only lifecycle action for non-superseded tasks; status: superseded records are preserved.
   • Legacy tasks (flat path, no date: frontmatter): if all DoD items satisfied → git rm and commit; if any unsatisfied → ask user "Delete or keep?"; if no DoD → ask user.
6. Session Complexity Check → Auto-Invoke Reflect
   • After all commits are done, analyze the current conversation for complexity signals:
     • Errors or failed attempts occurred (test failures, lint errors, build errors).
     • Agent retried the same action multiple times.
     • User corrected the agent's approach or output.
     • Workarounds or non-obvious solutions were applied.
   • Also check the user's invocation message for explicit complexity descriptors: phrases like "rough session", "had to retry", "wrong approach", "failed", "had to correct you". These count as direct signals.
   • If any of these signals are detected: a. Announce briefly which signals fired (one line, e.g., "Detected retries and user correction — running /flowai:reflect"). b. Pre-command signal check: if the signals appear only in the invocation message (i.e., the problematic interactions predated this command and are not visible in the conversation history), output: "You mentioned a rough session — briefly describe what went wrong and what you corrected. This will be included as reflect context." Use the user's answer as additional context when invoking reflect. c. Invoke the reflect skill directly (via the Skill tool, native slash-command execution, or inline execution of its SKILL.md instructions — whichever the host IDE supports). d. Do NOT ask the user for confirmation before invoking; proceed autonomously (the context question in step b is not a confirmation request — it gathers missing information).
   • If none detected, skip silently.
7. Post-Reflect Cleanup Commit (skip if reflect produced no edits)
   • Run git status. If reflect left working-tree edits (typically AGENTS.md, **/CLAUDE.md, framework/**, .claude/**, documents/**): stage them and commit as agent: apply reflect-suggested improvements (or narrower scope, e.g. agent(commit): tighten doc-audit gate). Do NOT amend earlier commits — keep reflect-driven edits as a separate commit. If git status is clean, skip.
8. Verify Clean State
   • Run git status to confirm all changes are committed.
   • If uncommitted changes remain, investigate and report to the user. </step_by_step>

Final Combined Report

Output a combined summary:

• Review: verdict + key findings (or "no issues found")
• Commit: files committed, commit message(s)

Verification

[ ] Empty diff guard checked before starting. [ ] Pre-flight project check executed (or skipped — no code changes since last check). [ ] Review phase completed with structured report. [ ] Verdict gate enforced: only Approve proceeds to commit. [ ] Documentation sync performed: affected sections updated or justified skip. [ ] Changes grouped by logical purpose. [ ] Commits executed with Conventional Commits format. [ ] Task lifecycle: every staged non-superseded new-shape task had `status:` auto-derived from DoD checkboxes (`to do | in progress | done`) and rewritten if it differed. `status: superseded` records were preserved and checked for `superseded_by:`. Never downgrades `done`. Warn-only on parse errors. [ ] Task file cleanup: legacy flat-path tasks — completed deleted, partial confirmed with user. New-shape tasks NEVER deleted. [ ] Session complexity check performed; `/flowai:reflect` auto-invoked if signals detected. [ ] Post-reflect cleanup commit created when reflect left uncommitted edits to project instructions; otherwise skipped. [ ] Both review and commit results reported to user.