diff --git a/deno.json b/deno.json index 733ad3c1..516298b0 100644 --- a/deno.json +++ b/deno.json @@ -1,7 +1,7 @@ { "$schema": "https://raw.githubusercontent.com/denoland/deno/main/cli/schemas/config-file.v1.json", "name": "@webjam/tools", - "version": "1.32.39", + "version": "1.32.40", "exports": "./src/uptime/cron.ts", "tasks": { "sheet-music:generate": "deno run --allow-read --allow-write src/sheet-music/generate_docx.ts", diff --git a/docs/scripts.md b/docs/scripts.md index 665f6b56..616eac6f 100644 --- a/docs/scripts.md +++ b/docs/scripts.md @@ -167,6 +167,21 @@ echo '{"model":{"id":"claude-opus-5","display_name":"Opus 5"}}' | scripts/status Overriding this is a test-only seam (the real default hits the network, which an automated test must not depend on); leave it unset for normal use. +### `install-hooks.sh` — what actually gets symlinked + +`scripts/install-hooks.sh` symlinks `hooks/*.sh` only — `hooks/lib/` is +never installed, so there is no `~/.claude/hooks/lib/` path on disk. A hook +script reaches its shared `hooks/lib/*.ts` modules by resolving its own +symlink back to the canonical clone (`HOOK_DIR=$(cd "$(dirname +"$(readlink -f "${BASH_SOURCE[0]}")")" && pwd)`), then reading +`$HOOK_DIR/lib/.ts` from there — agy reaches the same modules +through those same Claude Code symlinks via `hooks/agy-hook-shim.sh`. That +means a content-only change to an already-installed hook script, skill +body, or `hooks/lib/*.ts` module is live on both surfaces the moment `dev` +is pulled; no install step is needed unless the change is structural (a +new/renamed/deleted skill or hook, or a changed event/matcher +registration). + ### `permissions.defaultMode` (managed by `install-hooks.sh`) `~/.claude/settings.json` has no `permissions.defaultMode` key by default, so diff --git a/skills/design-issue/SKILL.md b/skills/design-issue/SKILL.md index df5bc21e..4328929a 100644 --- a/skills/design-issue/SKILL.md +++ b/skills/design-issue/SKILL.md @@ -174,7 +174,7 @@ Everything this skill designs works on both Claude Code and agy/Antigravity. A m ### Installing is for Structure, Never for Content -**Installing is for structure, never for content.** Skill bodies and hook scripts are symlinked into the canonical clone, and agy invokes those same Claude Code symlinks through `agy-hook-shim.sh` while symlinking the same skill sources into its own plugin directory. One set of files serves both surfaces. A content change to an existing skill or hook is therefore live on both the moment the canonical clone is on the merged commit, and the both-surfaces acceptance criterion for such a change asserts only that those symlinks still resolve into the clone — the failure that has actually occurred, when they were repointed into a temporary directory. The installers exist for the two things a symlink cannot carry: a link that does not exist yet, and a registration entry. A new, renamed or deleted skill runs the skill installer. A new or deleted hook, or an existing hook whose event or matcher changed, runs the hook installer, which re-merges the repository's entries into the two settings files — merge targets holding Josh's own settings beside the repository's, and so not themselves symlinkable. Both run from the canonical clone, never from a worktree, which the installer's own path validation enforces. +**Installing is for structure, never for content.** Skill bodies and hook scripts are symlinked into the canonical clone, and agy invokes those same Claude Code symlinks through `agy-hook-shim.sh` while symlinking the same skill sources into its own plugin directory. One set of files serves both surfaces, so a content change to an existing skill or hook is live on both the moment the canonical clone is on the merged commit. For the resolution mechanism, see `docs/scripts.md` ("`install-hooks.sh` — what actually gets symlinked"); for the both-surfaces acceptance-criterion shape a content-only change must use, see `skills/file-issue/SKILL.md` rule 14 — do not restate either here. The installers exist for the two things a symlink cannot carry: a link that does not exist yet, and a registration entry. A new, renamed or deleted skill runs the skill installer. A new or deleted hook, or an existing hook whose event or matcher changed, runs the hook installer, which re-merges the repository's entries into the two settings files — merge targets holding Josh's own settings beside the repository's, and so not themselves symlinkable. Both run from the canonical clone, never from a worktree, which the installer's own path validation enforces. --- diff --git a/skills/file-issue/SKILL.md b/skills/file-issue/SKILL.md index d18e7b76..5308b76a 100644 --- a/skills/file-issue/SKILL.md +++ b/skills/file-issue/SKILL.md @@ -128,8 +128,9 @@ that's what following this skill prevents. 14. **Hook and Skill Issues Must Target Both Claude Code and agy/Antigravity.** - Any issue about a **hook** (a script under `hooks/`) or a **skill** (a directory under `skills/`) must declare it targets **both** agent surfaces — Claude Code and agy/Antigravity — unless Josh has explicitly approved a single-surface exception for that specific issue *in this filing session*, with approval recorded in the issue body. - **Surfaces must be named in `## What this builds`.** The section opening the body must state what is being built for each surface, not just once for both (e.g. not "updates the hook" but "updates the hook in Claude Code and adds the agy variant"). - - **One acceptance criterion per surface.** The `## Acceptance criteria` section must carry a separate, distinct criterion checking each surface: Claude Code (`scripts/install-hooks.sh` for hooks; `scripts/install-skills.ts` for skills) and agy (`scripts/install-hooks.sh` on agy side; same for skills). - - **Both installers named in verification steps.** The `## How to test locally` section must name both `scripts/install-hooks.sh` and `scripts/install-skills.ts` as part of the verification steps, so the implementing agent installs and checks both surfaces rather than one. + - **Structural change: one acceptance criterion per surface, naming the installer.** A new, renamed, or deleted skill or hook, or an existing hook whose event or matcher registration changed, requires an installer to reach either surface. A brand-new `hooks/lib/*.ts` module consumed by an already-installed hook is **not** a new hook under this bullet — see the content-only bullet below. The `## Acceptance criteria` section must carry a separate, distinct criterion checking each surface: Claude Code (`scripts/install-hooks.sh` for hooks; `scripts/install-skills.ts` for skills) and agy (`scripts/install-hooks.sh` on agy side; same for skills). `## How to test locally` must name both `scripts/install-hooks.sh` and `scripts/install-skills.ts` as part of the verification steps, so the implementing agent installs and checks both surfaces rather than one. + - **Content-only change: one acceptance criterion per surface, naming the resolution check, not an installer.** A content-only change to a file that is already installed — a skill body, a new or existing supporting file inside an already-installed skill directory (e.g. adding `skills/fix-labels/labels.yaml` to the already-installed `fix-labels` skill), a hook script's contents, or a new or existing shared `hooks/lib/*.ts` module consumed by an already-installed hook — needs no installer: `git pull` on `dev` makes it live on both surfaces (see `docs/scripts.md` for the mechanism; do not restate it here). For a **skill**, `~/.claude/skills/` and `~/.gemini/config/plugins/webjam-tasks/skills/` are two genuinely distinct symlinks, so the `## Acceptance criteria` section asserts, per surface, that the installed symlink still resolves into the canonical clone. For a **hook or a `hooks/lib/*.ts` module**, there is only one hook symlink (`~/.claude/hooks/.sh`) — agy reaches it through that same Claude Code symlink via `hooks/agy-hook-shim.sh`, not a second symlink of its own — so the two per-surface criteria must NOT be the same assertion written twice: the Claude Code criterion asserts `~/.claude/hooks/.sh` resolves into the canonical clone — except for a hook listed in `scripts/install-hooks.sh`'s `AGY_ONLY_PRE_TOOL_USE_HOOKS` array (`hooks/agy-model-guard.sh`, `hooks/block-agy-gmail-send-delete.sh`), which is symlinked into `~/.claude/hooks/` like every other hook but is deliberately never registered in Claude Code's `settings.json`, because it depends on agy-native payload fields Claude Code's hook payload does not carry (`docs/agy-hooks.md` § "agy-only hooks"); for those, asserting only that the symlink resolves would be a true statement implying a Claude-Code-side effect that cannot occur, so the Claude Code criterion instead asserts that fact plainly: the hook has no Claude Code `settings.json` registration, and the content change has no Claude-Code-side effect. Never invent a Claude Code registration for an agy-only hook to satisfy this criterion. The agy criterion depends on which event the hook (or the hook consuming the `hooks/lib/*.ts` module) is registered under, checked against `scripts/install-hooks.sh`'s own `PRE_TOOL_USE_HOOKS`/`POST_TOOL_USE_HOOKS`/`AGY_ONLY_PRE_TOOL_USE_HOOKS` vs. `SESSION_START_HOOKS`/`STOP_HOOKS` arrays: for a `PreToolUse` or `PostToolUse` hook, which carries a `~/.gemini/config/hooks.json` shim registration, the agy criterion asserts that registration still targets that same `$HOME/.claude/hooks/` path — that registration, not a second symlink, is what makes the agy criterion distinct; for a `SessionStart` or `Stop` hook, `~/.gemini/config/hooks.json` carries no registration for it at all and never can — `scripts/merge-hooks-into-settings.ts --forbid-lifecycle-hooks` refuses one there because registering either lifecycle event silently disables agy's entire hooks config (`docs/agy-hooks.md` finding 9) — so the agy criterion asserts that fact plainly: the hook has no `~/.gemini/config/hooks.json` registration, and the content change has no agy-side effect. Never invent an agy-side path or registration for a `SessionStart`/`Stop` hook to satisfy this criterion. `## How to test locally` names that resolution check rather than an installer. + - **Never assert a `~/.claude/hooks/lib/` path.** `hooks/lib/` is never installed by `scripts/install-hooks.sh`, so that path does not exist and never has. No acceptance criterion, for either change kind, may claim a file resolves there. 15. **A Guard Has Three Outcomes, Not Two.** - Every issue that designs, modifies, or implements a guard, gate, permission check, or validation rule must explicitly specify all three outcomes: 1. When the condition holds (e.g. valid, permitted, or matched). diff --git a/test/skills_validation.test.ts b/test/skills_validation.test.ts index 64343d20..09d3bdba 100644 --- a/test/skills_validation.test.ts +++ b/test/skills_validation.test.ts @@ -160,6 +160,148 @@ Deno.test("skills/file-issue/SKILL.md contains the three-outcomes guard rule, po ); }); +Deno.test("skills/file-issue/SKILL.md item 14 splits both-surfaces acceptance criteria by change kind and bans the hooks/lib path", async () => { + const fileIssuePath = `${SKILLS_DIR}file-issue/SKILL.md`; + const text = await Deno.readTextFile(fileIssuePath); + + // Structural change kind still names an installer per surface + assert( + text.includes("Structural change: one acceptance criterion per surface, naming the installer."), + "skills/file-issue/SKILL.md item 14 must keep the structural-change bullet naming an installer per surface", + ); + + // Content-only change kind names the resolution check instead of an installer + assert( + text.includes( + "Content-only change: one acceptance criterion per surface, naming the resolution check, not an installer.", + ), + "skills/file-issue/SKILL.md item 14 must add the content-only-change bullet naming the resolution check", + ); + assert( + text.includes("needs no installer: `git pull` on `dev` makes it live on both surfaces"), + "skills/file-issue/SKILL.md item 14 must state that a content-only change needs no installer", + ); + + // A new supporting file inside an already-installed skill directory is content-only + assert( + text.includes( + "a new or existing supporting file inside an already-installed skill directory (e.g. adding `skills/fix-labels/labels.yaml` to the already-installed `fix-labels` skill)", + ), + "skills/file-issue/SKILL.md item 14 must classify a new supporting file in an already-installed skill directory as content-only", + ); + + // A brand-new hooks/lib/*.ts module consumed by an already-installed hook is content-only, not structural + assert( + text.includes( + "A brand-new `hooks/lib/*.ts` module consumed by an already-installed hook is **not** a new hook under this bullet", + ), + "skills/file-issue/SKILL.md item 14's structural bullet must disclaim a new hooks/lib/*.ts module as not a new hook", + ); + assert( + text.includes( + "a new or existing shared `hooks/lib/*.ts` module consumed by an already-installed hook", + ), + "skills/file-issue/SKILL.md item 14's content-only bullet must classify a new hooks/lib/*.ts module as content-only", + ); + + // Skill case: two genuinely distinct symlinks, one criterion per surface as before + assert( + text.includes( + "For a **skill**, `~/.claude/skills/` and `~/.gemini/config/plugins/webjam-tasks/skills/` are two genuinely distinct symlinks, so the `## Acceptance criteria` section asserts, per surface, that the installed symlink still resolves into the canonical clone.", + ), + "skills/file-issue/SKILL.md item 14 must keep the skill content-only case as two distinct symlink assertions", + ); + + // Hook case: only one hook symlink, so the agy criterion asserts the hooks.json shim registration, not a second symlink + assert( + text.includes( + "For a **hook or a `hooks/lib/*.ts` module**, there is only one hook symlink (`~/.claude/hooks/.sh`)", + ), + "skills/file-issue/SKILL.md item 14 must state that a hook or hooks/lib/*.ts module has only one hook symlink", + ); + assert( + text.includes( + "for a `PreToolUse` or `PostToolUse` hook, which carries a `~/.gemini/config/hooks.json` shim registration, the agy criterion asserts that registration still targets that same `$HOME/.claude/hooks/` path", + ), + "skills/file-issue/SKILL.md item 14 must state the agy criterion for a PreToolUse/PostToolUse hook content-only change asserts the hooks.json shim registration", + ); + + // Hook case: a SessionStart/Stop lifecycle hook carries NO agy registration at all (registering + // one there disables agy's entire hooks config), so the agy criterion asserts that absence as a + // deliberate fact rather than inventing a path or registration to assert instead. + assert( + text.includes( + "for a `SessionStart` or `Stop` hook, `~/.gemini/config/hooks.json` carries no registration for it at all and never can", + ), + "skills/file-issue/SKILL.md item 14 must state that a SessionStart/Stop hook carries no agy hooks.json registration at all", + ); + assert( + text.includes( + "registering either lifecycle event silently disables agy's entire hooks config (`docs/agy-hooks.md` finding 9)", + ), + "skills/file-issue/SKILL.md item 14 must cite docs/agy-hooks.md finding 9 for why lifecycle hooks are never registered with agy", + ); + assert( + text.includes( + "so the agy criterion asserts that fact plainly: the hook has no `~/.gemini/config/hooks.json` registration, and the content change has no agy-side effect", + ), + "skills/file-issue/SKILL.md item 14 must state the no-registration agy criterion wording for a SessionStart/Stop hook", + ); + assert( + text.includes( + "Never invent an agy-side path or registration for a `SessionStart`/`Stop` hook to satisfy this criterion.", + ), + "skills/file-issue/SKILL.md item 14 must prohibit inventing an agy-side path or registration for a SessionStart/Stop hook", + ); + + // agy-only hooks: the mirror case on the Claude Code side. These are symlinked into + // ~/.claude/hooks/ like every other hook but are never registered in Claude Code's + // settings.json, so a bare symlink-resolution criterion would be true while implying a + // Claude-Code-side effect that cannot occur. + assert( + text.includes( + "except for a hook listed in `scripts/install-hooks.sh`'s `AGY_ONLY_PRE_TOOL_USE_HOOKS` array (`hooks/agy-model-guard.sh`, `hooks/block-agy-gmail-send-delete.sh`)", + ), + "skills/file-issue/SKILL.md item 14 must carve out the AGY_ONLY_PRE_TOOL_USE_HOOKS hooks from the Claude Code symlink-resolution criterion", + ); + assert( + text.includes( + "is deliberately never registered in Claude Code's `settings.json`, because it depends on agy-native payload fields Claude Code's hook payload does not carry", + ), + "skills/file-issue/SKILL.md item 14 must state why an agy-only hook carries no Claude Code settings.json registration", + ); + assert( + text.includes( + "asserting only that the symlink resolves would be a true statement implying a Claude-Code-side effect that cannot occur", + ), + "skills/file-issue/SKILL.md item 14 must state why a bare symlink assertion is hollow for an agy-only hook", + ); + assert( + text.includes( + "the Claude Code criterion instead asserts that fact plainly: the hook has no Claude Code `settings.json` registration, and the content change has no Claude-Code-side effect", + ), + "skills/file-issue/SKILL.md item 14 must state the no-registration Claude Code criterion wording for an agy-only hook", + ); + assert( + text.includes( + "Never invent a Claude Code registration for an agy-only hook to satisfy this criterion.", + ), + "skills/file-issue/SKILL.md item 14 must prohibit inventing a Claude Code registration for an agy-only hook", + ); + + // Explicit prohibition on asserting a ~/.claude/hooks/lib/ path + assert( + text.includes("Never assert a `~/.claude/hooks/lib/` path."), + "skills/file-issue/SKILL.md item 14 must prohibit asserting a ~/.claude/hooks/lib/ path", + ); + assert( + text.includes( + "`hooks/lib/` is never installed by `scripts/install-hooks.sh`, so that path does not exist", + ), + "skills/file-issue/SKILL.md item 14 must state that hooks/lib/ is never installed and the path does not exist", + ); +}); + Deno.test("skills/file-issue/SKILL.md contains the issue titles rule for PM audience, skill/feature prefix, and Epic citation (#869)", async () => { const fileIssuePath = `${SKILLS_DIR}file-issue/SKILL.md`; const text = await Deno.readTextFile(fileIssuePath);