Skip to content

test(hermes): wait for the hook installer instead of racing a 1 s deadline - #15027

Merged
teamleaderleo merged 2 commits into
mainfrom
fix/hermes-hooks-flake
Sep 27, 2026
Merged

teamleaderleo merged 2 commits into
mainfrom
fix/hermes-hooks-flake

Conversation

@teamleaderleo

@teamleaderleo teamleaderleo commented Sep 27, 2026 •

Copy link
Copy Markdown
Collaborator

Summary

tests/test_hermes_wrapper_hooks.py fails on main in shard 4's CLI no-socket lane with bare: unexpected installer calls: [] (main run 36326938624; #14958 run 36329092303, attempts 1 and 2). That blocks ci-status on unrelated PRs.

Neither the wrapper nor the test changed near the first failure; both were last touched on 2026-09-24. The first failing main run (23d22d78439) is the first one to include #14990, which runs that lane eight tests at a time. The last passing run (3761671f520) didn't include it.

The harness gave every launch a 1 s installer deadline (CMUX_HERMES_AGENT_HOOK_INSTALL_TIMEOUT_SECONDS=1). When the deadline passes, the wrapper kills the installer and launches Hermes anyway, which is the intended product behavior. On a busy runner, the first launch's cold start of perl and the bash installer can take over a second. The fake installer was killed before it recorded anything, so the check saw no call.

Now only the deadline tests set an installer deadline: stalled installer keeps 1 s and non-positive timeout keeps 0. Every other launch gets as long as the wrapper hang guard (5 s, unchanged). Those checks now wait for the installer to finish and the wrapper to exit instead of racing the installer's start. A real hang still fails, and the hang guard now reports it as wrapper execution deadline exceeded before the installer deadline can hide it. The wrapper itself is unchanged.

Testing

The installer checks are portable, so these ran on Linux. The TUI bridge checks fail on Linux on both main and this branch because the wrapper needs /usr/bin/plutil; they're unrelated to this change.

  • Regression (f15cbd20b95): a new check delays the fake installer's start by 1.5 s. python3 tests/test_hermes_wrapper_hooks.py fails with the same messages as CI: slow installer start: unexpected installer calls: [] and installer lost surface/workspace attribution: {}.
  • Fix (2e18ce8008d): the same command passes that check. The only remaining failures are the two Linux-only TUI bridge lines that main also shows.
  • Under load: I ran the session-entrypoint, installer-failure and profile checks with 64 CPU spinners on 16 cores and the test at nice 12. On main, 2 of 2 iterations failed (installer lost surface attribution: {} and explicit-profile-long: installer targeted the wrong profile: {}, meaning the installer was killed mid-run). With the fix, 3 of 3 passed.
  • python3 scripts/verify-local.py passed 12 of 13 selected checks. It doesn't cover native builds.

The macOS lane on this PR is the first run of the full file on a Mac with this change.

Changelog

none

🤖 Generated with Claude Code


View with [code]smith Autofix with [code]smith
Need help on this PR? Tag @codesmith-bot with what you need. Autofix is disabled.


Summary by cubic

Fixes a flaky Hermes hooks test that failed on busy CI runners when the installer took over a second to start.

  • The launch-path checks no longer impose the 1 s installer deadline; only deadline-focused tests set one.
  • Added a check that a slow-starting installer's call is still observed.
  • A real installer hang still fails, now reported as wrapper execution deadline exceeded.

Written for commit 2e18ce8. Summary will update on new commits.

Review in cubic

teamleaderleo and others added 2 commits September 27, 2026 09:09
…owly

The launch-path checks in test_hermes_wrapper_hooks.py run the wrapper with a
1 s installer deadline, so an installer that takes over a second to start is
killed before it records its call. On main since the CLI no-socket lane went
parallel (#14990), the first `bare` launch fails that way on shard 4:
"bare: unexpected installer calls: []".

This adds a fixture delay before the fake installer records anything and a
check that a 1.5 s start is still observed. It fails on the current fixture
with the CI message.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
run_wrapper gave every launch a 1 s installer deadline, including the checks
that assert the installer's call and environment. The wrapper kills the
installer at that deadline and launches Hermes anyway, so on a busy runner
(the CLI no-socket lane runs eight tests at once since #14990) a cold
installer start lost the call and the check failed.

Only the deadline tests now pass a deadline. Every other run gives the
installer as long as the wrapper hang guard, so those checks wait for the
installer to finish and the wrapper to exit. The stalled-installer test keeps
its 1 s deadline and still waits on its own start and launch signals.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
@github-actions

Copy link
Copy Markdown
Contributor

All contributors have signed the CLA ✍️ ✅
Posted by the CLA Assistant Lite bot.

@coderabbitai

coderabbitai Bot commented Sep 27, 2026

Copy link
Copy Markdown

Review in Change Stack →

Navigate logical layers of code changes, visualize relationships, and explore their blast radius.

Note

Currently processing new changes in this PR. This may take a few minutes, please wait...

⚙️ Run configuration

Configuration used: Repository: manaflow-ai/cmux/.coderabbit.yaml

Review profile: ASSERTIVE

Plan: Advanced

Run ID: 5238749d-a231-49c7-9187-5348f07f45de

📥 Commits

Reviewing files that changed from the base of the PR and between 810ffba and 2e18ce8.

📒 Files selected for processing (1)
  • tests/test_hermes_wrapper_hooks.py
 __________________________________________________
< Brb...ordering more GPUs. CPUs are so last year. >
 --------------------------------------------------
  \
   \   \
        \ /\
        ( )
      .( o ).
✨ Finishing Touches
📝 Generate docstrings
  • Commit to this branch
  • Create a new PR
🧪 Generate unit tests (beta)
  • Commit to this branch
  • Create a new PR

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

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

@teamleaderleo

Copy link
Copy Markdown
Collaborator Author

Toolbox g1 🔔 reviewed the Hermes test-only fix. It keeps the one-second timeout only for deadline tests, gives normal launch assertions the existing five-second hang guard, and adds a regression for slow installer startup. Required checks are green; landing it to unblock ci-status failures.

@teamleaderleo
teamleaderleo merged commit c842f7d into main Sep 27, 2026
42 of 44 checks passed
@teamleaderleo
teamleaderleo deleted the fix/hermes-hooks-flake branch September 27, 2026 16:16
@github-actions

Copy link
Copy Markdown
Contributor

Merge receipt for 2e18ce8008: every check was green at merge (10 verified; 11 skipped by policy). Full suite runs on main after merge.

rustybret pushed a commit to rustybret/bmux that referenced this pull request Sep 27, 2026
8efe28d Add terminal.confirmUnsafePaste to confirm unsafe pastes in a window sheet (manaflow-ai#14951)
368ec9f fix(ci): restore app-host artifact rerun setup (manaflow-ai#15029)
e67ea0f perf: stop launching the cmux CLI for every queued Claude hook (manaflow-ai#14931)
badf9f6 test: give the tmux split mapping test its own portal authority (manaflow-ai#15028)
41a0c37 current-work: preserve remote machine kinds (manaflow-ai#14914)
c842f7d test(hermes): wait for the hook installer instead of racing a 1 s deadline (manaflow-ai#15027)
810ffba fix(ci): resolve binary modules in detached test reruns (manaflow-ai#15026)
d363290 test: await fork probe fixture start signals (manaflow-ai#15025)
8c98e64 Add cmux import for settings from other terminals (manaflow-ai#15004)
30aa6c1 Keep SSH workspace titles when cmux-tui creates the remote workspace (manaflow-ai#14976)
b33c467 Restore workspace group color and icon key handling from manaflow-ai#13877 (manaflow-ai#15000)

# Conflicts:
#	.github/workflows/app-host-test-rerun.yml
#	.github/workflows/ci-macos.yml
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.

1 participant