Compare commits

..

1 Commits

Author SHA1 Message Date
don-petry 423fdcb022
Merge 9ff99eceaf into 783601b576 2026-04-04 16:26:46 -07:00
3 changed files with 21 additions and 50 deletions

View File

@ -15,37 +15,18 @@ story_key: '' # set at runtime when discovered from sprint status
## INSTRUCTIONS ## INSTRUCTIONS
1. **Find the review target.** The conversation context before this skill was triggered IS your starting point — not a blank slate. Check in this order — stop as soon as the review target is identified: 1. **Detect review intent from invocation text.** Check the triggering prompt for phrases that map to a review mode:
- "staged" / "staged changes" → Staged changes only
**Tier 1 — Explicit argument.** - "uncommitted" / "working tree" / "all changes" → Uncommitted changes (staged + unstaged)
Did the user pass a PR, commit SHA, branch, spec file, or diff source this message? - "branch diff" / "vs main" / "against main" / "compared to {branch}" → Branch diff (extract base branch if mentioned)
- PR reference → resolve to branch/commit via `gh pr view`. If resolution fails, ask for a SHA or branch. - "commit range" / "last N commits" / "{sha}..{sha}" → Specific commit range
- Commit or branch → use directly. - "this diff" / "provided diff" / "paste" → User-provided diff (do not match bare "diff" — it appears in other modes)
- Spec file → set `{spec_file}` to the provided path. Check its frontmatter for `baseline_commit`. If found, use as diff baseline. If not found, continue the cascade (a spec alone does not identify a diff source). - When multiple phrases match, prefer the most specific match (e.g., "branch diff" over bare "diff").
- Also scan the argument for diff-mode keywords that narrow the scope: - **If a clear match is found:** Announce the detected mode (e.g., "Detected intent: review staged changes only") and proceed directly to constructing `{diff_output}` using the corresponding sub-case from instruction 3. Skip to instruction 4 (spec question).
- "staged" / "staged changes" → Staged changes only - **If no match from invocation text, check sprint tracking.** Look for a sprint status file (`*sprint-status*`) in `{implementation_artifacts}` or `{planning_artifacts}`. If found, scan for any story with status `review`. Handle as follows:
- "uncommitted" / "working tree" / "all changes" → Uncommitted changes (staged + unstaged) - **Exactly one `review` story:** Set `{story_key}` to the story's key (e.g., `1-2-user-auth`). Suggest it: "I found story {{story-id}} in `review` status. Would you like to review its changes? [Y] Yes / [N] No, let me choose". If confirmed, use the story context to determine the diff source (branch name derived from story slug, or uncommitted changes). If declined, clear `{story_key}` and fall through to instruction 2.
- "branch diff" / "vs main" / "against main" / "compared to <branch>" → Branch diff (extract base branch if mentioned) - **Multiple `review` stories:** Present them as numbered options alongside a manual choice option. Wait for user selection. If the user selects a story, set `{story_key}` to the selected story's key and use the selected story's context to determine the diff source as in the single-story case above, and proceed to instruction 3. If the user selects the manual choice, clear `{story_key}` and fall through to instruction 2.
- "commit range" / "last N commits" / "<from-sha>..<to-sha>" → Specific commit range - **If no match and no sprint tracking:** Fall through to instruction 2.
- "this diff" / "provided diff" / "paste" → User-provided diff (do not match bare "diff" — it appears in other modes)
- When multiple keywords match, prefer the most specific (e.g., "branch diff" over bare "diff").
**Tier 2 — Recent conversation.**
Do the last few messages reveal what the user wants to be reviewed? Look for spec paths, commit refs, branches, PRs, or descriptions of a change. Apply the same diff-mode keyword scan and routing as Tier 1.
**Tier 3 — Sprint tracking.**
Look for a sprint status file (`*sprint-status*`) in `{implementation_artifacts}` or `{planning_artifacts}`. If found, scan for stories with status `review`:
- **Exactly one `review` story:** Set `{story_key}` to the story's key (e.g., `1-2-user-auth`). Suggest it: "I found story <story-id> in `review` status. Would you like to review its changes? [Y] Yes / [N] No, let me choose". If confirmed, use the story context to determine the diff source (branch name derived from story slug, or uncommitted changes). If declined, clear `{story_key}` and fall through.
- **Multiple `review` stories:** Present them as numbered options alongside a manual choice option. Wait for user selection. If a story is selected, set `{story_key}` and use its context to determine the diff source. If manual choice is selected, clear `{story_key}` and fall through.
- **None:** Fall through.
**Tier 4 — Current git state.**
If version control is unavailable, skip to Tier 5. Otherwise, check the current branch and HEAD. If the branch is not `main` (or the default branch), confirm: "I see HEAD is `<short-sha>` on `<branch>` — do you want to review this branch's changes?" If confirmed, treat as a branch diff against `main`. If declined, fall through.
**Tier 5 — Ask.**
Fall through to instruction 2.
Never ask extra questions beyond what the cascade prescribes. If a tier above already identified the target, skip the remaining tiers and proceed to instruction 3 (construct diff).
2. HALT. Ask the user: **What do you want to review?** Present these options: 2. HALT. Ask the user: **What do you want to review?** Present these options:
- **Uncommitted changes** (staged + unstaged) - **Uncommitted changes** (staged + unstaged)
@ -55,19 +36,15 @@ story_key: '' # set at runtime when discovered from sprint status
- **Provided diff or file list** (user pastes or provides a path) - **Provided diff or file list** (user pastes or provides a path)
3. Construct `{diff_output}` from the chosen source. 3. Construct `{diff_output}` from the chosen source.
- For **staged changes only**: run `git diff --cached`.
- For **uncommitted changes** (staged + unstaged): run `git diff HEAD`.
- For **branch diff**: verify the base branch exists before running `git diff`. If it does not exist, HALT and ask the user for a valid branch. - For **branch diff**: verify the base branch exists before running `git diff`. If it does not exist, HALT and ask the user for a valid branch.
- For **commit range**: verify the range resolves. If it does not, HALT and ask the user for a valid range. - For **commit range**: verify the range resolves. If it does not, HALT and ask the user for a valid range.
- For **provided diff**: validate the content is non-empty and parseable as a unified diff. If it is not parseable, HALT and ask the user to provide a valid diff. - For **provided diff**: validate the content is non-empty and parseable as a unified diff. If it is not parseable, HALT and ask the user to provide a valid diff.
- For **file list**: validate each path exists in the working tree. Construct `{diff_output}` by running `git diff HEAD -- <path1> <path2> ...`. If any paths are untracked (new files not yet staged), use `git diff --no-index /dev/null <path>` to include them. If the diff is empty (files have no uncommitted changes and are not untracked), ask the user whether to review the full file contents or to specify a different baseline. - For **file list**: validate each path exists in the working tree. Construct `{diff_output}` by running `git diff HEAD -- <path1> <path2> ...`. If any paths are untracked (new files not yet staged), use `git diff --no-index /dev/null <path>` to include them. If the diff is empty (files have no uncommitted changes and are not untracked), ask the user whether to review the full file contents or to specify a different baseline.
- After constructing `{diff_output}`, verify it is non-empty regardless of source type. If empty, HALT and tell the user there is nothing to review. - After constructing `{diff_output}`, verify it is non-empty regardless of source type. If empty, HALT and tell the user there is nothing to review.
4. **Set the spec context.** 4. Ask the user: **Is there a spec or story file that provides context for these changes?**
- If `{spec_file}` is already set (from Tier 1 or Tier 2): verify the file exists and is readable, then set `{review_mode}` = `"full"`. - If yes: set `{spec_file}` to the path provided, verify the file exists and is readable, then set `{review_mode}` = `"full"`.
- Otherwise, ask the user: **Is there a spec or story file that provides context for these changes?** - If no: set `{review_mode}` = `"no-spec"`.
- If yes: set `{spec_file}` to the path provided, verify the file exists and is readable, then set `{review_mode}` = `"full"`.
- If no: set `{review_mode}` = `"no-spec"`.
5. If `{review_mode}` = `"full"` and the file at `{spec_file}` has a `context` field in its frontmatter listing additional docs, load each referenced document. Warn the user about any docs that cannot be found. 5. If `{review_mode}` = `"full"` and the file at `{spec_file}` has a `context` field in its frontmatter listing additional docs, load each referenced document. Warn the user about any docs that cannot be found.

View File

@ -32,19 +32,16 @@ Hand `{spec_file}` to a sub-agent/task and let it implement. If no sub-agents ar
### Tests ### Tests
**This is mandatory, not optional.** After implementation, write tests for every new or modified behavior. Follow the project's testing conventions discovered from `{project_context}`, CLAUDE.md, the existing test suite, and any configured test tooling. **This is mandatory, not optional.** After implementation, write tests for every new or modified behavior. Follow the project's testing conventions discovered from `{project_context}`, CLAUDE.md, or the existing test suite.
1. **Discover conventions.** Identify the project's test framework, file-naming patterns, and test directory structure in this order: 1. **Discover conventions.** Identify the project's test framework, file-naming patterns, and test directory structure from existing tests. If no existing tests exist, use the project's language-idiomatic defaults.
- First, inspect existing tests.
- If existing tests are missing or insufficient, inspect the repo's configured test tooling: test scripts, dependencies/devDependencies, and test config files (for example `package.json`, `pytest.ini`, `pyproject.toml`, `jest.config.*`, `vitest.config.*`, `mocha` config, `rspec` config, `go test` conventions, etc.).
- Only if neither existing tests nor configured tooling establish conventions should you fall back to the project's language-idiomatic defaults.
2. **Write tests.** Cover: 2. **Write tests.** Cover:
- Happy-path behavior for each new or changed feature. - Happy-path behavior for each new or changed feature.
- Edge cases and error scenarios from the I/O & Edge-Case Matrix (if present in the spec). - Edge cases and error scenarios from the I/O & Edge-Case Matrix (if present in the spec).
- Regressions — any behavior that could break due to the change. - Regressions — any behavior that could break due to the change.
3. **Run tests.** Execute the test suite. All new and existing tests must pass. If any test fails, fix the implementation or the test before proceeding. 3. **Run tests.** Execute the test suite. All new and existing tests must pass. If any test fails, fix the implementation or the test before proceeding.
If the project has no test infrastructure at all (no existing tests, no test framework, no relevant test config, no test directory, and no test scripts), note this in the spec under `## Verification` and skip — but this is the only acceptable reason to skip tests. Record this skip reason explicitly in the final summary output. If the project has no test infrastructure at all (no test framework, no test directory, no test scripts), note this in the spec under `## Verification` and skip — but this is the only acceptable reason to skip tests.
### Self-Check ### Self-Check

View File

@ -18,16 +18,13 @@ Implement the clarified intent directly.
### Tests ### Tests
**This is mandatory, not optional.** After implementation, write tests for every new or modified behavior. Follow the project's testing conventions discovered from `{project_context}`, CLAUDE.md, the existing test suite, and any configured test tooling. **This is mandatory, not optional.** After implementation, write tests for every new or modified behavior. Follow the project's testing conventions discovered from `{project_context}`, CLAUDE.md, or the existing test suite.
1. **Discover conventions.** Identify the project's test framework, file-naming patterns, and test directory structure in this order: 1. **Discover conventions.** Identify the project's test framework, file-naming patterns, and test directory structure from existing tests. If no existing tests exist, use the project's language-idiomatic defaults.
- First, inspect existing tests.
- If existing tests are missing or insufficient, inspect the repo's configured test tooling: test scripts, dependencies/devDependencies, and test config files (for example `package.json`, `pytest.ini`, `pyproject.toml`, `jest.config.*`, `vitest.config.*`, `mocha` config, `rspec` config, `go test` conventions, etc.).
- Only if neither existing tests nor configured tooling establish conventions should you fall back to the project's language-idiomatic defaults.
2. **Write tests.** Cover happy-path behavior, edge cases, and regressions. 2. **Write tests.** Cover happy-path behavior, edge cases, and regressions.
3. **Run tests.** All new and existing tests must pass. Fix failures before proceeding. 3. **Run tests.** All new and existing tests must pass. Fix failures before proceeding.
If the project has no test infrastructure at all (no existing tests, no test framework, no relevant test config, no test directory, and no test scripts), skip — but this is the only acceptable reason to skip tests. Record this skip reason explicitly in the final summary output. If the project has no test infrastructure at all (no test framework, no test directory, no test scripts), skip — but this is the only acceptable reason to skip tests.
### Review ### Review