From 52a04a720ceddc383d88f23c5cacaa46425b8e57 Mon Sep 17 00:00:00 2001 From: Logan Gagne Date: Sat, 17 Oct 2026 09:51:56 -0400 Subject: [PATCH] feat(session): pause for user review before shipping the final unit session-end used to ship the last unit and only then ask for completion, so a one-PR session opened and merged its PR without the user seeing a summary. The stop now comes before the final ship. - session-end: new step 5 summarizes the work and caveats and waits for the user's go via AskUserQuestion (Ship it / Make changes first; cancel through Other); after the go it ships, watches CI, fixes and re-merges, cleans up, and ends with a final-state report - review gate: effort scales with change complexity, a triage agent judges findings real/significant before any fix, re-review only after significant fixes, at most three review/fix passes then ask the user - earlier units of a multi-PR scope still ship without asking - spine, start/issue/orchestrate skills, Copilot commands and READMEs updated to match; session plugin bumped to 4.11.0 (Claude + Copilot) Co-Authored-By: Claude Sonnet 5.5 --- .../session/.claude-plugin/plugin.json | 2 +- plugins-claude/session/README.md | 11 +- plugins-claude/session/reference/spine.md | 32 ++-- .../session/skills/session-end/SKILL.md | 148 ++++++++++++----- .../session/skills/session-issue/SKILL.md | 9 +- .../skills/session-orchestrate/SKILL.md | 12 +- .../session/skills/session-start/SKILL.md | 9 +- .../session/.claude-plugin/plugin.json | 2 +- plugins-copilot/session/README.md | 16 +- plugins-copilot/session/commands/end.md | 157 ++++++++++++------ plugins-copilot/session/commands/issue.md | 2 +- .../session/commands/orchestrate.md | 10 +- plugins-copilot/session/commands/start.md | 12 +- .../session/skills/session-issue/SKILL.md | 7 +- 14 files changed, 287 insertions(+), 142 deletions(-) diff --git a/plugins-claude/session/.claude-plugin/plugin.json b/plugins-claude/session/.claude-plugin/plugin.json index e18dd97..0a3b5b2 100644 --- a/plugins-claude/session/.claude-plugin/plugin.json +++ b/plugins-claude/session/.claude-plugin/plugin.json @@ -1,6 +1,6 @@ { "name": "session", - "version": "4.10.1", + "version": "4.11.0", "dependencies": [ "git-tools" ], diff --git a/plugins-claude/session/README.md b/plugins-claude/session/README.md index b659f4d..cd8160c 100644 --- a/plugins-claude/session/README.md +++ b/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 diff --git a/plugins-claude/session/reference/spine.md b/plugins-claude/session/reference/spine.md index 94d4841..e26dce8 100644 --- a/plugins-claude/session/reference/spine.md +++ b/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 --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 diff --git a/plugins-claude/session/skills/session-end/SKILL.md b/plugins-claude/session/skills/session-end/SKILL.md index 55e582c..7f7b1c1 100644 --- a/plugins-claude/session/skills/session-end/SKILL.md +++ b/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,7 +140,7 @@ 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 @@ -116,27 +148,58 @@ Decide which issues this work resolves. 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 ""` 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. diff --git a/plugins-claude/session/skills/session-issue/SKILL.md b/plugins-claude/session/skills/session-issue/SKILL.md index 49ba2a6..d5b451d 100644 --- a/plugins-claude/session/skills/session-issue/SKILL.md +++ b/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 diff --git a/plugins-claude/session/skills/session-orchestrate/SKILL.md b/plugins-claude/session/skills/session-orchestrate/SKILL.md index 89eca70..e335bf8 100644 --- a/plugins-claude/session/skills/session-orchestrate/SKILL.md +++ b/plugins-claude/session/skills/session-orchestrate/SKILL.md @@ -11,7 +11,7 @@ The workflow has seven phases. Two have hard user gates (Refine and Execute). Th This skill is split across two execution surfaces, and the split is deliberate: -- **Interactive, in the main session** — Phases 0–4, the Phase 3/5 gates, the Phase 6 concerns gate, and Phase 7. These need a human in the loop (refinement conversation, approvals, the completion confirmation), so they stay in the main session where you can call `AskUserQuestion` and talk to the user. +- **Interactive, in the main session** — Phases 0–4, the Phase 3/5 gates, the Phase 6 concerns gate, and Phase 7. These need a human in the loop (refinement conversation, approvals, the pre-ship review of the last unit), so they stay in the main session where you can call `AskUserQuestion` and talk to the user. - **Headless, in a `Workflow`** — the Execute waves and the Review/blocker-auto-fix loop (`scripts/orchestrate-build.workflow.js`). This is a closed-loop machine — fan out by wave, escalate failures, review, re-dispatch blockers, re-review, cap the loop — with no human decision inside it. Expressing it as JS makes the control flow deterministic instead of something you have to police by hand. > **CRITICAL**: You MUST drive every phase to completion. Do NOT collapse the workflow into a single in-line plan, and do NOT hand-roll the build loop in the main session — Phase 5 dispatches the `Workflow` and Phase 6 consumes its structured result. Sub-agent dispatch is the point — the user is paying for parallelism and model tiering, not for you to do everything serially in the main session. @@ -108,7 +108,7 @@ Goal: iterate with the user until the spec and plan are agreed. - **More refinement needed** — return to step 2 of this phase - **Stop here** — abort the workflow - The plan must state its scope explicitly (a `Scope:` line listing the PR(s)/unit(s)); approval authorizes that whole scope through merge (Phase 7). + The plan must state its scope explicitly (a `Scope:` line listing the PR(s)/unit(s)); approval authorizes every unit before the last through merge; the last unit pauses for the user's go before it ships (Phase 7). On approval, write the agreed spec and plan to `$STATE/spec.md` and `$STATE/plan.md` (overwriting the Phase 1 draft). @@ -215,9 +215,9 @@ Goal: act on the `Workflow`'s structured result. The blocker auto-fix loop alrea Carry `perChunk`, `filesChanged`, `clean`, deferred `concerns`, and `nits` forward — Phase 7's summary draws on them. -### Phase 7 — Ship and confirm completion +### Phase 7 — Ship (pre-ship review of the last unit) -Goal: summarize, land the work, and confirm the session is complete. +Goal: summarize, land the earlier units, and have the user review the last unit before it ships. 1. **Produce a final summary** for the user covering: - One-line outcome of the feature @@ -226,9 +226,9 @@ Goal: summarize, land the work, and confirm the session is complete. - Test coverage added (if any) - **Caveats** — be up front and specific about known *or potential* problems: deferred concerns from Phase 6, chunks surfaced for manual handling, assumptions made, uncovered edge cases, and any known risks or follow-ups. If you are unsure something works, say so. Do not downplay risks to make the result look finished. -2. **Land the work without waiting.** The Phase 3/5 approvals authorize the stated scope through merge. Print the summary as a short progress note, land every unit before the last via `/git-tools:ship`, then run the last unit (a single-unit scope is just this) through `/session:session-end` (docs, review + fix, issue sweep, ships it once via `/git-tools:ship`) — do not ship it separately first. With linked issues, the PR body carries one closing line per issue (`Closes #N` / `Fixes #N`) — use the in-context list if there is one, otherwise read `git config branch.$(git branch --show-current).session-issues` (comma-separated closing lines, e.g. `Closes #12,Fixes #13`). If the scope spans several PRs/units, continue through each in turn without stopping between them. +2. **Land earlier units without waiting.** The Phase 3/5 approvals authorize every unit before the last through merge; the last unit pauses for the user's review first (step 3). Print the summary as a short progress note, land every unit before the last via `/git-tools:ship`, then run the last unit (a single-unit scope is just this) through `/session:session-end` (docs, review + fix, issue sweep, ships it once via `/git-tools:ship`) — do not ship it separately first. With linked issues, the PR body carries one closing line per issue (`Closes #N` / `Fixes #N`) — use the in-context list if there is one, otherwise read `git config branch.$(git branch --show-current).session-issues` (comma-separated closing lines, e.g. `Closes #12,Fixes #13`). If the scope spans several PRs/units, continue through each in turn without stopping between them. -3. **The one deliberate stop:** once `/session:session-end` has merged the last unit, **stop and confirm with the user before we call this entire session complete** (issue closing, cleanup / worktree teardown, final summary — `/session:session-end` step 7). Do not treat the session as finished until they answer. +3. **The one deliberate stop:** `/session:session-end` **stops before shipping the last unit** to summarize the work and caveats and wait for the user's go; after the go it ships, watches CI, fixes and re-merges if needed, and ends with a final-state summary (issue closing, cleanup / worktree teardown, final summary — `/session:session-end` steps 5-8). Do not ship the last unit before they answer. If this run created a worktree (Phase 0b), `/git-tools:ship` step 9 removes it after the PR merges (`/session:session-end` runs it too), or the user can leave it and exit later with `ExitWorktree`. diff --git a/plugins-claude/session/skills/session-start/SKILL.md b/plugins-claude/session/skills/session-start/SKILL.md index 3f999fd..9b0975b 100644 --- a/plugins-claude/session/skills/session-start/SKILL.md +++ b/plugins-claude/session/skills/session-start/SKILL.md @@ -14,10 +14,11 @@ issues instead, use `/session:session-issue`; for a read-only status view, use > **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 0 — Take the input and ground it diff --git a/plugins-copilot/session/.claude-plugin/plugin.json b/plugins-copilot/session/.claude-plugin/plugin.json index 972f53a..15805fc 100644 --- a/plugins-copilot/session/.claude-plugin/plugin.json +++ b/plugins-copilot/session/.claude-plugin/plugin.json @@ -1,6 +1,6 @@ { "name": "session", - "version": "4.10.1", + "version": "4.11.0", "description": "Work session management — issue-driven and freeform doors sharing an explore-then-plan spine, with multi-agent orchestration and a wrap-up skill that reviews, sweeps issues, and ships", "author": { "name": "Logan Gagne" diff --git a/plugins-copilot/session/README.md b/plugins-copilot/session/README.md index fb401d9..471d2ae 100644 --- a/plugins-copilot/session/README.md +++ b/plugins-copilot/session/README.md @@ -61,7 +61,7 @@ same either way. | `/session:start` | Start from your description — ground, branch, explore, plan | | `/session:issue` | Rank open issues, pick one, then explore and plan | | `/session:orchestrate` | Multi-agent feature workflow: spec → plan → refine → divide → execute → review | -| `/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: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) @@ -96,7 +96,12 @@ Pick based on what you need: finishes adjacent work, and enforces a code-review gate (`/sf-code-review:review` or a review agent, then fix) before shipping — skipped only if a review already ran this session on the current state. It also sweeps issues so `Closes #N` / - `Fixes #N` lines reach the PR. Ship does the rest, including worktree teardown. + `Fixes #N` lines reach 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 remotely); earlier units + of a multi-PR scope never stop. Ship does the rest, including worktree teardown. - **`/git-tools:ship`** — the quick canonical lifecycle, no review gate or docs pass. Also worktree-aware: after merge it returns to the main worktree, removes the merged worktree, prunes, and deletes the branch. @@ -107,9 +112,10 @@ Pick based on what you need: /session:start "add CSV export" # or /session:issue to pick one → isolates in a worktree, explores, proposes a plan ... plan approval fixes the scope; the agent implements, then commits, pushes, - opens the PR, watches CI and merges each unit without stopping ... -/session:end # docs, review gate, close issues, ship, then - # confirm the whole session is complete + opens the PR, watches CI and merges each unit before the last without stopping ... +/session:end # docs, review gate, close issues, then STOP: summary + # + caveats, wait for your go; then ship, CI, + # re-merge if needed, final-state summary ``` ## Branch Type Detection diff --git a/plugins-copilot/session/commands/end.md b/plugins-copilot/session/commands/end.md index 9de84dc..326eeee 100644 --- a/plugins-copilot/session/commands/end.md +++ b/plugins-copilot/session/commands/end.md @@ -8,9 +8,11 @@ commit → PR → CI → merge → cleanup lifecycle. This command owns only the pre-ship quality pass (the last or only unit of the scope is shipped here, exactly once; earlier units were already landed via ship); `ship` owns everything git/PR/worktree, including worktree teardown (via the `git-worktree` extension's `sf_git_worktree_remove` -tool or the direct `git worktree remove` flow). 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. +tool or the direct `git worktree remove` flow). Run straight 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 command hands off to `/git-tools:ship`. If that command is not available (the `git-tools` plugin is not installed), say so and stop before doing @@ -18,8 +20,10 @@ any docs or review work. Then detect the hosting platform (`github.com` remote → `gh`; a host matching a configured `tea` login → `tea`). If detection fails, stop and tell the user, or -offer to proceed locally without the issue and PR steps (skip steps 4-6 and do not -run ship). On success, capture `owner/repo` now (`gh repo view --json +offer to proceed locally without the issue and PR steps (skip steps 4, 6 and 7 and do not +run 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 `owner/repo` now (`gh repo view --json nameWithOwner -q .nameWithOwner`, or parse `git remote get-url origin`) and use `gh -R owner/repo` / `tea --repo owner/repo` for every later issue command, so they still work after ship removes the worktree. @@ -54,29 +58,60 @@ 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 `/sf-code-review:review` run (or an -equivalent review agent) earlier in the conversation whose findings were fixed, -with no code edits since other than those fixes. Doc edits from step 1 do not -count; any code or test edit 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** — run the cross-model review with `/sf-code-review:review` - (use `--base ` when there are commits ahead of the default - branch). If that command is not installed, use the Task tool to spawn a - review agent over the same changes (working tree plus any commits ahead of - the default branch), asking for correctness bugs, edge cases, error handling, - and missing tests, and for concise findings with file:line. Then fix the - findings you accept, 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 hand off to 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 `/sf-code-review:review` run (or an + equivalent review agent) earlier in the conversation whose findings were fixed, + followed by no code or test edits other than those fixes, and those fixes were + *minor*. Doc edits from step 1 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 depth from the diff's size and +risk, not a fixed default: prose, docs, or config only, or a few lines in one +file → a light pass; a handful of files with simple logic → a standard pass; +multi-file logic or behavior changes → a thorough pass; large, cross-cutting, or +security-sensitive changes (auth, hooks, shared utils, anything hard to reverse) → +the most thorough pass available. + +**Review → triage → fix, at most three passes:** + +1. **Review.** Run the cross-model review with `/sf-code-review:review` (use + `--base ` when there are commits ahead of the default branch), + treating its output as findings only. If that command is not installed, use the + Task tool to spawn a review agent over the same changes (working tree plus any + commits ahead of the default branch), asking for correctness bugs, edge cases, + error handling, and missing tests, with concise findings and file:line. +2. **Triage.** Do not fix findings on the reviewer's word alone. Spawn one triage + agent with a model matched to the change (a small, fast model for trivial + diffs, a mid-tier model for ordinary ones, the most capable model 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 run 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 run `/git-tools:ship` while a required review is outstanding, and do not offer the user a way to skip it. Skip only if the user -explicitly tells you to. Once the gate is satisfied, run straight through ship to -the single completion confirmation in step 7. +explicitly tells you to. Once the gate is satisfied, go to the step 5 summary and +wait for the user's go. ### 4. Issue sweep @@ -102,7 +137,7 @@ 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 @@ -110,22 +145,53 @@ Decide which issues this work resolves. 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, run `/git-tools:ship`. Pass as arguments the +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 remotely from their phone. Print the full summary +as text first, and put a condensed version (outcome, validation, top caveats) in +the question text itself, because a 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 run 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 run `/git-tools: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, run +`/git-tools:ship`. Pass as arguments the closing lines the PR body must contain — `Fixes #N` for bugs, `Closes #N` otherwise, one per resolved issue, each on its own line — and say the PR body should summarize the review and docs changes. -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 removes the worktree, so the current directory may no longer exist; if it is gone, switch to a valid directory (such as the main checkout) before running any @@ -142,17 +208,14 @@ Use `tea comment --repo owner/repo N ""` 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 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 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. ### Notes diff --git a/plugins-copilot/session/commands/issue.md b/plugins-copilot/session/commands/issue.md index 8059e24..9894930 100644 --- a/plugins-copilot/session/commands/issue.md +++ b/plugins-copilot/session/commands/issue.md @@ -12,4 +12,4 @@ This command invokes the sibling `session-issue` skill. The flow: 2. **Let the user choose** by issue count: 0 → suggest `/session:start`; 1 → start it; 2-4 → AskUserQuestion; 5 or more → print the full ranked list and have the user type the number(s). 3. **Fetch the selected issues** and keep their bodies + labels as context. 4. **Choose the branch type** from labels, isolate the work on a branch or worktree, and record `git config branch..session-issues`. -5. **Explore, then plan** — inspect the codebase and present a concrete implementation plan, with a `Scope:` line; approval authorizes the whole scope through merge. Land each unit before the last via `/git-tools:ship` without stopping; run the last unit through `/session:end`, which ships it and then confirms with the user before the entire session is called complete. +5. **Explore, then plan** — inspect the codebase and present a concrete implementation plan, with a `Scope:` line; approval authorizes every unit before the last through merge. Land each via `/git-tools:ship` without stopping; run the last unit through `/session:end`, which stops before shipping to summarize the work and caveats and wait for the user's go, then ships, watches CI, fixes and re-merges if needed, and ends with a final-state summary. diff --git a/plugins-copilot/session/commands/orchestrate.md b/plugins-copilot/session/commands/orchestrate.md index 7a89f2f..4c17004 100644 --- a/plugins-copilot/session/commands/orchestrate.md +++ b/plugins-copilot/session/commands/orchestrate.md @@ -96,7 +96,7 @@ Goal: iterate with the user until the spec and plan are agreed. - **More refinement needed** — return to step 2 of this phase - **Stop here** — abort the workflow - The plan must state its scope explicitly (a `Scope:` line listing the PR(s)/unit(s)); approval authorizes that whole scope through merge (Phase 7). + The plan must state its scope explicitly (a `Scope:` line listing the PR(s)/unit(s)); approval authorizes every unit before the last through merge; the last unit pauses for the user's go before it ships (Phase 7). On approval, write the agreed spec and plan to `$STATE/spec.md` and `$STATE/plan.md`. @@ -160,9 +160,9 @@ If concerns only: present via `AskUserQuestion`: If only nits or clean: proceed to Phase 7. -### Phase 7 — Ship and confirm completion +### Phase 7 — Ship (pre-ship review of the last unit) -Goal: summarize, land the work, and confirm the session is complete. +Goal: summarize, land the earlier units, and have the user review the last unit before it ships. 1. **Produce a final summary** for the user covering: - One-line outcome of the feature @@ -171,9 +171,9 @@ Goal: summarize, land the work, and confirm the session is complete. - Test coverage added (if any) - Caveats — deferred concerns from Phase 6, skipped or failed chunks, assumptions, and any known risks or follow-ups; be specific -2. **Land the work without waiting.** The Phase 3/5 approvals authorize the stated scope through merge. Print the summary as a short progress note, land every unit before the last via `/git-tools:ship` (commit, push, PR, watch CI, merge), then run the last unit through `/session:end` (docs, review, issue sweep, ships it once) — do not ship it separately first. With linked issues, the PR body carries one closing line per issue (`Closes #N` / `Fixes #N`) — use the in-context list, otherwise read `git config branch.$(git branch --show-current).session-issues` (comma-separated). +2. **Land earlier units without waiting.** The Phase 3/5 approvals authorize every unit before the last through merge; the last unit pauses for the user's review first (step 3). Print the summary as a short progress note, land every unit before the last via `/git-tools:ship` (commit, push, PR, watch CI, merge), then run the last unit through `/session:end` (docs, review, issue sweep, ships it once) — do not ship it separately first. With linked issues, the PR body carries one closing line per issue (`Closes #N` / `Fixes #N`) — use the in-context list, otherwise read `git config branch.$(git branch --show-current).session-issues` (comma-separated). -3. **The one deliberate stop:** once `/session:end` has merged the last unit, stop and confirm with the user before we call this entire session complete (issue closing, cleanup, final summary — `/session:end` closing step). +3. **The one deliberate stop:** `/session:end` stops before shipping the last unit to summarize the work and caveats and wait for the user's go; after the go it ships, watches CI, fixes and re-merges if needed, and ends with a final-state summary (issue closing, cleanup — `/session:end` steps 5-8). Do not ship the last unit before they answer. ### Notes diff --git a/plugins-copilot/session/commands/start.md b/plugins-copilot/session/commands/start.md index 3735ed9..ff6cc8b 100644 --- a/plugins-copilot/session/commands/start.md +++ b/plugins-copilot/session/commands/start.md @@ -83,9 +83,10 @@ open issues instead, use `/session:issue`. to a few lines. Present a concrete plan (files to change and how, testing only when behavior changes, risks) and get approval before implementing. **Always end the plan with a `Scope:` line** listing the PR(s)/unit(s) it covers; approval - authorizes that whole scope through merge. + authorizes every unit before the last through merge; the last unit pauses for + the user's go before it ships. -6. **Report, continue the scope, confirm at completion.** After each unit, print a +6. **Report, continue the scope, pause before the final ship.** After each unit, print a short progress note (not a wait) built from real commands (`git status --short`, commits ahead of the default branch): outcome and per-file changes, branch name, test/build status (say so if not run), specific caveats. Then land it without @@ -94,7 +95,8 @@ open issues instead, use `/session:issue`. line per issue in the PR body (read them from `git config branch.$(git branch --show-current).session-issues` if out of context). Do **not** ship the last unit (a single-unit scope is just this) here: - run `/session:end`, which does docs, review and issue sweep, ships it once, and - ends with the one stop: confirm with the user before we call this entire session - complete. Lightweight is the only flow; mention `/session:orchestrate` only if the + run `/session:end`, which does docs, review and issue sweep, then makes the one + stop: summarize the work and caveats and wait for the user's go before shipping. + After the go it ships once, watches CI, fixes and re-merges if needed, and ends + with a final-state summary. Lightweight is the only flow; mention `/session:orchestrate` only if the user asks for a heavier multi-agent flow. diff --git a/plugins-copilot/session/skills/session-issue/SKILL.md b/plugins-copilot/session/skills/session-issue/SKILL.md index 217989e..1b98e2b 100644 --- a/plugins-copilot/session/skills/session-issue/SKILL.md +++ b/plugins-copilot/session/skills/session-issue/SKILL.md @@ -111,13 +111,14 @@ code and propose a plan. To start from your own description instead, use call/data flow, find existing tests and conventions. Then present a concrete plan (files to change and how, testing, risks) and get approval before implementing. End the plan with a `Scope:` line listing the PR(s)/unit(s); approval authorizes - the whole scope through merge. After each unit print a short progress note (not a + every unit before the last through merge; the last pauses for review first. After each unit print a short progress note (not a wait) from real commands (`git status --short`, commits ahead of the default branch): outcome, branch, test status, caveats. Then ship it via `/git-tools:ship` (commit, push, PR, CI, merge) and start the next unit until the scope is done. List one closing line per issue (`Fixes #N` for bugs, `Closes #N` otherwise) for the PR body, reading them from the `session-issues` git config if out of context. - After the last unit run `/session:end`; its closing step is the one stop: confirm - with the user before we call this entire session complete. Lightweight is the only + After the last unit run `/session:end`; the one stop is before its ship step: + it summarizes the work and caveats and waits for the user's go, then ships, fixes + and re-merges if needed, and ends with a final-state summary. Lightweight is the only flow; mention `/session:orchestrate` only if the user asks for a heavier multi-agent flow.