feat: add supervised pause and human takeover#59
Merged
Conversation
Add a leased, fail-closed human takeover for desktop-capable sessions. `picklab watch --control` atomically acquires a TTL/heartbeat human lease, drains in-flight agent permits before granting control, switches the session's VNC to writable for the lease's duration, and hands control back with a fresh screenshot and evidence record on every exit path (return, cancellation, failed renewal, or crash recovery on the next VNC touch). Core (packages/core/src/takeover.ts): atomic O_EXCL lease file, agent permit files, withAgentPermit's fail-closed acquire/recheck/execute/ release protocol, staleness (identity-or-TTL), and takeover evidence recording. Desktop-linux (packages/desktop-linux/src/takeover.ts): VNC mode switching (read-only <-> writable) scoped to the lease, crash recovery wired into ensureSessionVnc so a stale writable VNC self-heals instead of blocking watch. Browser: the DevTools MCP relay gained a generic NDJSON intercept hook so a blocked tools/call is answered with a stable busy error directly, never forwarded to the child process. Desktop input (CLI + MCP): all seven input operations now go through withAgentPermit, so no permit or an active lease means no input delivery. New picklab takeover status / takeover_status expose read-only lease state; request_user_input's secret guidance now points at --control instead of the uncoordinated --vnc-control.
…ritable VNC Panel-adjudicated fixes to the human-takeover invariant (writable VNC must never outlive its lease), addressing two P0 violations and a P1 TOCTOU: - renewHumanLease refuses to extend a lease that has already gone stale by TTL, even for its own live owner, so a straggling renewal can never resurrect a lease a recovery is already reclaiming (P0-B). - recoverStaleTakeoverLocked re-reads and re-checks staleness immediately before the destructive VNC stop, and releases via compare-and-delete on the exact raw bytes captured at that check (not just leaseId, which a renewal never changes) — closing the window where an unlocked renewal between the initial check and the stop could make recovery kill a now-live takeover's VNC (P1-C). - watch --control now holds three independent mechanisms so writable VNC cannot outlive its lease in wall-clock terms, not only "the next time something happens to touch the session": it ends the takeover immediately on the first failed heartbeat renewal rather than waiting for the viewer to close; a hard deadline timer, rescheduled on every successful renewal, force-ends it if wall-clock time ever passes expiresAt; and a detached watchdog process (spawned via a new hidden `picklab internal takeover-watchdog` command, immune to a SIGKILL of its parent) independently polls the lease and reclaims a stale writable VNC on its own — proven against a genuinely separate OS process, not just an in-process mock (P0-A). A detached process was chosen over OS-level parent-death coupling (e.g. Linux PR_SET_PDEATHSIG) because the latter has no portable Node.js API without a native addon or an external wrapper binary. renewHumanTakeover's return type changes from boolean to the renewed lease (or undefined), needed to read the fresh expiresAt for the deadline-timer reschedule.
…ard pumps The takeover busy-intercept path and the normal child-response pump are two independent concurrent producers that can both write to the same client-facing output stream (the intercept answers a blocked tools/call directly on it; the forward pump also writes upstream responses to it). Add createJsonRpcWriteQueue, a small write-ordering queue, and thread it through pumpJsonRpcNdjson as writeSerializer / interceptWriteSerializer so every write either pump issues to a shared destination is fully ordered through one explicit gateway rather than left as an incidental property of whichever Writable happens to be passed in. Wired into runDevtoolsMcpRelay so a busy rejection racing an in-flight forwarded response can never interleave on the wire.
…rmit Security adjudication: launchApp spawns a new client onto the shared X11 display a human may be actively controlling under a takeover lease. On bare Xvfb a new client can grab input focus, which is exactly what the lease exists to prevent when a human is mid-entry of a secret. Wrap the launchApp call in withAgentPermit at both the CLI (picklab desktop launch) and MCP (desktop_launch) call sites, symmetric with the seven desktop input tools. desktop_screenshot remains ungated (read-only, delivers no input).
…ch gating) Update the README's takeover section and security model bullet for the active (not lazy-only) crash-path VNC reclaim, the immediate-end and hard-deadline-timer mechanisms, and desktop_launch joining the gated tool set with desktop_screenshot as the only ungated one. Record the same in docs/releases/UNRELEASED.md, plus the evidencing asymmetry between a blocked MCP desktop tool call (evidenced) and a blocked DevTools relay request (not — no equivalent per-call evidence lifecycle to hook).
12 tasks
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
1 participant
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.
Summary
Implements #21 — supervised pause and human takeover for desktop sessions:
wx-created lease (human.lease.json) with 30s TTL / 5s heartbeat, single-winner acquisition proven by a real two-OS-process race test; agent permits with acquire→recheck→execute→release-in-finally, fail-closed on any lease appearance-viewonlyflip only; reverted on every exit path (return/timeout/cancel), with three layered wall-clock guarantees — immediate end on failed renew, hard deadline timer atexpiresAt, and a detached identity-scoped watchdog process that survivesSIGKILLof the controller and reclaims via the TOCTOU-hardened recover (re-read + exact-raw-bytes compare-and-delete under the VNC lock)renewHumanLeaserefuses stale leases (no resurrection after agents unblock); every agent input route permit-gated incl.desktop_launch(security adjudication: a launched client can steal focus on bare Xvfb during secret entry);desktop_screenshotis the only ungated desktop tool (read-only)takeover_start/takeover_{return,timeout,cancelled,recovered})picklab watch --control(spec-literal) + read-onlytakeover status/ MCPtakeover_status; structured resume summary + fresh screenshot on returnRefs #21 — issue stays open for the one recorded end-to-end hardware takeover proof on Elberte-PC (RC phase; also reconciles parent #17 then). No feature flag:
--controlis per-invocation opt-in.Validation
Local review: serious risk class — Grok 4.5 (correctness/races: 2 P0s found and fixed), Opus 4.8 (security/trust: desktop_launch P1 found and fixed; full failure-matrix fix-verification clean), Sonnet 5 (conformance: clause-by-clause table, clean); KISS keep ×3.