Repository navigation
feat: add quiet waits with per-request cancellation - #281
Draft
Alexey Zhokhov (donbeave) wants to merge 16 commits into
Draft
Alexey Zhokhov (donbeave) wants to merge 16 commits into
Alexey Zhokhov (donbeave) wants to merge 16 commits into
Conversation
Live capture reads grid, cursor, dimensions, title, and resolved palette under one terminal-state lock. Default capture records raw visual data; highlighted capture needs an explicit option. Signed-off-by: Alexey Zhokhov <alexey@zhokhov.com>
Signed-off-by: Alexey Zhokhov <alexey@zhokhov.com>
Signed-off-by: Alexey Zhokhov <alexey@zhokhov.com>
Signed-off-by: Alexey Zhokhov <alexey@zhokhov.com>
Add WaitQuietOptions (private fields, validated constructors) with visual-quiet and byte-quiet modes, Session::wait_quiet plus Session::wait_quiet_cancellable mirroring Engine::execute_with_context_cancellable, and a monotonic PTY output timestamp in private session state for byte-quiet timing. Signed-off-by: Alexey Zhokhov <alexey@zhokhov.com>
Add tests/request_waits.rs with live timing tests for visual and byte quiet modes, timeout and cancellation bounds, session reuse after each outcome, timer isolation, the sampling limit, single-aspect visual resets, request-start timing, and equal-deadline timeout. Signed-off-by: Alexey Zhokhov <alexey@zhokhov.com>
Signed-off-by: Alexey Zhokhov <alexey@zhokhov.com>
…emption) - wait.rs: single post-observation timestamp checks overall timeout before quiet success in both visual and byte arms, so a blocking capture straddling coincident deadlines reports timeout (U10-J). - wait.rs: visual mode sleeps one interval after the request-start capture before the first re-sample; the start sample seeds previous once instead of sampling twice back-to-back. - engine.rs/runtime.rs: wait_quiet_cancellable composes has_pending_lifecycle into its cancellation check so announced Close/Restart requests preempt a quiet wait; documented on both. Signed-off-by: Alexey Zhokhov <alexey@zhokhov.com>
Author
|
@microsoft-github-policy-service agree |
…ts (R25-part) R7: byte quiet no longer conflates a missing session with no output. Engine::last_output_time now returns Result<Option<Instant>>: missing sessions and emulator faults propagate as errors, and Ok(None) is reserved for a live session with no output yet. The visual arm already propagated capture errors. R8: a completed Close/Restart can no longer escape quiet-wait cancellation. The wait now runs through Engine::run_quiet_wait_cancellable, which holds the shared lifecycle lock before any caller closure runs (so a wait parked in its cancellation callback blocks lifecycle writers instead of letting them complete underneath it) and re-checks a stable session generation captured at request start before any success return. The pending-lifecycle counter alone is insufficient because it returns to zero once the writer acquires the exclusive lock while teardown still runs. R25-part: run_quiet_wait takes an injected private QuietClock; production uses real time while unit tests drive a scripted FakeClock with exact poll/sample/deadline/cancellation assertions and zero wall-clock timing. Barrier-controlled Close/Restart PTY tests use a controlled Rust child fixture with bounded joins; existing shell-based PTY suites are kept and still pass separately. Signed-off-by: Alexey Zhokhov <alexey@zhokhov.com>
…ness) GAP 1 (R8): quiet_lifecycle Close/Restart tests parked the wait, then allowed a three-second grace for Close to announce itself, with no acknowledgement barrier proving the lifecycle request reached the critical point -- scheduling decided which behavior the test exercised. Replace the grace with a barrier handshake: once the wait is parked inside its cancellation callback (holding the shared lifecycle lock), await_lifecycle_pin spins until Session::has_pending_lifecycle reports the peer announced (proving it reached the preemption point), failing outright if the peer completes first. The post-pin pending check is then a deterministic observation: the peer cannot acquire the exclusive lock until the parked wait yields. Adds Session::has_pending_lifecycle (delegating to Engine) as the barrier query. GAP 2 (R25): request_waits still used shell children (sh/powershell branches), sleep rendezvous, and elapsed-time assertions. Converted all 18 tests to the deterministic harness: a controlled Rust child fixture (current_exe re-invocation, seven scripted modes, stdin echo so Submit works without a shell), entry-gate barriers proving each wait reached its first poll before cancel/change/peer action, marker/Event ordering instead of elapsed bounds, bounded joins everywhere. Deleted all timing-assumption assertions; exact poll/sample/deadline counting stays in the injected FakeClock unit tests in src/wait.rs. Signed-off-by: Alexey Zhokhov <alexey@zhokhov.com>
Reject orphan continuations and leading cells inside unfinished spans in both try_new and deserialization, using an explicit remaining-span counter. Ratatui adapter rejects uncovered zero-width cells with a coordinate-bearing error. Signed-off-by: Alexey Zhokhov <alexey@zhokhov.com>
Live capture recomputed each cell width with display_width, but the backend can lay a glyph out narrower than it measures: heart+VS16 and the keycap sequence measure two columns yet occupy one with no spacer. The recomputed width opened a two-column span the next real cell could not cover, so valid live output failed with BadContinuation. Capture now reconciles the recomputed width against the backend's own occupancy record (the run of continuation spacers behind the cell): collapse to the occupied columns when the backend is narrower, cover the attested spacers when it is wider, and keep the measured width for the clipped-edge form at a row end. Zero-width graphemes still record width zero. Offline construction and deserialization keep the strict relational validator unchanged. Every capture that previously validated emits byte-identical cells; only previously-failing captures change.
Fix cancellable acquisition: the waiter no longer blocks in lifecycle.read() before checking cancellation or starting its deadline. Acquisition polls try_read while checking the caller's cancellation flag and a single request deadline that covers acquisition and observation together; observation then runs on the remaining budget. A free lock is taken before any caller-supplied closure runs so parking inside cancelled (or a slow observer) still pins lifecycle writers (R8); ordinary waits keep their poll-first order unchanged. Fix quiet-failure finalization: wait_quiet_cancellable now threads the session execution context into the shared internal wrapper, so timeouts on a live child finalize like ordinary failures (TimedOut details, failure artifact, recording marked failed and retained by OnFailure close) while cancellation keeps the Cancelled reason. Docs state the new capabilities (configurable quiet criteria, byte-vs-visual observation, Session-level access), the pre-existing timeout and cancellable execution, the TuiTestError return, and the microsoft#278 dependency plus sampling limitation.
Alexey Zhokhov (donbeave)
changed the title
Quiet-wait lock-acquisition errors (pre-cancelled, cancelled-while- acquiring, deadline) returned directly before history begin and failure finalization, yielding empty details/artifact/report with no history entry. Route all four acquisition-error returns through one finalize_acquisition_error helper that sets Cancelled/TimedOut, completes history, marks the recording outcome, and builds diagnostics from non-waiting lock attempts only — never acquiring the occupied lifecycle lock. Extend the three lifecycle-write contention tests with structured reason/history/recording assertions and add a live-session failure-file/OnFailure case. Signed-off-by: Alexey Zhokhov <alexey@zhokhov.com>
… failures (F3-A/F3-B) F3-A: finalize_failure_with_session skipped all exports when the terminal observation was absent, so quiet-wait acquisition errors that finalized while a lifecycle transition held the session/state locks returned details but no failure files. Add write_observation_free_artifact in diagnostics.rs (manifest + text report with operation/reason/context/history, terminal evidence marked unavailable, no locks taken) and use it from the export loop when observation is None. F3-B: finish_trace_with_session read the recording flag and wrote back the (stale) value across two separate locks, so a quiet acquisition failure landing in between was cleared and Close deleted the recording. Read the flag and apply an explicit outcome under one recording lock; a None outcome no longer writes back the saved value; the lock is released before file work. Tests: quiet_acquisition_timeout_during_startup_writes_failure_file, quiet_acquisition_cancel_with_busy_state_writes_failure_file, quiet_acquisition_failure_is_not_lost_during_close. Signed-off-by: Alexey Zhokhov <alexey@zhokhov.com>
write_observation_free_artifact now emits a bounded, escaped HTML fallback (operation, reason, context, history, 'terminal evidence unavailable') via write_optional_file kind report_html before the wants_json early return. Partial status when the report lands without terminal evidence; no lifecycle/session locks. Adds quiet_acquisition_html/all lib tests covering startup timeout and busy-state cancellation. 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.
Depends-On: #278
Incremental diff vs #278 head:
cb40f25...528bbc0
Summary
Adds configurable quiet-period waits (byte-level and visual observation)
with per-request cancellation and session-level access, so tests can wait
for output to settle instead of sleeping fixed durations.
New since the original PR head:
child finalize like ordinary failures (timeout details, failure artifact,
recording marked failed and retained), while cancellation keeps its
distinct cancelled reason.
honoring the caller's cancellation flag and one deadline covering
acquisition plus observation, then observes on the remaining budget.
available (manifest plus text report with operation, reason, context and
history; terminal evidence marked unavailable; no locks taken). Previously
acquisition errors that finalized while a lifecycle transition held the
session/state locks returned details but no failure files.
outcome applied under a single lock, so an acquisition failure landing
mid-close is no longer cleared and its recording is retained.
feat: add owned terminal snapshot with atomic live capture #278; docs state the feat: add owned terminal snapshot with atomic live capture #278 dependency and the sampling limitation.
Head:
528bbc008c44e8b6edaebb7b00f441c5f54b798f(
engine.rs,diagnostics.rs,wait.rs,runtime.rs,tests/request_waits.rs)Testing
Focused suites at head
528bbc0, run locally withcargo test(default features):cargo fmt --all -- --check: clean, no diffcargo clippy -p tui-test-rs --all-targets -- -D warnings: no warningscargo test -p tui-test-rs --test request_waits:21 tests run, 21 passed, 0 failed (covers the quiet-wait acquisition paths: bounded timeout/cancel, pre-cancelled waits, timeout/cancel finalization)
--test quiet_lifecycle: 4 passed, 0 failedcargo test -p tui-test-rs --lib quiet_acquisition:4 passed, 0 failed (new timeout-during-startup, cancel-with-busy-state,
and failure-not-lost-during-close cases, plus the live-session cancel case)
Known limitations
results are pending upstream CI on this PR.