Repository navigation
fix(triggers): stop reporting observed activity as a confirmed submission - #168
Merged
Merged
Conversation
devsuitup
force-pushed
the
fix/submitted-semantics
branch
from
September 4, 2026 00:17
25405a1 to
094a417
Compare
devsuitup
force-pushed
the
fix/submitted-semantics
branch
from
September 4, 2026 04:10
094a417 to
c1d0fdd
Compare
4 tasks done
…sion submitted: "confirmed" was emitted whenever the session read busy in the window after a write. That observation is a level, not an edge measured against a pre-write baseline, so a session already mid-turn satisfies it with activity the write did not cause. And even a turn we did cause proves nothing about how the CLI read the text: a slash command that misses the completion menu is submitted as an ordinary message and produces a turn just the same. A caller that debounces on "confirmed" therefore believes it acted, and does not retry. That case now reports the new value "activity", which claims only what was seen. "confirmed" stays in the strength order, strictly above "activity", reserved for a transport that reads the intended effect back; no path in this repository emits it. Order: no < assumed < activity < confirmed. Readers testing submitted === 'confirmed' stop matching and fall through to their retry or escalation path, which is the safe direction. Readers testing against 'no' are unaffected. No discriminator was wired in. Measured on 13 real /compact occurrences across four session transcripts, an executed command and a non-executed one write an identical user record at Enter + a few seconds; the record that separates them (the compact_boundary system record) is only flushed 96-271 s later, once the compaction has finished. That readback is a caller's job, not the watcher's.
The chain path folds each step's verify.sawBusy into chainSubmitted via weakestSubmitted, but no chain test ever asserted result.submitted at the top level for a step that is legitimately submitted without ever being observed as busy. Two mutations stayed green as a result: dropping verify.sawBusy entirely (always folding in "activity"), and forcing the retry's own sawBusy to false. Add two chain tests reproducing the exact incident shape (a "/compact" step that IS observed, followed by a resume prompt that sits unobserved): one pins the fold at "assumed" when a step never sees busy, the other pins it at "activity" when only the retry Enter wakes the session. Each kills its mutation individually, confirmed by running the mutated line in isolation.
…rvation pollForBusyObserved is a level probe: a session already mid-turn when a trigger fires satisfies it in milliseconds, with a turn the write did not cause. That gap was already closed for "activity" (never confirmed from busy alone), but removing "confirmed" outright broke the harness side: DeliveryConfirmed is the only ordinary release of the one-chain-in-flight guard, so every chain fell back to the two-hour ceiling. Reintroduce "confirmed", on stricter footing: the composer is read back immediately after our own Enter (unconditionally, never skipped because activity was observed), and the result only counts when the session was not already busy the instant we wrote and a turn was still independently observed. Neither condition alone is enough -- proven by the "chain confirmed fold" tests, where a composer reading that nobody ever disturbed reads identically whether or not anything downstream reacted to it. A retry never earns "confirmed": needing one already means the first Enter did not visibly register. Also: each chain step's own politeness wait now counts toward that step's own waited_ms, not only toward the chain-level total_waited_ms -- the gap that let a real run report total_waited_ms far ahead of the sum of its steps, with no definition on file to contradict it. total_waited_ms's definition is now written down in docs/automation.md.
…ed_ms The preBusy-before-write ordering had no test that could fall on the mutated (after-write) ordering: it survived untouched, and that line is the one that separates the field incident from every other case. Added a mock with a deterministic order (busy true until our own write lands, false at that instant, true again once a genuine turn starts) rather than a timing window -- correct code samples busy still true and refuses "confirmed"; sampling after the write reads it as idle and would grant it, the overestimating direction that costs. Also: the single-command path's waited_ms left out the submit-verification poll (and its retry) entirely, the same shape of gap total_waited_ms had. Folded in, with its own test, and docs/automation.md now defines waited_ms and total_waited_ms side by side so the two paths can't drift apart again.
Two triggers for one session could reach its composer at the same time: the in-flight set is keyed on the trigger filename, never on the session, so their writes could interleave into a single line of input. They now queue behind one another per session; different sessions are unaffected. The same window is why a submission could be reported as confirmed on a turn it did not cause. That part is documented rather than fixed: this channel carries nothing that ties an observed turn to the write that would have caused it, so confirmed says the session was idle before the write and a turn followed within the window, and no more than that. Also folds a non-final step's turn-fall wait and a failed recovery keystroke's polled wait into total_waited_ms, both of which the written definition already claimed and the code did not deliver.
devsuitup
force-pushed
the
fix/submitted-semantics
branch
from
September 4, 2026 12:30
8d8bf30 to
f3d5474
Compare
devsuitup
added a commit
that referenced
this pull request
Sep 4, 2026
…cleanly Rebased onto main, which now carries the per-session FIFO lock (#168): the lock is acquired right after sessionId validation, before the target guard runs, so a guard refusal is briefly a lock holder. Proved by execution (and by mutating the release call) that this is safe -- the refusal's early return still runs inside the same try/finally that releases the lock, so a legitimate trigger queued behind a refused one still goes through. Also, from review: - normalizeCwd's bare-root exemption now checks n !== path.sep instead of a length threshold, which stopped protecting a one-character path.sep the moment someone shortened it; added the missing test. - translated a stray French heading in the (English) context doc. - corrected a test comment that named the wrong refusal branch.
Merged
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.
What was wrong
A remote machine reported an auto-compaction trigger that returned
ok=true,submitted=confirmed, 2/2 steps — with no compaction, and no/compactanywhere in the transcript. Neither the command nor a message.
pollForBusyRisewas never a rising-edge detector. It is a level probe thatanswers on its first tick, with no reference sample taken before the write. A
session that was already mid-turn therefore satisfied it immediately, and its
pre-existing turn was reported as a confirmation of our own submission.
Why it is not fixed by making the probe smarter
Measured on 13 real
/compactruns: in the window where the transport writesits result, no discriminator exists. At the moment Enter is pressed, the
executed case and the non-executed case write a strictly identical line. The
separating signal —
compact_boundary,isCompactSummary, the XML replay underthe same
promptId— arrives 96 to 271 seconds later. A compaction stays in thesame
.jsonl, and the session state file readsbusyexactly as for anordinary turn.
The transport cannot know. So it stops claiming to.
What changed
confirmedis no longer emitted. A fourth valueactivitysays what isactually known: activity was observed after the write. The order becomes
no < assumed < activity < confirmed, andconfirmedstays reserved for atransport that re-reads the effect rather than the symptom.
A reader that tested
=== 'confirmed'now falls into its retry branch: strictlymore cautious, never less.
Field evidence collected since
A local chain ran
/compactthen a resume prompt. Result:submitted=confirmed,submit_retries: 0, step 1 waited 212 ms. The resume prompt was never submitted— it sat in the composer until it was submitted by hand, four minutes later.
Step 0 was still executing when step 1 was written: the chain waits for the
composer to be free, which it is while the previous command runs.
That second defect is not addressed here. It is a chain-sequencing problem,
not a reporting one, and it deserves its own change.
Tests
824 pass, 0 fail. A pre-existing test pinned the false confirmation and was
rewritten rather than deleted: it now asserts the value the transport can
justify.