Skip to content

v1.2.0 — Checks that exercise what they report

Choose a tag to compare

@yottayoshida yottayoshida released this 04 Sep 03:55
· 6 commits to main since this release
d3a0583

Summary: doctor reported on things it had not looked at, and three of those gaps close here. The bundled assets are opened before setup links them, a hook matcher is only answered from a subset both engines were measured to read alike, and — behind a flag, because it means running a string out of a settings file — the registered interpreter can be asked whether mpg actually loads in it.

Added

  • mpg doctor --run-interpreter runs the hook's registered interpreter and reports whether mpg actually loads in it. Without the flag nothing is executed, which is the same behaviour as before and the reason the flag exists: closing this gap means running a string out of a settings file, and a settings file can arrive with a cloned repository. What is checked is the output rather than the exit status — a script that ignores its arguments and exits 0 passes every shape check and every exit-status probe while loading nothing, measured before the check was designed around it — so present requires a line naming mpg and a version. Any version counts: a hook wired to an interpreter holding an older mpg is working, and comparing against the running installation's own version would repeat the mistake the symlink and MCP channels each avoid deliberately. A non-zero exit, output that is not a version line, or an interpreter that cannot be started at all reads degraded — the last one because Claude Code spawning the same hook meets the same wall, so it is a measured failure rather than an unmeasured one; a timeout, a failure to start for reasons of the running process, or more than four distinct commands to try, read unknown. The execution is bounded: no stdin, four kilobytes of output read, five seconds per interpreter, its own process group killed on timeout, an environment holding only PATH and HOME (enough for a pyenv or conda shim to start, and nothing that carries a secret), a scratch working directory, and four interpreters at most. Three of those bounds exist because review broke the first version without them: draining the pipe after a timeout waits on an escaped grandchild that still holds it, and the command never returned; draining it at all reached 2.3 GB resident against cat /dev/zero; and accepting any space-free token as a version let a child print an erase-line escape and rewrite doctor's own verdict on the terminal. What the bounds do not cover is written beside them rather than left to be discovered — a child that calls setsid() outlives the group kill, the network is open to it, and it can write anything the invoking user can write. (closes #236)

Fixed

  • mpg doctor reported present from evidence that did not establish it, and now measures what it claims. The two symlink channels were judged by the name of the target: a directory called modern-python-guidance with nothing in it, or an empty modern-python.md, read as healthy — the decay the module's own docstring opens by naming, a link whose target moved still resolving as a name. Both paths to present now open the target and read a byte through a non-blocking descriptor confirmed to be a regular file, so a SKILL.md symlinked to a fifo cannot hang the command instead of answering it. The hook channel looked at whether the interpreter path existed and at nothing else: it never read args, never read type, and never read the matcher that decides whether the hook fires at all, so a registration pointing at Bash — one that cannot run on an edit — reported healthy, and so did /, which exists and is not a file. It also read only the first mpg entry in the file, while merge_hook has always promised to converge "from ANY starting state", meaning a second, broken registration sat behind a healthy verdict. Every mpg entry is now examined, and the count that matters is per hooked tool rather than per group: matcher: "Edit" beside matcher: "Write" covers what mpg's own matcher covers and is not a duplicate, while two entries inside one group are. Matchers follow the documented rule, where simple characters mean an exact name or a |/,-separated list of exact names and anything else means an unanchored regular expression; what this process cannot evaluate is reported unknown and never degraded, because Claude Code evaluates the regular-expression form in JavaScript and calling a matcher broken on the strength of Python's disagreement would report a working registration as broken. Failing to compile turned out to be only half of that problem, and the narrowing that closes the other half is the entry below. doctor executed nothing at all when this shipped — the interpreter path in a settings file is an arbitrary string, and running it to find out whether the hook works would make a read-only diagnostic a way to run whatever a settings file names, including one that arrived with a cloned repository. The gap that left was stated in the README rather than hidden, and the entry below closes it behind a flag rather than by default. (closes #231, #232, #233, #234)
  • link_state promised in its docstring that a link it cannot walk is answered as stale, and os.readlink sat outside the try that made that true — a second syscall against a path is_symlink() no longer owns, which took the caller down instead of classifying. Moving it inside widened what stale covers, and setup deletes on that verdict, so the fix had to travel: a path that stopped being a symlink is now refused the way the flattened branch already refuses it, rather than unlinked. Measured against the previous code, which printed Agent Skills linked and returned success after deleting the real file it had been pointed at.
  • mpg setup linked a project at a bundled source it had never opened. _find_skills_dir accepted the directory on is_dir() and _find_rule_source accepted the rule on is_file(), so a packaging accident shipping an empty skills/modern-python-guidance/ satisfied both: setup printed Agent Skills linked … and exited 0, and only doctor — run later, if at all — reported that the installation delivers nothing. Both linking call sites now open the source and read a byte before creating the link, using the same content predicate doctor measures links with, and refuse with the hollow path named; --dry-run refuses too, rather than promising a link the real run would decline. The locators themselves stay shape-only, which is the opposite of what the issue proposed: a content predicate inside them stops doctor locating a hollow source at all, so the channel answers unknown ("cannot locate") before reaching the branch that says the link is right and there is nothing behind it — turning a measured breakage into an unmeasured one, and doctor's exit from 1 into 2, against what this file's own [Unreleased] entry above and the README both promise. That direction is now held by a test walking the real locator rather than a patched one; every existing test monkeypatches the locators and none would have caught the regression. The predicate moved to setup_cmd, beside link_state, the other classification setup and doctor share, and opens with getattr(os, "O_NONBLOCK", 0): the bare attribute raises AttributeError on Windows, past every except OSError in the callers, which would have taken mpg setup down before it reached MCP or hook registration — a platform the README already documents as losing the two symlink steps and keeping the rest. scripts/verify_wheel_assets.py gained the same check on both assets; the rule file had none at all, so an empty modern-python.md shipped undetected. Three test fixtures created SKILL.md with touch(), pinning "an installation that delivers nothing sets up fine" — the bug itself — and now write content. (closes #238)
  • mpg doctor answered for an engine it does not run. The regular-expression form of a hook matcher was evaluated with Python's re and the result reported as what Claude Code would do, but Claude Code evaluates it with JavaScript's RegExp, and the two languages differ at the edges: (?P<x>Edit)|Write fires in Python and is a syntax error in JavaScript, so the hook never runs while the channel read present — a silently wrong answer, which is the one failure mode a diagnostic cannot afford. That form is now evaluated only when the pattern is written in a subset measured to mean the same thing in both engines, and only against a plain tool name; anything else reads unknown. The subset was drawn by exhaustive sweep against node and CPython rather than by reasoning about which constructs differ, and the difference mattered: a first subset chosen by hand admitted ( ) + ? and let through every possessive quantifier — Edit*+ fires in Python, which added them in 3.11, while JavaScript has no such syntax — disagreeing on 555 of 69,104 admitted patterns, every disagreement a possessive, on every interpreter requires-python allows. Dropping + and ? makes those unspellable and dropping ( alongside keeps the test single rather than a set plus a (? exception: 271,452 patterns of length five or less, no disagreement. The cost runs one way and is deliberate — matchers that do work, like (Edit|Write) or \d, now read unknown (exit 2, visible) rather than present (silent when wrong), and the channel's detail says what mpg setup --with-hook would do, since a matcher whose evaluation is what failed carries no fix. Two limits are stated rather than implied: the simple form (Edit|Write, Edit, Write) reaches no engine on either side and is still answered from the documented rule, and the subset does not make evaluation terminate — ".*" * 200 + "Z" is admitted and does not answer within 20 seconds, which predates this change and is tracked in #242. (closes #237)