Redesign finish-feature.sh as the closing step of a feature

README.md

@@ -74,6 +74,12 @@ A lightweight, model-agnostic repository template for agentic software engineeri
    the resulting implementation.
    Approved `DEFER` findings become linked follow-up Issues; `ACCEPT` findings
    retain their rationale in the triage artifact.
-9. Commit, push, and open a PR containing `Closes #12`. After CI and required gates pass, merge the PR and clean up the worktree.
+9. After a new review round confirms the fixes, commit with the closing checks:
+
+   ```bash
+   ./scripts/finish-feature.sh 12 "Implement player movement"
+   ```
+
+   Then push and open a PR containing `Closes #12`. After CI and required gates pass, merge the PR and clean up the worktree.
 
 See `docs/development.md` for commands, `docs/agentic-workflow.md` for the lifecycle, and `.agents/policies/` for boundaries.

docs/agentic-workflow.md

@@ -9,7 +9,7 @@ ready work.
 Roadmap item → GitHub Issue → feature plan (when warranted) → isolated
 branch/worktree → implementation → local verification → independent review
 when required → review triage → approved fix-now application →
-verification/re-review when needed → commit → push/PR → CI → human gate where
+verification/re-review when needed → `finish-feature.sh` → push/PR → CI → human gate where
 required → merge → automatic Issue closure → cleanup.
 
 Independent review happens before the implementation commit so it can include

docs/development.md

@@ -481,12 +481,42 @@ Check a review yourself with:
 
 It exits 0 when the review is current, 1 when it is stale, and 2 on an error.
 
-Verify again after review fixes, then commit and push:
+### Finishing a feature
+
+When the latest review round is resolved, finish the feature from its
+worktree:
+
+```bash
+./scripts/finish-feature.sh 12 "Implement player movement"
+```
+
+The script refuses another branch than `feature/12-*` or a tree without
+changes, runs `./scripts/verify.sh`, and checks the latest review of the
+feature: it must be current and have no critical or major finding. Every
+review round with findings needs an approved triage that was published on the
+Issue (the newest triage of that round counts), and the latest round may have
+no `FIX_NOW` findings left to apply. After fixes, run a new review round first;
+a passed newer round confirms the fixes of earlier rounds. A failed
+publication can be repeated with `./scripts/triage-review.sh --publish
+<triage-json>`.
+
+It then stages all changes (review and triage files are ignored) and opens a
+structured commit message in your editor: the summary, the Issue, a list of
+changes to fill in, the verification, the review round and verdict, and
+`Refs #12`. Emptying the message aborts the commit and leaves the changes
+staged. The script never pushes, opens a PR, or merges.
+
+A change that `.agents/policies/autonomy.md` classifies as low risk may be
+finished without an independent review; the reason is recorded in the commit
+message:
+
+```bash
+./scripts/finish-feature.sh 12 "Fix a typo in the README" --no-review "documentation only"
+```
+
+Then push and open the pull request:
 
 ```bash
-./scripts/verify.sh
-git add .
-git commit -m "Implement player movement"
 git push -u origin feature/12-player-movement
 ```
 
@@ -515,8 +545,5 @@ repository-relative plan path; the detailed plan remains in `.agents/plans/`.
 Roadmap item → GitHub Issue → optional implementation plan → isolated feature
 worktree → implementation → verification → independent review when required →
 triage → apply approved `FIX_NOW` findings → verification/re-review when needed
-→ commit → push/PR → CI and gates → merge → automatic Issue closure → worktree
-cleanup.
-
-Do not use `scripts/finish-feature.sh` as part of this flow until it has been
-redesigned or deprecated; its interface predates the current review script.
+→ `finish-feature.sh` (checks and commit) → push/PR → CI and gates → merge →
+automatic Issue closure → worktree cleanup.

scripts/finish-feature.sh

@@ -1,10 +1,163 @@
 #!/usr/bin/env bash
+
 set -euo pipefail
-./scripts/verify.sh
-if [[ -n "$(git status --porcelain)" ]]; then
-  echo "Working tree has changes. Review them before committing."
-  git status --short
-  exit 2
+
+script_dir="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd -P)"
+source "$script_dir/lib/review-data.sh"
+source "$script_dir/lib/fingerprint.sh"
+
+usage() {
+  echo "Usage: $0 <issue-number> \"<commit summary>\" [--no-review \"<reason>\"]"
+  echo
+  echo "Run in the feature worktree. Checks that verification passes and that the"
+  echo "latest independent review is current and resolved, then commits the feature"
+  echo "with an editable, structured message. It never pushes or merges."
+  echo
+  echo "--no-review is only for changes that .agents/policies/autonomy.md classifies"
+  echo "as low risk; the reason is recorded in the commit message."
+  exit 1
+}
+
+fail() {
+  echo "Error: $*" >&2
+  exit 1
+}
+
+# latest_triage <review-json>: prints the newest triage of a review. The first
+# triage has no number (-triage.json); later ones are numbered (-triage-02.json).
+latest_triage() {
+  local base
+  local number
+  local best=""
+  local best_number=0
+  local candidate
+
+  base="$root/.agents/triage/$(basename "$1" .json)"
+  [[ ! -f "$base-triage.json" ]] || { best="$base-triage.json"; best_number=1; }
+  for candidate in "$base-triage-"[0-9][0-9].json; do
+    [[ -f "$candidate" ]] || continue
+    number="${candidate%.json}"
+    number="${number##*-}"
+    if ((10#$number > best_number)); then
+      best="$candidate"
+      best_number=$((10#$number))
+    fi
+  done
+  printf '%s' "$best"
+}
+
+[[ $# -eq 2 || ( $# -eq 4 && "$3" == "--no-review" ) ]] || usage
+issue="$1"
+summary="$2"
+no_review_reason="${4:-}"
+
+[[ "$issue" =~ ^[0-9]+$ ]] || fail "issue number must be numeric: $issue"
+[[ "$summary" =~ [^[:space:]] ]] || fail "the commit summary is empty."
+[[ $# -eq 2 || "$no_review_reason" =~ [^[:space:]] ]] || fail "--no-review requires a reason."
+
+root="$(git rev-parse --show-toplevel)"
+branch="$(git branch --show-current)"
+[[ "$branch" == feature/${issue}-* ]] ||
+  fail "finish-feature.sh must run on feature/${issue}-*. Current branch: $branch"
+
+[[ -n "$(git -C "$root" status --porcelain)" ]] || fail "there are no changes to commit."
+review_data_require_jq
+
+# 1. Verification passes.
+echo "Running repository verification..."
+(cd "$root" && ./scripts/verify.sh) || fail "verification failed; nothing was committed."
+echo
+
+# 2. The latest review is current and resolved, unless explicitly skipped.
+if [[ -n "$no_review_reason" ]]; then
+  review_line="Review: no independent review (low risk: $no_review_reason)"
+else
+  slug="${branch//\//-}"
+  shopt -s nullglob
+  reviews=("$root/.agents/reviews/${slug}-review-"[0-9][0-9].json)
+  shopt -u nullglob
+  [[ "${#reviews[@]}" -gt 0 ]] ||
+    fail "no review of this feature exists. Run ./scripts/review-feature.sh $issue, or use --no-review for a low-risk change."
+  latest="${reviews[${#reviews[@]}-1]}"
+  latest_relative="${latest#"$root"/}"
+
+  errors="$(review_artifact_errors "$latest")"
+  [[ -z "$errors" ]] || fail "the latest review is invalid: ${errors//$'\n'/; }"
+  [[ "$(jq -r '.kind // "feature"' "$latest")" == "feature" && "$(jq -r '.issue' "$latest")" == "$issue" ]] ||
+    fail "$latest_relative is not a review of Issue #$issue."
+
+  tmp_work="$(mktemp -d "${TMPDIR:-/tmp}/finish-feature.XXXXXX")"
+  trap 'rm -rf "$tmp_work"' EXIT
+  current=0
+  review_is_current "$root" "$tmp_work" "$latest" || current=$?
+  case "$current" in
+    0) ;;
+    1) fail "the latest review ($latest_relative) is stale: the code changed after it. Run ./scripts/review-feature.sh $issue again." ;;
+    *) fail "could not compute the fingerprint of the working tree." ;;
+  esac
+
+  round="$(jq -r '.round' "$latest")"
+  verdict="$(jq -r '.verdict | gsub("_"; " ")' "$latest")"
+  blocking="$(jq '[.findings[] | select(.severity == "critical" or .severity == "major")] | length' "$latest")"
+  [[ "$blocking" -eq 0 ]] ||
+    fail "the latest review (round $round) has $blocking critical or major finding(s). Triage, fix, and review again."
+
+  # Every round with findings needs an approved triage that was published on
+  # the Issue; only the latest round may not have FIX_NOW findings left, since
+  # a newer, current review confirms the fixes of earlier rounds.
+  triage_note=""
+  for review in "${reviews[@]}"; do
+    [[ "$(jq '.findings | length' "$review")" -gt 0 ]] || continue
+    review_round="$(jq -r '.round' "$review")"
+    triage="$(latest_triage "$review")"
+    [[ -n "$triage" ]] ||
+      fail "the findings of round $review_round are not triaged. Run ./scripts/triage-review.sh ${review#"$root"/}."
+    triage_relative="${triage#"$root"/}"
+    errors="$(triage_artifact_errors "$triage" "$review")"
+    [[ -z "$errors" ]] || fail "the triage of round $review_round is invalid: ${errors//$'\n'/; }"
+    [[ "$(jq -r '.published_at // empty' "$triage")" != "" ]] ||
+      fail "the triage of round $review_round was not published on Issue #$issue. Run ./scripts/triage-review.sh --publish $triage_relative."
+    if [[ "$review" == "$latest" ]]; then
+      [[ "$(jq '[.decisions[] | select(.decision == "FIX_NOW")] | length' "$triage")" -eq 0 ]] ||
+        fail "round $review_round has FIX_NOW findings. Apply them with ./scripts/apply-triage.sh $triage_relative and review again."
+    fi
+    triage_note="; triage published on #$issue"
+  done
+  review_line="Review: round $round, $verdict, by $(jq -r '"\(.reviewer.agent) (\(.reviewer.model))"' "$latest")$triage_note"
 fi
-./scripts/review-feature.sh "${1:-main}"
-echo "Stable commit verified. Complete independent review before push/PR."
+
+# 3. Stage everything and commit with an editable, structured message.
+git -C "$root" add -A
+echo "Changes to commit:"
+git -C "$root" status --short
+echo
+
+message_file="$(mktemp "${TMPDIR:-/tmp}/finish-feature-message.XXXXXX")"
+trap 'rm -f "$message_file"; [[ -z "${tmp_work:-}" ]] || rm -rf "$tmp_work"' EXIT
+cat >"$message_file" <<EOF
+$summary
+
+Issue: #$issue
+
+Changes:
+- TODO: summarize the main changes
+
+Verification:
+- ./scripts/verify.sh passed
+
+$review_line
+
+Refs #$issue
+EOF
+
+if ! git -C "$root" commit --edit -F "$message_file"; then
+  echo "Error: the commit was not created. The changes remain staged." >&2
+  exit 1
+fi
+
+echo
+echo "Committed $(git -C "$root" rev-parse --short HEAD) on $branch."
+echo
+echo "Next steps:"
+echo "  git push -u origin $branch"
+echo "  Open a pull request whose description contains: Closes #$issue"

scripts/lib/review-data.sh

@@ -301,8 +301,11 @@ triage_artifact_errors() {
     if type != "object" or .schema != "triage/v1" then
       "the file is not a triage/v1 artifact"
     else
-      unexpected(["approved_at", "decisions", "issue", "review_verdict", "reviewed_tree",
+      unexpected(["approved_at", "decisions", "issue", "published_at", "review_verdict", "reviewed_tree",
                   "schema", "source_review", "triage"]; "the triage"),
+      (if .published_at == null or ((.published_at | type) == "string"
+          and (.published_at | test("^[0-9]{4}-[0-9]{2}-[0-9]{2}T[0-9]{2}:[0-9]{2}:[0-9]{2}Z$"))) then empty
+       else "published_at must be a UTC timestamp" end),
       (if (.approved_at | type) == "string"
           and (.approved_at | test("^[0-9]{4}-[0-9]{2}-[0-9]{2}T[0-9]{2}:[0-9]{2}:[0-9]{2}Z$")) then empty
        else "the triage has no valid UTC approval timestamp" end),