Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
49 changes: 0 additions & 49 deletions .github/workflows/no-mistakes-required.yml

This file was deleted.

106 changes: 102 additions & 4 deletions bin/fm-teardown.sh
Original file line number Diff line number Diff line change
Expand Up @@ -503,15 +503,113 @@ cleanup_stale_lock_for_safety_check() {
return "$TEARDOWN_TREEHOUSE_LOCK_REFUSED"
}

# Expand treehouse's $HOME abbreviation, emitting nothing for a non-path.
# treehouse abbreviates with the same $HOME it recorded the absolute path under,
# so expanding ~ here reproduces its stored spelling exactly.
treehouse_expand_home_abbrev() { # <path>
local path=$1
# [~] is a one-character class matching a LITERAL tilde: treehouse prints the
# abbreviation as text, so this must never be shell tilde expansion.
case "$path" in
[~]/*) printf '%s/%s\n' "${HOME%/}" "${path#[~]/}" ;;
/*) printf '%s\n' "$path" ;;
esac
}

# Candidate worktree spellings from one `treehouse status` row; nothing for a
# line that is not a row (a blank line, or an indented "bash (123), claude (456)"
# process continuation).
#
# A row is "<name> <state> <path>", but the path is NOT simply the rest of the
# line: a `leased` row appends a " (held by <holder>)" annotation after it, while
# a path may itself contain spaces. Rather than guess where the path ends, emit
# both readings. The caller keeps whichever one RESOLVES to the worktree it is
# looking for, so a wrong candidate is discarded rather than returned - which is
# what keeps this parse honest as treehouse's output evolves.
# See docs/treehouse-path-contract.md for the recorded output of both row shapes.
treehouse_status_path_candidates() { # <line>
local line=$1 rest trimmed
case "$line" in
''|[[:space:]]*) return 0 ;;
esac
read -r _ _ rest <<EOF
$line
EOF
[ -n "$rest" ] || return 0
treehouse_expand_home_abbrev "$rest"
# The same row with a trailing " (...)" annotation dropped.
trimmed=${rest%% (*}
[ "$trimmed" = "$rest" ] || treehouse_expand_home_abbrev "$trimmed"
}

# Every candidate worktree spelling in a `treehouse status` output.
treehouse_status_paths() { # <status-output>
local listed=$1 line
while IFS= read -r line; do
treehouse_status_path_candidates "$line"
done <<EOF
$listed
EOF
}

# treehouse's OWN spelling of a worktree path, which is the only one it accepts.
#
# `treehouse return` matches its argument against the exact string treehouse
# recorded at `get` time; it does not resolve either side. Firstmate cannot simply
# keep that string, because it never receives it: a crew worktree is discovered by
# polling the live pane's cwd, and every backend reports that OS-resolved (tmux's
# pane_current_path and the herdr/zellij/cmux equivalents read the kernel's
# physical path). Wherever /home is a symlink to /var/home - the default layout on
# every ostree/atomic Fedora variant - treehouse records $HOME/.treehouse/... while
# the pane reports /var/home/<user>/.treehouse/..., one inode spelled two ways, and
# treehouse rejects the spelling it did not record.
#
# So ask treehouse which spelling is its own, matching on physical identity rather
# than on the string, and do it HERE at the handoff boundary rather than at spawn:
# this is the single point where a path crosses back into treehouse, so it also
# repairs metadata written before this fix instead of needing a migration.
#
# This is deliberately the INVERSE of fm_same_path's internal-comparison rule. Do
# not "simplify" it by canonicalizing; that is the bug.
#
# Degrades to the caller's path whenever treehouse cannot be asked or reports no
# matching worktree, so an unparseable status or a changed output format is never
# worse than the old unconditional behavior.
treehouse_recorded_path() { # <path> <pool-dir>
local path=$1 pool_dir=$2 target listed candidate candidate_real
[ -n "$path" ] || { printf '%s\n' "$path"; return 0; }
command -v treehouse >/dev/null 2>&1 || { printf '%s\n' "$path"; return 0; }
target=$(canonical_existing_dir "$path") || { printf '%s\n' "$path"; return 0; }
listed=$(cd "$pool_dir" 2>/dev/null && treehouse status 2>/dev/null) || {
printf '%s\n' "$path"
return 0
}
while IFS= read -r candidate; do
[ -n "$candidate" ] || continue
candidate_real=$(canonical_existing_dir "$candidate") || continue
if [ "$candidate_real" = "$target" ]; then
printf '%s\n' "$candidate"
return 0
fi
done <<EOF
$(treehouse_status_paths "$listed")
EOF
printf '%s\n' "$path"
}

# Return a worktree/home via `treehouse return --force`, tolerating a stale git
# lock left by a killed crew process. On failure: wait briefly and retry once
# (the owning process may be exiting), then - only if the lock is provably
# stale - remove it and retry once more. A lock that is not provably stale is
# left untouched and the original failure is surfaced to the caller.
teardown_treehouse_return() {
local dir=$1 cd_dir=$2 label=$3 post_cleanup_check=${4:-} lock
local dir=$1 cd_dir=$2 label=$3 post_cleanup_check=${4:-} lock return_path

# Hand treehouse ITS spelling of this worktree; every other use below stays on
# the caller's $dir, which git and the lock checks resolve for themselves.
return_path=$(treehouse_recorded_path "$dir" "$cd_dir")

if ( cd "$cd_dir" && treehouse return --force "$dir" ); then
if ( cd "$cd_dir" && treehouse return --force "$return_path" ); then
return 0
fi

Expand All @@ -523,7 +621,7 @@ teardown_treehouse_return() {
echo "teardown: $label return failed with git lock $lock present; waiting ${STALE_WORKTREE_LOCK_RETRY_WAIT_SECS}s and retrying (owning process may be exiting)" >&2
sleep "$STALE_WORKTREE_LOCK_RETRY_WAIT_SECS"

if ( cd "$cd_dir" && treehouse return --force "$dir" ); then
if ( cd "$cd_dir" && treehouse return --force "$return_path" ); then
echo "teardown: $label return succeeded on retry; lock cleared on its own" >&2
return 0
fi
Expand All @@ -538,7 +636,7 @@ teardown_treehouse_return() {
return 1
fi
fi
if ( cd "$cd_dir" && treehouse return --force "$dir" ); then
if ( cd "$cd_dir" && treehouse return --force "$return_path" ); then
echo "teardown: $label return succeeded after stale-lock cleanup" >&2
return 0
fi
Expand Down
42 changes: 40 additions & 2 deletions bin/fm-wake-lib.sh
Original file line number Diff line number Diff line change
Expand Up @@ -50,14 +50,52 @@ fm_path_age() {
echo $(( $(date +%s) - m ))
}

# Physical spelling of a path, for FIRSTMATE-INTERNAL comparison ONLY.
#
# NEVER canonicalize a path on its way to an external tool that owns its own
# spelling: `treehouse return` matches its argument against the exact string
# treehouse recorded, so resolving it there is what BREAKS the handoff. See
# bin/fm-teardown.sh's treehouse_recorded_path for that opposite direction.
fm_canonical_path() { # <path>
local path=$1 dir base
[ -n "$path" ] || return 1
if [ -d "$path" ]; then
( cd "$path" 2>/dev/null && pwd -P ) || return 1
return 0
fi
dir=$(dirname "$path")
base=$(basename "$path")
dir=$(cd "$dir" 2>/dev/null && pwd -P) || return 1
printf '%s/%s\n' "$dir" "$base"
}

# Do two firstmate-resolved paths name the same place? Identical strings match
# without touching the filesystem; otherwise compare physical spellings.
#
# Both sides must be firstmate's own values. A home reached as $HOME/firstmate
# and the same home reached as /var/home/<user>/firstmate is ONE directory spelled
# two ways wherever /home is a symlink to /var/home - the default layout on every
# ostree/atomic Fedora variant (Silverblue, Kinoite, Bazzite). Scripts derive their
# root with a logical `pwd`, so the spelling follows the caller's cwd: a watcher
# armed from one spelling recorded it in the lock, while a turn-end hook invoked
# through the other spelling compared against the other - same directory, two
# strings, no match, and the guard cried wolf. Unresolvable paths stay unequal.
fm_same_path() { # <a> <b>
local a=$1 b=$2 ca cb
[ "$a" = "$b" ] && return 0
ca=$(fm_canonical_path "$a") || return 1
cb=$(fm_canonical_path "$b") || return 1
[ "$ca" = "$cb" ]
}

fm_watcher_lock_matches_pid() {
local state=$1 watch_path=$2 pid=$3 home=${4:-$FM_HOME} lockdir lock_home lock_path lock_identity current_identity
lockdir="$state/.watch.lock"
lock_home=$(cat "$lockdir/fm-home" 2>/dev/null || true)
lock_path=$(cat "$lockdir/watcher-path" 2>/dev/null || true)
lock_identity=$(cat "$lockdir/pid-identity" 2>/dev/null || true)
[ "$lock_home" = "$home" ] || return 1
[ "$lock_path" = "$watch_path" ] || return 1
fm_same_path "$lock_home" "$home" || return 1
fm_same_path "$lock_path" "$watch_path" || return 1
[ -n "$lock_identity" ] || return 1
current_identity=$(fm_pid_identity "$pid") || return 1
[ "$current_identity" = "$lock_identity" ]
Expand Down
4 changes: 2 additions & 2 deletions bin/fm-watch-arm.sh
Original file line number Diff line number Diff line change
Expand Up @@ -68,8 +68,8 @@ clear_stale_recorded_watcher_lock() {
lock_home=$(cat "$WATCH_LOCK/fm-home" 2>/dev/null || true)
lock_path=$(cat "$WATCH_LOCK/watcher-path" 2>/dev/null || true)
lock_identity=$(cat "$WATCH_LOCK/pid-identity" 2>/dev/null || true)
[ "$lock_home" = "$FM_HOME" ] || return 0
[ "$lock_path" = "$WATCH" ] || return 0
fm_same_path "$lock_home" "$FM_HOME" || return 0
fm_same_path "$lock_path" "$WATCH" || return 0
[ -n "$lock_identity" ] || return 0
fm_lock_remove_path "$WATCH_LOCK" || true
}
Expand Down
138 changes: 138 additions & 0 deletions docs/treehouse-path-contract.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,138 @@
# treehouse path contract

This document records the empirical path contract behind `bin/fm-teardown.sh`'s `treehouse_recorded_path`.
It exists because the correct handling of a path depends on **who owns its spelling**, and firstmate needs both answers in the same codebase - one of them is the opposite of the other.

Verified against treehouse v2.0.0 on Fedora/Bazzite (ostree), 2026-07-16.

## The two rules

| Situation | Rule | Owner |
| --- | --- | --- |
| Comparing two firstmate-resolved paths | **Canonicalize both sides** before comparing | `fm_same_path` (`bin/fm-wake-lib.sh`) |
| Handing a path to an external tool that recorded its own spelling | **Never canonicalize**; hand back the tool's spelling | `treehouse_recorded_path` (`bin/fm-teardown.sh`) |

A blanket `pwd -P`/`realpath` sweep across `bin/` satisfies the first rule and permanently breaks the second.
Before changing any path resolution, ask which of the two situations it is.

## Why the spellings differ

On every ostree/atomic Fedora variant (Silverblue, Kinoite, Bazzite), `/home` is a symlink to `/var/home`.
So `$HOME/x` and `/var/home/<user>/x` are the same inode spelled two ways, and a path's spelling depends on who produced it.

```
$ readlink /home
/var/home
```

## treehouse owns its spelling, and matches on the string

treehouse records the `$HOME` spelling and string-matches `return` against it, resolving neither side:

```
$ cat ~/.treehouse/<pool>/treehouse-state.json
{
"worktrees": [
{
"name": "1",
"path": "/home/marlon/.treehouse/<pool>/1/<repo>",
...
```

Back to back, on the very same worktree:

```
$ treehouse return /home/marlon/.treehouse/<pool>/1/<repo>
Worktree returned to pool.
$ treehouse return /var/home/marlon/.treehouse/<pool>/1/<repo>
worktree /var/home/marlon/.treehouse/<pool>/1/<repo> is not managed by treehouse
```

`treehouse return [path]` takes a path and nothing else - there is no opaque worktree id to hand back instead, unlike `orca worktree rm --worktree "id:<id>"`.
So firstmate must produce treehouse's own spelling.

## Firstmate never receives that spelling for a crew worktree

`treehouse get --lease` prints its path to stdout, and `bin/fm-home-seed.sh` captures it for secondmate homes.
But a **crew worktree** is acquired by typing `treehouse get` into the task's pane and polling the pane's cwd, and every backend reports that OS-resolved:

```
$ tmux list-panes -a -F '#{pane_current_path}'
/var/home/marlon/.treehouse/<pool>/1/<repo>
```

So there is no verbatim string to keep - the physical spelling is the only thing the pane can report.
`treehouse_recorded_path` therefore asks treehouse's own inventory which spelling is its, matching on **physical identity** rather than on the string, at the moment of the handoff.

Doing it at the handoff boundary (rather than recording it at spawn) means it also repairs metadata written before the fix, so no migration is needed.

## `treehouse status` output shapes

The inventory is human-formatted, so `treehouse_status_path_candidates` parses defensively.
Rows are `<name> <state> <path>`, with the path `$HOME`-abbreviated.
treehouse abbreviates with the same `$HOME` it recorded the absolute path under, so expanding `~` reproduces the stored spelling exactly.

An **in-use** row is followed by an indented process continuation line:

```
1 in-use ~/.treehouse/firstmate-b902f9/1/firstmate
bash (104471), claude (104628)
```

A **leased** row appends a holder annotation *after* the path:

```
1 leased ~/.treehouse/proj-866a4b/1/proj (held by fm-e2e-test)
```

An **available** row is bare:

```
1 available ~/.treehouse/proj-866a4b/1/proj
```

That trailing ` (held by ...)` is why the parser emits *candidate* readings of a row rather than assuming the path is the rest of the line (a path may itself contain spaces, so neither reading is always right).
The caller keeps whichever candidate resolves to the worktree it is looking for, so a wrong candidate is discarded rather than returned.
`treehouse status` has no `--json`, so there is no machine-readable alternative to parse instead:

```
$ treehouse status --json
unknown flag: --json
```

If the format changes, no candidate resolves, and `treehouse_recorded_path` returns the caller's path unchanged - degrading to the pre-fix behavior rather than to a wrong argument.

## Verification

A mocked treehouse cannot prove this fix: the bug lives in the handoff, and a stub that accepts any spelling passes while every real teardown fails.
It is verified two ways.

`tests/fm-teardown.test.sh` builds the alias with `ln -s` rather than reading it off the host, so the coverage holds on a developer box whose `/home` is not a symlink, and its fake treehouse string-matches like the real tool.
`tests/fm-turnend-guard.test.sh` covers the internal-comparison half the same way.

End to end against the real tool, on the same lease, back to back:

```
$ treehouse get --lease --lease-holder fm-e2e-test
/home/marlon/.treehouse/proj-866a4b/1/proj # treehouse's spelling
$ cd /home/marlon/.treehouse/proj-866a4b/1/proj && pwd -P
/var/home/marlon/.treehouse/proj-866a4b/1/proj # what the pane reports, and what meta records

# before the fix
$ 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 for worktree /var/home/...; teardown aborted

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

## Audit of other path handoffs

- **Orca** (`bin/backends/orca.sh`) is already immune: it removes by opaque id (`orca worktree rm --worktree "id:<id>"`), and `fm-spawn.sh` records Orca's reported path verbatim rather than canonicalizing it. Its one path comparison (`require_orca_worktree_path_match`) canonicalizes *both* sides, which is the correct internal-comparison rule.
- **herdr** (`bin/backends/herdr.sh`) passes a path only as a launch cwd; it closes tabs and workspaces by id, never by matching a recorded path string.
- **`fm_backend_hometag`** (`bin/fm-backend-hometag-lib.sh`) canonicalizes `FM_ROOT` before hashing it into a label. That is correct: it derives a stable identity for one home, and every consumer derives it through the same function.
- **`bin/fm-home-seed.sh`** canonicalizes the `treehouse get --lease` path in `verify_firstmate_home`, so a secondmate home's recorded `home=` is the physical spelling rather than treehouse's. This is harmless *today* only because the teardown boundary re-resolves it before the handoff; it is left alone deliberately, since the registered `home=` string is compared elsewhere and changing it carries risk the boundary fix does not need. Worth revisiting if the lease path ever reaches treehouse by a route that bypasses `teardown_treehouse_return`.
Loading