Repository navigation
Conversation
Author
|
This narrow replacement addresses the scope and live-evidence requests on closed PR #7586 and maintainer comment #7586 (comment) . It excludes the host-janitor and default process-reaping changes. Validation uses a real linked Git worktree in an isolated fixture, not the live translateimage pool; running teardown against that pool would alter another home and has not been authorized. |
Author
|
Opened against the wrong repository by mistake; moving to our fork. Sorry for the noise. |
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.
Intent
The translateimage second mate's project pool (16 worktree slots for nanobanana2) is jammed by 34 task records: each slot has one slot-owner claim file naming the task that really holds it, plus 1 to 4 stale records naming the same path. bin/fm-teardown.sh refuses any record on a shared slot at its record-exclusivity scan, which runs before the slot-owner claim check, so owner and stale records block each other and no record can be cleaned up safely. Verified live: tearing down ti-eur-pricing (its PR 85 merged, branch head equals merged head) was refused naming pe-q1-refine-seams. No supported command exists to retire a stale record; the only workaround is hand-editing state/.meta, which is forbidden. Several records hold genuinely unlanded work (local-only commits, open PRs, dirty files), so forcing is not acceptable. Full per-slot evidence: 16 slots each contain a positive owner claim and stale task records, so a record-only retirement must not read, return, prune, reset, or run git inside the occupied slot. The ask: a safe, supported way to retire a finished task record whose worktree slot is owned by a different task, without touching that other task's worktree.
What Changed
--force.Risk Assessment
✅ Low: The record-only path is narrowly gated by verified pool metadata, a foreign owner claim, project locking, and fail-closed landing proof while occupied-slot operations are skipped.
Testing
The targeted teardown safety suite passed. An end-to-end merged-PR fixture retired only the stale record, preserved the current owner's record, claim, and occupied files, made zero git calls inside the occupied slot, and never called Treehouse return. The same focused test also exercised unlanded, forced, malformed-claim, and unverifiable-pool refusal paths.
Evidence: Record-only teardown CLI transcript and persisted-state evidence
Source: Record-only teardown CLI transcript and persisted-state evidence
Pipeline
Updates from git push no-mistakes
✅ **intent** - passed
✅ No issues found.
✅ **Rebase** - passed
✅ No issues found.
🔧 **Review** - 3 issues found → auto-fixed (2) ✅
bin/fm-teardown.sh:471- This contradicts the required constraint that record-only retirement “must not read, return, prune, reset, or run git inside the occupied slot.” For a foreign claim, this changed path callsfm_treehouse_pool_slot, which executesgit -C "$slot" rev-parseinbin/fm-wake-lib.sh:1583;teardown_live_slot_pathrepeats the same check during ownership and landing proof. Move pool identity verification to the earliest shared boundary using only the separate project checkout and pool/claim metadata, without entering or invoking git in the occupied slot.tests/fm-teardown-endpoint-safety.test.sh:1249- The malformed-claim and unverifiable-pool test functions are each defined twice, and neither function is invoked by the test runner at the bottom of the file. Consequently the safety paths added by commits 06be872 and 2d774cc are never exercised. Keep one definition of each and invoke both alongside the record-only regression test.docs/scripts.md:156- This change adds entries forbin/fm-task-proc-lib.shandbin/fm-host-janitor.sh, but neither file exists in the target tree and neither relates to record-only teardown. Remove these two unrelated entries so the script inventory does not advertise unavailable commands.🔧 Fix: Avoid occupied-slot access during record-only teardown
1 warning still open:
bin/fm-teardown.sh:2669- Record-only teardown always queries PR state withgh pr view, so a landed task with a recorded Forgejo PR URL is refused. The normal landing path at lines 1686-1692 already handles Forgejo throughfm_pr_url_parseandfm_pr_forgejo_read_record. Reuse or extract that provider-aware proof while keeping Git operations scoped toproject_path.🔧 Fix: Support Forgejo proof for record-only teardown
✅ Re-checked - no issues remain.
✅ **Test** - passed
✅ No issues found.
bash tests/fm-teardown-endpoint-safety.test.shExecuted the focusedtest_shared_slot_record_only_requires_landed_branchfixture againstbin/fm-teardown.sh old-taskand captured CLI output, persisted records, owner claim, occupied-slot access log, and lifecycle calls.git status --shortconfirmed testing left no transient worktree files.✅ **Document** - passed
✅ No issues found.
✅ **Lint** - passed
✅ No issues found.
✅ **Push** - passed
✅ No issues found.