gabriel / muse public
issue-104-empty-dir-sentinel-roadmap.md markdown
216 lines 10.6 KB
Raw
sha256:09d4b6592647086ae8fe7356c0369b867aba50c718c3ae0b9622088c0490db3a docs: add local source-of-truth for muse#104 -- empty-dir-s… Sonnet 5 11 hours ago

Empty-directory sentinel: one root design flaw, four discovered symptoms — comprehensive fix roadmap

This is a parent/roadmap ticket. It supersedes ad hoc, one-symptom-at-a-time patching. Do not close this until every phase below is checked off and every linked child ticket is closed with a comment naming the exact commit that resolved it.

Linked tickets

# Symptom Status
(no number — fixed inline, no ticket filed) muse code add clobbers a file's stage entry when it replaces a tracked empty directory of the same path Fixed, shipped 0.2.1rc8
#101 Historical commits double-record a path in both manifest and directories, causing false "deleted" status Open
#102 A genuinely empty (0-byte) file is silently untrackable — misclassified as a directory sentinel Open
#103 No command can untrack a committed directory sentinel whose physical directory still exists on disk Open

Problem statement — the one root flaw behind all four

muse/plugins/code/stage.py:

EMPTY_DIR_OID: str = blob_id(b"")

The value used to mark "this entry represents a tracked empty directory, not a file" is the SHA-256 content hash of zero bytes. But the content hash of a genuinely empty file is the exact same value — blob_id() is a pure function of content; it has no way to know or care whether the zero bytes represent "a file with no content" or "a marker for an empty directory." Every place in the codebase that distinguishes "directory sentinel" from "file" does so by checking object_id == EMPTY_DIR_OID — there is no independent discriminator.

This is a regression, not an original design flaw

muse code symbol-log "muse/plugins/code/stage.py::EMPTY_DIR_OID" --json

surfaces the exact commit that introduced it:

refactor: directories are proper content-addressed objects (EMPTY_DIR_OID)

Replace the magic sentinel string 'dir:' with a real content hash: EMPTY_DIR_OID = blob_id(b'')

Before this refactor, the sentinel was the literal string "dir:" — a value that could never collide with a real blob_id() output (every real object ID is sha256:<64-hex>; "dir:" matches no such format). The refactor's goal was good — make tracked empty directories real, pushable, content-addressed objects instead of a magic string special case — but swapping to a real content hash reintroduced exactly the ambiguity the old string sentinel had structurally ruled out. _DIR_SENTINEL_LEGACY = "dir:" still exists in stage.py today, migrated transparently on read — proof the old, safe design is a known quantity, not something to invent from scratch.

Why patching each symptom individually doesn't work

The two fixes already shipped (the code_stage.py clobber-avoidance check, and the defensive - set(staged_manifest) subtraction in plugin.py::snapshot()) are real and correct, but they only prevent new corruption from one call path (muse code addmuse commit). They do not address:

  • Whatever other code path originally produced the ghost directory entries found in two real repos (~/thoughts, ~/ecosystem/presentations/build-with-muse) — neither repo's corruption was freshly introduced by code add; it was already present in history before this session started investigating. The actual introducing commit/command has never been identified.
  • #102 (empty files) at all — untouched by either shipped fix.
  • #103 (stuck sentinel, directory still on disk) at all — untouched.

Whack-a-mole on individual symptoms will keep surfacing new instances of the same root flaw in yet another call site. The fix has to be architectural: eliminate the ambiguity at its source, then verify every consumer agrees.

Blast radius — every place trusting the ambiguous discriminator

grep -rn "== _DIR_SENTINEL\|== EMPTY_DIR_OID\|!= EMPTY_DIR_OID\|!= _DIR_SENTINEL" muse/ --include="*.py"

~19 call sites across 6 files treat object_id == EMPTY_DIR_OID as "this is a directory, not a file":

  • muse/plugins/code/plugin.py (7 sites)
  • muse/cli/commands/code_stage.py (6 sites)
  • muse/cli/commands/status.py (4 sites)
  • muse/cli/commands/mv.py (2 sites)
  • muse/cli/commands/diff.py (1 site)
  • muse/plugins/code/stage.py (1 site, legacy migration)

Separately, 8 files assign or construct directories= values as part of commit/merge/rebase/cherry-pick/status/diff/pull — any of these is a candidate for being the actual still-unidentified source of #101's historical corruption, independent of the staging path above:

  • muse/core/merge_engine.py
  • muse/core/rebase.py
  • muse/cli/commands/cherry_pick.py
  • muse/cli/commands/commit.py
  • muse/cli/commands/diff.py
  • muse/cli/commands/merge.py
  • muse/cli/commands/pull.py
  • muse/cli/commands/rebase.py
  • muse/cli/commands/status.py

Design decision required (Phase 2) — not yet made, needs a call

Two viable directions, evaluated here rather than picked unilaterally:

Option A — revert to a non-colliding sentinel value. Replace EMPTY_DIR_OID = blob_id(b"") with a fresh value guaranteed to never equal a real content hash (e.g. reintroduce something shaped like the old "dir:", or a clearly-out-of-format marker string). Minimal change, but gives up the original refactor's actual goal (directories as real, pushable content-addressed objects) — pushing/fetching an object literally named "dir:" doesn't fit the object-store model the way a real sha256: object does.

Option B — add an explicit discriminator field. Give StagedEntry (and wherever a committed snapshot needs the same distinction) an independent kind: "file" | "dir" field, bump the stage schema version, and keep EMPTY_DIR_OID as the real object ID a tracked empty directory points at — a directory entry can legitimately have object_id == EMPTY_DIR_OID and kind == "dir"; a file with no content has object_id == EMPTY_DIR_OID and kind == "file". No collision, no ambiguity, directories stay real pushable objects.

Recommendation: Option B. It's strictly more correct — "distinguish by an explicit field" beats "distinguish by picking a value that hopefully never collides in practice," which is the exact failure mode already in production. It also preserves the original refactor's stated goal instead of reverting it. This needs sign-off before Phase 3 starts implementing across 19+ call sites.

Multi-phase TDD roadmap

  • [ ] Phase 1 — Consolidated reproduction suite (RED)
    • [ ] New test file, e.g. tests/test_empty_dir_sentinel_invariants.py, with one test class per known symptom: - dir→file transition (already fixed — reference the existing TestDirToFileTransitionStageClobber in test_cmd_code_add.py as a locked-in regression guard, don't duplicate it) - empty file silently dropped from manifest (#102 repro) - committed dir sentinel, directory still on disk, cannot be untracked by any command (#103 repro) - general invariant: no path is ever simultaneously present in a commit's manifest and directories
    • [ ] Bisection task: identify the actual code path (merge? rebase? cherry-pick? something else?) that produced the historical ghost-directory entries in ~/thoughts and ~/ecosystem/presentations/build-with-muse. Without this, #101's regression test only covers a guessed mechanism, not the real one.
  • [ ] Phase 2 — Design decision
    • [ ] Confirm Option A vs. B (or a third option) with gabriel.
    • [ ] Write the chosen schema/representation change up explicitly (exact field names, exact migration behavior for existing stage.json files written under the old schema) before touching implementation.
  • [ ] Phase 3 — Implement
    • [ ] Update all ~19 object_id-equality call sites (list above) to use the new discriminator.
    • [ ] Update all 8 directories=-assigning call sites as needed once Phase 1's bisection identifies which of them (if any) are actually implicated in #101.
    • [ ] Stage schema version bump + read-time migration for pre-existing stage.json files with no discriminator field.
  • [ ] Phase 4 — Green the suite
    • [ ] Every Phase 1 reproduction passes.
    • [ ] Full existing regression suite passes, zero regressions (test_cmd_code_add, test_code_stage*, test_directories_feature, test_cmd_commit, test_cmd_status, merge/rebase/cherry-pick suites for whichever files Phase 1's bisection implicates).
    • [ ] Final grep audit: zero remaining object_id == EMPTY_DIR_OID (or equivalent) comparisons used as a file/directory discriminator anywhere in the codebase.
  • [ ] Phase 5 — Real-world verification
    • [ ] Reproduce and confirm-fixed against muse-dev in fresh scratch repos for all four symptoms.
    • [ ] Confirm ~/thoughts's repo (the original real-world trigger) has zero residual ghost-directory noise.
    • [ ] Confirm ~/ecosystem/presentations/build-with-muse's repo (the other real-world instance found) is clean.
    • [ ] Confirm ~/ecosystem/musehub's .vscode directory-sentinel residue (found while fixing the .claude/.vscode ignore patterns) is finally untrackable and resolved.
  • [ ] Phase 6 — Ship
    • [ ] Version bump, same pipeline as every other fix this session (build, publish to staging, smoke test, install, verify against the real CLI — not just muse-dev).
    • [ ] Push dev/main to local/staging/production for muse.
    • [ ] Close #101, #102, #103, each with a comment naming the exact commit that resolved it.

Definition of done — the actual acceptance gate

This ticket does not close until all of the following are true simultaneously, not just "most":

  1. Every reproduction test written in Phase 1 passes.
  2. The full existing test suite passes with zero regressions.
  3. #101, #102, and #103 are all closed, each pointing at the specific commit that resolved it.
  4. Both real-world repos that surfaced this (~/thoughts, ~/ecosystem/presentations/build-with-muse) and the one that surfaced #103 (~/ecosystem/musehub) report a clean muse status with zero manual object-store surgery.
  5. A final grep -rn "== _DIR_SENTINEL\|== EMPTY_DIR_OID" across the codebase shows only the new, unambiguous discriminator checks — no remaining bare object-id-equality-as-file-vs-directory-test anywhere.
File History 1 commit
sha256:09d4b6592647086ae8fe7356c0369b867aba50c718c3ae0b9622088c0490db3a docs: add local source-of-truth for muse#104 -- empty-dir-s… Sonnet 5 11 hours ago