Skip to content

fix(herdr): make the presentation lock namespace per account - #2730

Open
YogiGit wants to merge 12 commits into
kunchenguid:mainfrom
YogiGit:fm/fm-herdr-presentation-lock-shared-tmp-k2
Open

fix(herdr): make the presentation lock namespace per account#2730
YogiGit wants to merge 12 commits into
kunchenguid:mainfrom
YogiGit:fm/fm-herdr-presentation-lock-shared-tmp-k2

Conversation

@YogiGit

@YogiGit YogiGit commented Aug 21, 2026

Copy link
Copy Markdown

Intent

The developer was fixing a defect in firstmate's Herdr backend where fm_backend_herdr_presentation_lock_namespace() in bin/backends/herdr.sh returned the fixed path /tmp/firstmate-herdr-presentation, which a second macOS account could create first at mode 700, permanently blocking every teardown in this home with no override, no fallback, and no remedy short of a privileged rmdir. They wanted the namespace made per-uid so a cross-account collision is impossible by construction rather than by check, the ownership and mode assertions kept but applied to a name no other account can claim, and the diagnostics separated so that "session unresolvable" is distinguished from "namespace owned by another uid and therefore permanently unusable", with the latter naming the owning uid and the actual remedy, because the old message sent supervisors into an infinite retry against a condition that could never clear and held cleanup open long enough that a teardown once ran against a reassigned worktree. Stated constraints: bin/.sh and bin/backends/.sh must pass shellcheck via bin/fm-lint.sh as the single owner of the lint definition, tests must be colocated in tests/ extending existing scripts and must exercise behaviour through an executable interface rather than asserting source bytes, other fixed shared /tmp paths in bin/ and other runtime backends must be surveyed and reported rather than folded into this task, and tracked Markdown must use one full sentence per line with plain dashes. During the no-mistakes validation run the developer accepted four gate decisions: narrow an overstated documentation claim about collision impossibility, route the two additional bin/fm-spawn.sh degradation sites through the same fault helper, delete a vacuous test case arm, and file the uncreatable-namespace case as a separate task rather than fixing it here. On test coverage they rejected building a portable non-gated spawn fixture as more machinery than the one-line change warranted, and after their proposed middle path was checked and found to drive lock contention rather than a faulted namespace, they directed adding one negative assertion per gated site asserting that no fault fragment appears under contention. They also required the handover to state the coverage shape without softening, naming that positive fault-fragment coverage at those two spawn sites is absent and tracked as fm-herdr-spawn-fault-fragment-coverage-k3, and to attribute the gate-repo git identity fix to firstmate rather than the captain.

What Changed

  • fm_backend_herdr_presentation_lock_namespace in bin/backends/herdr.sh now returns /tmp/firstmate-herdr-presentation-<uid> instead of the fixed shared path, so a second local account can no longer claim the namespace first and permanently block every teardown in this home. The uid is read back from the owner of a directory the process creates under /tmp itself and memoized for the process, so neither an id on PATH nor an inherited environment variable can move one process of an account onto a different namespace than another. The existing ownership and mode-700 assertions are kept and still catch a deliberate claim of the per-uid name.
  • Added fm_backend_herdr_presentation_lock_namespace_fault and fm_backend_herdr_presentation_lock_refusal_suffix, which separate a retryable "session unresolvable" refusal from a permanent namespace fault (foreign owner, symlink, non-directory, wrong mode, unreadable ownership, uncreatable, or an untrustworthy account id) and print the offending path, the owning uid, and the remedy that actually clears it. The suffix is appended at the refusal sites in bin/fm-teardown.sh, bin/fm-spawn.sh (both degradation paths), bin/fm-herdr-session-cleanup.sh, and fm_backend_herdr_kill; refusals reached after the lock was already acquired deliberately do not consult it. The teardown refusal now states that the task and its work were not touched, since consulting the diagnostic may itself create the namespace directory.
  • Extended tests/fm-backend-herdr.test.sh (8 cases), tests/fm-teardown.test.sh (5 cases), and tests/lib.sh with a shared fake-uid helper, all driving the behaviour through the executable interface; tests/fm-backend-herdr-presentation-e2e.test.sh gained two negative assertions that no namespace fault fragment appears under ordinary lock contention. Documented the per-account namespace, the untrustworthy-id case, and the cross-revision rollout window in docs/herdr-backend.md, and recorded the fixture run in docs/verification/runtime-backends.md. Positive fault-fragment coverage at the two bin/fm-spawn.sh sites remains absent and is tracked separately as fm-herdr-spawn-fault-fragment-coverage-k3; the gated presentation E2E lane fails in this environment identically on the base commit, so it stays owned by the remote Herdr CI lane.

Risk Assessment

✅ Low: Every accepted finding from the prior round is verifiably resolved at the source level, the one behavioural change (resolve-once in the fault helper) narrows a diagnostic path and is covered by a non-vacuous regression test, and the remaining work is test-hygiene and documentation with no change to the lock's acquisition semantics.

Testing

I ran the two targeted non-gated suites that own this behaviour (tests/fm-backend-herdr.test.sh and tests/fm-teardown.test.sh) and both passed with the 13 new namespace cases included, then went past unit-level pass/fail and drove the real bin/fm-teardown.sh CLI end to end against a staged cross-account collision, comparing the base commit and the target commit under identical machine state. The transcript shows the defect and the fix as an operator sees them: on base, a shared /tmp/firstmate-herdr-presentation reporting a foreign owner makes teardown exit 1 with "nothing was changed - rerun teardown once the session is reachable and unambiguous" while the task stays recorded in flight, which is the unbounded retry the intent describes; on target, the same state resolves /tmp/firstmate-herdr-presentation-501 instead, teardown completes, the pane closes under its lock, and the meta is retired. I also captured the two diagnostics staying apart - a genuinely foreign-owned per-uid namespace names the owning uid, says no later attempt will clear it, and gives the remove-the-directory remedy, while an unresolvable session still yields only the retryable wording with no namespace fragment. The real-Herdr-gated presentation e2e could not contribute evidence here: it fails at "treehouse get did not enter a worktree within 60s" before reaching any lock code, and reproduces identically from a clean base-commit checkout, so I verified it is pre-existing rather than caused by this change and left that lane to remote CI. Worktree is clean and all transient /tmp staging was removed.

Evidence: fm-teardown CLI transcript: base vs target under a cross-account lock namespace collision

SCENARIO 1 - another macOS account claimed the shared lock namespace first /tmp/firstmate-herdr-presentation reports owner uid 947161; this account is uid 501. --- BEFORE: base revision 1cb900c --- $ bin/fm-teardown.sh task-x1 --force error: herdr session presentation lock could not be resolved for task-x1; nothing was changed - rerun teardown once the session is reachable and unambiguous [exit status: 1] [herdr pane closed under the lock: no] [task still recorded in flight (state/task-x1.meta present): yes] --- AFTER: target revision, identical machine state --- $ bin/fm-teardown.sh task-x1 --force teardown task-x1 complete (window default:wG:pQ, worktree .../herdr-legacy-claimed-after/wt) [exit status: 0] [herdr pane closed under the lock: yes] [task still recorded in flight (state/task-x1.meta present): no] [lock namespace this account actually resolved:] /tmp/firstmate-herdr-presentation-501 SCENARIO 2 - target revision, this account's OWN per-uid namespace is deliberately owned by another account (mode 700) $ bin/fm-teardown.sh task-x1 --force error: herdr session presentation lock could not be resolved for task-x1; the task and its work were not touched - the presentation lock namespace /tmp/firstmate-herdr-presentation-947162 is owned by uid 947163 but this account is uid 947162, so its ownership check can never pass and no later attempt will clear it; have uid 947163 or an administrator remove that directory (it holds only lock files) and rerun [exit status: 1] [herdr pane closed under the lock: no] [task still recorded in flight (state/task-x1.meta present): yes] SCENARIO 3 - target revision, usable namespace but the named session cannot be resolved $ bin/fm-teardown.sh task-x1 --force error: herdr session presentation lock could not be resolved for task-x1; the task and its work were not touched - rerun teardown once the session is reachable and unambiguous [exit status: 1]

================================================================================
SCENARIO 1 - another macOS account claimed the shared lock namespace first
  /tmp/firstmate-herdr-presentation reports owner uid 947161; this account is uid 501.
  (staged with a stat shim; nothing on the machine is chowned or removed)
================================================================================

--- BEFORE: base revision 1cb900c ---
$ bin/fm-teardown.sh task-x1 --force
  error: herdr session presentation lock could not be resolved for task-x1; nothing was changed - rerun teardown once the session is reachable and unambiguous
  [exit status: 1]
  [herdr pane closed under the lock: no]
  [task still recorded in flight (state/task-x1.meta present): yes]

--- AFTER: target revision, identical machine state ---
$ bin/fm-teardown.sh task-x1 --force
  teardown task-x1 complete (window default:wG:pQ, worktree /var/folders/t0/l7l3qdgn6p51q2rbn48sv30w0000gn/T//fm-teardown-tests.2zMFtT/herdr-legacy-claimed-after/wt)
  Backlog: task-x1 just finished. Run tasks-axi done task-x1 --note "local main", then run tasks-axi ready for dependency-cleared candidates, check date gates, and dispatch only work whose blockers are gone and date is due.
  [exit status: 0]
  [herdr pane closed under the lock: yes]
  [task still recorded in flight (state/task-x1.meta present): no]

  [lock namespace this account actually resolved:]
  /tmp/firstmate-herdr-presentation-501

================================================================================
SCENARIO 2 - target revision, this account's OWN per-uid namespace is
  deliberately owned by another account (mode 700, denies this account)
================================================================================

$ bin/fm-teardown.sh task-x1 --force
  error: herdr session presentation lock could not be resolved for task-x1; the task and its work were not touched - the presentation lock namespace /tmp/firstmate-herdr-presentation-947162 is owned by uid 947163 but this account is uid 947162, so its ownership check can never pass and no later attempt will clear it; have uid 947163 or an administrator remove that directory (it holds only lock files) and rerun
  [exit status: 1]
  [herdr pane closed under the lock: no]
  [task still recorded in flight (state/task-x1.meta present): yes]

================================================================================
SCENARIO 3 - target revision, usable namespace but the named session cannot
  be resolved: the RETRYABLE diagnosis must stay, with no namespace fault
================================================================================

$ bin/fm-teardown.sh task-x1 --force
  error: herdr session presentation lock could not be resolved for task-x1; the task and its work were not touched - rerun teardown once the session is reachable and unambiguous
  [exit status: 1]
  [herdr pane closed under the lock: no]
  [task still recorded in flight (state/task-x1.meta present): yes]
Evidence: Evidence driver that produced the transcript (real CLI, base vs target)
#!/usr/bin/env bash
# Evidence driver: drive the REAL bin/fm-teardown.sh CLI through the herdr
# presentation-lock namespace collision, on the base revision and on the target
# revision, and capture the operator-visible transcript.
set -u
WT=${WT:?}
EV=${EV:?}
BASE_ROOT=${BASE_ROOT:?}

HELPDIR=$(mktemp -d -t fm-teardown-helpers.XXXXXX)
mkdir -p "$HELPDIR/tests"
ln -s "$WT/bin" "$HELPDIR/bin"
cp "$WT/tests/lib.sh" "$HELPDIR/tests/lib.sh"
sed -n '1,2843p' "$WT/tests/fm-teardown.test.sh" > "$HELPDIR/tests/harness.sh"
# shellcheck disable=SC1090
. "$HELPDIR/tests/harness.sh"
ROOT=$WT

SELF_UID=$(id -u)
FOREIGN_UID=$(fm_test_fake_uid 1)
OUT="$EV/teardown-lock-namespace-transcript.txt"
: > "$OUT"

say() { printf '%s\n' "$@" | tee -a "$OUT"; }

build_case() {  # <name>
  local case_dir
  case_dir=$(make_case "$1")
  write_meta "$case_dir" local-only ship
  configure_flat_herdr_teardown_case "$case_dir"
  : > "$case_dir/herdr.log"
  : > "$case_dir/state/task-x1.status"
  printf '%s' "$case_dir"
}

run_cli() {  # <root> <case-dir>
  local FAKE_SESSION_GARBAGE=${FAKE_SESSION_GARBAGE:-0}
  local root=$1 case_dir=$2 rc=0
  FM_ROOT_OVERRIDE="$root" \
  FM_STATE_OVERRIDE="$case_dir/state" \
  FM_CONFIG_OVERRIDE="$case_dir/config" \
  FM_FAKE_HERDR_LOG="$case_dir/herdr.log" \
  FM_FAKE_HERDR_CLOSED="$case_dir/closed" \
  FM_BACKEND_HERDR_IDLE_SHELL_PROOF_POLLS=1 \
  FM_FAKE_HERDR_SESSION_LIST_GARBAGE="${FAKE_SESSION_GARBAGE:-0}" \
  PATH="$case_dir/fakebin:$PATH" \
    "$root/bin/fm-teardown.sh" task-x1 --force \
      > "$case_dir/stdout" 2> "$case_dir/stderr" || rc=$?
  printf '%s' "$rc"
}

report() {  # <case-dir> <rc>
  local case_dir=$1 rc=$2
  say "\$ bin/fm-teardown.sh task-x1 --force"
  sed 's/^/  /' "$case_dir/stdout" | tee -a "$OUT"
  sed 's/^/  /' "$case_dir/stderr" | tee -a "$OUT"
  say "  [exit status: $rc]"
  if [ -e "$case_dir/closed" ]; then
    say "  [herdr pane closed under the lock: yes]"
  else
    say "  [herdr pane closed under the lock: no]"
  fi
  if [ -e "$case_dir/state/task-x1.meta" ]; then
    say "  [task still recorded in flight (state/task-x1.meta present): yes]"
  else
    say "  [task still recorded in flight (state/task-x1.meta present): no]"
  fi
  say ""
}

say "================================================================================"
say "SCENARIO 1 - another macOS account claimed the shared lock namespace first"
say "  /tmp/firstmate-herdr-presentation reports owner uid $FOREIGN_UID; this account is uid $SELF_UID."
say "  (staged with a stat shim; nothing on the machine is chowned or removed)"
say "================================================================================"
say ""

say "--- BEFORE: base revision $(git -C "$WT" rev-parse --short HEAD~11 2>/dev/null || echo base) ---"
C1=$(build_case herdr-legacy-claimed-before)
fm_test_uid_stat_shim "$C1/fakebin" '/tmp/firstmate-herdr-presentation' "$FOREIGN_UID"
RC=$(run_cli "$BASE_ROOT" "$C1"); rm -f "$C1/fakebin/stat"
report "$C1" "$RC"

say "--- AFTER: target revision, identical machine state ---"
C2=$(build_case herdr-legacy-claimed-after)
fm_test_uid_stat_shim "$C2/fakebin" '/tmp/firstmate-herdr-presentation' "$FOREIGN_UID"
RC=$(run_cli "$WT" "$C2"); rm -f "$C2/fakebin/stat"
report "$C2" "$RC"
say "  [lock namespace this account actually resolved:]"
say "  $(PATH="$C2/fakebin:$PATH" bash -c '. "$0/bin/backends/herdr.sh"; fm_backend_herdr_presentation_lock_namespace' "$WT")"
say ""

say "================================================================================"
say "SCENARIO 2 - target revision, this account's OWN per-uid namespace is"
say "  deliberately owned by another account (mode 700, denies this account)"
say "================================================================================"
say ""
NS_UID=$(fm_test_fake_uid 2)
OWNER_UID=$(fm_test_fake_uid 3)
NS="/tmp/firstmate-herdr-presentation-$NS_UID"
fm_test_register_cleanup "$NS"
mkdir -m 500 "$NS" || { echo "could not stage $NS"; exit 1; }
C3=$(build_case herdr-per-uid-foreign-owner)
fm_test_uid_stat_shim "$C3/fakebin" \
  '/tmp/firstmate-herdr-lock-uid.*' "$NS_UID" "$NS" "$OWNER_UID" "$NS" mode:700
RC=$(run_cli "$WT" "$C3"); rm -f "$C3/fakebin/stat"
chmod 700 "$NS" 2>/dev/null || true
rmdir "$NS" 2>/dev/null || true
report "$C3" "$RC"

say "================================================================================"
say "SCENARIO 3 - target revision, usable namespace but the named session cannot"
say "  be resolved: the RETRYABLE diagnosis must stay, with no namespace fault"
say "================================================================================"
say ""
C4=$(build_case herdr-session-unresolvable)
FAKE_SESSION_GARBAGE=1
RC=$(run_cli "$WT" "$C4")
FAKE_SESSION_GARBAGE=0
report "$C4" "$RC"

rm -rf "$HELPDIR"
Evidence: Targeted suite results
$ bash bin/fm-test-run.sh tests/fm-backend-herdr.test.sh
ok - herdr presentation lock: the namespace name is per account, so no account can claim another's
ok - herdr presentation lock: a foreign-owned namespace is diagnosed as permanent and names its remedy
ok - herdr presentation lock: an account id its own namespace contradicts is diagnosed through the real refusal ordering
ok - herdr presentation lock: an account id probe that fails is never reported as another account's ownership
ok - herdr presentation lock: an inherited account id can neither move the namespace nor refuse this account out of it
ok - herdr presentation lock: an absent namespace that cannot be created is diagnosed as permanent
ok - herdr presentation lock: one refusal tail keeps the permanent and retryable faults apart
ok - herdr presentation lock: a usable namespace reports no permanent fault
FM_TEST_SUMMARY total=1 failed=0 skipped_gate=0 duration_ms=54748

$ bash bin/fm-test-run.sh tests/fm-teardown.test.sh
ok - herdr teardown separates an unusable lock namespace from an unreachable session
ok - herdr teardown diagnoses an untrustworthy account id through the real refusal ordering
ok - herdr teardown reports an uncreatable lock namespace as permanent rather than retryable
ok - herdr teardown completes under an inherited account id instead of refusing permanently
ok - herdr teardown completes without ever touching the old shared presentation lock namespace
FM_TEST_SUMMARY total=1 failed=0 skipped_gate=0 duration_ms=130556
Evidence: Real-Herdr-gated e2e: same failure on target and on base (pre-existing)
$ bash bin/fm-test-run.sh tests/fm-backend-herdr-presentation-e2e.test.sh # target 5af6d67
not ok - opted-out anchor spawn failed: ...
error: treehouse get did not enter a worktree within 60s; inspect window fm-lab-fm-herdr-present-9408-31201:w1:p2
FM_TEST_SUMMARY total=1 failed=1 skipped_gate=0 duration_ms=76019

$ (from a clean checkout of base 1cb900c) bash bin/fm-test-run.sh tests/fm-backend-herdr-presentation-e2e.test.sh
not ok - opted-out anchor spawn failed: ...
error: treehouse get did not enter a worktree within 60s; inspect window fm-lab-fm-herdr-present-16457-16838:w1:p2
FM_TEST_SUMMARY total=1 failed=1 skipped_gate=0 duration_ms=76201
- Outcome: ⚠️ 1 warning across 1 run (11m30s)

Pipeline

Updates from git push no-mistakes

✅ **intent** - passed

✅ No issues found.

✅ **Rebase** - passed

✅ No issues found.

⚠️ **Review** - 1 info
  • ⚠️ bin/backends/herdr.sh:794 - fm_backend_herdr_presentation_lock_namespace_fault swallows the resolve failure with || true at line 794 and reads expected_uid from the parent frame (795), but dir at 797 comes from $(fm_backend_herdr_presentation_lock_namespace), a subshell that runs an entirely independent second probe. The two can disagree. Fix: replace ... || true with an early if ! fm_backend_herdr_presentation_lock_self_uid_resolve; then printf &#39;&lt;the id could not be read back ...&gt;&#39;; return 0; fi. Then dir is built from the same memoized value that expected_uid holds and cannot diverge, and the now-dead [ -z &#34;$dir&#34; ] branch collapses into it.
  • ℹ️ bin/backends/herdr.sh:742 - The block comment on fm_backend_herdr_presentation_lock_namespace still asserts that per-uid naming "removes the collision by construction" on "a name no other account can claim" - the exact claim the accompanying doc change deliberately walked back at docs/herdr-backend.md:110 ("A second local account can still deliberately create that name"). /tmp is world-writable with the sticky bit, so any local account can mkdir /tmp/firstmate-herdr-presentation-&lt;victim-uid&gt; first; only accidental collision is removed, not deliberate claiming. This comment sits directly above the ownership and mode assertions it justifies, so a future maintainer reading it could reasonably conclude those assertions are now redundant and drop them.
  • ℹ️ bin/backends/herdr.sh:806 - The fault probe's mkdir -m 700 &#34;$dir&#34; means consulting the diagnostic can create the namespace directory. bin/fm-teardown.sh:2121 was reworded for this ("the task and its work were not touched" instead of "nothing was changed"), but the four warning-only sites that also call the suffix - bin/fm-spawn.sh:722 (inside the EXIT trap spawn_abort_cleanup), bin/fm-spawn.sh:2017, bin/fm-herdr-session-cleanup.sh:224 ("skipped"), and bin/backends/herdr.sh:3115 - previously touched no filesystem state on that path and now may. Noting as an accepted tradeoff, not a defect: the created entry is a mode-700 directory under /tmp owned by the running account, identical to what the next successful lock resolution would create anyway, and the herdr.sh block comment documents it explicitly.
  • ℹ️ tests/fm-backend-herdr.test.sh:2684 - The new cases stage fixed machine-global paths (/tmp/firstmate-herdr-presentation-424241/424242/424245/424251 here; -424247/424253 and chmod 000 /tmp/firstmate-herdr-presentation in tests/fm-teardown.test.sh:1550,1595,1739) rather than a TMPDIR-private root, which is what the isolation contract in bin/fm-test-isolation-proof.sh otherwise requires of tests/fm-backend-herdr.test.sh as a proven-isolated parallel candidate. The staged uids are disjoint per file, so no two scripts in the shard set collide today, and the /tmp dependency is unavoidable because the production namespace lives there. Residual exposure is narrow: two concurrent runs of the same script on one host would destroy each other's staging (rmdir &#34;$ns&#34; at case entry, then mkdir -m 500 &#34;$ns&#34; || fail), and killing tests/fm-teardown.test.sh:1739 mid-case leaves the now-unused legacy path at mode 000 with no trap-based restore.

🔧 Fix: resolve lock uid once in fault, dedupe test stat shim
1 info still open:

  • ℹ️ tests/fm-backend-herdr.test.sh:2620 - mkdir -m 700 &#34;$staged&#34; 2&gt;/dev/null || chmod 700 &#34;$staged&#34; || fail keeps an already-exists fallback that is now unreachable: $path_a/$path_b are derived from fm_test_fake_uid, so the name is unique to this process and cannot pre-exist from another run. It is also the last in-place chmod of a machine-global /tmp namespace name left in the suite - the same shape that was removed from tests/fm-teardown.test.sh this round. It is safe as written (a genuinely foreign directory would make the chmod fail and reach fail), but a plain mkdir -m 700 &#34;$staged&#34; || fail says what the test actually requires and drops the silent-adopt path.
⚠️ **Test** - 1 warning
  • ⚠️ tests/fm-backend-herdr-presentation-e2e.test.sh:715 - tests/fm-backend-herdr-presentation-e2e.test.sh (real-Herdr-gated lane) fails in this environment at its first case with error: treehouse get did not enter a worktree within 60s, before any presentation-lock code executes. I confirmed it is pre-existing by running the same file from a clean checkout of the base commit 1cb900c, which fails identically in the same 76s. The change is not implicated, but it means the two negative assertions this change added there (! grep -F &#34;presentation lock namespace&#34; under lock contention, at the two bin/fm-spawn.sh degradation sites) were not exercised locally and remain owned by the required remote Herdr CI lane. Note this is the same file that, per the accepted handover, carries no positive fault-fragment coverage for those two spawn sites (tracked as fm-herdr-spawn-fault-fragment-coverage-k3).
  • bash bin/fm-test-run.sh tests/fm-backend-herdr.test.sh - passed (exit 0), covering the 8 new cases: test_presentation_lock_namespace_is_per_account, test_presentation_lock_namespace_foreign_owner_is_diagnosed, test_presentation_lock_namespace_untrustworthy_account_id_is_diagnosed, test_presentation_lock_namespace_fault_survives_a_transient_probe_failure, test_presentation_lock_namespace_ignores_an_inherited_account_id, test_presentation_lock_namespace_uncreatable_is_diagnosed, test_presentation_lock_refusal_suffix_separates_the_two_faults, test_presentation_lock_namespace_usable_reports_no_fault
  • bash bin/fm-test-run.sh tests/fm-teardown.test.sh - passed (exit 0), covering the 5 new cases: test_herdr_teardown_diagnoses_a_foreign_owned_lock_namespace, test_herdr_teardown_diagnoses_an_untrustworthy_account_id, test_herdr_teardown_diagnoses_an_uncreatable_lock_namespace, test_herdr_teardown_ignores_an_inherited_account_id, test_herdr_teardown_ignores_another_accounts_lock_namespace
  • Manual end-user E2E: drove the real bin/fm-teardown.sh task-x1 --force CLI through three scenarios on the target revision and, for the collision case, on a clean git archive checkout of base commit 1cb900c, capturing stdout/stderr, exit status, whether the herdr pane was closed under the lock, and whether state/task-x1.meta survived (driver: /var/folders/t0/l7l3qdgn6p51q2rbn48sv30w0000gn/T/no-mistakes-evidence/01M0H75QKS1SQ4G7X3PV0BJ5SK/drive-lock-namespace-e2e.sh)
  • Manual check of the resolved namespace through the executable interface: bash -c &#39;. &#34;$0/bin/backends/herdr.sh&#34;; fm_backend_herdr_presentation_lock_namespace&#39; &#34;$PWD&#34; -> /tmp/firstmate-herdr-presentation-501
  • bash bin/fm-test-run.sh tests/fm-backend-herdr-presentation-e2e.test.sh - failed at its first case in this environment; re-run from a base-commit checkout to confirm the failure is pre-existing and identical
  • Post-run hygiene: git status --porcelain clean, temporary base checkout removed, no fake-uid /tmp/firstmate-herdr-presentation-* or /tmp/firstmate-herdr-lock-uid.* residue left behind
⚠️ **Document** - 1 info
  • ℹ️ docs/herdr-backend.md:113 - docs/herdr-backend.md:113-117 documents a rollout window in which two worktrees of one account on differing revisions resolve different lock namespaces and both order at once. It is load-bearing operator guidance today, but it is written in transitional terms ("the older revision", "this revision") and becomes wrong once no worktree runs the pre-change name, with no recorded removal trigger. Left in place as in-scope guidance for this change; worth a follow-up to delete it once the old unsuffixed namespace is no longer reachable in any deployed worktree.
⚠️ **Lint** - 1 warning
  • ⚠️ linter found issues (exit code 127)
✅ **Push** - passed

✅ No issues found.

The presentation lock namespace was the fixed path
/tmp/firstmate-herdr-presentation, and resolving a lock asserted that the
directory is owned by the current uid at mode 700. On a machine with more
than one account, whichever account created it first owned it, and every
other account's ownership assertion then refused permanently: the directory
is mode 700 so it cannot even be inspected, and the only remedy was a
privileged removal.

That refusal blocks teardown outright, so a task whose work has already
landed stays recorded as in flight and keeps raising stale-endpoint alarms.
It also delays cleanup unboundedly, and a delayed cleanup can act on a stale
target: an observed case cleared a worktree that had since been reassigned to
a different live task.

Name the namespace /tmp/firstmate-herdr-presentation-<uid> so the collision
is impossible by construction. The ownership and mode assertions are kept
unchanged; they are correct, and they are now applied to a name no other
account can claim. The directory holds only lock files, so the old path is
left alone and nothing is migrated.

Separate the two faults that made this refusal misleading. An unresolvable
named session may clear on a later attempt; an unusable lock namespace never
does. fm_backend_herdr_presentation_lock_namespace_fault classifies the
second and prints the exact path, the fault, the owning uid, and the remedy,
and teardown, spawn recovery, task kill, and session cleanup report it
instead of blaming the session.

Regression coverage: the namespace name is per account and another account's
namespace is inert; a foreign-owned namespace is diagnosed as permanent and
names its remedy; and teardown reports that fault rather than the session,
and completes while another account's namespace exists.
@YogiGit

YogiGit commented Aug 21, 2026

Copy link
Copy Markdown
Author

Non-vacuity evidence for the tests in this change

Recording this as a comment because the commit bodies here are pipeline-generated and cannot carry it.

Each new or changed guard below was proven non-vacuous by reverting what it guards, observing the named assertion fail, then restoring and observing it pass.

1. Account-id divergence in the namespace fault probe

Reverting the resolve-once ordering in fm_backend_herdr_presentation_lock_namespace_fault fails:

the fault did not name the unreadable account id:
  ... reports owner uid 9720860 rather than uid , ...

Note the empty id. That is the defect precisely: a transient mktemp/stat failure in the function's own frame, with the $(...) subshell's independent retry succeeding, reaches the ownership comparison with an empty expected uid and prints a permanent verdict naming no account. Covered by tests/fm-backend-herdr.test.sh:2737, which shims the probe to fail once then succeed. With the pre-fix code that suite drops to 98 ok / rc=1.

2. The old shared namespace is never mutated by the tests

test_herdr_teardown_ignores_another_accounts_lock_namespace no longer creates or chmods the machine-global /tmp/firstmate-herdr-presentation. It wraps mkdir/rmdir/chmod/stat so any invocation naming that path is recorded and fails. Injecting a guarded touch of the legacy path fails:

herdr-other-account-namespace: teardown touched the old shared namespace name:
  mkdir /tmp/firstmate-herdr-presentation

This matters beyond hygiene: chmodding that path in place can lock a concurrently running pre-change process out of its own live lock namespace, and because releasing a lock also requires entering that namespace, such a process could not even release locks it already held.

Suite results at this commit (154af36a26f4053432abcac8aa9273fdf3e97a96):

tests/fm-backend-herdr.test.sh   188 ok, 0 failed, 0 gate-skipped   rc=0
tests/fm-teardown.test.sh         63 ok, 0 failed, 0 gate-skipped   rc=0

Not verified locally. tests/fm-backend-herdr-presentation-e2e.test.sh is real-Herdr-gated and fails in the local environment at treehouse get did not enter a worktree within 60s, before any presentation-lock code executes. It fails identically on the base commit, so this change is not implicated - but the two negative assertions it adds (! grep -F "presentation lock namespace" under lock contention, at the two bin/fm-spawn.sh degradation sites) are exercised only by the tests-herdr CI lane, which runs the real-herdr-gated family with --fail-on-gate-skip 'herdr not found' and hard-fails if Herdr or Treehouse is absent. That lane's result on this PR is the verification of those two assertions.

@kunchenguid kunchenguid left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Speaking as Kun's firstmate:

VISION verdict: align. This only scopes an existing Herdr presentation lock to a per-uid namespace and separates a permanent namespace fault from a retryable session miss. That strengthens a refusal path and restores teardown when another local account claimed the old shared /tmp name.

Class: corrective. Ownership and mode-700 assertions stay. Acquisition semantics are unchanged aside from the namespace name. The documented cross-revision window is an honest trade: older worktrees still use /tmp/firstmate-herdr-presentation, this revision uses the per-uid name.

No security issue in the lock mechanics. The uid is read back from a self-created /tmp probe, an inherited env uid is discarded, and a foreign-owned name is still refused. Approving first-time fork workflow runs. Not merge-eligible until CI including no-mistakes is green.

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.

3 participants