Repository navigation
feat: add raw byte input for PTY sessions - #279
Draft
Alexey Zhokhov (donbeave) wants to merge 6 commits into
Draft
Alexey Zhokhov (donbeave) wants to merge 6 commits into
Alexey Zhokhov (donbeave) wants to merge 6 commits into
Conversation
Signed-off-by: Alexey Zhokhov <alexey@zhokhov.com>
Signed-off-by: Alexey Zhokhov <alexey@zhokhov.com>
Signed-off-by: Alexey Zhokhov <alexey@zhokhov.com>
Author
|
@microsoft-github-policy-service agree |
…anches R9: Session::write_bytes now checks the recorded exit/error state (exited/exit_error under one lock) before logging or writing. The kernel may still accept PTY bytes after the child dies, so a post-exit write previously reported Ok. Non-empty writes after a recorded exit or a failed status query return Internal; an empty payload stays a no-op while the session is open; Engine still reports NoSession after Close. No error-enum changes. R25-part: deterministic writer-injection tests pin every Pty::write branch (Interrupted, short writes, WouldBlock, WriteZero, fatal) with exact-bytes assertions. Signed-off-by: Alexey Zhokhov <alexey@zhokhov.com>
Engine::write_bytes locked the session and invoked the writer directly, bypassing viewport serialization, execution tracking, history, failure finalization, and the recording outcome. Route raw byte input through the same request pipeline as Operation::Write via a shared RequestPlan/execute_request helper plus write_raw_input, which keeps the recorded-exit-state rejection before anything is logged or written. Correct the contract in docs: the promise covers exact bytes handed to the PTY (subject to line discipline/ConPTY), NUL/ESC were already representable, invalid UTF-8 is the new capability, retry pre-existed in the shared PTY writer, and post-exit failure uses recorded exit state. Add regression tests: raw input returns the viewport to the bottom and is recorded in history; a failed raw write carries structured details and an artifact and retains the OnFailure recording across close.
Route the public Session::write_bytes through Engine::execute_raw_bytes with the session context instead of the context-free Engine::write_bytes path that used ExecutionContext::default, so configured failure artifacts, the operation name, and diagnostic context are honored. Keep the public context-free Engine::write_bytes for direct engine users. Add public_write_bytes_preserves_execution_context regression test. Signed-off-by: Alexey Zhokhov <alexey@zhokhov.com>
Alexey Zhokhov (donbeave)
added a commit
to tailrocks/tuiscotti
that referenced
this pull request
Oct 27, 2026
Migrate tuiscotti onto the tui-test engine as its terminal runtime. Upstream (`tui-test`) owns the terminal engine: child processes, emulation, sessions, input, waiting, rendering, fonts, and pixel comparison. Tuiscotti keeps the application testing layer: pure-view capture, versioned samples, frozen baseline checks, explicit approval, static reports, and the offline CLI (`check` / `accept` / `render` / `report`, exit codes 0/1/2/3). The previous built-in live engine, renderer, recorder, and second approval engine are removed, leaving the two shipping crates (`tuiscotti`, `tuiscotti-cli`). ## Provisional fork dependency The new `tui-test` APIs this PR needs are still unmerged upstream, so this branch temporarily depends on a pinned fork via `[patch]`: - `donbeave/tui-test`, branch `goal/qual-stack-r5`, pinned at `81da26fa9dc29664a897dc802be816a4432c0618` The pin is provisional. Once the upstream PRs below merge and a registry release contains the new APIs, the `[patch]` is removed in favor of the registry version. ## Upstream PRs (all 11, all open) | PR | Change | Head SHA | Status | |----|--------|----------|--------| | [microsoft/tui-test#276](microsoft/tui-test#276) | Explicit read-only snapshot policy | `fc0150160c25ea83532b18f9507f9f1a155b91e4` | Reworked + verified | | [microsoft/tui-test#277](microsoft/tui-test#277) | RGBA image comparison for pixel equality | `4fb316aa56c65cc659b4a43ee4921c45e33cecc0` | Approved as-is; dependency-only, no rework | | [microsoft/tui-test#278](microsoft/tui-test#278) | Owned terminal snapshot API | `334c474d1c621c373333e9dc24e549623a133570` | Reworked + verified | | [microsoft/tui-test#279](microsoft/tui-test#279) | Raw byte input for Rust sessions | `3b1439c313e88d0a8867dfbcdadabb28624dc5b5` | Reworked + verified | | [microsoft/tui-test#280](microsoft/tui-test#280) | Public rendering for offline snapshots | `378036846a4e85174104e0f9c153b0a0f8f84161` | Reworked + verified | | [microsoft/tui-test#281](microsoft/tui-test#281) | Configurable quiet waits and request cancellation | `103d296494f251921f8cd61d3f6ce2f7e152690d` | Reworked + verified | | [microsoft/tui-test#282](microsoft/tui-test#282) | Mode-aware programmatic input | `212ba1db26987a45537708e98c82db77c90bb734` | Reworked + verified | | [microsoft/tui-test#283](microsoft/tui-test#283) | Explicit underline colors in rendering | `fff55c122ae210efbe9a68d1ac8683b1a8856d` | Reworked + verified | | [microsoft/tui-test#284](microsoft/tui-test#284) | Explicit pinned-font rendering | `2bdaa3f8d3692514a6cb9c1ab844e880ca8d0f23` | Reworked + verified | | [microsoft/tui-test#285](microsoft/tui-test#285) | Combined and wide cell text rendering | `edf2b525d6f8a5710cd56440545437ff4f146727` | Reworked + verified | | [microsoft/tui-test#286](microsoft/tui-test#286) | Explicit child launch settings | `d5d9bbab9ac4e8a5e0562175497661b7b1a4b158` | Reworked + verified | "Reworked + verified" means the PR went through a rework round after independent review, and its per-task suites plus the integrated stack re-ran green. #277 needed no changes — its head is unchanged since review — so it is listed as a pure dependency. Upstream merge order: #278 before #280 and #281; #280 before #283 and #284; #284 before #285. #276, #277, #279, #282, and #286 are independent and can merge in any order. ## Verification (at PR head `a056f8f60bae1bf81429b575076c86aed3e6717b`) - Tests: 209/209 passing in both the default-features and no-default-features lanes. - `cargo fmt --check` clean; `cargo clippy --all-targets -D warnings` clean (default and no-default features). - Downstream CI: [run 40408516574](https://github.com/tailrocks/tuiscotti/actions/runs/40408516574) (in progress, at `a056f8f`); prior run [40390626472](https://github.com/tailrocks/tuiscotti/actions/runs/40390626472) success at `9ae97ff`. ## Known limitations - Windows/macOS runner coverage is still pending; platform-specific behavior (for example raw-byte PTY paths) is unverified there. - The upstream PRs currently report only CLA checks; Rust CI on the upstream side still needs maintainer workflow approval, so upstream-side proof rests on the recorded per-task suites plus this downstream integration. - No crates.io release contains the new APIs yet, so registry adoption is blocked until the upstream PRs merge and release. The provisional fork pin above supports merging this PR only; it is not a release path. ## Depends-on / merge order This PR depends on all eleven upstream PRs merging (in the order noted above) plus a registry release; only then does the provisional `[patch]` come out. Nothing in this PR can ship to crates.io before that. Upstream maintainers: the stack is additive-only, and the pinned fork branch tracks the exact heads listed above. --------- Signed-off-by: Alexey Zhokhov <alexey@zhokhov.com> Signed-off-by: Muse Code <muse-code@meta.com> Signed-off-by: Muse Code <muse@example.local> Co-authored-by: Muse Code <muse-code@meta.com> Co-authored-by: Muse Code <muse@example.local>
This branch has not been deployed
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.
Title: feat: add raw byte input for PTY sessions
Summary
Adds
Session::write_bytes(backed byEngine::write_bytesand a PTY rawinput path) so tests can send exact bytes — including NUL, ESC, and invalid
UTF-8 — instead of only valid text.
The latest update on top of the original PR head routes raw input through
the same request pipeline as ordinary text writes (shared plan/execute
helper): viewport serialization, execution tracking, history, failure
finalization, and recording outcomes now apply equally. The recorded-exit
rejection still happens before anything is logged or written. Contract docs
are corrected: the promise covers exact bytes handed to the PTY (subject to
line discipline/ConPTY), NUL/ESC were already representable, invalid UTF-8
is the new capability, and post-exit failure uses recorded exit state.
Regression tests cover viewport return, history recording, and structured
failure details with artifact retention.
Head:
97f09f05453ce605fa4bc1bbeef245b320148dcb(
engine.rs,runtime.rs,terminal/pty.rs,diagnostics/input.rs)Testing
Focused suite at head
effb070, run locally withcargo test(default features):cargo test -p tui-test-rs --test raw_bytes:5 tests run, 5 passed, 0 failed (includes new
public_write_bytes_preserves_execution_context)Known limitations
translation; Windows and macOS results are pending upstream CI.