mirror of
https://github.com/obra/superpowers.git
synced 2026-06-12 13:49:05 +08:00
Shared: unique review-package collateral names
This commit is contained in:
committed by
Jesse Vincent
parent
d4dbf44162
commit
a995af2e24
@@ -112,7 +112,7 @@ single-file mechanical fixes.
|
|||||||
|
|
||||||
Implementer subagents report one of four statuses. Handle each appropriately:
|
Implementer subagents report one of four statuses. Handle each appropriately:
|
||||||
|
|
||||||
**DONE:** Generate the review package (`scripts/review-package BASE HEAD /tmp/sdd-task-N.diff`, from this skill's directory; BASE is the commit you recorded before dispatching the implementer — never `HEAD~1`, which silently drops all but the last commit of a multi-commit task), then dispatch the task reviewer with that path.
|
**DONE:** Generate the review package (`scripts/review-package BASE HEAD`, from this skill's directory — it prints the unique file path it wrote; BASE is the commit you recorded before dispatching the implementer — never `HEAD~1`, which silently drops all but the last commit of a multi-commit task), then dispatch the task reviewer with the printed path.
|
||||||
|
|
||||||
**DONE_WITH_CONCERNS:** The implementer completed the work but flagged doubts. Read the concerns before proceeding. If the concerns are about correctness or scope, address them before review. If they're observations (e.g., "this file is getting large"), note them and proceed to review.
|
**DONE_WITH_CONCERNS:** The implementer completed the work but flagged doubts. Read the concerns before proceeding. If the concerns are about correctness or scope, address them before review. If they're observations (e.g., "this file is getting large"), note them and proceed to review.
|
||||||
|
|
||||||
@@ -154,13 +154,13 @@ final whole-branch review. When you fill a reviewer template:
|
|||||||
floors, naming and copy rules, platform requirements) in the requirements
|
floors, naming and copy rules, platform requirements) in the requirements
|
||||||
you paste — a reviewer can only enforce what you hand them.
|
you paste — a reviewer can only enforce what you hand them.
|
||||||
- Hand the reviewer its diff as a file: run this skill's
|
- Hand the reviewer its diff as a file: run this skill's
|
||||||
`scripts/review-package BASE HEAD /tmp/sdd-task-N.diff` (or, without
|
`scripts/review-package BASE HEAD` and pass the reviewer the file path
|
||||||
bash: `git log --oneline`, `git diff --stat`, and `git diff -U10` for
|
it prints (or, without bash: `git log --oneline`, `git diff --stat`,
|
||||||
the range, redirected to the file). The output never enters your own
|
and `git diff -U10` for the range, redirected to one uniquely named
|
||||||
context, and the reviewer sees the commit list, stat summary, and full
|
file). The output never enters your own context, and the reviewer sees
|
||||||
diff with context in one Read call. Use the BASE you recorded before
|
the commit list, stat summary, and full diff with context in one Read
|
||||||
dispatching the implementer — never `HEAD~1`, which silently truncates
|
call. Use the BASE you recorded before dispatching the implementer —
|
||||||
multi-commit tasks.
|
never `HEAD~1`, which silently truncates multi-commit tasks.
|
||||||
- Dispatch fix subagents for Critical and Important findings. Record Minor
|
- Dispatch fix subagents for Critical and Important findings. Record Minor
|
||||||
findings and move on — then paste the accumulated Minor findings into the
|
findings and move on — then paste the accumulated Minor findings into the
|
||||||
final whole-branch review dispatch so it can triage which must be fixed
|
final whole-branch review dispatch so it can triage which must be fixed
|
||||||
@@ -201,7 +201,7 @@ Implementer: "Got it. Implementing now..."
|
|||||||
- Self-review: Found I missed --force flag, added it
|
- Self-review: Found I missed --force flag, added it
|
||||||
- Committed
|
- Committed
|
||||||
|
|
||||||
[Write diff to /tmp/sdd-task-N.diff, dispatch task reviewer with the path]
|
[Run review-package, dispatch task reviewer with the printed path]
|
||||||
Task reviewer: Spec ✅ - all requirements met, nothing extra.
|
Task reviewer: Spec ✅ - all requirements met, nothing extra.
|
||||||
Strengths: Good test coverage, clean. Issues: None. Task quality: Approved.
|
Strengths: Good test coverage, clean. Issues: None. Task quality: Approved.
|
||||||
|
|
||||||
@@ -219,7 +219,7 @@ Implementer:
|
|||||||
- Self-review: All good
|
- Self-review: All good
|
||||||
- Committed
|
- Committed
|
||||||
|
|
||||||
[Write diff to /tmp/sdd-task-N.diff, dispatch task reviewer with the path]
|
[Run review-package, dispatch task reviewer with the printed path]
|
||||||
Task reviewer: Spec ❌:
|
Task reviewer: Spec ❌:
|
||||||
- Missing: Progress reporting (spec says "report every 100 items")
|
- Missing: Progress reporting (spec says "report every 100 items")
|
||||||
- Extra: Added --json flag (not requested)
|
- Extra: Added --json flag (not requested)
|
||||||
@@ -291,8 +291,8 @@ Done!
|
|||||||
dispatch prompt ("treat it as Minor at most") — the plan's example code is
|
dispatch prompt ("treat it as Minor at most") — the plan's example code is
|
||||||
a starting point, not evidence that its weaknesses were chosen
|
a starting point, not evidence that its weaknesses were chosen
|
||||||
- Dispatch a task reviewer without a diff file — generate it first
|
- Dispatch a task reviewer without a diff file — generate it first
|
||||||
(`scripts/review-package BASE HEAD /tmp/sdd-task-N.diff`) and name that
|
(`scripts/review-package BASE HEAD`) and name the printed path in the
|
||||||
path in the prompt
|
prompt
|
||||||
- Move to next task while the review has open Critical/Important issues
|
- Move to next task while the review has open Critical/Important issues
|
||||||
|
|
||||||
**If subagent asks questions:**
|
**If subagent asks questions:**
|
||||||
|
|||||||
@@ -1,25 +1,35 @@
|
|||||||
#!/usr/bin/env bash
|
#!/usr/bin/env bash
|
||||||
# Generate a task review package: commit list, stat summary, and the net
|
# Generate a review package: commit list, stat summary, and the net
|
||||||
# diff with extended context, written to a file the reviewer reads in one
|
# diff with extended context, written to a file the reviewer reads in one
|
||||||
# call. Using the recorded per-task BASE (not HEAD~1) keeps multi-commit
|
# call. Using the recorded per-task BASE (not HEAD~1) keeps multi-commit
|
||||||
# tasks intact.
|
# tasks intact.
|
||||||
#
|
#
|
||||||
# Usage: review-package BASE HEAD OUTFILE
|
# Usage: review-package BASE HEAD [OUTFILE]
|
||||||
# Example: review-package a1b2c3d HEAD /tmp/sdd-task-3.diff
|
# Default OUTFILE: <git-dir>/sdd/review-<base7>..<head7>.diff — unique per
|
||||||
|
# repo instance and per range, so concurrent sessions cannot collide and a
|
||||||
|
# re-review after fixes always gets a distinctly named fresh file.
|
||||||
set -euo pipefail
|
set -euo pipefail
|
||||||
|
|
||||||
if [ $# -ne 3 ]; then
|
if [ $# -lt 2 ] || [ $# -gt 3 ]; then
|
||||||
echo "usage: review-package BASE HEAD OUTFILE" >&2
|
echo "usage: review-package BASE HEAD [OUTFILE]" >&2
|
||||||
exit 2
|
exit 2
|
||||||
fi
|
fi
|
||||||
|
|
||||||
base=$1
|
base=$1
|
||||||
head=$2
|
head=$2
|
||||||
out=$3
|
|
||||||
|
|
||||||
git rev-parse --verify --quiet "$base" >/dev/null || { echo "bad BASE: $base" >&2; exit 2; }
|
git rev-parse --verify --quiet "$base" >/dev/null || { echo "bad BASE: $base" >&2; exit 2; }
|
||||||
git rev-parse --verify --quiet "$head" >/dev/null || { echo "bad HEAD: $head" >&2; exit 2; }
|
git rev-parse --verify --quiet "$head" >/dev/null || { echo "bad HEAD: $head" >&2; exit 2; }
|
||||||
|
|
||||||
|
if [ $# -eq 3 ]; then
|
||||||
|
out=$3
|
||||||
|
else
|
||||||
|
dir=$(git rev-parse --git-path sdd)
|
||||||
|
mkdir -p "$dir"
|
||||||
|
dir=$(cd "$dir" && pwd)
|
||||||
|
out="$dir/review-$(git rev-parse --short "$base")..$(git rev-parse --short "$head").diff"
|
||||||
|
fi
|
||||||
|
|
||||||
{
|
{
|
||||||
echo "# Review package: ${base}..${head}"
|
echo "# Review package: ${base}..${head}"
|
||||||
echo
|
echo
|
||||||
|
|||||||
@@ -154,8 +154,8 @@ Subagent (general-purpose):
|
|||||||
- `[BASE_SHA]` — commit before this task
|
- `[BASE_SHA]` — commit before this task
|
||||||
- `[HEAD_SHA]` — current commit
|
- `[HEAD_SHA]` — current commit
|
||||||
- `[DIFF_FILE]` — REQUIRED: the path the controller wrote the review
|
- `[DIFF_FILE]` — REQUIRED: the path the controller wrote the review
|
||||||
package to (`scripts/review-package BASE HEAD /tmp/sdd-task-N.diff`,
|
package to (`scripts/review-package BASE HEAD` prints the unique path it
|
||||||
redirected so it never enters the controller's context)
|
wrote; the package never enters the controller's context)
|
||||||
|
|
||||||
**Reviewer returns:** Spec Compliance verdict (✅/❌/⚠️), Strengths, Issues
|
**Reviewer returns:** Spec Compliance verdict (✅/❌/⚠️), Strengths, Issues
|
||||||
(Critical/Important/Minor), Task quality verdict
|
(Critical/Important/Minor), Task quality verdict
|
||||||
|
|||||||
Reference in New Issue
Block a user