Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
96 changes: 96 additions & 0 deletions .ai/contexts/trigger-watcher.md
Original file line number Diff line number Diff line change
Expand Up @@ -184,6 +184,102 @@ The SGR-and-focus exemption above (PR #160) is correct and worth keeping, but it
**cannot** be the cause of this symptom: exempting mouse reports cannot quiet a
channel that carries none, since Claude Code never turns motion tracking on.

**Found it — CPR, fixed 2026-09-04.** A 340 s trace of two idle sessions
(`SWITCHBOARD_ACTIVITY_TRACE`, real machine, nobody at the keyboard) put a
number on the suspects above:

| Shape | Count | out of 2,422 `pty.input` chunks |
|---|---|---|
| CPR / DECXCPR (`CSI [?] row ; col [; page] R`) | 1,427 | 59% |
| SGR mouse (`CSI < b ; x ; y M`/`m`) | 495 | 20% — already excluded, working |
| plain text, no control byte (real typing) | 473 | 20% — real keystrokes, correctly counted |
| DEL / CR | 27 | 1% — real keystrokes |
| OSC, DCS | 0 | not observed on this machine, ever |

CPR alone, arriving roughly every 240 ms, kept `lastInputAt` reassigned
continuously: the 3000 ms quiet window (`DEFAULT_QUIET_MS`) could not open
between two sessions still on screen, whatever the pointer or the CLI's own
busy state were doing. The 2026-09-02 fix above was correct but aimed at 20%
of the traffic; the dominant 59% was untouched, which is why the symptom
outlived it.

`composer-state.js`'s `reportLength()` now recognises CPR/DECXCPR the same
way it recognises SGR mouse and focus reports: a dedicated regex bounding
every numeric field (`CPR_PARAMS_RE`), not a bare check on the final byte.
`applyCsi` has no case for final `R` — falls through its default — so before
this fix a CPR chunk pushed the clock but never touched `text`/`cursor`/
`pending`; the defect was confined to the quiet window, not to composer
content. Verified by reading the switch, not by a runtime test: once
`reportLength` claims `R`, `applyCsi` never sees it, so nothing post-fix can
exercise that unreachable case (see `test/composer-state.test.js`).

**"The same way" is an analogy, not the safety argument — the two exemptions
rest on different properties.** What makes `SGR_MOUSE_PARAMS_RE` safe is that
its final bytes (`M`/`m`) only reach `reportLength` on a chunk starting with
`CSI <`, and no keyboard on this machine's key-event handler
(`public/terminal-manager.js`, `attachCustomKeyEventHandler`) or xterm.js's own
`onData` emits `<` as the third byte of a CSI sequence — the prefix is
terminal-report-only by construction, so bounding the numeric fields is
enough. Final `R` has no such prefix to lean on: xterm.js emits bare
`CSI n ; m R` for a *modified F3 keypress* (`xterm.js`, `case 114`:
`ESC+"[1;"+(mod+1)+"R"`, `1;2` through `1;8` for Shift/Alt/Ctrl combinations).
An earlier version of `CPR_PARAMS_RE` made the `?` optional (`\??`), so it
matched that shape too — a real keystroke silently exempted from the quiet
clock, the direction this whole guard exists to prevent, and strictly worse
than the CPR flood it was fixing. Caught before merge by checking a keyboard
source (xterm.js) rather than reasoning by analogy from the mouse case.

The actual safety argument for `CPR_PARAMS_RE`, mandatory `?` included: DECXCPR
(`CSI ? row ; col [; page] R`) is what this terminal answers with, and it is
the *only* shape observed — 24,795 CPR chunks in the 2026-09-04 trace, `?`
present in all 24,795, present in zero of the responses this codebase has ever
seen without it. Requiring the `?` excludes exactly the modified-F3 shape
above and nothing measured. **Residual**: this is a measurement on one
CLI/terminal pairing, not a proof that no terminal ever answers CPR without
`?` (plain DSR-6, `CSI row ; col R`, is a legal *bare* CPR in the DEC standard
— just not one this xterm.js/ConPTY combination has been observed to send).
If a future terminal or `xterm.js` config starts answering bare CPR, this
regex will — correctly, per the "doubt resolves to busy" principle — count
that as a keystroke rather than false-exempt it; the failure mode of being
wrong here is a spurious wait, not a swallowed keypress.

The end-to-end proof — a CPR flood no longer blocking `waitForComposerFree`'s
free condition — lives in `test/composer-state-quiet-window.test.js`, which
reimplements the one-line predicate against fake time rather than importing
`waitForComposerFree`: that function is not exported, and PR #168 edits it
directly, so adding an export here would create an avoidable conflict for a
one-line predicate that composer-state.js's own clock behaviour already
proves.

**DSR, DA1/DA2, DECRPM, window-ops (`t`) — not added, zero occurrences
measured.** The brief that produced this fix asked for all of these as a
matter of course; the trace has none of them, on either the 103 s or the
340 s window, so they are deliberately left out rather than excluded on
reasoning alone — the same conservatism the SGR-params regex already applies
to mouse reports. DECRPM (`CSI ? Pd $ y`) additionally can't be told apart
from a bare final `y` with the current `matchEscape`: the intermediate `$`
byte is consumed but not returned in the match, so distinguishing it would
mean changing `matchEscape`'s return shape, not just adding a param regex —
out of scope for a fix this size. If a future CLI release starts querying any
of these (plausible — Claude Code already probes DECRPM-style modes for
things like synchronized-output support), re-run the trace and treat it the
same way CPR was: measure first, then add the exact shape, never the final
byte alone.

**OSC and DCS replies — parsed differently, and both unmeasured.** OSC
replies (colour query, `10`/`11`) are matched whole by `matchEscape` (kind
`osc`) and, since nothing in `noteUserInput` applies to that kind, push
`lastInputAt` without touching `text`/`pending` — a defect of the same shape
as CPR's, just with zero observed occurrences here. DCS is worse and
structural, not a report-recognition gap: `matchEscape` has no DCS
introducer case, so `ESC P` falls into the generic 2-byte `esc` catch-all,
and everything up to the terminator (`ESC \` or the DECRQSS/XTGETTCAP
payload) is then walked byte-by-byte as literal text — a DCS reply, if one
ever arrived, would be typed into the composer. Not fixed here: zero measured
occurrences, and the fix is a `matchEscape` change (recognising and
discarding a whole DCS sequence), not a `reportLength` addition — a
different, larger piece of work than this PR's scope.

**`submitted`.** Every result carries it, compared by strict equality:
`confirmed` (a busy rising edge was observed after our write), `assumed`
(written, no failure seen, nothing observed after), `no` (nothing written, or
Expand Down
11 changes: 8 additions & 3 deletions composer-state.js
Original file line number Diff line number Diff line change
Expand Up @@ -152,12 +152,15 @@ function matchEscape(buf, i) {
}

// ── Terminal reports ─────────────────────────────────────────────────────────
// Mouse and focus reports ride the same channel as keystrokes but are not user
// input: neither text nor activity. Recognition is deliberately strict — see
// .ai/contexts/trigger-watcher.md.
// Mouse, focus and cursor-position reports ride the same channel as
// keystrokes but are not user input: neither text nor activity. Recognition
// is deliberately strict — see .ai/contexts/trigger-watcher.md.

const SGR_MOUSE_PARAMS_RE = /^<\d{1,10};\d{1,10};\d{1,10}$/;

// DECXCPR only, `?` mandatory: `CSI ? row ; col [; page] R` — see .ai/contexts/trigger-watcher.md.
const CPR_PARAMS_RE = /^\?\d{1,4};\d{1,4}(?:;\d{1,4})?$/;

/**
* How many bytes of terminal report start at the sequence `seq` just matched.
* 0 when the sequence is not a report.
Expand All @@ -169,6 +172,8 @@ function reportLength(seq) {
if ((final === 'M' || final === 'm') && SGR_MOUSE_PARAMS_RE.test(params)) return seq.len;
// Focus in / focus out (CSI ?1004h).
if ((final === 'I' || final === 'O') && params === '') return seq.len;
// Cursor position report.
if (final === 'R' && CPR_PARAMS_RE.test(params)) return seq.len;
return 0;
}

Expand Down
67 changes: 67 additions & 0 deletions test/composer-state-quiet-window.test.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,67 @@
// test/composer-state-quiet-window.test.js — end-to-end cover for the free
// condition `waitForComposerFree` (trigger-watcher.js) polls on. Reimplements
// the predicate against fake time rather than importing it — see
// .ai/contexts/trigger-watcher.md ("Found it — CPR") for why.
'use strict';

const test = require('node:test');
const assert = require('node:assert/strict');

const { createComposerState, noteUserInput } = require('../composer-state');

const QUIET_MS = 3000; // trigger-watcher.js DEFAULT_QUIET_MS
const CPR = '\x1b[?59;3R';

function isFree(state, now) {
return state.pending === 0 && (now - (state.lastInputAt || 0)) >= QUIET_MS;
}

test('quiet window: a realistic CPR flood no longer blocks it forever', () => {
// Measured 2026-09-04: 1,427 CPR chunks over ~340s of idle-session trace,
// roughly one every 238ms — far more often than the 3000ms quiet window.
const state = createComposerState();
let now = 0;
let everFree = false;
for (let i = 0; i < 50; i++) {
now += 238;
noteUserInput(state, CPR, now);
if (isFree(state, now)) { everFree = true; break; }
}
assert.equal(everFree, true, 'the quiet window must open under CPR traffic alone');
});

test('quiet window: still refuses while the user is actually typing', () => {
// The direction that must never flip: real keystrokes interleaved with the
// same CPR flood keep the composer busy.
const state = createComposerState();
let now = 0;
for (let i = 0; i < 12; i++) {
now += 238;
noteUserInput(state, CPR, now);
if (i === 6) noteUserInput(state, 'x', now); // a keystroke lands mid-flood
}
assert.equal(state.pending, 1, 'the typed character is still sitting in the box');
assert.equal(isFree(state, now), false, 'a non-empty composer is never free');

noteUserInput(state, '\r', (now += 100)); // submit
assert.equal(state.pending, 0);
assert.equal(isFree(state, now), false, 'still inside the quiet window right after submit');
assert.equal(isFree(state, now + QUIET_MS), true, 'free once the quiet window elapses');
});

test('quiet window: pre-fix behaviour never opens under the same flood (control)', () => {
// Same drive as the first test, but reimplementing the pre-fix predicate —
// "every CSI that is not mouse/focus counts as input" — to show the old
// code path genuinely could not pass. This is the failing case the fix
// replaces, kept here as documentation rather than a live assertion on
// production code (it does not import anything from composer-state.js).
let lastInputAt = 0;
let now = 0;
let everFree = false;
for (let i = 0; i < 50; i++) {
now += 238;
lastInputAt = now; // CPR was, pre-fix, indistinguishable from a keystroke
if ((now - lastInputAt) >= QUIET_MS) { everFree = true; break; }
}
assert.equal(everFree, false, 'documents the bug: the clock never opened before this fix');
});
110 changes: 110 additions & 0 deletions test/composer-state.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -202,6 +202,11 @@ test('composer-state: an unrecognised sequence still counts as input', () => {
['\x1b[1;2O', 'a modified CSI O'],
['\x1bOM', 'SS3 M, not CSI M'],
['\x1b[<0;42;13X', 'the right shape with the wrong final byte'],
// Empty numeric fields: the near-miss a `\d*` mutant would let through
// silently, since `*` accepts zero digits where `{1,10}` requires one.
['\x1b[<;42;13M', 'SGR report with an empty button field'],
['\x1b[<0;;13M', 'SGR report with an empty x field'],
['\x1b[<0;42;M', 'SGR report with an empty y field'],
];
for (const [seq, why] of cases) {
const state = createComposerState();
Expand Down Expand Up @@ -236,6 +241,111 @@ test('composer-state: a typed ESC [ M is input, not a mouse report', () => {
assert.equal(keyByKey.lastInputAt, 7000);
});

// ── Cursor position reports (CPR / DECXCPR) ─────────────────────────────────
// Regression cover for the 2026-09-04 measurement: CPR chunks dominated a
// real idle-session trace and, left unhandled, kept the quiet clock from ever
// opening. See .ai/contexts/trigger-watcher.md ("Found it — CPR") for the numbers.

const CPR_DECX = '\x1b[?59;3R'; // DECXCPR, as measured
const CPR_PAGE = '\x1b[?59;3;1R'; // DECXCPR with a page field

test('composer-state: a cursor position report is neither text nor activity', () => {
assertInert([CPR_DECX]);
assertInert([CPR_PAGE]);
// The measured cadence: back-to-back queries as the column advances.
assertInert(['\x1b[?59;3R', '\x1b[?59;4R', '\x1b[?59;6R', '\x1b[?59;8R']);
});

// A bare `CSI n;m R` with no `?` is not a report at all on this CLI/terminal
// pairing: it is what xterm.js sends for a modified F3 keypress (`case 114`
// in xterm.js, `ESC[1;<mod+1>R`). 24,795/24,795 CPR chunks measured in the
// 2026-09-04 trace carried the `?`; zero did not — see
// .ai/contexts/trigger-watcher.md ("Found it — CPR"). The `?` is therefore
// mandatory in CPR_PARAMS_RE, not optional: a bare `n;m R` must count as
// input, exactly like any other unrecognised sequence.
test('composer-state: a bare (non-DECXCPR) CPR-shaped sequence is a keystroke, not a report', () => {
const bareCases = [
['\x1b[24;80R', 'bare CPR: no `?`, plausible-looking but never measured'],
['\x1b[1;2R', 'Shift+F3'],
['\x1b[1;3R', 'Alt+F3'],
['\x1b[1;4R', 'Alt+Shift+F3'],
['\x1b[1;5R', 'Ctrl+F3'],
['\x1b[1;6R', 'Ctrl+Shift+F3'],
['\x1b[1;7R', 'Ctrl+Alt+F3'],
['\x1b[1;8R', 'Ctrl+Alt+Shift+F3'],
];
for (const [seq, why] of bareCases) {
const state = createComposerState();
noteUserInput(state, 'hi', 1000);
noteUserInput(state, seq, 9000);
assert.equal(state.lastInputAt, 9000, `${why} (${JSON.stringify(seq)}) must push the quiet clock`);
}
});

test('composer-state: a CPR mixing with a keystroke counts only the keystroke', () => {
const state = createComposerState();
noteUserInput(state, 'hi', 1000);
noteUserInput(state, CPR_DECX + 'x', 5000);
assert.equal(state.pending, 3, 'the keystroke lands in the composer');
assert.equal(state.lastInputAt, 5000, 'and pushes the quiet clock');
// The discriminating half: further CPRs on their own, after the keystroke,
// must not advance the clock past it.
noteUserInput(state, CPR_DECX, 9000);
assert.equal(state.lastInputAt, 5000, 'a later CPR alone must not push the clock again');
});

test('composer-state: a CPR split across chunks is never counted as text', () => {
const state = createComposerState();
noteUserInput(state, '\x1b[?59;', 1000);
assert.equal(state.pending, 0, 'a half report is held back, not typed');
assert.equal(state.partial, '\x1b[?59;');
noteUserInput(state, '3R', 2000);
assert.equal(state.pending, 0, 'the completed report adds nothing');
assert.equal(state.partial, '');
assert.equal(state.lastInputAt, 1000, 'the completing chunk is silence');
});

test('composer-state: a near-miss CPR still counts as input', () => {
// Same safety principle as the SGR near-misses: only the exact shape is
// exempt. Arbitrary params ending in R must resolve towards busy.
const cases = [
['\x1b[24R', 'CPR missing the column field'],
['\x1b[24;80;1;1R', 'one field too many'],
['\x1b[24;80;1;1;1R', 'two fields too many'],
['\x1b[a;bR', 'non-numeric CPR parameters'],
['\x1b[24;80;1;R', 'a dangling separator'],
['\x1b[5R', 'CSI 5 R — not a position report at all'],
// Empty numeric fields on an otherwise well-formed DECXCPR: the near-miss
// a `\d*` mutant would let through silently, since `*` accepts zero
// digits where `{1,4}` requires one.
['\x1b[?;3R', 'DECXCPR with an empty row field'],
['\x1b[?59;R', 'DECXCPR with an empty column field'],
];
for (const [seq, why] of cases) {
const state = createComposerState();
noteUserInput(state, 'hi', 1000);
noteUserInput(state, seq, 9000);
assert.equal(state.lastInputAt, 9000, `${why} must push the quiet clock`);
}
});

// NOTE: whether a CPR could corrupt `text`/`cursor`/`pending` (not just the
// clock) was checked by reading `applyCsi` rather than by a test here — see
// .ai/contexts/trigger-watcher.md ("Found it — CPR") for why no runtime
// assertion on this can ever go red post-fix.

test('composer-state: ordinary typing still pushes the clock through a CPR flood', () => {
// The guarantee that matters: excluding CPR must not accidentally exclude
// real keystrokes that merely resemble one in shape.
const state = createComposerState();
let t = 1000;
for (let i = 0; i < 20; i++) noteUserInput(state, CPR_DECX, (t += 50));
assert.equal(state.lastInputAt, 0, 'a CPR flood alone must never look like typing');
noteUserInput(state, 'x', (t += 50));
assert.equal(state.lastInputAt, t, 'a real keystroke still pushes the clock');
assert.equal(state.pending, 1);
});

test('composer-state: a bare CSI M never swallows the bytes that follow it', () => {
// Treating it as the head of a report misaligned the next chunk and left the
// composer frozen on a phantom count.
Expand Down
Loading