diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index a3ffe59cf..af82a5fb9 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -492,6 +492,10 @@ jobs: # test_semantic_fallback_169.py and test_doctor.py cover #634's # sqlite-vec install-and-backfill fix; test_wiki_maintenance.py and # test_wiki_backlog_pass.py cover #636's consolidate-on-SQLite fix. + # The three capture files join per issue #659: here, and only here, + # capture_peer.is_supported() is false natively, msvcrt takes the real + # drainer lock and the detached child runs with the real Windows + # creationflags. source: ADR-1094 - name: Run portability + backend tests env: HF_HUB_OFFLINE: "1" @@ -510,6 +514,9 @@ jobs: tests_py/test_doctor.py tests_py/handlers/consolidation/test_wiki_maintenance.py tests_py/handlers/consolidation/test_wiki_backlog_pass.py + tests_py/hooks/test_capture_unsupported_platform.py + tests_py/hooks/test_capture_drain.py + tests_py/infrastructure/test_capture_spool.py # source: ADR-0802 diff --git a/docs/adr/ADR-1094-spool-captures-for-a-detached-drainer-where-the-resident-worker-cannot-run.md b/docs/adr/ADR-1094-spool-captures-for-a-detached-drainer-where-the-resident-worker-cannot-run.md new file mode 100644 index 000000000..a1eb5de97 --- /dev/null +++ b/docs/adr/ADR-1094-spool-captures-for-a-detached-drainer-where-the-resident-worker-cannot-run.md @@ -0,0 +1,105 @@ + + +--- +created: 2026-10-07T00:00:00Z +kind: adr +number: 1094 +status: accepted +tags: [capture, windows, hooks, issue-659] +title: Spool captures for a detached drainer where the resident worker cannot run +--- +# ADR-1094: Spool captures for a detached drainer where the resident worker cannot run + +## Status + +accepted + +## Context + +Issue #659: since 17370a61 every PostToolUse capture goes through a resident +worker that needs `AF_UNIX`, `fcntl`, `geteuid` and `Popen(pass_fds)`. CPython on +Windows has none of them, `capture_peer.supported()` raised, and the hook +reported `capture_skipped` for every event: 1,730 attempts, zero memories, with +no visible signal (reporter's `telemetry.jsonl`, Windows 11, 4.23.1). + +The first fix (PR #663 at baa8ece9) stored in the hook process on such platforms. +Measured on 2026-10-07 (macOS, isolated SQLite store, warm): `handlers/remember.py` +imports `sentence_transformers` in 3.61 s (`torch` 0.72 s, weights 0.10 s, one +encode 0.01 s); the whole in-process capture took 4.4 s warm and 10.5 s cold. The +hook has `timeout: 10` (`.claude-plugin/plugin.json`), and the host aborts a hook +that exceeds it ("PostToolUse hook timed out (per-hook abort)", Claude Code +2.1.292). The resident worker (ADR-0486, ADR-0508) exists precisely to pay that +import once; the in-process path put it back on the synchronous path of every +event, so the capture would be killed at the limit instead of refused. + +## Decision + +Where `capture_peer.is_supported()` is false the hook does not load the handler. +It validates the payload (`validate_payload`), writes it unchanged and atomically +to `/.capture-worker/spool/` (`infrastructure/capture_spool.py`) and +starts `python -m mcp_server.hooks.capture_drain` without waiting. POSIX is +unchanged (the resident worker). + +- The spawn options are a pure function, `capture_dispatch.popen_options(platform, stream)`: + Windows `creationflags = DETACHED_PROCESS | CREATE_NEW_PROCESS_GROUP`, + `close_fds`, stdin to `DEVNULL`, stdout and stderr to `stream`, the drainer's one + log `drain.log` (owner-only mode, size-rotated at each spawn by + `open_rotating_log`, ADR-0655; the drainer's logging goes to its stderr, so a + crash before logging exists and library progress bars land in the same bounded + file), the launcher's environment; no `pass_fds` and no `start_new_session` + (CPython ignores it on Windows). Other platforms use `start_new_session`. +- The drainer takes a non-blocking kernel lock (`msvcrt.locking` `LK_NBLCK` / + `flock` `LOCK_NB`; the pattern of the Codex SessionEnd queue, ADR-1084). A loser + exits at once without importing the handler. The winner stores every pending + file through the same `capture_store.store` the worker awaits, on one event loop + (one model load per burst), then releases the lock and scans once more. A file + written between its last scan and the release has a drainer that lost the lock + and exited; the rescan stores it. +- Delivery is at-least-once: a file is deleted after `store` returned; a replay after + a crash is absorbed by the write gate. A refused, unreadable or failing file is + renamed `*.rejected` and reported through `report_failure("capture_skipped")`; + nothing is swallowed. If the rename itself fails the file is deleted after the + report (a payload already refused is not replayed by every later drainer); if + that fails too it is skipped for the run. +- Bounded and observable. Each store is cancelled after `STORE_SECONDS` (120 s, + about ten times the measured cold capture) and the file refused like any + failure, so a hung handler cannot hold the lock for good. A hang that does not + yield to cancellation (a blocking call) is covered by the hook: when the oldest + pending file is older than `STALL_SECONDS` (600 s) it reports + `capture_spool_stalled` through telemetry on every capture, and still queues the + new payload. The drainer removes `.partial` files older than an hour (a hook that + died between write and rename) and `.rejected` files older than a week. +- Order is not a contract. Captures are independent writes to a write-gated store, so + the drainer processes files in name order only for determinism; the name's clock + prefix can tie or invert (Windows clock resolution is about 15.6 ms). Age is + therefore taken from the files' own `mtime` (the oldest of all pending files), + never from the name. +- The directory is private under the configured Cortex root (mode 0700 where the + platform has modes; on Windows it inherits the user-profile ACL). + +## Rejected + +- The PR as first written (store in the hook): 4.4 to 10.5 s against a 10 s budget. +- One process per event with no spool: N imports of `torch`. +- Algorithmic embedding re-embedded at `consolidate`: `sqlite_store_search.py` + isolates embedding spaces and the write gate is blind to neural rows. +- `async: true` on the hook: every platform, still one `torch` import per event. +- Named pipe with a current-user DACL: a second transport, lease and spawn stack. +- Spool on every platform: it would change the POSIX capture path (ADR-0486, + ADR-0508), outside this issue. + +## Consequences + +Positive: the hook returns in milliseconds on Windows (0.06 s measured for the hook +body on macOS with the Windows branch simulated); one model load serves a burst; +`ci.yml`'s Windows leg runs the capture test files, so `is_supported()` false +natively, the real `msvcrt` lock and the real detached spawn are exercised. + +Negative: a capture is persisted after the hook returns, not before; a drainer +killed by the host's process-tree cleanup leaves the file for the next event's +drainer. `CREATE_BREAKAWAY_FROM_JOB` is not requested: it fails when the host's job +object forbids breakaway. Whether the host destroys the tree on abort is unobserved, +since the hook no longer approaches its limit. The reporter's check, a +`telemetry.jsonl` with `remember` samples and no `capture_skipped`, is the +signal that closes #659's capture part. `check_setup` answering `ready: true` while +capture is broken is a separate defect (#660). diff --git a/docs/provenance/capture-worker-design.md b/docs/provenance/capture-worker-design.md index 5082ab3a1..6447d9f2b 100644 --- a/docs/provenance/capture-worker-design.md +++ b/docs/provenance/capture-worker-design.md @@ -44,15 +44,33 @@ lose accepted work. There is no automatic replay after uncertain delivery, which could duplicate a write. Durable delivery is outside this change. Absent/refused socket triggers one launch attempt. Spawn, transport, malformed -frame, unsupported platform and admission-deadline failures produce a diagnostic and a -`capture_skipped` telemetry sample; inference never falls back into the hook. +frame and admission-deadline failures produce a diagnostic and a +`capture_skipped` telemetry sample; a failed worker never falls back into the hook. Dispatch, receive and processing errors report their measured monotonic duration. Listener/cleanup failures use a distinct `capture_worker_lifecycle` sample with their own measured duration. Neither event is recorded as `remember`, so these diagnostics do not enter the handler's `remember` latency series. -Worker handler failure has the same observable error boundary. Windows is -explicitly unsupported because this implementation requires Unix credentials, -Unix socket paths and `flock`; no TCP or abstract-socket fallback exists. +Worker handler failure has the same observable error boundary. A platform the worker +cannot run on (Windows: CPython has no `AF_UNIX`, no `fcntl`, no `geteuid`, and +`Popen` has no `pass_fds`) is decided by the capability test +`capture_peer.is_supported()`, before any I/O, not by catching an error. There the +hook never loads the handler: importing `sentence_transformers` alone measured +3.61 s on 2026-10-07 (macOS, isolated SQLite store, `torch` 0.72 s, warm), against a +hook `timeout` of 10 s. It validates the payload, writes it atomically to +`.capture-worker/spool/