fix(install): probe /dev/tty by opening it, not bare existence (#16746) by briandevans · Pull Request #16750 · NousResearch/hermes-agent

briandevans · 2026-04-28T01:13:07Z

Summary

Replace [ -e /dev/tty ] with an open-based probe in run_setup_wizard() so Docker / CI installs skip the wizard cleanly instead of crashing on the next < /dev/tty redirect.
Add a static regression test that asserts the gate does not use the bare existence check and that the open-based probe is in place.

The bug

On a Docker build, /dev/tty exists as a device node (so [ -e /dev/tty ] returns true), but opening it fails with ENXIO: No such device or address. The skip at the top of run_setup_wizard() never triggered, the wizard proceeded, and bash aborted a few lines later when it tried to redirect from /dev/tty:

/tmp/install.sh: line 1347: /dev/tty: No such device or address

The --skip-setup flag works around this, but the bare existence check was supposed to catch this case automatically.

The fix

(: </dev/tty) 2>/dev/null runs the no-op : in a subshell with stdin redirected from /dev/tty. The subshell exits non-zero exactly when opening /dev/tty would fail (ENXIO in Docker, ENODEV in some CI sandboxes), and the existing skip path runs. On a real terminal — including the curl | bash flow the wizard targets — the open succeeds and the wizard runs as before. The subshell isolates the probe so we don't leave an inherited fd open in the script.

scripts/install.sh has two other [ -e /dev/tty ] sites (the apt sudo prompt fallback near line 732 and the gateway-install gate near line 1395). They have the same shape but are outside the reproduction path in #16746 and on a separate code path, so this PR keeps the diff scoped to the wizard gate the reporter actually hit. Happy to follow up with a second PR for those if maintainers prefer.

Test plan

Focused regression test: uv run --with pytest --with pytest-xdist python3 -m pytest tests/test_install_sh_setup_wizard_tty_probe.py -v — both assertions pass on this branch.
Regression guard: same suite run with scripts/install.sh reverted to origin/main fails with the documented assertion message; restoring the fix passes.
Adjacent: tests/hermes_cli/test_managed_installs.py and tests/hermes_cli/test_doctor_command_install.py — only baseline failure is test_windows_skips_check, which reproduces on clean origin/main (it depends on _winapi, macOS lacks the module).
Manual probe behavior:
- bash -c '(: </dev/tty) 2>/dev/null && echo OK || echo FAIL' < /dev/null → FAIL (matches Docker-build skip).
- On a real interactive terminal the same probe returns OK — the wizard still runs as before.

…esearch#16746) In Docker builds the `/dev/tty` device node is present in the mount namespace, so `[ -e /dev/tty ]` returns true — but opening it fails with `ENXIO: No such device or address`. Under the old gate the "no terminal available" skip never triggered, the setup wizard ran, and the build aborted a few lines later when bash tried `< /dev/tty`: /tmp/install.sh: line 1347: /dev/tty: No such device or address Replace the existence check with `(: </dev/tty) 2>/dev/null`, which actually attempts to open /dev/tty in a subshell. The probe succeeds when piped from `curl | bash` on a real terminal (the wizard's intended use case) and fails cleanly in Docker build / CI contexts so the skip kicks in before the redirect can crash. Add a regression test that statically asserts run_setup_wizard does not gate on the bare existence check and that the open-based probe is in place. Fixes NousResearch#16746. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

Copilot

Pull request overview

Improves the installer’s non-interactive detection so the setup wizard is skipped when /dev/tty exists but cannot be opened (e.g., Docker/CI), preventing a later < /dev/tty redirect crash.

Changes:

Replace the setup-wizard gate from a bare existence check on /dev/tty to an open-based probe.
Add a static pytest regression test ensuring the wizard gate uses an open-based probe and not the prior -e /dev/tty check.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.

File	Description
scripts/install.sh	Switches `run_setup_wizard()`’s tty availability check to an open-based probe to avoid Docker/CI crashes.
tests/test_install_sh_setup_wizard_tty_probe.py	Adds a regression test that inspects `install.sh` to ensure the new probe is present and the old check is not.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-04-28T01:15:53Z

+def _extract_run_setup_wizard() -> str:
+    """Return the body of run_setup_wizard() as a single string."""
+    text = INSTALL_SH.read_text()
+    start = text.index("run_setup_wizard()")
+    # The next top-level function follows immediately; use it as the end marker.
+    end = text.index("\nmaybe_start_gateway()", start)
+    return text[start:end]


_extract_run_setup_wizard() relies on str.index(...) with hard-coded markers; if maybe_start_gateway() is renamed/moved (or if the string appears in a comment), this will raise ValueError and produce a confusing stack trace. Consider using a regex anchored to function definitions (e.g., ^run_setup_wizard\(\) ... next ^[a-zA-Z0-9_]+\(\)) and failing with an explicit assertion message (or skipping if install.sh is absent).

Copilot · 2026-04-28T01:15:53Z

+def test_run_setup_wizard_does_not_use_bare_existence_check() -> None:
+    body = _extract_run_setup_wizard()
+    assert "[ -e /dev/tty ]" not in body, (
+        "run_setup_wizard guards on `[ -e /dev/tty ]`, which is true in Docker "
+        "builds where the device node exists but cannot be opened (ENXIO). "
+        "Use an open-based probe such as `(: </dev/tty) 2>/dev/null` so the "
+        "skip kicks in before the wizard tries to read from /dev/tty. See #16746."
+    )


The guard against the old -e /dev/tty check is an exact substring match ("[ -e /dev/tty ]"). This can miss equivalent spellings (different whitespace, test -e /dev/tty, [ -e "/dev/tty" ], etc.), which weakens the regression protection. Consider using a small regex that matches any -e test against /dev/tty independent of spacing/quoting.

Copilot · 2026-04-28T01:15:53Z

+    assert "(: </dev/tty)" in body, (
+        "run_setup_wizard must probe /dev/tty by actually opening it before "
+        "running the wizard. See #16746."
+    )


This assertion pins the implementation to the exact (: </dev/tty) probe string. That makes future refactors to an equivalent open-based probe (e.g., exec 3</dev/tty + close) fail the test even though the behavior is correct. Consider asserting the higher-level invariant instead (e.g., that there is an if !-guard that attempts to open /dev/tty before the later < /dev/tty redirect), rather than a specific spellings.

Suggested change

assert "(: </dev/tty)" in body, (

"run_setup_wizard must probe /dev/tty by actually opening it before "

"running the wizard. See #16746."

)

assert "/dev/tty" in body, (

"run_setup_wizard must reference /dev/tty when deciding whether the "

"wizard can run. See #16746."

)

assert "if !" in body, (

"run_setup_wizard must use a negated guard before running the wizard "

"when probing terminal availability. See #16746."

)

probe_pos = body.index("if !")

tty_open_pos = body.find("</dev/tty", probe_pos)

if tty_open_pos == -1:

tty_open_pos = body.find("< /dev/tty", probe_pos)

assert tty_open_pos != -1, (

"run_setup_wizard must attempt to open /dev/tty in its guard rather "

"than only checking for its existence. See #16746."

)

later_read_pos = body.rfind("< /dev/tty")

assert probe_pos < later_read_pos, (

"run_setup_wizard must probe /dev/tty before the later redirect that "

"reads from it. See #16746."

)

…Research#16750 Address the three Copilot inline findings on the regression test: - Switch _extract_run_setup_wizard() from str.index() with hard-coded markers (which raises ValueError if `maybe_start_gateway()` is renamed or the marker leaks into a comment) to an anchored regex on the function-definition + closing-brace boundaries. - Match `[ -e /dev/tty ]` with surrounding whitespace, optional quoting, and the `test -e /dev/tty` form so the regression guard catches every spelling of the existence-only check, not just the exact substring. - Replace the literal `(: </dev/tty)` substring assertion with a higher-level invariant — the gate must be an `if`/`if !` whose test redirects stdin from /dev/tty — so equivalent open-based probes (`exec 3</dev/tty` + close, brace-grouped variants, etc.) keep the test green while the bare existence check stays caught. Verified guard: both tests still pass on the fix and both fail on `origin/main` with the documented messages. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

briandevans · 2026-04-28T01:18:17Z

@copilot All three findings addressed in commit f6b9b8fed:

Finding 1 (line 23 — fragile str.index markers in _extract_run_setup_wizard()): Replaced with an anchored regex on ^run_setup_wizard\(\)\s*\{ … ^\} so the helper survives a rename or move of maybe_start_gateway(), and stops misfiring if the marker substring leaks into a comment.

Finding 2 (line 33 — exact-substring guard misses equivalent spellings): Replaced "[ -e /dev/tty ]" not in body with a verbose regex covering whitespace variants, optional quoting around /dev/tty, and the test -e /dev/tty form. Caught spellings now include [ -e /dev/tty ], [ -e "/dev/tty" ], [ -e /dev/tty ], and test -e /dev/tty.

Finding 3 (line 41 — implementation-pinned probe assertion): Replaced the literal "(: </dev/tty)" in body check with a higher-level invariant: the gate must be an if/if ! whose test redirects stdin from /dev/tty. Equivalent refactors such as if ! exec 3</dev/tty 2>/dev/null; then or if ! { exec 3</dev/tty; } 2>/dev/null; then now keep the test green.

Regression guard re-verified after the rewrite: both tests pass against this branch and both fail against origin/main with the documented assertion messages.

Address the three Copilot inline findings on the regression test: - Switch _extract_run_setup_wizard() from str.index() with hard-coded markers (which raises ValueError if `maybe_start_gateway()` is renamed or the marker leaks into a comment) to an anchored regex on the function-definition + closing-brace boundaries. - Match `[ -e /dev/tty ]` with surrounding whitespace, optional quoting, and the `test -e /dev/tty` form so the regression guard catches every spelling of the existence-only check, not just the exact substring. - Replace the literal `(: </dev/tty)` substring assertion with a higher-level invariant — the gate must be an `if`/`if !` whose test redirects stdin from /dev/tty — so equivalent open-based probes (`exec 3</dev/tty` + close, brace-grouped variants, etc.) keep the test green while the bare existence check stays caught. Verified guard: both tests still pass on the fix and both fail on `origin/main` with the documented messages. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

The contributor's PR (#16750) scoped the fix to run_setup_wizard() and explicitly punted the two sibling sites. Both have the identical [ -e /dev/tty ] pattern followed by a < /dev/tty redirect and crash in Docker the same way: - scripts/install.sh:732 install_system_packages() -- apt sudo prompt fallback. sudo ... < /dev/tty dies with the same ENXIO. - scripts/install.sh:1395 maybe_start_gateway() -- gateway-install gate, same function path as the wizard reproducer. Fix both with the same (: </dev/tty) 2>/dev/null probe, and parametrize the regression test over all three gated functions so any future regression is caught regardless of which site breaks.

Address the three Copilot inline findings on the regression test: - Switch _extract_run_setup_wizard() from str.index() with hard-coded markers (which raises ValueError if `maybe_start_gateway()` is renamed or the marker leaks into a comment) to an anchored regex on the function-definition + closing-brace boundaries. - Match `[ -e /dev/tty ]` with surrounding whitespace, optional quoting, and the `test -e /dev/tty` form so the regression guard catches every spelling of the existence-only check, not just the exact substring. - Replace the literal `(: </dev/tty)` substring assertion with a higher-level invariant — the gate must be an `if`/`if !` whose test redirects stdin from /dev/tty — so equivalent open-based probes (`exec 3</dev/tty` + close, brace-grouped variants, etc.) keep the test green while the bare existence check stays caught. Verified guard: both tests still pass on the fix and both fail on `origin/main` with the documented messages. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

The contributor's PR (#16750) scoped the fix to run_setup_wizard() and explicitly punted the two sibling sites. Both have the identical [ -e /dev/tty ] pattern followed by a < /dev/tty redirect and crash in Docker the same way: - scripts/install.sh:732 install_system_packages() -- apt sudo prompt fallback. sudo ... < /dev/tty dies with the same ENXIO. - scripts/install.sh:1395 maybe_start_gateway() -- gateway-install gate, same function path as the wizard reproducer. Fix both with the same (: </dev/tty) 2>/dev/null probe, and parametrize the regression test over all three gated functions so any future regression is caught regardless of which site breaks.

teknium1 · 2026-04-28T13:46:05Z

Merged via #17024 with your authorship preserved on the wizard fix and regex-test commits (rebase-merge, not squash). We widened the fix to the two sibling [ -e /dev/tty ] gates you flagged as follow-up candidates (apt sudo fallback at line 732, gateway-install gate at line 1395) and parametrized your regression test across all three functions. Thanks for the clean fix and the thorough writeup on #16746 — it made the widening trivial.

Address the three Copilot inline findings on the regression test: - Switch _extract_run_setup_wizard() from str.index() with hard-coded markers (which raises ValueError if `maybe_start_gateway()` is renamed or the marker leaks into a comment) to an anchored regex on the function-definition + closing-brace boundaries. - Match `[ -e /dev/tty ]` with surrounding whitespace, optional quoting, and the `test -e /dev/tty` form so the regression guard catches every spelling of the existence-only check, not just the exact substring. - Replace the literal `(: </dev/tty)` substring assertion with a higher-level invariant — the gate must be an `if`/`if !` whose test redirects stdin from /dev/tty — so equivalent open-based probes (`exec 3</dev/tty` + close, brace-grouped variants, etc.) keep the test green while the bare existence check stays caught. Verified guard: both tests still pass on the fix and both fail on `origin/main` with the documented messages. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

The contributor's PR (#16750) scoped the fix to run_setup_wizard() and explicitly punted the two sibling sites. Both have the identical [ -e /dev/tty ] pattern followed by a < /dev/tty redirect and crash in Docker the same way: - scripts/install.sh:732 install_system_packages() -- apt sudo prompt fallback. sudo ... < /dev/tty dies with the same ENXIO. - scripts/install.sh:1395 maybe_start_gateway() -- gateway-install gate, same function path as the wizard reproducer. Fix both with the same (: </dev/tty) 2>/dev/null probe, and parametrize the regression test over all three gated functions so any future regression is caught regardless of which site breaks.

…Research#16750 Address the three Copilot inline findings on the regression test: - Switch _extract_run_setup_wizard() from str.index() with hard-coded markers (which raises ValueError if `maybe_start_gateway()` is renamed or the marker leaks into a comment) to an anchored regex on the function-definition + closing-brace boundaries. - Match `[ -e /dev/tty ]` with surrounding whitespace, optional quoting, and the `test -e /dev/tty` form so the regression guard catches every spelling of the existence-only check, not just the exact substring. - Replace the literal `(: </dev/tty)` substring assertion with a higher-level invariant — the gate must be an `if`/`if !` whose test redirects stdin from /dev/tty — so equivalent open-based probes (`exec 3</dev/tty` + close, brace-grouped variants, etc.) keep the test green while the bare existence check stays caught. Verified guard: both tests still pass on the fix and both fail on `origin/main` with the documented messages. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

…h#16746) The contributor's PR (NousResearch#16750) scoped the fix to run_setup_wizard() and explicitly punted the two sibling sites. Both have the identical [ -e /dev/tty ] pattern followed by a < /dev/tty redirect and crash in Docker the same way: - scripts/install.sh:732 install_system_packages() -- apt sudo prompt fallback. sudo ... < /dev/tty dies with the same ENXIO. - scripts/install.sh:1395 maybe_start_gateway() -- gateway-install gate, same function path as the wizard reproducer. Fix both with the same (: </dev/tty) 2>/dev/null probe, and parametrize the regression test over all three gated functions so any future regression is caught regardless of which site breaks.