(triggers): hold a single trigger while the CLI shows a dialog (#379) - #424
Conversation
A single trigger was written without consulting the CLI descriptor, so a dialog (permission prompt, question) swallowed it. It now waits for the descriptor to read idle, like a chain step, up to its own deadline, and fails "not sent" with a dialog reason otherwise. Typed input stays as is: the terminal-input channel cannot tell a person from a driver and its keystrokes are what answer a dialog. Closes #379
|
Reviewing |
devsuitup
left a comment
There was a problem hiding this comment.
Adversarial review at 2429515: changes requested. (B1, reproduced) An idle session fails not sent when timeout_ms is at or below the 300 ms settle window, and every single trigger pays +300 ms. (B2) docs/automation.md still describes wait:none as write-now. (B3) wait:none lost its write-mid-turn capability. Decision: none keeps write-now; its only new hold is a dialog (waiting). idle gets the full readiness rule, and an already-idle descriptor is ready at once. Also needed: a pre-write deadline guard on the single path, tests for settle, session exit and descriptor loss (two mutations survived), coverage for the Node 22 patch gate, and restoring a CR byte in the context doc.
Review of the single-trigger readiness: wait "none" keeps its write-now meaning (the CLI queues a prompt written while busy, which also covers sessions held busy by background agents) and holds only while the CLI shows a dialog. wait "idle" keeps the full readiness rule. An idle descriptor older than the settle window is ready on the first read, and the settle is capped at the time left, so a short timeout_ms on an idle session writes instead of reporting that it never went idle. A pre-write deadline guard matches the chain's. Tests cover the settle, the exited-session and lost-descriptor branches, and the deadline guard. Closes #379
# Conflicts: # CHANGELOG.md
|
Reviewing |
devsuitup
left a comment
There was a problem hiding this comment.
Re-review at fc259b3 (delta from 2429515): 0 blocking. The decided semantics hold: none writes at once and holds only on a dialog, until the deadline. idle uses the readiness rule, the deadline guard has no await before the write, and the #414 post-compact anchor survives (its mutation turns the anchor test red). The merges brought nothing unintended. Being fixed: the new first-read shortcut also applies to chain steps, so it moves to the single-trigger path only and chain behaviour stays as on main. The REASON_IDLE_UNSETTLED wording also covers an idle that kept restarting.
The first-read shortcut (an idle stamped before the settle window is ready at once) is now a parameter of waitForCliIdleAfter, off by default and turned on only by the single-trigger path, so a chain step still pays the settle on a stale idle. The unsettled reason now also covers an idle that kept restarting. Closes #379
devsuitup
left a comment
There was a problem hiding this comment.
Re-review at 68bc9bf (delta from fc259b3): 0 blocking. The idle-stamp shortcut is behind trustIdleStamp (default off), which only the single-trigger path passes, so chain steps behave as on main. A new test pins that a chain step still pays the settle on an old idle, and forcing the shortcut on turns it red. REASON_IDLE_UNSETTLED is reworded.
The poll timer of the readiness helpers is unref'd on purpose, so a test that only awaits it was cancelled by the Node 20/22 runner. The two helper tests now keep a ref'd interval alive while they await. Closes #379
What
A single trigger no longer lands in a dialog (permission prompt, question) that the CLI shows instead of its composer. The two
waitvalues keep their documented meaning:wait: "none"(default) writes now. A prompt written while the CLI is busy is queued by the CLI, which stays legitimate and covers sessions held busy by background agents ((triggers): chain times out after step 0 because the busy flag stays up on an idle session #360). The only new hold is a dialog: while the descriptor readswaiting, nothing is written; attimeout_msthe result isnot sentwithreason"the CLI reports a dialog open (waiting); nothing was written into it".busy,idleor no readable descriptor write at once, with no settle. A descriptor lost after it readwaitingkeeps the hold. It closes the hole inwaitForIdle, which samples the descriptor only while_cliBusyis true.wait: "idle"gets the chain-step rule (waitForCliIdleAfter, reused) with the trigger's own deadline: held until the descriptor readsidle, busy/waiting/other failnot sentwith the matching reason.Both paths now refuse to write once
timeout_mshas passed, descriptor or not, as chains do.Settle (review finding): an
idlestamped before the settle window is ready on the first read (no flat +300 ms), and the single path caps the settle at the time left, sotimeout_msbelow the settle on an idle session writes. AtrustIdleStampparameter ofwaitForCliIdleAfter(off by default) carries the first-read rule, and only the single path turns it on: chain steps behave exactly as on main and still pay the settle on an old idle (pinned by a test and a mutation). An idle that never held long enough to settle reports a distinct reason ("the CLI was idle only briefly before the deadline; it never held long enough to settle"), never "never reported idle".Typed input
Unchanged on purpose:
terminal-inputis a fire-and-forget IPC fed by the renderer, keystrokes, pastes and drops share one call with no marker, and holding input onwaitingwould block the keystrokes that answer the dialog. See.ai/contexts/trigger-watcher.md, "Readiness before a single trigger".Docs
docs/automation.md(wait, "What this costs wait: none", the single-command bullets naming #360 foridle,waited_ms), the context doc, CHANGELOG.Tests
test/trigger-single-readiness.test.js, 19 tests with a fake descriptor: none on waiting / waiting that closes (no settle) / busy / idle / no descriptor / waiting then lost / session exits; idle on waiting / dialog that closes / busy / status then lost / session exits / old idle immediate / timeout below settle / settle capped / unsettled reason; the deadline guard with and without a descriptor; the helper's first-read rule with and withouttrustIdleStamp.Mutations (each made the file fail, then restored): settle cap removed, first-read stamp age removed (and, separately, the shortcut forced on for chains),
sessionExitedbranch removed (idle path, none path, helper), deadline guard removed, none timeout branch removed, lost-descriptor hold removed, none hold bypassed.Checks:
task checkexit 0 (3068 pass, 0 fail, lint 0 errors). Changed lines oftrigger-watcher.jsagainst the trigger test files under c8: 66 of 66 covered (the CI gate script itself, diff-cover, was not run). Not verified: Node 22 run, a live CLI.Closes #379