feat(session): pause for user review before shipping the final unit

plugins-claude/session/.claude-plugin/plugin.json

@@ -1,6 +1,6 @@
 {
   "name": "session",
-  "version": "4.10.1",
+  "version": "4.11.0",
   "dependencies": [
     "git-tools"
   ],

plugins-claude/session/README.md

@@ -72,7 +72,7 @@ same either way.
 | `/session:session-issue` | Rank open issues, pick one or more, then explore and plan |
 | `/session:session-orchestrate` | Multi-agent feature workflow: spec → plan → refine → divide → execute → review |
 | `/session:session-summarize` | Summarize the current repo state (also auto-triggers) |
-| `/session:session-end` | Update docs, finish adjacent work, review + fix (hard gate, skipped only if already reviewed this session), sweep issues, ship the final unit via `/git-tools:ship`, then confirm the whole session is complete |
+| `/session:session-end` | Update docs, finish adjacent work, review + fix (hard gate, skipped only if already reviewed this session), sweep issues, summarize the work and caveats and wait for your go, then ship the final unit via `/git-tools:ship` (CI, fix, re-merge) and give a final-state summary |
 
 ## Skills (Model-Triggered)
 
@@ -92,7 +92,10 @@ Copilot variants (`plugins-copilot/session/commands/`) stay user-invoked only.
 the scope (earlier units are landed by the begin-work flow via `ship`). Before handing off it updates
 docs, finishes adjacent work, runs a code review + fix pass (a hard gate: skipped only if a review already ran this
 session on the current state of the work, never offered as a skip), and sweeps open issues so
-resolved ones get `Closes #N` / `Fixes #N` in the PR. `ship` does the rest (commit, PR,
+resolved ones get `Closes #N` / `Fixes #N` in the PR. The review scales with the change, a triage agent
+judges whether findings are real before anything is fixed, and it stops after three review/fix passes to ask you
+if significant findings remain. It then stops, before anything is committed or pushed, to
+summarize the work and any caveats and wait for your go via `AskUserQuestion` (so you can answer from `/rc`); earlier units of a multi-PR scope never stop. `ship` does the rest (commit, PR,
 CI, merge, worktree teardown — step 9 of `ship`). Use `/git-tools:ship` directly when you want only the
 lifecycle with no pre-flight.
 
@@ -122,7 +125,9 @@ with `bash test.sh`.
   → isolates in a worktree (by default), explores, enters plan mode
   ... implement (left uncommitted) ...
 /code-review high --fix                     # optional
-/session:session-end                        # docs, review + fix, close issues, ship
+/session:session-end                        # docs, review + fix, close issues, then STOP:
+                                            # summary + caveats, wait for your go; then
+                                            # ship, CI, re-merge if needed, final summary
 ```
 
 ## Branch Type Detection

plugins-claude/session/reference/spine.md

@@ -19,10 +19,14 @@ invoke it; never raise it unprompted.
 > built from real code exploration.
 >
 > **EQUALLY CRITICAL — the other end of the flow**: plan approval authorizes the
-> whole stated scope through merge. Implement each unit; land every unit before the
-> last via `/git-tools:ship` and move on without stopping, then run the last unit
-> through `/session:session-end`, which ships it (Phase 4). The one deliberate stop
-> is at the end: confirm with the user before the entire session is called complete.
+> whole stated scope, but the last unit is reviewed by the user before it ships.
+> Implement each unit; land every unit before the last via `/git-tools:ship` and move
+> on without stopping, then run the last unit through `/session:session-end`
+> (Phase 4). The one deliberate stop is **before** that final ship: `session-end`
+> summarizes the work and caveats and waits for the user's go. After the go it ships,
+> watches CI, fixes and re-merges if needed, cleans up, and ends with a final-state
+> summary. A one-PR session always pauses before its PR; a multi-PR scope pauses once,
+> before the last PR.
 
 ## Inputs (supplied by the calling door)
 
@@ -272,12 +276,13 @@ For trivial work keep the whole plan short — a few lines, not a document. Do *
 otherwise describe the commit / push / PR / merge steps in the plan; Phase 4 defines
 them.
 
-## Phase 4 — Report, continue the scope, confirm at completion
+## Phase 4 — Report, continue the scope, pause before the final ship
 
-> Plan approval authorizes the **entire** `Scope:` through merge. Do **not** stop
-> between units, and do not ask before commit / push / PR / merge. The only
-> deliberate stop is confirming with the user before the whole session is called
-> complete.
+> Plan approval authorizes every unit before the last through merge: do **not** stop
+> between those units or ask before their commit / push / PR / merge. The last (or
+> only) unit is different: do not commit, push, or open its PR until the user has
+> reviewed `session-end`'s summary and given the go. That is the only deliberate
+> stop.
 
 After each unit is implemented:
 
@@ -286,10 +291,11 @@ After each unit is implemented:
    run), and any caveats — known or potential problems and assumptions, stated
    specifically. Omit caveats only if there are genuinely none.
 2. **If this is the last unit** (a single-unit scope is just this), do **not** ship
-   it here: go to `/session:session-end`, which does docs, review, issue sweep,
-   ships the unit once, and ends with the one stop: **stop and confirm with the
-   user before we call this entire session complete** (issue closing, cleanup /
-   worktree teardown, final summary).
+   it here: go to `/session:session-end`, which does docs, review, and the issue
+   sweep, then **stops before shipping** to summarize what was done and any
+   caveats and wait for the user's go. After the go it ships the unit once, watches
+   CI, fixes and re-merges if needed, closes out (issue closing, cleanup /
+   worktree teardown), and finishes with a final-state summary.
 3. Otherwise (a unit before the last), optionally run
    `/code-review <level> --fix` (e.g. `/code-review high --fix`), then land the unit:
    commit → push → open PR → watch CI → merge, via `/git-tools:ship` (a CI failure

plugins-claude/session/skills/session-end/SKILL.md

@@ -9,8 +9,10 @@ commit → PR → CI → merge → cleanup lifecycle. This skill owns only the
 pre-ship quality pass; `ship` owns everything git/PR/worktree. This is where the
 last (or only) unit of the scope gets shipped, exactly once; earlier units were
 already landed via `ship`. Run straight
-through to the report with no routine prompts; the one deliberate stop is step 7,
-where you confirm with the user that the entire session is complete.
+through steps 1-4 with no routine prompts; the one deliberate stop is step 5,
+**before** anything is shipped: summarize the work and caveats and wait for the
+user's go. Only after that go do steps 6-8 run (ship, CI, re-merge if needed,
+cleanup, final summary) without stopping again.
 
 **Preflight:** this skill hands off to `git-tools:ship`. If that skill is not
 available (the `git-tools` plugin is not installed or enabled), say so and stop
@@ -24,7 +26,9 @@ bash ${CLAUDE_PLUGIN_ROOT}/scripts/git-wait platform
 ```
 
 If it fails, stop and tell the user, or offer to proceed locally without the
-issue and PR steps (skip steps 4-6 and do not invoke `ship`). On success, capture
+issue and PR steps (skip steps 4, 6 and 7 and do not invoke `ship`); in that mode step 5 offers only
+**Done** and **Make changes first**, leaves the issue lines out of the summary, and
+step 8 reports the local state instead of a PR. On success, capture
 the `owner/repo` and the platform now (for example
 `gh repo view --json nameWithOwner -q .nameWithOwner`, or parse
 `git remote get-url origin`). Use `gh -R owner/repo` / `tea --repo owner/repo`
@@ -61,23 +65,51 @@ instead of fixing them. Do not run repo validation here; step 3 runs it once.
 ### 3. Review and fix (hard gate)
 
 Shipping requires a review of the current state of the work. First decide whether
-one already happened **in this session**: a `code-review ... --fix` run (or an
-equivalent review-and-fix agent) earlier in the conversation, with no code edits
-since other than that review's own fixes. Steps 1-2 edits to docs count as
-non-code; any code or test edit made after the review means it is stale.
-
-- **Already reviewed and still current** — skip the review and say so in one
-  line (for example "Review already ran this session; skipping").
-- **Otherwise** — invoke the `code-review` skill with `--fix` on the working
-  tree. If the `code-review` skill is not available, use the Agent tool to spawn
-  a general-purpose review-and-fix agent over the working-tree diff instead: it
-  reports findings (correctness bugs, edge cases, error handling, missing tests,
-  with file:line) and applies the fixes. Either way, apply the findings, then run
-  the repo's documented validation **once** (for example `validate-all.sh` or the
-  test command in CLAUDE.md) and fix what fails. Cap this fix loop at two
-  attempts: if validation or review findings still fail after the second, stop,
-  report what remains, and do not invoke `ship`. Surface only findings you chose
-  not to fix, with a one-line reason each.
+one already happened **in this session** and is still current:
+
+- **Already reviewed and still current** — a review-and-fix run earlier in the
+  conversation, followed by no code or test edits other than that review's own
+  *minor* fixes. Docs edits from steps 1-2 count as non-code. Skip the review and
+  say so in one line (for example "Review already ran this session; skipping").
+- **Stale** — any code or test edit since the review, or review fixes that were
+  *significant*: they changed logic or behavior, added or removed more than a few
+  lines, or touched files outside the reviewed diff. Typos, wording, formatting,
+  and comment tweaks are minor and do not make a review stale.
+
+Otherwise run the loop below.
+
+**Scale the effort to the change.** Pick the review level from the diff's size and
+risk, not a fixed default: prose, docs, or config only, or a few lines in one
+file → `low`; a handful of files with simple logic → `medium`; multi-file logic or
+behavior changes → `high`; large, cross-cutting, or security-sensitive changes
+(auth, hooks, shared utils, anything hard to reverse) → `xhigh`.
+
+**Review → triage → fix, at most three passes:**
+
+1. **Review.** Invoke the `code-review` skill at that level **without** `--fix`,
+   so findings come back unapplied. If the `code-review` skill is not available,
+   use the Agent tool to spawn a general-purpose review agent over the
+   working-tree diff: it reports findings (correctness bugs, edge cases, error
+   handling, missing tests, with file:line) and changes nothing.
+2. **Triage.** Do not fix findings on the reviewer's word alone. Spawn one triage
+   `Agent` with a model matched to the change: `haiku` for trivial diffs,
+   `sonnet` for ordinary ones, `opus` for complex or security-sensitive ones. Give
+   it the findings and the diff and ask, per finding, whether it is **real** (a
+   defect that can actually occur) and **significant** (worth fixing in this PR),
+   with a one-line reason. Findings judged not real, or real but minor, are
+   dropped and reported with that reason. They never trigger another pass.
+3. **Fix.** Apply the real, significant findings, then run the repo's documented
+   validation **once** (for example `validate-all.sh` or the test command in
+   CLAUDE.md) and fix what fails.
+4. **Decide whether to loop.** If the fixes were significant (see "Stale" above)
+   and this was not pass 3, run another pass from step 1; otherwise the review is
+   satisfied.
+
+Pass 3 is the last pass; do not re-review after its fixes. If pass 3 produced
+significant fixes, real significant findings remain, or validation still fails,
+**stop**: do not invoke `ship`. Ask the user via `AskUserQuestion`, listing what
+remains with a one-line reason each, and let them choose whether to ship as is or
+direct changes. Never start a fourth pass on your own.
 
 This is a hard gate: never invoke `ship` while a required review is outstanding,
 and do not offer the user a way to skip it. Skip only if the user explicitly
@@ -108,35 +140,66 @@ Decide which issues this work resolves.
    title is ambiguous.
 3. Resolved → will be closed via the PR. Partial → leave open and write down the list of partial issue numbers
    now (with a one-line note of the progress made on each), so it survives the
-   ship step; the comment is posted in step 6 after the merge. If the call is
+   ship step; the comment is posted in step 7 after the merge. If the call is
    genuinely unclear, ask once via `AskUserQuestion`, batched across all unclear
    issues.
 4. If something should be pushed through to completion and is small, finish it
    now with **one** extra pass (steps 2-3 once more, no further loop) rather than
    leaving it open; anything still unfinished stays open as partial.
 
 Also list any new problems found along the way that deserve their own issue;
-carry them to the step 7 confirmation rather than filing them unprompted.
-
-### 5. Ship
-
-Only once step 3 is satisfied, invoke `git-tools:ship` via the `Skill` tool. Pass as arguments the closing
+carry them to the step 5 summary rather than filing them unprompted.
+
+### 5. Summarize and wait for the user's go (STOP)
+
+Nothing has been committed, pushed, or opened as a PR yet. Build the summary, then
+**stop and wait for the user's reply** with `AskUserQuestion` (not a plain-text
+question), so the user can answer from `/rc` on their phone. Print the full summary
+as text first, and put a condensed version (outcome, validation, top caveats) in
+the question text itself, because the remote client may not show the text above it.
+Ask one question with these options: **Ship it** (recommended) and **Make changes
+first**; the user's own words, including a cancel, arrive through "Other". The
+summary covers:
+
+- One-line outcome and the branch.
+- What changed, per file (or per area for a large diff), including the docs edits.
+- Validation status (what ran and passed; say plainly if something was not run)
+  and the review outcome: findings fixed, and any you chose not to fix with the
+  reason.
+- **Caveats** — known or potential problems, assumptions, and uncovered edge
+  cases, stated specifically. Omit only if there are genuinely none.
+- Issues this work will close, partial issues, and proposed new issues.
+
+If the user cancels or says to keep the work local, stop: do not invoke `ship`,
+and report the branch and working-tree state. If they pick **Make changes first**
+without saying what to change, ask. If they pick it or answer with changes, make them,
+re-run validation once, redo the step 3 review if the edits made it stale, and ask
+again with a fresh summary. Do not invoke `ship`, commit, push, or open a PR until
+the user picks **Ship it** for the current state of the work; an earlier approval
+(for example of the plan) does not count.
+
+### 6. Ship
+
+Only once the user has given the go in step 5, and step 3 is satisfied, invoke
+`git-tools:ship` via the `Skill` tool. Pass as arguments the closing
 lines the PR body must contain — `Fixes #N` for bugs, `Closes #N` otherwise,
 one per resolved issue — and tell it the PR body should summarize the review
 and docs changes. `ship` handles staging, branch, push, PR, CI, merge wait,
 post-merge run, return to the default branch, and worktree teardown.
 
-If `ship` stops on a CI failure or a blocked merge, stop too: report the state
-and do not comment on any issue. Distinguish "merged" from "auto-merge enabled
-but not yet merged": post no "resolved" or progress comment until the PR is
-actually merged. Offer to wait for the merge, or tell the user to re-run the
-sweep (step 6) once it lands.
+The user's go covers the whole lifecycle: watch CI, fix forward a failure clearly
+caused by this change and re-merge (`ship` caps this at two attempts), and clean
+up. Do not ask again between those steps. If `ship` still stops on a CI failure
+or a blocked merge, stop too: report the state and do not comment on any issue.
+Distinguish "merged" from "auto-merge enabled but not yet merged": post no
+"resolved" or progress comment until the PR is actually merged. Offer to wait for
+the merge, or tell the user to re-run the sweep (step 7) once it lands.
 
-### 6. Comment on partial issues
+### 7. Comment on partial issues
 
 `ship` step 9 removes the worktree, so the current directory may no longer
 exist. Before running any command here, if it is gone, call `ExitWorktree` with
-`action: "keep"` (see step 7), or switch to a valid directory such as the main
+`action: "keep"` (see step 8), or switch to a valid directory such as the main
 checkout. Always use the `-R owner/repo` / `--repo` form captured in preflight.
 
 Only after ship reports the PR merged, post a short comment on each partial issue
@@ -150,20 +213,17 @@ Use `tea comment --repo owner/repo N "<same text>"` on Gitea. If the PR did not
 merge (closed, blocked, timed out, or auto-merge pending), do not comment; say
 so in the report instead. Skip this step when there are no partial issues.
 
-### 7. Report and confirm completion
-
-After `ship` finishes and partial-issue comments are posted, give a short summary:
-PR URL and merge/CI state, issues closed, partial issues commented (only those
-actually commented), proposed new issues from step 4, cleanup / worktree teardown
-done, and anything skipped or left for the user.
+### 8. Final report
 
-Then **stop and confirm with the user before we call this entire session
-complete** — this is the single completion confirmation, covering issue closing,
-cleanup, and the summary. Do not treat the session as finished, or file the
-proposed issues, until they answer.
+After `ship` finishes and partial-issue comments are posted, give a short summary
+of the final state: PR URL and merge/CI state (including any fix-forward commits
+or re-merges), issues closed, partial issues commented (only those actually
+commented), cleanup / worktree teardown done, and anything skipped or left for
+the user. List the proposed new issues from step 4 and file them only if the user
+asks. This is a report, not a further confirmation stop.
 
 If this session was entered with `EnterWorktree` and the worktree directory no
 longer exists after `ship` (ship removes it with plain git), call `ExitWorktree`
 with `action: "keep"` to reset the harness's session state (do this at the start
-of step 6 when that step has work to do). Never use `remove`
+of step 7 when that step has work to do). Never use `remove`
 or `discard` here; `ship` already removed the directory.

plugins-claude/session/skills/session-issue/SKILL.md

@@ -12,10 +12,11 @@ your own description instead, use `/session:session-start`.
 > **CRITICAL**: You MUST drive this to a plan. NEVER print "suggested first steps"
 > or ask "ready to start?" — the flow does not end until you have explored the code
 > and called `EnterPlanMode`. Plan approval then authorizes the whole stated scope
-> through merge: land each unit before the last via `/git-tools:ship` without
-> stopping, then run the last unit through `/session:session-end`, which ships it
-> (spine Phase 4); the one stop is confirming with the user before the entire session
-> is called complete.
+> for every unit before the last through merge: land each via `/git-tools:ship`
+> without stopping, then run the last unit through `/session:session-end` (spine
+> Phase 4). The one stop is before that final ship: summarize the work and caveats
+> and wait for the user's go. Only then ship, watch CI, fix, re-merge, close out, and
+> give a final summary.
 
 ### Phase 1 — Pick issues