Skip to content

Proposal: centralize daemon retirement and session lifetime ownership #3116

Description

@thymikee

Put identity checks, sequencing and cleanup inside the functions that own the work. Callers should be able to retire a daemon or finish a session resource with one operation, without repeating the safety rules.

Priority: registration and the daemon lock first, session lifetimes second. The payoff is correctness and fewer caller obligations. Changing classes to factories is optional.

Status: completed and landed. #3186, #3135, #3140, #3144, #3152, #3155 and #3127 are merged; inclusion is verified in main at 2212eacf5be976acc78378a7e65539be5fc81489. The implementation record retains helpers, diagrams, exact-head gates, review and native evidence.

The contract below preserves the accepted baseline and inventories. Physical CoreDevice recording-health verification was not run because Xcode signing was unavailable; review accepted this recorded risk. Request-scoped timeout recovery remains separate in #3177.

Baseline: main at c237027737, verified 2026-10-02. The cited source files are unchanged from e4716e6. Recheck the inventories against each implementation base.

Make each function own the whole operation

The repeated problem is an address passed separately from the resource: registration paths plus a process identity, or a session name plus a record. Bind them at the owning operation.

Caller Operation What the operation owns
Client lifecycle stopAndRetireDaemon / recoverAbandonedDaemonRegistration Process proof, stop policy, lock acquisition, metadata cleanup and result
Daemon lifecycle Registration functions bound to its acquired lock Publication, shutdown reports, own metadata retirement and release
Routes and session lifecycle Recording, resource and retirement functions taking SessionRef Lifetime check, latest record, field transition and artifact address
Device-claim recovery Artifact path functions Validation and path layout, without constructing a session store
flowchart TD
    C["Client lifecycle"] --> R["Shared registration jobs"]
    D["Daemon lifecycle"] --> R
    R --> H["host-kit process identity and lock"]
    Q["Routes and session lifecycle"] --> J["Recording and resource jobs"]
    J --> S["One session lifetime owner"]
    J --> A["Artifact paths and journal operations"]
    O["Device-claim recovery"] --> A
Loading

Use ordinary functions and composition. Keep private state where ownership needs it, in a small class or factory. Keep adapters that translate records or restrict authority; replacing one broad store with several broad objects does not reduce caller obligations.

Retire a daemon safely

Deepen the existing registration module into a shared root module, such as src/daemon-registration.ts. It already distinguishes matching, replaced, unproven, absent, unreadable and ownerless records. Client runtime imports from daemon internals are forbidden, so this implementation must sit above both.

Move takeover, replay shutdown, failed-startup cleanup and timeout reset to the shared operations. Timeout reset starts a fallback stop without awaiting it, then removes metadata. Awaiting the existing stop function is also insufficient: it can decline signaling or exhaust its exit wait.

Return proof and cleanup outcomes

The proposed boundary is:

type DaemonRetirementInput = Readonly<{
  paths: DaemonPaths;
  observed: DaemonRegistrationObservation;
  mode: 'graceful' | 'force';
  ownedStateDir?: OwnedReplayStateDir;
}>;

declare function stopAndRetireDaemon(
  input: DaemonRetirementInput,
): Promise<DaemonRetirementResult>;

declare function recoverAbandonedDaemonRegistration(
  input: Omit<DaemonRetirementInput, 'mode'>,
): Promise<DaemonRetirementResult>;

type ConfirmedDaemonTermination = Readonly<{
  owner: DaemonProcessIdentity;
  kind: 'exited' | 'already-dead';
}>;

type DaemonRetirementResult =
  | {
      status: 'retired';
      termination: ConfirmedDaemonTermination;
      removedInfo: boolean;
      removedStateDir: boolean;
    }
  | { status: 'absent' }
  | {
      status: 'retained';
      termination?: ConfirmedDaemonTermination;
      reason:
        | 'ownership-unproven'
        | 'registration-replaced'
        | 'stop-failed'
        | 'exit-unconfirmed'
        | 'lock-busy'
        | 'metadata-unreadable'
        | 'retirement-unconfirmed';
    };

Finalize the observation, private-directory capability, partial-failure details and error mapping before implementation. absent means metadata is absent under the lock; it does not prove process exit or authorize directory removal.

The function owns signaling and configured TERM/KILL wait budgets:

Caller Mode Policy
Timeout reset force Verify identity, send KILL immediately, await exit
Takeover and ordinary shutdown cleanup graceful Verify identity, send TERM, wait, escalate to KILL for the same lifetime, await exit
Abandoned-registration recovery No stop mode Prove abandonment; never signal a live process

No caller pre-signals or starts a fire-and-forget fallback.

Use one retirement sequence

  1. Confirm exit. Validate the observed process lifetime before signaling. Proven death skips signaling. Missing process start time, unreadable liveness, failed signaling without independent exit proof, or exhausted waits retain metadata and the owned directory.
  2. Acquire the startup lock. Use a bounded wait; host-kit alone reclaims proven dead claims. Lock release by a still-running daemon is insufficient.
  3. Re-read and retire under the lock. Remove daemon.json only if it still names the confirmed dead identity. Retain replaced, unreadable or unproven metadata.
  4. Release this acquisition. Report success only when the postconditions are confirmed. Keep typed partial outcomes for metadata or release failures.

Abandoned-registration recovery uses the same protected core. It never rediscovers and stops a live winner. Even an absent result requires acquisition and inspection under the lock.

sequenceDiagram
    participant C as Client retirement job
    participant P as Observed daemon lifetime
    participant L as Shared registration lock
    participant M as Daemon metadata
    C->>P: Verified stop and awaited exit
    P-->>C: Confirmed termination
    C->>L: Acquire; host-kit recovers dead claim
    L-->>C: Held acquisition
    C->>M: Read current registration
    alt Record names the dead identity
        C->>M: Remove while lock is held
    else Replaced or unproven
        Note over C,M: Retain current metadata
    end
    C->>L: Release this acquisition
Loading

Process identity permits signaling and matching a record. Lock possession prevents concurrent mutation. Both are required; checking identity and then unlinking without the lock leaves a race.

Bind daemon writes to its acquired lock

Daemon-side registration functions close over the actual acquisition. They assert it is still held immediately before each metadata/report mutation. Never reconstruct release authority from a path or PID.

Preserve these sequences:

Report clearing happens before daemon.json exists, so metadata ownership cannot authorize it. A released or lost acquisition cannot write or clear a report. Keep report readers, contents and best-effort reporting, and preserve primary errors plus normalized hint, diagnosticId, logPath and typed details.

Retire only the private replay directory you own

The directory capability comes from private mkdtemp creation and binds to its startup/daemon attempt. Before deletion:

  1. Confirm that daemon exited and all owned startup attempts completed.
  2. Acquire the registration lock inside the directory.
  3. Inspect repair evidence and retain held repair sessions, active close-less replay sessions and unrecovered repair-commit-failure evidence.
  4. Remove the eligible directory while holding the lock, then release.

Use the same lock for exclusion. This authority does not permit recursive deletion of a shared state directory. The immediate defect is deleting a private replay directory beneath its still-running daemon; normal successor occupancy has not been established.

Host-kit release already handles a missing owner file as not-owner without recreating the path. Add a regression test after guarded directory removal; no new release behavior is needed.

Relaunch startup after a transient holder releases

Failed-startup cleanup stays bound to its own attempt and cannot stop or remove a winning contender. The existing startup wait handles another daemon as holder; the replacement must also handle a client holding the lock during retirement:

  1. A child that loses nonblocking acquisition reports a dedicated lock-busy disposition through a shared named exit code and monitored launch result. Generic early exits, clean exits and stderr text are not contention proof.
  2. Join the losing child. Wait while inspection says held/publishing, regardless of the holder's role. Unknown inspection retains state and reports diagnostics.
  3. Attach if a winner publishes a usable registration, preserving version, code-signature and policy admission.
  4. If the claim is released or proven dead without a usable registration, launch a fresh attempt. Only acquisition reclaims dead claims.
  5. Keep the same startup deadline and existing bounded attempt budget across relaunches. Join every loser; exhaustion returns a typed failure and leaves the winner untouched.

Do not add daemon/retirement roles to host-kit's generic owner record.

After migration, delete raw info/lock removal exports, cleanupStaleDaemonLockIfSafe and recoverDaemonLockHolder. Every metadata/report mutation belongs to a held registration owner; every lock reclaim belongs to host-kit acquisition.

Replace the daemon lock

The bespoke daemon lock permits both contenders to succeed:

  • A second contender unlinks the first contender's incomplete JSON publication.
  • Two contenders read an abandoned claim; the second unlinks the first's replacement claim.

These are demonstrated unsafe interleavings, not frequency measurements. Use one host-kit process lock for the daemon's whole lifetime, with acquisition identity for reclaim/release.

The host-kit primitive needs:

Requirement Contract
Safe publication and reclaim A pause beyond publication grace cannot let an old claimant overwrite a successor. A reclaim mutex cannot be stolen solely because its live holder paused. Specify and prove the algorithm.
Nonblocking startup acquisition Make one real attempt with typed acquired/busy/unproven outcomes and no polling or sleep. timeoutMs: 0 makes zero attempts in the existing loop. Bounded client acquisition uses the same protected reclaim mechanism.
Read-only holder inspection Report held, publishing, absent and unreadable/unknown claims, with available liveness facts. A claim may precede metadata. Keep parsing/layout private; inspection does not authorize unlinking.
Lock ownership assertion Supply an acquisition-bound assertion for registration/report mutations alongside the release function.
Unknown-liveness handling Retain the claim and emit a typed reason plus an operator hint: restore inspection or stop a verified owner, then retry. Manual recovery requires external confirmation that all users of the state directory stopped. No automatic age-only or forced-delete fallback.

isAgentDeviceDaemonProcess(...) === false also covers missing start time, missing command and unreadable identity. Reclaiming on that value is unsafe. Blocking uncertain holders with diagnostics is a deliberate behavior change.

Remove the duplicate server lock type and client lock parser at cutover. Lock version has no reader and startedAt is parsed but unused; do not copy them into host-kit. Preserve daemon.json version/code-signature negotiation.

Cutover is settled in ADR 0030. Hardened daemon acquisition uses a directory at the same daemon.lock path used by the legacy exclusive file. It refuses an existing legacy file; legacy acquisition cannot unlink the directory. Real-child tests cover already-running and concurrently-starting legacy daemons. Mixed host-kit protocol access remains unsupported: stop legacy users before upgrading, and keep a single deployed version or separate environments. An uncertain mutation guard is retained; external confirmation that all users stopped is required for manual recovery.

The proven host-kit primitive, inspection/assertion APIs and cutover are integrated in the registration stack.

Keep session work tied to one lifetime

SessionRef captures a stored address and a mutable record. Lookup/list/find return fresh wrappers; rebuilding a record does not retarget an older wrapper.

Keep three concepts distinct:

Concept Meaning
Address The store key, such as cwd:<hash>:default; use it for paths, journals and store operations
Lifetime One occupant of that address, from publication to retirement
Record That lifetime's session state, which can be rebuilt

Repeated successful open rebuilds the same lifetime. Keep its resources, recording handle, actions, creation time, claims and idle-expiry handling. Preserve record construction and the second-open script-authoring abort rule, which differs from screen recording. Delete followed by publication at the same address creates a new lifetime. No inventoried caller needs a separate occupied-address/new-lifetime replace operation.

Store one stable entry per lifetime

Replace the existing map with stable entries. Each ref carries the entry's opaque identity and its captured record; .session stays a captured record, not a live getter.

type SessionRef = Readonly<{
  address: string;
  session: SessionState;
  lifetime: object;
}>;

type SessionEntry = { current: SessionState };
const entries = new Map<string, SessionEntry>();

function lookup(address: string): SessionRef | undefined {
  const entry = entries.get(address);
  return entry
    ? { address, session: entry.current, lifetime: entry }
    : undefined;
}

function resolveCurrent(ref: SessionRef): SessionState | undefined {
  const entry = entries.get(ref.address);
  return entry === ref.lifetime ? entry.current : undefined;
}

Keep entry contents and construction private. This is the canonical map, with no second registry or generated generations.

The store owns three operations:

  • Publish: refuse an occupied address and honor daemon shutdown/admission.
  • Update: require the captured lifetime; derive from its latest record; never insert a missing entry.
  • Retire: check that lifetime before removing the record and its runtime hints.
stateDiagram-v2
    [*] --> Draft
    Draft --> Live: publish at the existing adoption point
    Draft --> [*]: preparation or adoption fails
    Live --> Live: rebuild record or reopen app
    Live --> Retired: guarded deletion
    Retired --> [*]
    note right of Retired
        Address reuse creates a new lifetime.
        Old work gains no authority over it.
    end note
Loading

Put lifetime checks inside session operations

Recording, resource finish and teardown accept one ref. They resolve the latest matching record, run the owning field transition and use ref.address internally.

  • Recording owns recording transitions and journal append.
  • Repair finalization owns synthetic-close recording, script publication, failure classification and tombstones.
  • Resource functions bind their capture read/write/path adapters internally.
  • Retirement keeps hint cleanup, repair outcomes and idle markers tied to the captured lifetime.

Old work cannot gain authority by looking up its address again. Audit refs held by request binding, lock policy, replay, inventory and asynchronous cleanup. Read projections may intentionally use the captured record; durable writers resolve the matching current record inside their owning functions.

Cleaning up an old captured resource can continue after retirement. Guard the store mutation separately: clearing a slot requires both the lifetime and matching resource identity. Preserve durable resource fences. If required capture adoption loses its lifetime, run existing failed-adoption disposal/recovery; do not report success or overwrite successor evidence.

Keep publication points unchanged:

Session path Publication point
Record-only capture After successful adoption
Sessionless snapshot After capture
Provisional open Existing before-dispatch point

Shutdown can overlap canceled handlers and timed-out cleanup. Updates cannot resurrect a retired entry. An unpublished draft also needs the existing shutdown/admission owner's check before first publication; entry identity alone cannot prevent that late publication.

Preserve execution locks, idle-reaper lock order, shutdown order, wire errors, hints and artifact naming. Keep close and teardown sequencing separate: platform close, browser cleanup and materialized paths differ.

Keep field ownership enforceable

Use one lifetime-checked update for published-record replacement. Field owners pass explicit patches; the store merges them into the latest matching record:

updateSession(ref, { appLogFailure: undefined });

Keep this operation private to the owner/adapters. Domain callers use functions such as clearSessionAppLogFailure(ref). Holding a ref does not authorize every field. Existing owner-controlled in-place transitions remain in their domain functions and resolve the matching latest record.

The R7 scanner checks assignments but misses whole-record spread overrides. appLogFailure is declared store-established, yet the app-log owner writes it through spreads. Declare app-log-session-resource.ts as its owner and keep resource-slot transitions in their field owner.

Extend R7 with two syntax rules:

  1. Check top-level update keys against SESSION_STATE_FIELD_OWNERS. Accept an explicit patch literal or inline synchronous callback returning explicit named keys. Reject computed keys, patch spreads and opaque patch variables.
  2. Allow whole-SessionState spreads from an existing record only inside session-store.ts and declared draft constructors for open, sessionless snapshot and record-only capture. Existing-session branches use patches. Nested value spreads, such as a lease, remain allowed.

A callback reads the latest record when derivation needs it. Keep it synchronous and non-reentrant: one ownership check is enough. Do not add asynchronous callbacks or general value tracing.

Migrate sessionSlot.replace whole-session copies to explicit slot patches through a ref-bound capability. Capture-kit supplies the resource transition; the daemon adapter supplies its named field update. Do not bypass the update with a constructed whole record.

R7 fixtures must reject the existing app-log spread, unauthorized failure patches, computed keys, spread patches and opaque patches; pass the declared owner's explicit patch; and retain direct-assignment checks.

Migrate the callers together

There are 19 production session-record set sites: 16 in the daemon and three in capture-kit. Seven re-set the same object, eight rebuild it, and four mix creation with rebuilding. There are six direct delete callers. Exclude tests, runtime hints and internal Map operations.

Set inventory: all 19 sites and their migration
Site(s) Migration
interaction/index.ts:67 Remove same-object re-set after migrating snapshot mutation and reverse-lookup fallback
interaction-runtime.ts:71 Migrate owner write; remove re-set
selector-capture-runtime.ts:335 Migrate owner write; remove re-set
selector-runtime-backend.ts:84 Migrate owner write; remove re-set
selector-runtime.ts:94 Migrate ref-frame write; remove re-set
request-execution-scope.ts:493 Migrate recording-health write; remove re-set
session-replay-command.ts:115 Migrate coordinator write; remove re-set
app-log-session-resource.ts:120, 138 Same-lifetime patches for failure set/clear
session-app-deployment.ts:91 Same-lifetime Harmony app identity patch
session-selector-dispatch.ts:99 Same-lifetime patch from command result
lease-lifecycle.ts:129 Same-lifetime lease refresh
session-perf-runtime.ts:271 Same-lifetime profile patch; remove address-only fallback
capture transitions.ts:193, 232 Clear only the matching lifetime/resource slot
session-open-execution.ts:329, 356 Separate fresh publication from existing/provisional updates in each branch
snapshot-command-runtime.ts:147 Publish sessionless capture after capture; otherwise update captured lifetime
capture adoption.ts:45 Publish record-only draft; otherwise update after successful adoption
Delete inventory: all six callers and the policies to preserve
Site Retirement policy
record-runtime.ts:255 Retire record-only lifetime after successful recording stop
record-runtime.ts:264 On failed stop, retire only if its manifest is terminal
lease-lifecycle.ts:103 Teardown and claim cleanup, then retire captured address/lifetime
daemon-runtime.ts:205 Bounded teardown finalizes repair and retires even on cleanup timeout; guard later writes
daemon-session-idle-expiry.ts:494 Settle resources/claim before repair, retirement and idle marker; preserve failed-settle retention
session-close.ts:343 Preserve platform/resource teardown, provider/lease/claim handling and error reporting before guarded record/hint removal

The two Map.delete operations inside SessionStore.delete remove hints and the record; they are not extra callers.

Pair all seven re-set removals with their owner-write migration. A stale re-set loses an intervening rebuild. Removing it alone loses the later mutation on a detached object instead. Route the write through the latest matching record, remove the re-set, and prove both updates survive. Keep all seven in the session migration.

The boundary changes are:

  • Capture-kit: DurableCaptureSessionStore is only a narrowed type over the full store. Replace that dependency with a ref-bound read/write/path adapter that reports retired/resource-changed outcomes. Give drafts a separately admitted publication capability at the existing adoption point.
  • Command runtime: bind createDaemonRuntimeSessionStore getters and its three write projections to the captured lifetime. Screenshot's no-op setter is a read projection, not a twentieth set site.
  • Late updates: remove get(address) ?? oldRecord from perf and teardown:34,211,226. It can reinsert a deleted record or adopt a successor. App-log patches must also preserve intervening resource updates.
  • Lease expiry: teardown and deletion use session.name instead of the lookup's stored params.sessionName. Use ref.address for paths, slots and deletion; public name stays for display. Test a scoped leased address plus a separate slot under its public name. This is a helper-contract defect; production reachability with unequal names remains unproved.

Remove unrestricted upsert and resolveStoredSessionName after migration. Its object-identity scan and public-name fallback must not survive as compatibility. A rebuilt cwd-scoped default must use cwd:<hash>:default for journals, without creating a phantom public-name record.

Reduce the store interface in this change. Convert it to a factory only if that helps the resulting composition. Keep runtime projection, ref-frame authorization and capture merging adapters that still own useful behavior.

Land independent cleanups

These do not gate registration or include the seven re-set removals:

  1. Move canonical session-directory/app-log path functions into src/daemon/session-artifact-paths.ts. Device-claim recovery constructs a store only for these paths; remove that dependency while preserving validation, stored addresses, layout and errors.
  2. Replace SessionStore.expandHome with host-kit's expandSessionPath and remove the forwarder.
  3. Optionally replace SessionScriptWriter with writeSessionScript(sessionsDir, session, options). It stores one directory and its only production user is SessionStore. Keep formatting, no-clobber/force behavior, commit transitions, diagnostics, failure routing and public-name-based default filenames together.

Prove the behavior

Use deterministic interleavings and real temporary filesystem/process tests for exclusion and exit guarantees. A Map-only test cannot prove filesystem exclusion.

Registration regression checklist
Scenario Required result
Partial publication or two stale-lock reclaimers At most one owner succeeds; loser cannot remove winner
Claimant/reclaimer pauses beyond grace Cannot overwrite/release successor or lose reclaim mutex merely through age
Free, held and unproven nonblocking acquisition One real attempt; no polling/sleep; typed outcome
Old daemon running or starting concurrently Enforced cutover policy; no split exclusion or legacy stray reclamation
Replaced/unreadable metadata or unproved process start time Retain/refuse; no unproved-owner signaling or destructive cleanup
Unknown lock identity/liveness Retain state; typed reason and operator hint; no age-only fallback
Force reset versus graceful takeover Operation owns verified KILL-first versus TERM-then-KILL and awaits exit; callers do not pre-signal
Signal failure or exhausted exit wait Keep metadata/private directory unless independent death is proved
Losing startup attempt Cleanup cannot stop/remove winner
Startup meets a client-held retirement lock Typed busy; join loser; wait regardless of role; attach or relaunch with original deadline/attempt bounds
Non-contention early exit Existing failure policy; no false busy classification or winner cleanup
Winner publishes after old daemon exits Retirement re-reads under the lock and retains winner metadata
Startup clears report before metadata publication Held acquisition authorizes clearing
Released/lost acquisition mutates report Refused; successor report remains intact
Guarded private replay-directory deletion Confirm exit and completion of owned startups; retain repair evidence; release does not recreate path
Session regression checklist
Scenario Required result
Rebuild while a resource operation awaits Latest matching record; preserve unrelated app-log/profile changes
Former same-object re-set after rebuild Both rebuild and later owner mutation survive
Delete followed by address reuse Old work cannot adopt, reinsert, delete successor or clear its hints/slots
Old captured resource needs cleanup Dispose its handle without changing successor/durable evidence
Record-only capture fails or loses adoption admission No premature publication; disposal/recovery evidence retained
Repeated open with active recording Same lifetime/handle; authoring second-open rule preserved
Scoped leased address differs from public name Addressed lifetime retires; public-name slot untouched
Rebuilt cwd-scoped session is recorded Correct journal/address; no phantom public-name record
Shutdown overlaps canceled handler continuation No resurrection or successor adoption; unpublished draft cannot publish after admission closes
Bounded cleanup times out, then finishes Captured handle can finish; late capture/perf/app-log writes cannot reinsert or clear another lifetime
Snapshot/find/ref-frame/idle expiry Issuance, locks, errors and cleanup order preserved
Forbidden R7 writer/patch syntax Unauthorized assignments/keys and forbidden spreads/opaque/computed patches fail; owner's explicit patch passes

Prove reachable schedules separately from interface hazards. Request locks serialize ordinary open/close. Shutdown is an exception: session teardown takes no execution locks, server close forces closure after five seconds, and socket closure cancels without joining handlers. Exercise that real transport path and a precise handler continuation; existing open cancellation checks can block a particular interleaving.

Regression tests must fail when the relevant ownership check is removed or cleanup is made unconditional. Plant the concrete forbidden app-log spread for R7. Colocate tests with source and retain existing tests for pure moves.

Each slice runs focused checks and pnpm check:affected --run on its exact head, plus the full deterministic gate for a broad refactor as repository guidance requires. Preserve CI-owned provider/native/device checks. Report production additions/deletions separately from tests and moves, and name the mechanism or caller obligation removed.

Deliver in dependency order

  • Prove host-kit acquisition/reclaim; add nonblocking acquisition, inspection and lock ownership assertion. Settle mixed-version cutover.
  • Finalize retirement inputs/results, stop modes, bounded relaunch and operator diagnostics; migrate registration/report operations. Remove raw cleanup/recovery exports and legacy lock types/parsers.
  • Land independent path/forwarder cleanups; optionally simplify the script writer.
  • Implement stable entries/captured refs, audit ref holders, and enforce explicit update keys/whole-session spread rules.
  • Migrate all 19 set sites, six delete callers, recording/disposal inputs and runtime/capture adapters. Pair the seven re-set removals with owner writes; preserve adoption timing and resource fences. Remove upsert/reverse lookup and shrink the store interface.
  • Complete the regression checklists and record exact-head validation, including real shutdown/cancellation and late cleanup.

Related work: #3102 already adds daemon metadata publication/removal fencing; deepen it. Coordinate timeout reset with #3105 and report mutations with #3104. Preserve the #2833 idle-reaper lock seam/order. This follows #3069 without reopening it or closing related issues by association.

The implementation stack is fully landed. The implementation record preserves the original branch order, exact-head validation, completed review findings and the physical recording-health limitation. Request-scoped timeout cleanup is tracked in #3177; it is outside the retirement-ownership contract completed here.

Out of scope: a generic request executor, selector-observation aggregation, global immutable-state migration, actors/manager frameworks and promised line-count/performance gains. Request finalization and diagnostics may be separate bounded follow-ups; they do not gate this work.

Activity

  1. thymikee commented on Oct 2, 2026

    @thymikee
    MemberAuthor

    Review against 620cecb208

    None of the cited files changed between the e4716e6f51 baseline and 620cecb208, so the links below use the baseline. The direction and the design gates hold. The 19-site inventory is accurate. The findings below sharpen four gates and add two gaps: delete sites, and the shutdown path that runs outside request locks.

    1. Make lock possession the single authority for registration-adjacent state

    Shutdown report (#3104).

    • The daemon writes the report while it holds the lock, and releases the lock at L816.
    • It clears the report right after acquiring the lock, before daemon.json is published.
    • So fix(daemon): a superseded daemon overwrites the shared daemon-shutdown.json #3104's direction, reusing readRegisteredDaemonOwnership, cannot fence the clear: at that point no record names the new daemon.
    • Lock possession fences both mutations.
    • With a sound lifetime lock, and clients that refuse removal when exit is unconfirmed, a daemon that still holds the lock cannot have been superseded. The superseded-writer case then reduces to two requirements: write before release, and an assert-still-held check.

    Client retirement and recovery. This answers "specify how exclusion covers each destructive filesystem operation".

    • After confirmed exit, the client acquires the registration lock. Host-kit reclaims the dead claim.
    • The client then retires daemon.json only if the record names the dead identity, and releases the lock.
    • The owned replay directory follows the same pattern: acquire the lock inside it, then remove the directory. The directory is a private mkdtemp, so its only possible claimant is the daemon this client spawned, and no separate exclusion mechanism is needed.
    • recoverAbandonedDaemonRegistration therefore becomes acquire, retire, release.
    • cleanupStaleDaemonLockIfSafe and recoverDaemonLockHolder should be deleted, not migrated: only host-kit acquisition should ever reclaim a lock.
    • Consequence to design: while a client briefly holds the lock, a daemon that starts concurrently exits with "lock held" (L651). The client that started it must treat that exit as retryable.

    2. Host-kit gaps for a lifetime daemon lock, beyond the two race fixes

    • No fail-fast acquisition.
      • acquireProcessLock loops while (Date.now() < deadline), so timeoutMs: 0 makes zero attempts and throws a timeout.
      • The daemon needs exactly one attempt.
    • No holder inspection.
      • Clients read the lock holder today: isDaemonLockHeldByAnotherDaemon and failed-startup cleanup.
      • Host-kit exports no read API. Either it gains one, or these readers move to daemon.json.
    • Duplicated lock type.
      • DaemonLockInfo is declared twice: server and client.
      • Only pid and processStartTime appear to be consumed, so version and startedAt can probably go. Confirm before dropping them.
    • Liveness becomes fail-closed.
      • isAgentDeviceDaemonProcess returns false when the start time or the command is missing or unreadable. acquireDaemonLock then steals the lock, so today's behaviour is fail-open.
      • Host-kit's classifyOwnerLiveness is fail-closed. The migration is safer, but a holder whose liveness cannot be read will then block every daemon start.
      • The cutover gate should define the operator path for that case: a hint, a command, or both.

    3. Concrete evidence for the R7 blind spot

    • appLogFailure is in STORE_OWNED_SESSION_STATE_FIELDS, which claims that the store establishes the field and nothing mutates it later.
    • But app-log-session-resource.ts:120,138 changes it through a spread rebuild outside session-store.ts.
    • R7 walks only assignments, so it misses this. That makes the site a ready-made fixture for the "plant a forbidden writer" requirement.

    4. Set-site inventory: confirmed, with notes

    I enumerated the sites independently. All 19 sites, their categories and the 7/8/4 counts match. No SessionStore.set caller is missing. Notes:

    • Capture-kit port.
      • Capture-kit receives the full SessionStore through DurableCaptureSessionStore<S> (definition.ts:17), with no adapter.
      • That narrowed port is the natural place for the lifetime-checked write.
    • Command-layer writes.
      • Command-layer runtime.sessions.set goes through createDaemonRuntimeSessionStore (runtime-session.ts:47).
      • It reaches three of the table's sites, plus a no-op at screenshot-runtime.ts:128.
      • Include that adapter in the "lower capture publication interface" gate.
    • Fallback re-inserts.
      • Several rebuilds re-read with get(address) ?? session: session-perf-runtime.ts:271 and the teardown callers at session-teardown.ts:34,211,226.
      • The ?? session fallback re-inserts a deleted record. Name these sites under "a late update must not insert a deleted session".
    • Stale-record spreads.
      • The app-log rebuilds at L120 and L138 spread a record the caller already holds, without re-reading the store.
      • They can therefore revert another rebuild that landed in between. "Rebuild from the latest record" applies to them.
    • Same-object re-set removal changes behaviour slightly.
      • The re-sets are no-ops only while the stored object is unchanged.
      • After any rebuild, a stale holder's re-set reverts the store today. Without the re-set, the write lands on a detached object instead.
      • That is strictly better, but say so in the PR.

    5. Delete sites are not inventoried

    Guarded retirement needs the same classification for the 8 production sessionStore.delete callers. One already looks wrong:

    • lease-lifecycle.ts:80 and :103 pass session.name to teardown and to delete, although params.sessionName (the address) is in scope.
    • For any session whose address differs from its name, two things go wrong:
      • Teardown writes, including the capture-kit slot clears, target the wrong slot.
      • The delete misses, so the expired record stays live.
    • Either prove that leased sessions always have address === name, or pass the address.

    6. Add the one schedule that runs outside request locks

    The qualifier that ordinary request locks serialize open and close is correct. Daemon shutdown is the exception:

    • teardownDaemonSessions takes no execution locks.
    • closeDaemonServers force-closes after 5 s.
    • Socket close cancels in-flight requests cooperatively (transport.ts:54), but nothing waits for their handlers to return.

    Proposed acceptance row: shutdown teardown while a canceled request (e.g. a long open) is still running. It is the place where lifetime checks are not backed by request locks, so it is the right place to test whether the replacement schedule is reachable.

    7. The lifetime-representation gate is nearly closed already

    Taken together, the gate's constraints leave one practical representation: a stable per-lifetime entry object held in the store map, which refs carry alongside the record they captured. The constraints are:

    • identity lives at the canonical store entry;
    • no generation scheme;
    • .session is not a live getter;
    • newer rebuilt records of the same lifetime stay resolvable.

    Stating that representation explicitly would let the gate close without re-arguing the alternatives.

  2. thymikee commented on Oct 2, 2026

    @thymikee
    MemberAuthor

    Updated the proposal after checking the review against main at 35b7b4113d7071804acd8faf5528a771266762c3. The cited implementation files are unchanged from the original audit.

    Changes mapped to the seven review points:

    1. Lock authority and retirement: report clearing/writing now belongs to the held acquisition, including startup before metadata publication. Client retirement confirms exit, acquires the same lock, matches metadata and retires under exclusion. Private replay-directory removal uses that lock too. Both client stale-lock recovery helpers are deleted in the plan; startup overlapping client-held retirement receives a bounded, typed retry.
    2. Host-kit integration: added nonblocking acquisition, typed holder inspection and a held-capability assertion. The proposal explicitly rejects using timeoutMs: 0 as an attempt with today's loop. Unknown liveness retains state and gets an operator hint/recovery policy. Legacy lock types/parsers are removed at cutover; parsed startedAt has no production consumer and lock version has no reader.
    3. R7: added the concrete appLogFailure counterexample, its owning transition and planted forbidden-writer coverage for spread/rebuild paths as well as assignment.
    4. Write boundaries: named the capture-kit port and command runtime adapter, including screenshot's no-op setter. Named the perf/teardown fallback sites, required latest-record rebuilding, and documented how deleting stale re-sets prevents rollback but still requires routing valid writes through the lifetime owner.
    5. Deletion: added the full inventory and migration policy. I found six direct production SessionStore.delete callers; the store also contains two internal Map.delete operations. Lease expiry must carry the stored address through teardown and retirement, with a scoped-address/public-name regression case. The helper-contract defect is explicit without claiming an unproved production lease schedule.
    6. Shutdown: added real forced-close/cooperative-cancellation overlap, plus timed-out cleanup continuing after retirement. The tests must also cover unpublished drafts: lifetime identity alone cannot prevent their first publication after shutdown admission closes.
    7. Lifetime representation: explicitly chose a stable canonical entry, carried alongside the captured record. .session stays a captured record; same-lifetime refresh resolves the entry's latest record. The representation-choice gate is closed; holder migration/enforcement still needs evidence.

    The body includes the updated interfaces, inventories, lock-retirement sequence diagram and acceptance matrix. It remains needs-triage because the hardened lock algorithm and enforceable mixed-version cutover still need to be settled and proved.

  3. thymikee commented on Oct 2, 2026

    @thymikee
    MemberAuthor

    Re-review of the updated body (c237027737)

    The revision covers all seven points. The lifetime representation is settled, and the remaining open gates are the right ones: the hardened lock algorithm and the mixed-version cutover. One correction to my earlier comment: six delete callers is right. My 8 counted the two Map.delete calls inside SessionStore.delete.

    Five things are still worth settling in the body.

    1. Same-object re-set removal is no longer an independent cleanup

    Removing a re-set does not prevent a lost update. It changes which update is lost:

    • Today, a stale holder's re-set rolls the store back, which loses the rebuild.
    • After removal, the stale holder's mutation lands on a detached object, which loses the mutation.

    The body now pairs removal with owner-write migration, which is correct. But item 3 still sits under "Independent small cleanups … not prerequisites", and the first delivery checkbox still bundles it with the path and forwarder work.

    Either:

    • move it into the session migration; or
    • keep as independent only the sites shown not to straddle a rebuild within one request.

    2. Startup contention needs the holder's role, not only its liveness

    The launcher keeps waiting after its own daemon exits early only when isDaemonLockHeldByAnotherDaemon is true. That check requires the holder's command to be an agent-device daemon.

    When a retirement client holds the lock, the launcher therefore returns early_exit today. The "retry after release" contract needs one of these:

    • Holder role in the record. The host-kit inspection reports the holder's role (daemon or retirement). This means the owner record must carry a role, and ProcessLockOwner has no such field.
    • Relaunch on release. The launcher relaunches when its own daemon lost and the holder released without publishing daemon.json.

    Pick one in the "complete registration job inputs" gate.

    3. The stop policy belongs in DaemonRetirementInput

    The two client paths stop the daemon differently:

    The earlier sketch carried this as mode: 'graceful' | 'force', and the current input drops it. Keep it in the input, so that no caller pre-signals before calling the job.

    4. Enforce R7 at the update boundary, not by tracing values

    As written, the R7 requirement amounts to a data-flow analysis:

    • trace locally bound replacement records;
    • distinguish construction from update;
    • ignore spread fields that were copied unchanged.

    Once every published-lifetime write goes through the owner's update operation, two simpler rules cover the same ground:

    1. Check the field keys that each call site of the update operation overrides against SESSION_STATE_FIELD_OWNERS.
    2. Allow building a SessionState from an existing one (by spread) only in session-store.ts and in the declared draft constructors.

    The appLogFailure fixture fails both rules as it stands. That gives the same proof with a much smaller scanner.

    5. The owned-directory release needs a test, not new behaviour

    Host-kit release already treats a missing owner file as not-owner. It returns without recreating anything (process-lock.ts:176-179).

    "Release must handle the deliberately removed private lock" is therefore already true. Pin it with a test rather than specifying it as a change.


    None of these blocks the current design gates. Keeping needs-triage until the lock algorithm and the cutover policy are proved is the right call.

  4. thymikee commented on Oct 2, 2026

    @thymikee
    MemberAuthor

    Addressed the five follow-up points in the body after checking main at c237027737bf4737e324adae2146ba63891152a3:

    1. Re-set removals: moved all seven into the session migration and separated their delivery item from path/forwarder work. The proof now requires both the intervening rebuild and the stale holder's later valid mutation to survive.
    2. Startup contention: chose relaunch after release. The losing child reports a dedicated internal busy disposition through its monitored exit result; the launcher waits regardless of holder role, attaches to a validated winner if one publishes, or launches a fresh attempt after release. Deadline/attempt bounds remain intact. Generic early exits remain distinct. No holder-role field is required in host-kit.
    3. Stop policy: restored required mode: 'graceful' | 'force' in DaemonRetirementInput, with TERM-then-KILL for takeover/ordinary shutdown and KILL-first for timeout reset. The job owns every signal and awaited exit. Abandoned recovery has no stop mode.
    4. R7: replaced value tracing with checks of explicit update keys and a restriction on whole-session spreads to the store and declared draft constructors. Opaque variable patches, computed keys and patch spreads are rejected; field-owner functions use explicit patches. Capture slot adapters participate in this migration, and direct-assignment checks remain. The app-log fixture covers both forbidden spread syntax and unauthorized update keys.
    5. Owned-directory release: now states that missing-owner handling already exists. The task is to pin it with a release-after-directory-removal regression test.

    The hardened lock algorithm and enforceable mixed-version cutover remain the open design gates, so the issue stays needs-triage.

  5. thymikee commented on Oct 3, 2026

    @thymikee
    MemberAuthor

    Completed and user-landed. All seven keeper PRs are verified included in main at 2212eacf5be976acc78378a7e65539be5fc81489; #3127 was the final merge. #3116, #3104 and #3105 are closed for their implemented scope. Request-scoped timeout recovery remains open in #3177. No PR was merged by the agent.

    PR Scope Dependency Head / status
    #3186 Artifact paths main Landed
    #3127 Daemon startup and retirement main 5b12609ef5 — landed; local gates, 21 CI checks and human approval complete
    #3135 Session lifetimes and owned resources main 9252d8251f — landed; inclusion in main verified
    #3140 Session journals, observations and request health #3135; #3127 prerequisite 42ce911992 — landed; inclusion in main verified
    #3144 Session replay, open, interaction and shutdown #3140 62d6af6340 — landed; inclusion in main verified
    #3152 Complete explicit-lifetime fixture migration #3144 d5472b0523 — landed; inclusion in main verified
    #3155 Session state ownership enforcement #3152 3312b9869b — landed; inclusion in main verified

    Original work split (the five session groups are now landed):

    flowchart LR
      M[main] --> R[3127 Registration ownership]
      M --> S[3135 Session lifetimes and resources]
      S --> J[3140 Journals and observations]
      R -. prerequisite .-> J
      J --> F[3144 Open, replay and interactions]
      F --> T[3152 Fixture migration]
      T --> G[3155 R7 enforcement]
    
    Loading

    The owning helpers enforce process identity, session lifetime, and cleanup sequencing. Paths are ordinary functions; launch identity belongs to the monitored launch; optional alert/settings journaling uses the shared action recorder.

    The latest registration correction recovers a missed initial birth probe only from an active monitored child and agreeing held-lock/registration records. Kernel birth verification before signaling remains required. Both sendToDaemon controls were red before the fix; 69 focused controls and 6,336 related tests plus documentation/tooling passed. Original regression proof.

    The omitted alert/settings recorder is deleted. Completed native actions can skip journaling after their lifetime ends without writing to a successor. Both alert controls were red; clipboard and first-read button-shape mutants were red; all 52 focused controls passed. #3144's complete affected gate passed 13,443 tests with one skipped plus documentation/tooling. Descendant gates passed (2,177 related tests for #3152; selected static/tooling and twelve documentation controls for #3155).

    Native screenshots and observed JSON prove the real Cancel/Open recognition shape and refusal when the app runs behind an unresolved prompt. They do not prove successful acceptance. The controlled setup pre-raised a URL prompt and then the CLI sent its URL, so attribution to a new native regression is unproven. The reviewer accepted current-head iOS Smoke Tests as the alternative single-dispatch route evidence; the older double-handoff result remains a qualified negative observation. The task-created simulator, sessions, daemons and runners are cleaned up.

    Current-head iOS CI at 62d6af6340 succeeds through cold URL open, destination wait and event/payload reads (artifact 11301510447), followed by snapshots, interactions and close. Verified evidence reply. The scenario's accepted flag is not data.launchConfirmation; that response field was not separately retained. The reviewer accepts this run as satisfying the native-route requirement. All five session keepers have external ready-for-human labels and successful current-head CI replacements; canceled duplicate runs are not treated as passes.

    #3127 is now rebased onto the landed session stack at 052cba83c1, preserving #3200 cancellation and awaited timeout retirement. The marker conflict preserves scoped reads, owner-checked clearing of owned expired markers, strict finite timestamp/source-path validation and fail-closed cleanup scanning. Both registration fixtures pass their owner argument. A real 409/hanging-health/abort control proves typed cancellation, one RPC and zero timeout-retirement calls; removing the post-probe abort check is red. All 96 focused controls and the exact-head affected gate pass: 6,579 related tests in 803 files, twelve documentation controls and selected runnable tooling checks. Conflict-resolution review evidence. The final head received fresh review and ready-for-human approval with all 21 CI checks passing, then the user merged it. Already merged branches were not rebased or republished.

    #3140's optional publish-time marker-clear suggestion is retained as a follow-up: ADR 0012 defines repair evidence by scoped session key and clears it on fresh armed replay, while failed-commit evidence also protects private directory retention. Lifetime diagnostic invalidation must not erase durable failure evidence. Cross-platform fallback lookup remains excluded to preserve address isolation. Review disposition.

    Physical recording-health remains blocked by Xcode signing. The phone was connected/unlocked, but no recording started. Simulator evidence cannot replace that predicate. This remains an accepted, explicitly unverified risk; it is not reported as passing.

    The architecture audit also identifies client-side host-wide timeout pkill and shared-daemon reset as a separate cleanup-authority gap. It was audited, not changed by this work, and remains tracked separately in #3177.

    The registration/session composition now preserves the scoped repair-marker API and strict parser together; both registration fixtures use the owner argument. The earlier audit resolution remains historical evidence; historical tree equality does not describe these later corrected heads. The 21 superseded PRs remain closed with their evidence intact. Implementation, required review/CI evidence and user-landed inclusion are verified. The physical signing limitation is preserved as an accepted risk, and the recurring review monitor is stopped.

  6. thymikee commented on Oct 4, 2026

    @thymikee
    MemberAuthor

    Completed and verified in main at 2212eacf5b: all seven keeper PRs are landed. #3127 was the final merge, with fresh human approval, all 21 GitHub checks passing, 6,579 related tests, twelve documentation controls and selected local gates passing on its exact head.

    The final merge preserves both scoped repair-marker ownership and strict cleanup validation. Registration/retirement, host-kit lock exclusion, session lifetimes/resources, journals/observations, caller/fixture migration and R7 enforcement are complete. #3104 and #3105 are closed with their specific ownership evidence.

    Physical recording-health verification remains explicitly unperformed because of Xcode signing; review accepted that risk. The unresolved request-scope policy—host-wide runner cleanup/shared-daemon reset after one request's timeout—remains open in #3177. This issue does not claim that broader policy is fixed.

    The final checklist and implementation record are reconciled. Recurring reviews are stopped; temporary validation worktrees are cleaned while logs, native evidence and historical commits are retained. Closing as completed within the agreed scope.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions