chore(e2e): stop and reap leaked isolated Obsidian E2E instances#197
Merged
Conversation
The per-worktree isolation wrapper starts a private-HOME Obsidian instance per worktree but nothing stopped it, so a removed/merged worktree leaked an Obsidian process tree and a /private/tmp/podnotes-obsidian-e2e/<vault> dir. Add scripts/stop-obsidian-e2e-instance.mjs + a stop:e2e-obsidian npm script that identify this worktree's instance by its private --user-data-dir token, terminate that process tree (SIGTERM then SIGKILL), and remove its profile dir. It never touches the shared dev vault, other worktrees, or quickadd instances. Two layers prevent leaks: - orca.yaml scripts.archive hook runs the teardown on `orca worktree rm --run-hooks`. - start:e2e-obsidian and obsidian:e2e reap orphaned instances (backing vault gone) on next start, so leaks are cleaned even without the hook. Document the workflow in AGENTS.md and cover the engine (process-tree matching across /tmp vs /private/tmp, safe-path guard, orphan detection, reaping) with unit tests.
Review-driven hardening of the E2E instance teardown: - Seed the kill set only on the Obsidian `--user-data-dir=<instance>/` flag instead of a bare path substring, so an unrelated process that merely mentions the instance path (a log tail, an editor) is never reaped. - Detect orphans by "the worktree is gone" via a per-instance marker (podnotes-e2e-instance.json) instead of "the vault leaf is missing", so a momentarily-unmounted vault no longer triggers a destructive reap; fall back to the vault-path check for pre-marker instances. - Tolerate EPERM/ENOENT (not just ESRCH) when signalling, so a recycled foreign pid never aborts teardown before the profile dir is removed. - Isolate each orphan teardown in reap so one failure does not abort the scan. - Add a profile-root containment guard before removing a dir. - Guard the orca archive hook with `command -v node` and document the custom --vault / --profile-root limitations. Extends the unit tests to cover each case.
Deploying podnotes with
|
| Latest commit: |
62e8852
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://12fee50f.podnotes.pages.dev |
| Branch Preview URL: | https://chhoumann-devx-e2e-vault-tea.podnotes.pages.dev |
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.
Summary
The per-worktree Obsidian E2E isolation wrapper (PR #188) starts a private-
HOMEObsidian instance per worktree (profile dir under/private/tmp/podnotes-obsidian-e2e/<vault>-<hash>/) but nothing stopped it. When a worktree was removed on merge, every completed worker leaked an Obsidian process tree and a/private/tmpvault dir. This adds a reliable teardown.This is DevX tooling only — no plugin runtime / user-facing behavior changes.
What's added
scripts/stop-obsidian-e2e-instance.mjs+npm run stop:e2e-obsidian— identifies this worktree's instance by its private--user-data-dir=<instance>/token (handles the macOS/tmp↔/private/tmpfirmlink so the whole Electron tree is matched), terminates that process tree (SIGTERM, then SIGKILL for stragglers), and removes its profile dir behind a safe-path + profile-root containment guard. Supports--dry-run,--json,--prune.orca.yaml) —scripts.archiveruns the teardown onorca worktree rm --run-hooks(cwd = worktree,$ORCA_WORKTREE_PATH), guarded withcommand -v node. A failed hook is logged by Orca but never blocks removal.start:e2e-obsidianandobsidian:e2ereap orphaned instances (those whose worktree is gone, recorded via a newpodnotes-e2e-instance.jsonmarker) before launching, so leaks are cleaned even when a worktree is removed without--run-hooks. An idle instance for a worktree that still exists is left alone so concurrent workers reuse it.scripts/stop-obsidian-e2e-instance.test.ts(23 cases): process-tree matching across/tmpvs/private/tmp, flag-scoped seeding, descendant walk, SIGTERM→SIGKILL, safe-path/containment guards, marker-based orphan detection, and reap isolation.Safety invariants (held)
The teardown only ever targets the current worktree's instance. The shared
devvault (real$HOME, no/tmptoken), other worktrees (distinct per-worktree hash), and all quickadd instances (/tmp/quickadd-obsidian-e2e/...) are never touched.Runtime verification (real Obsidian, this worktree)
verifiedPodNotes: true; the marker recorded the worktree path.stop --dry-runlisted exactly the instance's 4 pids and removed nothing.stopterminated exactly those pids and removed the profile dir, while a live sibling worktree instance, 3 quickadd instances, and the shareddevvault all stayed alive and the sibling's dir was intact.Review
Ran a 3-reviewer pass (1 comprehensive + 2 adversarial), each finding independently verified against the code: 13 raised, 10 confirmed, 0 must-fix. Addressed the high-value confirmed items in the second commit (flag-scoped matcher, worktree-marker orphan signal, EPERM tolerance, per-orphan isolation, containment guard, hook
nodeguard).Commands run (Node 22)
npm run lint·npm run format:check·npm run typecheck·npm run build·npm run test(453 passing).docs:buildskipped (nodocs/changes; mkdocs not installed locally).Release / migration impact
None. Tooling/scripts only; no plugin code, settings, storage, API, or URI behavior changes.
Note on hook activation
Orca reads
orca.yamlfrom the registered repo root (/Users/christian/Developer/PodNotes), so the archive hook becomes active for future worktree removals once this merges tomaster. Thestop:e2e-obsidianscript and reap-on-start net work immediately regardless.