(triggers): wait for the CLI descriptor before every chain step (#407, #360) - #414
Conversation
…#360) Step 0 of a chain landed in the composer with its Enter absorbed while background subagents were running: the readiness wait only covered the step after /compact. It now runs before every step, never writes into a dialog, and falls back to the old behaviour without a usable descriptor. The busy-fall wait also takes the descriptor as authority: idle after the step's Enter ends it even when the terminal-derived busy flag is stuck, which timed out a chain on an idle session. Closes #360 Refs #407
|
Reviewing |
|
Adversarial review at Blocker: the #407 case itself is not covered.
Required:
Minor:
Local: the 4 trigger suites pass (157 pass, 0 fail, 1 skip). The new tests fail 4/5 against d9baecd. |
…360) The readiness wait wrote the step anyway after 60 s, so a parent whose descriptor stays busy while delegated agents run still got text typed into a busy composer, and the chain then typed the next step into the same composer. The wait now runs to the step deadline and a step is written only on idle; busy, waiting and unknown statuses fail the step with a reason. A step that stays unconfirmed with the recovery Enter withheld stops the chain. Post-compact readiness is anchored on the compact's Enter, and waiting is judged over the final settle window. Refs #360 Refs #407
|
Reviewing |
devsuitup
left a comment
There was a problem hiding this comment.
Re-review at 02e872f (delta from 7b8f633): changes requested — 2 blocking, 1 major.
- B1
trigger-watcher.js:1295: if the descriptor vanishes during the readiness wait (available:false: delete-and-rewrite, failed pid probe, momentary non-integer stamp), the step is still written. Reproduced:busyfor 500 ms, then no descriptor → text written at 562 ms,ok: true. Descriptor lost after it was read must keep waiting or fail "not idle"; only a descriptor absent at the start takes the legacy path. - B2
trigger-watcher.js:414: thereadybranch is checked before the deadline, so a settle finishing after the deadline returns ready and the step is written (reproduced: ready 38 ms past the deadline, text in the composer,chain timeout). Not ready past the deadline; re-check the deadline right before the write. - M1: the post-compact
enterAtanchor is untested — reverting it tostepSentAtleaves every test green, and that is the original #407 failure (pre-compact idle accepted). - Non-blocking: the
step not confirmeddoc row and the CHANGELOG omit the pending-input /waitingcases; commit7b8f633still saysCloses #360(squash message must be Refs-only); the multi-paragraph JSDoc ofwaitForCliIdleAfterbelongs in the context doc.
…st the step deadline (#407, #360) A descriptor lost after it had been read ended the readiness wait and let the step be typed into a CLI last seen busy. It now counts as not idle until it reappears or the deadline passes. A settle completing at or after the deadline no longer reads as ready, and the deadline is checked again right before the write. Refs #360 Refs #407
|
Reviewing |
devsuitup
left a comment
There was a problem hiding this comment.
Re-review at 399e89c (delta from 02e872f): 0 blocking. B1 (vanished descriptor), B2 (ready past the deadline) and M1 (post-compact enterAt anchor) are fixed: both repros now write nothing, and each guard has a test that goes red under mutation (173 trigger tests, 0 fail). One remaining narrow gap of the B1 class, being fixed: precheck read OK, then the first wait sample unusable → falls through to the write (trigger-watcher.js:1281-1294). Live test against a real CLI with background agents is running.
…407, #360) A separate precheck read could succeed and the wait's own first read then fail, which sent the step down the no-descriptor path and wrote it without any idle confirmation. The precheck is gone: once any read succeeded, a later failure is "not idle" and the wait goes on to the deadline. Refs #360 Refs #407
devsuitup
left a comment
There was a problem hiding this comment.
Re-review at 34786f8 (delta from 399e89c): 0 blocking. The separate precheck read is gone, so waitForCliIdleAfter owns the first read. Once one read has succeeded, a later failure counts as not idle until the deadline. The new test fails when the precheck is put back. The CHANGELOG states the change for users without a readable descriptor. Nit: the bare { … } block left at trigger-watcher.js:1282 could be dedented. Waiting for CI and the live test.
|
Live test at
|
# Conflicts: # CHANGELOG.md
A chain step is no longer typed while the CLI's own descriptor reads anything but
idle: it waits up to the step's deadline, then fails cleanly with a reason. This applies to every step, step 0 included, and the busy-fall wait takes the descriptor as an authority.What changed
trigger-watcher.js: before every chain step,waitForCliIdleAfterwaits for descriptoridleheld for the settle window with an unchangedstatusUpdatedAt. The wait is bounded by the step's deadline only (the 60 sSWITCHBOARD_CLI_READY_WAIT_MScap is removed, since a parent with delegated agents readsbusyfor minutes). At the deadline,busy,waitingor any unknown status (e.g.shell) means the step is not written:not sentfor step 0,chain timeoutafter, with areason(dialog open, turn still running, never idle). A dialog is reported whenwaitingwas sampled anywhere in the final settle window.ok: false,error: "step not confirmed", nothing more typed.waitForBusyFallends on descriptoridleat or after the step's Enter (held for the settle window), even if_cliBusyis stuck. On sessions with background agents the descriptor stays busy until the last agent ends, so this is not measured as fixed there..ai/contexts/trigger-watcher.md(the absorbed-Enter mechanism is marked not established; single triggers have the same exposure and are not addressed),docs/automation.md(new error value, env var row removed). CHANGELOG: one line.Tests (
test/trigger-every-step-readiness.test.js,test/trigger-descriptor-proof.test.js)not sent); a later step held to the deadline (chain timeout);waitingnever written; step not confirmed with recovery withheld stops the chain; (triggers): chain times out after step 0 because the busy flag stays up on an idle session #360 stuck_cliBusy; no descriptor unchanged.statusUpdatedAt, dialog seen in the final window, unknown status not idle, busy-fall settle.statusUpdatedAtreset removed;waitingjudged on the last sample only; unknown status treated as idle; busy-fall settle forced to 0.task check: 2943 pass, 0 fail.Not verified
Refs #360
Refs #407
Follow-up fixes after re-review
docs/automation.mdand the CHANGELOG now also name the withheld recovery caused by pending input of your own in the composer.not sent/chain timeout); noted in the CHANGELOG line anddocs/automation.md.