Skip to content

feat: add raw byte input for PTY sessions - #279

Draft
Alexey Zhokhov (donbeave) wants to merge 6 commits into
microsoft:mainfrom
tailrocks:goal/u09-raw-bytes
Draft

Alexey Zhokhov (donbeave) wants to merge 6 commits into
microsoft:mainfrom
tailrocks:goal/u09-raw-bytes

Conversation

@donbeave

@donbeave Alexey Zhokhov (donbeave) commented Oct 25, 2026 •

Copy link
Copy Markdown

Title: feat: add raw byte input for PTY sessions

Summary

Adds Session::write_bytes (backed by Engine::write_bytes and a PTY raw
input 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 with cargo 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

  • Byte delivery is subject to platform line discipline and ConPTY
    translation; Windows and macOS results are pending upstream CI.
  • Merging needs upstream maintainer review and green CI.

Signed-off-by: Alexey Zhokhov <alexey@zhokhov.com>
Signed-off-by: Alexey Zhokhov <alexey@zhokhov.com>
Signed-off-by: Alexey Zhokhov <alexey@zhokhov.com>
@donbeave

Copy link
Copy Markdown
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.
@donbeave Alexey Zhokhov (donbeave) changed the title Add raw byte input for Rust sessions feat: add raw byte input for PTY sessions Oct 26, 2026
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

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant