Skip to content

fix(bin): identify a task record by path identity in the slot-collision scan - #5487

Closed
taimurrabuske wants to merge 1 commit into
kunchenguid:mainfrom
taimurrabuske:fm/fm-repair-stale-worker-retirement-20260923
Closed

taimurrabuske wants to merge 1 commit into
kunchenguid:mainfrom
taimurrabuske:fm/fm-repair-stale-worker-retirement-20260923

Conversation

@taimurrabuske

Copy link
Copy Markdown

Problem

Retiring a stale worker was impossible in a home reached through a symlinked path spelling.

The pool-slot exclusivity scan in bin/fm-teardown.sh collected state directories from both the caller's FM_HOME spelling and the physically resolved root home, then compared record files as raw path strings. When a home is reached through an equivalent spelling - a /home/<user>/work symlink beside its physical /nobackup/<user>/work target - the same state directory was collected twice, so the task's single record was read twice and the second reading looked like a different task holding the same slot:

REFUSED: task X's recorded worktree /...pool/1/project is also task X's recorded worktree.

Cleanup then refused, not even with --force, and the stale record could not be retired at all.

Fix

  • State directories are collected once per physical directory (add_treehouse_owner_state).
  • Records are compared by identity - the state directory's physical path plus the record's own file name (task_record_identity) - instead of by the spelling each was reached through.

Only the directory is resolved, so two records in one directory, and one record name in two genuinely different directories, stay distinct identities. Genuine shared-slot collisions across tasks and across homes still refuse without touching the slot, and the slot-owner claim check, endpoint identity validation, worktree ownership claims, and uncommitted/unlanded-work refusals are untouched.

Validation

  • tests/fm-teardown-endpoint-safety.test.sh: 29/29 pass.
    • Recovered coverage: a symlinked home spelling returns its own sole slot; a genuine collision through a symlinked spelling still refuses, names the other task, and mutates nothing.
    • Added coverage: a Firstmate home registered through a symlinked path is still scanned for collisions, so resolving spellings never collapses two genuinely different homes.
    • Both directions were mutation-checked: the new cases fail against the unpatched script, and loosening the identity comparison (or keying state directories non-physically) fails the pre-existing collision cases.
  • tests/fm-teardown.test.sh: identical results before and after this change; its one failure is a pre-existing environment gap (installed tasks-axi 0.2.5, test requires 0.2.6+).
  • bin/fm-lint.sh clean (ShellCheck 0.11.0 pinned, actionlint 1.7.12, 3 workflow files valid), including a full extended-analysis pass over both changed files.

…on scan

The pool-slot exclusivity scan compared record file paths as strings while
collecting state directories from both the caller's FM_HOME spelling and the
physically resolved root home. A home reached through a symlinked spelling
(a /home path whose target is /nobackup) therefore appeared twice, so the
task's single record was read twice and the second reading was reported as a
different task holding the same slot. Teardown refused with "task X's recorded
worktree is also task X's recorded worktree", and the stale worker could not be
retired at all, not even with --force.

State directories are now collected once per physical directory, and records
are compared by identity - the state directory's physical path plus the
record's own name - so equivalent spellings of one home resolve to one record.
Only the directory is resolved: two records in one directory, and one record
name in two genuinely different directories, remain distinct, so a real slot
collision across tasks or homes still refuses without touching the slot, and
the claim, endpoint, and unlanded-work safeguards are unchanged.

Regression coverage recovers the focused endpoint-safety cases for a symlinked
home spelling (sole slot returns, real collision still refuses) and adds one
for a Firstmate home registered through a symlinked path, which must still be
scanned rather than collapsed into the record's own home.
@devin-ai-integration

Copy link
Copy Markdown

Closed as superseded — this work already landed on main via #5635.

— Kun's Firstmate

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