feat(skill): union-of-scopes детект изменений для code-режима
- Зачем:
- типичный PR-флоу: ветка ahead of base + (часто) локальный твик поверх. Прежняя логика смотрела только unstaged/staged и пропускала коммиты ветки, давая пустое или неполное ревью.
- Что:
- Step 1 auto-detect: добавлен третий независимый scope `git diff <base>...HEAD` с резолвом base (`symbolic-ref refs/remotes/origin/HEAD` → `origin/main` → `origin/master` → `main` → `master`).
- Step 2 dispatch: briefing для `code` и `code-vs-plan` переписан с fallback на union трёх скоупов, явное "do not skip branch-vs-base because working tree is non-empty".
- reviewer-prompt.md: `{ARTIFACT}` placeholder теперь спеллует все три scope (a)(b)(c) с инструкцией про labeling mixed-state.
- README.md: таблица "Three modes" и пояснение синхронизированы с union-семантикой.
- Проверка:
- на ветке feat/cross-platform-receiving-review одновременно непустые `git diff` и `git diff origin/main...HEAD` — финальная формулировка прошла Codex review (3 раунда, VERDICT: APPROVED).
This commit is contained in:
@@ -73,14 +73,21 @@ runs without operator access conclude unverified.
|
|||||||
|
|
||||||
### Three modes
|
### Three modes
|
||||||
|
|
||||||
| Mode | What it reviews | When to use |
|
| Mode | What it reviews | When to use |
|
||||||
|----------------|------------------------------------|--------------------------|
|
|----------------|------------------------------------------|--------------------------|
|
||||||
| `plan` | Implementation plan | Before writing code |
|
| `plan` | Implementation plan | Before writing code |
|
||||||
| `code` | Git diff (unstaged, staged, branch)| After writing code |
|
| `code` | Working-tree, staged, and branch changes | After writing code |
|
||||||
| `code-vs-plan` | Code changes against the plan | Verify implementation |
|
| `code-vs-plan` | Code changes against the plan | Verify implementation |
|
||||||
|
|
||||||
Mode is auto-detected from context, or you can force it with an argument.
|
Mode is auto-detected from context, or you can force it with an argument.
|
||||||
|
|
||||||
|
For `code` mode, "branch changes" means commits on the current branch
|
||||||
|
since it diverged from its base (`main`, `master`, or the remote
|
||||||
|
default). The three scopes (unstaged, staged, branch-vs-base) are
|
||||||
|
reviewed as a union, not a fallback chain — so the typical PR case
|
||||||
|
(committed changes on the branch plus an in-progress local tweak) is
|
||||||
|
covered without an explicit flag.
|
||||||
|
|
||||||
### Language
|
### Language
|
||||||
|
|
||||||
The skill detects the operator's session language from recent
|
The skill detects the operator's session language from recent
|
||||||
|
|||||||
@@ -56,9 +56,20 @@ message "Plan mode is active" → mode = `plan`, skip git.
|
|||||||
|
|
||||||
**3. Auto-detect** (no explicit argument, not in Plan Mode):
|
**3. Auto-detect** (no explicit argument, not in Plan Mode):
|
||||||
|
|
||||||
1. Check for code changes (any non-empty output means changes exist):
|
1. Check for code changes — there are **three independent scopes**, and
|
||||||
|
any non-empty output from any of them means code changes exist:
|
||||||
- `git diff --name-only` — unstaged
|
- `git diff --name-only` — unstaged
|
||||||
- `git diff --cached --name-only` — staged
|
- `git diff --cached --name-only` — staged
|
||||||
|
- `git diff --name-only <base>...HEAD` — committed on the current
|
||||||
|
branch since it diverged from its base. Resolve `<base>` by trying
|
||||||
|
in order: `git symbolic-ref refs/remotes/origin/HEAD` (canonical
|
||||||
|
remote default), then `origin/main`, `origin/master`, `main`,
|
||||||
|
`master`. If none resolve, skip this check.
|
||||||
|
|
||||||
|
The three scopes are independent, not a fallback chain. The typical
|
||||||
|
PR has commits on the branch *and* often an in-progress local tweak
|
||||||
|
on top; the briefing in Step 2 reviews the union, not just the first
|
||||||
|
non-empty one.
|
||||||
2. Check if a **structured plan artifact** exists in the current
|
2. Check if a **structured plan artifact** exists in the current
|
||||||
conversation context. This means one of:
|
conversation context. This means one of:
|
||||||
- The user pasted or referenced a markdown plan document with
|
- The user pasted or referenced a markdown plan document with
|
||||||
@@ -112,8 +123,16 @@ commands, and reads files on its own. **Do not pre-collect diffs or file
|
|||||||
lists for it.** Pass mode-specific context only:
|
lists for it.** Pass mode-specific context only:
|
||||||
|
|
||||||
- For `plan` — path to plan file, or inline plan text
|
- For `plan` — path to plan file, or inline plan text
|
||||||
- For `code` — instruct it to use git status / git diff
|
- For `code` — instruct the reviewer to review the **union of three
|
||||||
- For `code-vs-plan` — pass the plan and instruct git lookup for changes
|
scopes**: unstaged (`git diff`), staged (`git diff --cached`), and
|
||||||
|
committed since base (`git diff <base>...HEAD`, base resolved the
|
||||||
|
same way as in Step 1). Branch-vs-base is *not* a fallback — do not
|
||||||
|
skip it just because the working tree is non-empty. The typical PR
|
||||||
|
has both. When scopes are mixed, the reviewer should label findings
|
||||||
|
by scope where it matters and surface the mixed-state to the
|
||||||
|
operator.
|
||||||
|
- For `code-vs-plan` — pass the plan and instruct the same union git
|
||||||
|
lookup as `code` (all three scopes, not a fallback chain)
|
||||||
|
|
||||||
Fill the placeholders in `reviewer-prompt.md` and pass the result as the
|
Fill the placeholders in `reviewer-prompt.md` and pass the result as the
|
||||||
subagent's prompt. The template includes operating stance, finding bar,
|
subagent's prompt. The template includes operating stance, finding bar,
|
||||||
|
|||||||
+22
-2
@@ -27,8 +27,28 @@ is a real weakness.
|
|||||||
|
|
||||||
{ARTIFACT}
|
{ARTIFACT}
|
||||||
# For mode=plan: path to plan file, OR full inline plan text
|
# For mode=plan: path to plan file, OR full inline plan text
|
||||||
# For mode=code: "Use git status / git diff to find changes."
|
# For mode=code: "Find the changes via git. Review the UNION of
|
||||||
# For mode=code-vs-plan: plan path or inline plan + "use git for code changes"
|
# three independent scopes — do not stop after
|
||||||
|
# the first non-empty one:
|
||||||
|
# (a) unstaged: `git diff`
|
||||||
|
# (b) staged: `git diff --cached`
|
||||||
|
# (c) branch-vs-base: resolve <base> by
|
||||||
|
# trying `git symbolic-ref
|
||||||
|
# refs/remotes/origin/HEAD`, then
|
||||||
|
# `origin/main`, `origin/master`, `main`,
|
||||||
|
# `master`; then
|
||||||
|
# `git diff <base>...HEAD` plus
|
||||||
|
# `git log <base>..HEAD` for commit context.
|
||||||
|
# Do not skip (c) because (a) or (b) are
|
||||||
|
# non-empty — the typical PR has commits on the
|
||||||
|
# branch and an in-progress local tweak on top,
|
||||||
|
# and the operator is asking about both. If the
|
||||||
|
# scopes are mixed (e.g., committed changes plus
|
||||||
|
# unstaged ones), label findings by scope where
|
||||||
|
# it matters and surface the mixed state in the
|
||||||
|
# summary."
|
||||||
|
# For mode=code-vs-plan: plan path or inline plan + the same union git
|
||||||
|
# lookup instruction as mode=code
|
||||||
|
|
||||||
## Round
|
## Round
|
||||||
|
|
||||||
|
|||||||
Reference in New Issue
Block a user