Port the pack from bash to TypeScript, with tests and CI

[breferrari]

Replaces ~900 lines of bash with TypeScript run directly by Node, and adds the pack's first tests and CI.

Behaviour is byte-identical to the bash, except for two deliberate bug fixes listed under Behaviour changes. 136 golden files recorded from the original implementation enforce that.

The pack contains no shell. 11 runtime modules and 3 pack scripts, with no build step, no dependencies and no lockfile.

---

Why not bash

The pack's failure mode is not pushing, which nobody notices. Bash is unusually good at producing exactly that.

• A hook must never fail a turn, so every hook ends in exit 0. Combined with set -e and an ERR trap, that makes a typo and a healthy no-op indistinguishable.
• Every behavioural rule was duplicated by hand. The filename guardrail regex lived in four files, kept in step by a comment chain reading keep in sync with .... One definition now, with a test pinning the slash command's documented pattern to it.
• None of it was testable. Three push modes, a rebase/retry loop with jitter, and a migration importer that consults deletion history — with no way to exercise a branch short of driving a real Claude session against a real remote.
• The behaviour lived in tool semantics, not in the code. Six defects were hiding in what jq, grep and ls actually do versus what the code looked like it did. All six are below.

What replaces it: one runtime, one string encoding, one path convention, and a type checker with an opinion.

---

No build step

Hooks and scripts are .ts files executed straight by Node:

node --experimental-strip-types --disable-warning=ExperimentalWarning <file>.ts

Node strips type annotations at load and runs the result. There is no compiler, no dist/, no node_modules — the repo is the artifact, which matters for a pack that mcs copies onto engineers' machines with no install step of its own.

Why the flag is passed unconditionally. Type stripping landed in Node 22.6 behind --experimental-strip-types and became the default in 23.6. Passing it is required on Node 22 and a no-op on 24+, so one command string works across the supported range. It is not a deprecated or removed flag — verified on Node 26: the file runs identically with and without it, and the flag is still accepted. CI exercises both halves of this, Node 22 (where the flag does the work) and Node 24 (where it does nothing).

--disable-warning=ExperimentalWarning exists because Node 22/23 print an experimental-feature warning to stderr on every run, which would pollute hook output and any stderr assertion. Node 24+ is silent regardless.

The constraint this buys. Stripping erases types; it cannot transform syntax. So enum, namespace and constructor parameter properties are rejected at runtime — the loader throws on the enum keyword rather than compiling it away. tsconfig.json sets erasableSyntaxOnly, which turns that runtime failure into a typecheck error, and CI runs the typecheck. Both were verified non-vacuous by injecting an enum and a type error and watching each fail.

---

From the mcs side

Hooks here are TypeScript executed through their shebang, which needs the mcs change that stopped prefixing hook commands with bash. This section is what that side has to provide.

What mcs has to do, and all of it except the first line is already true today:

1. Honour hookInterpreter, which each hook component declares. .mts is in mcs's ambiguousExtensions, so inference alone falls back to bash. minMCSVersion is set to 2026.9.3, the release that reads the field; the two are useless apart, and a test asserts they travel together.
2. Set the exec bit — ComponentExecutor already chmods fileType: hook to 0o755.
3. Namespace hooks into <pack-id>/ — DestinationCollisionResolver already does this unconditionally, and the entries depend on it: they resolve the project root three levels up from their own location.
4. Run with the working directory at the project root, as the previously relative bash .claude/hooks/... command already required.

Nothing else changes. ScriptRunner.run already executes pack scripts this way — arguments: [], chmod first, shebang honoured — so configureProject.script and the shellScript doctor checks in this PR need no mcs change at all.

What the pack registers, three entries and one directory:

  - id: runtime                          # the library the entries import
    installAction:
      type: copyPackFile
      source: runtime/lib
      destination: hooks/shared-memories/lib
      fileType: generic

  - id: hook-memories-autopush           # one of three, each the same shape
    hookEvent: Stop
    hookAsync: true
    hookInterpreter: node --experimental-strip-types --disable-warning=ExperimentalWarning
    hook:
      source: runtime/autopush.mts
      destination: autopush.mts

What that produces on disk, and why the layout is what it is:

flowchart TD
    subgraph pack["pack checkout"]
        A["runtime/autopush.mts<br/>imports ./lib/git.mts"]
        B["runtime/lib/*.mts"]
    end

    A -->|"hook: source/destination<br/>chmod 755, namespaced"| C
    B -->|"copyPackFile, fileType: generic"| D

    subgraph inst["&lt;project&gt;/.claude/hooks/shared-memories/"]
        C["autopush.mts · pull.mts · announce.mts"]
        D["lib/*.mts"]
    end

    C -.->|"./lib/git.mts resolves<br/>the same in both trees"| D
    C -->|"../../.. is the project root,<br/>the depth the shim had"| E["&lt;project&gt;/"]

Loading

The entries sit beside their library in the pack and beside it once installed, so ./lib/... needs no build-time rewriting and the repo stays runnable in place.

The shebang, identical on all three, and matching the declared interpreter:

#!/usr/bin/env -S node --experimental-strip-types --disable-warning=ExperimentalWarning

Why .mts rather than .ts. Node resolves module type by walking up to the nearest package.json, and that walk leaves .claude/ and lands in the consumer's own project. A consumer declaring "type": "commonjs" broke every hook with SyntaxError and exit 1; a typeless one printed a warning on stderr on every run. .mts is unconditionally ESM, so module type is a property of the file rather than of wherever it was copied to.

On sequencing. 2026.9.3 is not released yet, so anyone syncing between this merging and that shipping gets a hard refusal from ExternalPackLoader rather than an install. That is the safe direction against hooks that fail on every event, but it is a real window and worth landing close to the release.

One capability is lost with the shim. It opened with command -v node || { echo "node not found; skipping" >&2; exit 0; }, so a missing interpreter degraded to a no-op exactly as the old missing-jq guard did. A shebang cannot do that: with no Node on PATH the exec fails and the hook exits non-zero. The mitigation is the manifest declaring node as a required dependency, the same way it declared jq — but if mcs wants hooks to stay fail-open, swallowing a hook's non-zero exit is the place to do it.

---

How the behaviour contract is enforced

tests/golden/ holds 136 recordings of what the bash produced for a given fixture: stdout, stderr, exit code, and the resulting repository state. The suite runs the TypeScript against them. A drift is therefore a change in what the pack does, not a stale test.

Those recordings were produced by running both implementations against the same fixture — same directory, same git objects, restored in between — and requiring them to agree before the bash was deleted. Reproducing that check needs only the merge base: git show main:hooks/memories_autopush.sh.

Three properties make the goldens worth trusting:

• Non-vacuity. Each fixture carries an expect pattern asserted against the recording, so a fixture where nothing happens fails rather than agreeing with an equally empty result.
• The state snapshot is repo-wide, not scoped to memories/. A snapshot scoped like the code under test is blind to the damage the -- memories/ pathspec exists to prevent.
• Determinism. Fixture commits are stamped at a fixed date, because %cr renders relative to now. Re-recording the whole suite a second time produced zero diffs against the first.

Gate strength: 28 mutations across every module, each verified by content hash to have actually applied and to have been reverted exactly. Every one that is not analytically equivalent turns the suite red.

---

The defects this surfaced

Six places where the obvious port is wrong, because each depends on what a tool does rather than what the code says.

1. jq as a validity gate is a stream parser

echo "$input_data" | jq '.' >/dev/null 2>&1 || { echo "not valid JSON; skipping" >&2; exit 0; }

JSON.parse is the obvious port and it is wrong. To jq, empty input is valid, whitespace-only is valid, and concatenated values ({}{}) are valid; JSON.parse throws on all three. Since that gate decides whether the hook works at all, the naive port converts empty stdin → do the work into exit 0 silently. The port emulates the stream semantics, checked against real jq over 23 inputs.

2. jq does not stop at an erroring value, and its status reflects only the last one

$ jq -r '.tool_input.file_path // empty' <<< '42 {"tool_input":{"file_path":"/a.md"}}'
/a.md      # rc=0 — the type error on 42 did not abort the run

$ jq -r '.tool_input.file_path // empty' <<< '{"tool_input":{"file_path":"/a.md"}} 42'
/a.md      # rc=5 — same output, and the caller's `|| exit 0` now fires

A bad value before a good one still announces; a bad value after one goes silent:

for (const value of values) {
    lastErrored = false;
    if (isObject(value)) toolInput = value["tool_input"] ?? null;
    else if (value !== null) { lastErrored = true; continue; }   // skip, do not abort
}
return lastErrored ? null : out.join("\n");   // silent only if the LAST one errored

3. // empty drops false, and jq -r pretty-prints non-strings

a // b treats null and false as absent. And jq -r renders a non-string result as multi-line JSON — String(v) would give [object Object] where jq gives {\n "x": 1\n}:

const render = (v: unknown): string => (typeof v === "string" ? v : JSON.stringify(v, null, 2));
if (filePath === null || filePath === false) continue;

4. grep's [[:space:]] includes CR

The review report previews the first non-blank line of a memory. In a CRLF file that line is \r — blank to grep, not blank to /^[\t ]*$/:

-const first = text.split("\n").find((l) => !/^[\t ]*$/.test(l)) ?? "";
+const first = text.split("\n").find((l) => !/^[\t\n\v\f\r ]*$/.test(l)) ?? "";

5. ${var:0:80} counts characters; String.slice counts UTF-16 code units

Identical for é, off by half for anything astral:

-return first.replace(CONTROL, "").slice(0, 80);
+return Array.from(first.replace(CONTROL, "")).slice(0, 80).join("");

6. git clone writes progress to stderr, and a piped spawnSync swallows it

The bash leaves the clone unredirected, so Cloning into '...' reaches the user. git() now takes an explicit option so the calls the bash left alone still speak:

git(cwd, ["clone", "--sparse", /* ... */], { inheritStderr: true });

---

Two bugs in the bash, now fixed

These were reproduced faithfully while parity was the working constraint, then fixed. Each is marked as a deviation on its fixture: the golden still records what the bash did, so the departure is visible in the test rather than edited into the recorded data, and a deviation identical to its golden is rejected so the marker cannot go stale.

A first install reported failure. The bash ended with a summary line that could not survive an empty directory:

count=$(ls "$memories_link"/*.md 2>/dev/null | wc -l | tr -d ' ')
echo "Done. $count memory file(s) available at $memories_link."

Under set -euo pipefail an unmatched glob stays literal, ls exits 2, 2>/dev/null hides the message but not the status, pipefail promotes that 2 over wc and tr, and set -e kills the script. So mcs sync against a branch with no memories/ tree yet did all its work correctly, then died silently with status 2 and no "Done." line — the moment a new adopter is watching most closely.

 const mds = entries(link).filter((f) => !f.startsWith(".") && f.endsWith(".md"));
-if (mds.length === 0) process.exit(2);
 out(`Done. ${mds.length} memory file(s) available at ${link}.`);

Zero memories is a normal bootstrap: Done. 0 memory file(s), exit 0. The dotfile exclusion the *.md glob implied is kept.

An unset MCS_PROJECT_PATH targeted the filesystem root. Paths were built by concatenation:

memories_link="$MCS_PROJECT_PATH/.claude/memories"

Empty, that resolves to /.claude/memories, and the script tries to clone into the filesystem root — failing deep inside git rather than saying what was wrong. It now fails fast, beside the existing MEMORIES_REPO_URL guard:

if (project === "") {
    err("MCS_PROJECT_PATH is not set; cannot locate the project's .claude directory.");
    process.exit(1);
}

Falling back to process.cwd() was the alternative and is wrong here. The doctor scripts do that, because their bash read ${MCS_PROJECT_PATH:-$PWD} and they only read. This script clones, moves directories and creates symlinks — silently configuring whichever directory the caller stood in is worse than refusing. Copilot flagged this same line independently and suggested the cwd fallback; that is the reasoning for the guard instead.

---

Behaviour changes in full

Incidental — two, both stderr-only. No stdout byte, exit code, or git side effect changes.

1. The missing-interpreter message names node instead of jq.
2. The abort diagnostic loses its line number — $LINENO and $BASH_COMMAND have no try/catch equivalent.

Deliberate — the two bug fixes above.

Everything else is byte-identical, enforced by the goldens.

---

Coverage the mutations forced

Four places where the suite was green and proving nothing, each now covered:

Gap	How it surfaced
The -- memories/ pathspec	Deleting it from git add -A changed nothing, because the state snapshot was also scoped to memories/. Fixed by widening the snapshot and adding a fixture with a dirty root-level README.md — the documented promise that teammates can edit the memories repo's README safely.
SessionStart's fast-forward	Removing pull --ff-only entirely kept the suite green: no fixture had the remote ahead, so the pack's core job had no test.
The *.md count filter	Every fixture held only .md files, so counting all entries was indistinguishable.
Clone failure exit status	Nothing made a clone fail after a passing preflight.

---

What ships

runtime/                    pull.mts · autopush.mts · announce.mts   (registered hooks)
  lib/                      git · paths · naming · mode · pending · report · push · hook-io
scripts/                    configure-memories.ts · doctor-memories.ts · doctor-memories-remote.ts
tests/                      200 tests · 136 goldens

tsconfig.json is strict with noUncheckedIndexedAccess, exactOptionalPropertyTypes, verbatimModuleSyntax and erasableSyntaxOnly.

CI is macOS × Node 22/24 — mcs is Homebrew-only, so macOS is the only platform that can install this pack. Beyond typecheck and tests it guards three things: that no .sh file exists anywhere in the pack, hostname -s still equalling os.hostname().split(".")[0] (the premise behind commit subjects being unchanged), and a clean working tree afterwards.

The README gains three mermaid diagrams — the sharing loop, the Stop hook's guardrail-then-mode dispatch, and the four paths a migrated file can take — plus a contents line and corrected wording where it described the bash.

---

Not verified

• No real mcs sync has been run. mcs is macOS-only and this was developed on Linux, so the install path is reasoned from the mcs Swift source. Unconfirmed: that mcs pack validate accepts the verbose copyPackFile / fileType: generic component, and that the generic directory copy lands at .claude/shared-memories/. The suite reproduces the installed layout in a temp dir and executes the entry point through its shebang the way mcs will, but cannot exercise mcs itself. This is the one thing worth checking on a Mac before merge.
• Goldens were recorded against bash 5.3 on Linux; the pack targets macOS, whose system bash is 3.2. CI runs on macOS, but only the TypeScript side now that the bash is gone. The harness portability bugs that made the goldens machine-specific are fixed, but no CI run has confirmed it, because a fork PR needs a maintainer to approve the workflow.
• The clone-failure fixture relies on a read-only directory and is skipped as root.

---

Reading this

Commits are ordered to be read in sequence: scaffolding, shared library, hooks, scripts, manifest, tests, CI, docs, then the two bug fixes, the README revamp, and finally the commit that drops the shims. The two worth most attention are the last one (what the mcs change buys) and the tests commit (how the behaviour contract was established).

🤖 Generated with Claude Code

[Copilot]

🟡 Changes recommended

scripts/configure-memories.ts currently treats an unset MCS_PROJECT_PATH as "", which can target /.claude/... and mutate the filesystem root when run outside mcs sync.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Pull request overview

Ports the shared-memories techpack implementation from large bash hooks/scripts to TypeScript executed directly by Node (type-stripped at runtime), and adds a golden-file harness plus CI to lock behavior to the previous bash outputs.

Changes:

• Replaces hook logic with TypeScript runtime modules and keeps only 3 bash hook shims (to satisfy mcs’ bash <hook> execution model).
• Adds a differential/golden harness (tests/golden/*.json) and node:test suites to pin stdout/stderr/exit codes and resulting git state.
• Updates techpack.yaml, docs, and introduces CI (macOS × Node 22/24) and strict TS config for erasable syntax.

File summaries

File	Description
tsconfig.json	Strict TypeScript configuration aligned with Node type-stripping constraints (no emit).
package.json	Defines node:test + typecheck scripts and Node engine requirement.
techpack.yaml	Switches dependency from jq→node and installs runtime as a generic directory copy; rewires scripts to TS.
README.md	Updates documentation to reflect TS runtime, shims, development workflow, and test/golden contract.
.gitignore	Ignores node_modules/ for CI/local typecheck deps.
.github/workflows/ci.yml	Adds macOS CI: typecheck, tests, shim constraints, hostname equivalence, clean tree enforcement.
hooks/memories_pull.sh	Reduces to bash shim that execs TS entrypoint with fail-open missing-node guard.
hooks/memories_autopush.sh	Reduces to bash shim that execs TS entrypoint with fail-open missing-node guard.
hooks/memories_announce.sh	Reduces to bash shim that execs TS entrypoint with fail-open missing-node guard.
scripts/configure-memories.ts	TS port of configure/migration flow for cloning, linking, and importing/migrating memories.
scripts/doctor-memories.ts	TS setup health check for mcs doctor.
scripts/doctor-memories-remote.ts	TS remote reachability/auth probe for mcs doctor.
runtime/lib/git.ts	Centralized git spawning helpers with explicit stderr inheritance option.
runtime/lib/hook-io.ts	Hook stdin parsing + JSON-stream emulation used to match jq behavior.
runtime/lib/mode.ts	Mode parsing with “unknown reported” behavior preserved.
runtime/lib/naming.ts	Single source of truth for guardrail regexes and rename hint.
runtime/lib/paths.ts	Project root and repo path resolution shared by hooks.
runtime/lib/pending.ts	Computes pending state and guardrail inputs scoped to memories/.
runtime/lib/push.ts	Implements pull/rebase/push retry loop with jitter and conflict classification.
runtime/lib/report.ts	Review-mode report rendering + canonical state hashing.
runtime/hooks/pull.ts	SessionStart logic: fast-forward + pending-state additionalContext.
runtime/hooks/autopush.ts	Stop hook logic: auto/full/review behaviors, dedupe, and sync-to-remote.
runtime/hooks/announce.ts	PostToolUse hook: jq-like stream semantics to surface review-mode writes.
tests/harness.ts	Fixture harness to run hooks/scripts, normalize outputs, and snapshot repo state.
tests/unit.test.ts	Unit tests for core library behaviors (mode, naming, retry budget, JSON stream gate, etc.).
tests/differential.test.ts	Hook behavior tests pinned to goldens (parity contract).
tests/configure.test.ts	Configure script behavior tests pinned to goldens (parity contract).
tests/scripts.test.ts	Doctor script behavior tests pinned to goldens (parity contract).
tests/manifest.test.ts	Manifest and installation contract checks (shims, runtime install, references).
tests/golden/*.json (136 files)	Recorded bash behavior contract used to assert TS parity across fixtures.

Review details

• Files reviewed: 166/168 changed files
• Comments generated: 1
• Review effort level: Lite

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[bguidolim]

Requesting changes

The port itself is careful, and I want to say that first because most of what follows is about the edges rather than the translation. I ran 11 injected differential scenarios against both implementations (filenames with spaces, mixed case, non-ASCII, subdirectory-only backups, FIFOs, hard links, relative symlinks, non-ASCII deleted-history entries) and they came back byte-identical including exit codes and resulting .claude/ state. All five setup cases, all 24 stdout/stderr stream assignments, the 20-item cap and every exit path check out. The six tool-semantics defects the description documents are all real, verified against jq 1.7.1, BSD grep, bash 3.2 and git. The JSON stream scanner held across roughly 9,700 differential cases against real jq and never rejected anything jq accepted on well-formed JSON. Dropping the -- memories/ pathspec really is killed by a fixture, exactly as claimed.

Four things need to change before this can merge.

1. The hooks will not run. Dropping the shims moved the interpreter choice onto the shebang, but no shipped mcs consults it. Released 2026.9.2 hardcodes bash .claude/hooks/, and the interpreter branch at ~/Developer/mcs deliberately excludes .ts from inference, so it also falls back to bash. mcs pack validate from that build emits one warning per hook. The fix is a hookInterpreter line on each hook component plus a minMCSVersion that names the release shipping it. Details in the techpack.yaml thread.

2. The installed runtime has no package.json. Module type gets inherited from the consumer's project, so a consumer "type": "commonjs" kills every hook with a SyntaxError and exit 1, and a consumer package.json with no "type" prints a MODULE_TYPELESS_PACKAGE_JSON warning on stderr on every hook run. Both reproduce on Node 26. Fail-closed hooks are the one outcome this pack has always avoided, and the suite structurally cannot see it.

3. configure-memories.ts reports a push that never happened. With no global git identity, which is what a fresh machine looks like when configureProject first runs, the commit fails, the push succeeds vacuously, and the script prints "Pushed migrated memories to the shared branch" and exits 0 with the file staged and uncommitted. The bash exited 128 and turned mcs sync red. Same shape from the unchecked sparse-checkout set and the unchecked git add in the Stop hook.

4. The suite cannot pass on macOS. 37 of 200 fail on a clean checkout, from four harness portability bugs: the /private realpath prefix, ls -A | sort collation differing between glibc and BSD, one golden with the recorder's username baked in, and unscrubbed short SHAs. None is behavioural drift, which is good news about the port and bad news about the goldens, since it means they were recorded somewhere this pack cannot run. GitHub Actions has still never run on this PR, so none of it has been seen.

Not in the diff, so no inline thread

commands/approve-memories.md:17 still says "keep in sync with hooks/memories_autopush.sh allowed_pattern, hooks/memories_announce.sh regex, and scripts/configure-memories.sh allowed_pattern". None of those holds the pattern now, two are gone, and the third is deleted by this PR. The single definition is runtime/lib/naming.ts. This file ships to consumers and Claude reads it when someone runs /approve-memories.

manifest.test.ts has a test called "no stale keep-in-sync comments survive in the TypeScript", written for exactly this, and it scans runtime/lib, runtime/hooks and scripts only. It misses commands/, which is the one place a stale chain actually survived, and its sibling test in the same describe block already opens that file.

Coverage notes worth acting on

Merged V8 coverage across all child processes puts several things at zero: failOpen's catch (delete the try/catch and the suite stays green), the entire restore-on-failure path in the configure script, the Case-1 offline-safe short-circuit (moving preflight ahead of it is green), and push.code !== 1, which is the classification #4 landed. git push --quiet changed to --force survives all 70 hook fixtures, because gitState ignores its project argument, captures no file contents and never inspects the remote.

Also flagged inline, lower priority: the -1 sentinel in GitRun.code escaping into process.exit, ModeResult admitting impossible states, the addedModified projection stored beside its source, the unsound p.length >= 3 predicate, a trailing-newline divergence in announce.ts that loses the review-mode nudge, and a genuine harness flake from a cpSync that is now vestigial.

I have not run mcs sync, so the install path is still unexercised end to end. That remains the right thing to check on a Mac once the interpreter question is settled.

[bguidolim]

I validated this against the interpreter work at ~/Developer/mcs (bruno/hook-interpreters), not just the released binary, and the hooks land on bash either way.

On released 2026.9.2 the prefix is hardcoded (strings $(which mcs) gives bash .claude/hooks/ and nothing else), so the registered command is bash .claude/hooks/shared-memories/pull.ts:

$ bash runtime/pull.ts
runtime/pull.ts: line 2: import: command not found
runtime/pull.ts: line 11: syntax error near unexpected token `NAME,'
exit=2

Every SessionStart, Stop and Write/Edit then exits non-zero, so the pack fails closed on every event.

On the interpreter branch it is still bash, because .ts is deliberately excluded from inference. HookInterpreter.swift puts ts, mts, cts and tsx in ambiguousExtensions, on the grounds that .ts could mean tsx, bun, deno or node with --experimental-strip-types, and guessing wrong is worse than falling back with a warning. mcs pack validate from that build gives one warning per hook saying exactly that.

Adding this line to each of the three hook components, beside hookEvent rather than inside hook:, makes validate pass clean:

hookInterpreter: node --experimental-strip-types --disable-warning=ExperimentalWarning

Three tokens against a maxTokens of 8, and --disable-warning=ExperimentalWarning matches the permitted argument shape. The brew: node component already satisfies the companion check that warns when no component installs the named binary.

minMCSVersion also needs to name the release that ships hookInterpreter. It is currently 2026.4.12, and every mcs before the interpreter release ignores the field silently, which puts the pack straight back on bash.

[bguidolim]

Update: the interpreter support ships in 2026.9.3, so this is now two manifest lines rather than an open question.

minMCSVersion: "2026.9.3"

plus hookInterpreter: node --experimental-strip-types --disable-warning=ExperimentalWarning on each of the three hook components.

I checked what the version gate actually buys, and it is more than documentation. ExternalPackLoader throws incompatibleVersion before anything installs, so with minMCSVersion: "2026.9.3" set and the 2026.9.2 build running:

[ERROR] Invalid pack: Pack 'shared-memories' requires mcs >= 2026.9.3, current is 2026.9.2
0 passed  0 warnings  1 issues

That is the outcome you want. Without the gate, an older mcs ignores hookInterpreter as an unknown key and runs the hooks under bash with no diagnostic anywhere.

Two things worth knowing when you make the change.

The two lines have to land together. hookInterpreter on its own falls back to bash on 2026.9.2 and earlier. The gate on its own refuses to install on older mcs and then still runs under bash on newer, because nothing declares the interpreter. Neither half is useful alone.

Keep the value parseable. VersionCompare wants three numeric components and treats anything it cannot parse as incompatible, so a typo in minMCSVersion makes the pack refuse to load for everyone rather than degrading. CalVer is fine, 2026.9.3 parses as 2026/9/3 and compares the way you would expect.

On sequencing. If this merges before 2026.9.3 is out, anyone who syncs in the gap gets the refusal above and cannot install the pack until they upgrade. That is the safe direction compared to hooks that fail on every event, but it is a real window, so it is worth landing this close to the release rather than well ahead of it.

[breferrari]

Done, and thanks for checking it against the branch rather than just the released binary. I had only read Constants.HookCommand.projectPrefix on main, so I knew the released path was bash, but I would have guessed .ts was inferable and shipped something that silently fell back.

All three components now carry:

hookInterpreter: node --experimental-strip-types --disable-warning=ExperimentalWarning

Superseded by your 2026.9.3 update, replying there on the version gate.

[breferrari]

Both lines are in: minMCSVersion: "2026.9.3" plus hookInterpreter on each of the three hooks. A test asserts they travel together, since either alone is worse than useless.

One thing worth flagging back, because it caught me while fixing the module-type issue in your other thread. I renamed the runtime to .mts so module type stops depending on the consumer project, and mts is in ambiguousExtensions too, so that rename does not remove the need for hookInterpreter. The two threads read as alternatives if skimmed. They are independent and both are required.

On sequencing, agreed and no objection. I checked and 2026.9.2 is the latest tag today, so this cannot merge before the release without giving anyone who syncs in the gap a hard refusal. Your call entirely; I would rather refuse than run under bash on every event.

[bguidolim]

Two things about this destination.

No package.json ships with the lib. It lands at .claude/hooks/shared-memories/lib/, so Node walks up past .claude/ into the consumer's project to decide CommonJS or ESM. Both outcomes reproduce on Node 26 in this layout.

Consumer package.json with "type": "commonjs":

SyntaxError: Cannot use import statement outside a module
exit=1

Consumer package.json with no "type" field, which is the more common case:

[MODULE_TYPELESS_PACKAGE_JSON] Warning: Module type of file://.../pull.ts is not specified
and it doesn't parse as CommonJS. Reparsing as ES module because module syntax was detected.

That prints on stderr on every hook run, and --disable-warning=ExperimentalWarning does not cover it because it is a different warning name. Declaring hookInterpreter does not help either, since node --experimental-strip-types <path> resolves module type exactly the way the shebang does.

Worth knowing when you fix it: a package.json inside lib/ does nothing, because it is a child of the entry point rather than an ancestor. I tested three placements. {"type":"module"} at hooks/shared-memories/package.json works, and renaming the runtime to .mts works with no package.json at all, since .mts is unconditionally ESM. Either also drops the reliance on Node's syntax detection, which was not the default before 22.7.

The suite cannot see this: makeProject creates no ancestor package.json, so it exercises one of the three consumer states and never the one that breaks.

The directory has two sources of truth. The shared-memories/ segment in the hook entry points comes from mcs, not from this manifest: DestinationCollisionResolver phase 0 always namespaces hooks into <pack-id>/, derived from pack.identifier. This component hardcodes the same segment with fileType: generic. They agree today and stop agreeing if the identifier changes. Phase 1b can also split them at sync time: a pre-existing untracked file at .claude/hooks/shared-memories/lib makes mcs retarget this to shared-memories/hooks/shared-memories/lib. It warns when it does, but the hooks then fail at import time rather than install time.

[breferrari]

Confirmed and fixed. This was the one that would have hurt.

Reproduced exactly as you describe on Node 26, in the installed layout:

consumer "type": "commonjs"   SyntaxError: Cannot use import statement outside a module, exit 1
consumer with no "type"       MODULE_TYPELESS_PACKAGE_JSON on stderr, every hook, every run
no package.json at all        works

I went with .mts over shipping a package.json. Both work, and I tested both, but the package.json fix reintroduces the ancestor lookup and would break again the moment that file gets separated from the entries, which is exactly the phase 1b retarget you describe below. .mts makes module type a property of the file.

Your point about the suite being structurally unable to see it was the useful half. makeProject now writes {"type":"commonjs"} at the fixture project root, the hostile case, so all seventy hook fixtures run under it. Renaming one entry back to .ts fails 28 tests; before this it failed none.

On the two sources of truth for shared-memories/: still true, and I have not fixed it. The generic destination has to hardcode the segment mcs derives from pack.identifier, because the entries import ./lib/... and that has to resolve the same in the pack checkout and once installed. A manifest test now asserts the destination tracks the identifier, so renaming the pack fails loudly instead of at import time. That does not help with the phase 1b retarget, which I have no way to detect from this side.

[bguidolim]

The shims carried command -v node >/dev/null || { echo "...: node not found; skipping" >&2; exit 0; }, and the comment above that line called it load-bearing. With the shims gone there is no interpreter guard left, so a missing or too-old node exits 127 or 9 and the hook fails closed.

Related: --experimental-strip-types arrived in Node 22.6, but engines says >=22 and the README says "Node.js 22 or newer". On 22.0 through 22.5 node dies on the unknown flag. CI's node: '22' resolves to current 22.x, so the matrix cannot reach that range. Worth raising the floor to 22.6 in engines, the README and the node component description.

[breferrari]

Right on both, and the guard is genuinely gone. There is no way to fail open on a missing interpreter once the shebang is what selects it: no Node means the exec fails and the hook exits non-zero, where the shim printed a skip line and exited 0. The manifest declaring node as a required dependency is the only mitigation left, same as it was for jq. If mcs wants hooks to stay fail-open on a missing interpreter, swallowing a non-zero hook exit is the place, and I would rather that lived there than in a wrapper per pack.

Node floor raised to 22.6 in engines, the README and the node component. CI pins 22.6.0 as an explicit matrix entry alongside 22 and 24, and the version step now asserts the runtime can actually strip types instead of echoing its version.

[bguidolim]

Neither this git add nor the -A one on line 99 checks its result, and git() pipes stderr into a struct nobody reads, so git's own message is destroyed too. Control reaches the diff --cached --quiet below, which reports nothing staged, committed stays false, unpushed is still 0, and the hook returns having printed nothing at all.

With a stale index.lock in the memories repo, one new memory pending, auto mode:

this branch:  (no output)                                                  exit 0, not pushed
main:         fatal: Unable to create '.../.git/index.lock': File exists.
              memories_autopush: aborted (rc=128) at line 322: git ... add -- "$f"
                                                                           exit 0, not pushed

The bash had these as bare commands under set -e, so a failure printed git's message plus the trap line. This is not a faithful port of a || true. The SessionStart net does fire next session, but it prints the generic "check SSH auth (ssh-add), network, or file naming", which names none of the real causes (locked index, read-only index, ENOSPC, a path outside the sparse cone). For a transient lock that is tolerable. For a persistent one the engineer hunts SSH keys while git add fails every turn with a message the hook threw away.

Cheapest fix is inheritStderr: true on both calls so git at least speaks. Better is to count the failures and print once: "Shared memories: could not stage N file(s); will retry on next Stop."

[breferrari]

Fixed, and the failure mode was worse than not printing. Control fell through to diff --cached --quiet, which correctly reported nothing staged, so committed stayed false and the hook returned having said nothing and pushed nothing. Exactly the silent stop this pack exists to prevent.

Staging goes through a helper now that passes inheritStderr so git speaks, then throws, which failOpen turns into exit 0 with a diagnostic. That is what the trap did.

There is a fixture for it: a stale index.lock with one memory pending. It also turned out to be the first fixture that reaches failOpen, so the catch you flagged as uncovered in the hook-io thread is covered by the same change. Deleting the try/catch now fails.

[bguidolim]

filePaths() never strips a trailing newline, where the bash's file_path=$(...) stripped all of them. Verified against the bash hook with MEMORIES_AUTOPUSH_MODE=review:

{"tool_input":{"file_path":"/p/.claude/memories/learning_a_b.md\n"}}
  bash: emits the full additionalContext      this branch: emits nothing

{"tool_input":{"file_path":"...learning_a_b.md"}} {"tool_input":{"file_path":""}}
  same divergence (jq -r prints one newline per result, so an empty last result adds a blank line)

Review mode loses the Claude-visible nudge, which is the whole job of this hook. One .replace(/\n+$/, "") restores parity. No fixture uses a file_path with a trailing newline or an empty-string file_path.

[breferrari]

Confirmed on both examples and fixed with the .replace(/\n+$/, "") you suggested.

Worth recording that my first attempt to reproduce this disagreed with you and I was wrong, not you. I used printf %b, which put a raw newline inside the JSON string and made the payload invalid, so jq bailed and both sides went silent. With the escape left intact jq emits ...md\n\n and $() strips both, which is your result.

Two fixtures, goldens recorded from the bash: a file_path with a trailing newline, and a valid path followed by an empty-string result.

[bguidolim]

normalizeRun is why 37 of 200 tests fail on macOS, which is the only platform this pack installs on and the only one CI runs. All of it is harness portability and none of it is behavioural drift, but it does mean the goldens were recorded somewhere the pack cannot run.

Three gaps.

root comes from mkdtempSync(tmpdir()), which macOS realpaths to /private/var/..., so paths in the output never match the scrub. Pointing TMPDIR at a non-symlinked directory drops the failures from 37 to 19, which isolates this to 18 of them. realpathSync on the root at creation fixes it.

scrub normalizes the hostname but not the OS username, so tests/golden/edge-a-commit-failure-is-reported-and-retried-next-stop.json carries got 'brenno@<HOST>.(none)' and fails for everyone else, CI included.

Abbreviated SHAs are not scrubbed either. audited-twenty-one-previously-deleted-files-straddle-the-reason-cap.json pins deleted by cd4c2ef, and a different global git config is enough to change the fixture SHAs.

While you are in here: the spawns pass {...process.env} with no GIT_CONFIG_GLOBAL=/dev/null, no GIT_CONFIG_SYSTEM=/dev/null, no explicit -c user.name / -c user.email and no LC_ALL. A global core.hooksPath with a failing pre-commit fails 21 of 25 configure tests, and five goldens embed English git prose verbatim, so a translated git breaks them. push.ts:32 pins LC_ALL=C for the one call whose output it greps, and the harness could use the same discipline for output it compares byte for byte.

[breferrari]

All three fixed, and you were right that this was the real news: the goldens were recorded somewhere the pack cannot run.

mkdtempSync is wrapped in realpathSync, so the /private prefix stops defeating the scrub. normalizeRun scrubs the OS username and abbreviated SHAs as well as the hostname; the two goldens you named no longer carry my account name or cd4c2ef.

On the git config point, spawns now pin GIT_CONFIG_GLOBAL, GIT_CONFIG_SYSTEM and a fixture identity, so a global core.hooksPath cannot reach the tests.

I did not pin LC_ALL, and I want to flag why, because it was my first instinct too. LC_ALL=C switches bash ${var:0:80} from characters to bytes, so it moves the reference implementation rather than stabilising it. The two truncation fixtures failed the moment I set it, which is what they are for. The English-git-prose exposure you mention is real and still there; I did not find a portable way to pin messages without also pinning collation and character semantics.

Goldens re-recorded from the bash with the corrected harness. Twenty-one changed: twenty listing order, two SHAs, one username. Nothing else moved, which I checked by classifying every diff rather than eyeballing the count.

[bguidolim]

This snapshot and the one in runConfigure both shell out to ls -A | sort, and the recorded order is not reproducible on macOS. Twelve configure goldens hold "git": "memories .memories-repo ...", which is glibc collation ignoring the leading dot. BSD sort gives the opposite byte order in every locale I tried (C, C.UTF-8, en_US.UTF-8, en_GB.UTF-8, de_DE.UTF-8):

actual:   '.memories-repo memories \nlearning_shared_seed.md'
expected: 'memories .memories-repo \nlearning_shared_seed.md'

No path is involved, so the realpath issue above does not explain these. This alone accounts for the 17 configure failures that survive a non-symlinked TMPDIR. readdirSync().sort() in Node removes the collation dependence and the shell-out together.

The same shape shows up in the product, though only cosmetically: badNames and stageable use JS .sort() where the bash used sort -u, and under en_US.UTF-8 those disagree (learning_a_b.md before learning_A_b.md in the shell, the reverse in JS). It only reorders the bad-filename warning list, but it does mean "byte-identical" holds in the recorder's locale rather than in general, and the goldens cannot detect it.

[breferrari]

Fixed with readdirSync().sort() in both snapshots, which takes the collation dependence and the shell-out together. The harness no longer spawns a shell at all. Twenty of the twenty-one golden diffs from the re-record were this.

The product-side half you flagged is still there and I left it deliberately. badNames and stageable use JS .sort() where the bash used sort -u, so under a UTF-8 locale they disagree on case ordering. It only reorders the offender list in the guardrail warning, and matching sort -u collation from JS is not something I can do portably. Worth knowing that "byte-identical" holds in the recorder locale rather than in general, which is your point and I would rather it were written down than implied.

[bguidolim]

This cpSync produces a real flake. Three occurrences in about seven runs, each on a different fixture, each unrelated to the code under test:

Error: ENOENT, No such file or directory '.../pristine/project/.claude/.memories-repo/.git/objects'
    at cpSync (node:fs:3823:3)
    at runHook (tests/harness.ts:192:3)

gc.autoDetach and maintenance.auto both default on, so the git commit and git push in makeProject leave a detached maintenance process mutating .git/objects while cpSync walks it. Another consequence of not pinning git config.

The fix is free, because the copy is vestigial now that the bash is gone: pristine is written, read once, and never touched again, so the write, remove and restore round trip is a no-op. Deleting it removes the flake and a chunk of runtime.

The doc comment just above still describes the two-run design ("Both implementations run in the SAME directory, sequentially, restoring the tree in between"), and differential.test.ts's eight "bash and TypeScript agree" describe names read the same way. The header at the top of this file states it correctly.

[breferrari]

Fixed by deleting it, for the reason you give: once only one implementation runs, pristine is written, read once, and never touched again, so the whole round trip is dead weight. That takes the flake with it. I had not connected it to gc.autoDetach walking .git/objects underneath the copy, which explains why it moved between fixtures.

The stale doc comment above it is gone, and the eight describe names now read "behaviour is pinned" rather than "bash and TypeScript agree", which stopped being true when the bash was deleted.

[bguidolim]

This comment describes how a future mcs will run hooks, not how any released one does. Current mcs invokes them as bash <path>, and even the interpreter branch resolves .ts to bash unless the component declares hookInterpreter (see the techpack.yaml thread). So the suite asserts a contract no shipped mcs implements, and it is the only thing standing between the pack and the fail-closed behaviour described there. Worth saying so here, and naming the mcs version the direct-execution path depends on.

[breferrari]

Fair, and it is now true rather than aspirational: with hookInterpreter and the minMCSVersion gate in place, the contract the suite asserts is the one the pack refuses to install without.

I did not add the version to the comment. It would be a fourth place naming 2026.9.3 after the manifest, the README and the manifest test, and the one that nothing checks. The comment says the shebang selects the interpreter and the manifest is what makes mcs honour it, which points at the file that has to stay right.

[bguidolim]

Worth adding 22.6.0 as an explicit matrix entry, or removing the floor question by pinning engines to >=22.6. node: '22' resolves to current 22.x, so the matrix cannot reach the 22.0 to 22.5 range where --experimental-strip-types does not exist.

Two smaller notes on this file. The "tools the shims rely on are present" step asserts git, which the hooks call from TypeScript rather than from a shim, and only echoes node's version without asserting anything about it. Now that the interpreter is the thing in question, node is what is worth asserting. And the hostname equivalence check guards a one-time migration premise that normalizeRun scrubs on both sides anyway, so it can never fail. Fine to keep, but a comment saying it is a migration artifact would stop it reading as permanently load-bearing.

[breferrari]

Done. 22.6.0 is an explicit matrix entry now, alongside 22 and 24, and engines plus the README and the node component description all say 22.6.

The step is rewritten too. It was asserting git, which the hooks call from TypeScript rather than from a shim, and only echoing node version. It now asserts the runtime can actually strip types and fails with the version if not, which is the thing in question.

Hostname check kept with a comment saying it is a migration artifact. You are right that normalizeRun scrubs both sides so it cannot fail; it pinned the premise that commit subjects were unchanged by the port, and that premise has served its purpose.

[breferrari]

Thanks for this. I verified every claim against the code before acting on it rather than taking them on trust, and I could not find one that was wrong. Two of my own spot-checks were wrong before yours were, which is noted in the relevant threads.

All four blocking items are addressed, plus the two out-of-diff findings and every inline note. Replies are on each thread; the short version:

1. Hooks will not run. hookInterpreter on all three components and minMCSVersion: "2026.9.3". A test asserts they travel together. Note that renaming the runtime to .mts for item 2 does not remove the need for this, since mts is in ambiguousExtensions too.

2. No package.json with the installed runtime. Reproduced exactly, including the SyntaxError and exit 1 under a "type": "commonjs" consumer. Fixed with .mts rather than a shipped package.json, because the latter keeps the ancestor lookup and would break again under the phase 1b retarget you describe. The structural half mattered more than the fix: makeProject now writes {"type":"commonjs"} at the fixture project root, so all seventy hook fixtures run under the hostile case. Renaming one entry back to .ts fails 28 tests; before this it failed none.

3. A push that never happened. Confirmed by differential, bash exits 128 where this branch exited 0 and claimed the push. The commit, the git add before it, and the sparse-checkout set in the clone path are all checked now, as is the same shape in the Stop hook.

4. The suite cannot pass on macOS. All four causes fixed: realpath on the temp root, username and abbreviated SHAs scrubbed, readdirSync().sort() instead of ls -A | sort, and pinned git config on every spawn. I did not pin LC_ALL, and that is deliberate: it switches bash ${var:0:80} from characters to bytes, so it moves the reference rather than stabilising it. The truncation fixtures caught that within a minute of my trying.

Goldens re-recorded from the bash with the corrected harness. Twenty-one changed, and I classified every diff rather than counting them: twenty listing order, two SHAs, one username.

Out of diff. commands/approve-memories.md fixed, and the test that was written to catch exactly that now scans commands/ and templates/ as well, since both ship. Reintroducing the old text fails it.

Coverage. failOpen catch is covered, by the locked-index fixture rather than one written for it. gitState records the remote-tracking log and file list now. Two things I could not close and would rather name than bury: git push --force still survives, because pull --rebase makes local a descendant in every reachable state, so it is an equivalent mutant here; and no fixture reaches the sparse-checkout check, same as your note that you could not find a trigger either.

One thing I did not do. The push exit-code classification. It is inherited from 436062b rather than introduced here, so narrowing it is a behaviour change I would rather you decided. Trying to write the fixture produced something sharper than the coverage gap, which is in that thread: I could not construct a push failing with anything other than 1 while the pull one line earlier still succeeds, so the !== 1 branch looks close to unreachable in practice.

206 tests, typecheck clean. GitHub Actions still has not run, since a fork PR from a first-time contributor needs a maintainer to approve the workflow.

[bguidolim]

Approving. The four blockers from last round are closed, and I checked each one rather than taking it on trust: hookInterpreter on all three components plus minMCSVersion: "2026.9.3" (pack validate clean against a build that knows the field), the .mts rename holding on 22.6.0 both with no ancestor package.json and under a hostile {"type":"commonjs"} one, migration add and commit covered by fixture and golden, 206/206 on macOS.

One inline note. It's the only blocking thing, and it makes CI red rather than affecting anyone's install.

The rest is edge-case or cosmetic and fine for a later pass: failOpen can print an EPIPE stack trace if a reader closes early; the .review-shown read fails closed where the bash failed open; git() has no maxBuffer; the harness doesn't pin LC_ALL (25 of 206 fail under a German locale, 206/206 under LC_ALL=C); some docs still say a hook's shebang picks its interpreter; and differential.test.ts's "scalar precedes a valid path" name and comment contradict its own golden.

On your push.exit !== 1 question: it is constructible with remote.origin.pushurl. A valid fetch url with a broken pushurl gives pull --rebase exit 0 and push exit 128. Keep the classification and add a fixture when convenient.

[bguidolim]

This fails on 22.6.0, the floor the PR declares. --check there doesn't apply the .mts-implies-ESM mapping, so it parses the file as CommonJS and dies on the first import: SyntaxError: Cannot use import statement outside a module. All three entry points, so 205/206 on 22.6.0 against 206/206 on current Node. The hooks themselves are fine there (I ran all three, they exit 0 with the right fail-open diagnostics), only --check is broken. ci.yml is new in this PR so that leg has never run, and it goes red on the first run after merge.

--check is parse-only anyway: no type checking, no module resolution. Worth noting that tsconfig.json's include ends in .ts, so it matches no .mts and these three files aren't in the typecheck either. Adding runtime/**/*.mts to include puts them under tsc and makes this gate redundant rather than just deleted.

[bguidolim]

Correcting myself on one line above: CI has run, and this leg is already red on this exact head. It runs on your fork, which is why nothing shows here.

Run 33629821602 on 0770a6bc: test (22.6.0) failure, test (22) and test (24) green, failing on not ok 6 - every entry point parses under type stripping with SyntaxError: Cannot use import statement outside a module, 205 of 206. The last green run was d7261e9, when the matrix was still ['22','24'], so every run since a1b78f5 added 22.6.0 has been red.

No check reaches this PR because it comes from a fork and mcs-cli produces no fork-PR workflow runs (no run, no check suite, not even one pending approval). That is an org-level Actions setting, and separate from this finding. The rest of the comment stands.

[breferrari]

Pushed bc66a4f. Takes the tsconfig route you suggested rather than just deleting the gate, and CI is green on all three legs.

The include half turned out to be worse than a redundant gate. You were right that a pattern ending in .ts matches no .mts, and the consequence reaches past the three entry points: runtime/lib/paths.mts is imported only by autopush.mts and pull.mts, so it arrived through neither the glob nor any test's import graph. tsc --listFiles before the change returned 7 of 11 runtime modules:

runtime/lib/{git,naming,hook-io,mode,push,pending,report}.mts

The missing four — announce.mts, autopush.mts, pull.mts and lib/paths.mts — were outside the typecheck entirely, not merely covered by something weaker. Injecting const x: number = "str" into pull.mts produces no diagnostic before and error TS2322 after.

The gate is pinned rather than dropped. --check is gone and the test now asserts that include reaches runtime/**/*.mts, so removing that line turns the suite red instead of quietly returning those four files to being unchecked.

CI on bc66a4f: run 34151646399 — test (22.6.0) 206/206, test (22) 206/206, test (24) green. First green 22.6.0 leg since a1b78f5 added it to the matrix.

Left alone deliberately, per your "fine for a later pass": the failOpen EPIPE trace, .review-shown failing closed where the bash failed open, git()'s missing maxBuffer, the unpinned LC_ALL, the docs still saying a shebang picks the interpreter, and differential.test.ts's name contradicting its golden. Same for the pushurl fixture — thanks for constructing that case, the classification stays as it is and the fixture can land with them.

The push dismissed your approval, so this needs another look when you have a moment.

[breferrari]

Second round: the whole "later pass" list is in, five commits on top of bc66a4f. CI on b83f04e is 208/208 on all three legs.

.review-shown failing closed was the one worth doing before merge rather than after, and it was slightly worse than the read. The existsSync in front of it made the failure racy as well as possible: EISDIR, EACCES, or a second session's Stop hook removing the file between the check and the read all escaped into failOpen and cost the user the entire report. Dropping the existsSync closes the race instead of narrowing it, since no hash is ever "" and an unreadable file therefore cannot collide with a real one. lastShownHash sits beside preview(), which already had this shape. Removing its try fails the new unit test with EISDIR.

failOpen's EPIPE needed two fixes, not one. The stream handlers cover the asynchronous error event that the try/catch structurally cannot see, and the report write inside the catch is itself wrapped — it could throw for the same reason, which would have escaped the handler that exists to be the last line of defence.

maxBuffer on git(), and on the three harness spawns for the same reason: past 1MB spawnSync kills the child and reports ENOBUFS, which gitOut turns into "". A large enough status --porcelain would have read as "nothing pending".

LC_ALL is pinned to C. Your call was right at the time and is now obsolete for a reason worth stating: nothing in tests/ executes bash any more, only describes it, so the locale can no longer move a reference — it reaches git's prose and collation, and five goldens quote git's English verbatim. The comment records the old reasoning rather than deleting it.

The interpreter docs were staler than the one line. harness.ts:241 claimed mcs "always runs hooks through bash", which is the thing this PR exists to stop; the README still listed the hooks as pull.ts / autopush.ts / announce.ts, which have not existed under those names since the .mts rename, and gave the CI matrix as 22/24. All corrected, and the shebang test renamed to what it actually asserts — that the shebang and the declared interpreter agree.

The leading-scalar fixture is renamed with its golden. Its golden always held the announcement, so the name and comment were the wrong half; the trailing-scalar mirror was right all along.

The pushurl fixture is the one judgment call I'd flag. Your construction reproduces exactly — pull --rebase exit 0, push exit 128 — but I asserted it directly instead of recording a golden, because every golden in that directory is a recording of the bash and the bash is gone. Hand-writing one would have made it claim to be a recording it isn't. If you'd rather it were a golden on the grounds that the TypeScript is the reference now, that's a one-line change and a recorded file.

Nothing here touches install behaviour. Ready for another look.

[bguidolim]

Turned the four small things from the last pass into suggestions you can commit from here, plus the EPIPE test as a paste-in file. None of these block anything.

The README ones are documentation drift the interpreter change left behind. The manifest.test.ts one is the same test with the assertion moved off the pattern string. Both test suggestions are verified against this head, in both directions.

[bguidolim]

The shebang is not what selects the interpreter for hooks. mcs prefixes the declared hookInterpreter, which this PR added, and the declared command also carries --disable-warning=ExperimentalWarning that this line drops. The shebang still matters for pack scripts and for the test harness, just not here.

Suggested change

	Each is executed directly by mcs, with `#!/usr/bin/env -S node --experimental-strip-types` selecting the interpreter. They install to `.claude/hooks/shared-memories/`, with the library they import beside them in `lib/`.
	Each runs as `node --experimental-strip-types --disable-warning=ExperimentalWarning <path>`: mcs prefixes the interpreter the hook declares in `hookInterpreter`, and never looks at the shebang. They install to `.claude/hooks/shared-memories/`, with the library they import beside them in `lib/`.

[bguidolim]

Cosmetic: these three carry one extra space before the #, so their comments sit a column right of every other line in the tree.

Suggested change

	│ ├── pull.mts # SessionStart: pull + stuck-state warning
	│ ├── autopush.mts # Stop: auto-commit + push (async)
	│ ├── announce.mts # PostToolUse: review-mode nudge to Claude (sync)
	│ ├── pull.mts # SessionStart: pull + stuck-state warning
	│ ├── autopush.mts # Stop: auto-commit + push (async)
	│ ├── announce.mts # PostToolUse: review-mode nudge to Claude (sync)

[bguidolim]

"the way mcs does" is the claim tests/harness.ts was just corrected to deny: mcs runs hooks through the declared hookInterpreter, the harness runs them through their shebang. The two agree today only because manifest.test.ts pins them to the same command, which is worth saying instead.

Suggested change

**`tests/golden/` is the behaviour contract.** Each file pins what the pack produces for one fixture: stdout, stderr, exit code, and the resulting repository state. The suite builds a throwaway project with a real git remote, installs the hooks the way mcs does and executes them through their shebang, and compares. A diff therefore means the pack's behaviour changed, not that a test went stale.

**`tests/golden/` is the behaviour contract.** Each file pins what the pack produces for one fixture: stdout, stderr, exit code, and the resulting repository state. The suite builds a throwaway project with a real git remote, installs the hooks, runs them through their shebang (pinned by `tests/manifest.test.ts` to the same command the manifest declares), and compares. A diff therefore means the pack's behaviour changed, not that a test went stale.

[bguidolim]

This pins the literal glob rather than the property it stands for. Broadening include to runtime/**/* keeps every file covered but fails this test, and renaming an entry point loses coverage while it still passes. Resolving the globs against what is actually in runtime/ holds in both directions.

I ran this against the branch: it passes as it stands, still passes with include rewritten to runtime/**/*, and fails with runtime/**/*.mts dropped, naming all eleven files it would leave unchecked. One wart survives either way, so it is worth knowing: JSON.parse breaks the moment someone puts a comment in tsconfig.json.

Suggested change

	test("every entry point is under the typecheck", () => {
	// This was `node --check`, which is parse-only and, on the declared 22.6.0
	// floor, does not apply the `.mts`-implies-ESM mapping: it read every entry
	// point as CommonJS and died on the first import. `tsc` subsumes it, but only
	// if `include` reaches these files -- a pattern ending in `.ts` matches no
	// `.mts`, which left all three plus lib/paths.mts unchecked entirely.
	const include = JSON.parse(readFileSync(join(REPO, "tsconfig.json"), "utf8")).include as string[];
	assert.ok(include.includes("runtime/**/*.mts"), "tsconfig include does not reach the .mts entry points");
	});
	test("every entry point is under the typecheck", () => {
	// This was `node --check`, which is parse-only and, on the declared 22.6.0
	// floor, does not apply the `.mts`-implies-ESM mapping: it read every entry
	// point as CommonJS and died on the first import. `tsc` subsumes it, but only
	// if `include` reaches these files -- a pattern ending in `.ts` matches no
	// `.mts`, which left all three plus lib/paths.mts unchecked entirely.
	const include = JSON.parse(readFileSync(join(REPO, "tsconfig.json"), "utf8")).include as string[];
	// Resolve the globs instead of pinning one pattern string: rewriting
	// `include`, or adding a runtime file, then still has to keep the coverage.
	const reaches = include.map(
	(p) =>
	new RegExp(
	`^${p
	.split("/")
	.map((s) => (s === "**" ? "\0" : s.replace(/[.+^${}()|[\]\\]/g, "\\$&").replace(/\*/g, "[^/]*")))
	.join("/")
	.replace(/\0\//g, "(?:[^/]+/)*")}$`,
	),
	);
	const unreachable = readdirSync(join(REPO, "runtime"), { recursive: true, encoding: "utf8" })
	.filter((f) => f.endsWith(".ts") || f.endsWith(".mts"))
	.map((f) => `runtime/${f}`)
	.filter((f) => !reaches.some((re) => re.test(f)));
	assert.deepEqual(unreachable, [], `tsconfig include does not reach: ${unreachable.join(", ")}`);
	});

[bguidolim]

These two listeners are the fix for a crash, and nothing in the suite would notice if they went away. No suggestion block for this one, because it needs a reader that closes before the hook writes, and that cannot be done through the spawnSync harness. As a new file it works:

import { test, describe } from "node:test";
import assert from "node:assert/strict";
import { spawn } from "node:child_process";
import { chmodSync, cpSync, mkdirSync, rmSync } from "node:fs";
import { dirname, join } from "node:path";
import { fileURLToPath } from "node:url";
import { makeProject } from "./harness.ts";

const REPO = dirname(dirname(fileURLToPath(import.meta.url)));

/** Runs a hook with its stdout reader gone before it writes: the EPIPE case. */
function runWithNoReader(hook: string, env: Record<string, string>, stdin: string) {
	const { root, project } = makeProject();
	const hooks = join(project, ".claude", "hooks", "shared-memories");
	mkdirSync(hooks, { recursive: true });
	cpSync(join(REPO, "runtime", hook), join(hooks, hook));
	chmodSync(join(hooks, hook), 0o755);
	cpSync(join(REPO, "runtime", "lib"), join(hooks, "lib"), { recursive: true });

	return new Promise<{ code: number | null; signal: string | null; stderr: string }>((resolve) => {
		const child = spawn(join(hooks, hook), [], {
			cwd: project,
			env: { ...process.env, ...env },
			stdio: ["pipe", "pipe", "pipe"],
		});
		let stderr = "";
		child.stderr.setEncoding("utf8");
		child.stderr.on("data", (d) => (stderr += d));
		child.stdout.destroy();
		child.stdin.end(stdin);
		child.on("close", (code, signal) => {
			rmSync(root, { recursive: true, force: true });
			resolve({ code, signal, stderr });
		});
	});
}

describe("a hook whose reader has gone away", () => {
	test("announce still exits 0 and reports no stack", async () => {
		const r = await runWithNoReader(
			"announce.mts",
			{ MEMORIES_AUTOPUSH_MODE: "review" },
			JSON.stringify({ tool_input: { file_path: "/p/.claude/memories/learning_a_b.md" } }),
		);
		assert.equal(r.signal, null, "the hook must not die on SIGPIPE");
		assert.equal(r.code, 0);
		assert.doesNotMatch(r.stderr, /EPIPE/, "EPIPE must not surface as a failure");
	});
});

Verified both ways on this branch: it passes as the branch stands, and with these two lines removed it fails with the unhandled write EPIPE and exit 1.

[breferrari]

All five applied in 1f49868, with the suggestions taken byte-exact via the API rather than retyped. CI is 209/209 on all three legs, and the EPIPE test passes on macOS where I could not run it.

I re-ran the include test in all three directions rather than taking the verification on trust: passes as it stands, passes with include rewritten to runtime/**/*, and fails when the .mts pattern is dropped, naming all eleven files. You were right that pinning the pattern string was the weaker gate — a renamed entry point silently losing coverage while the test stayed green is the failure mode that would actually have happened.

The tree alignment was mine. .ts → .mts widened three entries by a character and I did not re-space the comments.

Two notes, neither worth acting on:

readdirSync(..., { recursive: true }) yields the platform separator, so on Windows the failure message reads runtime/lib\git.mts. Message-only — the test is correct in all three directions there anyway — and this pack installs on macOS alone, so normalising it would be a line of code for a platform that cannot run the thing. Flagging it only because it is your file.

The tsconfig.json JSONC wart is recorded in the commit message rather than fixed, per your note. Stripping comments safely means not tripping over // inside a string, which is more machinery than the risk deserves.

Still open from the previous round, and genuinely your call: whether the pushurl case should become a recorded golden on the grounds that the TypeScript is the reference now, rather than the direct assertion it is today.

Nothing here touches install behaviour.

[breferrari]

Closed the three I'd left open, in 7f86a98. 216/216 on all three legs.

The tsconfig.json wart is fixed rather than documented. I'd called it more machinery than the risk deserved; that was wrong, because tsc genuinely accepts comments and trailing commas there — I added both and typechecked clean — so a legal config would have turned the coverage gate into a parse error naming neither the file nor the cause. stripJsonComments scans in one pass, so //, /* and a trailing , inside a string literal are untouched, with a unit test for each and one that parses the pack's own tsconfig.

The separator normalisation is in too. A diagnostic that misreports paths is worth one split/join regardless of which platform sees it.

The pushurl case stays a direct assertion, and that reasoning is now in the file instead of being a question aimed at you. A golden would add exactly one thing over the assertions: git's error prose, byte for byte. Five goldens already quote git's English and that is this suite's known fragility, so a sixth buys nothing for a branch whose contract is an exit code. If you disagree, say so and I'll record it — but you shouldn't have had to arbitrate that one.

No open questions from me now.

[bguidolim]

Thanks for your contribution.