feat(distill): centralized auto-distill health check (Phase A) - #14
Closed
cad0p wants to merge 48 commits into
Closed
feat(distill): centralized auto-distill health check (Phase A)#14cad0p wants to merge 48 commits into
cad0p wants to merge 48 commits into
Conversation
The two napkin kb tools did not register a `renderResult` callback, so `ToolExecutionComponent` fell back to `createResultFallback` which dumps the full text output unconditionally. Ctrl+O had nothing to toggle. Add a shared `formatKbResult` helper that truncates to N lines when `!expanded` and appends a "… (N more lines, Ctrl+O to expand)" hint, mirroring the built-in `read` / `grep` / `ls` / `find` / `bash` tools. Collapsed line counts: - kb_search: 15 lines (matches `grep`) - kb_read: 10 lines (matches `read`)
Remove vault-resolve.ts and use napkin-ai's findVault() directly, which now supports $XDG_CONFIG_HOME/napkin/config.json as a global vault config fallback. This eliminates duplicated vault resolution logic and ensures consistent behavior between the napkin CLI and pi-napkin extensions. - Delete extensions/vault-resolve.ts - napkin-context: use new Napkin(cwd) directly - distill: use new Napkin(cwd).vault.configPath for config access Requires napkin-ai >= 0.9.0 (with global config support).
- Update vault resolution section to reflect napkin-ai's native support - Add migration guide from ~/.pi/agent/napkin.json to ~/.config/napkin/config.json
…notes - Read _about.md files to understand folder purposes - Use relevant folder path when creating notes - Append to today's daily note in the relevant namespace - Minor punctuation cleanup
…nfig (#5) Make the distill model config optional. When omitted, the spawned pi subprocess will use whatever model pi resolves by default (the user's configured default), instead of always falling back to a hardcoded anthropic/claude-sonnet-4-6.
Guide the model on what _about.md files should look like — short folder descriptions explaining what kinds of notes belong there. Points the model at existing examples for style reference.
pnpm v10 blocks lifecycle scripts in dependencies by default unless they are listed in onlyBuiltDependencies. Because napkin-ai is a git-hosted dependency (github:cad0p/napkin), pnpm needs to run its 'prepare' script to build dist/ after fetch. Without the allowlist, pi fails to install pi-napkin with: ERR_PNPM_GIT_DEP_PREPARE_NOT_ALLOWED Failed to prepare git-hosted package ... napkin-ai@0.8.1 needs to execute build scripts but is not in the 'onlyBuiltDependencies' allowlist. Declaring napkin-ai in pnpm.onlyBuiltDependencies lets pnpm build it during install. npm users are unaffected (the 'pnpm' key is ignored).
…update README (#8) * docs(readme): point install snippet at the cad0p forks - Napkin link updated from Michaelliv/napkin to cad0p/napkin. - Install snippet updated: napkin from @cad0p scope with both pnpm and npm variants; pi-napkin installed via git from cad0p/pi-napkin rather than npm:pi-napkin, which resolves to the upstream package and would install the wrong fork. - Minor: drop the -ai suffix from prose references ("napkin's built-in vault resolution") since the fork publishes as @cad0p/napkin. * chore: depend on @cad0p/napkin from npm, drop pnpm build-script workaround Now that @cad0p/napkin is published to the npm registry, pi-napkin depends on it by name + semver range rather than as a git URL. The registry tarball ships `dist/` prebuilt, so there's no `prepare` lifecycle to run at install time and the pnpm.onlyBuiltDependencies workaround from #7 is no longer needed. Version range is `*` to match the other peer deps — @cad0p/napkin is under the same owner, so locking pi-napkin behind a range would just create busywork. npm's range matcher excludes prereleases from `*`, so calver builds from the `next` dist-tag don't accidentally get pulled — consumers stay on the stable `latest` until a base release. - package.json: "napkin-ai": "github:cad0p/napkin" → "@cad0p/napkin": "*" - package.json: drop pnpm.onlyBuiltDependencies (no longer applies) - package-lock.json: regenerated — resolves to the registry tarball - extensions/distill/index.ts: import updated to @cad0p/napkin - extensions/napkin-context/index.ts: same - skills/napkin/SKILL.md: install line now `pnpm add -g @cad0p/napkin` (or `npm install -g @cad0p/napkin`)
…re-vault footgun (#9) * docs(skill): document vault resolution and first-time setup The skill skipped over napkin's vault resolution logic, which contains a silent auto-creation footgun: running any vault-resolving command (e.g. `napkin vault`) from a directory with no ancestor `.napkin/` and no global `~/.config/napkin/config.json` makes napkin create a bare vault at cwd \u2014 `.napkin/` + `NAPKIN.md` + `.obsidian/` \u2014 with no prompt. This is easy to hit at first setup: an agent running `napkin vault` from the user's home or a random project directory litters that directory with empty-vault artifacts. Additions: - New "Vault Resolution" section near the top of the skill, listing the full 4-step resolution order (--vault flag -> ancestor .napkin -> global config -> bare-vault auto-creation), with an explicit warning callout about step 4. - "First-time setup" subsection showing the global config file format (`~/.config/napkin/config.json` with a `vault` field) and how to confirm resolution with `napkin vault --json | jq -r .path` before running commands that might trigger the fallback. - Cleanup instructions for when the bare-vault fallback has already fired: delete the stray `.napkin/`, `NAPKIN.md`, `.obsidian/`. - "Config" section rewritten to distinguish the two configs \u2014 per-vault (`<vault>/.napkin/config.json`, managed by `napkin config`) vs. global vault-path (`~/.config/napkin/config.json`, set manually). - `--vault <path>` flag description now cross-references the Vault Resolution section instead of giving a misleading one-line summary. * docs(skill): fix wrong `napkin version` invocation The skill listed `napkin version` as a subcommand, but the CLI exposes version as a flag `--version` (short: `-v`). Running `napkin version` returns "error: unknown command 'version'".
…10) * chore(lint): sort imports in napkin-context after @cad0p/napkin rename Biome's organizeImports flags the @cad0p/napkin import as out of order since PR #8 renamed the dependency from 'napkin-ai'. The old sort order placed 'napkin-ai' after @mariozechner/*, but the new @cad0p/* scope sorts first. Drive-by fix to unblock CI for downstream branches. * feat(distill): add /distill-auto-this-session to toggle auto-distill Adds a per-session toggle for the automatic distillation timer. State is persisted into the session file via a CustomEntry so that turning auto-distill off, quitting pi, and resuming the same session keeps it off. /distill-auto-this-session toggle on <-> off /distill-auto-this-session on turn auto-distill on /distill-auto-this-session off turn auto-distill off /distill-auto-this-session status report state and time to next run Only suppresses the scheduled timer. Manual /distill still works. Status bar shows 'distill: off (session)' when off for the session, deliberately distinct from 'distill: off' which means distill is disabled in the vault config. On resume (turn auto-distill on), the countdown resets so the next run respects the full interval instead of firing immediately. The in-flight poll loop owns the status bar while a distill runs -- renderIdleStatus short-circuits on isRunning to avoid flicker if the user turns auto-distill off mid-run. If distill is disabled in the vault config, the command warns that the session flag has no effect until vault-level distill is enabled. Transitions use 'info' severity since they're user-requested; 'warning' is reserved for the vault-disabled state and bad args. Persistence: - State is stored as a CustomEntry (type: "custom") with customType 'napkin-distill-session-state' and data { suppressed: bool }. CustomEntry is the documented pi mechanism for extension state persistence (session-format.md); unlike CustomMessageEntry it does not participate in LLM context. - session_start reads the latest matching entry on the current branch (via getBranch(), the canonical state-restoration pattern used by todo/summarize/handoff/qna examples) and restores the flag. Branch- scoped reads avoid leaking state across abandoned branches. - Schema is append-only: readers stop at the first matching customType and fall back to default on malformed data, so a newer writer is never silently shadowed by an older valid entry. - The handler only writes on real state changes, so idempotent calls don't bloat the session file. - Forked sessions inherit the state via SessionManager.forkFrom copying all entries; users can turn auto-distill back on in the fork. Session lifecycle: - autoDistillSuppressed is reset at the top of session_start so it doesn't leak across session switches that take early-return paths. - isRunning is reset in session_shutdown so an in-flight distill at shutdown doesn't stick 'true' across in-process session switches. - On session_start with reason resume/fork/startup, an off session emits a one-time notify so the user doesn't miss the status bar after a long gap.
) * chore(test): add bun test infrastructure pi-napkin had zero tests. Add bun test (matches @cad0p/napkin's stack) so upcoming distill work (shouldDistillOnShutdown predicate, worktree lifecycle) has a place to live. Also migrate package manager from npm to pnpm per repo steering: - Drop package-lock.json in favor of pnpm-lock.yaml - Update CI + release workflows to use pnpm + setup-bun for tests The trivial smoke test in extensions/distill/index.test.ts just wires up bun test. Real distill tests land in later phase-a commits. phase-a * feat(distill): add onShutdown config field (default true) Add `distill.onShutdown: boolean` to DistillConfig with default `true`. Export DistillConfig, VaultConfig, DEFAULT_DISTILL, and loadVaultConfig so config parsing is unit-testable from extensions/distill/index.test.ts. Schema + default only \u2014 nothing reads the field yet. Phase B wires it into the shutdown handler via shouldDistillOnShutdown. Tests cover: - Missing config.json \u2192 defaults - Empty distill block \u2192 onShutdown=true (default wins) - Explicit opt-out (onShutdown=false) - Missing onShutdown \u2192 default=true - Malformed values (null, string, 0) \u2192 !== false \u2192 still runs - Malformed JSON \u2192 full default fallback - Other distill fields preserved when setting onShutdown phase-a * feat(distill): add shouldDistillOnShutdown predicate Pure predicate that decides whether to spawn a final distill at session_shutdown. No I/O \u2014 caller assembles all inputs. Ready for the phase B shutdown handler to consume. Guard order matches the spec (NAPKIN_DISTILL_NO_RECURSE, reload, session suppression, enabled, onShutdown opt-out, sessionFile, size=0, lastSpawnedSize, lastSessionSize). The lastSpawnedSize guard dedupes the "shutdown right after interval fire" race without blocking the "shutdown with new content since last spawn" case. Test coverage: one test per guard (proves isolation), short-circuit order tests (proves guard order), and integration scenarios (typical /quit, /reload, just-spawned dedup, fresh session with no content, disabled config, per-vault opt-out, per-session suppression). Known deviation: pi's SessionShutdownEvent has no `reason` field in 0.67.1 (re-checked in node_modules). The predicate accepts an optional `reason` on a local `ShutdownDistillEvent` interface \u2014 phase B will have to detect reload externally (likely by tracking session_start with reason=reload) and pass it in. Flagged for reviewer. phase-a * feat(distill): track lastSpawnedSize for shutdown dedup Record the session file's byte size at the moment of a successful distill spawn \u2014 set BEFORE the completion poll starts. Separate from `lastSessionSize` (which updates on completion) so that a shutdown firing between spawn-and-complete can dedupe against the in-flight distill without waiting for it to finish. Scaffolding only: written in runDistill, not yet read. Phase B wires it into the shutdown handler via shouldDistillOnShutdown guard 8. No unit test \u2014 runDistill isn't exported and has substantial side effects (fork session, spawn subprocess). Testing in isolation needs heavy mocking; phase B integration tests cover the write-then-read path end to end. Per spec allowance. `biome-ignore noUnusedVariables` documents the temporary state explicitly \u2014 phase B removes the suppression when it adds the read. phase-a * test(distill): widen DEFAULTS types so Partial overrides compile Literal-inferred types (`reason: undefined`, `enabled: true`) in the shared DEFAULTS object made `call({ event: { reason: "reload" } })` and similar override overrides fail strict tsc ("Type 'string' is not assignable to type 'undefined'"). bun test runs them fine at runtime \u2014 this is purely a static-typing issue \u2014 but any future tsc-noEmit check (or editor type-check) would flag them. Widen DEFAULTS to an explicit `Inputs` interface matching the predicate's parameter shapes. No behavior change; all 51 tests still green. phase-a * chore(deps): bump pi peer dep to >=0.68.0 for session_shutdown.reason Phase A's `ShutdownDistillEvent` shim worked around pi 0.67.1 lacking `reason` on `SessionShutdownEvent`. The field was added in pi 0.68.0 (2026-04-20). Bump the peer dep floor to `>=0.68.0` and use pi's native type directly. - package.json: peer `@mariozechner/pi-coding-agent` becomes ">=0.68.0" - pnpm-lock.yaml: refreshed (pulls 0.73.1 currently) - should-distill-on-shutdown.ts: drop local `ShutdownDistillEvent`, accept `Pick<SessionShutdownEvent, "reason">` for caller flexibility - should-distill-on-shutdown.test.ts: update test inputs to canonical reason literals; drop "undefined reason" case (impossible post-0.68) 51 \u2192 50 tests (one merged loop no longer tests impossible values). pnpm install --frozen-lockfile, pnpm lint, pnpm test all green. phase-b * feat(distill): add distill-workspace module for worktree layout Introduce `extensions/distill/distill-workspace.ts` \u2014 the module that manages the on-disk layout used by per-distill isolated workspaces. Milestone 2 of Phase B: no git involvement yet, just the directory layout, session fork, and meta.json. Exports: - `createDistillWorkspace(vault, sourceSessionFile)` \u2014 mkdtemp root, fork session into `<wt>/.napkin/distill/session.jsonl`, write meta - `cleanupDistillWorkspace(worktreePath)` \u2014 idempotent rm -rf - `readDistillMeta(worktreePath)` \u2014 parse meta.json or null on any failure (missing, malformed, incomplete schema) - `generateDistillBranchName(now?, nonceHex?)` \u2014 `distill/<hex6>-<epoch>` - `DistillMeta` interface (pid, vault, branch, startedAt, parentSession) Key implementation choices documented inline: - Branch name carries a 24-bit random nonce so two distills firing in the same second never collide (timestamp-only is a latent race). - `readDistillMeta` never throws on I/O absence; only on unexpected system errors. Callers treat null as "stale or gone". - Tmp dir tag is `napkin-distill-<branch-suffix>-<random>` so orphans are traceable. - Milestone 3 will replace the tmp-dir with a real git worktree; the public API stays. Adds `distill-workspace.test.ts` (16 tests): - branch name format + nonce collision-free at scale - workspace layout, meta.json schema - two workspaces from the same source are independent - throws on missing source, cleans up tmp dir on fork failure - cleanup is idempotent on missing paths - readDistillMeta null-paths for missing \/ malformed \/ partial metadata 51 \u2192 66 tests. pnpm lint, pnpm test, pnpm install --frozen-lockfile all green. phase-b * feat(distill): create per-distill git worktree Milestone 3 of Phase B. Replace the tmp-dir placeholder from milestone 2 with a real git worktree rooted at the vault's HEAD. Each distill now gets its own branch (`distill/<hex6>-<epoch>`) and isolated working tree under `<vault>/.napkin/distill-worktrees/<branch-suffix>/`. New exports in distill-workspace.ts: - `createDistillWorktree(vault, branch, path)` \u2014 `git worktree add -b ...` with vault-has-git-repo precondition (throws DistillError otherwise) - `removeDistillWorktree(vault, path, branch)` \u2014 worktree-first, then branch; prunes stale entries on failure; no-ops if vault gone - `DistillError` class \u2014 distinguishable from stdlib errors - `DISTILL_WORKTREES_SUBDIR` constant (`.napkin/distill-worktrees`) `createDistillWorkspace` now: - Chooses a unique branch name and path under the vault - Calls `createDistillWorktree` before forking the session - Rolls back (`removeDistillWorktree`) if the session fork / meta write fails, so failures never leak a worktree or branch - Signature unchanged: still `(vault, sourceSessionFile) \u2192 handle` `cleanupDistillWorkspace` signature shifts to `(vault, workspace)` because cleanup needs both the vault and the branch name (the branch name only lives on the returned handle, not the path). No callers yet (added in milestone 5), so the API shift is free. Tests: adds per-worktree git setup (init, seed commit) and covers: - worktree add / remove happy path - vault-without-.git throws DistillError - branch-collision throws DistillError - workspace rollback on fork failure (no leaked branch) - cleanup idempotency on missing vault / missing paths - worktree path lives under the vault (important for napkin cwd \u2192 vault resolution in the distill subprocess) 66 \u2192 73 tests. pnpm lint + pnpm test + pnpm install --frozen-lockfile all green. phase-b * feat(distill): add LLM merge driver and git_retry wrapper Milestone 4 of Phase B. Ship the two shell-layer pieces that make worktree-based auto-distill safe: `extensions/distill/scripts/napkin-distill-merge` \u2014 LLM-powered git merge driver. Registered per-worktree via `git config --local`: merge.napkin-distill-merge.name = "napkin distill LLM merge driver" merge.napkin-distill-merge.driver = "<abs-path> %O %A %B %P" and activated by a `*.md merge=napkin-distill-merge` line in `.gitattributes` (appended by `registerMergeDriver` when missing). Driver contract: - Reads base (%O), ours (%A), theirs (%B), filename (%P) - Calls `pi -p <prompt>` (prompt preserves both sides, asks for de-dup and frontmatter validity) - Sanity-checks the output: non-empty, length in [0.3, 3.0]x max input, frontmatter-at-top if all three inputs had frontmatter (syntactic) - 3 strikes on any failure (pi exit !=0, empty, sanity fail) - Exit 0: write resolved content to %A - Exit 1: leave %A unmodified (caller's partial-merge salvage handles) - Reusable test mocks via `NAPKIN_DISTILL_MERGE_MOCK` env var `extensions/distill/scripts/git_retry.sh` \u2014 bash function that retries main-mutating git commands on transient index.lock contention: - Up to NAPKIN_GIT_RETRY_MAX (default 5) attempts - Linear backoff base*attempt (default 0.5s; env-overridable) - Max ~5s total wait \u2014 good for transient locks, tiny blast radius for real failures (exits fast after last attempt) - Uses `awk` for float arithmetic (avoids bc dependency) Integration with distill-workspace.ts: - Add `MERGE_DRIVER_SCRIPT` import via new `scripts-paths.ts` (module that resolves script locations via `import.meta.url`, cwd-independent) - `createDistillWorktree` now calls `registerMergeDriver` after creating the worktree, rolls back the worktree + branch on failure (unregistered driver would defeat the whole merge design) - `.gitattributes` append is idempotent: if the line is already present (future: Phase C's auto-init scaffolds it), no change Tests (`scripts.test.ts`, 12 new tests): - napkin-distill-merge: ok, fail, empty, tiny, huge, no-fm, ok-after-2 (3rd-attempt success), ok-after-3 (3-strike give-up) - git_retry: happy path, always-fail hits retry cap, eventual success, REAL index.lock contention with a backgrounded releaser 73 \u2192 85 tests. pnpm lint + pnpm test + pnpm install --frozen-lockfile all green. phase-b * feat(distill): implement worktree-based auto-distill spawn Adds `spawnDistillInWorktree` \u2014 a detached spawn that performs the full auto-distill lifecycle inside a per-distill git worktree so concurrent distills (interval, shutdown, or multiple pi sessions) don't race on vault files. Shipped: - `extensions/distill/scripts/distill-wrapper.sh`: bash orchestrator that runs pi in the worktree, commits, merges main back, squash-merges to main from the vault cwd, and cleans up via EXIT trap. Takes `<vault> <worktree> <branch> <sessionFork> <prompt> <errorDir> [<model>]`. Writes fatal failures to `<vault.configPath>/distill/errors/<ts>-<pid>-<branch>.log` and always completes cleanup regardless of failure path. - `spawnDistillInWorktree({ vault, sessionFile, prompt, model?, spawnFn? })` in `distill-workspace.ts`: creates the workspace, resolves the vault error dir via Napkin, and spawns the wrapper detached with `NAPKIN_DISTILL_NO_RECURSE=1`. Parent returns synchronously with `{ workspace, pid }` and MUST NOT wait. - `DISTILL_WRAPPER_SCRIPT` export in `scripts-paths.ts` so callers and tests resolve the script path via `import.meta.url` (test cwd agnostic). Why: - Existing `spawnDistill` (temp-dir based) isn't safe under concurrency. The worktree design isolates each distill on its own branch so git's own `.git/index.lock` + the LLM merge driver + git_retry handle interleaved main-HEAD writes. See Phase B Item 5 in the shutdown-distill spec. - Kept `spawnDistill` untouched: the manual `/distill` command still uses it (git-optional), and future work (Phase C) can decide when to migrate it. - Error dir lives on the MAIN vault (resolved via `Napkin(vault).vault.configPath`) because worktrees are removed on cleanup, which would otherwise lose the logs. Self-healing scaffolding (temporary until Phase C's auto-init): - The wrapper's `git add -A` excludes `.napkin/distill/` (session fork + meta.json) and `.napkin/distill-worktrees/` (sibling-worktree pool) via pathspec so they never enter the vault's git history. Phase C replaces this with a committed `.gitignore`. Tests (+9, 85 \u2192 94): - Unit: mock `spawn` to verify positional args, detached flags, env, cwd, errorDir creation, return value shape. - Integration: real wrapper against a tmp git repo with pi stubbed via `NAPKIN_DISTILL_SKIP_PI=1`. Covers happy path (squash commit lands on main), empty distill (no commit, clean exit), two concurrent workspaces (both complete without interference), and missing-arg error (exit 2). Next: Item 6 (partial-merge salvage integration test) and Item 7 (route interval distill through worktree). * feat(distill): partial-merge salvage keeps clean files, reverts conflicted ones to main Adds integration-test coverage for the partial-merge salvage path that already lives in distill-wrapper.sh (shipped with Item 5). The salvage code runs after `git merge main` completes: any file still unmerged \u2014 meaning the LLM merge driver 3-struck on it \u2014 is reverted to main's version via `git checkout main -- <file>`, logged, and staged so the merge commit can complete with partial content. Tests (+2, 94 \u2192 96): - "clean file keeps distill's content, conflicted file reverts to main": Seeds main with `clean.md` + `conflict.md`, creates a workspace, then diverges main's `conflict.md`. Distill mutates both. With `NAPKIN_DISTILL_MERGE_MOCK=fail` forcing 3-strikes, asserts that the squash commit on main keeps main's `conflict.md` content and gets distill's `clean.md` content, and that the error log names the conflicted file + "partial-merge" reason. - "all files conflict: salvage reverts everything, no squash commit created": Verifies the empty-squash guard (Item 5's `git diff --cached --quiet` check on main before commit) survives the salvage path \u2014 when every file gets salvaged back to main's version, the squash has no net change and we correctly skip the commit. Why this matters: - LLM driver failures are a normal operating condition (the spec specifies 3 strikes per file). Without salvage, a single bad merge would abort the whole distill and discard all its work. With salvage, the vault gets the work that merged cleanly and loses only the specific files the LLM couldn't handle. - Error-log semantics: each reverted file gets its own line in the log so the forensic trail is file-granular, not distill-granular. Combined with the dangling-SHA log entry (Item 5), a user can `git cat-file -p <sha>` to recover the distill's version of any salvaged file within the git gc grace window (default 2 weeks). Wrapper behavior confirmed by tests: - Salvage loop runs `git checkout main -- <f>` + `git add -- <f>` per unmerged file, then `git commit --no-edit` to finalize the merge. - MERGE_HEAD check (post-salvage) catches the driver-wrote-conflict-markers edge case and bails with a forensic log. - EXIT trap always cleans up worktree + branch regardless of salvage outcome. * feat(distill): route interval distill through worktree spawn Adds `runAutoDistill(ctx)` alongside the existing `runDistill` (untouched) and switches the interval timer to use it. Manual `/distill` command still calls `runDistill` so vaults without git remain supported. `runAutoDistill`: - Pre-flight check: vault must have `.git/`. Missing git surfaces a one-line status hint ("distill: needs git") and returns without throwing. Phase C's auto-init will ensure git is always present. - Spawns via `spawnDistillInWorktree` (Item 5) \u2014 each call creates an isolated git worktree on a fresh `distill/<hex>-<epoch>` branch. - Polls `workspace.worktreePath` for disappearance as the completion signal: the wrapper's EXIT trap removes the worktree on any exit path. - Bookkeeping mirrors `runDistill` exactly: `lastSpawnedSize` on successful spawn (pre-poll dedup for shutdown) `isRunning` while the worktree exists `lastSessionSize` on completion (post-completion dedup) `lastDistillTimestamp` on completion or no-op skip - On 10-min timeout, calls `cleanupDistillWorkspace` to force-remove the stalled worktree + branch. - Catches `DistillError` from the workspace layer separately from generic spawn failures to surface clearer status-bar text. Imports from distill-workspace: `cleanupDistillWorkspace`, `DistillError`, type `DistillWorkspace`, `spawnDistillInWorktree`. Interval wiring (session_start): before: intervalHandle = setInterval(() => runDistill(ctx), intervalMs) after: intervalHandle = setInterval(() => runAutoDistill(ctx), intervalMs) Manual `/distill` command wiring unchanged \u2014 still calls `runDistill`. Tests (+2, 96 \u2192 98): - `runAutoDistill vs runDistill routing (Item 7) > interval callback creates a worktree`: mocks the ExtensionAPI, stubs globalThis.setInterval to capture the interval callback, invokes it synchronously, and asserts `.napkin/distill-worktrees/<hex>-<epoch>` was created. - `/distill command creates a tmp dir, NOT a worktree`: invokes the registered `/distill` command handler, asserts a `napkin-distill-*` tmp dir appeared under `os.tmpdir()` but no worktree was created under the vault. The test ExtensionAPI mock is intentionally minimal \u2014 only `on()` and `registerCommand()` are exercised by the extension during session_start and command invocation. Other methods are stubbed with no-ops. Race-free observation: both `runAutoDistill` and `runDistill` create their respective artifacts (worktree / tmpDir) synchronously before returning the spawned subprocess handle to the caller. The detached wrapper's cleanup trap runs later, so post-return assertions are stable. * feat(distill): wire shutdown handler to spawn auto-distill in worktree Completes Phase B. The session_shutdown handler now consults `shouldDistillOnShutdown` (Phase A predicate) and spawns a final auto-distill via `spawnDistillInWorktree` (Item 5) when all guards pass, capturing any session work added since the last interval distill \u2014 or all of it, for short sessions that finish before the first interval fires. Handler flow (after existing timer/isRunning cleanup, before uiRef reset): 1. Resolve vault via Napkin(ctx.cwd). If no vault, bail. 2. loadVaultConfig to get current distill config (not captured from session_start \u2014 cost of one JSON parse at shutdown is negligible). 3. statSize the current session file. 4. Call shouldDistillOnShutdown with all assembled inputs. 5. If predicate true AND vault has .git/ AND sessionFile exists: spawnDistillInWorktree({ vault, sessionFile, prompt: DISTILL_PROMPT, model: modelStr }). Set lastSpawnedSize = currentSize so any re-entry within the same closure is a no-op (currentSize === lastSpawnedSize guard). 6. Outer try/catch swallows ALL errors and falls through to the final uiRef reset. Shutdown MUST NEVER block; failures log to stderr only. Guards deferred to shouldDistillOnShutdown: NAPKIN_DISTILL_NO_RECURSE (recursion inside distill subprocess) event.reason === "reload" (session continuing) autoDistillSuppressed (per-session /distill-auto-this-session off) !config.enabled (master switch) config.onShutdown === false (per-vault opt-out) !sessionFile (ephemeral session) currentSize === 0 (nothing happened) currentSize === lastSpawnedSize (interval just grabbed this content) currentSize === lastSessionSize (previous distill completed on it) Additional guards in the handler itself: .git/ must exist in vault \u2014 Phase B doesn't auto-init (Phase C will). Silent skip rather than UI hint because shutdown has no UI to surface to. Also: dropped the temporary biome-ignore on `lastSpawnedSize` now that it's genuinely read in the shutdown handler (was a Phase A breadcrumb). Tests (+9, 98 \u2192 107): - spawns on normal exit with enabled config + git vault + content - does NOT spawn when shutdown reason is 'reload' - does NOT spawn when config.onShutdown=false - does NOT spawn when config.enabled=false - does NOT spawn when session file is empty (size === 0) - does NOT spawn when vault is not a git repo (needs-git guard) - does NOT spawn when NAPKIN_DISTILL_NO_RECURSE is set - shutdown never blocks when vault resolution fails (catches throw) - sets lastSpawnedSize so a re-entry on same content is a no-op Tests observe filesystem state (count of `.napkin/distill-worktrees/` entries) as a proxy for "spawnDistillInWorktree was invoked". The workspace is created synchronously before detached exec, so there's no race between the handler return and the assertion. Phase B exit criteria met: - 107 tests pass (85 before Phase B Item 5) - pnpm lint clean - pnpm install --frozen-lockfile clean - 4 new commits (Items 5\u20138), each SSH-signed, Conventional Commits - Each commit builds + tests green between * fix(test): clear NAPKIN_DISTILL_NO_RECURSE in test beforeEach Tests run inside a distill subprocess (env var set to 1), causing all 'should return true' assertions to fail via guard 1 short-circuit. Add top-level beforeEach/afterEach to each affected describe block to clear and restore the env var around each test. * chore(lint): fix biome formatting in test files Pre-existing formatting issues from fb51fb5 that biome format surfaced. No behavior change. * feat(distill): cleanup stale distill worktrees at session_start Add cleanupStaleWorktrees() to distill-workspace.ts: scans 'git worktree list --porcelain', filters to branches under 'distill/', and removes any worktree whose meta.json is missing, whose pid is dead, or whose mtime is older than 60 minutes. Best-effort — failures on one worktree don't abort the sweep. Wire the helper into session_start's distill-enabled branch, before the per-session pause state is restored. Runs inside try/catch so any internal failure never blocks session startup. Exposes parseWorktreeList, STALE_META_AGE_MS, and a minimal StaleCleanupVault interface for testability. Tests (distill-workspace.test.ts): parseWorktreeList covers porcelain format, detached-HEAD skip, missing trailing newline; cleanupStaleWorktrees covers live-worktree preservation, dead-pid removal, missing-meta removal, stale-mtime removal, non-distill-branch preservation, and mixed-state sweep. 10 new tests (117 total, up from 107). * feat(distill): auto-init git + scaffold .gitignore/.gitattributes for auto-distill New module extensions/distill/auto-setup.ts with ensureVaultReadyForAutoDistill({ contentPath }): SetupResult and a small countTrackedFiles helper used by the first-run notify. Contract: - Idempotent: re-running on a fully-set-up vault returns { initialized: false, scaffolded: [] } with no new commits. - Non-destructive: .gitignore / .gitattributes merges only append lines not already present; user content is preserved verbatim. - Fail-soft: every git / fs failure is translated into SetupResult.error instead of throwing, so callers can notify and keep the session alive. Wiring in index.ts's session_start (distill-enabled branch only, before the existing cleanupStaleWorktrees sweep): - On error: notify with the failure + disable auto-distill for the session (in-memory; no persisted state). - On fresh init: multi-line notify with tracked file count + undo hint (rm -rf .git/) + opt-out hint (distill.enabled: false). - On scaffold-only: single-line notify listing the files added. Tests (auto-setup.test.ts): fresh-vault init+commit, existing-repo scaffold commit, idempotent second run, .gitignore user-content preservation, preseeded .gitignore \u2192 only .gitattributes written, partial-setup fill-in, chmod-based init failure (skipped under root), and countTrackedFiles coverage. 9 new tests (126 total, up from 117). * feat(distill): teach auto-distill about supersedes: frontmatter convention Append a short frontmatter instruction to DISTILL_PROMPT so that when a distill run creates a note that replaces an older one, it records the relationship as: supersedes: ["path/to/old/note.md"] A future janitor (not part of this phase) will use that field to archive superseded notes. Standalone notes leave the field empty or omit it. No behavior change beyond the prompt text. No test \u2014 prompt-only change. * refactor(distill): simplify wrapper git add now that .gitignore excludes distill state Phase B's distill-wrapper.sh used pathspec excludes (:(exclude).napkin/distill, etc.) on git add -A as belt-and-braces, because at that point the vault's .gitignore didn't yet cover auto-distill's ephemeral state. Phase C's auto-setup.ts now installs those rules into the vault's .gitignore at session_start, so the wrapper can drop the pathspec form. Replace the multi-line 'git add -A -- :(exclude)...' with a plain 'git add -A' and update the surrounding comment to point at the new source of truth. Test fixture (spawn-distill-in-worktree.test.ts) pre-seeds a minimal .gitignore that mirrors what auto-setup installs so the integration wrapper tests reproduce the production invariant. Existing 11 wrapper tests still pass unchanged. * fix(distill): auto-setup targets ctx.cwd (matches worktree spawn root) spawnDistillInWorktree and cleanupStaleWorktrees both operate on `ctx.cwd` as the git root. 790314e's auto-setup called into napkin's resolved `contentPath`, which can diverge from `ctx.cwd` on legacy bare vaults where Napkin resolves `contentPath` to the `.napkin/` subdir. The mismatch meant auto-setup would `git init` inside `.napkin/` while worktree code looked for git at the vault root, silently breaking auto-distill on those layouts. Realign auto-setup, countTrackedFiles, and cleanupStaleWorktrees calls to `ctx.cwd`. Update the first-run notify's 'To undo' path so the command it prints actually matches where the repo got created. Shutdown-handler test 'does NOT spawn when vault is not a git repo' becomes 'auto-inits git at session_start' \u2014 Phase C1's auto-init makes the needs-git guard dead in the happy path. The new assertion verifies .git was created at the vault root AND a worktree was spawned (= the full auto-distill chain works end-to-end on a bare vault). * refactor(distill): extract STALE_WORKTREE_MINUTES constant Previously the 60-minute stale-worktree threshold was encoded inline as `60 * 60 * 1000`. Lift to a named `STALE_WORKTREE_MINUTES = 60` so the value has a single source of truth callable from tests and documented in one place (the /60 minute/ margin above MAX_DISTILL_DURATION_MS). STALE_META_AGE_MS is kept as a separate export since existing callers already import it by that name. * feat(distill): add /distill-status slash command Exposes the state of active distill worktrees and unmerged distill branches as a slash command. Humans can now answer 'what's going on with auto-distill?' without grepping git worktree list and reading meta.json by hand. The core state lives in new helpers on distill-workspace: - getActiveDistills(vault): scans `git worktree list` for branches matching `distill/*`, reads each worktree's meta.json, reports pid liveness + elapsed time + start SHA. - getUnmergedDistillBranches(vault): surfaces distill branches that exist as refs but have no active worktree (crashed distill breadcrumbs pending gc). createDistillWorkspace now records the vault's HEAD SHA at worktree-creation time in meta.json so downstream consumers (the before_agent_start overlap detector, next commit) can diff exactly what the distill has written since it forked. The /distill-status command formats the result as plain text, routing through ctx.ui.notify when a UI is present and falling back to console.log in headless mode (pi -p /distill-status ...). * feat(distill): add napkin_distill_status pi tool for agent visibility Exposes the same state as /distill-status (active distill worktrees + unmerged distill branches) through a pi tool so the LLM can pull-query before making vault edits. Tool returns compact JSON: { active: [{ pid, branch, elapsedSeconds, session, alive, ... }], unmerged: ["distill/xyz-456", ...] } Shape matches what the human formatter renders: basename of sessionPath, floored elapsed seconds, explicit alive flag. Output is small (tens of tokens in typical cases) and stable \u2014 new keys may be added, existing keys MUST NOT be renamed or repurposed. When the agent is outside any napkin vault, the tool returns { error: "no vault in cwd" } rather than throwing, so an errant call doesn't abort the turn. Serialiser (distillStatusToJson) and formatter (formatDistillStatus) are exported from index.ts as pure functions and unit-tested directly (distill-status.test.ts) without needing a full ExtensionAPI mock. * feat(distill): inject concurrent-distill notice via before_agent_start When the current pi session has written to vault files that a background auto-distill is also editing, append a one-line notice to the per-turn system prompt so the agent knows its recent writes may be clobbered or merged when the distill completes. Per-turn and ephemeral: uses before_agent_start's 'systemPrompt' return rather than 'message' injection, so 0 tokens are spent when there's no overlap, and nothing is persisted to the session file. Implementation: - session-touched-files.ts: reimplements pi's internal extractFileOpsFromMessage (dist/core/compaction/utils.js) to walk assistant messages and collect paths from 'write' / 'edit' tool calls, plus a conservative bash-redirection heuristic (>, >>, tee) for agents that shell out for writes. Not exported from pi's public API, so we reimplement + pin via version-check test. - intersectFiles / formatOverlapNotice in index.ts: pure functions, unit-tested, handle absolute-vs-relative path mismatches via suffix-match. - before_agent_start handler: env-guarded (skips inside distill subprocess), wraps every step in try/catch so failure collapses to 'no overlap detected' — must never block the agent turn. - diffWorktreeSinceStart: drives the distill-side of the intersection. Uses the startSha recorded in meta.json (Phase C2+) to diff exactly what the distill has committed; falls back to 'git status --porcelain' for legacy meta.json files written before startSha was introduced. Tests: 78 new tests across session-touched-files.test.ts, session-touched-files.version-check.test.ts, overlap-injection.test.ts, and diffWorktreeSinceStart tests in distill-workspace.test.ts. The version-check test asserts pi's internal extractFileOpsFromMessage still exists at the expected path, so upstream breakage surfaces as a test failure instead of silent drift. * docs: invert README/SKILL.md \u2014 README is now the comprehensive doc Before: SKILL.md was 811 lines of comprehensive napkin CLI reference, README.md was 75 lines of install + tiny doc stub. Humans found the README first and got almost nothing; agents found SKILL.md via its frontmatter description and got too much (the CLI reference mixed with pi-napkin setup). After: README.md is the comprehensive doc (249 lines) covering the whole pi-napkin surface \u2014 install, extensions, auto-distill config, concurrency (worktree + LLM merge driver + partial-merge salvage), commands (/distill, /distill-auto-this-session, /distill-status), agent tool (napkin_distill_status), agent visibility (before_agent_start overlap injection), vault setup, troubleshooting, and a pointer to the builder-deleter design. SKILL.md shrinks to 44 lines: frontmatter (needed for pi skill discovery), a 2-sentence preamble, a pointer at README, and the vault resolution section retained verbatim. Vault resolution is the one piece of operational info that MUST stay at the skill level \u2014 misresolution silently creates a bare vault (data-loss hazard). For napkin CLI reference, agents now use `napkin --help` / `napkin <cmd> --help` at runtime. Removing the duplicated reference from SKILL.md prevents drift between the skill doc and napkin's actual CLI surface. Concurrency doc includes the partial-merge salvage behavior per spec Item 20 ("Document partial-merge behavior in README"): when the LLM merge driver fails on some files, those files fall back to main's version while the cleanly-merged files keep distill's content. Error logs land in <vault>/.napkin/distill/errors/ with the dangling distill commit SHA for git-gc-grace recovery. * test(distill): use real pi session_shutdown reason values (G3+C5) Shutdown-handler tests used 'exit'|'switch'|'error' for reason, but pi's SessionShutdownEvent.reason union is 'quit'|'reload'|'new'|'resume'|'fork'. Tests passed by accident (guard 2 only short-circuits on 'reload', other values fall through) rather than by exercising real upstream contract. Replace all fabricated reasons with real ones: - 'exit' -> 'quit' (canonical normal-exit case, 8 call sites) - widen the runLifecycle type to the full union - add explicit 'fork' test so the second non-'reload' branch has coverage Addresses coverage-review G3 and correctness-review C5. * test(distill): add end-to-end wrapper test with real-driver conflict resolution (G1) The previous wrapper tests covered happy-path (disjoint files, driver never fires), concurrent worktrees (disjoint files, sequential), and 3-strike salvage (driver always fails). None exercised the core story: driver fires during 'git merge main', resolves the conflict, and its output survives the squash-merge to main. Add a new describe block with two integration tests: 1. LLM-resolved conflict: driver output lands on main - Seeds a baseline, creates worktree, mutates main to force divergence, stages distill's divergent version, runs wrapper with NAPKIN_DISTILL_MERGE_MOCK=ok so the driver concatenates ours+theirs - Asserts the resolved content contains BOTH sides' markers on main (proves driver fired AND its output reached main) - Asserts no error log entries 2. Driver retries: ok-after-2 succeeds on attempt 3 - Verifies the 3-strike retry loop bridges transient failures Addresses coverage-review G1 (BLOCK). * fix(distill): preserve setup-error safety flag after persistence restore (C1) The session_start handler set 'autoDistillSuppressed = true' inside the setup-failure branch (line 316) then ~40 lines later unconditionally overwrote it with 'readPersistedSuppressed(...)' (line 356). For fresh sessions the persisted value is 'false', so the force-suppression was erased before the interval timer armed. Net effect: when 'git init' fails (readonly FS, permission denied, etc.) the user saw the error notify AND auto-distill still tried to run on every interval tick. Fix: track setup outcome in a 'setupFailed' local, and only re-read the persisted state when setup succeeded. When setup failed, force-suppress regardless \u2014 the issue is vault-level, not user intent, and the next session will re-try setup. Add shutdown-handler test that: - Simulates setup failure by placing a directory where '.gitignore' should be a file (mergeLines hits EISDIR) - Asserts no shutdown distill spawns - The previous implementation would have let persisted=false override the safety flag and spawned. Addresses correctness-review C1. * fix(distill): record wrapper pid in meta.json, not parent pi session pid (C2) createDistillWorkspace() wrote meta.json before the wrapper had been spawned, so 'pid' was the parent pi session's pid \u2014 not the wrapper's. Consequences: - /distill-status.alive reflected whether the parent pi was still running, not whether the distill subprocess was alive. A crashed distill showed alive=true as long as the parent lived. - cleanupStaleWorktrees uses isPidAlive(meta.pid) as its primary signal; with a long-lived orchestrator (cr-auto-action spawns many sub-distills from one pi) dead distill worktrees could accumulate for up to 60 min until the mtime fallback kicked in. Fix: the wrapper rewrites meta.json's pid field to its own pid ($$) immediately after installing its cleanup trap, before any work. The JS side's write becomes a pre-spawn placeholder; within ~100ms of spawn the wrapper's own pid lands in meta.json. sed+mv gives atomicity on the same filesystem. Add NAPKIN_DISTILL_HALT_AFTER_META=1 testing hook so integration tests can inspect the updated meta.json before the cleanup trap wipes it. Test: wrapper rewrites meta.json pid to its own pid (C2). - Creates workspace (meta.pid = process.pid) - Runs wrapper with HALT_AFTER_META - Asserts meta.pid is a different positive number, other meta fields unchanged. Update DistillMeta.pid JSDoc to document the pre-spawn-placeholder + wrapper-rewrite sequence. Addresses correctness-review C2. * fix(distill): detect and respect vault default branch instead of hardcoding main (C3) The wrapper hardcoded 'main' in three places: git merge --no-edit main, git checkout main -- <f>, and implicitly in the squash step (squashes into whatever is checked out in VAULT). Two failure modes: 1. Vaults with 'master' as default (older git, init.defaultBranch=master in global config): every 'git merge main' fails with 'not something we can merge' \u2014 salvage block finds nothing, MERGE_HEAD check passes, squash runs against a distill branch never merged with mainline. 2. Main vault with a feature branch checked out at shutdown: squash lands distill commits on the user's feature branch, corrupting history. Fix JS side: add detectDefaultBranch(vault) helper using 'git symbolic-ref refs/remotes/origin/HEAD' (conventional), falling back to current branch via 'git symbolic-ref --short HEAD', then 'main'. Pass the detected value as the 8th argv to the wrapper (optional, defaults to 'main' to preserve backward compat for direct wrapper invocations). Fix wrapper: accept $8 as DEFAULT_BRANCH, use it for merge + salvage checkout. Add a defensive HEAD check before squash \u2014 if vault HEAD isn't on DEFAULT_BRANCH, refuse to squash and log to error dir. Tests: - unit: wrapper spawn call has 'main' at argv[8] for a main-default vault. - integration: detectDefaultBranch returns 'master' for a master-default vault; wrapper squash-merges into master when invoked with argv[8]=master. Addresses correctness-review C3. * refactor(distill): migrate legacy spawnDistill to argv-based spawn (SEC-1) The legacy `spawnDistill` built a shell command string and ran it via spawn('sh', ['-c', cmd]). Today's inputs are safe (constant prompt, shellEscape applied), but the shape was a regression-magnet: any future caller that swapped DISTILL_PROMPT for a user-constructed string, or funneled config.model.id through without shell-escaping, would produce a shell-injection hazard that's easy to miss in review. Extract the 'pi ... >/dev/null 2>&1; rm -rf <tmp>' shell body into scripts/distill-wrapper-legacy.sh and spawn it as spawn('sh', [script, sessionFile, tmpDir, prompt, model]). Every variable is a positional argv entry \u2014 no interpolation, no escaping, no injection surface. Matches the argv style already used by the worktree spawn path. Export spawnDistill (was a module-local function) and accept an optional spawnFn override to support unit testing. Legacy path remains git-optional; behaviour preserved \u2014 manual `/distill` in a non-git vault still works. Tests (spawn-distill-legacy.test.ts): - No "-c" argv - positional argv order: [script, sessionFile, tmpDir, prompt, model] - empty model -> empty string at argv[4] - detached + stdio:ignore + NAPKIN_DISTILL_NO_RECURSE - shell-metacharacter values stay isolated as argv, never concatenated Remove the now-unused shellEscape helper. Addresses security-review SEC-1. * test(distill): integration test for before_agent_start overlap injection (G2) overlap-injection.test.ts only covered the pure helpers (extractFileOpsFromMessage, intersectFiles, formatOverlapNotice). The handler wiring (env guard, vault resolution, session walking, active- distill filtering, diff union, systemPrompt append) had no direct test coverage \u2014 author comment said 'covered indirectly' but none of these code paths were exercised. Add overlap-injection.integration.test.ts with 7 tests: - overlap exists -> returns {systemPrompt: prefix + notice} - no overlap -> handler returns undefined - no active distills -> handler returns undefined - NAPKIN_DISTILL_NO_RECURSE=1 -> handler short-circuits even with overlap - dead distill (pid=999999) -> filtered out by alive check - empty session -> handler returns undefined before touching git - bogus ctx.cwd -> handler returns undefined, no throw The 'overlap exists' test uncovered a latent bug: the handler resolved vaultPath via 'new Napkin(ctx.cwd).vault.contentPath' which points at '<root>/.napkin' for content-layout vaults. getActiveDistills then checked '<root>/.napkin/.git' which doesn't exist, so the handler silently returned no overlap for every real vault. Fix: use ctx.cwd directly (the git root) for vaultPath; keep the 'new Napkin(ctx.cwd)' call for its throw-on-unresolvable semantics. Addresses coverage-review G2. * test(distill): cover wrapper MERGE_HEAD escape-hatch (G4) The wrapper's last-line defense at distill-wrapper.sh (the check for MERGE_HEAD still being present after the driver + salvage) was dead code in CI. No mock mode emitted conflict markers with exit 0, and when I tried that approach git actually cleared MERGE_HEAD on driver exit 0 (the driver's output gets committed even if it contains markers). Add an explicit NAPKIN_DISTILL_FORCE_MERGE_HEAD=1 testing hook that creates MERGE_HEAD immediately before the escape-hatch check. Also add a conflict-markers mock mode for completeness (emits exit 0 output with raw <<<<<<</=======/>>>>>>> markers). Integration test asserts: - wrapper exits 1 when MERGE_HEAD is present at the check - error log contains 'merge did not complete (MERGE_HEAD still present)' - error log contains the dangling SHA for forensic recovery - no distill squash commit lands on main Addresses coverage-review G4. * fix(distill): quote merge-driver path in git config for space-safety (SEC-2+C4) registerMergeDriver stored the driver path as part of a shell command string that git runs via sh -c when a merge fires. Unquoted, any path containing a space (e.g. '/Users/Foo Bar/...' on macOS display-name home dirs, or runner-controlled paths in CI) would word-split at the first space and the driver would silently fail. A malicious path prefix could also inject shell arguments. Fix: single-quote the path and escape any literal single-quote inside it as '\''. Git's sh -c now sees the path as one token. Tests: - config readback confirms the driver value starts with ' and ends with ' %O %A %B %P - integration: worktree creation succeeds when the vault path contains a space Addresses correctness-review C4 and security-review SEC-2. * fix(distill): delimit merge driver inputs to prevent prompt injection (SEC-3) The LLM merge driver fenced file contents between unfenced pseudo- markers ('<<<<<<<< BASE', '<<<<<<<< OURS', '<<<<<<<< THEIRS'). Any file already containing these markers (or a crafted prompt-injection payload that synthesises them) could terminate the section and speak to the LLM as the outer prompt author. Concrete attack path: a note whose body ended with a fake '========' + 'OURS' section could instruct the LLM to emit chosen content. Since auto-distill from one distill becomes input to the next distill's merge driver, content an earlier agent wrote gets fed back \u2014 a worm pattern. Fix: generate per-invocation random 16-hex delimiters for BASE / OURS / THEIRS sections. Regenerate if any delimiter accidentally appears in the inputs (astronomically unlikely). Also add an explicit instruction at the top of the prompt telling the LLM to treat content between markers strictly as data and not to follow instructions inside. Test (scripts.test.ts): - Feed inputs that contain the OLD static marker strings. With random delimiters the merge still succeeds and output is preserved (not replaced by an injected instruction). Addresses security-review SEC-3. * refactor(distill): extract common runner from runDistill/runAutoDistill (CLN-1) runDistill and runAutoDistill were 1:1 duplicates of a ~100-line polling + bookkeeping sequence, differing only in: 1. Pre-flight (runAutoDistill requires .git, runDistill is git-optional) 2. Spawn call (spawnDistill vs spawnDistillInWorktree) 3. Poll-completion target (tmpDir vs worktree path) 4. Timeout cleanup (fs.rmSync vs cleanupDistillWorkspace) The Phase B commit had to remember to update both bookkeeping paths; a future bug fix touching one path risked missing the other. Extract runDistillWith(ctx, strategy) that owns the shared lifecycle: config load, size dedup, status-bar painting, poll loop wiring, completion bookkeeping. The two callers supply a DistillStrategy with: - preflight?: optional short-circuit check - spawnFn: returns { target, cleanup } on success, null on failure runDistill now: 10 lines (strategy setup). runAutoDistill now: ~35 lines (strategy + DistillError-specific status). No behaviour change \u2014 all 226 tests still pass. Visible state mutations (isRunning, lastSessionSize, lastSpawnedSize, lastDistillTimestamp, pollHandle) remain in the closure, so existing closures-over-state contracts (shutdown handler, /distill, interval) are preserved. Addresses security-review CLN-1. * fix(distill): wrapper distinguishes conflict-remaining from real merge failure (C6) The wrapper swallowed ALL non-zero exits from 'git merge main' via '|| true', then let the salvage path decide from the file-level --diff-filter=U output. This masks real merge failures: - exit 128 (corrupt index, invalid ref, refusing to overwrite tracked files, etc.): the merge never started. salvage finds no unmerged files and falls through to squash, which silently lands against a distill branch that was never actually synced with mainline. - Race with concurrent distills: if index.lock contention outlasts the retry budget, exit 128 \u2014 same silent corruption window. Capture the merge exit code explicitly and branch on it: - 0: clean merge, continue. - 1: conflicts remain, proceed to salvage (expected). - other: log 'failed unexpectedly (exit N)' + dangling SHA + exit 1. Also: drop the git_retry wrapper around this one call. Retrying a partially-failed merge yields 'you have unmerged files' (exit 128) on the second attempt, which would now false-positive into the unexpected-exit branch. Transient index.lock on this single call isn't a real concern \u2014 we're the sole writer on the distill branch. Add NAPKIN_DISTILL_FORCE_MERGE_RC=<n> testing hook so tests can exercise the unexpected-exit branch without contriving a real git failure of that shape. Test (C6): force merge_rc=128 via hook, assert wrapper exits 1, error log contains 'failed unexpectedly (exit 128)' + 'aborting' + dangling SHA, no squash commit lands. Addresses correctness-review C6. * fix(distill): add timeout on merge driver's pi invocation (C7) The LLM merge driver shelled out to `pi` with no timeout. If the nested pi hangs (network stall, provider throttle, stuck stdin, infinite model loop), each attempt blocks indefinitely. 3 attempts per conflicted file means a single hung file can exhaust the parent wrapper's 10-min MAX_DISTILL_DURATION_MS budget \u2014 and when that fires, the worktree is yanked while the pi call is still active. Fix: wrap the real-path pi invocation in coreutils `timeout`, falling back to perl's alarm() when timeout isn't installed. Default 60s per attempt, overridable via NAPKIN_DISTILL_MERGE_TIMEOUT_SECS. If neither timeout binary nor perl is present (unusual), the call runs unbounded (acknowledged best-effort). Test (scripts.test.ts, 3s wall time): - Place a stub `pi` in PATH that sleeps 30s - Run driver with NAPKIN_DISTILL_MERGE_TIMEOUT_SECS=1 - Assert driver exits non-zero within 15s (proves timeout fired three times before retry budget exhausted, not that stub's sleep completed) - Skip cleanly if neither timeout nor perl is available on the runner. Addresses correctness-review C7. * feat(distill): add common secret patterns to auto-scaffold .gitignore (SEC-5) On a fresh vault with distill.enabled=true, auto-setup runs 'git add .' on first init. The scaffolded .gitignore covered napkin/Obsidian state and .DS_Store but nothing else \u2014 a .env, id_rsa, cert.pem, or similar in the vault root would be permanently captured in the initial commit (two-week gc grace doesn't help for tracked files). An Obsidian git plugin or manual push would then expose the secret. Extend GITIGNORE_LINES with standard secret patterns: - .env, .env.local, .env.*.local - *.pem, *.key - id_rsa, id_ecdsa, id_ed25519 - secrets.json - .aws/credentials Tests (auto-setup.test.ts): - Fresh vault: .gitignore contains all 10 secret patterns. - Fresh vault with pre-existing .env, id_rsa, cert.pem alongside a note: initial commit tracks the note but NOT the secrets. Addresses security-review SEC-5. * fix(distill): invoke wrapper via bash + propagate env for CI portability Two portability fixes so the distill extension works on Ubuntu runners (and Ubuntu/Debian users in general): 1. Spawn the distill wrappers with `bash` instead of `sh`. The wrappers have a `#!/usr/bin/env bash` shebang and use bash-specific syntax (arrays, `set -o pipefail`). On Ubuntu, `/bin/sh` is `dash`, which parse-errors on bash syntax and exits 2 before any wrapper logic runs. This is a latent production bug, not only a test issue. 2. Pass `env: process.env` explicitly in auto-setup's `runGit`. Unlike Node, Bun's `spawnSync` does not propagate mutations to process.env to the child unless env is passed explicitly — it snapshots env at runtime startup. Passing process.env is a no-op on Node and restores Node-compat semantics on Bun so tests can control git identity via GIT_AUTHOR_* env vars. 3. In shutdown-handler.test.ts, set dummy GIT_AUTHOR_* env vars in beforeEach so the Phase C1 test (production does `git init` + `git commit` on a fresh vault) succeeds on CI runners without a global ~/.gitconfig. Local users with a global gitconfig are unaffected; this only kicks in when no `~/.gitconfig` is present (typical of CI runners). Fixes the 16 failing tests on PR #11 CI: - 11 distill-wrapper.sh integration tests (dash parse error) - 4 ensureVaultReadyForAutoDistill tests (env not propagated on Bun) - 1 session_shutdown Phase C1 test (no identity available) * chore(deps): migrate from @mariozechner to @earendil-works pi-coding-agent scope pi-coding-agent and pi-tui were renamed from @mariozechner to @earendil-works starting with 0.74.0. Migrate all imports, peer dependency declarations, comments, and version-check test paths to the new scope. Pin >=0.74.0 to pick up the session_shutdown.reason field and the SIGTERM handling fix we rely on in shutdown-handler. - package.json peerDeps: @earendil-works/pi-coding-agent >=0.74.0, @earendil-works/pi-tui * - All TS imports updated across extensions/distill and extensions/napkin-context - session-touched-files.version-check.test.ts path updated - Comments / JSDoc references updated Verified: bun test (230 pass), bunx biome check (clean). * refactor(distill): replace any types with precise UIRef / RunCtx types (CLN-2) The distill extension stored `ctx.ui` and the shared runner's narrowed context as `any`, suppressing type-checking at the pi boundary. Replace both with precise types that track pi's public `ExtensionContext` / `ExtensionUIContext` surfaces: - `DistillUIRef.ui` is now `ExtensionUIContext` (gives us setStatus and theme type-checked, and any upstream rename surfaces as a compile error rather than silently widening). - `RunCtx` becomes `Pick<ExtensionContext, 'sessionManager' | 'hasUI' | 'ui' | 'cwd'>` so the four fields we actually read stay in sync with pi at the type level. No behavior change; no test changes. Verified: bun test (230 pass), bunx biome check (clean). * refactor(distill): consolidate worktree enumeration to single git call (CLN-3) `getActiveDistills` and `getUnmergedDistillBranches` both invoked `git worktree list --porcelain` independently. The /distill-status handler (collectDistillStatus) calls them back-to-back, so on every invocation pi ran two identical worktree enumerations. Introduce `getDistillState(vault)` that does exactly one `git worktree list --porcelain` + one `git branch --list 'distill/*'` and returns `{ active, unmerged }` derived from the same snapshot. Retain the two original functions as thin wrappers that call `getDistillState` and destructure — keeps the public API intact for existing tests and downstream callers. - collectDistillStatus (/distill-status + napkin_distill_status tool) migrated to the new call; saves one git invocation per status refresh. - Added describe block with 4 tests exercising mixed state + wrapper agreement. Verified: bun test (234 pass, +4), bunx biome check (clean). * test(distill): assert interval-fires-shortly-before-shutdown race captures delta (G5) Scenario: auto-distill interval ticks, spawns worktree A at size S1, and records lastSpawnedSize = S1. User types for another second, session grows to S2 > S1, then quits. Expected: the shutdown handler spawns a SECOND worktree B to capture the (S1 \u2192 S2) delta \u2014 the shouldDistillOnShutdown guard 8 lets us through because currentSize !== lastSpawnedSize. Add a dedicated describe block with two tests that drive the race end-to-end: - interval fires at S1, content grows to S2>S1, shutdown spawns a second worktree (count goes 0 \u2192 1 \u2192 2) - interval fires at S1, no new content, shutdown is a no-op (count stays at 1) Mechanism: stub setInterval to capture the auto-distill tick callback (rather than swallow it like the existing suite), fire it manually, then drive session_shutdown. Exercises the real runAutoDistill \u2192 lastSpawnedSize wiring instead of relying on the predicate-level should-distill-on-shutdown.test.ts alone. Verified: bun test (236 pass, +2), bunx biome check (clean). * feat(distill): detect conflicting .gitattributes merge rule and refuse to override (G7) Previously, auto-setup appended '*.md merge=napkin-distill-merge' to an existing .gitattributes unconditionally. Because gitattributes is last-match-wins, a user with '*.md merge=union' would end up with our driver quietly overriding theirs the next time a .md file was merged. Detect that case and refuse to scaffold: - `detectConflictingMdMergeRule(gaPath)` (new helper, exported) scans for any `*.md` line with a `merge=<driver>` attribute where <driver> != napkin-distill-merge. Scope is deliberately narrow: exact `*.md` pattern only, no pattern-overlap inference. - `ensureVaultReadyForAutoDistill` checks this right after git init and returns `{ error: 'conflicting merge rule\u2026', conflict: { rule, file, driver } }` WITHOUT touching .gitattributes or .gitignore on conflict. The existing `setupFailed` path in session_start then suppresses auto-distill for the session \u2014 manual /distill still works. - session_start notify explains the options: - remove the conflicting rule, OR - set distill.onShutdown: false in vault config.json Test coverage: - 5 tests in ensureVaultReadyForAutoDistill describe block (fresh / idempotent / conflict / narrower pattern / defensive) - 8 tests in new detectConflictingMdMergeRule describe block (missing / empty / comments / our rule / foreign / extras on same line / different pattern / no merge=) - 1 integration test in shutdown-handler.test.ts verifying the full session_start \u2192 setupFailed \u2192 shutdown-skips-spawn path, and that the user's .gitattributes is preserved byte-for-byte. Exports: - New public: `detectConflictingMdMergeRule`, `NAPKIN_MERGE_DRIVER` (test convenience). - `SetupResult.conflict?: { rule, file, driver }` alongside existing `error`; both are populated on conflict so fail-soft callers keep working. Verified: bun test (250 pass, +14), bunx biome check (clean). * test(distill): exercise pollHandle timeout branch with env-var override (G8) The 10-minute pollHandle timeout in runDistillWith was untested \u2014 a regression in the timeout path (missed cleanup, stuck isRunning, wrong status) would only surface after a real 10-minute hang. Add coverage so the branch is exercised on every CI run. Mechanism: - Convert the private `MAX_DISTILL_DURATION_MS` constant into an exported `getMaxDistillDurationMs()` that checks NAPKIN_DISTILL_MAX_DURATION_MS_OVERRIDE. Production default (10 min) is unchanged; malformed / zero / negative overrides fall back to the default to prevent accidental instant-timeout in production. - New test file `pollhandle-timeout.test.ts` stubs setInterval to capture both the auto-distill tick AND the pollHandle callback, fires the poll callback twice (once before timeout, once after a 150 ms real-time sleep past the 100 ms override), and asserts: - the timeout branch removes the worktree (via spawnCleanup) - isRunning resets so a subsequent auto-tick spawns a fresh distill - the production default is 10 minutes when the override is unset - malformed / zero / negative overrides fall back to the default Verified: bun test (254 pass, +4), bunx biome check (clean). * test(distill): pin pi before_agent_start systemPrompt contract for regression (C8) The distill overlap injector concatenates onto `event.systemPrompt` and returns `{ systemPrompt: <concat> }`. Both halves of that contract live in pi's public types: BeforeAgentStartEvent.systemPrompt: string (we read+concat) BeforeAgentStartEventResult.systemPrompt?: string (we return) If pi ever changes either field (e.g. structured object, required vs optional, renamed), our concat would silently truncate or become a type error without any test in our suite complaining. Add a companion version-check test modelled after session-touched-files.version-check.test.ts that: - verifies the pi types.d.ts is still at the expected path - extracts the BeforeAgentStartEvent body and asserts `systemPrompt: string` - extracts the BeforeAgentStartEventResult body and asserts `systemPrompt?: string` - asserts pi's docstring still documents chaining (so a rewrite to last-wins would be caught) A failure tells us to inspect the new types, update the overlap injector if the contract genuinely changed, or resync the assertions if it was just a rename. Verified: bun test (258 pass, +4), bunx biome check (clean). * ci: mirror cad0p/napkin — bun install + semver-calver-release Migrate CI and local tooling from pnpm to bun to mirror the upstream cad0p/napkin pattern (bun install + biome + tsc + bun test, OIDC release via cad0p/semver-calver-release). Workflows: - ci.yml: single 'test' job on push/PR to main, runs `bun install` then biome check + tsc --noEmit + bun test. Drops the pnpm setup and the split lint/test jobs. - release.yml: auto-release on push to main (and release/* branches with skip_release) via cad0p/semver-calver-release/release@v1, followed by OIDC npm publish via npm-publish@v1. Replaces the softprops/action-gh-release + manual npm publish flow. - validate-package-version.yml (new): runs cad0p/semver-calver-release/validate-package-version@v1 on PRs to catch missing / malformed version bumps. - validate-release-pr.yml (new): runs validate-release-pr@v1 on PRs for release-style checks (changelog, etc.). Local tooling: - package.json scripts: `biome check` → `bunx biome check`, `bun test` unchanged, add `typecheck` script (`bunx tsc --noEmit`). - Add typescript@^5.8.0 devDep so bunx tsc has a pinned compiler. - Add publishConfig/repository/homepage/bugs metadata to match napkin's shape so semver-calver-release and npm publish have everything they need. - New tsconfig.json mirrors napkin's (strict, esnext, bundler moduleResolution, skipLibCheck); excludes *.test.ts because bun test types them at runtime. Lockfiles: - Switch to bun.lock (committed) for reproducibility. Delete the tracked pnpm-lock.yaml and gitignore it (pnpm is no longer used for this project). Content fixes needed to make the new tsc gate green: - extensions/distill/index.ts: ctx.ui.notify expects 'info' | 'warning' | 'error'; 'success' was invalid. Change the distill-complete notify to 'info'. No visible UX change \u2014 the status bar still paints the success glyph via theme.fg. - extensions/napkin-context/index.ts: pi's CustomMessage.content is `string | (TextContent | ImageContent)[]`. Narrow to string for the Markdown renderer (we only ever set string content). - extensions/napkin-context/index.ts: pi's ExtensionContext narrows sessionManager to ReadonlySessionManager, which hides appendCustomMessageEntry. At runtime it's always the full SessionManager; cast through a `SessionManager` type import. Verified locally: - bun install (clean) - bunx biome check extensions/ (clean) - bunx tsc --noEmit (clean) - bun test (258 pass, same as before migration) * fix(distill): match *.md merge rule case-insensitively to honor APFS/NTFS core.ignorecase (R2-1) The G7 detector's regex used case-sensitive matching, which missed `*.MD merge=union` rules. On macOS (APFS default) and Windows (NTFS default) filesystems, git's pattern matching respects `core.ignorecase=true`, so `*.MD merge=union` DOES apply to `foo.md` files. Without this fix we silently layer our driver on top via last-match-wins — exactly the scenario G7 exists to prevent. - Add `i` flag to the regex in `detectConflictingMdMergeRule`. - Document the APFS/NTFS rationale in the function docstring. - Add two new tests: `*.MD merge=union` and mixed-case `*.Md MERGE=Ours`. Total tests: 258 → 260. * fix(napkin-context): guard appendCustomMessageEntry cast with runtime check (R2-2) pi's `ExtensionContext.sessionManager` is typed as `ReadonlySessionManager` \u2014 a `Pick` that omits mutation methods. The cast to the full `SessionManager` worked at runtime because pi hands out the concrete instance, but this is a load-bearing assumption about pi's internals. If pi ever wraps the session manager in a readonly proxy, the cast would silently become a `TypeError` at `appendCustomMessageEntry(...)` call time \u2014 fatal for the extension. Defense-in-depth: - Duck-type check via `typeof sm.appendCustomMessageEntry === 'function'` before calling (handles the "method simply missing" case). - `try/catch` around the call (handles the "proxy throws on write" case). - Graceful degradation: on failure, emit a `notify(..., 'warning')` and continue without the context injection. Worst-case is a session without the vault overview, not a crashed extension. - `Partial<SessionManager>` cast keeps the type-level surface narrow while allowing the runtime probe. * chore(deps): pin @earendil-works/pi-tui peer dep floor to >=0.74.0 (R2-3) Previously `@earendil-works/pi-tui` was an unbounded wildcard (`*`) peer-dep. This allowed a lockfile-less install to resolve to a pre-migration `@mariozechner` version or an incompatible future `@earendil-works` major. Match the floor already set on `@earendil-works/pi-coding-agent` (`>=0.74.0`) so both peer deps track the same migration boundary. - `bun install` refreshed `bun.lock`'s recorded constraint. - No resolved-version changes \u2014 already on `@earendil-works/pi-tui@0.74.x`. * perf(distill): skip branch listing in getActiveDistills when unmerged not needed (R2-4) Post-CLN-3, `getActiveDistills` delegated to `getDistillState`, which unconditionally invoked `git worktree list` AND `git branch --list`. The sole production caller of `getActiveDistills` (the `before_agent_start` overlap injector) doesn't need the unmerged branch list, so the extra git call was wasted work. Split `getDistillState` into three internal primitives: - `listDistillWorktrees(vaultPath)` \u2014 `git worktree list --porcelain` + distill/* filter. - `listDistillBranches(vaultPath)` \u2014 `git branch --list`. - `toActiveDistills(entries)` \u2014 meta.json + pid-liveness hydration (no git). Compose: - `getActiveDistills` = `listDistillWorktrees` + `toActiveDistills` (one git call, no branch listing). - `getUnmergedDistillBranches` = `listDistillWo…
* feat(distill): externalise distill prompt to distill-prompt.md with template placeholders
Item A1 of PR #12 (agent-driven merge architecture). Introduces the
externalised distill prompt as a new artefact pair: `distill-prompt.md`
holds the full agent-driven worktree prompt (10 steps + worktree-isolation
prefix) with four template placeholders ({{worktreePath}}, {{vaultPath}},
{{branchName}}, {{defaultBranch}}); `distill-prompt.ts` exports
`buildDistillPrompt(inputs)` which reads the .md at runtime and
substitutes placeholders.
Why externalise: easier prompt iteration without touching code, markdown
rendering in editors, snapshot-testable against a single source of truth.
Per the PR #12 design's "DISTILL_PROMPT location" lock-in.
Why not yet wired to production callers: at A1 the wrapper still owns
merge/squash/cleanup. Wiring the agent-driven prompt now would have the
agent and the wrapper double-execute steps 7-10, which is worse than
either end. A2 rewrites the wrapper to invoke the agent for those steps;
that is when buildDistillPrompt replaces the inline DISTILL_PROMPT in
index.ts. The .md+loader+test ship at A1 as a freestanding architectural
piece that A2 builds on.
The loader is strict on placeholder coverage and rejects empty inputs —
silent corruption (e.g. `git merge ` with empty branch name) is the
failure mode to avoid, per the methodology guide's never-deferrable
"stale references to renamed concepts" category.
package.json `files` now includes `distill-prompt.md` so the .md ships
in the npm publish (the loader resolves it relative to its own location
via `import.meta.url` + `fileURLToPath`, matching the existing
`scripts-paths.ts` pattern).
Tests: 15 new (363 -> 378 baseline). Cover happy-path substitution,
multi-occurrence replaceAll, empty-input rejection (4), missing-placeholder,
empty-template, and 7 .md content invariants (10 step markers, no-force,
pull-merge, do-not-loop, isolation prefix).
* feat(distill): replace per-file merge driver invocation with single agent call + hard timeout
Item A2 of PR #12 (agent-driven merge architecture). Rewrites the distill
wrapper from a multi-step pipeline (pi -> add -> commit -> merge -> squash
-> commit) into a single bounded agent call. The agent now owns content
production AND the integration phases (merge, squash, push, cleanup) per
the PR #12 design's "Agent contract" section; the wrapper is a thin
shell: shim install, cd parentCwd, `timeout(1) pi -p $PROMPT`, write
outcome.
Wrapper changes:
- new positional arg 10: maxDurationSecs (hard agent-task budget;
default 600s = 10 minutes from `distill.maxDurationMinutes` config,
no per-phase timeouts anymore — folded into this single knob)
- replaces git add/commit/merge/squash/commit block (lines 558-680
of the PR #11 wrapper) with a single `timeout --foreground
"$MAX_DURATION_SECS" pi --session ... -p "$PROMPT"` invocation
- drops merge-driver-specific test hooks: NAPKIN_DISTILL_FORCE_MERGE_HEAD,
NAPKIN_DISTILL_FORCE_MERGE_RC (Phase B will drop the merge driver
script entirely)
- drops the partial-merge log file (no driver to 3-strike on)
- retains: shim install, meta.json pid rewrite, startSha extraction,
cleanup trap, all HALT_AFTER_* / FORCE_CLEANUP test hooks
- leaves TODO(A3) markers for post-validation (markers absent, HEAD
on default, commit count, merged-local detection) and TODO(A4)
for the salvage path (force-cleanup + failed:<reason> outcome)
- on agent exit-0: writes `merged-content` unconditionally (placeholder
until A3); on agent non-zero: exits 1 without writing an outcome
(placeholder until A4 — JS-side abnormal-termination warning fires)
JS-side changes (extensions/distill/distill-workspace.ts +
extensions/distill/index.ts):
- SpawnDistillOptions drops `prompt` (built internally now via
buildDistillPrompt against the .md template introduced in A1) and
adds `maxDurationSecs` (positive integer, derived from
getMaxDistillDurationMs(config) / 1000)
- spawnDistillInWorktree now resolves the agent prompt internally;
the worktree-isolation prefix is in distill-prompt.md so
buildWorktreeDistillPrompt is no longer called from the spawn path
(still exported pending Phase B deletion)
- 11th wrapper arg added on the JS side to match the new wrapper signature
Test deltas: 363 -> 365 pass, 13 skip, 0 fail.
Skipped tests document the pre-existing driver-specific behavior and are
either replaced in Phase C (mocked-pi fixtures simulating each agent-
behavior class) or deleted in Phase B (driver-specific tests):
- 4 in (integration): happy-path, empty-distill, POST-CONV-1, concurrent
- 3 in (partial-merge salvage): clean-file, all-conflict, log-co-locate
- 2 in (MERGE_HEAD escape-hatch): persists-after-merge, unexpected-rc
- 2 in (LLM-resolved conflict, end-to-end): driver-output-on-main,
driver-retries-ok-after-2
- 1 in (non-main default branch): wrapper-squash-merges-into-master
- 1 in routing: no-content outcome -> warning notify (resumes after A3
wires validate_commit_count for no-content detection)
Tests retained and unchanged: shutdown-handler.test.ts (predicate logic),
routing.test.ts spawn-routing tests (worktree vs legacy spawn invariant),
distill-workspace.test.ts (worktree creation, parentCwd validation),
system-prompt.cache-parity.test.ts (POST-R6-CACHE byte-equality),
pollhandle-timeout.test.ts (JS-side poller).
Tests updated to A2 contract: 5 unit tests in
spawn-distill-in-worktree.test.ts (`spawnDistillInWorktree (unit, mocked
spawn)`) — drop `prompt:`, add `maxDurationSecs: 600`, assert new arg [10]
is the maxDurationSecs string and arg [5] (prompt) contains the steps
1-10 markers + substituted placeholders + isolation prefix.
DO NOT YET DELETED (Phase B owns the deletion): the merge driver script
`extensions/distill/scripts/napkin-distill-merge`, the `.gitattributes`
install in auto-setup.ts, scripts.test.ts, and git_retry.sh.
* feat(distill): wrapper post-validation — markers, commit count, HEAD on default, merged-local detection
Item A3 of PR #12 (agent-driven merge architecture). Replaces the A2
TODO(A3) stub with four post-agent-exit validators that decide the
outcome class deterministically, plus a JS-side dispatch update so the
new classes surface with the right notification severity.
Wrapper helpers added (extensions/distill/scripts/distill-wrapper.sh):
- validate_no_markers <vault> — scans tracked *.md files for
residual conflict markers (`<<<<<<< `, `======= ` exact, `>>>>>>> `
at line start). Uses `git ls-files -z` so .gitignore'd content
(`.napkin/distill/`) is skipped automatically. On hit, logs offending
paths and returns 1.
- validate_head_on_default <vault> <default> — confirms vault HEAD is
the symbolic ref `refs/heads/<default>` via
`git symbolic-ref --short HEAD` (V3-locked: detects detached HEAD
with a non-zero exit, unlike `branch --show-current` which returns
empty silently).
- validate_commit_count <vault> <startSha> — prints the count of
commits since startSha on the current HEAD; returns non-zero only
if rev-list itself fails. The wrapper dispatches `no-content` on 0
and proceeds to the merged-content/local-only dispatch otherwise.
- detect_local_only <vault> <default> — returns 0 (true) when origin
is configured AND local is ahead of `origin/<default>`. No origin
configured means the user never expected a push, so this returns
false and the outcome stays merged-content.
Wrapper post-agent-exit dispatch (replaces the A2 unconditional
`merged-content` placeholder):
AGENT_RC == 124 or 137 (timeout SIGTERM or SIGKILL after grace)
-> failed:agent-timeout
AGENT_RC != 0 -> failed:agent-exit-nonzero
HEAD not on default -> failed:head-not-on-default
markers found in vault -> failed:markers-after-agent-exit
commit_count == 0 -> no-content
origin diverges -> merged-local
otherwise -> merged-content
Magic numbers 124 / 137 are coreutils-defined `timeout(1)` exit codes
(SIGTERM exit-within-grace / SIGKILL after grace); named as
TIMEOUT_TERM_RC / TIMEOUT_KILL_RC in the wrapper per the methodology
guide's never-deferrable magic-number rule.
JS-side update (extensions/distill/index.ts formatOutcomeNotification):
- new `merged-local` case → warning ⚠ "Distillation complete locally;
not pushed to origin (Ns)"
- new `failed:<reason>` prefix → error ✗ "Distillation failed: <reason>"
(the reason code surfaces in both the message and status text so the
user can diagnose without opening the error log first)
Salvage at A3 is minimal: failure paths write the `failed:<reason>`
outcome and exit 1; the cleanup trap still removes worktree+branch
best-effort. A4 will replace this with the explicit salvage helper that
force-cleans pre-exit and emits a recovery hint into the outcome
sidecar.
Tests: 365 -> 376 pass (+11). New file
extensions/distill/wrapper-validation.test.ts adds 10 end-to-end tests
that drive the wrapper with `NAPKIN_DISTILL_PI_BIN=<bash-stub>`
producing each behavior class:
- validate_head_on_default PASS / FAIL
- validate_no_markers PASS / FAIL
- validate_commit_count PASS / FAIL (no-content)
- detect_local_only PASS (no origin) / TRIGGERED (merged-local)
- agent exit non-zero / agent exceeds maxDurationSecs
Plus: routing.test.ts "no-content outcome → warning notify" was skipped
in A2 because the wrapper hadn't yet wired no-content detection. A3
ships that detection, so the test is unskipped here and now passes.
Skipped tests unchanged from A2 (12 driver-specific): partial-merge
salvage, MERGE_HEAD escape-hatch, LLM-resolved conflict, integration
happy/empty/POST-CONV-1/concurrent, non-main default branch
squash-merges. All marked `// will be deleted in Phase B/C`.
* feat(distill): wrapper salvage path — force-cleanup worktree+branch + failed:<reason> outcome
Item A4 of PR #12 (agent-driven merge architecture). Replaces the A3
"write outcome + exit 1" failure-path stubs with an explicit `salvage`
helper that V3-locked never touches main vault history.
Wrapper helper added (extensions/distill/scripts/distill-wrapper.sh):
salvage <vault> <worktree_path> <branch_name> <reason>
Steps in order:
1. cd to vault (so `git worktree remove --force` doesn't refuse)
2. force-remove worktree (`git worktree remove --force`)
3. rm -rf the leaf if shim residue survives (POST-CONV-3 pattern)
4. `git worktree prune` for stale entries
5. `git branch -D` the distill branch
6. rmdir parent vault-hash dir if empty (POST-CONV-4 pattern)
7. log a critical-error if vault HEAD isn't on the default branch
(V3: NEVER `git checkout` on the user's behalf — the salvage
path is a janitor for the worktree+branch, not a rollback agent
for main)
8. write `failed:<reason>` outcome with reason-specific recovery hint
Reason codes (V3-locked):
markers-after-agent-exit -> "git revert HEAD --no-edit" + "reflog ~90 days"
head-not-on-default -> "git checkout <default>" + "reflog"
agent-exit-nonzero -> "see error log" + "reflog"
agent-timeout -> "bump distill.maxDurationMinutes" + "reflog"
Outcome sidecar format (multi-line for failed:* classes):
line 1 = outcome class (machine-readable, drives JS-side dispatch)
lines 2+ = optional human-readable recovery hint
Happy-path classes (merged-content, merged-local, no-content) stay
single-line so the legacy single-line shape continues to parse cleanly.
JS-side updates:
- findDistillOutcomeForBranch (distill-workspace.ts) parses line 1 as
outcomeClass and exposes lines 2+ as `recoveryHint: string | null`.
Backward-compatible with single-line legacy sidecars (recoveryHint
is null for those).
- formatOutcomeNotification (index.ts) renders the recovery hint inline
in the failed:* notification: "Distillation failed: <reason> — <hint>"
- DistillStrategy.checkOutcome's return type extended with
recoveryHint; runDistillWith propagates it through to the dispatch.
Tests: 376 -> 381 pass (+5). New file
extensions/distill/wrapper-salvage.test.ts:
- markers-after-agent-exit: outcome carries reason + 'git revert' hint;
main vault HEAD is NOT reset (V3 lockdown — the agent's commit with
markers stays; user `git revert`s manually per the hint)
- head-not-on-default: outcome carries reason + 'git checkout main' hint
- agent-exit-nonzero: outcome carries reason + 'reflog' hint; main
vault HEAD is unchanged (agent crashed pre-squash)
- agent-timeout: outcome carries reason + 'maxDurationMinutes' hint
- happy-path regression: merged-content sidecar stays single-line and
`recoveryHint` is null
Plus a fix-up to wrapper-validation.test.ts: the A3 helper read the
outcome via `readFileSync.trim()` which now collapses the multi-line
sidecar into one string. Updated to take only line 1, mirroring the
JS-side parser.
Salvage operations are best-effort (worktree-remove / branch-D / rmdir
all swallow errors). The outcome sidecar write is the user-facing
signal; transient cleanup races (concurrent salvage on different distills,
ENOENT on already-removed dirs) don't block it.
V3 lockdown reaffirmed: NO main vault history mutation under any
salvage path. Per case-C analysis in research/v2-v3-verification.md,
`git reset --hard $START_SHA` would silently destroy concurrent user
commits in the autosave-while-editing scenario. The recovery hint
points the user at the forward-only `git revert` path instead.
* fix(distill): correct prompt CWD framing for agent shell cwd (CLEAN-A-2, CLEAN-A-3, SEC-A-6)
Round 1 spec-blind review caught CLEAN-A-2, CLEAN-A-3, SEC-A-6.
The prompt's opening line told the agent "You are running in an isolated
git worktree at <path>", and step 7 used bare "git merge" / "git add -A"
/ "git commit -m" commands. But pi spawns the agent at PARENT_CWD per
POST-R6-CACHE prompt-cache parity, not the worktree. So bare git in
step 7 would either silently corrupt a parent project repo or fail
with "not a git repository". Steps 8-10 already used "git -C vaultPath"
correctly; step 7 was the inconsistent outlier.
Three changes share one .md, so they share one commit:
CLEAN-A-2 high: rewrite step 7 to use "git -C worktreePath" for every
git operation. Brings step 7's shape into line with steps 8-10.
CLEAN-A-3 medium: replace the false opening with a statement of the
cwd contract. Self-explaining for future readers and the agent itself,
per methodology guide "improve the code, not the comments": the prompt
now describes its actual contract instead of relying on reviewer-side
context to interpret it.
SEC-A-6 medium: add --no-rebase to step 9's pull recovery so a user
with global pull.rebase=true cannot silently bypass the spec's
pull-merge requirement.
Tests: distill-prompt.test.ts gains three assertions; the legacy
"isolated git worktree" string assertion is replaced with the new
contract-shape assertion. spawn-distill-in-worktree.test.ts updates
its prompt-shape assertion to match the new opening.
* fix(distill): timeout(1) hardening — kill-after grace + macOS gtimeout fallback (CLEAN-A-1, SEC-A-3, CI-A-1)
Round 1 cross-reviewer convergence on the timeout(1) invocation:
cleanness-blind, security, and CI-portability lenses all flagged the
same root-cause cluster.
CLEAN-A-1 + SEC-A-3 (high): the wrapper invoked
"timeout --foreground <budget>" without -k/--kill-after. Per
coreutils' man page, without -k, timeout sends a single SIGTERM and
waits indefinitely for the target. A SIGTERM-ignoring agent (a
stuck-in-libc-syscall pi process, a tool subprocess that traps
SIGTERM, an OOM-killing-deadlock scenario) hangs the wrapper forever,
holding the worktree, the gitignored shim, and the .napkin/distill/
cache slot. The header comment claimed SIGKILL escalation happened
after a "10s grace"; that was untrue. The rc-137 (SIGKILL escalation)
arm in the dispatch case was dead code under the bare invocation.
CI-A-1 (high): macOS without "brew install coreutils" doesn't ship
timeout(1) at all; Homebrew installs it as gtimeout by default. The
wrapper's bare "timeout --foreground" expanded to "command not found"
on stock macOS, AGENT_RC=127, routed to failed:agent-exit-nonzero
with no diagnostic naming the missing binary. Every distill on a
stock-macOS user's machine surfaced that misleading outcome.
Fix:
- Detect TIMEOUT_BIN at wrapper start with
"$(command -v timeout || command -v gtimeout || true)". Fail-fast
with "exit 2" + actionable error message (mentions
"brew install coreutils") if neither is on PATH. Solves CI-A-1
without runtime branching.
- Add TIMEOUT_KILL_GRACE_SECS=30 named constant (per the methodology
guide's never-deferrable magic-number rule) and pass it via
-k "${TIMEOUT_KILL_GRACE_SECS}" to the timeout invocation. Now
SIGKILL escalation actually happens, the rc-137 dispatch arm is
reachable, and the comment claims match the code.
- Make the grace period overridable via
NAPKIN_DISTILL_TIMEOUT_KILL_GRACE_SECS so SIGTERM-ignoring stub
tests can complete within the test timeout budget without waiting
the full 30s. Documented as a testing hook alongside the existing
HALT_AFTER_* / FORCE_CLEANUP hooks.
- Replace bare "timeout" with "$TIMEOUT_BIN" in the invocation so the
detected binary (timeout or gtimeout) is what runs.
Tests:
- New regression: "SIGTERM-ignoring agent → SIGKILL escalates within
grace, classifies as agent-timeout". Stub does
"trap '' TERM; sleep 60; touch <marker>". Wrapper runs with
maxDurationSecs=1 + grace=2. Asserts wrapper exits with
failed:agent-timeout, the marker was never created (proving
SIGKILL fired before sleep returned), and elapsed wall-clock is
under 15s (would hang past 30s outer timeout if -k were dropped).
- The pre-existing "agent exceeds maxDurationSecs" test continues to
pass — sleep cooperatively exits on SIGTERM at 1s, doesn't reach
the SIGKILL escalation path. Both paths are now covered.
Verified: bun test green (384 pass, 12 skip, 0 fail; +1 new test),
bunx tsc --noEmit clean, biome check clean.
* ci: macOS matrix + git identity + bun pin (CI-A-2, CI-A-3, CI-A-4)
Round 1 CI-portability lens flagged three workflow-level gaps the four
review lenses (correctness, cleanness, security, CI portability) only
saw because CI was added as a fourth lens after PR #11's missing-git-
identity blind spot. Per the methodology guide's "Known blind spot:
CI/environment portability" section, two of these are never-deferrable
(CI-A-3 unpinned bun version, CI-A-4 missing git identity).
CI-A-2 (high): the matrix ran only ubuntu-latest. Any macOS-only
regression — CI-A-1's missing timeout(1), BSD vs GNU grep flag drift,
BSD xargs -I quirks, bash 3.2 surface — was invisible until a user
reported it. Adds macos-latest to the matrix with fail-fast: false so
one platform's failure doesn't mask the other. Combined with Commit
a2c8c21 (timeout(1) hardening + gtimeout fallback), the macOS slot
now exercises the same wrapper path Linux does. brew install coreutils
runs only when runner.os == macOS so Linux jobs are unchanged.
CI-A-3 (medium → never-deferrable): bun-version: latest floats with
whatever bun release is current on the day CI runs. A bun-internal
regression (test runner, fs API, bun:test semantics) could break CI
without any code change in this repo. Pinned to 1.3.13, the
contributor-machine version. Bump deliberately when a new bun release
ships something useful.
CI-A-4 (medium → never-deferrable): the workflow had no defensive git
identity step. The new wrapper-validation / wrapper-salvage tests do
set identity inside their stubs AND in spawnSync env, but a future
contributor adding a test that calls git commit without all three
layers would silently regress to the PR #11 failure mode. Adds
git config --global user.email/user.name + init.defaultBranch=main
before bun install. Mirrors the recipe captured in PR #11's
deferred.md "Discovered on pi-napkin PR #11 (2026-05-12)" entry.
Verified: bun test green locally (384 pass, 12 skip, 0 fail). YAML
syntax validates via python yaml. The GH workflow itself can only be
verified post-merge against actual runners; the changes are mechanical
and the macOS-slot coverage is the new thing to watch on the next CI
run.
* fix(distill): force-push detection via ancestry check, not SHA equality (SEC-A-1)
The locked spec at "Push behavior: never force" mandates wrapper-side
post-validation:
"Spec the prohibition in the prompt; the wrapper post-validates by
checking that origin/<default> only fast-forwarded (its tip is an
ancestor of the new tip after push)."
Round 1 security review caught that the wrapper implemented only the
first half. The prompt at distill-prompt.md:45 says "NEVER use --force
or --force-with-lease", but detect_local_only's SHA-equality check
couldn't distinguish a fast-forward push from a force-push: in either
case after the agent's push fetches back, local == origin/<default>.
An agent that ignored the prompt prohibition (model error or prompt-
injection from vault content) would land corrupt origin history with
the wrapper writing merged-content checkmark to the user.
The prompt is a soft control. The wrapper's post-validation is the
hard one. Today, the hard one didn't exist.
Fix:
- Replace the rc=0/rc=1 binary in detect_local_only with three states:
rc=0 — local-only (merged-local): legitimate fast-forward state
(origin configured AND remote is ancestor of local) OR
origin configured but origin/<default> never fetched.
rc=1 — in sync OR no origin: proceed to merged-content.
rc=2 — divergent histories: remote is NOT an ancestor of local.
Force-push or remote rewrite happened.
The ancestry check is `git -C <vault> merge-base --is-ancestor
<remote_sha> <local_sha>`. Returns 0 when remote is ancestor of
local — exactly the condition the spec asks for, just inverted to
detect violations.
- Update the dispatch site to capture rc explicitly (because bash
`if cmd; then` collapses rc=1 and rc=2 into the same arm) and route
rc=2 to a new failed:force-push-detected outcome class via the
salvage helper.
- Add a recovery hint for force-push-detected: points the user at
`git log origin/<default>..<default>` and `git log
<default>..origin/<default>` to inspect the divergence, plus reflog
for distill content recovery.
- Update the salvage() docstring and the dispatch-site comment to
list force-push-detected alongside the four existing reason codes.
- Update extensions/distill/index.ts's formatOutcomeNotification
comment to mention the new reason code (the dispatch logic itself
is generic over failed:<reason> and needed no code change).
Tests:
- New regression: "detect_local_only DIVERGENT: origin rewritten
under local → failed:force-push-detected (SEC-A-1)". Builds a
divergent state by giving origin a foreign commit (via a second
clone) and committing a different file on the vault's local main
before the agent runs. Asserts wrapper exits 1 with
failed:force-push-detected.
- The existing detect_local_only TRIGGERED test (origin behind local
in a fast-forwardable way) continues to pass — that's now the
rc=0/merged-local arm of the new tri-state semantics, exercised
exactly the way the spec describes legitimate "didn't push" state.
Verified: bun test green (385 pass, 12 skip, 0 fail; +1 new test),
bunx tsc --noEmit clean, biome check clean.
* fix(distill): path-safety guard before rm -rf in salvage and cleanup-trap (SEC-A-2)
Round 1 security review caught defense-in-depth gap. Today's salvage()
and the cleanup trap call:
if [ -d "$worktree" ]; then
rm -rf "$worktree" 2>/dev/null || true
fi
after `git worktree remove --force`. Note: `git worktree remove --force
<path>` REFUSES paths not registered as worktrees of the vault (returns
non-zero, suppressed by `|| true`). The directory then still exists,
the second `[ -d ]` is true, and `rm -rf` runs on whatever path was
passed in.
JS-side `resolveCacheRoot` always builds worktrees under
`<XDG_CACHE_HOME or ~/.cache>/napkin-distill/<vault-hash>/<branch-suffix>/`,
so under normal use the wrapper is safe. But "JS-side construction is
correct" is the only line of defense. An upstream bug (out-of-tree
caller, malformed test fixture, future code path) that passed
`worktree=/etc` or `worktree=$HOME` would silently turn into
`rm -rf /etc` or `rm -rf $HOME`. Per the methodology guide's "Severity
ladder → high" entry, defense-in-depth gaps that could cause silent
data loss are high, not medium — JS-side correctness is the primary
defense, this is the safety net.
Fix:
- Add `safe_rm_worktree` helper. Resolves the input via `cd <path>
&& pwd -P` (canonicalises symlinks portably across BSD/GNU; defeats
symlink-tricks). Refuses to `rm -rf` unless the resolved path
matches `*/napkin-distill/*/*` (i.e. lives at least two levels
deep under a `napkin-distill` directory — matching the production
cache layout's `<cache>/napkin-distill/<hash>/<suffix>/` shape).
On refusal, logs a precise diagnostic naming both the input and
the resolved path and returns 1; caller proceeds (best-effort
cleanup is the contract).
- Replace `rm -rf "$worktree"` in salvage() with
`safe_rm_worktree "$worktree" || true`.
- Replace `rm -rf "$WORKTREE"` in the EXIT trap (cleanup function)
with `safe_rm_worktree "$WORKTREE" || true`.
Tests:
- New describe block `safe_rm_worktree path-safety guard (PR #12
SEC-A-2)` with four assertions:
- refuses to rm-rf a path outside any napkin-distill subtree
(asserts the directory and its sentinel file survive, and the
stderr names "refusing to rm-rf" + "napkin-distill")
- removes a path inside a `napkin-distill/<hash>/<suffix>/`
subtree (asserts the directory is gone)
- refuses an empty path (rc 1, stderr names "empty worktree path")
- returns 0 for a non-existent path (idempotent / already-removed)
- The function is extracted from the wrapper via awk-style range
scan (start at `safe_rm_worktree() {`, end at the next bare `}`)
and sourced into a tiny bash harness with a stub log_error so the
test can call the function with crafted paths the production
wrapper would never construct.
- All five existing salvage scenarios (markers, head-not-on-default,
agent-exit-nonzero, agent-timeout, happy-path) continue to pass —
they hit the `*/napkin-distill/*/*` arm of the new guard and behave
identically to the pre-fix code.
Verified: bun test green (389 pass, 12 skip, 0 fail; +4 new tests),
bunx tsc --noEmit clean, biome check clean.
* fix(distill): atomic outcome sidecar write via temp + rename (SEC-A-4)
The pre-fix write_outcome used two printf calls inside a single redirect group. Bash flushes between the two printfs, so a JS-side poller (findDistillOutcomeForBranch) could open the file between the kernel writes of line 1 and line 2, see only the outcome class, and misclassify a failed distill as having no recovery hint.
Fix: write to a sidecar.tmp file then mv to the canonical path. POSIX rename(2) is atomic on the same filesystem, so the poller either sees the file absent or sees the complete contents, never a partial write. Standard temp-and-rename pattern.
Tests: extract write_outcome from the wrapper into a bash harness (same pattern as the SEC-A-2 safe_rm_worktree extraction) and assert single-line and multi-line outcomes write the exact expected bytes, no .tmp straggler is left behind after the call, idempotent rewrite replaces the file cleanly. Plus an end-to-end test that spawns the real wrapper and asserts the post-run error dir contains no .tmp files.
Cites SEC-A-4 (medium).
* fix(distill): reject control chars and prompt-injection metachars in buildDistillPrompt inputs (SEC-A-5)
Defense-in-depth at the API contract boundary: validateInput() now rejects (a) non-string / empty, (b) ASCII control chars in [\x00-\x1F] + \x7F, and (c) {{ / }} placeholder-syntax collisions for every field of DistillPromptInputs.
Rationale: a literal newline in any input field would break out of the prompt step it embeds in (classic prompt-injection class). NUL bytes truncate strings on most C-side consumers. Mustache-collision is a future-refactor hazard if the substitution loop ever multi-passes.
Realistic inputs (paths with spaces / hyphens / dots, branch names with slashes like 'feat/foo' or 'release/2026-q1') still pass.
Tests: 42 new in distill-prompt.test.ts cover each control-char representative (NUL, LF, CR, TAB, BEL, ESC, DEL) across all four input fields, three placeholder-syntax shapes across all four fields, and two regression-guard happy-path inputs.
References: SEC-A-5 (medium) per Phase A round-1 security review.
* fix(distill): tighten validate_no_markers to require all three marker types co-present (CORR-A-1, SEC-A-7)
Real merge conflicts always emit ALL THREE marker types ('<<<<<<< ', '======= ', '>>>>>>> ') in the same file. The prior validator flagged any one of them, false-positiving on two common shapes: (1) markdown setext H1 underlines ('# title' followed by '=======' on its own line), and (2) documentation prose that quotes a single conflict marker to discuss merges (e.g. README sections that walk the reader through what '<<<<<<< HEAD' means). Either shape would have permanently blocked distills on the affected vault.
Tighten the per-file predicate to require all three markers co-present in the same file via a 'grep -q && grep -q && grep -q' chain. Use a bash 3.2 portable 'while IFS= read -r -d "" file' loop with array '+=' aggregation so the helper still runs on macOS default bash. Continues to enumerate via 'git ls-files -z -- *.md' so .gitignore'd content (e.g. '.napkin/distill/') is skipped.
Tradeoff: a vault that documents a complete '<<<<<<<' / '=======' / '>>>>>>>' example inside a single file (including in a fenced code block — the validator does not parse markdown structure) will trip the check. Users escape with leading whitespace, HTML comments, or by splitting the example across two files. The cost of any-of-three false-positives (block-all-distills-forever on legitimate documentation) is much higher than this rare-explicit-doc case.
Tests: five regression guards in wrapper-validation.test.ts pin the new behavior — setext H1 underline passes, single-marker prose passes, two-marker prose passes, markers split across two files passes, and all-three inside a code block fails (acknowledged tradeoff). Existing all-three real-conflict FAIL test still passes.
References: CORR-A-1 (low→elevated), SEC-A-7 (low→elevated). Elevation rationale: false-positives would permanently block distills on user vaults documenting merge conflicts in notes. CLEAN-A-15 tracks the per-file grep -q optimisation opportunity.
* refactor(distill): extract shared test scaffolding to _test-helpers.ts (CLEAN-A-6)
wrapper-validation.test.ts and wrapper-salvage.test.ts duplicated ~80 LOC of identical scaffolding (Scaffold interface, makeScaffold, writeStubPi, runWrapper). Move them to _test-helpers.ts as makeWrapperScaffold(prefix), writePiStub, runWrapperWithStub plus the WrapperScaffold interface, and rename the call sites in both files via local aliases so the test bodies stay byte-identical.
runWrapperWithStub returns the union of both prior runWrappers' outputs (outcome/outcomePath from the validation variant + preSha from the salvage variant). Both consumers selectively read what they need; the extra computation is cheap (one rev-parse + one readdir scan).
Side-effect on tsconfig: extensions/**/_test-helpers.ts is now in the tsc exclude list alongside *.test.ts. The previously-included withNapkinOnPath helper passed strict checks because it never touched SessionManager; the SessionManager.create + appendMessage incantation moved here is the same loose pattern every test file uses (already excluded). package.json files already excluded _test-helpers.ts from the published bundle, so this aligns the two configs.
Pure refactor — no behavior change. All 30 wrapper-{validation,salvage} tests still pass; full suite unchanged at 440 pass / 12 skip / 0 fail. Phase C will benefit when adding bash-stub fixtures for ~10 mocked-pi behaviors.
Cites: CLEAN-A-6 (medium, never-deferrable cleanness).
* chore(distill): clean up stale TODO comments and skip annotations (CLEAN-A-4, CI-A-6)
CLEAN-A-4: distill-wrapper.sh's docstring still carried 'A2 transitional state', 'TODO(A3)', and 'TODO(A4)' markers from the staged-rollout commits. A3 (afed6ae) and A4 (0d0a262) both landed weeks ago, so the markers misrepresent the wrapper's actual state. Replace the multi-paragraph A2/A3 evolution block with a one-line implementation-history reference, drop 'stubbed in this A3 commit' parentheticals from the lifecycle steps, and trim the TODO(A3)/TODO(A4) markers from the agent-responsibilities footer. The numbered Lifecycle section already documents the current shape correctly.
CI-A-6: two describe.skip blocks in spawn-distill-in-worktree.test.ts (MERGE_HEAD escape-hatch at L1440, LLM-resolved conflict end-to-end at L1650) lacked the '// will be deleted in Phase B' annotation that the parallel partial-merge-salvage block at L1126 carries. Both target the pre-A2 NAPKIN_DISTILL_MERGE_MOCK driver path that A2 deleted, so they share Phase B fate. Add the same annotation form (suffix on the closing comment-block line) for grep parity.
Comment-only changes. bash -n on the wrapper still parses; full test suite unchanged at 440 pass / 12 skip / 0 fail; tsc + lint clean.
Cites: CLEAN-A-4 (medium, never-deferrable: stale comments), CI-A-6 (medium, never-deferrable: stale references). Both never-deferrable per release-grade methodology.
* test(distill): isolate distill-prompt.test.ts from shipped .md (CI-A-5)
The previous test suite mutated extensions/distill/distill-prompt.md in place when exercising template error paths (missing placeholder, empty file). On a test crash before the finally-block restored the backup, the shipped artifact would be left degraded — polluting the bundled npm output and risking committing test residue. Methodology guide flags testability seams as never-deferrable, so this is a fix-now class.
Refactor (Option B — separate helper): extract buildDistillPromptFromFile(promptPath, inputs) as the path-injected core. buildDistillPrompt(inputs) is now a thin public-API wrapper that delegates with DISTILL_PROMPT_PATH. Tests that need to exercise template error paths write into a per-suite tmpdir (mkdtempSync + afterAll rmSync) and call buildDistillPromptFromFile against the tmpdir copy. The shipped .md is read-only from the test suite's perspective. Picked Option B over an optional second parameter because it makes the seam explicit — the public API stays single-arg, the testable surface is a clearly-named separate export, and there's no risk of a future caller accidentally mutating production behavior via an opts arg.
Added a regression-guard test that sha256-checksums the shipped .md after the test-suite runs to catch any future test that reintroduces direct mutation. Also added a smoke test that buildDistillPromptFromFile against a tmpdir copy returns identical output to the default buildDistillPrompt call. Test count: 440 -> 443 pass (+3 net), 12 skip, 0 fail. tsc + biome clean. Shipped .md sha256 unchanged after bun test.
* fix(distill): scope prompt prefix to file edits + add --no-edit to merge + no-content branch (CLEAN-4, CLEAN-11, CLEAN-5)
Round 2 review caught two regressions and one gap in the agent-driven distill prompt that Pass 1A did not surface:
CLEAN-4 (high, regression from Pass 1A's CLEAN-A-2 fix): the opening paragraph said "Do NOT mix worktree files with the main vault path {{vaultPath}} — writing to {{vaultPath}} bypasses isolation" without scope. Steps 8-10 explicitly run `git -C {{vaultPath}} checkout/merge --squash/commit/push/worktree-remove/branch -D`, which a literalist agent could read as forbidden by the prefix and skip — leaving the distill content stranded on the worktree branch. The prefix now scopes the prohibition to the distill-content phase (steps 1-6) and explicitly calls out steps 7-10's `git -C {{vaultPath}}` operations as correct and required.
CLEAN-11 (high): step 7's `git -C {{worktreePath}} merge {{defaultBranch}}` lacked `--no-edit`, so on a clean auto-merge git would open core.editor for the merge commit message. The agent's bash tool has no TTY, so the editor call hangs or returns non-zero — silent distill failure. Added `--no-edit` to the merge step. Defense-in-depth: the wrapper now exports `GIT_TERMINAL_PROMPT=0` and `GIT_EDITOR=true` near the other env exports so any git op the agent runs (`git pull`, `git revert`, `git commit -a`, future prompt revisions) inherits the same fail-fast/no-op-editor posture.
CLEAN-5 (medium): step 7 told the agent to commit the worktree's content unconditionally, which would fail with `nothing to commit, working tree clean` if the agent decided per the "Be selective" directive that nothing in the conversation merited capturing. The prompt now opens step 7 with an explicit no-content branch: skip steps 7-9 entirely and proceed straight to step 10 cleanup. The wrapper's commit-count validator then classifies the run as `no-content` (warning, not failure).
Tests: distill-prompt.test.ts gains three new content-invariant assertions covering the new prefix scoping, `--no-edit` on the worktree merge, and the no-content branch wording. wrapper-validation.test.ts gains a new describe block with a textual assertion (export lines present in script source) and a runtime assertion (a stub-pi process spawned by the wrapper sees `GIT_TERMINAL_PROMPT=0` and `GIT_EDITOR=true` in its env). 460 tests run, 448 pass, 12 skip, 0 fail.
* fix(distill): rename force-push-detected to divergent-history with refined recovery hint (SEC-1, CORR-2)
Round 2 cross-reviewer consensus (security + correctness): the `force-push-detected` reason code added in Pass 1A's SEC-A-1 fix over-classifies the divergence detection. The actual check is `merge-base --is-ancestor origin/<default> <default>` returning false — i.e. local and origin share no linear ancestry. Force-push is the rare cause; the common cause is a teammate (or the user from another clone) pushing to origin from another clone while this distill ran. The original name implied attacker action and the recovery hint led with "force-push or remote rewrite", which would mislead users in the normal case.
Renamed the reason code to `divergent-history` (neutral on cause) across the wrapper script, the JS-side comment in formatOutcomeNotification, and the wrapper-validation test that asserts the dispatch. The recovery hint now leads with the normal case ("typically because someone (you or a teammate) pushed to origin from another clone") and demotes force-push to a less-common cause, with the same git inspection commands. The test gains assertions on the new hint shape so a future regression can't silently revert to the attacker-framing language.
Files: distill-wrapper.sh (reason code in salvage's case statement, recovery hint, detect_local_only's docstring + return-code 2 doc + dispatch comment, salvage call site, log_error wording), index.ts (formatOutcomeNotification's reason-code-list comment), wrapper-validation.test.ts (test name, docstring, expected outcome string, new sidecar-content assertions). 460 tests run, 448 pass, 12 skip, 0 fail.
* fix(distill): tighten safe_rm_worktree pattern to require resolved cache root (SEC-2, CORR-3)
Round 2 cross-reviewer consensus (security + correctness): the prior `safe_rm_worktree` glob `*/napkin-distill/*/*` matches ANY canonical path containing that segment, not specifically the resolved XDG cache root. A bug elsewhere that constructed a path like `/some/random/dir/napkin-distill/foo/bar` would bypass the cache-root intent yet pass the glob — the worktree would be `rm -rf`'d outside the cache subtree. The primary defense (JS-side construction in `resolveCacheRoot()` + `git worktree remove --force` refusing unregistered paths) remained intact, but the wrapper-side defense-in-depth was looser than reviewers expected.
Tightened to take the resolved cache root as an explicit positional arg (option B from the deferred-tracker brief — explicit > implicit, doesn't fragile on path arithmetic, JS-side already has the source of truth):
- `distill-workspace.ts`: `spawnDistillInWorktree` now passes `resolveCacheRoot(vault)` as the 12th positional arg of the wrapper. The same function that builds `workspace.worktreePath` produces this value, so a future cache-layout change updates both sides at once.
- `distill-wrapper.sh`: parses the new `EXPECTED_CACHE_ROOT` arg (empty default for backward compatibility with out-of-tree callers on the 10-arg shape). `safe_rm_worktree` now takes an optional second positional arg `<expected_cache_root>`. When non-empty, it canonicalises the cache root with `pwd -P` and requires the resolved worktree to begin with `<resolved-root>/` (trailing slash prevents sibling-prefix collisions like `/cache/abc-evil` matching root `/cache/abc`). When empty, falls back to the legacy `*/napkin-distill/*/*` glob so older callers still work. Both call sites (salvage + cleanup trap) pass `EXPECTED_CACHE_ROOT`.
- Tests: `wrapper-salvage.test.ts` `safe_rm_worktree` describe block grows three new strict-mode tests covering (a) acceptance of paths inside the expected cache root, (b) refusal of paths matching the legacy glob `*/napkin-distill/*/*` but OUTSIDE the expected root (the case the prior glob would have accepted — direct regression for SEC-2/CORR-3), and (c) refusal of sibling-prefix collisions like `/cache/abc-evil` vs root `/cache/abc`. The two existing legacy-mode tests still run unchanged.
- `_test-helpers.ts`: `runWrapperWithStub` now passes `path.dirname(workspace.worktreePath)` as the new positional arg so wrapper-validation and wrapper-salvage end-to-end tests exercise strict mode by default. The dedicated safe_rm_worktree describe-block tests still cover legacy mode explicitly.
463 tests run, 451 pass, 12 skip, 0 fail. tsc + biome clean.
* fix(distill): pre-existing markers don't classify as agent-induced (CORR-1)
validate_no_markers used to flag any vault file with co-present conflict markers as agent-induced (markers-after-agent-exit). But pre-existing markers — from a prior failed run, user error, or a botched manual merge — would also trip the validator and be misattributed to the agent.
Capture a pre-distill marker snapshot just before the agent runs (after START_SHA is recovered from meta.json), then diff it against the post-distill snapshot. Files marker-bearing in BOTH snapshots are pre-existing — not the agent's fault. Files marker-bearing only in the post snapshot are NEW — agent-induced.
Dispatch on the diff:
rc 1 (NEW markers, alone or alongside pre-existing) → markers-after-agent-exit (existing reason; agent's run made things worse, dominant signal)
rc 2 (only pre-existing markers, agent didn't introduce any) → pre-existing-markers (new reason code; recovery hint points the user at fixing the listed files manually before re-running distill)
Implementation:
- Extract list_marker_files <vault> <output_file> helper (the per-file all-three-co-present predicate stays in one place; previously inlined in validate_no_markers)
- Add PRE_DISTILL_MARKER_FILES_FILE tmp file capture between FORCE_CLEANUP hook and 'cd PARENT_CWD'; cleaned up in the EXIT trap. mktemp failure degrades gracefully to legacy every-marker-is-agent-induced behaviour
- validate_no_markers now takes the pre-distill snapshot path as second arg; uses comm(1) for the set diff. Empty path collapses to legacy classification (out-of-tree callers / tests using the 1-arg shape continue to work)
- salvage() gains a pre-existing-markers reason hint pointing the user at the listed files; the wrapper's prefix-match dispatch in formatOutcomeNotification handles failed:pre-existing-markers without further changes
Tests:
- agent CLEANS pre-existing markers (post snapshot empty) → merged-content
- only pre-existing markers, agent commits unrelated file → failed:pre-existing-markers
- pre-existing AND new markers (agent makes things worse) → failed:markers-after-agent-exit (dominant signal)
- regression guard: no pre-existing, agent-induced markers → failed:markers-after-agent-exit (unchanged)
Tests: 455 pass (was 451, +4), 12 skip, 0 fail. tsc --noEmit clean. lint clean.
* refactor(distill): extract DistillOutcome interface (CLEAN-7)
The wrapper-emitted outcome sidecar shape ({ outcomeClass, outcomePath, partialMergeLogPath, recoveryHint }) was hand-redeclared at 4 call sites in extensions/distill/index.ts (the strategy interface's checkOutcome callback, the runDistillWith dispatch's outcome variable, the worktreeSpawnFn's checkOutcome return, and formatOutcomeNotification's input). The source-of-truth was findDistillOutcomeForBranch's return type in distill-workspace.ts.
Extract DistillOutcome as an exported interface in distill-workspace.ts. Replace the 3 full-shape sites in index.ts with DistillOutcome | null. formatOutcomeNotification (which only reads 3 of the 4 fields — no outcomePath) takes Pick<DistillOutcome, 'outcomeClass' | 'partialMergeLogPath' | 'recoveryHint'> | null so its dependency on the parent shape is documented in the type.
Pure type-rename refactor: no runtime change. Phase B's planned trim of partialMergeLogPath (after the merge driver retires) will now be a single-edit.
Tests: 455 pass, 12 skip, 0 fail. tsc --noEmit clean. lint clean.
* chore(distill): rename DISTILL_PROMPT, drop stale env-var docs (CLEAN-3, CLEAN-2)
CLEAN-3: rename the legacy DISTILL_PROMPT constant in extensions/distill/index.ts to LEGACY_DISTILL_PROMPT. The constant is used only by the legacy spawnDistill path (argv-based, pre-PR-12); the canonical agent-driven prompt lives in distill-prompt.md and loads via buildDistillPrompt. Both share roughly steps 1-5 (overview/templates/identify/search/daily-note). Naming the legacy one explicitly makes the drift hazard visible — adding a step here without cross-checking distill-prompt.md is now a code-review red flag rather than a hidden invariant. Add a docblock pointing maintainers at the canonical source.
Updated: index.ts (constant declaration + the spawnDistill arg site that passes it). spawn-distill-legacy.test.ts comment that references args[6] as 'the DISTILL_PROMPT positional'. The 'DISTILL_PROMPT location' citation in distill-prompt.ts:10 is left intact — it's a quoted design-doc section title, not a code reference. (DISTILL_PROMPT_PATH in distill-prompt.ts is a different constant — path to the .md file — and is unaffected.)
CLEAN-2: drop the two NAPKIN_GIT_RETRY_MAX / NAPKIN_GIT_RETRY_DELAY env-var lines from distill-wrapper.sh's Environment header docs. The docs claimed those vars were 'forwarded to git_retry (cleanup paths only)' but PR #12 Phase A removed every git_retry invocation from the wrapper — the agent owns merge/squash/push retries now. The vars are inert in the wrapper. Phase B's CI-1 deletes the lingering source line for git_retry.sh entirely.
Both findings are never-deferrable per the methodology guide (stale references / drift hazard).
Tests: 455 pass, 12 skip, 0 fail. tsc --noEmit clean. lint clean.
* refactor(distill): delete dead buildWorktreeDistillPrompt + tests (CLEAN-1)
buildWorktreeDistillPrompt was the worktree-isolation prompt prefix helper from PR #11 (POST-R6-CACHE) but PR #12 moved that framing into distill-prompt.md. The function had zero production callers; only its own unit tests and a misleading comment in spawn-distill-legacy.test.ts referenced it.
Originally scoped for Phase B (dead-code cleanup) per the deferred.md tracker, but pulled into Phase A per the methodology guide's re-flagged-findings rule: spec-blind reviewers re-flagged this same dead code 3 times across Round 1 (CLEAN-A-5/SEC-A-11/CORR-A-4 — 3-way consensus), Round 2 (CLEAN-1 R2), and Round 3 (CLEAN-1 R3). The 3rd flag is the signal to improve the code, not to re-decline.
Changes: delete buildWorktreeDistillPrompt from distill-workspace.ts; remove the describe("buildWorktreeDistillPrompt (POST-R6-CACHE)", ...) block and import from spawn-distill-in-worktree.test.ts; update the worktree-isolation pin in spawn-distill-legacy.test.ts to reference distill-prompt.md (the actual home of that framing now) and check against the current signature phrase "lives in the git worktree at" instead of the obsolete "isolated git worktree at".
Tests: 453 pass / 12 skip / 0 fail (down from 455 by the 2 deleted assertions in the dead-code describe block). tsc clean. lint clean.
* fix(distill): graceful degradation on post-distill mktemp failure (CORR-1, SEC-1 R3)
When the post-distill marker validator's mktemp call fails (full disk, locked-down TMPDIR, etc.), the wrapper used to return rc 1 from validate_no_markers — which the caller dispatched to failed:markers-after-agent-exit. That misled the user into believing the validator had observed conflict markers in the vault and reverting a possibly-correct squash commit on that false claim. The validator never actually scanned.
Round 3 cross-reviewer consensus (CORR-1 R3 + SEC-1 R3) flagged this as misclassification that defeated CORR-1 R2's anti-misattribution work for the unobserved-validator case. Fix: distinct rc 3 from validate_no_markers — the caller maps rc 3 → failed:internal-validator-error whose recovery hint truthfully tells the user the validator could NOT run, and offers 'git revert' as a CONDITIONAL recovery step (gated on the user inspecting the vault manually) rather than a categorical instruction.
Note: pre-distill mktemp failure already gracefully degraded prior to this fix (PRE_DISTILL_MARKER_FILES_FILE empty → comm collapses to legacy every-marker-is-NEW classification). That path is unchanged; the regression risk was specifically in the post-distill scan.
Reason-code listing comments updated in three places (validate_no_markers helper docstring, post-agent-dispatch listing, salvage() reason-code listing) to reflect the new internal-validator-error code. JS-side dispatch in formatOutcomeNotification needs no change — the failed:<reason> default-case prefix-match handles it generically.
Tests: new mktemp-failure simulation in wrapper-validation.test.ts using a PATH-shim (fake mktemp that fails for the post-distill-marker template, delegates real mktemp for everything else — needed because TMPDIR=/nonexistent also breaks the agent stub). 454 pass / 12 skip / 0 fail (+1 new test). tsc clean. lint clean.
* fix(distill): pi_stderr tmpfile cleanup + naming prefix (SEC-2 R3)
The agent's stderr-capture tmpfile (pi_stderr) had two hygiene gaps: it used the default mktemp template (yielding anonymous tmp.XXXXXX in $TMPDIR with no attribution back to napkin-distill), and it wasn't registered in the EXIT trap. The inline 'rm -f "$pi_stderr"' that runs after the agent exits handles the happy path, but a SIGTERM-on-grace, OOM kill, or any wrapper crash before that block leaves the file orphaned.
Round 3 security finding (SEC-2 R3) called this out as defense-in-depth attributability + leak hygiene. Fix:
1. mktemp template gains the napkin-distill- prefix (mktemp -t napkin-distill-pi_stderr.XXXXXX) so orphans are traceable when found in TMPDIR. 2. cleanup() trap function gains a guarded 'rm -f "$pi_stderr"' alongside the existing PRE_DISTILL_MARKER_FILES_FILE cleanup. The trap fires on normal exit and on SIGTERM-grace (the latter is the relevant additional coverage). Kernel-enforced SIGKILL still bypasses the trap — that's expected; the new naming prefix is what makes those orphans attributable so an admin can mass-clean them.
Tests: no new tests — this is defense-in-depth cleanup; the existing 'no .tmp straggler in error dir' test (SEC-A-4) already exercises the happy-path cleanup and stays green. 454 pass / 12 skip / 0 fail. tsc clean. lint clean.
* refactor(distill): delete napkin-distill-merge driver script and auto-setup install (B1)
PR #12 Phase B item B1 — remove the now-dead per-file LLM merge driver and stop installing it on new vaults. The agent-driven merge architecture (Phase A) made this code unreachable: the distill agent now resolves merges itself in its worktree (steps 7-10 of distill-prompt.md), so there is no driver to register, no .gitattributes rule to install, and no scripts.test.ts to maintain.
Deleted: extensions/distill/scripts/napkin-distill-merge (455 LOC bash driver), extensions/distill/scripts.test.ts (driver test suite — would fail at module-load once MERGE_DRIVER_SCRIPT export is removed; pulled forward from B2 to keep tests green between commits per methodology), MERGE_DRIVER_SCRIPT export from scripts-paths.ts, registerMergeDriver() helper + its call site in distill-workspace.ts, GITATTRIBUTES_LINES + NAPKIN_MERGE_DRIVER + detectConflictingMdMergeRule + SetupResult.conflict from auto-setup.ts, the G7 conflict-dispatch arm + comment in index.ts.
Test cleanup: remove pre-PR-12 '*.md merge=napkin-distill-merge' fixture writes from routing.test.ts, pollhandle-timeout.test.ts, shutdown-handler.test.ts, spawn-distill-in-worktree.test.ts (lines were inert no-ops post-driver-deletion). Drop the G7 'session_shutdown handler — conflicting .gitattributes blocks setup' integration test from shutdown-handler.test.ts (the conflict path it pinned no longer exists). Drop the two merge-driver-specific tests from distill-workspace.test.ts (shell-quoting + space-in-vault-path). Rewrite auto-setup.test.ts to drop GITATTRIBUTES_LINES / NAPKIN_MERGE_DRIVER / G7 conflict-detection coverage and update the 'fresh vault' / 'partial-setup' / FB-2 tests so .gitignore is the only scaffolded file.
Migration policy (locked per design.md 'Migration', user direction 2026-05-15): manual cleanup, no automatic removal. Existing vault .gitattributes files retain the now-orphaned '*.md merge=napkin-distill-merge' line — git falls back to its built-in merge driver once the script is gone, so the line becomes inert, not harmful. Auto-setup change removes only the install path; no remove-old-rule path is added. README will document manual cleanup steps in Phase D (D1).
Net delta: -1960 / +58 LOC. Tests: 423 pass / 12 skip / 0 fail (down 31 tests vs Phase A tip 454: -25 from scripts.test.ts deletion, -3 from distill-workspace.test.ts merge-driver tests, -1 from shutdown-handler.test.ts G7, -2 net from auto-setup.test.ts G7 + GITATTRIBUTES rewrites). bunx tsc --noEmit clean. bun run lint clean.
Refs: design.md 'PR scope table > Phase B > B1' / 'What gets deleted' / 'Migration'. Closes CLEAN-A-8, CLEAN-5 (R3) from deferred.md.
* refactor(distill): delete merge-driver-specific tests (B2)
PR #12 Phase B item B2 — remove tests that exercised the now-deleted napkin-distill-merge driver and the wrapper salvage / merge-driver code paths that B3 will trim. With the agent owning merge resolution end-to-end, these tests pin behaviors that no longer exist.
Deleted from extensions/distill/spawn-distill-in-worktree.test.ts: the 'distill-wrapper.sh (partial-merge salvage)' describe.skip block (3 tests, NAPKIN_DISTILL_MERGE_MOCK=fail-driven salvage flow), the 'distill-wrapper.sh (MERGE_HEAD escape-hatch)' describe.skip block (2 tests, FORCE_MERGE_HEAD/FORCE_MERGE_RC testing-hook coverage), the 'distill-wrapper.sh (LLM-resolved conflict, end-to-end)' describe.skip block (2 tests, NAPKIN_DISTILL_MERGE_MOCK=ok happy-path coverage), and the four pending-Phase-C 'happy path' / 'empty distill' / 'POST-CONV-1' / 'concurrent worktrees' test.skip entries plus their now-orphaned helpers (runWrapper, createWorkspaceWithChanges) inside the integration describe. Also dropped the master-default-branch test.skip (Phase B/C-annotated). Phase C will replace the four happy-path / no-content / pi-self-commit / concurrent slots with bash-stub mocked-pi fixtures that exercise the agent's behavior space.
Side-effect cleanup: 'distill-wrapper.sh (non-main default branch)' describe block lost its only consumer of sessionFile (the deleted master-default test.skip), so the let-binding was removed; the createSeededSessionFile call is retained for its side-effect of seeding the SessionManager disk fixture.
scripts.test.ts deletion was pulled forward into B1 to keep tests green between commits (it imports MERGE_DRIVER_SCRIPT, which B1 removed). FORCE_MERGE_HEAD/FORCE_MERGE_RC search came up empty in wrapper-validation.test.ts — those were already absent.
Net delta: -1145 / +44 LOC. Tests: 411 pass / 0 skip / 0 fail (down from 423 pass / 12 skip after B1; -12 from the test.skip / describe.skip deletions). bunx tsc --noEmit clean. bun run lint clean.
Refs: design.md 'PR scope table > Phase B > B2' / 'What gets deleted'.
* refactor(distill): trim wrapper + JS-side merge-driver-specific code paths (B3)
PR #12 Phase B item B3 — remove the residual merge-driver-specific code paths in the wrapper and the JS-side dispatch arm. With B1 + B2 done, no caller invokes any of these paths; this commit deletes them.
Wrapper (extensions/distill/scripts/distill-wrapper.sh): drop the 'source git_retry.sh' line and the HERE script-dir resolution that only existed to load it (CI-A-1 R1, CI-1 R2, CORR-8 R3); refresh the 'single fatal-error log per branch' comment to drop the now-stale 'PR #12 removes the partial-merge log' wording. NAPKIN_DISTILL_MERGE_TIMEOUT_SECS / NAPKIN_DISTILL_MERGE_MOCK / MERGE_HEAD escape-hatch / partial-merge log emission / salvage-via-checkout-main paths were already absent from the post-Phase-A wrapper (Phase A's A2 rewrite never carried them forward; only the source-git_retry stub remained, hence single-line wrapper change here).
Helper script: delete extensions/distill/scripts/git_retry.sh (65 LOC) entirely — no callers remain after the wrapper's source line is gone. Also drop the GIT_RETRY_SCRIPT export from extensions/distill/scripts-paths.ts (only referenced by the deleted scripts.test.ts in B1).
JS-side cleanup (CLEAN-2 R3 / CORR-7 R2 / CORR-10 R3 — partial-merge JS-arm cleanup): drop the partialMergeLogPath field from the DistillOutcome interface in distill-workspace.ts, drop the partial-merge-specific parser branch in findDistillOutcomeForBranch (the .partial-merge.log path lookup), drop the 'partial-merge' switch arm + readPartialMergeLog plumbing from formatOutcomeNotification in index.ts, and update its docstring/severity-contract comment in index.ts and routing.test.ts to drop the partial-merge row. The merged-local row is now documented alongside merged-content / no-content / failed:<reason>.
Test updates: drop the four partial-merge tests + the partialMergeLogPath:null fixture cruft in index.test.ts (formatOutcomeNotification suite). Drop the two partial-merge tests in error-log-surfacing.test.ts (findDistillOutcomeForBranch suite); rename the surviving 'returns class + path, no partial-merge log' tests to 'returns class + path' since the field is gone. Retain the two R8-CC-1 .partial-merge.log filter tests (findDistillErrorLogForBranch / assertNoWrapperFailures) — they pin the defensive filter that excludes orphaned .partial-merge.log files left over from PR #11 vaults.
What's NOT deleted (deliberately): the auto-setup.ts doc-comment + .gitignore-only-scaffold comment that explain the migration policy ('PR #12 deleted that driver — the distill agent now resolves merges itself in its worktree...'). These are user-facing migration notes that Phase D's README will mirror; deleting them would lose context. The 'napkin-distill-merge' string only appears as policy explanation, not as a live identifier.
Net delta: -249 / +24 LOC. Tests: 405 pass / 0 skip / 0 fail (down from 411 after B2; -6 from the merge-driver dispatch / partialMergeLogPath test deletions). bunx tsc --noEmit clean. bun run lint clean.
Refs: design.md 'PR scope table > Phase B > B3' / 'What gets deleted'. Closes CLEAN-A-10, CLEAN-2 (R3), CORR-7 (R2), CORR-10 (R3), CI-A-1, CI-1 (R2), CORR-8 (R3) from deferred.md.
* docs(distill): refresh stale doc-comments referring to deleted merge driver (CORR-1, CLEAN-1, CLEAN-2, CLEAN-3, CORR-2)
Phase B Round 1 surfaced 4 medium-severity stale-prose findings across 3 reviewers (cross-reviewer consensus). Per methodology never-deferrable category (stale documentation), comment refresh is mandatory in this PR.
Refreshed 5 doc-comment sites in production code: distill-workspace.ts:712 (wrapper-failure-log description), index.ts:996 (forensic log description), index.ts:1125 (runAutoDistill JSDoc), index.ts:1179 (worktreeSpawnFn JSDoc), auto-setup.ts:351 (seedless-init corner-case comment).
Each refresh removes references to the deleted merge driver / merge-driver 3-strike / .gitattributes routing and replaces with the agent-driven flow per design.md "Locked decisions". Comment-only changes; behavior unchanged.
Authored by Phase B Pass 1 fixer subagent (terminated mid-commit at 7.1m). Orchestrator persisted the pre-authored changes via git add + commit (state management, not authoring) per methodology's "final one-line polish" allowance.
* chore(distill): remove orphan NAPKIN_GIT_RETRY_* env vars from test fixture (CLEAN-4)
These env vars were consumed by git_retry.sh which Phase B deleted (B3, fac0cb1). They became inert no-ops in this test fixture.
Removed lines 819-820 of spawn-distill-in-worktree.test.ts. Test still passes — the env vars had no observable effect post-deletion.
References: CLEAN-4 (B-R1) per Phase B Round 1 cleanness review.
* test(distill): bash-stub agent-behavior fixtures (PR #12 C1)
Adds extensions/distill/test-fixtures/agent-stubs/ with 10 executable bash scripts simulating each behavior class from the design's Mocked-pi behaviors testing plan: clean-distill, conflict-resolve-clean, conflict-leave-markers, no-distill, squash-skipped, multiple-commits-on-main, pushed-success, push-fail-merged-local, agent-timeout, agent-crashes.
Each fixture is self-contained, reads NAPKIN_STUB_* env vars from the test harness for vault/worktree/branch paths, and produces deterministic filesystem effects (commits, marker files, etc.) so the wrapper's full pipeline (post-validation, salvage, outcome dispatch) can be exercised without burning real LLM tokens.
Adds an exclusion to package.json files so the test-fixtures dir stays out of the published bundle (verified via bun pack --dry-run). Phase A's existing inline writePiStub patterns in wrapper-validation.test.ts and wrapper-salvage.test.ts are left in place; this commit only adds the formal fixture surface. C2 wires tests against these fixtures.
Per design spec PR scope table > Phase C > C1.
* test(distill): integration tests against bash-stub fixtures (PR #12 C2)
Adds extensions/distill/agent-driven-merge.test.ts with 10 integration tests, one per Mocked-pi behavior class, driving the wrapper end-to-end against the formal fixtures from C1. Each test points NAPKIN_DISTILL_PI_BIN at a fixture file and asserts the resulting outcome class + sidecar contents.
Extends runWrapperWithStub in _test-helpers.ts with an opts.fixturePath knob that bypasses the inline writePiStub step and auto-injects NAPKIN_STUB_VAULT/_WORKTREE/_BRANCH/_DEFAULT_BRANCH env vars derived from the workspace, so fixtures can reach the test scaffold without JS-side template-string interpolation. Inline-stub callers ignore these env vars; behavior unchanged for existing tests.
Coverage relationship to existing tests: wrapper-validation.test.ts and wrapper-salvage.test.ts already cover most validators/salvage paths in isolation. This file adds a higher-level integration view + fills four genuine gaps that inline-stub tests don't reach: (a) conflict-resolve-clean — actual git-merge-with-conflict + agent resolves + squashes, (b) squash-skipped — agent commits to worktree distill branch but never squashes, (c) multiple-commits-on-main — wrapper accepts >=1 commits per validate_commit_count contract, (d) pushed-success — origin configured + agent pushes successfully (complementing the existing merged-local test that covers the no-push case).
Test count: +10 (405 → 415). Per design spec PR scope table > Phase C > C2.
* test(distill): full-document prompt snapshot (PR #12 C3)
Adds a toMatchSnapshot regression guard over buildDistillPrompt(SAMPLE_INPUTS) to catch unintended drift in the rendered prompt prose. Existing tests in distill-prompt.test.ts assert specific substrings (10 numbered step markers, force-push prohibition, pull-merge-not-rebase, worktree-isolation prefix); the snapshot pins the FULL document at canonical sample inputs so a copy-edit that softens a directive (e.g. "never use --force" → "avoid --force") is caught even if the named substrings still match.
Snapshot file lands at extensions/distill/__snapshots__/distill-prompt.test.ts.snap (Bun's default location, sibling to the test file). The snapshot is checked into git; int…
* docs(changelog): changes from v0.2.4 to v0.2.4-20260518.0 * chore(release): curate v0.3.0 release notes + bump version Add curated user-facing summary to CHANGELOG.md's USER-EDITABLE section: agent-driven merge architecture, distill.maxDurationMinutes config knob, race-fix outcome-write ordering, macOS in CI. Bump package.json version 0.2.4 -> 0.3.0 (minor: new architecture, new config key, deleted public surfaces — verify:agent-prompt and the merge driver registration). * docs(changelog): changes from v0.2.4-20260518.0 to v0.3.0 * docs(changelog): expand v0.3.0 release notes to cover all changes vs upstream v0.2.4 Replace the curated 4-bullet user-facing summary with a comprehensive list of every distinct change since upstream Michaelliv/pi-napkin v0.2.4 (66 commits). Grouped by impact area: - Architecture (agent-driven merge, worktree-based isolation, wrapper post-validation, salvage path, distill.maxDurationMinutes, externalised prompt template, /distill command + status surfaces) - Hardening (force-push detection, safe_rm_worktree path safety, atomic outcome writes, prompt-injection metachar rejection, marker classification, pi stderr cleanup, salvage cleanup-trap path guard) - Race fix (drop step 10, salvage reorder, wrapper-invariant test) - Test infrastructure (verify:e2e, bash-stub fixtures, integration suites, _test-helpers, snapshot fixture realism) - CI (macOS matrix, bun pin, coreutils install, git identity in fixtures, timeout(1) hardening) - Cleanup (named-constant extraction, speculative-compat deletion, dead-code purge from feature deletions, stale-reference sweep, DistillOutcome interface) * docs(changelog): rewrite v0.3.0 user-editable section to cover all fork drift Earlier draft was scoped only to PR #12's body of work. The cad0p fork has additional PRs landed since upstream Michaelliv/pi-napkin v0.2.4 (the actual fork point): #2, #3, #5–#11 plus the napkin-ai dependency switch to the cad0p fork. v0.3.0 is the first semver release on this fork and consolidates ALL of that drift, not just the most recent PR. Replace the v0.3.0 user-editable section with a flat bullet list, one bullet per fork-drift change, in chronological order. PR #12 collapses to a 2-line summary (its full breakdown lives in the PR description); the rest get a single descriptive line each. * docs(changelog): make PR references markdown links Match the format used by the auto-generated Features section so all PR references render as clickable links: `[#N](https://github.com/cad0p/pi-napkin/pull/N)`. * docs(changelog): drop redundant 'see PR #12 description' tail The PR link earlier in the same bullet already points readers there; the explicit pointer is noise. * docs(changelog): collapse v0.3.0 fork-drift list to thematic groupings Previous flat 14-bullet list was verbose. Consolidate into 6 themed bullets: agent-driven merge (#12), worktree concurrency (#11), /distill-auto-this-session (#10), distill prompt enhancements (#5+#6+earlier), @cad0p/napkin dependency switch (#7+#8+#9+earlier), and UX fixes (#2+#3). Each bullet links the relevant PRs inline so readers can jump to detail when they need it. --------- Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> Co-authored-by: Pier Carlo Cadoppi <dev@pcad.it>
…m + findings Rename ensureVaultReadyForAutoDistill to ensureVaultReadyForDistill, since the function now also runs before manual /distill on subdir-layout vaults (the worktree-spawn path) and the 'Auto' prefix would be misleading. The old name and new signature do not co-exist; both land in this commit so callers can't accidentally use the legacy contract. Extend the function with two new shapes that subsequent commits will fill in: - HealthLevel = 'fast' | 'full' parameter, required (no default). Each call site explicitly chooses depth; this catches accidental cheap-path consumption at compile time. - HealthFinding[] field on SetupResult, always populated (empty array when no invariants fired). Carries structured per-invariant outcomes for callers to render via notify and decide whether to abort. Existing logic is unchanged; findings is [] for now.
…tection
Replace the line-by-line append in mergeLines with a managed-block
rewrite. The bracketed region between '# BEGIN NAPKIN-DISTILL MANAGED'
and '# END NAPKIN-DISTILL MANAGED' is owned by auto-setup; everything
outside the markers is user territory and is preserved byte-identically.
The new mergeManagedBlock helper handles every shape we have to
support:
- Markers absent, no orphan canonical lines: install the block (recovery
'installed').
- Markers absent, orphan canonical lines anywhere in the file: remove
orphans + install block (recovery 'migrated from line-by-line').
This is the v0.3.0 -> v0.3.1 migration path.
- Markers well-formed, content matches: idempotent no-op.
- Markers well-formed, content drifts: rewrite the bracketed region in
place (recovery 'reset').
- Multiple BEGIN markers / BEGIN without END / END before BEGIN: refuse
to auto-fix; emit gitignore-block-correct error finding so the user
can resolve manually.
BLOCK_CONTENT is a strict superset of the v0.3.0 GITIGNORE_LINES, pinned
by a unit test so future edits to either constant break loudly rather
than silently dropping a SEC-5 secret pattern. GITIGNORE_LINES stays
exported for one release as the migration source-of-truth and is marked
@deprecated.
Findings shape:
- Successful rewrites: { kind: 'auto-recovered', invariant:
'gitignore-block-correct', recovery: '<flavor>' }
- Malformed markers: { kind: 'error', invariant:
'gitignore-block-correct' } - file untouched.
The findings array is plumbed but the call sites that surface notifies
and abort spawns are wired up in a subsequent commit.
…ON validity Wire three structured findings into the existing fast-level lifecycle: - subdir-layout (loud error): the legacy-embedded refusal now also emits a finding so callers can render a notify and abort. The legacy 'error' field is preserved for backward compatibility. - vault-is-git-repo (auto-recovered): the existing 'git init' branch emits a finding when the recovery actually fires (i.e. .git/ was absent and we initialised it). - config.json-valid-json (loud error): new check. Read <configPath>/config.json and JSON.parse it. On parse failure, emit a loud-error finding describing the corruption. Skip silently if the file is missing — the missing-file case is the typical fresh-setup state (napkin init writes the file later) and not a health concern. The findings array is plumbed through the existing return shapes; the call sites that surface notifies and abort spawns are wired in a subsequent commit.
…ree-spawn paths
Run the full-level health check immediately before each worktree-based
spawn (interval tick, manual /distill on subdir-layout, session_shutdown
handler). The new surfaceHealthFindings helper renders auto-recovered
findings as a single info notify, error findings as a single error
notify, and returns hasErrors so the caller can abort the spawn cleanly.
Three call sites are wired:
- runAutoDistill: health check inside spawnFn, before worktreeSpawnFn.
Auto-distill always uses the worktree path; aborting on hasErrors
skips the wrapper subprocess entirely.
- runDistill (manual /distill): health check inside the worktree
branch of spawnFn. Legacy-embedded routing (configPath ===
contentPath) bypasses the health check and falls back to
legacySpawnFn unchanged — preserves the zero-friction experience
for legacy vaults.
- session_shutdown handler: health check before spawnDistillInWorktree.
On hasErrors, lastSpawnedSize is left untouched so a subsequent
recovery attempt is not silently deduplicated.
The helper lives in a separate health-notify module so its contract is
testable in isolation and easy to extend (Phase B's full-level
invariants will share the same surfacing pipeline).
session_start continues to call the function at fast level; surfacing
fast-level findings (and gating setupFailed on them) is left to a
follow-up commit since the existing session_start flow already
reports the legacy 'error' field.
…loses #14) Issue #14: distill worktrees report 'Empty vault' on vaults where .napkin/config.json is untracked. The worktree is checked out via 'git worktree add HEAD' which copies only tracked files; an untracked config.json never reaches the worktree, so napkin's findVault inside the worktree falls back to the legacy embedded layout and writes silently bypass the worktree. Add a full-level invariant config.json-tracked: when level === 'full', probe '<configPath>/config.json' with 'git ls-files --error-unmatch'. On non-zero status, push the relative path onto scaffolded[] and emit an auto-recovered finding. The existing scaffolded[] consumption (existing-repo branch's 'git add ...scaffolded' + commit) stages and commits the file on the same pass, so the next worktree-add picks up the tracked file. Fast-level (session_start) does NOT run this check \u2014 the tracking probe is the user-facing slow path of full-level (a single git invocation per spawn) and would add latency to every session start where it is not needed. The wiring from the prior commit fires this check at every worktree- based spawn (auto-distill tick, manual /distill on subdir-layout, session_shutdown), so the recovery happens transparently before the spawn that would otherwise hit the bug.
The session_start handler consumed setup.error, setup.initialized,
and setup.scaffolded but never read setup.findings. As a result,
fast-level error invariants emitted in earlier commits — invalid
config.json and malformed managed-block markers — fired the
structured finding but no notify, leaving setupFailed = false.
Auto-distill stayed armed; the user only saw the error at the first
interval tick (~60 minutes by default), violating the design's
contract that fast-level errors block subsequent auto-distill
activity for the session.
Wire the existing surfaceHealthFindings helper into session_start so
the same notify treatment applies as at the worktree-spawn call sites:
- error findings -> error notify + setupFailed = true
- auto-recovered findings (gitignore install / migrate / reset) ->
info notify
- healthy vault -> no notify
The legacy setup.error path is preserved for git init / add / commit /
scaffolding-write failures that don't produce structured findings;
setupFailed = !!setup.error || hasErrors covers both signals. The
legacy-embedded layout case keeps its README-pointer notify (the
generic per-finding render can't carry the migration instructions) and
skips the structured render to avoid firing two error notifies for the
same condition. The first-run onboarding notify (file count, undo
command, opt-out hint) still fires alongside the structured render
because it carries scope context the per-finding text doesn't.
The previous setup.scaffolded.length > 0 info notify is dropped: the
gitignore auto-recovery now flows through surfaceHealthFindings's
info-notify path with the per-flavor recovery label
("installed" / "reset" / "migrated from line-by-line").
(CORR-1, SBT-1, COV-1, CORR-2)
…med gitignore
Two new tests in health-check-wiring.test.ts pin the session_start
branch of the per-spawn wiring contract:
- Pre-corrupt `.gitignore` (BEGIN without END) BEFORE session_start.
Assert: one error notify with "malformed", subsequent interval tick
creates zero worktrees (autoDistillSuppressed=true via setupFailed).
- Healthy subdir vault. Assert: no error notify, the install info
notify drains, the next interval tick creates exactly one worktree
(setupFailed=false branch).
The orchestrator's task hint also requested an "invalid config.json at
session_start" test. That path is unreachable today: loadVaultConfig
fails open on JSON.parse errors and returns DEFAULT_DISTILL with
enabled=false, so the session_start handler returns early before
ensureVaultReadyForDistill runs. Surfacing the config.json-valid-json
invariant at session_start would require either propagating the parse
error from loadVaultConfig or moving the validity check to a separate
pre-config probe; both are behavior changes outside the scope of the
current commit set. The function-level `config.json-valid-json` test in
auto-setup.test.ts already pins the finding shape.
Test count: +2 in extensions/distill/health-check-wiring.test.ts.
(CORR-3)
The wiring tests exercise surfaceHealthFindings indirectly via real
extension call sites, leaving three branches with no direct coverage:
- Multi-finding render (>=2 auto-recovered or >=2 errors): the
bulleted-list format is currently observable only when wiring tests
happen to produce multiple findings, which they do not.
- hasUI=false (subprocess / detached test contexts): the helper's
documented contract — "no notifications are emitted but the return
value still reflects the error-finding signal" — is unverified.
- Boolean return shape: a future refactor that returns the count or
array would slip through type-narrowing.
Add a focused health-notify.test.ts with seven cases covering empty
findings, single auto-recovered, single error, mixed, multi-error,
hasUI=false (both error and recovered branches), and the return-shape
pin. Each notify is asserted by exact string equality to lock the
user-facing prefix ("Auto-distill recovered: ..." / "Auto-distill
cannot proceed: ...") and the per-finding rendering ("(<recovery>)"
suffix on auto-recovered, no suffix on errors).
The test ctx is structurally typed inline rather than imported from
health-notify.ts so the helper's interface remains internal-only at
the source level.
Test count: +7 in extensions/distill/health-notify.test.ts.
(COV-2, SBT-2)
The existing malformed-marker tests cover 1 BEGIN + 0 END
("BEGIN-without-END") and 2 BEGIN + 2 END ("two-complete-blocks"). The
mergeManagedBlock predicate rejects three additional shapes that had no
direct test:
- END marker before BEGIN marker (1 of each, but END index <
BEGIN index): hits `beginIndices[0] < endIndices[0]` predicate.
- END marker without any BEGIN (0 BEGIN, 1+ END): hits the
`markersAbsent === false` clause that distinguishes orphan-end
from a clean install.
- Asymmetric markers (2 BEGIN, 1 END): distinct from the
"two-complete-blocks" shape — the marker counts disagree without
forming a duplicate-block pattern.
For each, the test asserts the loud-error finding fires
(`gitignore-block-correct` / kind=error / message contains "malformed")
and the original `.gitignore` is not modified, mirroring the existing
malformed-marker test contracts so a future refactor that drops one
shape from the predicate surfaces immediately.
Test count: +3 in extensions/distill/auto-setup.test.ts.
(COV-4)
…n race window
Two testability hygiene improvements to the per-spawn wiring tests:
(1) The error / info notify assertions previously used loose
`toContain("malformed")` / `toContain("recovered")` matches that would
pass on a wide range of regressions: a typo dropping the
"Auto-distill cannot proceed:" prefix, removing the file path from
the body, or swapping the recovery label all slipped through. Tighten
each assertion to:
- error notifies: `msg.startsWith("Auto-distill cannot proceed: ")`
AND `msg.includes(<filePath>)` AND `msg.includes("malformed")`.
- info notifies on auto-recovery: `msg.startsWith("Auto-distill
recovered: ")` AND `msg.includes("(<recovery-label>)")` for the
canonical "(installed)" / "(reset)" / etc.
The user-visible prefix is part of the public contract (support docs
reference it; users grep notify output). Pinning by exact prefix +
path + invariant keyword makes a future rename or reformat surface
loudly instead of silently.
(2) The `worktreeCount(vault)` assertion observes the worktree dir on
disk after the JS-side spawn returns, but the wrapper is detached and
its cleanup trap can race with the assertion when the wrapper exits
faster than the test can read the cache directory. Mirror routing
tests' approach by calling `withNapkinOnPath()` in `beforeEach` so the
wrapper passes its `napkin --version` smoke check and proceeds along
the same later-failure path as routing.test.ts. The race window is
widened to where it has been verified empirically stable.
A spawn-fn-spy approach (per the spec-blind review's recommendation
(b)) was prototyped using `mock.module` against `./distill-workspace`
but leaks the spy into other test files in the same Bun process
(unless `bun test --isolate` is used, which the project's CI script
does not). Recommendation (a) — `withNapkinOnPath()` — is a smaller
intervention with no cross-file side effects. Documented here so a
future cleanup PR that switches CI to `--isolate` can revisit the
spy approach.
Test count unchanged (9 wiring tests). All assertions tightened.
(SBT-3, SBT-4)
…d unused exports
Four cleanness items, all in the never-deferrable categories
(comments-that-contradict-code and speculative exports):
- auto-setup.ts file-header "Design contract" block claimed
"First-run auto-setup" and "only appends missing lines". The
function now runs at every worktree-based spawn (full level)
and reconciles a managed block in place (rewrites contents,
removes orphans, refuses on malformed markers). Rewrite the
header to describe the recurring health-check role and the
block-reconciliation semantics; reference the per-level
invariant matrix in the function's JSDoc.
- ensureVaultReadyForDistill JSDoc claimed the level parameter was
"reserved for subsequent commits" and that "both calls behave
identically". That was true in the introducing commit but is no
longer: full-level adds the config.json-tracked invariant, and
fast-level explicitly skips it for latency reasons. Replace the
JSDoc with a per-step lifecycle that explicitly notes which
invariants run at which level.
- auto-setup.test.ts test "both 'fast' and 'full' levels return the
same shape on a healthy fresh vault" had a comment claiming the
level parameter "does not yet branch behaviour"; the test still
passed because both levels happen to produce empty findings on
a healthy vault, but the explanatory comment was misleading.
Retitle and update the comment to make the happy-path-only scope
explicit ("on a healthy vault, both 'fast' and 'full' levels
produce empty findings").
- health-notify.ts exported `HealthNotifyCtx` but the only consumer
was `surfaceHealthFindings` in the same file. The direct unit
tests added in the previous commit set use structural typing on
the helper's parameter rather than importing the interface, so
no out-of-file reference exists. Drop the `export` keyword;
`grep -rn "HealthNotifyCtx" extensions/` now returns hits only
inside `health-notify.ts`.
No new tests; existing tests still green.
(CLN-M-1, CLN-M-2, CLN-M-3, CLN-M-4)
At runAutoDistill, runDistill (subdir-layout path), and the inline session_shutdown handler, check both setup.error and surfaceHealthFindings' hasErrors. If setup.error is populated (legacy fail-soft errors from git init / git add / git commit failures inside ensureVaultReadyForDistill), surface via ctx.ui.notify and abort the spawn. Mirrors session_start's existing symmetric handling that the round-1 fixer landed; the spawn-path sites missed the symmetric treatment. Tests added per call site: synthetic setup.error populated + findings empty → exactly one error notify, no spawn. The fourth combined-channel test originally drafted (corrupted config.json + gitignore-as-directory) was dropped as unreachable under current loadVaultConfig fail-open behavior — config.enabled goes false on JSON parse failure, runDistillWith short-circuits before the health check runs. Tracked separately as FIXER-1 in the deferred follow-up backlog. (CORR-1 R2, SBT-2 R2, SBT-3 R2, SBT-6 R2, COV MED-1 R2)
…lock rewrite mergeManagedBlock previously read .gitignore, split on '\n', and joined back with '\n'. On Windows-checkout vaults that store .gitignore with CRLF, this silently stripped the '\r' from every line, looking like spurious churn in git diffs. Detect the existing file's line-ending convention (presence of '\r\n' \u2192 CRLF, otherwise LF) before rewriting, and round-trip it on write. New / empty files default to LF, matching the rest of the repo's TS-code conventions. Tests added: - CRLF input: legacy v0.3.0 line-by-line .gitignore migrates to managed block with CRLF preserved - CRLF input: managed block with drift is reset in place with CRLF preserved - CRLF input: idempotent re-run is byte-identical (no LF/CRLF flip) - LF input: continues to use LF on rewrite (regression check) (CORR-3 R2, COV MED-2 R2)
…s, and superset checks
Loose assertions \u2014 expect.any(String), tautological set-membership,
weak typeof boolean, and ambiguous worktreeCount(0) \u2014 are upgraded to
content-pinning checks so future regressions surface as the test that
should fail rather than silently passing.
- BLOCK_CONTENT/GITIGNORE_LINES superset: replace
set-membership-on-identical-arrays with per-entry toContain(entry)
iteration, plus a length backstop. The original test was tautological
because both arrays are byte-identical; the new shape catches a
per-line typo or accidental drop in either constant.
- Finding message content: replace expect.any(String) with
expect.stringContaining("...") for every error/auto-recovered
finding. The substring is the distinguishing keyword for that
invariant + recovery flavor (e.g. 'Initialized git repo',
'legacy embedded layout', 'did not contain', 'drifted',
'had unmanaged', 'orphan canonical lines outside', 'untracked').
- autoDistillSuppressed test: repair .gitignore between session_start
and the interval tick, so worktreeCount === 0 has only one valid
causal path \u2014 the suppression flag short-circuiting the callback.
The previous shape was satisfied by either suppression OR the
same finding re-firing inside the tick's health check.
- surfaceHealthFindings hasErrors return shape: replace the weak
typeof === 'boolean' pin with Object.keys(...).toEqual(["hasErrors"])
+ toBe(true)/toBe(false), exercising both branches and pinning the
absence of additional fields.
(SBT-1 R2, SBT-4 R2, SBT-7 R2, SBT-9 R2)
…Doc accuracy Three mechanical-hygiene tightenings on the auto-setup interface and its rationale comments: - mergeManagedBlock's fresh-init rationale comment referenced GITIGNORE_LINES as the source of truth for secret-file ignores; after e885919 the live source is BLOCK_CONTENT (GITIGNORE_LINES is the legacy migration baseline). Update the @see reference to BLOCK_CONTENT to stop misdirecting readers to a soon-to-be-deleted constant. - SetupResult.findings: readonly HealthFinding[]. The helper consumer surfaceHealthFindings already takes readonly; tightening the interface side makes accidental post-return mutation a type error at zero runtime cost. Construction site uses local push() then returns; widening on assignment is a no-op for TS structural typing. - SetupResult.error JSDoc: clarify the field is consumed at all extension call sites (session_start, runDistill, runAutoDistill, session_shutdown handler) to surface notify("error") and abort the spawn, rather than implying any 'backward compat' or single-site use. (CLN-M-1 R2, CLN-M-2 R2, CORR-9 R2)
…ror contract The 'Design contract' file-header bullet for auto-setup.ts described `error` as a 'legacy ... preserved for one release' field for callers that branch on the collapsed string. That framing was already contradicted by the SetupResult.error JSDoc (updated in 0cf9fc2), which now describes `error` as the canonical fail-soft channel consumed at every extension call site. The historical claim is also structurally wrong: only session_start branches on the collapsed string (the LEGACY_EMBEDDED_LAYOUT_ERROR sentinel match); runDistill, runAutoDistill, and the session_shutdown handler all branch on truthiness to gate the worktree spawn. Rewrite the bullet to match the field's JSDoc: `error` is the fail-soft channel for generic IO and git-subprocess failures that have no corresponding structured finding (and the legacy-embedded layout sentinel). Doc-only; no behaviour change. Refs: CLN-R3-1, SB-2 R3
…ify pattern
The 8-line fail-soft notify block — "if setup.error is populated and
hasUI, emit \`Auto-distill setup failed: ${setup.error}.\`" — was
duplicated byte-identically (modulo the ctx access path) at the three
worktree-spawn call sites: the session_shutdown handler, runDistill's
spawnFn, and runAutoDistill's spawnFn. The byte-identical 5-line
rationale comment above each block is doing the job a helper should
do; any future change to the prefix or the gating predicate would
need lockstep edits across three sites with nothing but convention to
enforce the symmetry.
Extract a private module-level surfaceSetupError(ctx, setupError)
helper structurally typed on the hasUI + ui.notify subset of
ExtensionContext (mirrors HealthNotifyCtx in health-notify.ts). Each
call site collapses from ~12 lines to one. The session_start handler
keeps its bespoke notify because the contextual suffix ("Disabling
auto-distill for this session.") differs from the worktree-spawn
sites; symmetry is maintained at the structural-typing level.
Existing wiring tests (runAutoDistill / /distill / session_shutdown
with setup.error) cover the helper's behaviour at every call site;
no new tests required.
Refs: CLN-R3-2, SB-3 R3
…lock JSDoc mergeManagedBlock emits four distinct recovery labels in practice: `installed`, `reset`, `migrated from line-by-line`, and the compound `reset and migrated from line-by-line` (well-formed markers + content drift + orphan canonical lines outside the block on the same pass). Three JSDoc sites enumerated only the first three, and the lifecycle description glossed over the well-formed-match-with-orphans-outside case as part of the `reset` branch when it actually flows through the `migrated from line-by-line` branch. Expand the file-header bullet, the INVARIANT_GITIGNORE_BLOCK JSDoc, and the mergeManagedBlock function header to enumerate all four flavors and the five well-formed sub-cases. Add a regression test that pre-commits a .gitignore with both drift inside the markers AND an orphan canonical line outside, then asserts the compound recovery fires (one finding, recovery: `reset and migrated from line-by-line`, both the drift and the orphan resolved). Tests: +1 (458 → 459, all green; 1157 expect() calls). Refs: CLN-R3-lows-info
The wiring tests in `health-check-wiring.test.ts` exercise `surfaceSetupError` indirectly via three real call sites, but no test pinned the helper's contract directly. Mutation-test gaps: `setupError === undefined` (a refactor that hoists the notify out of the truthiness guard would emit `Auto-distill setup failed: undefined.`), `hasUI === false` (subprocess path silently drops the notify), and the canonical message format itself (the trailing period and `error` severity were only loosely pinned via `startsWith` and `toContain` in the wiring tests). Move the helper from `index.ts` to `health-notify.ts` alongside its sibling `surfaceHealthFindings`. Both helpers now share the `HealthNotifyCtx` type and the same dual-channel-output JSDoc, so the test file naturally covers both contracts. Add three tests mirroring the existing `surfaceHealthFindings` shape: 1. undefined setupError -> 0 notify calls 2. populated setupError + hasUI=true -> 1 error notify with the canonical message `Auto-distill setup failed: <error>.` 3. populated setupError + hasUI=false -> 0 notify calls Test count: 458 -> 461 (+3 new). tsc + lint clean. Wiring tests unaffected (the imports were updated to the new module location). (SB-2 R4)
… comment Two stale-doc cleanups surfaced by cumulative review: 1. The new test comment at `auto-setup.test.ts:494` (introduced by the strict-superset assertion test in this branch) referenced "belt-and-braces protections" via a finding-tracker token from a prior review round. Reframe to describe the technical content directly: "secret-pattern protections silently". The two pre-existing test names at lines 143 and 170 inherit the same token shape from PR #11; those are out of scope for this branch and stay untouched. 2. The `GITIGNORE_LINES` JSDoc claimed it was "the source of truth for migration", but the `mergeManagedBlock` orphan-detection reads from `BLOCK_CONTENT` (the canonical managed-block content) rather than from this constant. The two arrays are byte-identical today so the difference is invisible, but a future edit that grows `BLOCK_CONTENT` would leave the JSDoc claim quietly wrong. Reword to describe `BLOCK_CONTENT` as the source of truth and `GITIGNORE_LINES` as a migration shim pinned by test as a strict subset. The `@deprecated` tag and "removed in a future release" guidance both remain. Doc-only changes: no behavior touched; no new or removed tests. Test count unchanged at 461 pass. tsc + lint clean. (SB-3 R4, SB-5 R4)
…ed vault The previous fixture pre-bootstrapped a healthy vault (git init + .napkin/config.json + git add -A + git commit) before triggering /distill. The gate's GREEN signature was uninformative because the new fast-level auto-init code paths (vault-is-git-repo auto-recover, gitignore-block-correct install, scaffold-and-commit) ran in dead-store mode — the fixture handed them a pre-finished vault. The new fixture mirrors the production user-onboarding flow: write .napkin/config.json + content (what `napkin init` and the user produce), trigger session_start, let the production code do `git init`, install the managed gitignore block, scaffold-and-commit. Then trigger /distill to exercise the full-level health check + wrapper subprocess + JS poller seam as before. 5 new post-conditions added on the auto-init phase: - <vault>/.git/ exists after session_start - .gitignore contains the managed block + canonical content - initial commit tracks .napkin/config.json - HEAD resolves to a real commit on main - session_start emitted the info notify for auto-init Result: 14/14 post-conditions pass, auto-init wall ~0.02s, total wall ~134s, cost ~$0.50/run. Closes the methodology gap that prompted the 'end-to-end gate' fixture-shape rule + 'fixture pre-bootstrap of production-created state' anti-pattern.
The previous gitignore post-condition in `assertAutoInitPostConditions`
asserted `BLOCK_CONTENT.every(line => giContent.includes(line))`. Two
gaps:
- Empty-string entries trivially pass: `BLOCK_CONTENT` includes
blank-line separators between sections, and `String.includes("")`
is unconditionally true. A regression in `mergeManagedBlock` that
drops the blank-line separators (collapsing the block to a single
contiguous run) would still pass.
- No locality enforcement: `includes` is a substring search over the
whole file. A pathological regression that drops the BEGIN/END
markers but leaks canonical lines into user territory \u2014 or scatters
them across two malformed managed blocks \u2014 would still pass. That
defeats the JSDoc's central claim ("every line of the canonical
`BLOCK_CONTENT` between them").
Replace with two helpers:
- `extractManagedBlockBody`: parses the file, locates the unique
BEGIN/END markers (rejecting indented variants and duplicates the
same way `mergeManagedBlock` does on read), and returns the lines
strictly BETWEEN them. Returns a tagged result so the post-condition
can surface the specific marker invariant that failed.
- `describeBlockBodyDrift`: walks the extracted body against
`BLOCK_CONTENT` and reports the first structural mismatch (length
differs, or first index whose content differs). Diagnostic detail in
the gate's failure summary points at the dropped/changed line, not
just "drift".
The post-condition now compares `extractedBody` ordered against
`BLOCK_CONTENT` and only passes when length AND every index matches.
Catches: dropped lines (including blank separators), reordered lines,
modified lines, lines moved out of the markers, missing markers,
duplicated markers.
Validation: `bunx tsc --noEmit` clean, `bunx biome check` clean.
Refs: (CORR-1, SB-2, CLN-7)
The gate's `assertAutoInitPostConditions` JSDoc promised that the auto-init invariants are checked BEFORE the bare-origin wiring + `/distill` trigger so a regression in the auto-init path is reported 'as soon as it fires \u2014 not after a 2-minute wait for the wrapper to do work that was already doomed.' But the auto-init success notify (the user-visible signal that the fail-soft branch took the success path) was asserted in `assertGreenPostConditions`, which only runs after the LLM-driven distill phase completes (or times out at 630s). A regression where session_start performs auto-init successfully on disk but silences the notify (e.g. a UI gating wraps it in a stale conditional) would not surface until ~134s of wall clock + ~$0.50 of LLM cost had already burned. Move the notify assertion into `assertAutoInitPostConditions`. The function now takes `notifyCalls` as a second argument, and the auto-init phase's fail-fast short-circuit in `main()` catches notify-silenced regressions before the distill phase runs. `assertGreenPostConditions` keeps its own notify check \u2014 the `Distillation complete (...)` post-LLM dispatch \u2014 and drops the `notifyCalls` parameter (no longer needed; `notify` alone covers the post-LLM contract). The function now only asserts phase-3 invariants, matching its name; a follow-up commit verifies whether the name is fully reconciled with scope after this move. Validation: `bunx tsc --noEmit` clean, `bunx biome check` clean. Refs: (CORR-2, SB-5)
The cleanup pattern `gitEnv.restore() + fs.rmSync(tmpdir) + return`
was duplicated 5+ times across the early-exit branches in `main()`,
with subtle drift between branches:
- The auto-init-fail branch honored `--keep-tmpdir` with a forensic
print; the missing-handler / missing-command / missing-session-file
/ bare-origin-wiring-fail branches did not.
- One branch ran `fs.rmSync` before `setupFixture` could possibly
succeed (no-op but locally surprising).
- The happy path's cleanup was textually distinct from the fail
paths' cleanup despite doing the same thing.
Wrap the body from `setupFixture` onwards in `try { ... } finally`.
The finally block centralises:
- `gitEnv.restore()` (always; matches `installGitEnvOnProcess`'s
contract that callers MUST restore on every exit path);
- tmpdir cleanup (gated on `--keep-tmpdir`, with the forensic print
on the keep branch).
A `tmpdir: string | null` capture before the try-block lets the
finally tolerate the case where `setupFixture` itself throws \u2014 no
tmpdir to clean up yet.
The companion finding (CLN-4) flagged that
`assertGreenPostConditions`'s name might no longer match scope after
the prior commit moved the auto-init notify check out. After the
move, the function only contains phase-3 invariants (poller notify,
no-conflict-markers, HEAD-on-default, squash commit count, distill
branches removed, worktrees pruned, outcome class, origin advanced).
The name reconciles with the scope; no rename needed.
Validation: `bunx tsc --noEmit` clean, `bunx biome check` clean.
Refs: (CLN-3, CLN-4)
Four small cleanups, one cohesive commit: - The `Fixture.startSha` JSDoc referenced `wireAutoInitOnVault`, a helper that does not exist anywhere in the repo. Likely a half-finished rename signal (the actual chain is session_start \u2192 `ensureVaultReadyForDistill` \u2192 `wireBareOrigin`). Rewrite the JSDoc with the real mechanism so a future maintainer's `grep` doesn't hit a dead reference. - `installGitEnvOnProcess`'s JSDoc commits the gate to mutating `process.env` for git identity + signing override because `runGit` (in `auto-setup.ts`) shells out with `env: process.env`. Bun's `spawnSync` empirically does NOT inherit `process.env` mutations made after process startup unless `env:` is passed explicitly. The gate's own git invocations (`wireBareOrigin`'s init/remote/push, every post-condition's `git ls-files` / `rev-parse` / `symbolic-ref` / `rev-list` / `branch` / `worktree`) did not pass `env:`. Today this is benign \u2014 none of these invocations need identity \u2014 but the discipline is asymmetric and one new commit-emitting line away from a silent regression on a CI runner with global `commit.gpgsign=true`. Add `env: process.env` to every `spawnSync` call in the gate script for symmetry with `runGit`. - The commit-count post-condition's comment said `count >= 2 (1 baseline + agent)`, but the assertion is `count >= 1` and the framing dates back to the pre-fixture-upgrade shape (which had an explicit `git commit -m "verify: baseline"` on top of `git init`). In the current shape, `startSha` is captured by `wireBareOrigin` AFTER auto-init has produced the initial commit, so `startSha..HEAD` only contains the agent's commits \u2014 the agent's squash commit alone satisfies `>= 1`. The threshold is correct; the comment lied. Update the comment to match. - `grep -rn 'wireAutoInitOnVault' scripts/ extensions/` now returns zero hits. Validation: `bunx tsc --noEmit` clean, `bunx biome check` clean. Refs: (CLN-1, SB-1, CLN-2, CORR-4)
Two doc/dead-code cleanups in scripts/verify-e2e.ts. Zero behavior change; the gate's GREEN signature is unaffected. describeBlockBodyDrift had a defensive fallback return after the length-mismatch + first-differing-line branches. The helper is only called when giBlockMatches is false, which is exactly the disjunction of those two branches — so the fallback was unreachable by construction. Replaced the silent string return with a throw that names the broken caller-gate invariant explicitly. TypeScript needs some terminator after the loop; the throw makes the unreachability checkable rather than papering over it. The combined-results comment in main() said the auto-init block was 're-run here', but autoInitResults is captured once during phase 1 (in assertAutoInitPostConditions) and reused via spread. Rewrote the comment to describe what the code does: re-include the phase-1 capture so the final summary lists every invariant in one PASS/FAIL block. Also dropped the 'filesystem invariants' framing since the phase-1 assertion now covers both filesystem and user-visible-notify effects. Verified: bunx tsc --noEmit clean, bunx biome check scripts/verify-e2e.ts clean. No need to re-run verify:e2e — the real-LLM gate confirmed PASS at the parent commit 25a8e12 and this commit changes only a comment and an unreachable error path. Findings: CLN-2 R2, SB-3 R2, CLN-1 R2
The previous fixture wrote `.napkin/config.json` directly, which is
the same fixture-pre-bootstrap-of-production-created-state anti-pattern
at smaller scope: production users run `napkin init` to seed the
config, so the fixture should too. Synthesising the config in-process
masks regressions in napkin init's default schema (a section rename, a
default value flip, a dropped `.obsidian/*` artefact) because the
fixture would re-emit whatever shape the gate's hand-rolled JSON
expects.
The new fixture invokes `napkin init --path <vault>` from the
`@cad0p/napkin` CLI dependency (resolved via
`node_modules/.bin/napkin`, which `bun install` populates on both
local and CI runners), then read-modify-writes the resulting
`.napkin/config.json` to add the `distill.*` block. That mirrors the
two distinct user steps: `napkin init` produces the default schema,
then the user edits the config to enable auto-distill. The merge
preserves napkin's other sections (overview/search/daily/templates/
graph/vault) so a default-schema regression isn't masked by overwrite.
Adds `assertNapkinInitPostConditions` covering napkin init's
filesystem artefacts: `.napkin/config.json` parses with all six
default sections plus `vault.root: ".."`, the fixture-patched
`distill.enabled=true` survives the merge, and each of
`.obsidian/{app,templates,daily-notes}.json` is present. Asserted
right after `setupFixture()` so a regression in napkin's default
schema surfaces in seconds rather than minutes (auto-init phase) or
~$0.50 (LLM phase).
GREEN signature unchanged: 19/19 post-conditions pass (was 14/14), wall
time 188s (was ~185s; the napkin-init invocation + config read-modify-
write add <100ms on top of LLM-bound time).
Findings: orchestrator-self-flagged after fixture-shape rule introduction
…zero exit
When `node_modules/.bin/napkin` is missing (fresh checkout without
`bun install`, broken symlink, or the shebang's `node` not on PATH),
`spawnSync` returns `{ status: null, error: ENOENT }` with stdout/stderr
undefined. The previous failure surface read `initRc.stderr || initRc.stdout`,
which collapses to `undefined || undefined = undefined`, leaving the operator
with the opaque `napkin init failed in <path> (rc=null): undefined`. The
diagnostic info \u2014 `spawnSync /\u2026/napkin ENOENT` \u2014 lives on `initRc.error`
and was being dropped.
Add a binary-existence preflight via `napkin --version` mirroring the
`pi --version` preflight in `main()` (which mirrors the production preflight
pattern at extensions/distill/index.ts:1158-1169). Throw early with a path-
naming error message and a `bun install` hint when the version probe hits
ENOENT or exits non-zero.
On the actual `napkin init` invocation, surface
`initRc.error?.message ?? initRc.stderr ?? initRc.stdout` so a missing-binary
failure that slipped past the preflight (or any other `spawnSync` syscall-layer
error \u2014 EACCES, ENOEXEC) lands a useful message instead of `undefined`.
Findings: (CORR-1, SB-3, CLN-2, SB-2, CORR-6)
…itions
The post-init banner check at `setupFixture` matched the substring
"Initialized vault at <vault>/.napkin" against `initRc.stdout`. Two
problems compound:
1. The check breaks under `FORCE_COLOR=1`. napkin's CLI emits the banner
via `${dim("Initialized vault at")} ${bold(result.path)}` (chalk).
chalk strips ANSI when stdout is not a TTY, so under default
`spawnSync` capture the substring matches. Under `FORCE_COLOR=1`
chalk emits codes regardless of TTY, splitting the substring with
`\\e[22m \\e[1m` between "at" and the path. The gate then throws on
a successful napkin init invocation. CI today doesn't set the env
var, but a developer's shell does, and a future CI runner could.
2. The check duplicates the post-condition file-existence assertions
in `assertNapkinInitPostConditions`. The two paths napkin init's
stdout could indicate ("Initialized vault at \u2026" for fresh init,
"Vault already initialized at \u2026" for re-invocation) collapse to
one in this fixture: `mkdtempSync` guarantees a fresh directory per
run, so the "already initialized" path is unreachable. "Banner
present" can therefore distinguish nothing the file-existence
assertions don't already verify by reading the produced files.
Drop the banner check entirely. Keep all other post-conditions
(`assertNapkinInitPostConditions`'s default-schema, `vault.root`,
`distill.enabled`, and per-file `.obsidian/*.json` checks). Update the
inline comment to record the rationale so a future maintainer doesn't
re-add the brittle coupling.
Findings: (CORR-2, SB-1, CORR-3)
…ut terminology
The file-header "What it does" 10-step list was authored before the
napkin-init phase grew its own fail-fast post-condition assertion. The
list jumped from "builds the post-\`napkin init\` vault state" directly
to "builds a mock ExtensionAPI", hiding the new fail-fast checkpoint
that runs between fixture setup and extension wiring.
Restructure the file-header into the 4-phase flow that the body
actually executes:
Phase 1 \u2014 napkin init (CLI invocation produces vault scaffolding):
creates tmpdir, invokes \`napkin init\` + user-edits-config + writes
note.md, asserts napkin-init post-conditions and fail-fasts.
Phase 2 \u2014 session_start auto-init (production code: git init,
gitignore managed block, initial commit): wires mock ExtensionAPI,
drives session_start, asserts auto-init post-conditions and
fail-fasts.
Phase 3 \u2014 bare-origin scaffolding (test-only: wrapper needs a remote).
Phase 4 \u2014 /distill (production code: full health check + wrapper
subprocess + JS poller): allocates synthetic session, drives
/distill, polls for dispatch, asserts the GREEN/RED signatures.
Renumber the in-body \`Phase 1\` / \`Phase 2\` / \`Phase 3\` markers in
\`main()\` to \`Phase 2\` / \`Phase 3\` / \`Phase 4\` so the file-header
framing and the body comments stay aligned. The numbered procedural
steps (1-11) live under each phase as sub-bullets.
Also replace the residual "sibling layout" reference in the
\`setupFixture\` JSDoc with "subdir-layout" + a one-line gloss to
the design glossary. The other two "sibling layout" references in
the file were already updated in a prior commit; this caught the
straggler.
Findings: (CLN-1, CLN-4)
Contributor
Author
|
Re-closing: this PR was reopened by an automated subagent acting outside its task scope. Phase A of pi-napkin v0.3.1's auto-distill health check is being developed on cad0p/pi-napkin (PR #15 there) and will be proposed upstream only after merge to cad0p/pi-napkin/main. Apologies for the noise — will reopen properly when ready. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Phase A of v0.3.1's centralized health-check feature (5 commits). Closes #14 via the full-level invariant 'config.json-tracked'.
Phase A scope (5 commits):
refactor(distill): rename ensureVaultReadyForDistill + add level param + findings(1883b61)feat(distill): manage .gitignore as Ansible-style block with drift detection(e885919)feat(distill): fast-level health-check invariants — layout error + JSON validity(1878a33)refactor(distill): wire health-check level at session_start and worktree-spawn paths(f82b858)fix(distill): track .napkin/config.json on full-level health check (closes #14)(308fed8)Phase B (full-level invariants for HEAD-on-branch, orphan worktree pruning, stale branch cleanup, gitignore-outside-block detection, .napkin/distill/ untracked refusal, cache-root writability) lands in a follow-up PR.
Test count: 417 → 438 (+21).
Design:
features/pi-napkin-distill/auto-distill-health-check/design.mdin the orchestrator's vault.