Skip to content

fix(cli): keep passive update checks noninteractive - #101421

Closed
RobbertC5 wants to merge 1 commit into
NousResearch:mainfrom
RobbertC5:fix/passive-update-noninteractive
Closed

RobbertC5 wants to merge 1 commit into
NousResearch:mainfrom
RobbertC5:fix/passive-update-noninteractive

Conversation

@RobbertC5

@RobbertC5 RobbertC5 commented Sep 2, 2026 •

Copy link
Copy Markdown
Contributor

What does this PR do?

Passive update checks can invoke Git against a public HTTPS remote while inheriting the terminal. If a proxy or transient GitHub response requests authentication, Git asks for a GitHub username and blocks Hermes startup even though the repository is public.

This change applies the existing non-interactive Git contract to both passive network probes in hermes_cli/banner.py:

  • detach subprocess stdin with subprocess.DEVNULL
  • set GIT_TERMINAL_PROMPT=0 and GCM_INTERACTIVE=Never through noninteractive_git_env()
  • retain the existing fail-closed behavior when the upstream probe or local fetch is unavailable

This is the passive-banner counterpart to merged PR #73709. PR #73751 covers explicit hermes update operations instead. It overlaps the passive-probe portion of #64478, but deliberately excludes that PR's unrelated OAuth and update-count changes, which have unresolved review findings.

Related Issue

No issue exists; reproduced directly on current main.

Type of Change

  • 🐛 Bug fix (non-breaking change that fixes an issue)
  • ✨ New feature (non-breaking change that adds functionality)
  • 🔒 Security fix
  • 📝 Documentation update
  • ✅ Tests (adding or improving test coverage)
  • ♻️ Refactor (no behavior change)
  • 🎯 New skill (bundled or hub)

Changes Made

  • hermes_cli/banner.py: make the hard-coded upstream ls-remote and local-checkout fetch non-interactive and unable to read terminal stdin.
  • tests/hermes_cli/test_update_check.py: assert both passive call sites disable terminal and Git Credential Manager prompts.
  • tests/hermes_cli/test_update_check.py: make the touched cache fixture's JSON encoding explicit for Windows compatibility.

How to Test

  1. Run scripts/run_tests.sh tests/hermes_cli/test_update_check.py.
  2. Run uvx ruff==0.15.10 check hermes_cli/banner.py tests/hermes_cli/test_update_check.py.
  3. Run python3 scripts/check-windows-footguns.py with the two changed files staged.
  4. Redirect the hard-coded upstream URL to a local HTTP endpoint that returns 401; _upstream_main_sha() should return None immediately without prompting.

Verified on Linux Mint 22.3 with Python 3.11.15:

  • focused suite: 7 passed
  • Ruff: passed
  • Windows-footgun scan: passed
  • real Git/HTTP 401 reproduction: returned None in 0.038 seconds without prompting
  • independent review: passed with no blocking findings

Full canonical-suite validation was completed on the exact pushed head. The first run reported 81 failures across 20 files; a fresh canonical retry left 80 failures across 19 files. Running that exact 19-file set against an untouched detached origin/main worktree at afc3d9d34 reproduced the same 80 failures in the same files. The one non-repeating failure (tests/gateway/test_turn_lease.py) passed on retry. This establishes the remaining failures as base/environment failures rather than regressions from this two-file PR. The focused update-check suite remains 7/7 passing.

Checklist

Code

  • I've read the Contributing Guide
  • My commit messages follow Conventional Commits (fix(scope):, feat(scope):, etc.)
  • I searched for existing PRs to make sure this isn't a duplicate
  • My PR contains only changes related to this fix/feature (no unrelated commits)
  • I've run pytest tests/ -q and all tests pass
  • I've added tests for my changes (required for bug fixes, strongly encouraged for features)
  • I've tested on my platform: Linux Mint 22.3, Python 3.11.15

Documentation & Housekeeping

  • I've updated relevant documentation (README, docs/, docstrings) — N/A; behavior-only bug fix
  • I've updated cli-config.yaml.example if I added/changed config keys — N/A; no config changes
  • I've updated CONTRIBUTING.md or AGENTS.md if I changed architecture or workflows — N/A
  • I've considered cross-platform impact (Windows, macOS) per the compatibility guide
  • I've updated tool descriptions/schemas if I changed tool behavior — N/A

Screenshots / Logs

Not applicable; this removes an invisible blocking credential prompt from a passive subprocess path.

@alt-glitch alt-glitch added type/bug Something isn't working P2 Medium — degraded but workaround exists comp/cli CLI entry point, hermes_cli/, setup wizard area/auth Authentication, OAuth, credential pools area/install-update Installer, updater, packaging, wheels, doctor sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades labels Sep 2, 2026
teknium1 added a commit that referenced this pull request Sep 2, 2026
GitHub answers anonymous fetches with HTTP 401 during outages (and for
renamed/private repos). git then prompts `Username for 'https://github.com':`
on the inherited terminal and `hermes update` sits there — users read it as
Hermes demanding a GitHub login.

Every network git call in the updater (fetch/pull/push, apply + --check +
fork sync) now runs with GIT_TERMINAL_PROMPT=0 / stdin=DEVNULL, so the 401
fails fast into the fetch-failure classifier, which now reports it as a
GitHub-side rejection (likely outage) rather than blaming the user's
credentials. Credential helpers/askpass are left configured so private-fork
origins still authenticate.

Live repro: PTY-attached update --check against a 401 origin hung 15s+ on
the prompt before; exits rc=1 in 0.2s with the diagnosis after.

Same class as #73751 (@Frowtek, pre-main.py decomposition); passive banner
half salvaged from #101421 (@RobbertC5).
@teknium1

teknium1 commented Sep 2, 2026

Copy link
Copy Markdown
Collaborator

Merged via #101557 (30b83ab) — your commit was cherry-picked as-is with authorship preserved, alongside the same fix for the hermes update apply/check/fork-sync paths. Thanks @RobbertC5!

@teknium1 teknium1 closed this Sep 2, 2026
melon-xf added a commit to melon-xf/hermes-agent that referenced this pull request Sep 3, 2026
GitHub answers anonymous fetches with HTTP 401 during outages (and for
renamed/private repos). git then prompts `Username for 'https://github.com':`
on the inherited terminal and `hermes update` sits there — users read it as
Hermes demanding a GitHub login.

Every network git call in the updater (fetch/pull/push, apply + --check +
fork sync) now runs with GIT_TERMINAL_PROMPT=0 / stdin=DEVNULL, so the 401
fails fast into the fetch-failure classifier, which now reports it as a
GitHub-side rejection (likely outage) rather than blaming the user's
credentials. Credential helpers/askpass are left configured so private-fork
origins still authenticate.

Live repro: PTY-attached update --check against a 401 origin hung 15s+ on
the prompt before; exits rc=1 in 0.2s with the diagnosis after.

Same class as NousResearch#73751 (@Frowtek, pre-main.py decomposition); passive banner
half salvaged from NousResearch#101421 (@RobbertC5).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/auth Authentication, OAuth, credential pools area/install-update Installer, updater, packaging, wheels, doctor comp/cli CLI entry point, hermes_cli/, setup wizard P2 Medium — degraded but workaround exists sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants