fix: harden Lavish session lifecycle and cleanup safeguards - #118
Merged
Conversation
…ocused regressions passed
…ree private Lavish evidence files from the index and deleting their .gitignore unignore rules. Local evidence remains intact and ignored. Verified the exact invariant exits 0, evidence parity passes, and git diff --check passes. No other no-mistakes phase was run
…s/documentation-audiences.json. Focused checker, regression test, and git diff --check all pass; only that file changed
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
Implement the Lavish session lifecycle design from data/fm-lavish-session-behaviour-a1/report.md section 5 steps A-D and migrate the eligible class only. Add a private ownership ledger registered at process-event arm and lifecycle serve/re-serve, verified lifecycle-owned ending before teardown, safe-park transfer, retire-and-end while keeping plain retire narrow, read-only bootstrap diagnostics that distinguish registry rows from live connections, and a conservative fail-closed audit/apply helper covering primary and registered secondmate homes. Preserve all sessions with current ownership, retained worktrees, captain holds or decision bindings, registered/live process-event sources, feedback, pending prompts or unacknowledged delivery, browser/poll clients, or ambiguous ownership; never auto-prune or hand-edit Lavish state, never end missing-path rows unsupported by current Lavish, and never end preserve/ambiguous rows. Keep the three captain-protected historical review sessions open. Record the live migration results and ambiguous evidence in data/fm-lavish-session-prune-f1/report.md, copy section 6 verbatim to upstream-issue-draft.md, and do not contact or push upstream. Tests must exercise executables against isolated Lavish state, remain shellcheck-clean, and bin/fm-lint.sh must pass. Ship by direct PR without merging. Prior review findings and their pipeline fixes are preserved at the current head; validate this exact head and drive every gate unattended.
What Changed
retire-and-end, and teardown of ephemeral task/secondmate sessions while keeping plainretirenarrow.Risk Assessment
🚨 High: Destructive teardown, safe-park, and apply paths still have reachable fail-open or non-atomic transitions that can orphan or end live Lavish reviews and their ownership evidence.
Testing
Completed 1 recorded test check.
Pipeline
Updates from git push no-mistakes
✅ **intent** - passed
✅ No issues found.
✅ **Rebase** - passed
✅ No issues found.
bin/fm-lavish-session.sh:299- The authoritative criterion requires “one private per-task or home ownership ledger registered at process-event arm and lifecycle serve/re-serve points.”touch_ledger_pollreturns success when the ledger is missing or has no matching active row, sopoll-activitythen permitslavish-axi pollto continue without lifecycle ownership evidence. Fail closed before polling when the ledger or matching row is absent.bin/fm-procevent-lavish.sh:185- The arm path registers the private ledger before registering the process-event source. If source registration fails, the active ledger remains without a live source, violating the required arm-time ownership invariant and leaving misleading teardown evidence. Add an explicitly authorized rollback or recovery boundary.bin/fm-lavish-session.sh:534- The safe-park containment check is lexical and runs before canonicalization. A target such as$FM_HOME/data/task/../other/board.html, or a symlinked task directory, passes the prefix check and can copy/serve a durable artifact outside the task's owned directory. Canonicalize and validate the destination before writing or serving it.bin/fm-lavish-audit.sh:336- Audit client detection hardcodes TCP port 4387, while the supported live process-event path runs Lavish on a configured non-defaultLAVISH_AXI_PORT. A connected browser on that port is reported as absent, allowing an expired session to become eligible and be ended despite the preservation requirement for browser clients. Derive the validated active port used by the session.bin/fm-lavish-session.sh:254- After Lavish is successfully transitioned to ended,mark_endedcan fail on a ledger lock or write error. The ledger then still appears active, while later cleanup retries an already-ended session and audit treats the row as inconsistent. The end path should use the existing finalize-only recovery boundary or otherwise make verified finalization retryable.🔧 Fix: Hardened Lavish lifecycle ownership and teardown safeguards
11 issues (10 errors, 1 warning) still open:
bin/fm-lavish-session.sh:323- The required criterion is “verified lifecycle-owned ending before teardown.”ledger_rows()returns success with no rows when the ledger is absent ([ -f "$ledger" ] || return 0at line 323), soend-ephemeralexits successfully and teardown can remove a worktree without proving that no active Lavish session remains. Missing ownership evidence must fail closed at this boundary.bin/fm-teardown.sh:2782- The required behavior includes safe-park transfer and preservation of current ownership. Normal teardown calls onlyend-ephemeral, which filters outdurable-reviewrows; an active durable row whose artifact is still inside the worktree is skipped, then the worktree is returned/removed at lines 2836-2858, leaving an open Lavish session pointing at a missing file. Teardown must refuse until durable ownership is transferred and verified.bin/fm-teardown.sh:2782- The required behavior is “verified lifecycle-owned ending before teardown” while preserving registered/live process-event sources. Forkind=secondmate, line 2782 skips even the root task's ephemeral ledger; root removal at line 2945 only runs the generic process-event sweep, which retires sources but does not end Lavish sessions. The added child preflight also callsend-ephemeralonly, so active durable rows are skipped. Root or child homes can therefore be removed while Lavish sessions remain open and their ledgers disappear.bin/fm-lavish-session.sh:647- The required behavior is a safe-park transfer. Lines 647-652 retire the old source and unbind the old decision binding beforecmd_endends the old Lavish session. If ending or ledger finalization fails after retirement, the old session remains open but unmonitored, with no recoverable handoff. This needs an authorized reservation/recovery boundary for the transfer.bin/fm-procevent-lavish.sh:220- The required criterion is a private ownership ledger “registered at process-event arm.”cmd_armpublishes the source at line 210, releases the generic source lock, and registers the ledger later at line 220. A concurrent retire or re-arm can leave an active ledger with no source, while a failed registration rollback can retire another registration generation. Source and ledger publication need a shared generation/transaction boundary.bin/fm-lavish-session.sh:560- The intent requires preserving live ownership and never ending preserve rows.finalize_key_in_ledgermarks every matching active row ended at line 560 without checking that the corresponding Lavish state is actually ended. The publicfinalize-keycommand can therefore hide a live session fromend-ephemeral, after which teardown may remove its artifact/worktree while Lavish remains open. Finalization must require verified ended state.bin/fm-lavish-audit.sh:381- The required audit/apply helper must fail closed.validSessionRowchecks only key/file types at line 381 and does not enforce Lavish's path-derived key. A malformed existing row with keycandidateand file/artifactcan be frozen as eligible; apply then ends the actual file-keyed session and only afterward looks upstate.sessions.candidateat line 705, aborting after ending the wrong session. Reject key/file mismatches before eligibility.bin/fm-lavish-audit.sh:704- The intent says to “never end preserve/ambiguous rows.” Apply re-audits and compares a frozen tuple at lines 689-695, but line 704 performs an unconditional end with no lock or conditional reservation. A prompt, hold, delivery, browser client, or source can appear between the final audit and the end, so a row that is now preserve/ambiguous is still ended. The same check-then-act gap exists in direct durable end.bin/fm-lavish-audit.sh:350- The intent requires preserving browser/poll clients. The only lsof-derived browser evidence at line 350 counts process names matchingGoogle|Chromium|Chrome; an established Safari, Firefox, or other browser connection to the active Lavish port is treated as no browser client, allowing an idle session to become eligible and be ended. Client detection must cover the supported browser surface conservatively.bin/fm-bearings-board.sh:197- The changed existing-source branch checks only whether the board source ID appears infm-procevent.sh list, then records a board ledger and reportsalready-armed. If that ID belongs to a different adapter or poll argv, the board is falsely recorded as owned although its Lavish listener is not armed. Route this path through the existing exactsource_registration_matches/arm validation.bin/fm-procevent-lavish.sh:220- The new arm-time ledger is not finalized by the generic terminal-retirement path. When a user ends a session, the Lavish poll result is terminal andfm-procevent.shretires the source at lines 933-975, but no ledger row is marked ended. The audit then treats the active ledger pointing to an ended state as an inventory error at lines 280 and 489, leaving stale ownership and blocking later audits. This needs an explicit adapter/runner lifecycle integration decision.bin/fm-test-run.sh --changed --exclude-family real-herdr-gated🔧 Fix: Fixed changed-test mapping and Lavish teardown fixture regression
1 error still open:
bin/fm-test-run.sh --changed --exclude-family real-herdr-gated✅ **Document** - passed
✅ No issues found.
🔧 **Lint** - 1 issue found → auto-fixed ✅
🔧 Fix: Fix ShellCheck assignment expansion warning
✅ Re-checked - no issues remain.
✅ **Push** - passed
✅ No issues found.