diff --git a/.github/workflows/upstream-sync.yml b/.github/workflows/upstream-sync.yml index 6c3ee4b0..865f4c5a 100644 --- a/.github/workflows/upstream-sync.yml +++ b/.github/workflows/upstream-sync.yml @@ -96,26 +96,26 @@ jobs: skill's `agents/openai.yaml` (policy: allow_implicit_invocation: false) and keep its implicit_allowed false in tools/skill-trigger-cases.json. When upstream removes it, delete - that file, add the skill to AUTO_INVOCABLE in - tools/test_skill_invocation_policy.py, and set its - implicit_allowed to true. Write every incoming load of another - skill as "Use the **x** skill" with the bare name, per the - PORTING.md row for sibling-skill loads, never as `/x` or `$x`. - A passage that quotes what the user types goes in USER_FACING in - tools/test_skill_references.py. When a skill directory is added, - renamed, or removed, update each list that names skills: + that file and set its implicit_allowed to true. Write every + incoming load of another skill as "Use the **x** skill" with the + bare name, per the PORTING.md row for sibling-skill loads, never + as `/x` or `$x`. A passage that quotes what the user types goes + in USER_FACING in tools/skill-references.py. When a skill + directory is added, renamed, or removed, update each list that + names skills: - PSTACK_SKILLS in skills/setup-pstack/scripts/skill-listing.py. - The skill's row in tools/skill-trigger-cases.json. + - A row in README.md's "all skills" table, which + evals/promises/test_static.py checks against the skill + directories. - In evals/promises/test_static.py, PLAYBOOK_FILES for a poteto-mode playbook, PRINCIPLE_RULES for a principle-* skill, and GUIDE_SLASH_SKILLS when docs/guide names the skill as /x. A playbook added or removed also changes the - two `25` counts in test_twenty_five_playbooks, the - twenty-five-playbooks promise in evals/promises/ledger.json, + twenty-five-playbooks promise in evals/promises/ledger.json and the "twenty-five" in README.md and docs/guide/02-poteto-mode.md. - - For a skill with no agents/openai.yaml gate, AUTO_INVOCABLE - in tools/test_skill_invocation_policy.py and the "A skill + - For a skill with no agents/openai.yaml gate, the "A skill didn't load on its own" row in skills/poteto-help/SKILL.md. 4. Every SKILL.md frontmatter `name:` must equal its directory name. Verify after editing. diff --git a/PORTING.md b/PORTING.md index e64dd84b..7c9ddcaa 100644 --- a/PORTING.md +++ b/PORTING.md @@ -22,10 +22,12 @@ Upstream is the source of truth for the workflows themselves. This file records | Cursor `/loop` | Claude Code `/loop`; in a CLI without one, re-prompt on an interval or run a watcher subagent | | Cursor automations / `api2.cursor.sh` webhooks (benny) | any scheduled agent runner (Claude Code scheduled agents, cron-driven CI job) exposing a webhook or schedule; paths move from `.cursor/automations/` to `.agents/automations/` | | `.cursor-plugin/plugin.json`, and the root Agent Plugins `plugin.json` that upstream #576 added | removed; `npx skills` discovers `skills/*/SKILL.md` directly | -| `disable-model-invocation: true` in the frontmatter of a skill under `skills/` | dropped. The skill keeps or gains `agents/openai.yaml` with `policy.allow_implicit_invocation: false`, which gates it on Codex and puts it in setup-pstack's Claude Code name-only listing. Keep its `implicit_allowed` false in `tools/skill-trigger-cases.json`. When upstream removes the flag, delete the policy file, add the skill to `AUTO_INVOCABLE` in `tools/test_skill_invocation_policy.py`, and set its `implicit_allowed` to true, unless a port divergence keeps it gated | -| A sibling-skill load written as "run the **x** skill", "per the **x** skill", "via the **x** skill", a backticked `x`, `/x`, or `$x` | "Use the **x** skill" as its own sentence or step, with the bare skill name. The **pstack-harness** skill defines using a skill on each CLI. A playbook step keeps its opening sentence, and the load follows as the next sentence, because recorded worklists and the canon chain tests match a step by its first five words. The exception is a step whose opening clause carries the load, as in Autonomous run step 5, Hillclimb step 3, and Investigation step 4, whose first words changed. Feature also keeps every skill name as spelled, because the canon chain tests count its pointers. Elsewhere a principle pointer may now spell the full `principle-` name, as in Autonomous run step 3, Perf issue step 3, and Refactoring step 8. figure-it-out's Start keeps "read the Principles section of the **poteto-mode** skill", because "Use the **poteto-mode** skill." there sent Codex leads back to pick an Autonomous run worklist. Principle citations after a rule the sentence already states, ownership statements, descriptions, and Bug fix step 2's measured how and why spawn keep upstream's wording. The owner lifecycle lists in Autopilot-full step 2 and Autopilot-stack step 1, and the **swarm** attribution in Multi-phase plan's verification block, are such statements. `tools/test_skill_references.py` fails on a `/x` or `$x` spelling of a pstack skill under `skills/` outside its `USER_FACING` passages, which quote what the user types or text shown to the user, outside its `MODE_REMINDERS` passage, poteto-mode's `reminder:` line, which reaches the model but names the mode already on, and outside poteto-help, whose `USER_FACING_SKILLS` entry exempts the whole skill because it answers users. The evidence is in `docs/skill-cross-reference-phrasing.md` and `evals/promises/results/2026-10-10-use-the-skill.md` | +| `disable-model-invocation: true` in the frontmatter of a skill under `skills/` | dropped. The skill keeps or gains `agents/openai.yaml` with `policy.allow_implicit_invocation: false`, which gates it on Codex and puts it in setup-pstack's Claude Code name-only listing. Keep its `implicit_allowed` false in `tools/skill-trigger-cases.json`. When upstream removes the flag, delete the policy file and set its `implicit_allowed` to true, unless a port divergence keeps it gated | +| A sibling-skill load written as "run the **x** skill", "per the **x** skill", "via the **x** skill", a backticked `x`, `/x`, or `$x` | "Use the **x** skill" as its own sentence or step, with the bare skill name. The **pstack-harness** skill defines using a skill on each CLI. A playbook step keeps its opening sentence, and the load follows as the next sentence, because recorded worklists and the canon chain tests match a step by its first five words. The exception is a step whose opening clause carries the load, as in Autonomous run step 5, Hillclimb step 3, and Investigation step 4, whose first words changed. Feature also keeps every skill name as spelled, because the canon chain tests count its pointers. Elsewhere a principle pointer may now spell the full `principle-` name, as in Autonomous run step 3, Perf issue step 3, and Refactoring step 8. figure-it-out's Start keeps "read the Principles section of the **poteto-mode** skill", because "Use the **poteto-mode** skill." there sent Codex leads back to pick an Autonomous run worklist. Principle citations after a rule the sentence already states, ownership statements, attributions, descriptions, and Bug fix step 2's measured how and why spawn keep upstream's wording. The owner lifecycle lists in Autopilot-full step 2 and Autopilot-stack step 1 are ownership statements, and the **swarm** attribution in Multi-phase plan's verification block is an attribution. `tools/test_skill_references.py` runs `violations()` and `stale_exemptions()` from `tools/skill-references.py`. `violations()` fails on a `/x` or `$x` spelling of a pstack skill under `skills/` outside four exemptions. A skill's own `/x` inside its frontmatter `description:` value, folded continuation lines included, is exempt. The `USER_FACING` passages quote what the user types or text shown to the user. The `MODE_REMINDERS` passage is poteto-mode's `reminder:` line, which reaches the model but names the mode already on. poteto-help's `USER_FACING_SKILLS` entry exempts the whole skill because it answers users. A passage that no longer matches a spelling fails `stale_exemptions()`. The evidence is in `docs/skill-cross-reference-phrasing.md` and `evals/promises/results/2026-10-10-use-the-skill.md` | | Origin forge CLI (`origin pr ...`, upstream's optional alternative to `gh` for PR create/view/merge) | kept verbatim. Every use is gated on `command -v origin` succeeding, with `gh` as the stated default, so the text is a no-op on hosts without Origin. Graphite (`gt`) remains optional only for Orchestrate frontier discovery. `orch frontier set` uses `gh` for a same-repository linear stack and runs `gt` only with `--graphite`. Never restore a hard Graphite requirement during sync. | | Cursor `Task` semantics (`subagent_type`, `model`, `readonly`, `environment: "cloud"`) assumed everywhere | a new **pstack-harness** skill (`skills/pstack-harness/`) maps spawning, per-subagent models, parallelism, read-only posture, structured questions, loops, and transcript access to Claude Code (`Task` tool), Codex (`spawn_agent`, with `codex exec` subprocesses as the fallback), Hermes (delegation toolset, `hermes -z` subprocesses, `hermes sessions export`), and Grok Build (`spawn_subagent`, `grok -p` subprocesses, per-session directories); fan-out skills point to it | +| "Open a todolist ..." and "add the phases below as todos" (architect, arena, figure-it-out, swarm, and poteto-mode's "task-specific todos"), "into the todo list" (poteto-help), and "Add its steps to the todolist" (figure-it-out) | "Open a worklist ...", "items", "into the worklist", and "Add its steps to the worklist". The **pstack-harness** skill's worklist adapter defines ordered items and their states, not a tool name | +| arena Phase C: "Prefer a different model family from the parent's." | "Prefer a different model family or capability tier from the parent's." The default pool is `fable`, `opus`, and `sonnet`, which are one family, so a different tier is often the only difference available | ## Port additions beyond substitution @@ -47,8 +49,8 @@ Upstream is the source of truth for the workflows themselves. This file records So step 0d treats a live session as the answer and config as signals. It offers an edit only when a session confirms `spawn_agent` is missing, and only for a possible cause a config file shows. On sync, keep step 0d of setup-pstack and the setup guide's sentences. - The **pstack-harness** skill owns the three-root runtime contract. `PSTACK_SKILLS_ROOT` comes from the logical path of the loaded harness skill and works without Git or network access. `PSTACK_SOURCE_ROOT` accepts only the configured HTTPS or SSH identity for `mdsmithaustin/pstack`. Physical symlink ancestry is only a source candidate. A copied install creates a private shallow clone when a fresh trunk read needs one, and the program removes only a generated directory with its ownership marker. `PROJECT_ROOT` stays separate for consumer control skills. A source resolution, identity, clone, fetch, or object failure stops the trunk read. Installed files are never its fallback. The skill-ci content check, through the retired text in `tools/skill-content-conventions.json`, rejects both runtime substitutions from upstream in every raw skill Markdown line, including fenced templates. Keep this contract and lint during future syncs. -- Every skill that must not load from its description ships `agents/openai.yaml` with `policy.allow_implicit_invocation: false`. Codex honors it as installed. On Claude Code, setup-pstack step 0c runs `skills/setup-pstack/scripts/skill-listing.py`, which derives the `skillOverrides` name-only set from the same files. No skill under `skills/` sets `disable-model-invocation`, because Claude Code's Skill tool refuses such a skill when a pstack skill names it. `tools/test_skill_invocation_policy.py` enforces three rules. It fails on the flag, on any skill without the policy file that its `AUTO_INVOCABLE` set does not name, and when `PSTACK_SKILLS` in `skill-listing.py` differs from the `skills/` directories. Adding, renaming, or removing a skill directory updates `PSTACK_SKILLS`, and `AUTO_INVOCABLE` too when the skill has no policy file. `tests/skills/setup-pstack/scripts/test_skill_listing.py` derives the gated inventory from the same files and checks selected gated and ungated names as literal anchors. On sync, keep setup-pstack step 0c and `skill-listing.py`. -- Deliberate divergence from upstream #300: **how**, **why**, and **unslop** stay auto-invocable in this port (no `agents/openai.yaml`, named in `AUTO_INVOCABLE`), because they own high-value automatic triggers in the maintainer's roster ("how does X work", motivation questions, prose cleanup). The port-owned **deslop** skill is auto-invocable for the same reason: the guide tells readers to say `deslop it`, and a gated skill never fires on that phrase. Do not reapply upstream's gating to these four on future syncs; `typescript-best-practices` and `make-bot-ui` keep it. Guide page 5 says `typescript-best-practices` "doesn't load from its description" once `/setup-pstack` sets the Claude Code skill listing, where upstream says it doesn't load on its own. The Hermes and Grok Build caveat lives on guide page 1 and records that they loaded it anyway in `evals/promises/results/2026-10-03.md`. +- Every skill that must not load from its description ships `agents/openai.yaml` with `policy.allow_implicit_invocation: false`. Codex honors it as installed. On Claude Code, setup-pstack step 0c runs `skills/setup-pstack/scripts/skill-listing.py`, which derives the `skillOverrides` name-only set from the same files. No skill under `skills/` sets `disable-model-invocation`, because Claude Code's Skill tool refuses such a skill when a pstack skill names it. `tools/test_skill_invocation_policy.py` enforces two rules. It fails on the flag and when `PSTACK_SKILLS` in `skill-listing.py` differs from the `skills/` directories. `skill-ci check` fails when a skill's `implicit_allowed` row in `tools/skill-trigger-cases.json` disagrees with its policy file. Adding, renaming, or removing a skill directory updates `PSTACK_SKILLS` and the skill's trigger row. `tests/skills/setup-pstack/scripts/test_skill_listing.py` derives the gated inventory from the same files and checks selected gated and ungated names as literal anchors. On sync, keep setup-pstack step 0c and `skill-listing.py`. +- Deliberate divergence from upstream #300: **how**, **why**, and **unslop** stay auto-invocable in this port (no `agents/openai.yaml`, `implicit_allowed: true` in `tools/skill-trigger-cases.json`), because they own high-value automatic triggers in the maintainer's roster ("how does X work", motivation questions, prose cleanup). The port-owned **deslop** skill is auto-invocable for the same reason: the guide tells readers to say `deslop it`, and a gated skill never fires on that phrase. Do not reapply upstream's gating to these four on future syncs; `typescript-best-practices` and `make-bot-ui` keep it. Guide page 5 says `typescript-best-practices` "doesn't load from its description" once `/setup-pstack` sets the Claude Code skill listing, where upstream says it doesn't load on its own. The Hermes and Grok Build caveat lives on guide page 1 and records that they loaded it anyway in `evals/promises/results/2026-10-03.md`. - **poteto-help** (upstream #502, #506, #507, #509) ports with the substitution map applied: install through `npx skills`, the models config at `~/.agents/pstack-models.md`, `/poteto-teach` and `/poteto-tdd`, `/deslop` as a pstack skill, and no cloud agents. Upstream #506 made it typed-only, so it carries `agents/openai.yaml` in place of upstream's `disable-model-invocation`, and its row in `tools/skill-trigger-cases.json` sets `implicit_allowed: false`. `npx skills` ships only skill directories, so its README and guide links point at this repository's public copy instead of `../../`. Its skill table adds rows for the port-owned skills. Its list of skills that load from the user's words names every ungated skill a user types, and `test_poteto_help_lists_every_ungated_skill` in `evals/promises/test_static.py` fails when the gating and the list disagree. That list names Claude Code and Codex, and the README row and guide index say the skill "doesn't load from its description" once `/setup-pstack` sets the Claude Code skill listing, as guide page 5 says of `typescript-best-practices`. The Hermes and Grok Build caveat lives on guide page 1 and records that they loaded a gated skill in `evals/promises/results/2026-10-03.md`. The paragraph on harnesses replaces upstream's "built for Cursor" paragraph and restates rules 3 and 4 of setup-pstack's models config reference, so a change to either rule must update it. The "review this pr" phrase under Playbooks is a port addition for the Code review playbook. Deliberate divergence: #502 replaced "`/poteto-mode` is sticky" in the README, guide pages 1 and 2, and the skill with Cursor's Custom Mode instructions (Option+Enter, the Agents Window). The port's harnesses have no Custom Modes and keep an invoked skill in the conversation, so the port keeps the sticky wording, which the `poteto-mode-sticky` live case passed on Claude Code, Codex, and Hermes in `evals/promises/results/2026-10-03.md`. On sync, do not reapply the Custom Mode wording. - Four sites carry a conditional GitNexus nudge, active only when `.gitnexus/` exists in the repo and the `gitnexus` CLI is on `PATH`: the **how** skill (SKILL.md and `references/explorer-prompt.md`) seeds exploration with `gitnexus query`/`context`/`trace`; **blast-radius** seeds the change map with `gitnexus detect-changes`/`impact`; the **refactoring** playbook enumerates callers with `gitnexus impact`/`context` before a migration wave; and the Code review playbook's review lens lists a shared symbol's callers with `gitnexus impact`. Freshness-first policy: an existing index is refreshed with `gitnexus analyze --index-only` (incremental, no-op when up to date) before the graph is consulted — by the lead only, never from parallel arms (concurrent analyzes can corrupt the KuzuDB store). Creating a first index stays suggest-only. The code itself remains the source of truth: the index reflects the last analyzed commit and never sees uncommitted edits. No-op on hosts and repos without GitNexus. - The GitHub watcher treats a PR whose head has no checks as having no CI (port addition, 2026-09). An empty GraphQL rollup proves it, so the watcher reads `ci-clean` instead of throwing `ChecksUnavailable`. It still throws that retryable error when an earlier commit on the PR had checks, because the head's checks may not be registered yet. So a later push that never gets checks in a repo with CI, such as a path-filtered or `[skip ci]` commit, still ends at the error cap. Upstream throws whenever both check sources are empty. Keep this on sync. diff --git a/evals/promises/test_static.py b/evals/promises/test_static.py index c0e75f53..9e00e372 100644 --- a/evals/promises/test_static.py +++ b/evals/promises/test_static.py @@ -119,6 +119,21 @@ def guide_text(): return "\n".join(read(p) for p in sorted(GUIDE.glob("*.md"))) +def spell_number(n): + ones = ("zero one two three four five six seven eight nine ten eleven twelve thirteen fourteen fifteen " + "sixteen seventeen eighteen nineteen").split() + tens = "_ _ twenty thirty forty fifty sixty seventy eighty ninety".split() + if n < 20: + return ones[n] + return tens[n // 10] + (f"-{ones[n % 10]}" if n % 10 else "") + + +def stated_playbook_counts(text): + by_word = {spell_number(n): n for n in range(1, 100)} + counts = {by_word[word] for word in re.findall(r"\b([a-z]+(?:-[a-z]+)?) playbooks\b", text) if word in by_word} + return counts | {int(digits) for digits in re.findall(r"\b(\d+) playbooks\b", text)} + + def first_mermaid_block(path): text = read(path) start = text.index("```mermaid") @@ -129,11 +144,16 @@ def first_mermaid_block(path): class TestStaticPromises(unittest.TestCase): def test_twenty_five_playbooks(self): bullets = poteto_playbook_bullets() - self.assertEqual(len(bullets), 25) self.assertEqual(sorted(f for _, f in bullets), PLAYBOOK_FILES) on_disk = sorted(p.stem for p in PLAYBOOKS.glob("*.md")) self.assertEqual(on_disk, PLAYBOOK_FILES) - self.assertEqual(len(list(PLAYBOOKS.iterdir())), 25) + self.assertEqual(len(list(PLAYBOOKS.iterdir())), len(on_disk)) + for doc in (ROOT / "README.md", GUIDE / "02-poteto-mode.md"): + self.assertEqual(stated_playbook_counts(read(doc)), {len(on_disk)}, doc.name) + + def test_a_stated_playbook_count_reads_digits_and_words(self): + self.assertEqual(stated_playbook_counts("it comes with twenty-five playbooks, not 24 playbooks"), {25, 24}) + self.assertEqual(stated_playbook_counts("one playbooks, 7 playbooks"), {1, 7}) def test_principle_count_matches_shipped_skills(self): on_disk = sorted(p.parent.name for p in SKILLS.glob("principle-*/SKILL.md")) @@ -194,24 +214,19 @@ def test_principle_promise_owners_are_the_pinned_leaves(self): f"{pid} lists owners {promise['owners']} and claims {promise['claim']!r}; {fix}", ) + def test_readme_has_a_row_for_every_non_principle_skill(self): + rows = re.findall(r"^\| \[`/([a-z0-9-]+)`\]\(\./skills/\1/SKILL\.md\) \|", read(ROOT / "README.md"), flags=re.M) + shipped = sorted(p.parent.name for p in SKILLS.glob("*/SKILL.md") if not p.parent.name.startswith("principle-")) + self.assertEqual(sorted(rows), shipped, "README.md's 'all skills' table needs one row per skill directory") + def test_playbook_directory_complete(self): text = guide_text() self.assertIn("(../../skills/poteto-mode/playbooks/)", text) self.assertTrue(PLAYBOOKS.is_dir()) linked = sorted(set(re.findall(r"skills/poteto-mode/playbooks/([a-z-]+)\.md", text))) - self.assertEqual( - linked, - [ - "authoring-a-skill", "autonomous-run", "autopilot-full", "autopilot-stack", "babysit", - "bug-fix", "code-review", "eval", "feature", "hillclimb", "investigation", "multi-phase-plan", - "opening-a-pr", "orchestrate", "pause-safely", "perf-issue", "prototype", "refactoring", "research", - "runtime-forensics", "session-pickup", "shipping", "trace-forensics", "visual-parity", "worktree-cleanup", - ], - ) + self.assertEqual(linked, PLAYBOOK_FILES) for name in linked: self.assertTrue((PLAYBOOKS / f"{name}.md").is_file(), name) - for name in PLAYBOOK_FILES: - self.assertTrue((PLAYBOOKS / f"{name}.md").is_file(), name) def test_guide_named_skills_exist(self): text = guide_text() diff --git a/tools/skill-references.py b/tools/skill-references.py index 9578affe..896969d8 100644 --- a/tools/skill-references.py +++ b/tools/skill-references.py @@ -1,5 +1,5 @@ #!/usr/bin/env python3 -"""List slash and dollar skill spellings in model-facing text, one TSV row per spelling. +"""Fail on a slash or dollar skill spelling in model-facing text that no exemption covers. The scan covers tracked markdown under skills/ that is still present on disk, so deleted files, an installed node_modules, or another untracked file never fail it. Root agents/*.md @@ -17,12 +17,30 @@ ROOT = Path(__file__).resolve().parents[1] +USER_FACING_SKILLS = frozenset({"poteto-help"}) +USER_FACING = { + "skills/architect/SKILL.md": ['"/architect with checkpoint,"'], + "skills/create-verification-skill/SKILL.md": ["Point the user at `/maintain-verification-skill`"], + "skills/deslop/SKILL.md": ["and on `/deslop`,"], + "skills/ketchup/SKILL.md": ["the last message they wrote before `/ketchup`"], + "skills/maintain-verification-skill/SKILL.md": ["point at `/create-verification-skill`"], + "skills/pstack-harness/SKILL.md": ["Run `/setup-pstack` to register it.\""], + "skills/reflect/SKILL.md": ['the user says "reflect" or "/reflect"'], + "skills/setup-pstack/SKILL.md": ["4 of 4 `/poteto-mode` runs", + "I can generate one with /create-verification-skill.\""], +} +MODE_REMINDERS = { + "skills/poteto-mode/SKILL.md": ["apply /poteto-mode."], +} +EXEMPT = {rel: USER_FACING.get(rel, []) + MODE_REMINDERS.get(rel, []) for rel in USER_FACING | MODE_REMINDERS} + class Sigil(NamedTuple): file: str line: int spelling: str text: str + in_description: bool def skill_names(root: Path) -> list[str]: @@ -39,24 +57,66 @@ def sigil_pattern(name: str) -> re.Pattern[str]: return re.compile(rf"(? set[int]: + """Line numbers of the frontmatter `description:` value, including a folded continuation.""" + if not lines or lines[0].strip() != "---": + return set() + found: set[int] = set() + inside = False + for number, line in enumerate(lines[1:], 2): + if line.strip() == "---": + break + if line.startswith("description:"): + inside = True + elif inside and not line.startswith((" ", "\t")): + inside = False + if inside: + found.add(number) + return found + + def sigils(root: Path = ROOT) -> list[Sigil]: compiled = {name: sigil_pattern(name) for name in skill_names(root)} hits = [] for path in model_facing_files(root): rel = path.relative_to(root).as_posix() - for number, line in enumerate(path.read_text(encoding="utf-8").splitlines(), 1): + lines = path.read_text(encoding="utf-8").splitlines() + described = description_lines(lines) + for number, line in enumerate(lines, 1): for pattern in compiled.values(): - hits += [Sigil(rel, number, match.group(0), line) for match in pattern.finditer(line)] + hits += [Sigil(rel, number, match.group(0), line, number in described) + for match in pattern.finditer(line)] return hits +def exempt(hit: Sigil) -> bool: + skill = hit.file.split("/")[1] + if skill in USER_FACING_SKILLS: + return True + if hit.file == f"skills/{skill}/SKILL.md" and hit.spelling == f"/{skill}" and hit.in_description: + return True + return any(hit.spelling in passage and passage in hit.text for passage in EXEMPT.get(hit.file, ())) + + +def violations(root: Path = ROOT) -> list[str]: + return [f"{hit.file}:{hit.line}: {hit.spelling}" for hit in sigils(root) if not exempt(hit)] + + +def stale_exemptions(root: Path = ROOT) -> list[str]: + hits = sigils(root) + return [f"{rel}: {passage}" for rel, passages in EXEMPT.items() for passage in passages + if not any(hit.file == rel and hit.spelling in passage and passage in hit.text for hit in hits)] + + def main() -> int: - parser = argparse.ArgumentParser(description=__doc__.splitlines()[0]) - parser.add_argument("--sigils", action="store_true", help="list every /name or $name spelling") - parser.parse_args() - for rel, number, spelling, line in sigils(): - print("\t".join([rel, str(number), spelling, line.strip()])) - return 0 + argparse.ArgumentParser(description=__doc__.splitlines()[0]).parse_args() + found = violations() + stale = stale_exemptions() + for row in found: + print(f"{row}: write \"Use the **name** skill.\" as its own sentence or step") + for row in stale: + print(f"{row}: stale exemption, remove it") + return 1 if found or stale else 0 if __name__ == "__main__": diff --git a/tools/test_skill_invocation_policy.py b/tools/test_skill_invocation_policy.py index 778356ee..5225267e 100644 --- a/tools/test_skill_invocation_policy.py +++ b/tools/test_skill_invocation_policy.py @@ -11,14 +11,6 @@ skill_listing = importlib.util.module_from_spec(_spec) _spec.loader.exec_module(skill_listing) -# Skills that load from their description on every harness. Every other skill ships -# agents/openai.yaml with allow_implicit_invocation: false, which gates it on Codex and -# puts it in setup-pstack's Claude Code name-only listing. -AUTO_INVOCABLE = frozenset({ - "deslop", "documentation-impact", "how", "pstack-harness", "runtime-probes", - "setup-pstack", "spec-probes", "unslop", "verify-commands", "why", -}) - class SkillInvocationPolicy(unittest.TestCase): def test_no_skill_sets_disable_model_invocation(self): @@ -31,13 +23,6 @@ def test_listing_inventory_names_every_skill(self): self.assertEqual(sorted(skill_listing.PSTACK_SKILLS), shipped, "PSTACK_SKILLS in skills/setup-pstack/scripts/skill-listing.py must name exactly the skills/ directories") - def test_every_skill_is_gated_or_listed_as_auto_invocable(self): - open_skills = set(skill_listing.skill_dirs(SKILLS)) - set(skill_listing.managed_skills(SKILLS)) - self.assertEqual(sorted(open_skills - AUTO_INVOCABLE), [], - "add agents/openai.yaml with policy.allow_implicit_invocation: false, or add the skill to AUTO_INVOCABLE") - self.assertEqual(sorted(AUTO_INVOCABLE - open_skills), [], - "AUTO_INVOCABLE names a skill that is gated or no longer exists") - if __name__ == "__main__": unittest.main() diff --git a/tools/test_skill_references.py b/tools/test_skill_references.py index 66ec3444..c32321ec 100644 --- a/tools/test_skill_references.py +++ b/tools/test_skill_references.py @@ -3,60 +3,84 @@ import tempfile import unittest from pathlib import Path +from unittest import mock ROOT = Path(__file__).resolve().parents[1] _spec = importlib.util.spec_from_file_location("skill_references", ROOT / "tools" / "skill-references.py") skill_references = importlib.util.module_from_spec(_spec) _spec.loader.exec_module(skill_references) -USER_FACING_SKILLS = frozenset({"poteto-help"}) -USER_FACING = { - "skills/architect/SKILL.md": ["Use for /architect,", '"/architect with checkpoint,"'], - "skills/arena/SKILL.md": ["Use for /arena,"], - "skills/correct/SKILL.md": ["Use for /correct."], - "skills/create-verification-skill/SKILL.md": ["Use for /create-verification-skill,", - "Point the user at `/maintain-verification-skill`"], - "skills/deslop/SKILL.md": ["Use for /deslop,", "and on `/deslop`,"], - "skills/figure-it-out/SKILL.md": ["Use for /figure-it-out or"], - "skills/ketchup/SKILL.md": ["Use for /ketchup.", "the last message they wrote before `/ketchup`"], - "skills/maintain-verification-skill/SKILL.md": ["Use for /maintain-verification-skill", - "point at `/create-verification-skill`"], - "skills/poteto-mode/SKILL.md": ["Use for poteto, /poteto-mode,"], - "skills/pstack-harness/SKILL.md": ["Run `/setup-pstack` to register it.\""], - "skills/reflect/SKILL.md": ['the user says "reflect" or "/reflect"'], - "skills/setup-pstack/SKILL.md": ["Use for /setup-pstack,", "4 of 4 `/poteto-mode` runs", - "I can generate one with /create-verification-skill.\""], - "skills/show-me-your-work/SKILL.md": ["Use for /show-me-your-work,"], - "skills/swarm/SKILL.md": ["Use for /swarm,"], - "skills/technical-writing/SKILL.md": ["Use for /technical-writing"], -} -# A mode's `reminder:` frontmatter reaches the model every turn while that mode is on. It names -# the mode already active, so it is model-facing text that loads nothing. -MODE_REMINDERS = { - "skills/poteto-mode/SKILL.md": ["apply /poteto-mode."], -} -EXEMPT = {rel: USER_FACING.get(rel, []) + MODE_REMINDERS.get(rel, []) for rel in USER_FACING | MODE_REMINDERS} - - -def exempt(rel, spelling, line): - if rel.split("/")[1] in USER_FACING_SKILLS: - return True - return any(spelling in passage and passage in line for passage in EXEMPT.get(rel, ())) + +def tracked_repo(tmp, files): + root = Path(tmp) + for rel, text in files.items(): + (root / rel).parent.mkdir(parents=True, exist_ok=True) + (root / rel).write_text(text) + subprocess.run(["git", "init", "-q", tmp], check=True) + subprocess.run(["git", "-C", tmp, "add", "."], check=True) + return root class SkillReferenceSpelling(unittest.TestCase): def test_model_facing_text_never_spells_a_skill_with_a_slash_or_dollar(self): - found = [f"{rel}:{number}: {spelling}" for rel, number, spelling, line in skill_references.sigils() - if not exempt(rel, spelling, line)] - self.assertEqual(found, [], 'write "Use the **name** skill." as its own sentence or step; a passage that ' - "quotes the user or text shown to the user goes in USER_FACING") + self.assertEqual(skill_references.violations(), [], + 'write "Use the **name** skill." as its own sentence or step; a passage that quotes the ' + "user or text shown to the user goes in USER_FACING in tools/skill-references.py") def test_every_exempt_passage_still_matches(self): - hits = skill_references.sigils() - stale = [f"{rel}: {passage}" for rel, passages in EXEMPT.items() for passage in passages - if not any(r == rel and spelling in passage and passage in line for r, _, spelling, line in hits)] - self.assertEqual(stale, [], "remove USER_FACING or MODE_REMINDERS passages that no longer hold a " - "/name or $name spelling") + self.assertEqual(skill_references.stale_exemptions(), [], + "remove USER_FACING or MODE_REMINDERS passages in tools/skill-references.py that no " + "longer hold a /name or $name spelling") + + def test_a_skill_may_spell_itself_only_on_its_description_line(self): + with tempfile.TemporaryDirectory() as tmp: + root = tracked_repo(tmp, { + "skills/demo/SKILL.md": "---\ndescription: Use for /demo.\n---\nThen run /demo.\n", + "skills/other/SKILL.md": "---\ndescription: Use for /other.\n---\n", + }) + self.assertEqual(skill_references.violations(root), ["skills/demo/SKILL.md:4: /demo"]) + + def test_a_body_line_that_starts_with_description_is_not_exempt(self): + with tempfile.TemporaryDirectory() as tmp: + root = tracked_repo(tmp, { + "skills/demo/SKILL.md": "---\ndescription: Use for /demo.\n---\ndescription: then run /demo again\n", + }) + self.assertEqual(skill_references.violations(root), ["skills/demo/SKILL.md:4: /demo"]) + + def test_a_folded_description_may_spell_its_own_skill_on_any_continuation_line(self): + with tempfile.TemporaryDirectory() as tmp: + root = tracked_repo(tmp, { + "skills/demo/SKILL.md": "---\nname: demo\ndescription: >-\n Build a page.\n Use for /demo, not /other.\nicon: x\n---\nBody.\n", + "skills/other/SKILL.md": "---\ndescription: Use for /other.\n---\n", + }) + self.assertEqual(skill_references.violations(root), ["skills/demo/SKILL.md:5: /other"]) + + def test_a_key_after_a_folded_description_is_not_part_of_it(self): + with tempfile.TemporaryDirectory() as tmp: + root = tracked_repo(tmp, { + "skills/demo/SKILL.md": "---\ndescription: >-\n Build a page.\nreminder: apply /demo.\n---\n", + }) + self.assertEqual(skill_references.violations(root), ["skills/demo/SKILL.md:4: /demo"]) + + def test_a_description_line_may_not_spell_another_skill(self): + with tempfile.TemporaryDirectory() as tmp: + root = tracked_repo(tmp, { + "skills/demo/SKILL.md": "---\ndescription: Use for /demo, then /other.\n---\n", + "skills/other/SKILL.md": "---\ndescription: Use for /other.\n---\n", + }) + self.assertEqual(skill_references.violations(root), ["skills/demo/SKILL.md:2: /other"]) + + def test_an_exempt_passage_missing_from_its_file_is_stale(self): + with tempfile.TemporaryDirectory() as tmp: + root = tracked_repo(tmp, { + "skills/demo/SKILL.md": "Say /demo to start.\n", + "skills/other/SKILL.md": "Nothing here.\n", + }) + table = {"skills/demo/SKILL.md": ["Say /demo to start.", "Say /demo to pause."], + "skills/other/SKILL.md": ["Run /other."]} + with mock.patch.object(skill_references, "EXEMPT", table): + self.assertEqual(skill_references.stale_exemptions(root), + ["skills/demo/SKILL.md: Say /demo to pause.", "skills/other/SKILL.md: Run /other."]) def test_the_scan_skips_untracked_markdown(self): with tempfile.TemporaryDirectory() as tmp: @@ -67,7 +91,7 @@ def test_the_scan_skips_untracked_markdown(self): subprocess.run(["git", "init", "-q", tmp], check=True) subprocess.run(["git", "-C", tmp, "add", "skills/demo/SKILL.md"], check=True) self.assertEqual(skill_references.sigils(root), - [("skills/demo/SKILL.md", 1, "/demo", "Then run /demo.")]) + [("skills/demo/SKILL.md", 1, "/demo", "Then run /demo.", False)]) def test_the_scan_skips_a_tracked_file_deleted_from_disk(self): with tempfile.TemporaryDirectory() as tmp: @@ -79,7 +103,7 @@ def test_the_scan_skips_a_tracked_file_deleted_from_disk(self): subprocess.run(["git", "-C", tmp, "add", "skills"], check=True) (root / "skills/demo/notes.md").unlink() self.assertEqual(skill_references.sigils(root), - [("skills/demo/SKILL.md", 1, "/demo", "Then run /demo.")]) + [("skills/demo/SKILL.md", 1, "/demo", "Then run /demo.", False)]) def test_the_scan_finds_a_slash_and_a_dollar_spelling(self): names = skill_references.skill_names(ROOT)