The ordering invariant only proves a guard sits above the Escape branch it
protects. It cannot see a second, wholly unguarded Escape handler added below a
guarded one, so a file with one correct handler vouched for handlers it never
covered. MarkdownPreview.tsx was exactly that case: the find field dispatched
Enter and Escape with no check at all, and its wrapper stops propagation, so the
guarded window listener never saw those keys. A composing Enter stepped the match
cursor while committing a candidate; a composing Escape closed search and pulled
focus out of the live composition.
Guard the field, and extend the reviewed-file allowlist so absence is checked
there too, not just ordering. Paired marked/unmarked tests cover both keys;
deleting the guard fails exactly the two marked cases.
Co-authored-by: Orca <help@stably.ai>
The reviewer-picker inputs on the PR page and the GitHub item dialog had no
composition check at all: a marked ArrowDown moved the app's selection, a marked
Enter requested a reviewer, and a marked Escape closed the picker — three
keystrokes the user aimed at the candidate window. The duplicate-issue search ran
on a composing Enter for the same reason.
MentionTextarea is guarded at its own DOM handler rather than in each caller. It
is the only keydown listener on that textarea, so refusing there is what keeps a
composing Escape from reaching a delegate that would discard the draft.
The ordering invariant could not have caught any of this, and it also could not
see the native chat composer: its regex matched the hook's outer function and
brace-matched the destructured parameter list, so the body was never scanned and
an Escape-before-guard regression there passed. Body detection now skips the
brace forms that are not bodies — destructuring, JSX containers, object types —
and the coverage the scanner claims is asserted by name, so a parser regression
fails naming the file it stopped seeing.
Co-authored-by: Orca <help@stably.ai>
An Escape that dismisses an IME candidate window carries the composition
markers, so a handler that runs its Escape branch before consulting the guard
acts on a keystroke the user aimed at the IME — discarding a comment draft,
reverting a search, or cancelling shortcut recording.
Eight keydown handlers guarded Enter but dispatched Escape first. Two of them
(the reviewer picker and the workspace-name field) also left the arrows
unguarded, so candidate navigation moved the app's selection too.
The marker sweep that found the dropped-isComposing adapters cannot see this:
the markers are present and the guard is called, just one branch too late.
Ordering is now pinned as a structural invariant over every keydown handler in
src/, which is what a per-component test could not do.
Co-authored-by: Orca <help@stably.ai>
A sixth site rebuilt the detector input field-by-field without the composition
markers. Found by re-running the sweep the way round 5 showed it had to be run:
by every construction of a matcher-input literal, not by call site.
Recording is the surface most likely to hide this, because capturing whatever
key arrives is its whole job. But an IME-owned keystroke carries no chord — its
`key` is 'Process' or 'Unidentified' — so capturing one wrote a binding that can
never fire, and the modifier path could arm a double-tap from a composing press.
Ownership is fed to the detector rather than short-circuiting above it, so a
composing press resets the gesture instead of leaving it armed.
Each of the three guards fails exactly one of the new tests and no others.
Co-authored-by: Orca <help@stably.ai>
Two more places rebuilt a keybinding input field-by-field and dropped the
markers the matcher refuses on.
`matchesRecentTabSwitcherChord` is the harder one: its callers pass the raw
event, so a call-site sweep reads clean, but the helper normalizes Shift away
by hand-building a literal one level down. Ctrl+Tab therefore still resolved
mid-composition.
The two `before-input-event` handlers omitted `isComposing` when feeding the
double-tap detector. `Electron.Input` does carry it — a completed gesture
dispatches `{ doubleTapModifier }` with no key or modifier flags, so once one
is armed the matcher has nothing left to refuse on.
Also covers the floating terminal panel, whose product fix already landed but
whose suite passed unchanged when both ownership decisions were mutated away.
Every guard here is mutation-checked: disabling one fails exactly the tests
that name it, and no others.
Co-authored-by: Orca <help@stably.ai>
A detected double-tap is dispatched as `{ doubleTapModifier }` with no key or
modifier flags, so once one is armed the shortcut matcher has nothing left to
refuse on. Decide ownership where the gesture is built instead: an IME-owned
event now yields a null modifier, which the detector already treats as a reset.
The three renderer listeners feed that decision in rather than returning early.
Withholding the press looked safer but left the gesture armed, so a later real
press completed a tap the user never made.
Also routes the quick-commands listener through the shared dispatch adapter, so
it can no longer drop the IME markers while rebuilding the input field-by-field.
Co-authored-by: Orca <help@stably.ai>
Guarding the shared matcher was not sufficient. App.tsx rebuilt the matcher
input field-by-field from the DOM event and copied only key, code, and the
modifier flags, so `isComposing` and `keyCode` never reached the matcher and
it had nothing to refuse on. A composing Mod+Alt+W the matcher rejected when
handed the event directly still resolved `tab.closeAll` once it had been
through the adapter, and the plugin-command guard was blind on that path for
the same reason.
The adapter is now a named module with its own tests rather than an inline
object literal, because the defect was invisible at the call site and no
test of the matcher could ever catch it. It refuses to build an input at all
for an IME-owned keystroke, and copies the markers verbatim when it does, so
the matcher can still refuse independently.
Composing keydown and keyup no longer reach the double-tap detector either.
The detector is stateful across the pair, and the synthetic input a
detection produces carries no key or modifier flags, so there would be
nothing left downstream to refuse on.
Also exempts the xterm source patch from the whitespace checks: a unified
diff prefixes context lines with a space, so a blank source line is a line
holding exactly one space, and stripping those corrupts the patch.
Co-authored-by: Orca <help@stably.ai>
Every trace in ime-recorded-composition-traces was `derived`; the only
hardware recording in the suite is the ibus-hangul one it re-exports. The
name asserted evidence the file does not have, so it is now
ime-derived-composition-traces with a header saying so outright.
The Escape exemption in isImeCompositionKeyDown was justified in prose as an
assumption. It is stronger than that: UI Events defines `isComposing` as
true between compositionstart and compositionend, so an unmarked Escape is
the browser contradicting our derived flag, and the browser is authoritative.
Recorded the reasoning and the residual risk that a browser under-reports
the bit.
Co-authored-by: Orca <help@stably.ai>
Application shortcut dispatchers are capture-phase siblings on `window`.
Returning early from one does not stop the others, and the terminal's guard
deliberately does not preventDefault because the IME still needs the key. So
guarding listeners one at a time cannot hold: a composing Mod+Alt+W still
resolved `tab.closeAll` in the Terminal dispatcher and closed every editor
tab mid-preedit.
Move the decision to the matcher every dispatcher already shares.
`keybindingMatchesAction` and `matchKeybindingDigitIndex` now refuse input
carrying any IME ownership marker, and `KeybindingInput` carries those
markers straight off the DOM event.
The terminal is the one surface that must resolve a chord anyway, because
input-source switching and deferred Enter are legitimate mid-composition; it
opts out via `allowImeOwnedInput` through a single local wrapper, so its
call sites are unchanged and ownership is still judged after the action is
known.
Plugin commands resolve against the low-level matcher and so did not inherit
the refusal. They can be bound to any chord and run arbitrary code, so they
get the same rule explicitly.
The renderer's `isImeCompositionKeyDown` now imports the shared predicate
rather than restating the three markers, so the two cannot drift.
Co-authored-by: Orca <help@stably.ai>
Three surfaces acted on keystrokes an IME still owned:
- the popout preview terminal fed IME-owned key releases to xterm, which
encodes them as escape sequences once event reporting is on. Scoped to
keyup: a composing keypress can carry the committed glyph.
- the task page's Mod+F window listener tore down a live preedit by
focusing and selecting the search field.
- the deferred-newline ceiling emitted Enter into a composition the route
still positively reported as open, splitting the preedit.
The ceiling now drops the newline and releases the sender's absorb credit,
so the user's next Enter is not swallowed. Dropping it costs one repeatable
keypress; splitting a live preedit does not undo.
Adds a contract test for the previously implicit invariant that every Enter
shape the shortcut guard claims is also one the xterm suppression policy
withholds.
Co-authored-by: Orca <help@stably.ai>
Both the task page and the markdown preview listen for keys on window in the
capture phase, so they run ahead of whatever element is composing. Escape is how
the user dismisses a candidate window: the task page blurred the field and the
preview closed search and moved focus, either of which ends the composition.
The IME rollout hardened the per-key handlers in both files but not these
listeners, which is the case the rules call out. The task page's policy moves to
its own module so it can be tested at all — the component is too large to render.
Co-authored-by: Orca <help@stably.ai>
The preview handed every non-keydown to xterm before consulting the IME guard.
Once Kitty REPORT_EVENT_TYPES is negotiated xterm encodes an accepted release, so
a keyup arriving during a live composition reached the pty as a release sequence.
The pane path already evaluates its IME policy on every event type; this closes
the same hole in the preview, which writes to a real pty.
Co-authored-by: Orca <help@stably.ai>
The guard released every Enter-shaped marked event whenever the caller set
enterIsDeferredToCommit, but only the sendInput branch defers. Mod+Shift+Enter is
bound to expand a pane by default, so composing that chord rearranged the layout
mid-preedit, and any terminal action rebound onto an Enter chord did the same.
The guard now asks what the event resolves to and exempts only sendInput. The
caller and the guard also share one resolution: Windows reports the composing
Enter as Process and the dispatch rewrote it to Enter before resolving, so the
guard had been deciding against a binding the dispatch would never run.
Co-authored-by: Orca <help@stably.ai>
Enumerating platform names in a second place is what let one user agent
read as Linux everywhere except the policy that cared.
Co-authored-by: Orca <help@stably.ai>
The pinned behavior looks like a false positive until you check what
ignoring it would cost on the first commit after focus.
Co-authored-by: Orca <help@stably.ai>
The candidate-key guards and the standalone 229 policy were reading one
flag, so tightening either one moved both. Derive them together instead.
Co-authored-by: Orca <help@stably.ai>
The preview's custom key event handler resolved the full shortcut policy and
wrote the result to a live pty with no composition check anywhere in it. Only
the macOS native-text bridge was wired up, so on Linux and Windows the surface
had no IME handling at all.
Ctrl+Backspace mid-preedit reached the agent's shell as \x17, the same way it
did from a pane. Shift+Enter sent \x1b\r before the composition committed, so
the newline arrived ahead of the text it was meant to follow.
Enter is claimed here rather than exempted. The pane defers it and replays the
newline once the commit lands; this surface has no defer path, so the only
correct move is to let the keystroke close the composition. That difference is
now an argument on the guard instead of an assumption inside it.
Returning true leaves xterm's own composition handling intact.
The window-capture keydown handler resolved and dispatched terminal chords
without checking for a live composition. It runs ahead of the pane handler and
stops propagation on a match, so the chord was the only thing the keystroke did.
Ctrl+Backspace mid-preedit reached the PTY as \x17 and erased a word of the real
command line behind the candidate window, leaving the composition untouched.
Alt+Backspace, Alt and Ctrl arrows, and the macOS Cmd line-edit chords have the
same shape. Separately, the non-Latin fallback re-derives bindings from
event.code, so a keystroke reported as Process still matched Ctrl+W and closed
the pane out from under a composition.
The only composition check was inside the sendInput branch and covered Enter
alone, which is the one key that was already safe.
Enter and the input-source switch stay exempt: the send path defers Enter until
the commit lands, and the switch only hands the key back to the OS.
Nothing in the app called it; only its own spec did, and every one of those
assertions is more precise stated against getFeatures(). The remaining
refresh() export is kept deliberately as a test seam and now says so.
Co-authored-by: Orca <help@stably.ai>
The Linux candidate-state heuristic and the candidate key-release guard each
carried their own copy of "unmodified ASCII digit", identical in behavior and
free to drift apart. Only the predicate is shared; the two guards keep their
separate windows and evidence, which are not the same thing.
Co-authored-by: Orca <help@stably.ai>
The caret-range derivation checked only that the growth was arithmetically
consistent with an insertion. A replacement that grows by the same amount fits
that test and emitted just its tail, so a commit arriving on this path could
reach the pty with its leading characters missing. Require the text outside the
claimed range to be unchanged, and fall into the existing emit-nothing path.
Co-authored-by: Orca <help@stably.ai>
The recorded composition extent tracks the preedit, so for any conversion engine
it overshoots the commit it produced. Used as a floor it made the strict variant
of the pending-input read exclude nothing, and the character typed straight after
a commit went to the pty twice.
Co-authored-by: Orca <help@stably.ai>
`CompositionHelper._handleAnyTextareaChanges` is the path an active IME takes
when it commits a plain glyph outside a composition — punctuation and digits,
the first key after a macOS input-source switch, Sogou/fcitx candidate commits
on Linux. `xterm-bypass-policy.ts` deliberately lets a bare keydown-229 through
to it on both platforms, so this is live product behavior, not a latent path.
It derived the new text as `newValue.replace(oldValue, '')`. String.replace
removes the first occurrence of the pattern *anywhere*, not a prefix, so any
insertion that was not a pure append produced garbage: a buffer of "ab" with
the caret between the two characters gaining an "x" sent the whole "axb" to the
pty instead of "x". Confirmed by running the new tests against the prior patch —
they fail with `expected [ 'axb' ] to deeply equal [ 'x' ]`.
Records the caret alongside the buffer and reads the insertion as the slice
between two known offsets, which is what `_finalizeComposition` already does
and what the IME rule in AGENTS.md requires. When the caret does not account
for the growth the evidence is ambiguous and nothing is emitted rather than
guessing bytes into a live shell. `_dataAlreadySent` now always equals the text
actually sent, which is the invariant its consumer assumes when it advances the
composition start to avoid re-sending (xterm Issue #3191).
Deletion and same-length-replacement keep their existing behavior; neither is
the defect proven here. Windows keeps full 229 suppression: its race is preedit
reaching the textarea before compositionstart, which still reads as an ordinary
insertion, so the range fix does not close it.
Co-authored-by: Orca <help@stably.ai>
The recorder captured isComposing faithfully and then lost it at the POST:
JSON.stringify omits an undefined value, so the written trace had no such
key at all. null carries it instead.
Adds repeat and selectionDirection, and a contract test so a field the
recorder learns about cannot again go somewhere the schema cannot hold it.
Co-authored-by: Orca <help@stably.ai>
CDP-dispatched keystrokes bypass the OS input-method layer, so no amount of
page.keyboard.type() produces a real composition. The Linux matrix drives ibus
through X11, but there is no macOS or Windows IME CI, and on those platforms a
human typing into a page is the only way to get a trace that can honestly be
labelled recorded.
pnpm ime:record serves a localhost page that captures the same field set as
tests/e2e/terminal-ime-boundary-probe.ts — including the selection offsets at
each event, which are what distinguish a recorded trace from a reconstructed
one — and writes it in the shape the fixture loader already consumes. Nothing
leaves the machine.
The engine name and the description of what the human typed are left as TODOs
in the output, because the page cannot observe either.
Co-authored-by: Orca <help@stably.ai>
Telex commits on a word boundary, not on Return, so the final syllable was
still composing when Return fired and committed into the following line.
No two repetitions agreed. The other three engines commit on Return.
Co-authored-by: Orca <help@stably.ai>
The native suite only ever exercised ibus-hangul, so every non-Korean IME
regression had to be caught by hand. Generalize the runner and spec over an
engine profile and add ibus-libpinyin (Pinyin), ibus-anthy (Romaji/kana) and
ibus-unikey (Telex) as matrix legs, covering the candidate-window, multi-commit
and rule-based rewriting shapes that Hangul alone does not produce.
Session config (apt package, GSettings, `ibus engine` name) and the keystroke
scripts live in separate files because one is loaded by the Node runner and the
other by the Playwright spec. They are joined only by the engine id, so the
contract test asserts the two sets agree and that the workflow matrix covers
exactly them.
Only the hangul expectations have been recorded from a real run. The other three
engines' committed glyphs are predicted from their input rules, so the exact-text
oracle is `expect.soft` for them while the structural oracle stays hard: every
repetition must produce identical bytes, in the engine's script, with xterm's
onData matching what the PTY received. That is the half that actually detects
the duplication and leaked-ASCII bugs, and it holds without knowing the glyphs.
A first green run turns each `expectationsVerified` to true.
fail-fast is off so a drifting libpinyin candidate cannot mask a hangul
regression, and evidence uploads are per-engine because upload-artifact rejects
a duplicate name.
Escape was suppressed whenever the terminal's derived composition flag was set.
That flag is not the per-event `isComposing` bit — it is tracked from
composition events, and compositionend is not guaranteed, so it can stand until
the tracker's 10 s stale expiry. For up to that long, Escape never reached the
PTY and vim could not leave insert mode, with no way to recover.
An Escape the IME really owns arrives marked, by `isComposing` or by keyCode 229
/ key 'Process', and those clauses still suppress it. An unmarked one only ever
means the flag has drifted.
Letting a drifted Escape through bounds recovery by keystrokes rather than by
the expiry timer. Escape does not take the finalize-then-encode path the other
keys take: our vendored xterm build gives it its own branch in
`CompositionHelper.keydown` (:278-282), ahead of the `_finalizeComposition`
fall-through. While xterm still believes a composition is in flight that branch
cancels — discarding the preedit and returning false — and
`CoreBrowserTerminal._keyDown` (:875-879) then returns before `evaluateKeyDown`,
so no `\x1b` is written. The cancel clears xterm's `_isComposing` but not this
tracker, so the next Escape passes through. Worst case is one extra keypress;
when only the tracker drifted and xterm has already settled, the first Escape
reaches the shell.
The rest of the IME-owned set — arrows, Backspace, Delete, Enter, Home/End,
PageUp/PageDown — is unchanged: those are candidate-window keys and forwarding
them mid-composition is what the set exists to prevent.
Co-authored-by: Orca <help@stably.ai>
Two ways a committed composition was lost or corrupted.
The composition route called preventDefault() before checking whether it owned
the session. A route installed mid-composition never saw the matching session
start, so it suppressed xterm's own insertion and then returned without
delivering anything — the commit vanished. Ownership is now decided first;
preventDefault stays for sessions the route does own, including the ones it
deliberately drops after a transport swap, since that drop is its decision to
make.
The deferred-newline fallback fired `finish` directly, bypassing the
pending-composition re-check that the event path uses. A composition still open
at 200 ms got a newline sent into the middle of its preedit. The deadline now
re-arms while a composition is pending, and its 2 s ceiling is measured from the
last compositionupdate rather than from the start. A slow candidate window no
longer splits the preedit, and neither does multi-segment bunsetsu conversion,
which stays legitimately pending well past any fixed total-elapsed ceiling. A
composition that stops progressing still cannot strand the keystroke.
Co-authored-by: Orca <help@stably.ai>
The re-arm also treats a live compositionupdate as evidence in its own right, not
just a session the route recorded. A route installed while a composition was
already in flight never saw that session start, so it has nothing to report as
pending and the deadline would otherwise split that composition too.
The Escape exemption assumes an IME-owned Escape arrives marked. Nothing
recorded showed that, so the assumption lived only in prose. Encode it as a
labelled derived trace with the command that promotes it.
Co-authored-by: Orca <help@stably.ai>
The schema could not express repeat or selectionDirection, and the replayer
coerced an absent isComposing to false — the one distinction the field is
typed `boolean | undefined` to capture.
repeat is what a press-and-hold accent panel is recognised by, so a trace
recorded for that quirk could not reproduce it.
Co-authored-by: Orca <help@stably.ai>
An Escape the IME owns always arrives marked, so an unmarked one means the
document flag has drifted. compositionend is not guaranteed, and eleven
call sites guard Escape above their key dispatch, so the drift left rename
fields, search boxes and pickers with no way to cancel.
Matches the rule xterm-bypass-policy.ts already applies to the terminal.
Co-authored-by: Orca <help@stably.ai>
Synthetic keystrokes cannot produce a composition — KeyboardEvents dispatched
through CDP bypass the OS input-method layer — so the IME suites each grew their
own ad-hoc dispatch helpers that assert against invented event orderings.
Adds a trace harness that replays an event sequence captured from a real engine,
stamping the observed buffer before each event so the code under test sees what
the browser actually produced. Traces carry a provenance field: `recorded`
traces come from a real engine and are authoritative, `derived` traces pin
current behavior but prove nothing about the engine, so a fix justified only by
a derived trace is visibly unverified. Two of the four seeded traces are
recorded from ibus-hangul in CI; the Telex and Pinyin ones are derived and say
so.
Also replaces 17 inline `nativeEvent.isComposing` guards with one predicate.
`isComposing` alone misses the extra keydown Safari emits after compositionend
and the engines that report only keyCode 229 or key 'Process', so each call site
was subtly different. The shared predicate adds a document-wide composition flag
cleared on focusout, because compositionend is not delivered when the composing
element loses focus. It also tracks the composing target and revalidates that the
target is still connected on read: Chromium fires no focusout when a focused node
is removed, so a composer that unmounts mid-composition would otherwise latch the
flag on for the session and kill every Enter-to-commit surface in the app.
The three remaining TaskPage sites go through shouldSuppressEnterSubmit, which now
delegates to the same predicate. An earlier version of this message called them
telemetry-only; they are Enter-submit gates, so bare isComposing was missing an
IME-owned Enter on Windows and Android.
Trace assertions compare an interpretation derived from event data against each
trace's declared final state. The previous assertion read back the buffer the
harness itself had stamped, so it could not fail.
Co-authored-by: Orca <help@stably.ai>
pnpm records the hash in patchedDependencies and again in every resolution key
that depends on the patched package. The generator only maintained the first, so
a regenerated patch left the rest stale: installs succeeded on a warm store and
the lockfile drifted on a cold one, which is CI.
Co-authored-by: Orca <help@stably.ai>
The patch carried 5.8MB of sourcemaps for bundles it rewrites, so the maps it
shipped described the wrong offsets anyway. The manifest's sourcemaps.policy was
documentation the generator never read; it is now the switch that drives this.
Deleting the maps rather than omitting their hunks is what keeps the patch
honest: omission saves the same bytes but leaves a map pointing into code that
moved. Nothing here consumes them - sourcemaps are off in every build config and
the package is a devDependency, so it never reaches a packaged app.
Also moves the .gitattributes -text rule down to the commit that introduces the
patch. It was added at the top of the stack, which left every commit in between
shipping a byte-hashed artifact that a CRLF checkout would corrupt.
Co-authored-by: Orca <help@stably.ai>
The xterm patch is 7.3 MB of diff over minified bundles with no recorded
provenance, so no one could regenerate it or tell whether the source hunks and
the bundle hunks still agree. Any IME fix that touches CompositionHelper had to
be hand-applied to four generated files.
Adds config/patches/xterm-src/ as the source of truth (832 lines over four
files), a pinned upstream manifest, and a generator that rebuilds the bundles
from that commit with the toolchain its lockfile pins, then derives the full
patch. `--check` runs it in CI as the new xterm_patch_sync job.
The committed patch is regenerated to match pnpm's own diff output (-U3 rather
than the hand-made -U6). This changes the patch bytes but no installed byte:
both patches applied to pristine tarballs produce recursively identical trees,
and all 8 patched files hash-identical to the installed package. pnpm's
patchedDependencies hash is sha256 of the patch file — verified for all five of
Orca's patches — so pnpm-lock.yaml carries the matching one-line update.
Five invariants that every IME fix in this stack depends on, most of which
were violated somewhere in the codebase before it. The one that keeps
recurring: committed text derived by diffing two observations of a mutable
buffer, which races the engine and silently drops or doubles characters.
Co-authored-by: Orca <help@stably.ai>
* Tier GitHub PR lookup polling to prevent quota exhaustion
The selected worktree (O(1)) checks per-minute; card list (O(N)) per-15-minutes.
Introduce process-wide cache to collapse concurrent polling and gate lookups on
available rate-limit budget with exponential backoff on failure.
- Preserve last-known review during backoff
- Invalidate cache when Orca opens a PR
- Stop coordinator from double-charging
* Tier GitHub PR lookup polling to prevent quota exhaustion
- Return the latest reset time when both GitHub API buckets are rate-limited, preventing premature retries against still-blocked buckets.
- Serve the last known review on transient lookup failures, preventing reviews from blinking out on temporary errors.
- Discard in-flight lookups that predate an invalidation so stale answers cannot overwrite newly opened reviews.
* fix: give rate-limit reset tests unique titles
oxlint vitest/no-identical-title was failing static analysis because two
cases shared the same describe title.
* fix(watch/relay): bound remote watcher fan-out and read the live relay grace
Three P1 fixes from the SSH/remote freeze audit:
- Remote watchers now debounce on the same 150/500 window as local ones
(finding D), and every teardown path drops the trailing flush timer
instead of letting it fire into a dead watch. The deferred send is
wrapped so a frame disposed mid-window can't escape as a fatal
main-process exception.
- File Explorer refreshes are scheduled and concurrency-capped rather
than fanned out unbounded over expanded dirs (finding C). Local
transports use a zero window, since main already coalesced the burst.
- relay.startGrace reads ptyHandler.configuredGraceTimeMs instead of the
launch-time argv closure, so a grace raised after launch is honored.
The branch selection moves to relay-grace-branch.ts because relay.ts
has no exports and calls main() at import, making it untestable.
Consequence: a host-sleep relay holding zero PTYs now exits after the
idle cap. Pinned by test and documented in
docs/reference/relay-grace-time-reconfiguration.md.
Also drops the duplicated 150/500/5000 constants in the runtime-RPC
batcher in favor of the shared window module.
* docs(relay): correct grace-reconfiguration line numbers after the relay.ts edit
Co-authored-by: Orca <help@stably.ai>
* refactor(file-explorer): use useMemo for paths; remove relay reference
Replace manual ref-based caching with proper React hooks for content-stable path memoization. Remove outdated relay grace-time reference documentation from code review cycle.
* rm design doc
* fix(watcher/explorer/relay): coalesce POSIX paths by byte identity; pres
- Remote watcher event coalescing now keeps NFC/NFD-distinct POSIX paths separate while still folding Windows path spellings, fixing cache invalidation when recreating directories with different Unicode compositions.
- Relay grace reconfiguration now preserves shutdown-deferred state when grace is set to zero, preventing premature shutdown when the grace timer is reconfigured mid-flight.
- File explorer refresh refactored from fixed-wave batching to concurrent task execution with `forEachWithConcurrency`, batching results every N settled reads instead of every wave, and reporting whether cancellation discarded pending work.
- Watch handler now resyncs when events arrive after disposal, ensuring refresh requests aren't lost when events race cleanup during worktree switches.
* fix(watcher/explorer): resilient commit batches on callback error
Cache writes precede callbacks, so a throwing callback cannot strand
the batch with stale marks. All callbacks complete despite errors,
with the first error thrown after.
- Fix commitBatchSize calculation for empty dirs
- Add scheduler discard-report test
- Move disposed variable before handleFsChanged
- Expand batching-strategy comments
* test(activity): stabilize portal readiness latch release under CI load
The fixed 9-flip budget could miss enough MutationObserver deliveries under
shard load for sibling DOM to still report loading. Drain readiness rAF after
each flip and wait until latched unavailable is actually observed.
* fix(watcher/explorer): stop batches on commit error; verify drain settle
Enhance resilience under CI load and error conditions:
- Portal readiness drain now returns a boolean confirming settlement; test expects verify the drain completed before proceeding.
- File explorer commit batches stop executing after a callback throws, preventing stale writes after caller-observed rejection.
- Watch hook requires worktree ID upfront in effect guard, eliminating redundant checks inside loops.
* fix(relay): re-arm grace on configured change
Refactor grace reconfiguration into a dedicated decision function. startGrace
samples the configured grace at arm time, so a raised value landing mid-window
would still fire at the old deadline without re-arming. Extract the logic with
proper tests to ensure shutdown-deferred state is preserved across re-arms.
* refactor(relay): extract grace reconfiguration to testable function
Extract `applyRelayGraceTimeConfiguration` so the grace-time re-arming
logic can be tested independently. relay.ts runs `main()` on import and
exports nothing, making the call site (including retryDeferredShutdown
hand-off into startGrace) otherwise untestable.
---------
Co-authored-by: Orca <help@stably.ai>
* fix(watch/relay): bound remote watcher fan-out and read the live relay grace
Three P1 fixes from the SSH/remote freeze audit:
- Remote watchers now debounce on the same 150/500 window as local ones
(finding D), and every teardown path drops the trailing flush timer
instead of letting it fire into a dead watch. The deferred send is
wrapped so a frame disposed mid-window can't escape as a fatal
main-process exception.
- File Explorer refreshes are scheduled and concurrency-capped rather
than fanned out unbounded over expanded dirs (finding C). Local
transports use a zero window, since main already coalesced the burst.
- relay.startGrace reads ptyHandler.configuredGraceTimeMs instead of the
launch-time argv closure, so a grace raised after launch is honored.
The branch selection moves to relay-grace-branch.ts because relay.ts
has no exports and calls main() at import, making it untestable.
Consequence: a host-sleep relay holding zero PTYs now exits after the
idle cap. Pinned by test and documented in
docs/reference/relay-grace-time-reconfiguration.md.
Also drops the duplicated 150/500/5000 constants in the runtime-RPC
batcher in favor of the shared window module.
* docs(relay): correct grace-reconfiguration line numbers after the relay.ts edit
Co-authored-by: Orca <help@stably.ai>
* refactor(file-explorer): use useMemo for paths; remove relay reference
Replace manual ref-based caching with proper React hooks for content-stable path memoization. Remove outdated relay grace-time reference documentation from code review cycle.
* rm design doc
* fix(watcher/explorer/relay): coalesce POSIX paths by byte identity; pres
- Remote watcher event coalescing now keeps NFC/NFD-distinct POSIX paths separate while still folding Windows path spellings, fixing cache invalidation when recreating directories with different Unicode compositions.
- Relay grace reconfiguration now preserves shutdown-deferred state when grace is set to zero, preventing premature shutdown when the grace timer is reconfigured mid-flight.
- File explorer refresh refactored from fixed-wave batching to concurrent task execution with `forEachWithConcurrency`, batching results every N settled reads instead of every wave, and reporting whether cancellation discarded pending work.
- Watch handler now resyncs when events arrive after disposal, ensuring refresh requests aren't lost when events race cleanup during worktree switches.
* fix(watcher/explorer): resilient commit batches on callback error
Cache writes precede callbacks, so a throwing callback cannot strand
the batch with stale marks. All callbacks complete despite errors,
with the first error thrown after.
- Fix commitBatchSize calculation for empty dirs
- Add scheduler discard-report test
- Move disposed variable before handleFsChanged
- Expand batching-strategy comments
---------
Co-authored-by: Orca <help@stably.ai>
* fix(ssh,relay): stop remote connections from being killed by backoff and frame caps
Three independent connection killers found in the SSH/remote freeze audit.
FINDING A - the reconnect ladder never escalated for post-handshake drops.
scheduleReconnect() used the single published state.reconnectAttempt for both
the delay index and the give-up test, and runReconnectAttempt() zeroed it
before connecting (ssh.ts gates the relay redeploy on 0-at-connected). Every
post-handshake drop therefore re-entered at 1000ms forever, ~3600 relay
redeploys/hour, and 'reconnection-failed' was unreachable for a flapping host.
New SshReconnectLadder splits the delay index (advanced by every retry) from
the failure streak (advanced only by a failed handshake), so flaps back off
while give-up semantics stay byte-identical to shipped.
FINDING B - notify() closed the client whenever a frame exceeded the producer
frame capacity, conflating a permanently un-sendable frame with transient
backpressure. A 5000-event fs.changed is 425KB against a 49KB cap, so the
watcher flood killed the link and re-killed on every reattach+replay. notify()
now drops and logs once per generation; fs.changed is chunked to each sink's
capacity with a control-lane overflow marker as the resync fallback; agent-hook
envelopes shed lastAssistantMessage/interactivePrompt/subagents to fit.
FINDING B2 - sendResponse routed >1MB responses to a lane whose admission
ignores the frame cap and closed the client on rejection, so a large
fs.listFiles dropped the SSH host. It now substitutes a JSON-RPC error so the
request fails instead of the connection.
Also moves fs.streamEnd/fs.streamError to the control lane so a terminal frame
cannot be dropped by the producer-lane check.
Co-authored-by: Orca <help@stably.ai>
* fix(relay): stop the overflow marker from re-killing the link it protects
Round-1 review fixes on the P0 freeze work.
The control-lane overflow marker could reinstate the exact failure this P0
removes: dispatcher-client-writer closes the client when control-lane
admission fails, and admitControl is the only lane that returns an error, so
one marker per failing batch accumulated to the 256-frame/1MB bound and
dropped the link. Markers are now deduped to one outstanding per
(client, root), cleared on settle.
Chunking also defeated the renderer's per-payload directory dedupe -- events
are now stable-grouped by parent directory so one directory lands in one
chunk -- and the halving walk overshot the byte minimum ~1.7x while the fast
path paid three JSON encodes; both are fixed by publishing first and sizing
from a measured bytes-per-event estimate.
Agent-hook shedding now surrenders the blocking interactive prompt LAST
rather than first, so a degraded envelope cannot strand a pane at
state=waiting with no answerable question card.
The dropped-notification log now distinguishes over-capacity from producer
queue backpressure and no longer lets the first dropped method silence every
other producer for the life of the connection.
* fix(relay,ssh): keep status delivery and terminal frames from trading one freeze for another
Round-2 review fixes.
The round-0 change from close-on-rejection to silent drop removed the only
redelivery path for agent.hook envelopes: they are fire-and-forget and the
per-pane cache only replays on handler install, so a saturated link stranded
a pane on a stale Working spinner until reconnect. Closing used to guarantee
delivery by forcing that replay. Envelopes now publish per client and pend
for bounded latest-wins redelivery when the producer queue rejects them.
Shed fields are now named on the wire. The subagent roster is not cosmetic --
the renderer replaces rather than merges it, and hibernation gates on its
length -- so an unmarked shed could sleep a live pane.
fs.streamEnd rode the control lane because it must not be dropped, but that
lane kills rather than drops. The stream's concurrency slot is now held until
the terminal frame settles rather than until the fd closes, capping queued
terminal frames well under the control budget; overflow costs one refused
read instead of the connection.
The watcher chunk walk now stops while producer retention sits past its
reserve and degrades to a resync, so a 5000-event flood cannot fill the queue
that interactive PTY traffic shares and stall every remote terminal.
The reconnect ladder caps its flap-path delay so delay plus handshake timeout
cannot cross the relay grace floor and let the remote daemon kill live PTYs.
Also: the suppression key no longer embeds a NUL byte, which had made the
file binary to git and grep; producerEnvelopeBudget no longer reports
infinite capacity for a departed client; the drop logger no longer encodes a
frame it will not log; and an over-capacity response substitution no longer
settles as if the result had been delivered.
* fix(relay,ssh): restore relay-shed status fields and scope backpressure per client
Round 3 + 4 review fixes.
Watcher chunking is now gated on the *client's* retention reserve rather than
the dispatcher-wide one, so one stalled peer no longer forces a healthy client
into a full file-tree resync. The relay-lost redeploy ladder no longer burns its
6-attempt budget while the SSH transport itself is down: it holds at the 15s step
with a non-terminal status and rearms, so a laptop that slept past the ladder
comes back instead of landing on a terminal "give up" banner.
The shedFields wire marker had no consumer, so an agent-hook envelope whose
subagent roster was dropped to fit the frame read as "roster cleared" on the Orca
side: live child rows blanked and a done pane became hibernation-eligible while
its teammates were still running. ingestRemote now restores shed fields from the
cached payload (interactivePrompt deliberately excluded — a stale answerable
question card is worse than none).
Also: stream terminal-frame slots are counted per client, since the control queue
they protect is per client; the chunking fast path no longer logs a drop for a
batch it goes on to deliver in full; -32010 is now RelayErrorCode.ResponseOverCapacity.
Test debt from the review: pending-pane eviction, per-client stream isolation, and
the reconnect budget are now asserted rather than assumed; four fragile exact-byte
pins dropped in favour of the tier comparisons that carry the requirement.
* fix(relay,ssh): restore relay-shed status fields and scope backpressure
- Oversized relay responses now fail their request instead of closing the connection,
preventing one frame from killing every pane on the host
- Restore subagent state for correct hibernation; don't resurrect stale prose
across turns
- Account for relay re-establishment and PTY reattach time in SSH flap delay caps
- Only log drops of final unsendable envelopes, not temporary rejections during
measurement probes
- Fix watcher overflow marker release race when notification admission rejects
without settlement; use precise byte counting for event batching
* Restore relay-shed fields with digest validation and scoped backpressure
Validate that shed subagent rosters match their wire digest and turn identity before
restoration, preventing stale roster resurrection. Compact interactive prompts for waiting
states instead of dropping them. Demote control-queue overflow to non-fatal rejection so
clients can retry on capacity recovery, keeping the link alive during transient backpressure.
* fix(relay): correct ResponseOverCapacity error code
ResponseOverCapacity should use -33008 to stay in the -33xxx range
for relay protocol errors, not -32010.
* fix(relay): close client when pty.replay overflows control queue
Replay is never retried, so it uses the control lane where overflow
is fatal — the writer closes the client and reconnect reloads history
rather than stranding a short buffer.
* fix(relay): prevent infinite redeploy on flapping SSH transports
Charge reconnect attempts when connection restores mid-backoff, preventing
infinite loop on transports that flap between states. Refactor control overflow
handling to use entry property instead of WeakSet marker for clarity.
---------
Co-authored-by: Orca <help@stably.ai>
Neil
闲人
github-actions[bot]
Jinjing