diff --git a/ALWAYS-ON.md b/ALWAYS-ON.md index c2040f0..4b68d09 100644 --- a/ALWAYS-ON.md +++ b/ALWAYS-ON.md @@ -22,7 +22,7 @@ So the suite plus any three other plugins stays under 5,000 tokens. `scripts/rel home folder. `claude plugin details` labels hooks "no model context cost", but a SessionStart hook that returns `additionalContext` does add to context, so it is counted here at characters divided by 3.5. dojo-protocol's ten rules are the only SessionStart text in a default setup. A project's own `DOJO.md` replaces them and is not - counted. + counted. A hook that exits non-zero stops the check with an error: it measured nothing, which is not zero. Both are estimates, not billed usage. Run `python3 scripts/always_on.py --write` to refresh the table below. diff --git a/CHANGELOG.md b/CHANGELOG.md index 4299f7a..e703a93 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -83,6 +83,11 @@ method, the rubric and the tables. It started from five misfires reported from o binaries marked) pins line endings, so a Windows clone with `core.autocrlf=true` gets the same bytes as git stores. Before, such a clone had CRLF files, every byte count on /proof read high, and the website's deploy guard refused the deploy unless `DOJO_PLUGINS_DIR` pointed at an LF export. +- **The always-on check can't pass on a hook that didn't run** — `scripts/always_on.py` counted a SessionStart hook + that exited non-zero as 0 tokens, with no error. On Windows that was every hook, because `cmd.exe` doesn't expand + `${CLAUDE_PLUGIN_ROOT}`: the `always-on` row read `PASS` at about 1,288 when the suite was about 1,856, so it could + have passed over budget. The script now fills in the path itself, and a hook that exits non-zero stops the check + with an error naming the plugin. The suite's real weight hasn't changed. ## Unreleased — 2.3.0 — the desktop pass diff --git a/scripts/always_on.py b/scripts/always_on.py index 20d8a46..e6dbd44 100644 --- a/scripts/always_on.py +++ b/scripts/always_on.py @@ -15,6 +15,7 @@ labels hooks "no model context cost", which is not true of a SessionStart hook that returns additionalContext, so it is counted here. Tokens are characters / 3.5, rounded up: an estimate, close to what the CLI's estimator gives for English prose. + A hook that exits non-zero stops the run: it measured nothing, which is not zero. Budget (ADR 039 D7': "the always-on budget"; the paper's D8 action item): - the suite (every marketplace entry with category "suite", together) is under SUITE_BUDGET; @@ -88,6 +89,8 @@ def injected_text(stdout): def injection_chars(pdir): + # The hook runs from the temporary project below, so the path has to work from there. + pdir = os.path.abspath(pdir) total = 0 with tempfile.TemporaryDirectory() as tmp: proj = os.path.join(tmp, "project") @@ -99,14 +102,20 @@ def injection_chars(pdir): payload = json.dumps({"session_id": "always-on", "cwd": proj, "hook_event_name": "SessionStart", "source": "startup"}) for cmd in session_start_commands(pdir): + # cmd.exe, the shell on Windows, leaves ${...} as it is, so it can't be left to the shell. + cmd = cmd.replace("${CLAUDE_PLUGIN_ROOT}", pdir) try: r = subprocess.run(cmd, shell=True, cwd=proj, env=env, input=payload, - stdout=subprocess.PIPE, stderr=subprocess.DEVNULL, - universal_newlines=True, timeout=30) + stdout=subprocess.PIPE, stderr=subprocess.PIPE, + universal_newlines=True, errors="replace", timeout=30) except (OSError, subprocess.TimeoutExpired) as e: raise RunError("%s: SessionStart hook did not run (%s)" % (os.path.basename(pdir), e.__class__.__name__)) - if r.returncode == 0: - total += len(injected_text(r.stdout)) + if r.returncode != 0: + # Counting this as zero would let the budget pass on a hook that measured nothing. + last = r.stderr.strip().splitlines()[-1:] + raise RunError("%s: SessionStart hook exited %d: %s" % ( + os.path.basename(pdir), r.returncode, last[0] if last else "no error output")) + total += len(injected_text(r.stdout)) return total diff --git a/scripts/tests_suite/test_always_on.py b/scripts/tests_suite/test_always_on.py index 81717ff..83f6857 100644 --- a/scripts/tests_suite/test_always_on.py +++ b/scripts/tests_suite/test_always_on.py @@ -1,14 +1,24 @@ """Tests for scripts/always_on.py, with a stand-in claude binary so no real CLI is needed.""" import json import os +import shutil import stat import subprocess import sys import tempfile import unittest +from unittest import mock + +sys.dont_write_bytecode = True HERE = os.path.dirname(os.path.abspath(__file__)) -SCRIPT = os.path.join(os.path.dirname(HERE), "always_on.py") +SCRIPTS = os.path.dirname(HERE) +REPO_ROOT = os.path.dirname(SCRIPTS) +SCRIPT = os.path.join(SCRIPTS, "always_on.py") +if SCRIPTS not in sys.path: + sys.path.insert(0, SCRIPTS) + +import always_on # noqa: E402 # Prints the "Always-on:" line that `claude --plugin-dir DIR plugin details NAME` prints, # with the number read from DIR/listing.txt. @@ -30,27 +40,34 @@ def __init__(self, tmp): with open(self.claude, "w") as f: f.write(FAKE_CLAUDE) os.chmod(self.claude, os.stat(self.claude).st_mode | stat.S_IEXEC) + if os.name == "nt": # Windows can't start a file by its #! line + with open(self.claude + ".cmd", "w") as f: + f.write('@"%s" "%s" %%*\n' % (sys.executable, self.claude)) + self.claude += ".cmd" - def add(self, name, category, listing, inject=None): + def add(self, name, category, listing, inject=None, exit_code=0, command=None): pdir = os.path.join(self.root, "plugins", name) os.makedirs(os.path.join(pdir, "hooks")) with open(os.path.join(pdir, "listing.txt"), "w") as f: f.write("{:,}".format(listing)) if inject is not None: with open(os.path.join(pdir, "hooks", "say.py"), "w") as f: - f.write("import json\nprint(json.dumps({'hookSpecificOutput': {'hookEventName': 'SessionStart', " - "'additionalContext': 'x' * %d}}))\n" % inject) - hooks = {"hooks": {"SessionStart": [{"hooks": [ - {"type": "command", "command": '%s "${CLAUDE_PLUGIN_ROOT}/hooks/say.py"' % sys.executable}]}]}} + f.write("import json, sys\nprint(json.dumps({'hookSpecificOutput': {'hookEventName': 'SessionStart', " + "'additionalContext': 'x' * %d}}))\n" + "sys.stderr.write('note from the hook\\n')\nsys.exit(%d)\n" % (inject, exit_code)) + command = '%s "${CLAUDE_PLUGIN_ROOT}/hooks/say.py"' % sys.executable + if command is not None: + hooks = {"hooks": {"SessionStart": [{"hooks": [{"type": "command", "command": command}]}]}} with open(os.path.join(pdir, "hooks", "hooks.json"), "w") as f: json.dump(hooks, f) self.plugins.append({"name": name, "source": "./plugins/" + name, "category": category}) with open(os.path.join(self.root, ".claude-plugin", "marketplace.json"), "w") as f: json.dump({"name": "t", "plugins": self.plugins}, f) + return pdir - def run(self, *extra): - return subprocess.run([sys.executable, SCRIPT, "--repo", self.root, "--claude", self.claude] + list(extra), - stdout=subprocess.PIPE, stderr=subprocess.PIPE, universal_newlines=True) + def run(self, *extra, repo=None, cwd=None): + return subprocess.run([sys.executable, SCRIPT, "--repo", repo or self.root, "--claude", self.claude] + list(extra), + stdout=subprocess.PIPE, stderr=subprocess.PIPE, universal_newlines=True, cwd=cwd) class TestAlwaysOn(unittest.TestCase): @@ -103,6 +120,55 @@ def test_write_replaces_only_the_marked_block(self): self.assertNotIn("old", text) self.assertTrue(text.endswith("\noutro\n"), text) + def test_failing_hook_is_could_not_run_not_zero_weight(self): + # 1,500 listed plus 1,000 injected is over budget. Counted as zero, it reads 1,500 and passes. + self.repo.add("s1", "suite", 1500, inject=3500, exit_code=1) + r = self.repo.run() + self.assertEqual(r.returncode, 2, r.stdout + r.stderr) + self.assertIn("s1: SessionStart hook exited 1: note from the hook", r.stderr) + self.assertNotIn("Suite together", r.stdout) + + def test_hook_command_that_cannot_start_is_could_not_run(self): + self.repo.add("s1", "suite", 100, command="no-such-program-for-always-on") + r = self.repo.run() + self.assertEqual(r.returncode, 2, r.stdout + r.stderr) + self.assertIn("s1: SessionStart hook exited", r.stderr) + + def test_hook_that_exits_clean_and_prints_nothing_is_zero_weight(self): + # bring-loop and kata-harness stay quiet in a project that doesn't use them: they ran, and weigh nothing. + self.repo.add("s1", "suite", 1000, command='%s -c "pass"' % sys.executable) + r = self.repo.run() + self.assertEqual(r.returncode, 0, r.stdout + r.stderr) + self.assertIn("| s1 | suite | ~1,000 | 0 | ~1,000 |", r.stdout) + + def test_plugin_root_is_filled_in_before_the_shell_sees_it(self): + # POSIX shells would expand it anyway, so this looks at what the shell is handed, on any platform. + pdir = self.repo.add("s1", "suite", 100, inject=7) + ran = subprocess.CompletedProcess([], 0, stdout="", stderr="") + with mock.patch.object(always_on.subprocess, "run", return_value=ran) as run: + always_on.injection_chars(pdir) + self.assertEqual(run.call_args[0][0], '%s "%s/hooks/say.py"' % (sys.executable, pdir)) + + def test_relative_repo_path_still_reaches_the_hook(self): + # The hook runs from a temporary project, where a path relative to here finds nothing. + self.repo.add("s1", "suite", 1000, inject=700) + r = self.repo.run(repo="repo", cwd=self.tmp.name) + self.assertEqual(r.returncode, 0, r.stdout + r.stderr) + self.assertIn("| s1 | suite | ~1,000 | ~200 | ~1,200 |", r.stdout) + + +class TestMarketplaceHooks(unittest.TestCase): + """The marketplace's real SessionStart hooks, through this machine's real shell. No claude needed.""" + + @unittest.skipUnless(shutil.which("python3"), "the hooks' own commands call python3") + def test_every_hook_runs_and_only_dojo_protocol_adds_text(self): + with open(os.path.join(REPO_ROOT, ".claude-plugin", "marketplace.json"), encoding="utf-8") as f: + names = [p["name"] for p in json.load(f)["plugins"]] + chars = {n: always_on.injection_chars(os.path.join(REPO_ROOT, "plugins", n)) for n in names} + with open(os.path.join(REPO_ROOT, "plugins", "dojo-protocol", "PROTOCOL.md"), encoding="utf-8", newline="") as f: + rules = f.read() + self.assertEqual({n: c for n, c in chars.items() if c}, {"dojo-protocol": len(rules)}) + if __name__ == "__main__": unittest.main()