fix(hermes): reset health streak after recovery (Fixes #7671) - #7894
fix(hermes): reset health streak after recovery (Fixes #7671)#7894deepujain wants to merge 7 commits into
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughThe Hermes gateway supervisor now resets its consecutive health-failure counter after recovering a relaunched gateway. A regression test verifies that the recovered gateway receives a fresh failure budget and remains active. ChangesHermes gateway recovery
Estimated code review effort: 2 (Simple) | ~10 minutes Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
PR Review Advisor — InformationalAdvisor assessment: Informational / low confidence Model lanes
Second-opinion terminology and E2E selections are advisory. They do not change the primary assessment or E2E / PR Gate. 1 semantic terminology decisionTerminology decisions are advisory. They affect the assessment only when a separate finding identifies concrete semantic impact.
E2E guidanceAdvisory only. E2E / PR Gate selects and runs jobs independently. Recommended E2E: This automated review informs maintainers. Warnings and suggestions do not require a response. A maintainer decides whether to merge. |
757a8f4 to
61f641e
Compare
|
Rebased onto current main. Focused tests and |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
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/hermes-gateway-supervisor-recovery.test.ts`:
- Around line 721-722: Strengthen the assertions around the recovered gateway’s
health-validation failures so they verify the exact ordered failure sequence,
including "(1/4)" occurring again after recovery. Update the test using the
existing result.stderr assertions, while preserving the check that no "(5/4)"
value is emitted.
🪄 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: 0abb2342-0e7f-46a3-a9cf-542e88d88a92
📒 Files selected for processing (2)
agents/hermes/start.shtest/hermes-gateway-supervisor-recovery.test.ts
61f641e to
55774d0
Compare
wscurran
added
area: sandbox
|
✨ Thanks for the PR. This fixes the Hermes health-streak reset after gateway recovery so replacement gateways receive the full four-probe failure budget. Maintainers will review the streak reset logic and the shell-harness regression test. Related open issues: Related open issues: |
apurvvkumaria
left a comment
apurvvkumaria
left a comment
There was a problem hiding this comment.
Approve — reviewed exact head 55774d0. Resetting unhealthy_streak only after successful managed recovery gives the replacement gateway the intended fresh four-probe budget without weakening exact-PID stopping or relaunch quarantine. The runtime shell harness proves the ordered failure sequence, recovery, replacement transient failure, healthy follow-up, and absence of a replacement stop. No blocking defect found. The recorded E2E gate failure was a coordination timeout, not a product/test failure attributable to this diff.
Signed-off-by: Deepak Jain <deepujain@gmail.com>
Signed-off-by: Deepak Jain <deepujain@gmail.com>
55774d0 to
1ac7dd4
Compare
|
Rebased on current main. The 45 recovery tests and full PR validation pass, and both commits are verified. @coderabbitai review |
|
✅ Action performedReview finished.
|
|
Synced with latest main. All 45 recovery tests and PR validation pass; CI is rerunning. |
|
Synced with current main while preserving the approved history. CodeRabbit is clean; E2E and advisor checks are rerunning. |
|
Synced with today's main. CodeRabbit and fast gates pass; advisors and E2E are still running. |
|
Synced with current main. All commits remain verified, approval and CodeRabbit are intact, and fresh advisor and E2E checks are running. |
4 participants
Summary
Hermes keeps its consecutive health-failure count after a successful gateway recovery. As a result, the replacement gateway can be stopped after its first transient failed probe. Reset the count after recovery so every replacement receives the intended four-probe failure budget.
Related Issue
Fixes #7671
Changes
recover_hermes_gateway_current_usersucceeds.Current
mainalready covers the other two failure modes from #7671:recover_hermes_gateway_current_userprepares and launches the replacementunder the existing sandbox service identity, and
record_hermes_managed_gateway_exitquarantines relaunch after five exits in60 seconds. This PR fixes the remaining stale probe state that could keep
feeding that recovery loop.
Type of Change
Quality Gates
Documentation Writer Review
no-docs-neededDGX Station Hardware Evidence
Verification
Signed-off-by:line and every commit appears asVerifiedin GitHubpre-commit,commit-msg, andpre-pushhooks passed, ornpm run validate:prpassed after refreshingorigin/mainwhen hooks were skipped or unavailablenpx vitest run --project integration test/hermes-gateway-supervisor-recovery.test.ts(45/45); related Hermes recovery neighborhood (92/92)npm testfor broad runtime/test-harness changes;npm run checkfor repo-wide validation/coverage changes — command/result:npm run docsbuilds without warnings (doc changes only)Signed-off-by: Deepak Jain deepujain@gmail.com
Summary by CodeRabbit
Bug Fixes
Tests