mirror of
https://github.com/stablyai/orca.git
synced 2026-09-22 00:02:31 +00:00
fix(terminal): make DECSET 2031 subscriptions silent (#13904)
fish arms `CSI ?2031h` before painting each prompt and withdraws it when it hands the tty to a child — a ~1ms window. Orca answered that subscribe with `CSI ?997;Nn` across a 1-3ms renderer hop, so the reply landed after the withdrawal and was read as stdin by the next child, corrupting `brew`/`npx` `[y/N]` prompts. The reply is not stale by Orca's own view when written (measured staleReplies: 0), so no suppress-the-stale-reply scheme can close this — the information needed to suppress does not exist yet. Nothing asked for the reply either. The Contour spec says a terminal "should only send out the DSR when the palette has been updated"; Ghostty (Termio.zig:729 — force=true reachable only from the ?996n DSR), iTerm2 (VT100Terminal.m:995 — flag only) and xterm.js (InputHandler.ts:2035 — flag only) all emit nothing on the DECSET. So stop entering the race: record the subscription, answer nothing. Of 17 real programs measured under a pty, only fish, tmux, claude and opencode subscribe; none block on a reply, and answering produces one redundant palette re-query and zero rendering difference. tmux is the only one that sends `?996n`, which Orca still answers. - Subscribes are record-only at all four emitters (live scan, hidden-gate fact, parked byte watcher, parked responder — the last is deleted, it only replied). - `?996n` answers, the subscription registry, and the theme-flip push are unchanged. `paneLastThemeMode` is still seeded at subscribe so the next appearance re-apply is not read as a flip. - Replay grammar carries `?2031l` alongside `?2031h`, so a late-attaching remote client no longer registers a subscription the TUI already retired. Also closes fish-integration gaps found alongside: `unset` (which fish lacks) becomes `set -e` on paths parsed by the client's login shell, `config.fish` is parsed for agent-home detection, and bracketed-paste startup delivery is made consistent across local/daemon/relay. Regression test drives real fish 4.7.1 under node-pty and asserts on what the child process reads; it fails against pre-fix code with the exact payload from the issue. CI installs fish 4 and fails loudly rather than skipping. Closes #9993 Co-authored-by: Orca <help@stably.ai>
This commit is contained in:
@@ -176,6 +176,12 @@ jobs:
|
||||
shell_contracts:
|
||||
name: shell contracts
|
||||
runs-on: ubuntu-latest
|
||||
env:
|
||||
# Why: the suites below gate their live fish tests on the binary, which is
|
||||
# right on a developer machine and wrong here — this job is a required check
|
||||
# and its fish lane is the only end-to-end guard for #9993, so a skip would
|
||||
# report green with nothing exercised. Turns those skips into failures.
|
||||
ORCA_REQUIRE_FISH: '1'
|
||||
|
||||
steps:
|
||||
- name: Checkout
|
||||
@@ -186,8 +192,46 @@ jobs:
|
||||
# Why fish: shell-ready.test.ts gates its live fish test on the binary being
|
||||
# present, so without this the fish barrier is only covered by config-shape
|
||||
# assertions and never actually exercised.
|
||||
# Why release-4: DECSET 2031 arming lives in the fish 4.0 Rust tty_handoff, and
|
||||
# fish-color-scheme-child-stdin.node-pty.test.ts (#9993) needs it. Noble ships
|
||||
# 3.7, so the PPA is what makes that lane real.
|
||||
- name: Install zsh and fish
|
||||
run: sudo apt-get update && sudo apt-get install -y zsh fish
|
||||
run: |
|
||||
# Why the update/PPA/fish steps are tolerant: a repo the runner image already
|
||||
# ships can lack a Release file for this suite, and a failed add-apt-repository
|
||||
# still leaves its list entry behind — either makes `apt-get update` exit
|
||||
# non-zero and would red this required check over something unrelated to the
|
||||
# PR. Every fish outcome is judged by the version gate below instead, so only
|
||||
# the zsh install (which has no such gate) stays fatal here.
|
||||
sudo apt-get update || true
|
||||
# Why retry only here: adding the PPA is the network-flaky step, and the
|
||||
# version gate below is fatal, so a transient Launchpad blip would
|
||||
# otherwise red a required check on PRs unrelated to shells.
|
||||
for attempt in 1 2 3; do
|
||||
sudo add-apt-repository -y ppa:fish-shell/release-4 && break
|
||||
echo "add-apt-repository attempt ${attempt} failed; retrying" >&2
|
||||
sudo add-apt-repository -y -r ppa:fish-shell/release-4 || true
|
||||
sleep 5
|
||||
done
|
||||
sudo apt-get update || true
|
||||
# Why both shells on one line: pr-workflow-parallelism.test.mjs parses only the
|
||||
# first install command in this step to prove the lane really installs them.
|
||||
sudo apt-get install -y zsh fish
|
||||
|
||||
# Separate from the install so the failure names the contract, not an apt error.
|
||||
# ORCA_REQUIRE_FISH re-checks this at test time; this step just fails in seconds
|
||||
# instead of after a full dependency install.
|
||||
- name: Require fish 4+
|
||||
run: |
|
||||
version="$(fish --version 2>/dev/null || true)"
|
||||
major="${version##*version }"
|
||||
major="${major%%.*}"
|
||||
case "$major" in '' | *[!0-9]*) major=0 ;; esac
|
||||
echo "${version:-<fish not installed>}"
|
||||
if [ "$major" -lt 4 ]; then
|
||||
echo "::error::shell contracts needs fish 4+ (DECSET 2031 arming, #9993) but got '${version:-none}'. Fix the ppa:fish-shell/release-4 install rather than letting the fish lane skip." >&2
|
||||
exit 1
|
||||
fi
|
||||
|
||||
- uses: ./.github/actions/install-node-dependencies
|
||||
with:
|
||||
@@ -201,6 +245,7 @@ jobs:
|
||||
src/main/providers/local-pty-shell-ready.test.ts \
|
||||
src/main/providers/__tests__/shell-ready-framework-example.test.ts \
|
||||
src/main/pty/omp-shell-wrapper.node-pty.test.ts \
|
||||
src/renderer/src/components/terminal-pane/fish-color-scheme-child-stdin.node-pty.test.ts \
|
||||
src/shared/posix-command-path-lookup.test.ts
|
||||
|
||||
test:
|
||||
@@ -235,6 +280,7 @@ jobs:
|
||||
--exclude=src/main/providers/local-pty-shell-ready.test.ts \
|
||||
--exclude=src/main/providers/__tests__/shell-ready-framework-example.test.ts \
|
||||
--exclude=src/main/pty/omp-shell-wrapper.node-pty.test.ts \
|
||||
--exclude=src/renderer/src/components/terminal-pane/fish-color-scheme-child-stdin.node-pty.test.ts \
|
||||
--exclude=src/shared/posix-command-path-lookup.test.ts \
|
||||
--exclude=tests/e2e/cross-version-wire/** \
|
||||
--shard=${{ matrix.shard }}/${{ matrix.shard_total }}
|
||||
|
||||
Reference in New Issue
Block a user