Skip to content

fix: reject non-interactive tui - #51

Merged
vyncint merged 1 commit into
vyncint:mainfrom
Rayan-and-beyond:fix/tui-non-interactive-35
Sep 13, 2026
Merged

vyncint merged 1 commit into
vyncint:mainfrom
Rayan-and-beyond:fix/tui-non-interactive-35

Conversation

@Rayan-and-beyond

Copy link
Copy Markdown
Contributor

Closes #35.

Rejects oxide tui before terminal initialization when stdin or stdout is not attached to an interactive terminal. The CLI now prints the requested guidance and exits with usage code 2 instead of leaking the raw terminal initialization error.

The TUI entry point uses the same preflight so direct callers fail cleanly as well. This also adds an assert_cmd regression and an Unreleased changelog entry.

Validation:

  • red/green regression: pristine base exits 1 with the raw terminal error; the fix exits 2 with the expected message
  • cargo fmt --all -- --check
  • Clippy: default, CUDA, and predict feature lanes
  • cargo test --workspace --locked --timings
  • predict test lane
  • CUDA compile check
  • all-feature rustdoc and dependency-coherence checks
  • cargo deny --all-features check
  • release scripts, crate metadata, CI script tests, and skill-version check
  • zizmor --persona=pedantic --offline .github/workflows/
  • MSRV check on Rust 1.94.1
  • macOS Metal lint/tests/device conformance
  • DCO and no-AI-attribution policy checks

make gate reaches check-predict-no-second-cuda and fails because ctor is already present in the predict/CUDA dependency graph. The same check fails identically on pristine upstream 28c42fc; this change does not touch manifests or dependencies. The gate stages after that abort point were run individually and passed.

Signed-off-by: Rayan-and-beyond <263488867+Rayan-and-beyond@users.noreply.github.com>

@vyncint vyncint left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approved and merging as is. This does exactly what #35 asked and a little more, and the extra is the right extra.

I ran it rather than reading it:

  • oxide tui </dev/null >file now exits 2 with oxide tui needs an interactive terminal; use oxide sql or oxide explain for non-interactive output on stderr and nothing on stdout. Before the fix that path exited 1 with the raw No such device or address (os error 6), which is what the issue was filed about.
  • tui_rejects_non_interactive_terminal passes, and it pins the exit code and the exact stderr rather than just "it failed".
  • Requiring both stdin and stdout to be terminals is the correct reading. A dashboard needs stdin for keys, so oxide tui | less and oxide tui </dev/null should both be refused, and they are.
  • Guarding inside run_terminal as well as in the binary is worth the small duplication: a library caller gets a typed NotConnected error instead of whatever ratatui::try_init would have surfaced, and the demo binary inherits it.
  • The real-PTY path is untouched. oxide_tui_pty.rs drives oxide tui through an actual PTY where both handles are terminals, and the default test lane is green, so the guard does not fire where the dashboard is meant to run.

CHANGELOG entry, exit code and message all match the issue's "done when".

One correction to the PR description, so nobody chases it. make check-predict-no-second-cuda does not fail on pristine 28c42fc. I ran it there and on your branch, with both the rustup cargo and the Homebrew one that shadows it on my machine, and all four runs print one cudarc (OxideLake's own), no oxmera-cuda, no pre-main ctor and exit 0. CI runs the same target in the predict lane and it is green on this PR. So ctor is not in the predict/CUDA graph; something in that checkout resolved differently. Worth a git status and a fresh cargo tree -p oxidelake-runtime --features predict,cuda -e normal next time it happens, because that fence is load-bearing: it is what stops oxmera creating a second CUDA context before main.

Thanks for the careful red/green note and for listing what you ran. That is the part that made this quick to check.

@vyncint
vyncint merged commit 731dc94 into vyncint:main Sep 13, 2026
17 checks passed
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.

oxide tui: detect a non-interactive terminal and explain instead of ENXIO

2 participants