P2: Stale Bot lock recovery can grant ownership to two callers #392

Activity

1. sambitcreate

   added

   bugSomething isn't workingSomething isn't working

   codex

   on Oct 8, 2026
2. pullfrog commented on Oct 8, 2026

   

   pullfrogBot

   on Oct 8, 2026 – with Pullfrog

   Last edited by pullfrog

   Contributor

   More actions

   Assessment
   Actionable; retain P2. The current code supports the reported race: reclaimStaleLock removes the authoritative pathname before checking the moved bytes. Restoring B's record afterward cannot revoke C's successful acquisition or stop B's already-open runtime. createBotHarnessHost acquires once and retains that ownership assumption. Existing tests cover single-reclaimer crash recovery, not overlapping reclaimers.

   This assessment is source-verified, not a new execution of the issue's reproduction. Ordinary Electron startup has an earlier single-instance gate; the demonstrated impact is broken service-level exclusion and possible duplicate effects, not demonstrated SQLite corruption.

   Recommended Direction
   Replace pathname/PID-based authority with a process-owned, OS-backed lifetime lock. The smallest promising option here is a dedicated node:sqlite guard database holding BEGIN EXCLUSIVE for the host lifetime, using verified rollback-journal mode and zero busy timeout. node:sqlite already ships in Aiden, so this avoids a new native dependency. Validate this option first against SQLite's locking guarantees and locking hazards; if those constraints cannot be maintained, use an audited in-process flock binding instead. Do not use a lock-only child helper: its death could release exclusion while the parent still runs Bots.

   Implementation Plan

   1. Establish the failing behavioral invariant. Extend profile-lock.test.ts and the existing child/scenario harness to reproduce the A/B/C ordering with barriers around real filesystem operations, without fabricated results or timing sleeps. Keep successful owners alive and assert that no second claimant succeeds before release or process death. Cover concurrent first acquisition and competing claimants after SIGKILL as well.
   2. Replace acquisition and recovery authority. Use a new, permanent guard pathname separate from legacy runtime.lock. Grant success only after obtaining the lifetime lock; retain the connection privately until release. Never rename, unlink, atomically replace, or recreate the guard as stale/corrupt recovery. Preserve same-process refusal with a canonical-profile pending/held/releasing reservation established before file preparation. For SQLite, prohibit raw reads/open-close operations on the held guard, including backup/hash paths, because they can cancel POSIX locks. Use PID metadata only for diagnostics, with an explicit unknown-holder value. Fail closed on corruption, unsafe paths, permissions, unsupported modes, or I/O errors; only recognized contention maps to unavailable.
   3. Bind release to completed teardown. Keep acquisition before any Bot session database opens. Review harness-host.ts shutdown, which currently releases after Promise.allSettled, so a failed close is not mistaken for completed teardown. Share the shutdown promise, stop admission, settle pending lifecycle work, and release only after protected harness/storage activity has stopped. Retain exclusion on failed or timed-out teardown until process exit. Ensure orphan scans ignore the guard and release is idempotent.
   4. Validate recovery and protocol transition. Extend behavioral coverage for concurrent same-process attempts, canonical path aliases, loser cleanup while a winner remains active, graceful release/reacquire, crash recovery, initialization errors, corrupt guards, and failed teardown. Preserve explicit Resume/Dismiss and zero provider calls during startup inspection. Require a quiescent upgrade with all Bot hosts using the new protocol: unmodified old binaries ignore the new guard, and a legacy marker cannot safely fence an already-paused old reclaimer. Document that limitation rather than claiming mixed-version exclusion.
   5. Verify and document. Run npm run test:bot-runtime, npm run test:bots, npm run type-check, and focused lint; register any additional test files. Exercise real cross-process contention and SIGKILL recovery on macOS and Linux, including packaged Electron. Update .memory/bot-durable-runtime.md and the relevant Bot plan/index with the implemented ownership protocol and acceptance evidence. No UI or Remote wire-contract change is intended; reassess native-client coverage if implementation changes those contracts.

   Acceptance
   Among cooperating upgraded hosts, at most one live claimant owns a profile, including during recovery. Losing acquisition/cleanup cannot displace the winner. Ownership survives until completed teardown or process death, and malformed state never causes destructive lock reclamation.

   No implementation changes were made; the worktree is clean. Tests and builds were not run for this planning-only evaluation.

   Task list (3/3 completed)

   • Read issue, repository guidance, and relevant Bot lock context
   • Evaluate stale-lock ownership race and identify safe recovery and test strategy
   • Publish an actionable issue plan without implementing changes

     ^{｜ Implement plan ➔ ｜ View workflow run ｜ via Pullfrog ｜ Using gpt-6.1-sol ｜ 𝕏}
3. sambitcreate commented on Oct 16, 2026

   

   sambitcreate

   on Oct 16, 2026

   OwnerAuthor

   More actions

   Resolved by #499, merged as 6c2b44bb00f37dc14b18b4a373d7b137a01e3944 after exact-head CI and reviews passed. The permanent flock guard replaces pathname/PID-based ownership for cooperating upgraded Bot hosts. Behavioral tests cover three competing claimants, crash recovery, failed teardown, and shutdown holding the guard until admitted work finishes.

   Packaged-runtime acceptance also passed on a signed macOS development package built from the same Git tree as the merge. Using isolated temporary profiles, the packaged Electron main process held the Bot guard against another host. After SIGKILL, three ready claimants produced exactly one owner; a later launch recovered the guard after that owner exited. The reverse check confirmed a packaged contender did not displace a live source host, then acquired the guard after that host exited. Package verification and signature checks passed. The installed production app and user profiles were not used.

   This satisfies the issue's stated exclusion and crash-recovery acceptance among cooperating upgraded hosts. It does not establish notarized-release, Linux-package, network-filesystem, or mixed-version behavior; old Bot hosts must be stopped before upgrading a shared profile. The acceptance audit and project-memory update are being preserved separately from the source merge.
4. sambitcreate

   closed this as completedon Oct 16, 2026