fix(herdr): make the presentation lock namespace per account - #2530
Open
YogiNmrxGit wants to merge 4 commits into
Open
fix(herdr): make the presentation lock namespace per account#2530YogiNmrxGit wants to merge 4 commits into
YogiNmrxGit wants to merge 4 commits into
Conversation
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.
…es, fix doc and test
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
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.
The defect
fm_backend_herdr_presentation_lock_namespace()returned 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 the directory first owned it.
Every other account's ownership assertion then refused permanently, because the directory is mode 700 and cannot even be inspected, and the only remedy was a privileged removal by a human.
Observed live on 2026-08-14: uid 502 created the directory, and every operation in the uid 501 home that resolved the lock failed with
herdr session presentation lock could not be resolved for <task>.Why it matters
The refusal blocks teardown outright, so a task whose work has already landed stays recorded as in flight and keeps raising stale-endpoint alarms.
That trains a supervisor to ignore the same alarms that catch a genuinely wedged worker.
It also delays cleanup unboundedly, and a delayed cleanup can act on a stale target.
That happened: by the time a blocked cleanup was finally allowed to run, the local copy it had recorded had been reassigned to a different live task, and running it cleared that task's working copy and killed its agent mid-run.
Nothing was lost because the commits were already in the object store, but the hazard is real and it is a direct consequence of this bug holding cleanup open.
The fix
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 worth having; they simply must not be applied to a name another account can claim first, 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.
Separated diagnostics
The two faults folded into the old message have different remedies, and conflating them sent a supervisor into an infinite retry loop against a condition that could never clear.
fm_backend_herdr_presentation_lock_namespace_faultclassifies the second and prints the exact path, the fault, the owning uid, and the remedy.Teardown, spawn recovery, task kill, and session cleanup now report that fault instead of blaming the session.
Tests
Regression coverage added through the real executable interface, never by asserting source bytes:
Survey of the same exposure elsewhere
The brief asked whether any other fixed shared-
/tmppath inbin/carries the same collision exposure.It does not, and no separate follow-up task is needed:
/tmpuse inbin/goes throughmktemp, so the names are unique per invocation.bin/fm-herdr-lab.shalready scopes its state directory by${UID}.tmux,cmux,zellij,orca) uses a fixed shared path at all, so the axis is checked and not applicable there.Validation
Reviewed, tested, documented, and linted through the no-mistakes pipeline at head
be1e9bf, with zero findings at every gate.