Reapply the native single-view adapter with a terminating parity E2E - #1117
Merged
Conversation
Move `smallestClientSize` to the pure shared resize-protocol module so the parity corpus no longer imports pty-server, whose module-load `Bun.serve` kept the standalone E2E alive after its checks passed. Dispose every reconnect controller the native harness hands out, exit the E2E explicitly, and cap its CI step with a timeout. Declare `shell: bash` on the sharded suite steps: the default `run:` shell has no pipefail, so `vitest | tee` recorded a failing suite as successful and the aggregate gate went green over a real failure. Add guards for both regressions and classify the adapter in the tmux audit inventory.
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
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.
Hi, this is Claude (the AI assistant that wrote #1111 and reverted it in #1113). This reapplies that work with the two defects that caused the revert actually fixed, plus the real test failure it had been hiding.
1. The parity E2E never exited
Checks passed in ~18s, then the process hung until the job's
timeout-minutes: 20killed it — on all three platforms.Two open handles kept the Bun event loop alive:
terminal-parity/checks.tsimported the pure helpersmallestClientSizefrompty-server.ts, which callsBun.serve({...})at module load. The E2E's very first log line was[pty] PTY WebSocket server running on ws://localhost:40905, printed before its own output. The helper now lives in the puresrc/shared/resize-protocol.ts(its unit tests moved with it), so the corpus no longer touches that module.reconnect()controllers it never disposed, so an attach WebSocket opened by a reconnect check stayed open. All of them are tracked and disposed now.Belt and braces: the E2E ends with an explicit
process.exit(0/1)like every sibling native e2e, and its CI step getstimeout-minutes: 5so a future hang fails fast instead of eating the job budget.Verified by running the E2E with the explicit exit removed — it terminates on its own in 19s, which shows the leak is gone rather than papered over. An isolation test now fails if
checks.ts/corpus.ts/runner.tsever importspty-serveragain.2. The sharded test gate was silently passing over a real failure
This one is on me. My reporting improvement piped each suite through
tee, and the defaultrun:shell isbash -e {0}— no pipefail (visible in the logs asshell: /usr/bin/bash -e {0}). The pipeline reported tee's exit code, so a failing suite was recorded asoutcome: success.Concretely: #1111's first run correctly said
shard 4/5: cli=failure; after the tee change every shard went green while shard 4's log still containedFAIL tmux-audit/__tests__/tmux-audit.test.ts. That is how a red PR auto-merged.Fixed by declaring
shell: bash(which is-eo pipefail) on the three suite steps, guarded bysrc/bun/__tests__/workflow-pipefail.test.ts— verified by removingshell: bashfrom one step and watching the test name it.3. The failure it was hiding
tmux-auditgenuinely failed: the four adapter production files mention tmux (only in comments asserting it never touches tmux) and no rule classified them. Added a^src/bun/native-terminal-adapter/rule ascompatibility / MIG-006 / isolation / reference, mirroring the native registry, and regeneratedinventory.json/inventory.md.Verification
bun run lintclean;bun run testgreen (mainview 2931 / bun 3228 / cli 600); the tmux live parity e2e green (14 tests); the native parity E2E printsALL CHECKS PASSEDand exits0in 19s. Decisions 165 and 166 record both root causes.Note: one bun test flaked a single time right after the rebase and did not reproduce in two subsequent full runs; the grouped output did not name it, so I could not identify it — flagging it rather than claiming it was nothing.