[v0.8 Core WIP] bind project trust, recovery, and typed host requests - #1186
[v0.8 Core WIP] bind project trust, recovery, and typed host requests#1186sethkarten wants to merge 42 commits into
Conversation
… workers A worker with a durable stop intent is now reported as "stopping" and a disconnected worker is never reported as "ready". Stopping workers are excluded from live session lists and daemon-wide fan-out commands. (cherry picked from commit f12dfbd)
…y checks The list response feeds shutdownStaleDaemonIfNotBusy and probeRunningDaemonSessions, so hiding stopping workers made a tombstoned-but-running worker look idle and let a stale-daemon replace terminate it silently. Stopping workers stay listed with an honest "stopping" workerState; command fan-outs still skip them. (cherry picked from commit db0137a)
…ates
Revision 14 records the workerState wire-semantics change ("stopping"
state; disconnected workers no longer report "ready") so version probes
can tell old and new daemons apart. The field stays optional and
backward-tolerant, so no capability gate is needed.
(cherry picked from commit 39b0ef7)
…ng registrations When a worker does not exit within the stop deadline, the supervisor now keeps watching the process, escalates to SIGKILL, and completes the interrupted cleanup once the process dies. Process liveness checks also treat zombie processes as dead so cleanup is not deferred forever.
… generation The background finalizer now snapshots pid, processStartId, and stopRevision when scheduled and aborts if the stop is rescinded or the worker is relaunched, so it can never SIGKILL a retried worker or an unrelated process that reused the pid. stopWorker signalling is likewise identity-aware.
- Fail closed when a recorded processStartId cannot be observed, so a recycled pid is never signalled even if identity observation fails. - Record a schedule-time identity for workers that never had one. - Retry transient finalization cleanup failures instead of stranding the dead registration permanently. - Probe liveness with a cheap kill(0) on every poll and throttle the ps-backed zombie/identity checks so wedged workers cannot saturate the supervisor event loop.
…gone stopWorker used the identity check as a liveness predicate, so a transient getProcessStartId failure could skip signalling and delete the registration of a still-running worker. Identity verdicts are now directional: only a confirmed-current pid is signalled, only a confirmed-gone/replaced pid is cleaned up, and an unknown verdict keeps waiting.
…ched mid-await stopWorker can yield during archival while a retry rescinds the stop and relaunches the worker on the same registration. The cleanup tail now verifies the registered process is still the one it stopped before removing the registration or descriptor, so a relaunched worker is never orphaned by a stale stop invocation.
The throttled identity cache can be up to 500ms old, long enough for a pid to be recycled. Both SIGKILL sites (stopWorker force escalation and the stop finalizer) now run a fresh identity check immediately before signalling; the cache remains only for read-only wait-loop polling.
…ages A transiently unobservable identity at the escalation deadline now skips that attempt without marking the kill done, so a later pass that re-verifies the original process still escalates instead of leaving a wedged worker registered forever.
…scinded stops All stopWorker polling and signalling now use the pid and start identity captured at entry, so a retry relaunching the worker mid-stop can never be SIGKILLed through the mutable descriptor. The cleanup guard also aborts when a removeDescriptor stop lost its tombstone, catching a rescission that lands before the successor pid does.
Reopening a saved session used to fail forever when a stopped worker left a tombstoned registration behind (stop timed out, process died later, and finalization was interrupted). The supervisor now detects such stale registrations during create/resume, completes the interrupted stop, and launches a fresh worker for the same saved transcript.
…gistrations A recycled pid used to make a dead worker look alive, so its stale registration was never reclaimed and resume kept failing. Reclaim now checks processStartId, treating a recycled pid as gone; the stop path never signals a pid whose identity no longer matches.
…tion A timed-out stop already has a background finalizer completing the same cleanup, so the resume-time reclaim now awaits it instead of running a duplicate stop that could repeat archival and cron-lock cleanup. Also document the intentional finalizer/reclaim race in the end-to-end test: both paths are covered deterministically by unit tests.
…ervable Align resume-time reclaim with the directional identity verdicts: only a confirmed-gone or confirmed-replaced pid is reclaimed; a transient identity lookup failure leaves the registration untouched.
…inalizer Reclaim now checks confirmed process death first and then delegates the cleanup to scheduleWorkerStopFinalization, so concurrent resumes share one stop instead of duplicating archival, and the bounded wait keeps a resume request from blocking on a finalizer that cannot settle.
…esume When the bounded reclaim wait expires before the finalizer finishes, the resume now fails with a retry hint instead of falling through to reuseWorkerForCreate with a registration whose process is confirmed dead - the exact failure mode this PR heals.
![macroscopeapp[bot]](https://avatars.githubusercontent.com/in/900172?s=60&v=4){kind=small}
| ); | ||
| } | ||
| /** All active typed handlers must observe revocation and settle before teardown continues. */ | ||
| private async waitForHostRequestsToSettle(tasks: Promise<void>[]): Promise<void> { |
There was a problem hiding this comment.
🟠 High kernel/index.ts:1458
waitForHostRequestsToSettle now calls Promise.allSettled(tasks) with no timeout, so shutdown(), kill(), restart(), and dispose() can hang indefinitely when an in-flight host handler does not settle. Aborting the controller does not guarantee settlement because existing handlers (e.g. the rlm.run handler) do not consume HostRequestContext.signal. This is especially problematic in kill(), which awaits host requests before sending SIGKILL, so a stuck handler blocks the forced termination. Consider reintroducing a bounded timeout so teardown cannot hang forever.
🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file @packages/coding-agent/src/core/kernel/index.ts around line 1458:
`waitForHostRequestsToSettle` now calls `Promise.allSettled(tasks)` with no timeout, so `shutdown()`, `kill()`, `restart()`, and `dispose()` can hang indefinitely when an in-flight host handler does not settle. Aborting the controller does not guarantee settlement because existing handlers (e.g. the `rlm.run` handler) do not consume `HostRequestContext.signal`. This is especially problematic in `kill()`, which awaits host requests *before* sending `SIGKILL`, so a stuck handler blocks the forced termination. Consider reintroducing a bounded timeout so teardown cannot hang forever.
| // dead worker's registration is never stranded permanently. Each attempt | ||
| // bumps the worker's stopRevision, so rescission is detected through the | ||
| // registration and tombstone instead of the waiting-phase snapshot. | ||
| const isCleanupStillWanted = isStopGenerationCurrent; |
There was a problem hiding this comment.
🟡 Medium daemon/daemon-supervisor.ts:5196
finalizeTimedOutWorkerStop strands a dead worker's tombstoned descriptor after a single failed cleanup attempt. The retry loop captures stopRevision once at the top, but each call to stopWorker increments worker.stopRevision in stopWorkerUntracked. When the first cleanup attempt fails (e.g. during catalog archival), the catch block delays and then isCleanupStillWanted() compares the current worker.stopRevision against the stale captured value, finds them unequal, and exits the loop — so the retry never happens. Consider capturing a stop-generation snapshot that accounts for the increment (e.g. check worker.stopRequestedAt === stopRequestedAt without binding to the original stopRevision, or re-snapshot the revision after each stopWorker call).
🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file @packages/coding-agent/src/modes/daemon/daemon-supervisor.ts around line 5196:
`finalizeTimedOutWorkerStop` strands a dead worker's tombstoned descriptor after a single failed cleanup attempt. The retry loop captures `stopRevision` once at the top, but each call to `stopWorker` increments `worker.stopRevision` in `stopWorkerUntracked`. When the first cleanup attempt fails (e.g. during catalog archival), the catch block delays and then `isCleanupStillWanted()` compares the *current* `worker.stopRevision` against the stale captured value, finds them unequal, and exits the loop — so the retry never happens. Consider capturing a stop-generation snapshot that accounts for the increment (e.g. check `worker.stopRequestedAt === stopRequestedAt` without binding to the original `stopRevision`, or re-snapshot the revision after each `stopWorker` call).
| } | ||
| const orphanProcessJournalPath = worker.descriptor.orphanProcessJournalPath; | ||
| const orphanProcessJournalPath = descriptor.orphanProcessJournalPath; | ||
| if (orphanProcessJournalPath) { |
There was a problem hiding this comment.
🟠 High daemon/daemon-supervisor.ts:3083
The broad catch block at the orphan-reaping loop also swallows ownership/tuple fencing failures from the newly added assertCurrentOwnership() and assertWorkerTupleCurrent() checks. If supervisor ownership is lost or the worker tuple changes during those checks, recovery merely logs the error and proceeds to mutate the recovery journal and mark operations recovered — even though this supervisor no longer owns the worker. Re-throw generation/tuple fencing failures instead of treating them as ordinary orphan cleanup errors so recovery is aborted when ownership is lost.
🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file @packages/coding-agent/src/modes/daemon/daemon-supervisor.ts around line 3083:
The broad `catch` block at the orphan-reaping loop also swallows ownership/tuple fencing failures from the newly added `assertCurrentOwnership()` and `assertWorkerTupleCurrent()` checks. If supervisor ownership is lost or the worker tuple changes during those checks, recovery merely logs the error and proceeds to mutate the recovery journal and mark operations recovered — even though this supervisor no longer owns the worker. Re-throw generation/tuple fencing failures instead of treating them as ordinary orphan cleanup errors so recovery is aborted when ownership is lost.
| } | ||
| this.handledHostRequestCommIds.add(commId); | ||
|
|
||
| const callerSignal = this.activeExecution?.opts.signal; |
There was a problem hiding this comment.
🟠 High kernel/index.ts:1294
A detached host request (emitted after its spawning cell went idle) can arrive on the comm channel while a later, unrelated cell is executing. startHostRequestFromComm wires the request's abort signal to this.activeExecution?.opts.signal — the currently executing cell — without verifying the comm message's parent request ID belongs to that execution. Aborting the later cell then aborts the detached request, and isHostRequestCurrent() suppresses its reply, so the kernel never receives a response. The lastCellCode fallback in handleHostRequest explicitly supports detached requests, so this cross-cell cancellation is reachable. Consider deriving callerSignal only when the comm's parent_header.msg_id matches the active execution's requestMsgId, and leaving it undefined otherwise.
| const callerSignal = this.activeExecution?.opts.signal; | |
| const parentMessageId = (incoming.parent_header as { msg_id?: string }).msg_id; | |
| const callerSignal = | |
| this.activeExecution?.requestMsgId === parentMessageId | |
| ? this.activeExecution?.opts.signal | |
| : undefined; |
🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file @packages/coding-agent/src/core/kernel/index.ts around line 1294:
A detached host request (emitted after its spawning cell went idle) can arrive on the comm channel while a later, unrelated cell is executing. `startHostRequestFromComm` wires the request's abort signal to `this.activeExecution?.opts.signal` — the *currently* executing cell — without verifying the comm message's parent request ID belongs to that execution. Aborting the later cell then aborts the detached request, and `isHostRequestCurrent()` suppresses its reply, so the kernel never receives a response. The `lastCellCode` fallback in `handleHostRequest` explicitly supports detached requests, so this cross-cell cancellation is reachable. Consider deriving `callerSignal` only when the comm's `parent_header.msg_id` matches the active execution's `requestMsgId`, and leaving it `undefined` otherwise.
There was a problem hiding this comment.
🟡 Medium
reapWorkerResources sets retainOrphanJournal = true when an orphan's identity changes during reaping, but the finally block at line 485 unconditionally calls clearOrphanProcessJournal(orphanProcessJournalPath), deleting the retained entries. This defeats the retention logic: orphans whose identity shifted mid-reap are never recorded for later cleanup, leaving detached resources orphaned. Additionally, during RPC recovery each worker reap initializes retainOrphanJournal to false, so a later worker with no identity mismatch clears entries an earlier reap deliberately retained. Consider either skipping clearOrphanProcessJournal in finally when retention is flagged, or removing only the reaped owner's records instead of clearing the entire shared journal.
🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file @packages/coding-agent/src/cli/owned-session-worker.ts around line 484:
`reapWorkerResources` sets `retainOrphanJournal = true` when an orphan's identity changes during reaping, but the `finally` block at line 485 unconditionally calls `clearOrphanProcessJournal(orphanProcessJournalPath)`, deleting the retained entries. This defeats the retention logic: orphans whose identity shifted mid-reap are never recorded for later cleanup, leaving detached resources orphaned. Additionally, during RPC recovery each worker reap initializes `retainOrphanJournal` to `false`, so a later worker with no identity mismatch clears entries an earlier reap deliberately retained. Consider either skipping `clearOrphanProcessJournal` in `finally` when retention is flagged, or removing only the reaped owner's records instead of clearing the entire shared journal.
| } | ||
|
|
||
| private deleteWorkerDescriptor(worker: ResidentWorker): void { | ||
| /** Remove a registration only while this generation still owns its exact worker incarnation. */ |
There was a problem hiding this comment.
🟠 High daemon/daemon-supervisor.ts:1011
deleteWorkerDescriptor now asserts assertWorkerReclaimCommitCurrent before unlinking the worker descriptor. On a normal worker-initiated shutdown, the root session_closed(reason: "shutdown") path calls deleteWorkerDescriptor while the worker process is still alive — the process hasn't exited yet, so the reclaimability check fails and the descriptor is never removed. Since intentionalStop is already true, handleWorkerClose returns immediately when the socket closes, leaving the worker and its descriptor permanently registered for this supervisor generation. This strands sessions and blocks normal cleanup.
The assertion should not require the process to be gone at this point. Consider deferring descriptor deletion until process exit, or finalizing the stop asynchronously rather than requiring reclaimability synchronously when the session_closed frame arrives.
🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file @packages/coding-agent/src/modes/daemon/daemon-supervisor.ts around line 1011:
`deleteWorkerDescriptor` now asserts `assertWorkerReclaimCommitCurrent` before unlinking the worker descriptor. On a normal worker-initiated shutdown, the root `session_closed(reason: "shutdown")` path calls `deleteWorkerDescriptor` while the worker process is still alive — the process hasn't exited yet, so the reclaimability check fails and the descriptor is never removed. Since `intentionalStop` is already `true`, `handleWorkerClose` returns immediately when the socket closes, leaving the worker and its descriptor permanently registered for this supervisor generation. This strands sessions and blocks normal cleanup.
The assertion should not require the process to be gone at this point. Consider deferring descriptor deletion until process exit, or finalizing the stop asynchronously rather than requiring reclaimability synchronously when the `session_closed` frame arrives.
2 participants
Links #1182
This is the reviewed, coherent MCP-prerequisite substack only; it is not complete Core and is not human-ready.
Lineage: trust → C06 → R01, at head
35c71c45e6f5e21d91ad2793f24b30f329f3a897(35c).Reviewed evidence: monitor 80/80 and matrix 141/1.
Before integration, upstream and post-merge replay are required. Remaining blockers are the C01–C08 integration blockers; this substack does not resolve them.
Note
Bind project trust authority, catalog-driven agent family reachability, and typed kernel host requests
createMcpProjectTrustAuthorityin project-trust-authority.ts, an authority that canonicalizes an allowlist of project directories, issues opaque bindings, and validates them with strict symlink/alias rejection.AgentFamilyCatalogEntry[]snapshot assembled from saved sessions, passive subagents, and in-memory states; all ACL surfaces (observe, message send, sibling checks) now evaluate against this snapshot.HostRequestContext(requestId, AbortSignal,isCurrent()) to typed kernel host-request handlers in kernel/index.ts, with payload size/depth validation, per-comm revocation on close, and at-most-once reply semantics.stoppingworker state, allowlist-only env persistence for resident workers, and single-flight resident recovery.processIdExists,isZombieProcess,isProcessAlive) in child-process.ts.DAEMON_SCHEMA_REVISIONbumped to 15; resident workers now require a fresh client-supplied env for recovery after a dead process, so automatic recovery without a reconnecting client will stall withlifecycle='failed'.📊 Macroscope summarized 35c71c4. 19 files reviewed, 0 issues evaluated, 0 issues filtered, 0 comments posted
🗂️ Filtered Issues
No issues evaluated.