Repository navigation
feat: pin Ghostty v1.4.0 so DECSTR soft resets apply - #43
Conversation
Ghostty v1.4.0 needs Zig 0.17.0 and replaces two C calls. Terminal creation now takes cols and rows and sets scrollback through ghostty_terminal_set. The byte limit keeps the old scrollbackLimit behavior, including 0 for no history. Mode reads go through ghostty_terminal_get. _Generated with [`xum`](https://github.com/coder/xum) • Model: `anthropic:claude-opus-5-5` • Thinking: `high`_ Change-Id: I04864bff70e3982b1b76e833037e9afed8dfd574 Signed-off-by: Thomas Kosiewski <tk@coder.com>
|
@codex review |
|
@codex security review |
Codex Review SummaryThis 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. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 90c38d0254
ℹ️ 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".
🛡️ 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. |
The verify skill's fallback PATH named Zig 0.15.2, which the new pin rejects. dogfood/regenerate.sh reruns commands.sh and rewrites the bundle, which now records a passing build on the new pin. _Generated with [`xum`](https://github.com/coder/xum) • Model: `anthropic:claude-opus-5-5` • Thinking: `high`_ Change-Id: Ib17d40e43d656e1d2e6185959c2af87b31501e8c Signed-off-by: Thomas Kosiewski <tk@coder.com>
|
@codex review |
|
@codex security review |
🛡️ 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. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: aa49c04447
ℹ️ 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".
|
FAIL I am an independent verifier. I did not write this change.
I packed each commit with Why FAIL: one resize regressionA single A consumer that forwards window resizes to Repro journey ( import assert from "node:assert/strict";
import { createTerminal } from "@coder/libghostty-vt-node";
const t = createTerminal({ cols: 80, rows: 24, scrollbackLimit: 10_000 });
t.feed(Array.from({ length: 40 }, (_, i) => `out ${i}\r\n`).join("") + "$ ");
t.resize(80, 10);
t.resize(60, 30); // narrower and taller in one call
const s = t.snapshot({ includeScrollback: true });
assert.equal(s.cursorRow, 29); // main: 29, head: 9
A seeded fuzz of 400 random sessions shows the same pattern in 30 scenarios. In 25 of them, doing the resize at head as two calls (rows first) reproduces main exactly. Likely cause. I inferred this from the Ghostty source and did not bisect. Ghostty Claims
Claim 2 details for 80x24 with 200 lines fed: every nonzero limit keeps 177 lines at both commits. For 20x3 with 2,000 lines fed: every nonzero limit keeps 1998 at both. Difference listMy compare script found 90 differences in 1,960 Expected
Upstream fix (Ghostty commits between
Regression
Other differences that I did not trace one by one: 22 fuzz scenarios differ in layout after a resize, 2 differ after a DECRC that follows resizes, and 1 ( Notes
Review count: this is one verifier pass on this PR. I did not review the code beyond the C API port, and I made no edits. Generated with |
Ghostty v1.4.0 skips the scrollback pull when one resize both narrows the grid and adds rows, so the prompt stayed up with blank rows below it. Resizing the rows first, at the old width, restores the earlier result. regenerate.sh now fails when a step fails or when npm scripts run a Node other than the mise.toml pin. _Generated with [`xum`](https://github.com/coder/xum) • Model: `anthropic:claude-opus-5-5` • Thinking: `high`_ Change-Id: I6b009462abc8865ad9052dd655bc3c8d3107e642 Signed-off-by: Thomas Kosiewski <tk@coder.com>
_Generated with [`xum`](https://github.com/coder/xum) • Model: `anthropic:claude-opus-5-5` • Thinking: `high`_ Change-Id: I6b79e14cd14acdb142433ed6f7588cd17e8a629f Signed-off-by: Thomas Kosiewski <tk@coder.com>
|
@codex review |
|
@codex security review |
🛡️ 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. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7ff47a56ba
ℹ️ 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".
|
PASS+NOTES I am an independent verifier. I did not write this change. The resize regression from the earlier FAIL (#43 (comment)) is gone, and I found no new regression. The notes below are pre-existing limits or upstream Ghostty behavior, and none of them blocks the merge.
I packed each commit with Checks
How the head changes trace to the fixCompared with the earlier head run, head output changed in these journeys, and nowhere else:
I also proved the trace mechanically. I removed the block in a throwaway clone and replayed all 400 fuzz scenarios on that build. Plain replay reproduces the earlier head hashes in 400 of 400 scenarios. When the replay splits every narrower and taller resize into rows first, then cols, from JS, it reproduces the new head hashes in 400 of 400. So the fix changes nothing else. Diff against the earlier FAIL classification
Of the 32 regression scenarios, 16 now match main after every op. The other 16 now show a different first difference, and I classified each one by replaying both packed builds through that op:
Eight more scenarios ( Notes
Review count: this is the second verifier pass on this PR. The first one posted FAIL. I made no edits to the repo. Generated with |
_Generated with [`xum`](https://github.com/coder/xum) • Model: `anthropic:claude-opus-5-5` • Thinking: `high`_ Change-Id: I05348d97a66fea2f4619268072c74aee96e29342 Signed-off-by: Thomas Kosiewski <tk@coder.com>
|
@codex review |
|
@codex security review |
|
Codex Review: Didn't find any major issues. Breezy! 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. |
## Why Ship the changes merged since `0.1.0-beta.1` to npm. They include mode-aware mouse encoding (#12), the new snapshot attributes and `cursorVisible` (#17, #38), and Ghostty `v1.4.0` with DECSTR support (#43). ## What changed - `package.json` and `package-lock.json` go from `0.1.0-beta.1` to `0.1.0-beta.2`. The branch name matches the version, so `publish.yml` creates `v0.1.0-beta.2` after merge. - The `release-changelog` workflow adds the CHANGELOG entry to this branch. - `communique.toml` raises `max_tokens` to 16384. With Communique 1.0.3's default of 4096, generating these notes failed twice with "submit_release_notes was malformed 3 times". The next run succeeded. - `prepare-release-changelog.mjs` now detects Communique's `## [vX](url) - date` heading. Before, every push regenerated the entry over hand edits. - I corrected the generated entry. `cursorVisible` and `encodeMouse` first ship in this release, so they are listed as Added, not as changes or fixes. ## Blast Radius The merge publishes `0.1.0-beta.2` to npm under the `beta` dist-tag. `latest` stays on `0.1.0-beta.0`. Consumers that build a `TerminalSnapshot` themselves must now set `cursorVisible` (#38). ## Verification - `RELEASE_BRANCH_NAME=release/v0.1.0-beta.2 node .github/scripts/prepare-release-changelog.mjs origin/main` prints `should_generate=true` and `tag_name=v0.1.0-beta.2`. `resolve-npm-publish-tag.mjs` prints `beta`. - `npm run typecheck` and `npm test` (73 passed). - Three independent verifier passes, the last PASS+NOTES on the head. They check every CHANGELOG bullet against the `v0.1.0-beta.1` tag and confirm that the packed tarball loads its addon from `prebuilds/`. - The local linux-x64 prebuild steps from `mise run release-prebuilds` pass: `build:libghostty`, `build`, `build:prebuild`, `verify:prebuilds` and `npm pack --dry-run`. --- _Generated with [`xum`](https://github.com/coder/xum) • Model: `anthropic:claude-opus-5-5` • Thinking: `high`_
## Why The `verify-native-lifecycle` skill keeps a sync commit. At that commit, its TLA+ model describes the covered native code. The sync commit was still `7366b75` (#12). Since then, #40 to #43, #46, and #49 changed covered code. #46 promised this follow-up. ## What changed - The sync commit moves to `c54051e`, the current main. It includes #48 and #49. - The coverage rows now name `ghostty_terminal_set` (scrollback byte limit) in the constructor and its failure path, `Resize`'s second `ghostty_terminal_resize` call, and the `GHOSTTY_TERMINAL_DATA_MODE` read in `EncodeMouse`. Ghostty v1.4.0 (#43) brought these. - The "no JS inside native methods" assumption now says how each method reads its arguments: 1. `Snapshot` reads its options before `RequireTerminal`, and it builds its result with `DefineValue` and `DefineIndex`. 2. `Feed` and `Resize` call `RequireTerminal` first. After that, they only test types and read strings, bytes, and numbers, which runs no JS. 3. `GetVisibleText`, `FormatPlain`, and `FormatHtml` read no arguments. 4. The assumption was false for `snapshot` until #46. - The bullet that lists where caller JS can run now names public `feed`: `assertFeedData` runs `data instanceof Uint8Array`, which runs a Proxy's `getPrototypeOf` trap after `#assertUsable`. The native null check covers it, as it does for `snapshot`. - `AddonEnvLifecycle.tla` changes in comments only. The `CreateFails` comment now names the `ghostty_terminal_set` failure too. The model itself does not change. #48 changes only a test. #49 changes only `Feed`'s argument checks, so the `Use(t)` step still describes `feed`. ## Verification - I checked each row against `native/terminal.cc` and `src/index.ts` at `c54051e`. - `git diff 7366b75 c54051e` on the covered paths adds no new static state, `Napi::Reference`, or new place that frees `terminal_` or `mouse_encoder_`. - `run.sh`: all 35 rows match EXPECT (`OK: every result matches EXPECT.`). The current config holds all 5 rules. Each of the 6 mutants breaks the rule it guards. The verifier's two notes on `0ff0941` (the `feed` window and the `CreateFails` comment) are fixed in `584c561`. `run.sh` still passes all 35 rows. Workspace 36beaed175 drafted the first version of this commit. I corrected the assumption bullet: it said that every method reads its arguments before `RequireTerminal`, but `Feed` and `Resize` do not. --- _Generated with [`xum`](https://github.com/coder/xum) • Model: `anthropic:claude-opus-5-5` • Thinking: `xhigh`_
Closes #37
Why
Terminals ignored the DECSTR soft reset (
ESC[!p), so a program that reset with it left the cursor hidden and the pen styled in every later snapshot. This moves the Ghostty pin to thev1.4.0release, which includes upstream DECSTR support.What changed
v1.4.0and Zig 0.17.0 (mise.toml,mise.lock, the build script and its docs).native/terminal.ccfollows two C API changes. Terminal creation now sets scrollback withghostty_terminal_set, and the UTF-8 mouse mode read usesghostty_terminal_get.resizedoes a narrower and taller resize in two steps, rows first. Ghostty v1.4.0 skips the scrollback pull when one call does both, which left the prompt up with blank rows below it.Scope
Tradeoffs
scrollbackLimitmaps to the byte limit, not the new line limit. The old Ghostty countedmax_scrollbackin bytes, and only the byte setter keeps 0 as "no history". The unit was never documented, so changing it would be a silent behavior change.v1.4.0release instead off473f31, the first commit with DECSTR. It is a supported release, and it needs Zig 0.17.0 instead of 0.16.0.Blast Radius
Every terminal now runs on Ghostty
v1.4.0(2,472 upstream commits). An independent verifier diffed main against this branch through the packed package. The differences it traced to upstream fixes include reflow cursor placement, C1 and C0 control characters no longer printing as cells, Unicode 18 width tables, andformatHtmlno longer adding a newline at each page. Building from source now needs Zig 0.17.0.Upstream changes consumers can see
These come from Ghostty
v1.4.0, not from this binding:scrollbackLimitof 0 no longer keeps rows from a scroll region at the top.c2a0,c2a1).formatHtmlno longer adds a newline at each page.Verification
npm pack, the UBSan build and tests, and the TLA+ lifecycle model.mise.lockmatch ziglang.org'sindex.jsonfor linux-x64, linux-arm64 and macos-arm64.Generated with
xum• Model:anthropic:claude-opus-5-5• Thinking:high