mirror of
https://github.com/garrytan/gstack.git
synced 2026-05-11 07:17:12 +02:00
3e3843c4a9
* feat: contributor mode, session awareness, universal RECOMMENDATION format
- Rename {{UPDATE_CHECK}} → {{PREAMBLE}} across all 10 skill templates
- Add session tracking (touch ~/.gstack/sessions/$PPID, count active sessions)
- ELI16 mode when 3+ concurrent sessions detected (re-ground user on context)
- Contributor mode: auto-file field reports to ~/.gstack/contributor-logs/
- Universal AskUserQuestion format: context → question → RECOMMENDATION → options
- Update plan-ceo-review and plan-eng-review to reference preamble baseline
- Add vendored symlink awareness section to CLAUDE.md
- Rewrite CONTRIBUTING.md with contributor workflow and cross-project testing
- Add tests for contributor mode and session awareness in generated output
- Add E2E eval for contributor mode report filing
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
* feat: add Enum & Value Completeness to /review critical checklist
New CRITICAL review category that traces new enum values, status strings,
and type constants through every consumer outside the diff. Catches the
class of bugs where a new value is added but not handled in all switch/case
chains, allowlists, or frontend-backend contracts.
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
* chore: bump v0.4.1, user-facing changelog, update qa-only template and architecture docs
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
* docs: add CHANGELOG style guide — user-facing, sell the feature
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
* docs: rewrite v0.4.1 changelog to be user-facing and sell the features
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
* feat: add evals for RECOMMENDATION format, session awareness, and enum completeness
Free tests (Tier 1): RECOMMENDATION format + session awareness in all
preamble SKILL.md files, enum completeness checklist structure and CRITICAL
classification.
E2E eval: /review catches missed enum handlers when a new status value
is added but not handled in case/switch and notify methods.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
* feat: add E2E eval for session awareness ELI16 mode
Stubs _SESSIONS=4, gives agent a decision point on feature/add-payments
branch, verifies the output re-grounds the user with project, branch,
context, and RECOMMENDATION — the ELI16 mode behavior for 3+ sessions.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
* fix: contributor mode eval marked FAIL due to expected browse error
The test intentionally runs a nonexistent binary to trigger contributor
mode. The session runner's browse error detection catches "no such file
or directory...browse" and sets browseErrors, causing recordE2E to mark
passed=false. Override passed to check only exitReason since the browse
error is the expected scenario.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
132 lines
6.2 KiB
Cheetah
132 lines
6.2 KiB
Cheetah
---
|
|
name: review
|
|
version: 1.0.0
|
|
description: |
|
|
Pre-landing PR review. Analyzes diff against main for SQL safety, LLM trust
|
|
boundary violations, conditional side effects, and other structural issues.
|
|
allowed-tools:
|
|
- Bash
|
|
- Read
|
|
- Edit
|
|
- Write
|
|
- Grep
|
|
- Glob
|
|
- AskUserQuestion
|
|
---
|
|
|
|
{{PREAMBLE}}
|
|
|
|
# Pre-Landing PR Review
|
|
|
|
You are running the `/review` workflow. Analyze the current branch's diff against main for structural issues that tests don't catch.
|
|
|
|
---
|
|
|
|
## Step 1: Check branch
|
|
|
|
1. Run `git branch --show-current` to get the current branch.
|
|
2. If on `main`, output: **"Nothing to review — you're on main or have no changes against main."** and stop.
|
|
3. Run `git fetch origin main --quiet && git diff origin/main --stat` to check if there's a diff. If no diff, output the same message and stop.
|
|
|
|
---
|
|
|
|
## Step 2: Read the checklist
|
|
|
|
Read `.claude/skills/review/checklist.md`.
|
|
|
|
**If the file cannot be read, STOP and report the error.** Do not proceed without the checklist.
|
|
|
|
---
|
|
|
|
## Step 2.5: Check for Greptile review comments
|
|
|
|
Read `.claude/skills/review/greptile-triage.md` and follow the fetch, filter, classify, and **escalation detection** steps.
|
|
|
|
**If no PR exists, `gh` fails, API returns an error, or there are zero Greptile comments:** Skip this step silently. Greptile integration is additive — the review works without it.
|
|
|
|
**If Greptile comments are found:** Store the classifications (VALID & ACTIONABLE, VALID BUT ALREADY FIXED, FALSE POSITIVE, SUPPRESSED) — you will need them in Step 5.
|
|
|
|
---
|
|
|
|
## Step 3: Get the diff
|
|
|
|
Fetch the latest main to avoid false positives from a stale local main:
|
|
|
|
```bash
|
|
git fetch origin main --quiet
|
|
```
|
|
|
|
Run `git diff origin/main` to get the full diff. This includes both committed and uncommitted changes against the latest main.
|
|
|
|
---
|
|
|
|
## Step 4: Two-pass review
|
|
|
|
Apply the checklist against the diff in two passes:
|
|
|
|
1. **Pass 1 (CRITICAL):** SQL & Data Safety, Race Conditions & Concurrency, LLM Output Trust Boundary, Enum & Value Completeness
|
|
2. **Pass 2 (INFORMATIONAL):** Conditional Side Effects, Magic Numbers & String Coupling, Dead Code & Consistency, LLM Prompt Issues, Test Gaps, View/Frontend
|
|
|
|
**Enum & Value Completeness requires reading code OUTSIDE the diff.** When the diff introduces a new enum value, status, tier, or type constant, use Grep to find all files that reference sibling values, then Read those files to check if the new value is handled. This is the one category where within-diff review is insufficient.
|
|
|
|
Follow the output format specified in the checklist. Respect the suppressions — do NOT flag items listed in the "DO NOT flag" section.
|
|
|
|
---
|
|
|
|
## Step 5: Output findings
|
|
|
|
**Always output ALL findings** — both critical and informational. The user must see every issue.
|
|
|
|
- If CRITICAL issues found: output all findings, then for EACH critical issue use a separate AskUserQuestion with the problem, then `RECOMMENDATION: Choose A because [one-line reason]`, then options (A: Fix it now, B: Acknowledge, C: False positive — skip).
|
|
After all critical questions are answered, output a summary of what the user chose for each issue. If the user chose A (fix) on any issue, apply the recommended fixes. If only B/C were chosen, no action needed.
|
|
- If only non-critical issues found: output findings. No further action needed.
|
|
- If no issues found: output `Pre-Landing Review: No issues found.`
|
|
|
|
### Greptile comment resolution
|
|
|
|
After outputting your own findings, if Greptile comments were classified in Step 2.5:
|
|
|
|
**Include a Greptile summary in your output header:** `+ N Greptile comments (X valid, Y fixed, Z FP)`
|
|
|
|
Before replying to any comment, run the **Escalation Detection** algorithm from greptile-triage.md to determine whether to use Tier 1 (friendly) or Tier 2 (firm) reply templates.
|
|
|
|
1. **VALID & ACTIONABLE comments:** These are already included in your CRITICAL findings — they follow the same AskUserQuestion flow (A: Fix it now, B: Acknowledge, C: False positive). If the user chooses A (fix), reply using the **Fix reply template** from greptile-triage.md (include inline diff + explanation). If the user chooses C (false positive), reply using the **False Positive reply template** (include evidence + suggested re-rank), save to both per-project and global greptile-history.
|
|
|
|
2. **FALSE POSITIVE comments:** Present each one via AskUserQuestion:
|
|
- Show the Greptile comment: file:line (or [top-level]) + body summary + permalink URL
|
|
- Explain concisely why it's a false positive
|
|
- Options:
|
|
- A) Reply to Greptile explaining why this is incorrect (recommended if clearly wrong)
|
|
- B) Fix it anyway (if low-effort and harmless)
|
|
- C) Ignore — don't reply, don't fix
|
|
|
|
If the user chooses A, reply using the **False Positive reply template** from greptile-triage.md (include evidence + suggested re-rank), save to both per-project and global greptile-history.
|
|
|
|
3. **VALID BUT ALREADY FIXED comments:** Reply using the **Already Fixed reply template** from greptile-triage.md — no AskUserQuestion needed:
|
|
- Include what was done and the fixing commit SHA
|
|
- Save to both per-project and global greptile-history
|
|
|
|
4. **SUPPRESSED comments:** Skip silently — these are known false positives from previous triage.
|
|
|
|
---
|
|
|
|
## Step 5.5: TODOS cross-reference
|
|
|
|
Read `TODOS.md` in the repository root (if it exists). Cross-reference the PR against open TODOs:
|
|
|
|
- **Does this PR close any open TODOs?** If yes, note which items in your output: "This PR addresses TODO: <title>"
|
|
- **Does this PR create work that should become a TODO?** If yes, flag it as an informational finding.
|
|
- **Are there related TODOs that provide context for this review?** If yes, reference them when discussing related findings.
|
|
|
|
If TODOS.md doesn't exist, skip this step silently.
|
|
|
|
---
|
|
|
|
## Important Rules
|
|
|
|
- **Read the FULL diff before commenting.** Do not flag issues already addressed in the diff.
|
|
- **Read-only by default.** Only modify files if the user explicitly chooses "Fix it now" on a critical issue. Never commit, push, or create PRs.
|
|
- **Be terse.** One line problem, one line fix. No preamble.
|
|
- **Only flag real problems.** Skip anything that's fine.
|
|
- **Use Greptile reply templates from greptile-triage.md.** Every reply includes evidence. Never post vague replies.
|