feat(cli): kild watch — the wake path a turn-end hook cannot provide - #677
Merged
Conversation
The Stop hook fires when a turn ENDS. A harness sitting idle is therefore unreachable: mail queues, nothing wakes it, and it finds out whenever its operator next happens to prompt. Both attached agents hit this today — each sat on the other's messages believing comms were fine, and the operator had to say "check your inbox" to break it. No hook can fix it. Every harness hook event fires on a turn, tool or session edge and none ticks while a session is idle, so the wake has to come from outside the hook system. kild cannot push either — not owning the process is what makes an attached agent attached. So kild owns the WAITING and the harness owns the WAKING. `kild watch` blocks until somebody else speaks and exits; whatever background facility the harness has runs it and reacts to that exit. kild learns nothing about the harness. It reads the message LOG, never the inbox. Non-destructive by construction, so a watcher on a timer can never eat what the Stop hook exists to deliver: watch is the wake signal, inbox is the delivery. That also means it needs nothing new from the engine — a cursor, which /messages?since= has always served — so it works against any engine already on main rather than requiring a lockstep upgrade. Credit to helm's agent for that correction; I was about to build it on a pending count it does not need. EXIT CODES ARE THE INTERFACE, and a quiet engine is not a dead one: 0 mail 1 usage 2 quiet (window elapsed) 3 unreachable If quiet and unreachable both exited 0, every harness built on this would inherit the failure the command exists to remove — a wake mechanism that goes silent and is indistinguishable from one with nothing to report. Not retrofittable once harnesses depend on it, so it is decided now. Two bugs found by running it rather than testing it: - The initial cursor fetch was unguarded, so a dead engine at startup exited 1 (usage) instead of 3. A harness could not tell "the engine is gone" from "you typed it wrong" — the exact distinction the codes exist for, defeated at the moment it matters most. Every existing test passed --since, which skips that call, so they all walked straight past it. Now covered. - Timing constants were const declarations in cli.ts, which calls dispatch() at the top of the module — the same temporal-dead-zone failure already paid for once in this codebase. They live in the imported slice now, where evaluation order makes it impossible rather than merely fixed. Class, not instance. Gates: 432 tests, typecheck, lint, e2e 70/70. Dogfooded live against the running engine: a quiet window exits 2, a dead engine exits 3, and a watcher started with no arguments resolved its own kild and woke on a real reply.
…ferred The obvious wrong assumption about a wake verb is that it also marks-as-seen. A harness built on that would drop every message it was woken for and look entirely correct doing it — the wake fires, the harness treats it as delivery, and the mail sits in an inbox nobody drains. Non-destructiveness was in the docstring and the design, but not in anything a caller actually reads. Now the usage text, the human output and the --json shape all say it: 'nothing consumed — still in your inbox', consumed: false, and a pointer at `kild inbox` for the drain. Raised by helm's agent before wiring against it, which is the right time to settle a contract.
Review found the tolerance loop did not tolerate anything. A failed poll hit `continue`, which jumped past both the deadline check and the sleep — those only ran on the success path — so three "tolerated" failures burned in about eight milliseconds. The constant's own comment says a restart "takes a moment and must not be reported as a dead engine"; the code gave it no moment. Any real restart was declared unreachable, which is the tolerance inverted into exactly the bug it exists to prevent. The bootstrap fetch had the opposite half of the same problem: no tolerance at all, so one blip at startup — in the common no-`--since` shape — exited unreachable immediately. Two copies of one decision, drifted in opposite directions. There is now a single `poll()` both call sites use. Timing is the only thing that separates the fixed loop from the broken one, so the regression test asserts elapsed time, and `--interval` makes that testable without a ten-second test. It is a real knob besides: a harness that wants a snappier wake should not have to fork the CLI for it. Two smaller findings, both the "assert around the risky path" shape: - The `read with` hint was computed as `cursor - incoming.length`, which assumes the new messages are the highest-numbered in the batch. One own message with a higher seq made the printed command skip the very message that woke the watcher — a wrong answer with the confidence of a working command. It now derives from the oldest incoming seq. - `pollResult` filtered own messages but not `seq <= cursor`, and the test that fed it a stale seq asserted only the cursor, never `incoming`. `since` is documented exclusive, so it should not happen — but relying on that means one server-side slip re-delivers a seen message as fresh mail forever. One comparison makes the function correct on its own terms. Gates: 438 tests, typecheck, lint, e2e 70/70. Dogfooded: a dead engine now takes 837ms to declare unreachable rather than 8ms, and still exits 3.
Verification found the previous fix introduced a new bug of the same shape as the one it fixed. Extracting `poll()` unified how much transient failure is tolerated, but the DEADLINE check stayed inline at each call site — and only the loop's copy got it. The bootstrap fetch, which is the default no-`--since` path, would sleep a full interval on a blip with no regard for the window: a one-second request took six seconds, and the message then reported "nothing new in 1s" after six. One decision, two copies, one updated. Twice now. So the decision has one home. `waitForNextPoll()` answers "is there room for another attempt", and both loops ask it. `expire()` is the single quiet exit, and it reports the time that actually elapsed rather than the timeout that was requested — printing the request made the message a lie in exactly the case worth reporting. The helpers are function declarations rather than const arrows. TypeScript only treats a never-returning call as terminating when it can see the declaration, and hoisting keeps them clear of the temporal dead zone this file has been bitten by twice today. The coverage gap that let it through is closed: nothing asserted bootstrap timing at all, which is why "unify the failure decision" looked complete. There is now a test that fails if a blip during bootstrap overruns the window, and one that pins the elapsed-time message. Writing that second test also corrected me: a dead engine with a short interval exhausts tolerance BEFORE the window closes, so it exits `unreachable`, not `quiet`. My first version of the test asserted otherwise and was wrong; the code was right. Gates: 440 tests, typecheck, lint, e2e 70/70. Dogfooded the exact reported case — 1s window, 6s interval, dead engine — now 25ms instead of 6066ms.
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.
The Stop hook fires when a turn ends. A harness sitting idle is therefore unreachable: mail queues, nothing wakes it, and it finds out whenever its operator next happens to prompt.
Both attached agents hit this today. Each sat on the other's messages believing comms were fine, and it took the operator saying "check your inbox" to break it.
Why no hook can fix it
Every harness hook event fires on a turn, tool, or session edge. None ticks while a session is idle, so the wake has to come from outside the hook system entirely. And kild cannot push either — not owning the process is what makes an attached agent attached.
So kild owns the waiting and the harness owns the waking.
kild watchblocks until somebody else speaks, then exits; whatever background facility the harness has runs it and reacts to that exit. kild learns nothing about the harness, and the harness needs to know nothing about kild's storage.It reads the log, never the inbox
Non-destructive by construction, so a watcher running on a timer can never eat what the Stop hook exists to deliver.
watchis the wake signal;inboxis the delivery.That also means it needs nothing new from the engine — a cursor, which
/messages?since=has always served — so it works against any engine already onmainrather than requiring a lockstep upgrade.Credit to helm's agent for that correction: I was about to build this on a new pending-count endpoint it does not need. Those are two different questions —
GET /messages?since=<seq>watchonly needs the first.Exit codes are the interface
A quiet engine and a dead one must never look alike. If both exited 0, every harness built on this would inherit the exact failure the command exists to remove — a wake mechanism that goes silent and is indistinguishable from one that simply had nothing to report. Not retrofittable once harnesses depend on the behaviour, so it is decided now, first.
Two bugs found by running it, not by testing it
The initial cursor fetch was unguarded, so a dead engine at startup exited
1(usage) instead of3. A harness could not tell "the engine is gone" from "you typed it wrong" — the distinction the codes exist for, defeated at the moment it matters most.The instructive part: every test passed
--since, which skips that call entirely. All of them walked straight past the only unguarded path. Green suite, real hole. Now covered by a test that deliberately omits--since.Timing constants were
constdeclarations incli.ts, which callsdispatch()at the top of the module — the same temporal-dead-zone failure already paid for once in this codebase, reintroduced hours later. They live in the imported slice now, where evaluation order makes it impossible rather than merely fixed. Class, not instance.Gates
bun testbun run typecheckbun run lint./scripts/e2e.shDogfooded live against the running engine: a quiet window exits 2, a dead engine exits 3 (with and without
--since), and a watcher started with no arguments resolved its own kild and woke on a real reply.