mirror of
https://github.com/garrytan/gstack.git
synced 2026-07-07 08:37:52 +02:00
fix: replace multi-line eval+mkdir+echo blocks with atomic helpers
- Review log writes now use gstack-review-log (single command) - Review dashboard reads now use gstack-review-read (single command) - Remaining source+mkdir blocks use && chaining for variable persistence - Regenerated all SKILL.md files Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
This commit is contained in:
Garry Tan
+16
-24
@@ -190,7 +190,7 @@ Do NOT make any code changes. Do NOT start implementation. Your only job right n
|
||||
|
||||
## Prime Directives
|
||||
1. Zero silent failures. Every failure mode must be visible — to the system, to the team, to the user. If a failure can happen silently, that is a critical defect in the plan.
|
||||
2. Every error has a name. Don't say "handle errors." Name the specific exception class, what triggers it, what rescues it, what the user sees, and whether it's tested. rescue StandardError is a code smell — call it out.
|
||||
2. Every error has a name. Don't say "handle errors." Name the specific exception class, what triggers it, what catches it, what the user sees, and whether it's tested. Catch-all error handling (e.g., catch Exception, rescue StandardError, except Exception) is a code smell — call it out.
|
||||
3. Data flows have shadow paths. Every data flow has a happy path and three shadow paths: nil input, empty/zero-length input, and upstream error. Trace all four for every new flow.
|
||||
4. Interactions have edge cases. Every user-visible interaction has edge cases: double-click, navigate-away-mid-action, slow connection, stale state, back button. Map them.
|
||||
5. Observability is scope, not afterthought. New dashboards, alerts, and runbooks are first-class deliverables, not post-launch cleanup items.
|
||||
@@ -248,8 +248,8 @@ Run the following commands:
|
||||
git log --oneline -30 # Recent history
|
||||
git diff <base> --stat # What's already changed
|
||||
git stash list # Any stashed work
|
||||
grep -r "TODO\|FIXME\|HACK\|XXX" --include="*.rb" --include="*.js" -l
|
||||
find . -name "*.rb" -newer Gemfile.lock | head -20 # Recently touched files
|
||||
grep -r "TODO\|FIXME\|HACK\|XXX" -l --exclude-dir=node_modules --exclude-dir=vendor --exclude-dir=.git . | head -30
|
||||
git log --since=30.days --name-only --format="" | sort | uniq -c | sort -rn | head -20 # Recently touched files
|
||||
```
|
||||
Then read CLAUDE.md, TODOS.md, and any existing architecture docs.
|
||||
|
||||
@@ -362,8 +362,7 @@ Rules:
|
||||
After the opt-in/cherry-pick ceremony, write the plan to disk so the vision and decisions survive beyond this conversation. Only run this step for EXPANSION and SELECTIVE EXPANSION modes.
|
||||
|
||||
```bash
|
||||
source <(~/.claude/skills/gstack/bin/gstack-slug 2>/dev/null)
|
||||
mkdir -p ~/.gstack/projects/$SLUG/ceo-plans
|
||||
source <(~/.claude/skills/gstack/bin/gstack-slug 2>/dev/null) && mkdir -p ~/.gstack/projects/$SLUG/ceo-plans
|
||||
```
|
||||
|
||||
Before writing, check for existing CEO plans in the ceo-plans/ directory. If any are >30 days old or their branch has been merged/deleted, offer to archive them:
|
||||
@@ -478,24 +477,24 @@ For every new method, service, or codepath that can fail, fill in this table:
|
||||
```
|
||||
METHOD/CODEPATH | WHAT CAN GO WRONG | EXCEPTION CLASS
|
||||
-------------------------|-----------------------------|-----------------
|
||||
ExampleService#call | API timeout | Faraday::TimeoutError
|
||||
ExampleService#call | API timeout | TimeoutError
|
||||
| API returns 429 | RateLimitError
|
||||
| API returns malformed JSON | JSON::ParserError
|
||||
| DB connection pool exhausted| ActiveRecord::ConnectionTimeoutError
|
||||
| Record not found | ActiveRecord::RecordNotFound
|
||||
| API returns malformed JSON | JSONParseError
|
||||
| DB connection pool exhausted| ConnectionPoolExhausted
|
||||
| Record not found | RecordNotFound
|
||||
-------------------------|-----------------------------|-----------------
|
||||
|
||||
EXCEPTION CLASS | RESCUED? | RESCUE ACTION | USER SEES
|
||||
-----------------------------|-----------|------------------------|------------------
|
||||
Faraday::TimeoutError | Y | Retry 2x, then raise | "Service temporarily unavailable"
|
||||
TimeoutError | Y | Retry 2x, then raise | "Service temporarily unavailable"
|
||||
RateLimitError | Y | Backoff + retry | Nothing (transparent)
|
||||
JSON::ParserError | N ← GAP | — | 500 error ← BAD
|
||||
ConnectionTimeoutError | N ← GAP | — | 500 error ← BAD
|
||||
ActiveRecord::RecordNotFound | Y | Return nil, log warning | "Not found" message
|
||||
JSONParseError | N ← GAP | — | 500 error ← BAD
|
||||
ConnectionPoolExhausted | N ← GAP | — | 500 error ← BAD
|
||||
RecordNotFound | Y | Return nil, log warning | "Not found" message
|
||||
```
|
||||
Rules for this section:
|
||||
* `rescue StandardError` is ALWAYS a smell. Name the specific exceptions.
|
||||
* `rescue => e` with only `Rails.logger.error(e.message)` is insufficient. Log the full context: what was being attempted, with what arguments, for what user/request.
|
||||
* Catch-all error handling (`rescue StandardError`, `catch (Exception e)`, `except Exception`) is ALWAYS a smell. Name the specific exceptions.
|
||||
* Catching an error with only a generic log message is insufficient. Log the full context: what was being attempted, with what arguments, for what user/request.
|
||||
* Every rescued error must either: retry with backoff, degrade gracefully with a user-visible message, or re-raise with added context. "Swallow and continue" is almost never acceptable.
|
||||
* For each GAP (unrescued error that should be rescued): specify the rescue action and what the user should see.
|
||||
* For LLM/AI service calls specifically: what happens when the response is malformed? When it's empty? When it hallucinates invalid JSON? When the model returns a refusal? Each of these is a distinct failure mode.
|
||||
@@ -792,9 +791,7 @@ If any AskUserQuestion goes unanswered, note it here. Never silently default.
|
||||
After producing the Completion Summary above, persist the review result:
|
||||
|
||||
```bash
|
||||
source <(~/.claude/skills/gstack/bin/gstack-slug 2>/dev/null)
|
||||
mkdir -p ~/.gstack/projects/$SLUG
|
||||
echo '{"skill":"plan-ceo-review","timestamp":"TIMESTAMP","status":"STATUS","unresolved":N,"critical_gaps":N,"mode":"MODE","commit":"COMMIT"}' >> ~/.gstack/projects/$SLUG/$BRANCH-reviews.jsonl
|
||||
~/.claude/skills/gstack/bin/gstack-review-log '{"skill":"plan-ceo-review","timestamp":"TIMESTAMP","status":"STATUS","unresolved":N,"critical_gaps":N,"mode":"MODE","commit":"COMMIT"}'
|
||||
```
|
||||
|
||||
Before running this command, substitute the placeholder values from the Completion Summary you just produced:
|
||||
@@ -810,12 +807,7 @@ Before running this command, substitute the placeholder values from the Completi
|
||||
After completing the review, read the review log and config to display the dashboard.
|
||||
|
||||
```bash
|
||||
source <(~/.claude/skills/gstack/bin/gstack-slug 2>/dev/null)
|
||||
cat ~/.gstack/projects/$SLUG/$BRANCH-reviews.jsonl 2>/dev/null || echo "NO_REVIEWS"
|
||||
echo "---CONFIG---"
|
||||
~/.claude/skills/gstack/bin/gstack-config get skip_eng_review 2>/dev/null || echo "false"
|
||||
echo "---HEAD---"
|
||||
git rev-parse --short HEAD 2>/dev/null || echo "unknown"
|
||||
~/.claude/skills/gstack/bin/gstack-review-read
|
||||
```
|
||||
|
||||
Parse the output. Find the most recent entry for each skill (plan-ceo-review, plan-eng-review, plan-design-review, design-review-lite, codex-review). Ignore entries with timestamps older than 7 days. For Design Review, show whichever is more recent between `plan-design-review` (full visual audit) and `design-review-lite` (code-level check). Append "(FULL)" or "(LITE)" to the status to distinguish. Display:
|
||||
|
||||
@@ -37,7 +37,7 @@ Do NOT make any code changes. Do NOT start implementation. Your only job right n
|
||||
|
||||
## Prime Directives
|
||||
1. Zero silent failures. Every failure mode must be visible — to the system, to the team, to the user. If a failure can happen silently, that is a critical defect in the plan.
|
||||
2. Every error has a name. Don't say "handle errors." Name the specific exception class, what triggers it, what rescues it, what the user sees, and whether it's tested. rescue StandardError is a code smell — call it out.
|
||||
2. Every error has a name. Don't say "handle errors." Name the specific exception class, what triggers it, what catches it, what the user sees, and whether it's tested. Catch-all error handling (e.g., catch Exception, rescue StandardError, except Exception) is a code smell — call it out.
|
||||
3. Data flows have shadow paths. Every data flow has a happy path and three shadow paths: nil input, empty/zero-length input, and upstream error. Trace all four for every new flow.
|
||||
4. Interactions have edge cases. Every user-visible interaction has edge cases: double-click, navigate-away-mid-action, slow connection, stale state, back button. Map them.
|
||||
5. Observability is scope, not afterthought. New dashboards, alerts, and runbooks are first-class deliverables, not post-launch cleanup items.
|
||||
@@ -95,8 +95,8 @@ Run the following commands:
|
||||
git log --oneline -30 # Recent history
|
||||
git diff <base> --stat # What's already changed
|
||||
git stash list # Any stashed work
|
||||
grep -r "TODO\|FIXME\|HACK\|XXX" --include="*.rb" --include="*.js" -l
|
||||
find . -name "*.rb" -newer Gemfile.lock | head -20 # Recently touched files
|
||||
grep -r "TODO\|FIXME\|HACK\|XXX" -l --exclude-dir=node_modules --exclude-dir=vendor --exclude-dir=.git . | head -30
|
||||
git log --since=30.days --name-only --format="" | sort | uniq -c | sort -rn | head -20 # Recently touched files
|
||||
```
|
||||
Then read CLAUDE.md, TODOS.md, and any existing architecture docs.
|
||||
|
||||
@@ -209,8 +209,7 @@ Rules:
|
||||
After the opt-in/cherry-pick ceremony, write the plan to disk so the vision and decisions survive beyond this conversation. Only run this step for EXPANSION and SELECTIVE EXPANSION modes.
|
||||
|
||||
```bash
|
||||
source <(~/.claude/skills/gstack/bin/gstack-slug 2>/dev/null)
|
||||
mkdir -p ~/.gstack/projects/$SLUG/ceo-plans
|
||||
source <(~/.claude/skills/gstack/bin/gstack-slug 2>/dev/null) && mkdir -p ~/.gstack/projects/$SLUG/ceo-plans
|
||||
```
|
||||
|
||||
Before writing, check for existing CEO plans in the ceo-plans/ directory. If any are >30 days old or their branch has been merged/deleted, offer to archive them:
|
||||
@@ -325,24 +324,24 @@ For every new method, service, or codepath that can fail, fill in this table:
|
||||
```
|
||||
METHOD/CODEPATH | WHAT CAN GO WRONG | EXCEPTION CLASS
|
||||
-------------------------|-----------------------------|-----------------
|
||||
ExampleService#call | API timeout | Faraday::TimeoutError
|
||||
ExampleService#call | API timeout | TimeoutError
|
||||
| API returns 429 | RateLimitError
|
||||
| API returns malformed JSON | JSON::ParserError
|
||||
| DB connection pool exhausted| ActiveRecord::ConnectionTimeoutError
|
||||
| Record not found | ActiveRecord::RecordNotFound
|
||||
| API returns malformed JSON | JSONParseError
|
||||
| DB connection pool exhausted| ConnectionPoolExhausted
|
||||
| Record not found | RecordNotFound
|
||||
-------------------------|-----------------------------|-----------------
|
||||
|
||||
EXCEPTION CLASS | RESCUED? | RESCUE ACTION | USER SEES
|
||||
-----------------------------|-----------|------------------------|------------------
|
||||
Faraday::TimeoutError | Y | Retry 2x, then raise | "Service temporarily unavailable"
|
||||
TimeoutError | Y | Retry 2x, then raise | "Service temporarily unavailable"
|
||||
RateLimitError | Y | Backoff + retry | Nothing (transparent)
|
||||
JSON::ParserError | N ← GAP | — | 500 error ← BAD
|
||||
ConnectionTimeoutError | N ← GAP | — | 500 error ← BAD
|
||||
ActiveRecord::RecordNotFound | Y | Return nil, log warning | "Not found" message
|
||||
JSONParseError | N ← GAP | — | 500 error ← BAD
|
||||
ConnectionPoolExhausted | N ← GAP | — | 500 error ← BAD
|
||||
RecordNotFound | Y | Return nil, log warning | "Not found" message
|
||||
```
|
||||
Rules for this section:
|
||||
* `rescue StandardError` is ALWAYS a smell. Name the specific exceptions.
|
||||
* `rescue => e` with only `Rails.logger.error(e.message)` is insufficient. Log the full context: what was being attempted, with what arguments, for what user/request.
|
||||
* Catch-all error handling (`rescue StandardError`, `catch (Exception e)`, `except Exception`) is ALWAYS a smell. Name the specific exceptions.
|
||||
* Catching an error with only a generic log message is insufficient. Log the full context: what was being attempted, with what arguments, for what user/request.
|
||||
* Every rescued error must either: retry with backoff, degrade gracefully with a user-visible message, or re-raise with added context. "Swallow and continue" is almost never acceptable.
|
||||
* For each GAP (unrescued error that should be rescued): specify the rescue action and what the user should see.
|
||||
* For LLM/AI service calls specifically: what happens when the response is malformed? When it's empty? When it hallucinates invalid JSON? When the model returns a refusal? Each of these is a distinct failure mode.
|
||||
@@ -639,9 +638,7 @@ If any AskUserQuestion goes unanswered, note it here. Never silently default.
|
||||
After producing the Completion Summary above, persist the review result:
|
||||
|
||||
```bash
|
||||
source <(~/.claude/skills/gstack/bin/gstack-slug 2>/dev/null)
|
||||
mkdir -p ~/.gstack/projects/$SLUG
|
||||
echo '{"skill":"plan-ceo-review","timestamp":"TIMESTAMP","status":"STATUS","unresolved":N,"critical_gaps":N,"mode":"MODE","commit":"COMMIT"}' >> ~/.gstack/projects/$SLUG/$BRANCH-reviews.jsonl
|
||||
~/.claude/skills/gstack/bin/gstack-review-log '{"skill":"plan-ceo-review","timestamp":"TIMESTAMP","status":"STATUS","unresolved":N,"critical_gaps":N,"mode":"MODE","commit":"COMMIT"}'
|
||||
```
|
||||
|
||||
Before running this command, substitute the placeholder values from the Completion Summary you just produced:
|
||||
|
||||
Reference in New Issue
Block a user