Skip to content

fix(install): require terminal for Station DeepSeek express - #7045

Merged
apurvvkumaria merged 5 commits into
mainfrom
codex/7014-station-deepseek-tty
Jul 17, 2026
Merged

fix(install): require terminal for Station DeepSeek express#7045
apurvvkumaria merged 5 commits into
mainfrom
codex/7014-station-deepseek-tty

Conversation

@apurvvkumaria

@apurvvkumaria apurvvkumaria commented Jul 16, 2026

Copy link
Copy Markdown
Collaborator

Summary

Require --station-deepseek to have a readable interactive terminal so the installer fails before host mutation instead of silently selecting a different configuration. This is a clean current-main replacement for #7018 that preserves Yanyun Liao's original implementation and contributor credit.

Related Issue

Fixes #7014

Changes

  • Validate stdin or /dev/tty during the explicit Station DeepSeek preflight, before Docker or build-dependency setup.
  • Add a cross-platform no-controlling-terminal regression that proves the flag fails without host mutation while normal PTY and conflict paths remain covered.
  • Document the terminal requirement, the curl | bash behavior, and the equivalent headless managed-vLLM configuration in installer help, quickstart, vLLM setup, and generated platform support.

Type of Change

  • Code change (feature, bug fix, or refactor)
  • Code change with doc updates
  • Doc only (prose changes, no code sample modifications)
  • Doc only (includes code sample changes)

Quality Gates

  • Tests added or updated for changed behavior
  • Existing tests cover changed behavior — justification:
  • Tests not applicable — justification:
  • Docs updated for user-facing behavior changes
  • Docs not applicable — justification:
  • Sensitive paths changed (security, policy, credentials, preflight, onboarding, inference, runner, sandbox, or messaging)
  • Sensitive-path review completed or maintainer-approved waiver recorded — reviewer/approval link/justification: Independent maintainer review found no security finding. The local terminal probe is fail-closed, uses the existing installer TTY boundary, and the regression proves failure occurs before Docker or build-dependency mutation.
  • Non-success, skipped, or missing CI check accepted by maintainer — check name, approval link, and follow-up issue:

Verification

  • PR description includes a Signed-off-by: line and every commit appears as Verified in GitHub
  • Normal pre-commit, commit-msg, and pre-push hooks passed, or npm run check:diff passed when hooks were skipped or unavailable
  • Targeted behavior tests pass for the current change set, or tests are marked not applicable above — npx vitest run --project installer-integration test/install-express-prompt.test.ts (26 passed, 1 skipped); installer help rendering and platform-doc sync also passed.
  • Applicable broad gate passed — npm test for broad runtime/test-harness changes; npm run check for repo-wide validation/coverage changes — npm run check:diff passed against current origin/main.
  • Quality Gates section completed with required justifications or waivers
  • No secrets, API keys, or credentials committed
  • npm run docs builds without warnings (doc changes only) — build passed with 0 errors; Fern reported two pre-existing hidden/default warnings.
  • Doc pages follow the style guide (doc changes only)
  • New doc pages include SPDX header and frontmatter (new pages only) — not applicable; no new page was added.

Signed-off-by: Yanyun Liao yanyunl@nvidia.com
Signed-off-by: Apurv Kumaria akumaria@nvidia.com

Summary by CodeRabbit

  • Bug Fixes

    • Enforced that DGX Station --station-deepseek interactive express install requires an interactive terminal; the installer now fails fast with a clear error when terminal access isn’t available.
    • If the interactive prompt can’t be read, installation stops before any onboarding/express steps proceed.
  • Documentation

    • Clarified DGX Station installer behavior, /dev/tty/terminal requirements for --station-deepseek, and the deferred status of end-to-end express onboarding on physical hardware.
    • Added clearer headless/non-interactive examples using environment variables.
  • Tests

    • Added failure-mode and prompt read-resiliency coverage for non-interactive terminal scenarios.

yanyunl1991 and others added 3 commits July 16, 2026 13:06
…7014)

--station-deepseek selects the interactive DGX Station express prompt. With no
TTY, maybe_offer_express_install logged "Skipping express prompt (no TTY)" and
continued, silently ignoring the flag and installing a different configuration.
This surfaces once #7008 stops notice acceptance from forcing non-interactive,
which lets accept-notice + --station-deepseek reach the express step.

Add a TTY-availability check to validate_station_deepseek_override so the flag
fails fast at preflight (before Docker / build deps) with a clear message,
mirroring the existing --non-interactive rejection. Checked last so a genuine
provider/model conflict is still reported first. A curl|bash pipe with /dev/tty
available (the normal interactive case) still proceeds.

Verified on a DGX-Station-mocked aarch64 host: no-TTY --station-deepseek now
errors before any host mutation; --non-interactive and conflicting-model errors
still take precedence; a real pty proceeds.

Stacked on #7010 (#7008/#7009), which this builds on.

Signed-off-by: Yanyun Liao <yanyunl@nvidia.com>
Signed-off-by: Apurv Kumaria <akumaria@nvidia.com>
Co-authored-by: Yanyun Liao <yanyunl@nvidia.com>
Signed-off-by: Apurv Kumaria <akumaria@nvidia.com>
Document the interactive terminal requirement.

Show the equivalent headless managed-vLLM configuration.

Co-authored-by: Yanyun Liao <yanyunl@nvidia.com>
Signed-off-by: Apurv Kumaria <akumaria@nvidia.com>
@apurvvkumaria apurvvkumaria added bug-fix PR fixes a bug or regression v0.0.85 labels Jul 16, 2026
@apurvvkumaria apurvvkumaria self-assigned this Jul 16, 2026
@coderabbitai

coderabbitai Bot commented Jul 16, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: d9b7cb2a-a077-4705-a908-ae59fee0ff7f

📥 Commits

Reviewing files that changed from the base of the PR and between 96c635e and d0b6de9.

📒 Files selected for processing (3)
  • docs/get-started/quickstart.mdx
  • scripts/install.sh
  • test/install-express-prompt.test.ts
🚧 Files skipped from review as they are similar to previous changes (3)
  • test/install-express-prompt.test.ts
  • docs/get-started/quickstart.mdx
  • scripts/install.sh

📝 Walkthrough

Walkthrough

DGX Station’s --station-deepseek express-install flow now requires a readable interactive terminal and fails before setup mutations when unavailable. Documentation describes interactive and headless alternatives, while platform metadata records supported recipes and deferred hardware validation.

Changes

DGX Station install flow

Layer / File(s) Summary
Interactive terminal validation
scripts/install.sh, test/install-express-prompt.test.ts
The installer checks stdin or /dev/tty, reports an error when unavailable, and tests failures before Docker or build mutations and after prompt-read failures.
Express-install guidance
docs/get-started/quickstart.mdx, docs/inference/set-up-vllm.mdx
Documentation explains interactive prompt requirements and provides environment-variable examples for headless managed-vLLM setup.
Platform support metadata
ci/platform-matrix.json, docs/reference/platform-support.mdx
DGX Station notes describe express recipes, model downloads, alternative flags, and deferred physical-hardware validation; the platform table row is re-rendered.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Possibly related PRs

Suggested labels: area: install, platform: dgx-station

Suggested reviewers: ericksoa, sandl99

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 14.29% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly matches the main change: requiring a terminal for Station DeepSeek express installs.
Linked Issues check ✅ Passed The PR implements the requested clear error when --station-deepseek is used without an interactive terminal.
Out of Scope Changes check ✅ Passed The docs, tests, and installer updates all support the terminal requirement and stay within the linked issue scope.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch codex/7014-station-deepseek-tty

Comment @coderabbitai help to get the list of available commands.

@github-actions

Copy link
Copy Markdown
Contributor

@github-code-quality

github-code-quality Bot commented Jul 16, 2026

Copy link
Copy Markdown
Contributor

Code Coverage Overview

Languages: TypeScript

TypeScript / code-coverage/plugin

The overall coverage remains at 96%, unchanged from the main branch.


Updated July 17, 2026 00:01 UTC
Code Coverage is in Public Preview. Learn more and provide us with your feedback.

@github-actions

github-actions Bot commented Jul 16, 2026

Copy link
Copy Markdown
Contributor

PR Review Advisor — Informational

Advisor assessment: Informational / high confidence
Next action: No advisor follow-up needed.
Findings: 0 blockers · 0 warnings · 0 suggestions
Status: No actionable findings remain in the canonical review ledger.

Model lanes

  • GPT-5.6 Terra (primary): Completed · high confidence · 0 blockers · 0 warnings · 0 suggestions
  • Nemotron 3 Ultra (second opinion): Completed · high confidence · 0 blockers · 0 warnings · 0 suggestions
  • Model comparison: normalized findings match; normalized E2E selections differ; severity counts match.

Nemotron output stays in workflow artifacts and does not change the assessment above.

E2E guidance

Advisory only. E2E / PR Gate selects and runs jobs independently.

Recommended E2E: cloud-onboard

1 optional E2E recommendation
  • docs-validation

Workflow run details

This automated review informs maintainers. Warnings and suggestions do not require a response. A maintainer decides whether to merge.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@test/install-express-prompt.test.ts`:
- Around line 448-507: Wrap the temporary-directory setup and all assertions in
the test block around the station deepseek invocation with a try/finally, and
move fs.rmSync(tmp, { recursive: true, force: true }) into the finally block.
Preserve the existing test execution and assertions while guaranteeing cleanup
when any assertion or process operation fails.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: aace1854-e9d6-40ed-888b-9174e0486800

📥 Commits

Reviewing files that changed from the base of the PR and between 41a5885 and 1b9472f.

📒 Files selected for processing (6)
  • ci/platform-matrix.json
  • docs/get-started/quickstart.mdx
  • docs/inference/set-up-vllm.mdx
  • docs/reference/platform-support.mdx
  • scripts/install.sh
  • test/install-express-prompt.test.ts

Comment thread test/install-express-prompt.test.ts
Fail closed when the explicit Station prompt read fails after preflight.

Preserve ordinary opt-in prompt skips and guarantee temporary test cleanup.

Co-authored-by: Yanyun Liao <yanyunl@nvidia.com>
Signed-off-by: Apurv Kumaria <akumaria@nvidia.com>
@apurvvkumaria
apurvvkumaria enabled auto-merge (squash) July 16, 2026 23:28
Signed-off-by: Apurv Kumaria <akumaria@nvidia.com>
@apurvvkumaria
apurvvkumaria merged commit da3eb35 into main Jul 17, 2026
73 of 75 checks passed
@apurvvkumaria
apurvvkumaria deleted the codex/7014-station-deepseek-tty branch July 17, 2026 00:06
cv pushed a commit that referenced this pull request Jul 17, 2026
<!-- markdownlint-disable MD041 -->
## Summary

Add the canonical dated changelog entry required before the v0.0.85
release plan can be generated.
The entry summarizes the user-visible OpenShell, DGX Station, inference,
MCP, onboarding, and recovery changes merged since v0.0.84 and links to
their owning guides.

## Changes

- Add `docs/changelog/2026-07-16.mdx` with the exact `## v0.0.85`
heading, parser-safe SPDX comment, release summary, and detailed
bullets.
- Link every documented theme to its most specific published OpenClaw
guide routes.
- Reconcile the release entry with these merged source PRs:
- #6726 -> `docs/changelog/2026-07-16.mdx`: Document the supported
OpenShell v0.0.85 upgrade, immutable consumed artifacts, multiline exec,
credential rewrite diagnostics, and child-process TLS boundary.
- #6986 -> `docs/changelog/2026-07-16.mdx`: Document managed MCP
behavior shared across supported agents.
- #6991 and #7045 -> `docs/changelog/2026-07-16.mdx`: Document qualified
DGX Station host preparation and the interactive-terminal boundary for
`--station-deepseek`.
- #6992, #7001, #7006, and #7044 -> `docs/changelog/2026-07-16.mdx`:
Document managed-model reasoning behavior, safe inference route
mutation, and verified vLLM served aliases.
- #6865, #7010, and #7028 -> `docs/changelog/2026-07-16.mdx`: Document
onboarding DNS recovery, explicit notice acceptance, and upgrades with
user-local OpenShell.
- #7005, #7021, #7029, and #7049 -> `docs/changelog/2026-07-16.mdx`:
Document rebuild backup safety, no-dashboard state, managed gateway
discovery, and Hermes shields topology checks.

## Type of Change

- [ ] Code change (feature, bug fix, or refactor)
- [ ] Code change with doc updates
- [x] Doc only (prose changes, no code sample modifications)
- [ ] Doc only (includes code sample changes)

## Quality Gates

- [ ] Tests added or updated for changed behavior
- [x] Existing tests cover changed behavior — justification:
`test/changelog-docs.test.ts` validates the canonical heading,
parser-safe SPDX comment, and detailed entry structure; the docs build
validates published routes.
- [ ] Tests not applicable — justification:
- [x] Docs updated for user-facing behavior changes
- [ ] Docs not applicable — justification:
- [ ] Sensitive paths changed (security, policy, credentials, preflight,
onboarding, inference, runner, sandbox, or messaging)
- [ ] Sensitive-path review completed or maintainer-approved waiver
recorded — reviewer/approval link/justification:
- [ ] Non-success, skipped, or missing CI check accepted by maintainer —
check name, approval link, and follow-up issue:

## Verification

- [x] PR description includes a `Signed-off-by:` line and every commit
appears as `Verified` in GitHub
- [x] Normal `pre-commit`, `commit-msg`, and `pre-push` hooks passed, or
`npm run check:diff` passed when hooks were skipped or unavailable
- [x] Targeted behavior tests pass for the current change set, or tests
are marked not applicable above — `npx vitest run
test/changelog-docs.test.ts` passed 6/6.
- [ ] Applicable broad gate passed — `npm test` for broad
runtime/test-harness changes; `npm run check` for repo-wide
validation/coverage changes — not applicable to this doc-only entry.
- [x] Quality Gates section completed with required justifications or
waivers
- [x] No secrets, API keys, or credentials committed
- [ ] `npm run docs` builds without warnings (doc changes only) — passed
with 0 errors and 2 pre-existing Fern warnings.
- [x] Doc pages follow the [style
guide](https://github.com/NVIDIA/NemoClaw/blob/main/docs/CONTRIBUTING.md)
(doc changes only)
- [x] New doc pages include SPDX header and frontmatter (new pages only)
— native changelog entries use the required parser-safe MDX SPDX comment
instead of frontmatter.

---
Signed-off-by: Charan Jagwani <cjagwani@nvidia.com>


<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->

## Summary by CodeRabbit

* **Documentation**
  * Added release notes for NemoClaw v0.0.85.
* Documented improvements to compatibility, credential handling, setup
validation, recovery workflows, endpoint configuration, gateway
discovery, and runtime validation.
  * Added links to relevant user-guide sections.

<!-- end of auto-generated comment: release notes by coderabbit.ai -->

Signed-off-by: Charan Jagwani <cjagwani@nvidia.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug-fix PR fixes a bug or regression

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[DGX Station][Install] --station-deepseek silently skips express install when no TTY is available instead of erroring

4 participants