mirror of
https://github.com/garrytan/gstack.git
synced 2026-05-02 11:45:20 +02:00
fix: review fixes — ETHOS.md, runtime root, repo-local guard, kiro assets, upgrade paths
Paranoid 4-pass review found 7 issues, all fixed: - Add ETHOS.md to create_codex_runtime_root - Clean old real dirs (not just symlinks) on upgrade - Skip runtime root for repo-local installs (prevent self-referential symlinks) - Add review/, ETHOS.md, gstack-upgrade/ to Kiro install - Update gstack-upgrade to detect ~/.gstack/repos/ and .agents/skills/ - Guard --host without value from silent exit - Fix Kiro sed patterns + timeout instruction in gen-skill-docs.ts Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
This commit is contained in:
Garry Tan
@@ -12,23 +12,28 @@ description: |
|
||||
## Preamble (run first)
|
||||
|
||||
```bash
|
||||
_UPD=$(~/.codex/skills/gstack/bin/gstack-update-check 2>/dev/null || .agents/skills/gstack/bin/gstack-update-check 2>/dev/null || true)
|
||||
_ROOT=$(git rev-parse --show-toplevel 2>/dev/null)
|
||||
GSTACK_ROOT="$HOME/.codex/skills/gstack"
|
||||
[ -n "$_ROOT" ] && [ -d "$_ROOT/.agents/skills/gstack" ] && GSTACK_ROOT="$_ROOT/.agents/skills/gstack"
|
||||
GSTACK_BIN="$GSTACK_ROOT/bin"
|
||||
GSTACK_BROWSE="$GSTACK_ROOT/browse/dist"
|
||||
_UPD=$($GSTACK_BIN/gstack-update-check 2>/dev/null || .agents/skills/gstack/bin/gstack-update-check 2>/dev/null || true)
|
||||
[ -n "$_UPD" ] && echo "$_UPD" || true
|
||||
mkdir -p ~/.gstack/sessions
|
||||
touch ~/.gstack/sessions/"$PPID"
|
||||
_SESSIONS=$(find ~/.gstack/sessions -mmin -120 -type f 2>/dev/null | wc -l | tr -d ' ')
|
||||
find ~/.gstack/sessions -mmin +120 -type f -delete 2>/dev/null || true
|
||||
_CONTRIB=$(~/.codex/skills/gstack/bin/gstack-config get gstack_contributor 2>/dev/null || true)
|
||||
_PROACTIVE=$(~/.codex/skills/gstack/bin/gstack-config get proactive 2>/dev/null || echo "true")
|
||||
_CONTRIB=$($GSTACK_BIN/gstack-config get gstack_contributor 2>/dev/null || true)
|
||||
_PROACTIVE=$($GSTACK_BIN/gstack-config get proactive 2>/dev/null || echo "true")
|
||||
_BRANCH=$(git branch --show-current 2>/dev/null || echo "unknown")
|
||||
echo "BRANCH: $_BRANCH"
|
||||
echo "PROACTIVE: $_PROACTIVE"
|
||||
source <(~/.codex/skills/gstack/bin/gstack-repo-mode 2>/dev/null) || true
|
||||
source <($GSTACK_BIN/gstack-repo-mode 2>/dev/null) || true
|
||||
REPO_MODE=${REPO_MODE:-unknown}
|
||||
echo "REPO_MODE: $REPO_MODE"
|
||||
_LAKE_SEEN=$([ -f ~/.gstack/.completeness-intro-seen ] && echo "yes" || echo "no")
|
||||
echo "LAKE_INTRO: $_LAKE_SEEN"
|
||||
_TEL=$(~/.codex/skills/gstack/bin/gstack-config get telemetry 2>/dev/null || true)
|
||||
_TEL=$($GSTACK_BIN/gstack-config get telemetry 2>/dev/null || true)
|
||||
_TEL_PROMPTED=$([ -f ~/.gstack/.telemetry-prompted ] && echo "yes" || echo "no")
|
||||
_TEL_START=$(date +%s)
|
||||
_SESSION_ID="$$-$(date +%s)"
|
||||
@@ -36,13 +41,13 @@ echo "TELEMETRY: ${_TEL:-off}"
|
||||
echo "TEL_PROMPTED: $_TEL_PROMPTED"
|
||||
mkdir -p ~/.gstack/analytics
|
||||
echo '{"skill":"cso","ts":"'$(date -u +%Y-%m-%dT%H:%M:%SZ)'","repo":"'$(basename "$(git rev-parse --show-toplevel 2>/dev/null)" 2>/dev/null || echo "unknown")'"}' >> ~/.gstack/analytics/skill-usage.jsonl 2>/dev/null || true
|
||||
for _PF in ~/.gstack/analytics/.pending-*; do [ -f "$_PF" ] && ~/.codex/skills/gstack/bin/gstack-telemetry-log --event-type skill_run --skill _pending_finalize --outcome unknown --session-id "$_SESSION_ID" 2>/dev/null || true; break; done
|
||||
for _PF in ~/.gstack/analytics/.pending-*; do [ -f "$_PF" ] && $GSTACK_BIN/gstack-telemetry-log --event-type skill_run --skill _pending_finalize --outcome unknown --session-id "$_SESSION_ID" 2>/dev/null || true; break; done
|
||||
```
|
||||
|
||||
If `PROACTIVE` is `"false"`, do not proactively suggest gstack skills — only invoke
|
||||
them when the user explicitly asks. The user opted out of proactive suggestions.
|
||||
|
||||
If output shows `UPGRADE_AVAILABLE <old> <new>`: read `~/.codex/skills/gstack/gstack-upgrade/SKILL.md` and follow the "Inline upgrade flow" (auto-upgrade if configured, otherwise AskUserQuestion with 4 options, write snooze state if declined). If `JUST_UPGRADED <from> <to>`: tell user "Running gstack v{to} (just updated!)" and continue.
|
||||
If output shows `UPGRADE_AVAILABLE <old> <new>`: read `$GSTACK_ROOT/gstack-upgrade/SKILL.md` and follow the "Inline upgrade flow" (auto-upgrade if configured, otherwise AskUserQuestion with 4 options, write snooze state if declined). If `JUST_UPGRADED <from> <to>`: tell user "Running gstack v{to} (just updated!)" and continue.
|
||||
|
||||
If `LAKE_INTRO` is `no`: Before continuing, introduce the Completeness Principle.
|
||||
Tell the user: "gstack follows the **Boil the Lake** principle — always do the complete
|
||||
@@ -68,7 +73,7 @@ Options:
|
||||
- A) Help gstack get better! (recommended)
|
||||
- B) No thanks
|
||||
|
||||
If A: run `~/.codex/skills/gstack/bin/gstack-config set telemetry community`
|
||||
If A: run `$GSTACK_BIN/gstack-config set telemetry community`
|
||||
|
||||
If B: ask a follow-up AskUserQuestion:
|
||||
|
||||
@@ -79,8 +84,8 @@ Options:
|
||||
- A) Sure, anonymous is fine
|
||||
- B) No thanks, fully off
|
||||
|
||||
If B→A: run `~/.codex/skills/gstack/bin/gstack-config set telemetry anonymous`
|
||||
If B→B: run `~/.codex/skills/gstack/bin/gstack-config set telemetry off`
|
||||
If B→A: run `$GSTACK_BIN/gstack-config set telemetry anonymous`
|
||||
If B→B: run `$GSTACK_BIN/gstack-config set telemetry off`
|
||||
|
||||
Always run:
|
||||
```bash
|
||||
@@ -140,7 +145,7 @@ Never let a noticed issue silently pass. The whole point is proactive communicat
|
||||
|
||||
## Search Before Building
|
||||
|
||||
Before building infrastructure, unfamiliar patterns, or anything the runtime might have a built-in — **search first.** Read `~/.codex/skills/gstack/ETHOS.md` for the full philosophy.
|
||||
Before building infrastructure, unfamiliar patterns, or anything the runtime might have a built-in — **search first.** Read `$GSTACK_ROOT/ETHOS.md` for the full philosophy.
|
||||
|
||||
**Three layers of knowledge:**
|
||||
- **Layer 1** (tried and true — in distribution). Don't reinvent the wheel. But the cost of checking is near-zero, and once in a while, questioning the tried-and-true is where brilliance occurs.
|
||||
@@ -238,7 +243,7 @@ Run this bash:
|
||||
_TEL_END=$(date +%s)
|
||||
_TEL_DUR=$(( _TEL_END - _TEL_START ))
|
||||
rm -f ~/.gstack/analytics/.pending-"$_SESSION_ID" 2>/dev/null || true
|
||||
~/.codex/skills/gstack/bin/gstack-telemetry-log \
|
||||
$GSTACK_ROOT/bin/gstack-telemetry-log \
|
||||
--skill "SKILL_NAME" --duration "$_TEL_DUR" --outcome "OUTCOME" \
|
||||
--used-browse "USED_BROWSE" --session-id "$_SESSION_ID" 2>/dev/null &
|
||||
```
|
||||
|
||||
@@ -79,9 +79,15 @@ Continue with the current skill.
|
||||
if [ -d "$HOME/.claude/skills/gstack/.git" ]; then
|
||||
INSTALL_TYPE="global-git"
|
||||
INSTALL_DIR="$HOME/.claude/skills/gstack"
|
||||
elif [ -d "$HOME/.gstack/repos/gstack/.git" ]; then
|
||||
INSTALL_TYPE="global-git"
|
||||
INSTALL_DIR="$HOME/.gstack/repos/gstack"
|
||||
elif [ -d ".claude/skills/gstack/.git" ]; then
|
||||
INSTALL_TYPE="local-git"
|
||||
INSTALL_DIR=".claude/skills/gstack"
|
||||
elif [ -d ".agents/skills/gstack/.git" ]; then
|
||||
INSTALL_TYPE="local-git"
|
||||
INSTALL_DIR=".agents/skills/gstack"
|
||||
elif [ -d ".claude/skills/gstack" ]; then
|
||||
INSTALL_TYPE="vendored"
|
||||
INSTALL_DIR=".claude/skills/gstack"
|
||||
|
||||
@@ -77,9 +77,15 @@ Continue with the current skill.
|
||||
if [ -d "$HOME/.claude/skills/gstack/.git" ]; then
|
||||
INSTALL_TYPE="global-git"
|
||||
INSTALL_DIR="$HOME/.claude/skills/gstack"
|
||||
elif [ -d "$HOME/.gstack/repos/gstack/.git" ]; then
|
||||
INSTALL_TYPE="global-git"
|
||||
INSTALL_DIR="$HOME/.gstack/repos/gstack"
|
||||
elif [ -d ".claude/skills/gstack/.git" ]; then
|
||||
INSTALL_TYPE="local-git"
|
||||
INSTALL_DIR=".claude/skills/gstack"
|
||||
elif [ -d ".agents/skills/gstack/.git" ]; then
|
||||
INSTALL_TYPE="local-git"
|
||||
INSTALL_DIR=".agents/skills/gstack"
|
||||
elif [ -d ".claude/skills/gstack" ]; then
|
||||
INSTALL_TYPE="vendored"
|
||||
INSTALL_DIR=".claude/skills/gstack"
|
||||
|
||||
+2
-2
@@ -745,7 +745,7 @@ TMPERR_ADV=$(mktemp /tmp/codex-adv-XXXXXXXX)
|
||||
codex exec "Review the changes on this branch against the base branch. Run git diff origin/<base> to see the diff. Your job is to find ways this code will fail in production. Think like an attacker and a chaos engineer. Find edge cases, race conditions, security holes, resource leaks, failure modes, and silent data corruption paths. Be adversarial. Be thorough. No compliments — just the problems." -s read-only -c 'model_reasoning_effort="xhigh"' --enable web_search_cached 2>"$TMPERR_ADV"
|
||||
```
|
||||
|
||||
Use a 5-minute timeout (`timeout: 300000`). After the command completes, read stderr:
|
||||
Set the Bash tool's `timeout` parameter to `300000` (5 minutes). Do NOT use the `timeout` shell command — it doesn't exist on macOS. After the command completes, read stderr:
|
||||
```bash
|
||||
cat "$TMPERR_ADV"
|
||||
```
|
||||
@@ -790,7 +790,7 @@ TMPERR=$(mktemp /tmp/codex-review-XXXXXXXX)
|
||||
codex review --base <base> -c 'model_reasoning_effort="xhigh"' --enable web_search_cached 2>"$TMPERR"
|
||||
```
|
||||
|
||||
Use a 5-minute timeout. Present output under `CODEX SAYS (code review):` header.
|
||||
Set the Bash tool's `timeout` parameter to `300000` (5 minutes). Do NOT use the `timeout` shell command — it doesn't exist on macOS. Present output under `CODEX SAYS (code review):` header.
|
||||
Check for `[P1]` markers: found → `GATE: FAIL`, not found → `GATE: PASS`.
|
||||
|
||||
If GATE is FAIL, use AskUserQuestion:
|
||||
|
||||
@@ -2044,7 +2044,7 @@ TMPERR_ADV=$(mktemp /tmp/codex-adv-XXXXXXXX)
|
||||
codex exec "Review the changes on this branch against the base branch. Run git diff origin/<base> to see the diff. Your job is to find ways this code will fail in production. Think like an attacker and a chaos engineer. Find edge cases, race conditions, security holes, resource leaks, failure modes, and silent data corruption paths. Be adversarial. Be thorough. No compliments — just the problems." -s read-only -c 'model_reasoning_effort="xhigh"' --enable web_search_cached 2>"$TMPERR_ADV"
|
||||
\`\`\`
|
||||
|
||||
Use a 5-minute timeout (\`timeout: 300000\`). After the command completes, read stderr:
|
||||
Set the Bash tool's \`timeout\` parameter to \`300000\` (5 minutes). Do NOT use the \`timeout\` shell command — it doesn't exist on macOS. After the command completes, read stderr:
|
||||
\`\`\`bash
|
||||
cat "$TMPERR_ADV"
|
||||
\`\`\`
|
||||
@@ -2089,7 +2089,7 @@ TMPERR=$(mktemp /tmp/codex-review-XXXXXXXX)
|
||||
codex review --base <base> -c 'model_reasoning_effort="xhigh"' --enable web_search_cached 2>"$TMPERR"
|
||||
\`\`\`
|
||||
|
||||
Use a 5-minute timeout. Present output under \`CODEX SAYS (code review):\` header.
|
||||
Set the Bash tool's \`timeout\` parameter to \`300000\` (5 minutes). Do NOT use the \`timeout\` shell command — it doesn't exist on macOS. Present output under \`CODEX SAYS (code review):\` header.
|
||||
Check for \`[P1]\` markers: found → \`GATE: FAIL\`, not found → \`GATE: PASS\`.
|
||||
|
||||
If GATE is FAIL, use AskUserQuestion:
|
||||
|
||||
@@ -24,7 +24,7 @@ esac
|
||||
HOST="claude"
|
||||
while [ $# -gt 0 ]; do
|
||||
case "$1" in
|
||||
--host) HOST="$2"; shift 2 ;;
|
||||
--host) [ -z "$2" ] && echo "Missing value for --host (expected claude, codex, kiro, or auto)" >&2 && exit 1; HOST="$2"; shift 2 ;;
|
||||
--host=*) HOST="${1#--host=}"; shift ;;
|
||||
*) shift ;;
|
||||
esac
|
||||
@@ -293,6 +293,10 @@ create_codex_runtime_root() {
|
||||
|
||||
if [ -L "$codex_gstack" ]; then
|
||||
rm -f "$codex_gstack"
|
||||
elif [ -d "$codex_gstack" ] && [ "$codex_gstack" != "$gstack_dir" ]; then
|
||||
# Old direct installs left a real directory here with stale source skills.
|
||||
# Remove it so we start fresh with only the minimal runtime assets.
|
||||
rm -rf "$codex_gstack"
|
||||
fi
|
||||
|
||||
mkdir -p "$codex_gstack" "$codex_gstack/browse" "$codex_gstack/gstack-upgrade" "$codex_gstack/review"
|
||||
@@ -318,6 +322,10 @@ create_codex_runtime_root() {
|
||||
ln -snf "$gstack_dir/review/$f" "$codex_gstack/review/$f"
|
||||
fi
|
||||
done
|
||||
# ETHOS.md — referenced by "Search Before Building" in all skill preambles
|
||||
if [ -f "$gstack_dir/ETHOS.md" ]; then
|
||||
ln -snf "$gstack_dir/ETHOS.md" "$codex_gstack/ETHOS.md"
|
||||
fi
|
||||
}
|
||||
|
||||
# 4. Install for Claude (default)
|
||||
@@ -348,7 +356,11 @@ if [ "$INSTALL_CODEX" -eq 1 ]; then
|
||||
fi
|
||||
mkdir -p "$CODEX_SKILLS"
|
||||
|
||||
create_codex_runtime_root "$SOURCE_GSTACK_DIR" "$CODEX_GSTACK"
|
||||
# Skip runtime root creation for repo-local installs — the checkout IS the runtime root.
|
||||
# create_codex_runtime_root would create self-referential symlinks (bin → bin, etc.).
|
||||
if [ "$CODEX_REPO_LOCAL" -eq 0 ]; then
|
||||
create_codex_runtime_root "$SOURCE_GSTACK_DIR" "$CODEX_GSTACK"
|
||||
fi
|
||||
# Install generated Codex-format skills (not Claude source dirs)
|
||||
link_codex_skill_dirs "$SOURCE_GSTACK_DIR" "$CODEX_SKILLS"
|
||||
|
||||
@@ -367,9 +379,24 @@ if [ "$INSTALL_KIRO" -eq 1 ]; then
|
||||
KIRO_GSTACK="$KIRO_SKILLS/gstack"
|
||||
# Remove old whole-dir symlink from previous installs
|
||||
[ -L "$KIRO_GSTACK" ] && rm -f "$KIRO_GSTACK"
|
||||
mkdir -p "$KIRO_GSTACK"
|
||||
mkdir -p "$KIRO_GSTACK" "$KIRO_GSTACK/browse" "$KIRO_GSTACK/gstack-upgrade" "$KIRO_GSTACK/review"
|
||||
ln -snf "$SOURCE_GSTACK_DIR/bin" "$KIRO_GSTACK/bin"
|
||||
ln -snf "$SOURCE_GSTACK_DIR/browse" "$KIRO_GSTACK/browse"
|
||||
ln -snf "$SOURCE_GSTACK_DIR/browse/dist" "$KIRO_GSTACK/browse/dist"
|
||||
ln -snf "$SOURCE_GSTACK_DIR/browse/bin" "$KIRO_GSTACK/browse/bin"
|
||||
# ETHOS.md — referenced by "Search Before Building" in all skill preambles
|
||||
if [ -f "$SOURCE_GSTACK_DIR/ETHOS.md" ]; then
|
||||
ln -snf "$SOURCE_GSTACK_DIR/ETHOS.md" "$KIRO_GSTACK/ETHOS.md"
|
||||
fi
|
||||
# gstack-upgrade skill
|
||||
if [ -f "$AGENTS_DIR/gstack-upgrade/SKILL.md" ]; then
|
||||
ln -snf "$AGENTS_DIR/gstack-upgrade/SKILL.md" "$KIRO_GSTACK/gstack-upgrade/SKILL.md"
|
||||
fi
|
||||
# Review runtime assets (individual files, not whole dir)
|
||||
for f in checklist.md design-checklist.md greptile-triage.md TODOS-format.md; do
|
||||
if [ -f "$SOURCE_GSTACK_DIR/review/$f" ]; then
|
||||
ln -snf "$SOURCE_GSTACK_DIR/review/$f" "$KIRO_GSTACK/review/$f"
|
||||
fi
|
||||
done
|
||||
|
||||
# Rewrite root SKILL.md paths for Kiro
|
||||
sed -e "s|~/.claude/skills/gstack|~/.kiro/skills/gstack|g" \
|
||||
@@ -385,10 +412,11 @@ if [ "$INSTALL_KIRO" -eq 1 ]; then
|
||||
skill_name="$(basename "$skill_dir")"
|
||||
target_dir="$KIRO_SKILLS/$skill_name"
|
||||
mkdir -p "$target_dir"
|
||||
sed -e "s|~/.codex/skills/gstack|~/.kiro/skills/gstack|g" \
|
||||
# Generated Codex skills use $HOME/.codex (not ~/), plus $GSTACK_ROOT variables.
|
||||
# Rewrite the default GSTACK_ROOT value and any remaining literal paths.
|
||||
sed -e 's|\$HOME/.codex/skills/gstack|$HOME/.kiro/skills/gstack|g' \
|
||||
-e "s|~/.codex/skills/gstack|~/.kiro/skills/gstack|g" \
|
||||
-e "s|~/.claude/skills/gstack|~/.kiro/skills/gstack|g" \
|
||||
-e "s|\.agents/skills/gstack|.kiro/skills/gstack|g" \
|
||||
-e "s|\.agents/skills|.kiro/skills|g" \
|
||||
"$skill_dir/SKILL.md" > "$target_dir/SKILL.md"
|
||||
done
|
||||
echo "gstack ready (kiro)."
|
||||
|
||||
+2
-2
@@ -1112,7 +1112,7 @@ TMPERR_ADV=$(mktemp /tmp/codex-adv-XXXXXXXX)
|
||||
codex exec "Review the changes on this branch against the base branch. Run git diff origin/<base> to see the diff. Your job is to find ways this code will fail in production. Think like an attacker and a chaos engineer. Find edge cases, race conditions, security holes, resource leaks, failure modes, and silent data corruption paths. Be adversarial. Be thorough. No compliments — just the problems." -s read-only -c 'model_reasoning_effort="xhigh"' --enable web_search_cached 2>"$TMPERR_ADV"
|
||||
```
|
||||
|
||||
Use a 5-minute timeout (`timeout: 300000`). After the command completes, read stderr:
|
||||
Set the Bash tool's `timeout` parameter to `300000` (5 minutes). Do NOT use the `timeout` shell command — it doesn't exist on macOS. After the command completes, read stderr:
|
||||
```bash
|
||||
cat "$TMPERR_ADV"
|
||||
```
|
||||
@@ -1157,7 +1157,7 @@ TMPERR=$(mktemp /tmp/codex-review-XXXXXXXX)
|
||||
codex review --base <base> -c 'model_reasoning_effort="xhigh"' --enable web_search_cached 2>"$TMPERR"
|
||||
```
|
||||
|
||||
Use a 5-minute timeout. Present output under `CODEX SAYS (code review):` header.
|
||||
Set the Bash tool's `timeout` parameter to `300000` (5 minutes). Do NOT use the `timeout` shell command — it doesn't exist on macOS. Present output under `CODEX SAYS (code review):` header.
|
||||
Check for `[P1]` markers: found → `GATE: FAIL`, not found → `GATE: PASS`.
|
||||
|
||||
If GATE is FAIL, use AskUserQuestion:
|
||||
|
||||
@@ -967,7 +967,10 @@ describe('Codex generation (--host codex)', () => {
|
||||
for (const skill of ALL_SKILLS) {
|
||||
const content = fs.readFileSync(path.join(ROOT, skill.dir, 'SKILL.md'), 'utf-8');
|
||||
expect(content).not.toContain('~/.codex/');
|
||||
expect(content).not.toContain('.agents/skills');
|
||||
// gstack-upgrade legitimately references .agents/skills for cross-platform detection
|
||||
if (skill.dir !== 'gstack-upgrade') {
|
||||
expect(content).not.toContain('.agents/skills');
|
||||
}
|
||||
}
|
||||
});
|
||||
});
|
||||
|
||||
Reference in New Issue
Block a user