Skip to content

fix(paths): respect treehouse's path spelling, canonicalize our own - #5

Merged
Marl0nL merged 3 commits into
mainfrom
fm/fm-treehouse-path-x2
Jul 16, 2026
Merged

Marl0nL merged 3 commits into
mainfrom
fm/fm-treehouse-path-x2

Conversation

@Marl0nL

@Marl0nL Marl0nL commented Jul 16, 2026

Copy link
Copy Markdown
Owner

Two path-aliasing bugs with one root cause and opposite fixes.

Where /home is a symlink to /var/home (the default on every ostree/atomic Fedora variant - Bazzite, Silverblue, Kinoite), $HOME/x and /var/home/<user>/x are one inode spelled two ways. Whether to canonicalize depends entirely on who owns the spelling:

Situation Rule
Comparing two firstmate-resolved paths Canonicalize both sides (fm_same_path)
Handing a path to a tool that recorded its own spelling Never canonicalize (treehouse_recorded_path)

A blanket pwd -P sweep across bin/ fixes bug 2 and permanently entrenches bug 1.

Bug 1: every teardown failed (external contract)

treehouse return string-matches its argument against the spelling treehouse recorded, resolving neither side, so teardown failed with is not managed by treehouse on every aliased box. Firstmate was working around it by hand: sed -i 's|^worktree=/var/home/|worktree=/home/|' before each teardown.

Firstmate never receives treehouse's spelling for a crew worktree - the worktree is discovered by polling the pane's cwd, and every backend reports that OS-resolved (pane_current_path and friends read the kernel's physical path). There is no verbatim string to keep, so treehouse_recorded_path asks treehouse's own inventory which spelling is its, matching on physical identity rather than on the string.

It resolves at the handoff boundary (teardown_treehouse_return, the one function that calls treehouse return) rather than at spawn. That covers all three call sites, and repairs metas written before this fix without a migration. No /var/home -> /home rewrite is encoded anywhere. If treehouse cannot be asked or reports no match, the caller's path passes through unchanged - never worse than before.

Bug 2: turn-end guard cried wolf (internal comparison)

fm_watcher_lock_matches_pid string-compared two of firstmate's own values. Every script derives its root with a logical pwd, so the spelling follows the arming cwd: a watcher armed from one spelling recorded it in the lock, while Claude's Stop hook resolved the other - same directory, two strings, and the guard blocked turn ends at a watcher that was alive, holding the lock, and beating 1s earlier.

Both sides are ours, so fm_same_path canonicalizes both before comparing. The arming cwd is now irrelevant. Same fix in fm-watch-arm.sh, which declined to clear its own stale lock for the same reason. The matcher stays strict - a genuinely different home still fails (covered by a test).

Audit of both classes

  • Orca - already correct: removes by opaque id (--worktree "id:<id>"), records its provider's path verbatim, and its one path comparison canonicalizes both sides. No change.
  • herdr - passes a path only as a launch cwd; closes tabs/workspaces by id. No change.
  • fm_backend_hometag - canonicalizes FM_ROOT before hashing into a label. Correct (stable identity for one home). No change.
  • fm-home-seed.sh - canonicalizes the treehouse get --lease path, so a secondmate's home= is physical rather than treehouse's. Harmless today because the teardown boundary re-resolves it; left alone deliberately (the registered string is compared elsewhere). Reported in the doc.
  • Swept bin/ for raw path comparisons: the only other hits are GIT_DIR/GIT_COMMON_DIR (one git invocation, consistently spelled) and a dirname idiom. Both fine.

Verification

A test on a non-symlinked home passes while both bugs survive - that is how this shipped. So:

  • Tests build the alias with ln -s rather than reading it off the host, so coverage holds on any developer's box.
  • The teardown fake string-matches like the real tool. The previous mock was exit 0, which accepts any spelling - exactly the blind spot.
  • Bug 1 verified end to end against real treehouse v2.0.0, on the same lease, back to back:
# meta records /var/home/...; treehouse recorded /home/marlon/...
# before
$ bin/fm-teardown.sh e2e-x1 --force
worktree /var/home/marlon/.treehouse/proj-866a4b/1/proj is not managed by treehouse
error: treehouse return failed ...; teardown aborted        # exit 1

# after, no hand-patched meta
$ bin/fm-teardown.sh e2e-x1 --force
Worktree returned to pool.                                  # exit 0
$ treehouse status
1     available    ~/.treehouse/proj-866a4b/1/proj

The real run earned its keep: it caught that a leased row trails a (held by <holder>) annotation after the path, which my mock-only parser read as part of the path. It silently fell back and the return still failed. That shape is now a regression case (verified to fail without the fix), and both row shapes are recorded in docs/treehouse-path-contract.md.

  • Bug 2 verified by arming from both spellings and confirming silence either way. Each new test was confirmed to fail without its fix, with the exact reported symptom.

Notes

  • tests/fm-backend.test.sh's old-vs-new conformance diff now compares the mutating command sequence: the new treehouse status read is a deliberate read-only probe, outside that test's backend-refactor contract. The mutating sequence still must match exactly.
  • shellcheck clean; 62/63 suites pass. tests/fm-session-start.test.sh fails on the pristine base commit too (pi supervision block missing) - pre-existing and unrelated.

Two path-aliasing bugs with one root cause and opposite fixes. Where /home
is a symlink to /var/home (the default on every ostree/atomic Fedora
variant), one directory has two spellings, and whether to canonicalize
depends on who owns the spelling.

teardown: `treehouse return` string-matches its argument against the
spelling treehouse recorded, so every teardown failed on such a box with
"is not managed by treehouse" - firstmate was hand-patching the meta with
sed before each one. Firstmate never receives that spelling for a crew
worktree: the worktree is found by polling the pane's cwd, which every
backend reports OS-resolved. So ask treehouse's own inventory which
spelling is its, matching on physical identity, at the handoff boundary -
the single point where a path crosses back into treehouse, which also
repairs metas written before this fix. Degrades to the caller's path when
treehouse cannot be asked, so a changed output format is never worse than
before.

turn-end guard: fm_watcher_lock_matches_pid string-compared two of
firstmate's OWN values, so a watcher armed from one spelling and a hook
invoked through the other failed to match and the guard blocked turn ends
at a live, beating watcher. Both sides are ours, so canonicalize both
before comparing (fm_same_path). Same fix in fm-watch-arm.sh, which
declined to clear its own stale lock for the same reason. The matcher
stays strict: a genuinely different home still fails.

Audited both classes. Orca already removes by opaque id and records its
provider's path verbatim; herdr closes by id; fm_backend_hometag
canonicalizes before hashing a home identity, which is correct. Recorded
in docs/treehouse-path-contract.md, including the leased-row
"(held by ...)" annotation a real run caught and a mock would not.

Verified end to end against real treehouse v2.0.0, not only on mocks:
pre-fix reproduces the exact error on a real lease, post-fix returns the
worktree and `treehouse status` reports it available. Tests build the
alias with ln -s rather than reading it off the host, so they hold on a
box whose /home is not a symlink.
@Marl0nL Marl0nL closed this Jul 16, 2026
@Marl0nL Marl0nL reopened this Jul 16, 2026
Marlon added 2 commits July 16, 2026 16:45
Empty commit to fire the pull_request synchronize event now that GitHub
Actions is enabled on the fork. No code changes.
DELIBERATE FORK DIVERGENCE FROM UPSTREAM POLICY - NOT A MISTAKE.
NEVER CONTRIBUTE THIS COMMIT UPSTREAM.

Upstream (kunchenguid/firstmate) requires every PR to be raised through
`git push no-mistakes`, and enforces it with this workflow. That policy is
correct for upstream and must stay there.

This fork does not run no-mistakes validation, so the gate fails on every
PR while checking nothing about the code: its only step greps the PR body
for the literal marker

    Updates from [git push no-mistakes](...)

and fails when absent. It asserts nothing about correctness, tests, or
lint - the real signal is the CI workflow, which is untouched and stays
required.

Authorised by the captain for this fork only.
@Marl0nL
Marl0nL merged commit 8107742 into main Jul 16, 2026
3 checks passed
@Marl0nL
Marl0nL deleted the fm/fm-treehouse-path-x2 branch July 16, 2026 07:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant