Repository navigation
fix: report a ghostty-web page crash as REPLAY_ERROR on a live session - #363
Conversation
|
@codex review |
|
@codex security review |
Codex Review Summary: ✅ CompletedThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
|
Codex Review: Didn't find any major issues. Bravo. Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
🛡️ Codex Security ReviewSecurity review completed. No security issues were found in this pull request. Reviewed commit: Only the user who started this review can view the report in Codex. ℹ️ About Codex security reviews in GitHubThis is an experimental Codex feature. Security reviews are triggered when:
Once complete, Codex will leave suggestions, or a comment if no findings are found. |
|
@codex review |
|
@codex security review |
|
Codex Review: Didn't find any major issues. More of your lovely PRs please. Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
🛡️ Codex Security ReviewSecurity review completed. No security issues were found in this pull request. Reviewed commit: Only the user who started this review can view the report in Codex. ℹ️ About Codex security reviews in GitHubThis is an experimental Codex feature. Security reviews are triggered when:
Once complete, Codex will leave suggestions, or a comment if no findings are found. |
|
@codex review |
|
@codex security review |
|
Codex Review: Didn't find any major issues. Nice work! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
🛡️ Codex Security ReviewSecurity review completed. No security issues were found in this pull request. Reviewed commit: Only the user who started this review can view the report in Codex. ℹ️ About Codex security reviews in GitHubThis is an experimental Codex feature. Security reviews are triggered when:
Once complete, Codex will leave suggestions, or a comment if no findings are found. |
#363 wraps replayWithTiming in an outer method that describes page crashes. The outer method now passes onProgress on to the inner one.
Why
ghostty-web 0.4.0 has a bug. A short program output crashes the Chromium page of the
ghostty-webrenderer. The pinned version is 0.4.0 and the newest prerelease (0.4.0-next.21) still has the bug. No release has a fix.The shortest input has 12 bytes. They are
a, U+0301 (a combining acute),ESC c(full reset),ESC [ ? 4 7 h(enter the alternate screen) ando. Inprintfform:printf 'a\xcc\x81\033c\033[?47ho'.?47hor?1047henters it.?1049his not affected.page.evaluate: Target crashed.libghostty-vtrenderer shows a blank alternate screen and does not fault on the same bytes.I found the input by delta-reduction of seeded random logs. The same input crashes in plain Node with only the WASM file, and 300 of 300 random logs agree between Node and Chromium. A random log needs all three parts (full reset,
?47hor?1047h, grapheme cluster). Reverse index (ESC M) is not needed.Upstream issue: coder/ghostty-web#215. When a ghostty-web release closes it and agent-tty upgrades to it, remove the error class and the tests that carry a
KNOWN-BUGheader, or invert them to expect a screenshot.This PR does not fix the crash. It changes what
agent-ttytells the user when it happens.What
This PR has seven commits:
screenshot --renderer libghostty-vtthe sameREPLAY_ERROR(see below).crashedwithoutTarget crashed.GhosttyWebCrashedErrorinsrc/renderer/ghosttyWeb/crashError.ts. Its message names the crash and the known cause. It ends with the page error in parentheses. Itscauseis the Playwright error.GhosttyWebBackend.replayToandreplayWithTimingthrow it when a page call fails withTarget crashed. Every other error passes through unchanged. The two bodies are nowreplayToInnerandreplayWithTimingInner, and the public methods wrap them.withOfflineReplayRenderer(exited sessions) andrecord exportadd the message to their fixed text. Before,REPLAY_ERRORandEXPORT_ERRORhid the cause inerror.cause, which no CLI output prints.HostRendererManager.withBackend(src/host/renderer.ts) turns aGhosttyWebCrashedErrorinto aCliErrorwith codeREPLAY_ERRORand the same message. A livescreenshot,snapshot --renderer ghostty-webor renderwaitnow fails like the same command on an exited session: exit 10,retryable: false. Before, the host sent a plainError, which becameRPC_ERROR(exit 9) withretryable: true. The crash is deterministic, so a retry crashes again. Every other error passes through.withOfflineReplayRenderer(src/replay/offlineReplay.ts) also maps aGhosttyWebCrashedErrorthat its callback throws. A libghostty-vt screenshot replays the log a second time in a ghostty-web page that it boots inside the callback, so on an exited session that crash was outside the replaytry/catch. Before, it exited 1 withINTERNAL_ERROR. Now it gives the sameREPLAY_ERRORas the replay (Failed to boot or replay renderer for session <id>.and the explanation). Every other callback failure keeps its code.REPLAY_ERRORalready exists inERROR_CODES, its exit code (10) is inEXIT_CODE_BY_ERROR_CODE, andRpcErrorSchema.codeis a free string. Theretryableflag is not on the wire: the CLI derives it from the code.record export --format webmkeepsEXPORT_ERROR(exit 7).10row of the exit-code table (it says that a crash in ascreenshot,snapshotorwaitreplay exits 10, and that a WebM export exits 7) and a pointer indocs/USAGE.md, a newA Command Says ghostty-web crashedsection indocs/TROUBLESHOOTING.md, and the READMEs ofsrc/host,src/renderer/ghosttyWeb,src/replayandsrc/cli/commands.What the user sees for the 12-byte log (real CLI,
AGENT_TTY_HOMEunder/tmp):screenshot,snapshot --renderer ghostty-webRPC_ERROR,retryable: true, messagepage.evaluate: Target crashedREPLAY_ERROR,retryable: false. The message startsghostty-web crashed while it replayed this session's output.and ends(page.evaluate: Target crashed)wait --text o --renderer ghostty-webREPLAY_ERRORafter 10 failed polls (median 10.1 s, see below)screenshot --renderer libghostty-vtRPC_ERROR,retryable: true, messagepage.evaluate: Target crashedREPLAY_ERROR,retryable: falsescreenshot --renderer libghostty-vtINTERNAL_ERROR, messagepage.evaluate: Target crashedREPLAY_ERROR,retryable: false, messageFailed to boot or replay renderer for session <id>. ghostty-web crashed …screenshot,snapshot --renderer ghostty-web,wait --renderer ghostty-webREPLAY_ERROR,Failed to boot or replay renderer for session <id>.record export --format webmEXPORT_ERROR,Failed to export session "<id>" as webm.snapshot,wait --text,inspect,record export --format svgThe host and the session stay up after a crash. Later default commands on the same session work.
Costs and limits
RPC_ERRORfor this failure sees exit 10 andREPLAY_ERRORnow. I did not test an old CLI against a new host. The code string is one that old CLIs know, so I expect exit 10 there too.try/catcharound each of two replay methods, and one.catchinwithBackend.destroy. This PR does not change that.ghostty-web 0.4.0. UpdatecrashError.tswhen ghostty-web changes.snapshot()(the replay succeeds,snapshot()crashes) staysRPC_ERROR(exit 9,retryable: true), and the retry gives the same error. A retry getsREPLAY_ERRORonly when the replay itself crashes. No known input crashes insnapshot().wait --renderer ghostty-webdepends on host load. Each of the 10 failed polls boots a new Chromium. I measured it 5 times onc4f8d7b7, host load 38 to 61: 5.9, 11.0, 11.3, 8.9 and 10.1 s (median 10.1 s).Validation
Run on commit
c4f8d7b7, from agit archivecopy of it, Node 26.8.1, Chromium headless shell 1248.oxfmt . --check,oxlint src test vitest.config.ts(0 warnings, 0 errors),tsc -p tsconfig.json --noEmitandtsc -p tsconfig.build.json --noEmitpass.check.sh --drift-onlysays the covered code matches the sync record.vitest run test/unit: 149 files, 2318 tests, all pass.--maxWorkers=1 --retry=0: 20 integration files (mouse,record-export,host-renderer-rpc,run,cli,empty-session-capture,wait-render,batch,slow-replay,backend-selection,event-log,screen-hash,ghostty-web-crash,screenshot-out,io-loop,artifact-manifest-race,screenshot-missing-glyphs,record-diff,renderer-backend,cross-backend-screen-hash) and all 10test/e2efiles. The offline screenshot paths are inghostty-web-crash(4 tests),screenshot-out,empty-session-capture,record-exportandrenderer-backend. 299 tests passed in the full run, with 3 files red.lifecycle.test.tshas one test that reaches the renderer (failed session supports offline snapshot). It passes alone.record-exportexports animated svg with recorded timing:Hook timed out in 10000ms. Alone: 17 of 17.slow-replayanswers a run whose catch-up replay goes on for longer than the client waits:RPC request timed out after 13000msandHook timed out in 10000ms. Alone: 5 of 5.backend-selection, 6 of 7 red withTest timed out in 5000ms. Alone at load 58 to 76 it failed 2 of 7 three times with the same 5 s timeouts. With--testTimeout=120000it passed 7 of 7. Origin/main fails this file 3 of 7 the same way under load.New tests (they carry a
KNOWN-BUGheader with the upstream issue, so remove or invert them when ghostty-web has a fix):test/integration/ghostty-web-crash.test.ts, 4 tests, Linux only, all on the 12-byte log through the real CLI. The two libghostty-vt tests skip when the native package is missing. On3a5fad6cthe testreports the crash behind a libghostty-vt screenshot of an exited session as REPLAY_ERRORis red (expected 1 to be 10). The test for the live variant is green there. It guards that the live path keeps exiting 10. On origin/main the first two tests are red (screenshot <id> --json: expected 9 to be 10).test/unit/renderer/ghosttyWebCrash.test.ts, 10 tests, no browser. 5 messages that are not a crash must stay unchanged (ghostty-web bridge write() is unavailable,page.evaluate: Target page, context or browser has been closed,page.evaluate: Timeout 30000ms exceeded.,page.evaluate: Error: crash reporter is unavailableandpage.evaluate: Error: worker crashed).test/unit/replay/offlineReplay.test.ts, 1 new test: a crash thrown by the callback becomesREPLAY_ERROR(retryable: false, message with the session id and the explanation), and the backend is disposed.test/unit/host/renderer.test.ts, 2 tests: a crashed page becomes aREPLAY_ERRORthat is not retryable, and every other error passes through.Mutations (one change each to the committed tree, restored with
git checkout). An identical-text control stays green.maps a ghostty-web crash inside the callback to REPLAY_ERROR and disposes the backend. At the CLI seam: integrationreports the crash behind a libghostty-vt screenshot of an exited session as REPLAY_ERRORpropagates callback failures without wrapping and still disposes the backendwithBackendnever calls it)reports a crashed ghostty-web page as a REPLAY_ERROR that is not retryable. At the CLI seam: integrationnames the crash when a live screenshot and a live ghostty-web snapshot failandreports the crash behind a live libghostty-vt screenshot as REPLAY_ERRORreports a crashed ghostty-web page as a REPLAY_ERROR that is not retryableErrorpasses every other renderer failure through unchangedretryable: truereports a crashed ghostty-web page as a REPLAY_ERROR that is not retryableretryable: true, at the CLI seamretryablefrom the code, so the host flag never reaches it. The unit test above catches it/Target/returns the failure "page.evaluate: Target page, context or browser has been closed" as it is/crash/ireturns the failure "page.evaluate: Error: crash reporter is unavailable" as it is/crashed/returns the failure "page.evaluate: Error: worker crashed" as it isTarget page, context or browser has been closedreturns the failure "page.evaluate: Target page, context or browser has been closed" as it isTimeoutreturns the failure "page.evaluate: Timeout 30000ms exceeded." as it is/^Target crashed/)turns a crashed page into a GhosttyWebCrashedError that keeps the cause,replayTo rejects with the crash errorandreplayWithTiming rejects with the crash error/Target crashed/iTarget crashed, so I do not pin the caseThe offline-callback rows, the
/crashed/row and the live libghostty-vt test of the "host mapper off" row ran on987518b2(the last commit with code or test changes). The other host-mapper rows ran ona963cde5, and the host code is unchanged since then. The/Target/,/crash/i, closed-page,Timeout, anchored and/irows ran on3a5fad6c. The first three of them survived ona963cde5. The classifier code is unchanged since then. The mutants of the first commit (describeReplayFailure, the two wrappers and the two text sites) are red with its tests. I ran them on that commit (886f1c21) only.Dogfood with the real CLI (
node --import tsx src/cli/main.ts, ownAGENT_TTY_HOMEunder/tmp, every session destroyed), onc4f8d7b7: a live session that prints the 12 bytes.wait --text o --renderer ghostty-web --timeout-ms 90000on the live session exits 10 withREPLAY_ERROR. It took 5.9, 11.0, 11.3, 8.9 and 10.1 s in 5 runs onc4f8d7b7at host load 38 to 61 (median 10.1 s). Each of the 10 failed polls boots a new Chromium, so the time depends on the load.After each failing
screenshot, the host has two children: the program and one headless Chromium with a crashed page (5 processes, about 313 MB RSS in total). The next render command replaces it.Open PR #358 also changes
src/renderer/ghosttyWeb/backend.ts. A trial merge of this tip with the current head of #358 (e98841cd,git merge-tree --write-tree) has no conflicts, and neither has a trial merge with origin/main (9c019706).Generated with
xum• Model:anthropic:claude-sonnet-5-5• Thinking:xhigh