[ThomasK33]

Why

A JS accessor on Object.prototype could crash the process during snapshot(). snapshot() assigned its result keys with Set(), which runs a matching setter, and it read its options with Get() after it checked for dispose. An accessor that called dispose() then left native code using a freed or null terminal. A setter for row or text segfaulted Node, and a getter for includeCells hit the terminal_ != nullptr assertion (SIGABRT). Every published version has this. Reaching it needs JS in the same process, for example a prototype-pollution bug elsewhere in an app.

What changed

• BuildSnapshot, BuildLines and BuildCellObject define own data properties (DefineValue, DefineIndex), so no setter runs. Consumers get the same objects.
• snapshot() reads both options before RequireTerminal, so a dispose during that read throws "disposed".
• test/snapshot-safety.test.ts covers both paths, through the package and the raw addon, in child processes.

Scope

• getNativeInfo and module init still use Set(), because they touch no terminal.
• A follow-up moves the lifecycle model's sync commit and records that its "no JS inside native methods" assumption now holds for snapshot.

Verification

• The new tests fail on main: the setter test exits 1, and the getter test dies with SIGABRT. They pass with the fix.
• Full gate on Node 26.12.1: 77 tests, smoke, the UBSan build and tests, and the TLA+ lifecycle model.

---

Generated with xum • Model: anthropic:claude-opus-5-5 • Thinking: high

[ThomasK33]

@codex review

[ThomasK33]

@codex security review

[chatgpt-codex-connector]

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review	Status	Commit	Review trigger
📝 Code Review	✅ Completed 2026-10-30T02:36:08.347902Z	110a789	Manual request
🔒 Security Review	✅ Completed 2026-10-30T02:36:55.604343Z	110a789	Manual request

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

[chatgpt-codex-connector]

Codex Review: Didn't find any major issues. Can't wait for the next one!

Reviewed commit: 110a789915

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

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".

[chatgpt-codex-connector]

🛡️ Codex Security Review

Security review completed. No security issues were found in this pull request.

Reviewed commit: 110a789915

View security finding report

Only the user who started this review can view the report in Codex.

ℹ️ About Codex security reviews in GitHub

This is an experimental Codex feature. Security reviews are triggered when:

• You comment "@codex security review"
• A regular code review gets triggered (for example, "@codex review" or when a PR is opened), and you’re opted in so security review runs alongside code review

Once complete, Codex will leave suggestions, or a comment if no findings are found.

[ThomasK33]

PASS+NOTES

I verified this PR independently, the way a package user runs it. I did not write this code. For each commit I ran .agents/skills/verify-libghostty-vt-node/scripts/launch.sh, which builds and packs the package and installs the tarball in a scratch consumer. The journeys import @coder/libghostty-vt-node by name, and every crash case runs in its own child process. Node is v26.12.1, also inside npm scripts (unset Path), and Zig is 0.17.0.

Commits

• Head: 110a7899159b0a154a6bfb3b92afef4d4430b4a1. It equals pulls/46 .head.sha, which I checked at the start and again before posting.
• Parent: the merge base 83d79f78b1831f770e3ff045cacf7f5ed564dd96.
• Main: 35a554ab0b11344fd92d2589432473eb9f2cc092. git merge-tree --write-tree origin/main 110a789 exits 0 with tree 0984f1a7dc8a3b9afc61e66f8995590aa29791f7. The PR and the release commit change no file in common.
• Packed addon sha256: parent 5bd939e49064e955cb92b76121e323b96498e9ba671f719ae65ed58577565b18, head c9d41248b50d67c844078ecbc95f3d53dc8a6e32e3d3fbd67c9010fdaf399ee4. doctor.sh passed for both instances.

Checks

#	Check	Parent 83d79f7	Head 110a789	Result
1	Object.prototype setter for row or text that calls dispose(), then snapshot({ includeScrollback: true, includeCells: true }) on a styled screen	SIGSEGV for both keys	Exit 0. The setter fired 0 times. The snapshot equals a reference snapshot from an untouched terminal.	pass
1	The same setters with snapshot({ includeScrollback: true })	SIGABRT at terminal.cc:650 (ReadLine, terminal_ != nullptr)	Exit 0, 0 setter calls, snapshot equals the reference	pass
1	Object.prototype getter for includeCells that calls dispose(), then snapshot(). Same for includeScrollback.	SIGABRT at terminal.cc:577 (BuildSnapshot, terminal_ != nullptr)	Exit 0. The getter fired once. The call throws "GhosttyVtTerminal has been disposed".	pass
2	Re-entry probe, 294 child runs per side: 38 trap families, 4 call shapes, 2 modes	32 crashes. Setters fire for all 22 result keys, for index keys on Object.prototype and Array.prototype, and for a Proxy in the prototype chain of Array.prototype.	0 crashes. The only JS that ran inside native snapshot was the two option reads. Each dispose there threw "disposed".	pass
3	Output parity for 10 inputs and 4 option sets (40 snapshots). It compares JSON, Object.keys, Reflect.ownKeys, every property descriptor, prototypes, array holes and extensibility.	1,171,562-byte dump	Byte-identical dump. Both have sha256 137e05e7c3db7cfe2ee79bd309129ae1027a69f412b9351102c404c5d5477dee.	pass
4	npm test at head. Head's test/snapshot-safety.test.ts against the parent build.	4 of 4 new tests fail. The setter tests exit 1 and the getter tests die with SIGABRT.	77 of 77 tests pass in 6 files, with no skips	pass
5	2,000 snapshot({ includeCells: true }) calls on an 80x24 screen with 1,920 styled cells. 7 interleaved runs per side.	Median 6539.3 ms, range 6492.1 to 6692.3 ms	Median 4388.1 ms, range 4351.4 to 4462.6 ms	Head is 32.9% faster

Check 2 detail

The four call shapes are public snapshot(full), public snapshot(), raw addon snapshot(full) and raw addon snapshot({}). One mode only counts trap calls. The other mode counts them and calls dispose() on the first one. The trap families are:

• An accessor with a getter and a setter on Object.prototype, one run per key, for all 24 keys that snapshot reads or writes. These are the 22 result keys plus includeScrollback and includeCells.
• Accessors on index keys 0 to 399, once on Object.prototype and once on Array.prototype.
• length on Object.prototype. V8 refuses to redefine Array.prototype.length, so that trap cannot exist.
• constructor on both prototypes, and Array[Symbol.species].
• Symbol.toPrimitive, Symbol.iterator, Symbol.toStringTag, Symbol.isConcatSpreadable, Symbol.species, Symbol.hasInstance and Symbol.unscopables on the prototypes.
• toString, valueOf, toJSON, hasOwnProperty, isPrototypeOf, propertyIsEnumerable, toLocaleString and then on both prototypes, and Object.prototype.__proto__.
• A Proxy with all 13 traps as the prototype of Array.prototype, and a Proxy as the options object.
• Wrappers on Object.defineProperty, Object.defineProperties, Object.assign, Object.create, Reflect.defineProperty, Reflect.set, Reflect.get, Array.prototype.push, Array.from, and the global Array, Object and Error.
• A FinalizationRegistry with 200,000 entries, 50 unreferenced terminals and --max-semi-space-size=1 on a 200x60 screen. GC ran 1 to 3 times inside the snapshot call in each of the 4 runs with cells per side, and no finalizer callback ran inside the call.
• A getter on includeCells that calls feed, resize and a nested snapshot during the option read.

Every installed trap fired on a plain JS access in a positive-control step after the snapshot. A zero count therefore means that native code never touched the trap.

Head has no JS-reentry path after RequireTerminal. JS still runs during the two OptionBool reads, which come before RequireTerminal. Through the public API this happens only for snapshot() with no argument, because the wrapper passes {}. A dispose there throws "disposed".

Notes

1. On the parent, the crash signal depends on the screen. In my runs a row or text setter gave SIGSEGV only when the visible cells had styling. With plain cells it threw ghostty_terminal_grid_ref failed: GHOSTTY_INVALID_VALUE, and without includeCells it hit the ReadLine assertion. I infer that the SIGSEGV is a read of a freed page in ghostty_grid_ref_style, but I did not confirm it in a debugger. On main the new test's setter case takes the thrown-error path, so it fails with exit 1 and no signal. It still guards the regression, because it asserts that the setter never fires and that the child exits 0 with empty stderr.
2. The speedup in check 5 has an explanation. Both sides did the same work: each run asserted 1,920 cells for every timed call, and both produced output hash 990aaf8d4442438b. The runs alternated in ABBA order on a 96-core host with load average 6.9 to 12.7. I ran node --prof once per side for 600 calls and resolved the symbols with nm. Parent took 2,074 ticks and head 1,445, a ratio of 0.697. The timed ratio was 0.692. The V8 [[Set]] path (v8::Object::Set, Runtime::SetObjectProperty, LookupIterator prototype walks) took 310 ticks on parent and 13 on head. Head's define path (v8::Object::CreateDataProperty, napi_define_properties) took 83 ticks. Map-transition work fell from 293 to 152 ticks. At head the largest buckets are key string decode and internalization (314 ticks), libc (263 ticks) and the addon with libghostty-vt (194 ticks). Guess: most of the libc time is snprintf in HexColor, because libc resolves only to vfprintf and parse_printf_format there.
3. getNativeInfo() still uses Set(), as the PR's scope says. With an Object.prototype setter for napiVersion, it throws native napiVersion must be a number on both commits. It touches no terminal and does not crash.
4. Both packs report version 0.1.0-beta.1, because the PR branch predates the release commit. The release commit changes no file in native/, src/ or test/.
5. I did not rerun the UBSan build or the TLA+ lifecycle model.

---

Generated with xum • Model: anthropic:claude-opus-5-5 • Thinking: xhigh