-
Notifications
You must be signed in to change notification settings - Fork 48.1k
fix(tui): auto-recover session on unexpected gateway death (+ persist lifecycle breadcrumbs) #35893
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
Merged
Changes from all commits
Commits
Show all changes
9 commits
Select commit
Hold shift + click to select a range
274643b
fix(tui): persist gateway lifecycle breadcrumbs to crash log
OutThisLife 2d18d22
feat(tui): auto-recover the session when the gateway dies unexpectedly
OutThisLife e79ed89
fix(tui): sanitize newlines + soften SIGTERM-cause claim in parentLog
OutThisLife 5381bcd
fix(tui): clear sid during recovery + extract/test the recovery budget
OutThisLife c249654
fix(tui): cap parent-log breadcrumb length (PR review)
OutThisLife 7f2b04f
fix(tui): keep "recovering session…" status visible during resume (PR…
OutThisLife 80508ea
fix(tui): keep recovery budget alive across a startup crash-loop (PR …
OutThisLife e88a36f
chore(tui): drop non-null assertion + clarify breadcrumb cap comment …
OutThisLife db3e67e
chore(tui): soften "absence ⇒ external signal" + "any in-flight reply…
OutThisLife File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,47 @@ | ||
| import { describe, expect, it } from 'vitest' | ||
|
|
||
| import { GATEWAY_RECOVERY_LIMIT, GATEWAY_RECOVERY_WINDOW_MS, planGatewayRecovery } from '../app/gatewayRecovery.js' | ||
|
|
||
| describe('planGatewayRecovery', () => { | ||
| it('recovers the live session and records the attempt', () => { | ||
| const plan = planGatewayRecovery('sess-1', null, [], 1000) | ||
|
|
||
| expect(plan).toEqual({ attempts: [1000], recover: true, sid: 'sess-1' }) | ||
| }) | ||
|
|
||
| it('does not recover when there is no session to resume', () => { | ||
| expect(planGatewayRecovery(null, null, [], 1000)).toEqual({ attempts: [], recover: false, sid: null }) | ||
| }) | ||
|
|
||
| it('keeps retrying the recovery target through a startup crash-loop, bounded by the budget', () => { | ||
| // First exit: live sid present. | ||
| let attempts: number[] = [] | ||
| let plan = planGatewayRecovery('sess-1', null, attempts, 0) | ||
|
|
||
| expect(plan.recover).toBe(true) | ||
| expect(plan.sid).toBe('sess-1') | ||
| attempts = plan.attempts | ||
|
|
||
| // Respawn crash-loops before gateway.ready: live sid is now null, but the | ||
| // recovery target carries it forward so we keep trying up to the budget. | ||
| for (let i = 1; i < GATEWAY_RECOVERY_LIMIT; i++) { | ||
| plan = planGatewayRecovery(null, 'sess-1', attempts, i) | ||
| expect(plan.recover).toBe(true) | ||
| expect(plan.sid).toBe('sess-1') | ||
| attempts = plan.attempts | ||
| } | ||
|
|
||
| // Budget exhausted: fall back to the inert state instead of spawn-storming. | ||
| plan = planGatewayRecovery(null, 'sess-1', attempts, GATEWAY_RECOVERY_LIMIT) | ||
| expect(plan.recover).toBe(false) | ||
| expect(plan.sid).toBe('sess-1') | ||
| }) | ||
|
|
||
| it('prunes attempts older than the window so recovery re-arms', () => { | ||
| const old = Array.from({ length: GATEWAY_RECOVERY_LIMIT }, (_, i) => i) | ||
| const plan = planGatewayRecovery('sess-1', null, old, GATEWAY_RECOVERY_WINDOW_MS + 100) | ||
|
|
||
| expect(plan.attempts).toEqual([GATEWAY_RECOVERY_WINDOW_MS + 100]) | ||
| expect(plan.recover).toBe(true) | ||
| }) | ||
| }) |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,75 @@ | ||
| import { mkdtempSync, readFileSync, rmSync } from 'node:fs' | ||
| import { tmpdir } from 'node:os' | ||
| import { join } from 'node:path' | ||
|
|
||
| import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' | ||
|
|
||
| // parentLog gates itself off under VITEST so unit tests can't pollute a real | ||
| // ~/.hermes. To exercise the real persistence path we clear that gate, point | ||
| // HERMES_HOME at a temp dir, and re-import the module fresh (path + enabled | ||
| // flag are captured at module load). | ||
| const loadFresh = async (home: string) => { | ||
| vi.resetModules() | ||
| vi.stubEnv('VITEST', '') | ||
| vi.stubEnv('HERMES_HOME', home) | ||
|
|
||
| return import('../lib/parentLog.js') | ||
| } | ||
|
|
||
| describe('recordParentLifecycle', () => { | ||
| let home: string | ||
|
|
||
| beforeEach(() => { | ||
| home = mkdtempSync(join(tmpdir(), 'hermes-parentlog-')) | ||
| }) | ||
|
|
||
| afterEach(() => { | ||
| vi.unstubAllEnvs() | ||
| rmSync(home, { force: true, recursive: true }) | ||
| }) | ||
|
|
||
| it('appends a timestamped breadcrumb to logs/tui_gateway_crash.log', async () => { | ||
| const { recordParentLifecycle } = await loadFresh(home) | ||
|
|
||
| recordParentLifecycle('graceful-exit received signal=SIGHUP → killing gateway') | ||
|
|
||
| const contents = readFileSync(join(home, 'logs', 'tui_gateway_crash.log'), 'utf8') | ||
|
|
||
| expect(contents).toContain('[tui-parent]') | ||
| expect(contents).toContain('graceful-exit received signal=SIGHUP → killing gateway') | ||
| expect(contents).toMatch(/\d{4}-\d{2}-\d{2}T/) | ||
| }) | ||
|
|
||
| it('collapses embedded newlines so a value stays one breadcrumb', async () => { | ||
| const { recordParentLifecycle } = await loadFresh(home) | ||
|
|
||
| recordParentLifecycle('uncaughtException: boom\n at foo()\r\n at bar()') | ||
|
|
||
| const lines = readFileSync(join(home, 'logs', 'tui_gateway_crash.log'), 'utf8').trimEnd().split('\n') | ||
|
|
||
| expect(lines).toHaveLength(1) | ||
| expect(lines[0]).toContain('boom ↵ at foo() ↵ at bar()') | ||
| }) | ||
|
|
||
| it('caps an oversized breadcrumb so it cannot bloat the shared crash log', async () => { | ||
| const { recordParentLifecycle } = await loadFresh(home) | ||
|
|
||
| recordParentLifecycle('x'.repeat(10_000)) | ||
|
|
||
| const line = readFileSync(join(home, 'logs', 'tui_gateway_crash.log'), 'utf8') | ||
|
|
||
| expect(line).toContain('[truncated 10000 chars]') | ||
| expect(line.length).toBeLessThan(4_500) | ||
| }) | ||
|
|
||
| it('is a no-op under VITEST so tests stay hermetic', async () => { | ||
| vi.resetModules() | ||
| vi.stubEnv('VITEST', 'true') | ||
| vi.stubEnv('HERMES_HOME', home) | ||
|
|
||
| const { recordParentLifecycle } = await import('../lib/parentLog.js') | ||
|
|
||
| expect(() => recordParentLifecycle('should not be written')).not.toThrow() | ||
| expect(() => readFileSync(join(home, 'logs', 'tui_gateway_crash.log'), 'utf8')).toThrow() | ||
| }) | ||
| }) |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,35 @@ | ||
| // Crash-recovery budget for the gateway exit handler. A gateway that | ||
| // crash-loops on startup must not let the TUI spawn-storm, so respawn+resume | ||
| // attempts are capped to GATEWAY_RECOVERY_LIMIT within a sliding | ||
| // GATEWAY_RECOVERY_WINDOW_MS; past the budget the app falls back to the inert | ||
| // "gateway exited" state. Kept pure (no refs/UI) so the bound — including the | ||
| // crash-loop case — is unit-testable. | ||
| export const GATEWAY_RECOVERY_LIMIT = 3 | ||
| export const GATEWAY_RECOVERY_WINDOW_MS = 60_000 | ||
|
|
||
| export interface RecoveryPlan { | ||
| // Attempt timestamps to persist (the pruned window, plus `now` iff recovering). | ||
| attempts: number[] | ||
| recover: boolean | ||
| // Session to resume — the live sid, or the not-yet-consumed recovery target | ||
| // when the live sid was already cleared by a prior exit. | ||
| sid: null | string | ||
| } | ||
|
|
||
| // Decide whether to respawn+resume after a gateway death. `liveSid` is the | ||
| // current session (nulled on the first exit); `recoverSid` is a pending | ||
| // recovery target carried across a respawn that died before gateway.ready — | ||
| // so a startup crash-loop keeps retrying the same session up to the budget | ||
| // instead of stranding it after one attempt. | ||
| export function planGatewayRecovery( | ||
| liveSid: null | string, | ||
| recoverSid: null | string, | ||
| attempts: number[], | ||
| now: number | ||
| ): RecoveryPlan { | ||
| const sid = liveSid ?? recoverSid | ||
| const recent = attempts.filter(t => now - t < GATEWAY_RECOVERY_WINDOW_MS) | ||
| const recover = Boolean(sid) && recent.length < GATEWAY_RECOVERY_LIMIT | ||
|
|
||
| return { attempts: recover ? [...recent, now] : recent, recover, sid } | ||
| } |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.