Repository navigation
fix(skill-preamble): use $HOME instead of literal ~ in quoted bin paths (#1656) - #1692
0xDevNinja wants to merge 2 commits into
Conversation
|
@0xDevNinja Please rewrite this to include the newer generated outputs and tests proving these commands activate safely with $HOME paths. |
- The artifacts-sync preamble assigned `_BRAIN_SYNC_BIN="~/.claude/..."` and the telemetry block `-x`-tested the same path; tilde inside double quotes is literal, so 42+ SKILL.md files silently no-op'd brain-sync - `2>/dev/null || true` swallowed the ENOENT, so users never saw failure while cross-machine artifact handoff quietly never pushed - Normalize `ctx.paths.binDir` to `$HOME/...` form for env-var-host (Codex etc.) paths, untouched - Add static invariants `="~/` and `[ -x "~/` so the pattern can't regress under future resolver edits Fixes garrytan#1656. Same shape as garrytan#785 / garrytan#843, different surface (binDir, not browseDir/designDir).
Rebase onto current main picked up two skills added since this branch was cut (diagram, spec) whose generated SKILL.md still carried the literal-tilde quoted assignments. Regenerated via bun run gen:skill-docs with the $HOME-normalized resolvers from this branch; the existing static invariants in test/skill-validation.test.ts scan every generated SKILL.md, so they cover these files from now on. Refs garrytan#1656.
25a390e to
8c21134
Compare
|
rebased onto current main and regenerated. the rebase picked up two skills added since this branch was cut (diagram, spec) whose SKILL.md still had the literal-tilde assignments — regenerated both, zero |
|
Thank you for this fix! The bug it addresses (#1656) has since been fixed on main and the issue is closed, so I'm closing this PR as done. Re-verified against current main (v1.91.12.0). If you think part of your change is still missing, reply and we'll reopen. Really appreciate the contribution. |
Summary
scripts/resolvers/preamble/generate-brain-sync-block.tsemits_BRAIN_SYNC_BIN="${binDir}/gstack-brain-sync"and_BRAIN_CONFIG_BIN="${binDir}/gstack-config". For the Claude hostbinDiris~/.claude/skills/gstack/bin, so the rendered assignments hold a literal~. Tilde inside double quotes is never expanded.scripts/resolvers/preamble/generate-preamble-bash.tshas the same shape on line 55 —[ -x "${binDir}/gstack-telemetry-log" ]— the-xtest then always returns false on Claude hosts.2>/dev/null || trueswallows ENOENT, so the artifacts-sync preamble silently no-ops in 48 generated SKILL.md files (/context-save,/context-restore,/autoplan,/codex, etc.). Cross-machine artifact handoff never pushes.ctx.paths.binDirto$HOME/...form before interpolating into the quoted contexts. Env-var hosts (Codex, Hermes, etc.) whosebinDiris already$GSTACK_BINare untouched — only the~/...prefix is rewritten.Fixes #1656. Same defect class as #785 / #843 (which targeted
browseDir/designDir); this PR is the matching fix on the brain-sync surface.Why this shape
scripts/resolvers/types.ts. PR fix: use $HOME instead of ~ for browse/design binary paths #843 already proposes a global swap onbrowseDir/designDir; doing the same swap onbinDirfrom this PR would silently conflict with anyone else touchingtypes.ts. The local pattern keeps the diff narrow and reversible.test/skill-validation.test.ts. Two new tests fail loudly if any generated SKILL.md re-introduces="~/or[ -x "~/. Existing skills are now clean (grepreturns 0 hits), so the invariants pass.Diff shape
bun run gen:skill-docs)Tests
The 7 failures are the pre-existing
--dry-run freshness checkcases on non-Claude hosts that reproduce onupstream/mainwith this branch's changes stashed — unrelated.Verification
Sample lines from the regenerated
context-save/SKILL.md:grep -rEl '_BRAIN_SYNC_BIN="~/|_BRAIN_CONFIG_BIN="~/|-x "~/' */SKILL.md SKILL.mdreturns 0 files post-patch.