Skip to content

Fix Ghostty search resize race#2975

Open
austinywang wants to merge 3 commits intomainfrom
issue-2738-search-resize-race
Open

Fix Ghostty search resize race#2975
austinywang wants to merge 3 commits intomainfrom
issue-2738-search-resize-race

Conversation

@austinywang
Copy link
Copy Markdown
Contributor

@austinywang austinywang commented Apr 18, 2026

Summary

  • bump the Ghostty submodule to snapshot page contents before formatting so search/highlight work does not read page storage that resizeCols has freed or recycled
  • carry cached page row counts in the retained search data so later highlight assembly does not dereference live page nodes after screen mutation
  • update docs/ghostty-fork.md with the new fork pin and conflict notes for the retained-page search path

Testing

  • not run locally per repo policy

Notes

  • the Ghostty submodule commit includes a regression test covering search data retained after the source screen is torn down

Note

Low Risk
Low risk: this PR only updates documentation and the GhosttyKit checksum mapping to reflect a new pinned Ghostty fork head; no runtime code changes in this repo.

Overview
Updates the pinned Ghostty fork reference to e36dd9d50 and documents the new search snapshot isolation change set that prevents retained search/highlight from reading page storage that can be freed/recycled during resize.

Refreshes merge-conflict notes for the retained-page search path and adds the corresponding GhosttyKit.xcframework checksum entry for the new submodule SHA.

Reviewed by Cursor Bugbot for commit eecb27a. Bugbot is set up for automated code reviews on this repo. Configure here.


Summary by cubic

Fixes a race between search/highlight and terminal resize by isolating search from live page storage. Updates the ghostty submodule and pins GhosttyKit checksums to support the snapshot-based search path (addresses issue-2738).

  • Bug Fixes

    • Snapshot page contents before formatting/search so background work never reads storage resizeCols can free or recycle.
    • Cache per-page row counts with retained search data so highlight assembly avoids dereferencing live page nodes after screen changes; fix snapshot build errors while keeping allocator error behavior.
  • Dependencies

    • Bump ghostty submodule and update docs/ghostty-fork.md with the new pin, snapshot notes, and conflict guidance; includes a regression test for retained search after screen teardown.
    • Pin GhosttyKit checksum in scripts/ghosttykit-checksums.txt for the snapshot fix.

Written for commit eecb27a. Summary will update on new commits.

Summary by CodeRabbit

  • Bug Fixes

    • Fixed a race condition in cross-page search so highlights remain correct when pages are reformatted or destroyed.
  • Documentation

    • Updated fork/submodule reference and added merge-conflict guidance documenting the retained-page snapshot and cached-row approach.
  • Tests

    • Added a regression test covering destruction of source screens while validating retained cross-page search matches.
  • Chores

    • Added checksum entry for the updated submodule.

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Apr 18, 2026

📝 Walkthrough

Walkthrough

Updated the ghostty submodule to commit e36dd9d50 and documented a fork change that snapshots page contents before formatting and caches row counts with flattened search chunks to avoid dereferencing mutated page nodes; added a regression test for validating retained cross-page matches when the source screen is destroyed.

Changes

Cohort / File(s) Summary
Documentation
docs/ghostty-fork.md
Advanced fork head to e36dd9d50; added notes documenting search/terminal behavior change, merge-conflict bullets, and the retained-page snapshot + cached-rows approach.
Submodule pointer
ghostty
Updated submodule pointer from 3b684a085 to e36dd9d50 (references commits a2f864d61, e36dd9d50).
Terminal search / formatting
src/terminal/highlight.zig, src/terminal/search/sliding_window.zig
Snapshot page contents before formatting; cache rows metadata alongside flattened search chunks to prevent later deref of mutated page nodes; adjust logic for cross-page match correctness.
Tests
... (regression test added)
Added regression test covering destruction of the source screen while validating retained cross-page matches.

Sequence Diagram(s)

sequenceDiagram
  participant Client as Client
  participant Search as Search Engine
  participant Page as Page/Screen
  participant Formatter as Formatter
  participant Test as Regression Test

  Client->>Search: initiate search across pages
  Search->>Page: snapshot page contents (copy nodes & rows)
  Page-->>Search: return snapshot
  Search->>Formatter: request formatting on snapshot
  Formatter-->>Search: return formatted chunks + cached rows
  Search->>Test: validate matches (including cross-page)
  Test->>Page: destroy source screen (simulated)
  Search-->>Client: return validated matches (no deref of original nodes)
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

Poem

🐰 I nibbled a fork and copied the page,
so matches stay safe from reflow's rage.
Rows tucked in pockets, chunks snugly kept,
Tests hopped through while the source screen slept,
A tiny hop, a stable stage. 🥕

🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and concisely summarizes the main fix: addressing a race condition between Ghostty search and terminal resize operations.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Description check ✅ Passed The pull request description covers the key changes (submodule bump, snapshot isolation approach, documentation updates) and notes testing was not run per repo policy.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch issue-2738-search-resize-race

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@ghostty`:
- Line 1: Ensure the submodule commit 641d58aed1c28ad284036e240a1f577ffb9f58d2
is pushed to your fork's origin/main so the submodule reference is reachable
from origin/main, then rebuild the GhosttyKit artifact and add the resulting
checksum entry to scripts/ghosttykit-checksums.txt (use the exact commit hash
and artifact name in the entry) before updating the PR; this ensures the
submodule reference is available and the checksum file contains a matching
record.
🪄 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: defaults

Review profile: CHILL

Plan: Pro

Run ID: 04793ad8-3aae-4a4a-b8e3-025c4926cedb

📥 Commits

Reviewing files that changed from the base of the PR and between ea438e1 and 641d58a.

📒 Files selected for processing (2)
  • docs/ghostty-fork.md
  • ghostty

Comment thread ghostty Outdated
@@ -1 +1 @@
Subproject commit 3b684a085d40ec79e8a0ae863a4f2b48ed4dba74
Subproject commit a2f864d614b5318bb0d1476d12c18d787d6033bf
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

ghostty_commit="$(git -C ghostty rev-parse HEAD)"

echo "Ghostty submodule commit: ${ghostty_commit}"

echo "Checking checksum entry..."
grep -F "${ghostty_commit}" scripts/ghosttykit-checksums.txt

echo "Checking submodule commit is on origin/main..."
git -C ghostty merge-base --is-ancestor HEAD origin/main

Repository: manaflow-ai/cmux

Length of output: 153


Push the Ghostty commit to origin/main and add the matching checksum entry.

The submodule now points to 641d58aed1c28ad284036e240a1f577ffb9f58d2, which is not reachable from origin/main and has no entry in scripts/ghosttykit-checksums.txt. Per submodule workflow requirements, push the commit to the fork's origin/main branch first, then add the checksum for the rebuilt GhosttyKit artifact.

🧰 Tools
🪛 GitHub Actions: Build GhosttyKit

[error] CI step failed: 'zig build -Demit-xcframework=true -Demit-macos-app=false -Dxcframework-target=universal -Doptimize=ReleaseFast' exited with code 1.

🪛 GitHub Actions: CI

[error] 1-1: FAIL: scripts/ghosttykit-checksums.txt is missing an entry for ghostty a2f864d614b5318bb0d1476d12c18d787d6033bf


[error] 1-1: Command failed with exit code 1.

🪛 GitHub Actions: macOS Compatibility

[error] 1-1: Missing pinned GhosttyKit checksum for ghostty a2f864d614b5318bb0d1476d12c18d787d6033bf in /Users/runner/work/cmux/cmux/scripts/ghosttykit-checksums.txt


[error] 1-1: Step failed: ./scripts/download-prebuilt-ghosttykit.sh (Process completed with exit code 1)

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@ghostty` at line 1, Ensure the submodule commit
641d58aed1c28ad284036e240a1f577ffb9f58d2 is pushed to your fork's origin/main so
the submodule reference is reachable from origin/main, then rebuild the
GhosttyKit artifact and add the resulting checksum entry to
scripts/ghosttykit-checksums.txt (use the exact commit hash and artifact name in
the entry) before updating the PR; this ensures the submodule reference is
available and the checksum file contains a matching record.

@greptile-apps
Copy link
Copy Markdown
Contributor

greptile-apps Bot commented Apr 18, 2026

Greptile Summary

This PR bumps the Ghostty submodule from 3b684a085 to a2f864d61 to fix a search/resize race condition: page contents are now snapshotted before SlidingWindow runs PageFormatter, and page row counts are cached alongside flattened search chunks so later highlight assembly never dereferences live page nodes that resizeCols may have freed. docs/ghostty-fork.md is updated with the new pin, section 9 description, and rebase conflict notes for the affected files.

  • The pinned commit is labeled issue-2738-search-resize-race rather than a git describe output from the fork's main, which conflicts with the CLAUDE.md submodule safety policy requiring commits to be on the fork's main branch before the pointer is updated.

Confidence Score: 4/5

Mostly safe to merge, but the submodule pin label suggests the commit may be on a feature branch rather than the fork's main, which violates CLAUDE.md's submodule safety policy and risks the SHA becoming unreachable.

One P1 concern: the fork pin label issue-2738-search-resize-race does not match the established git describe format from manaflow/main (e.g., tip-1717-g3b684a085), and CLAUDE.md explicitly requires the submodule commit to land on the fork's main branch before the pointer is updated. All other changes (doc structure, conflict notes) follow established patterns and are correct.

docs/ghostty-fork.md — verify the pinned commit is reachable from manaflow/main before merging.

Important Files Changed

Filename Overview
docs/ghostty-fork.md Documents the new fork pin and conflict notes for the retained-page search path; pin label format diverges from established git-describe convention, raising a submodule branch-safety concern.
ghostty Submodule pointer bumped from 3b684a085 to a2f864d61 (search snapshot isolation fix + regression test); actual Zig source changes in sliding_window.zig and highlight.zig cannot be verified as submodule is not initialized.

Sequence Diagram

sequenceDiagram
    participant R as resizeCols (main thread)
    participant S as SlidingWindow (search bg)
    participant P as PageList storage

    Note over R,P: BEFORE fix – race condition
    R->>P: free / recycle page storage
    S->>P: PageFormatter reads live page (use-after-free)
    P-->>S: ❌ freed / recycled memory

    Note over R,P: AFTER fix – snapshot isolation
    S->>P: snapshot page contents (copy)
    S->>S: cache page row counts in flattened chunks
    R->>P: free / recycle page storage (safe)
    S->>S: PageFormatter reads snapshot (not live storage)
    S->>S: highlight assembly reads cached row counts
    Note over S: ✅ no live-page dereference after screen mutation
Loading

Reviews (1): Last reviewed commit: "Fix Ghostty search resize race" | Re-trigger Greptile

Comment thread docs/ghostty-fork.md Outdated
Fork main has advanced beyond the March 30, 2026 rebase onto upstream `main`
at `3509ccf78` (`v1.3.1-457-g3509ccf78`).
Current cmux pinned fork head: `3b684a085` (`tip-1717-g3b684a085`).
Current cmux pinned fork head: `a2f864d61` (`issue-2738-search-resize-race`).
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Submodule pin may be on feature branch, not fork main

The previous pinned head was labeled tip-1717-g3b684a085 — a git describe output indicating a position on the fork's main. The new label issue-2738-search-resize-race does not follow that TAG-N-gHASH pattern and looks like a feature-branch name.

Per the CLAUDE.md submodule safety policy: "always push the submodule commit to its remote main branch BEFORE committing the updated pointer … Never commit on a detached HEAD or temporary branch — the commit will be orphaned and lost." If this commit is only on the issue-2738-search-resize-race branch and that branch is later deleted, the submodule SHA a2f864d61 could become unreachable for future git submodule update calls.

Please confirm git merge-base --is-ancestor a2f864d614b5318bb0d1476d12c18d787d6033bf manaflow/main passes, and update the pin label to the git describe form from manaflow/main if it does.

Copy link
Copy Markdown

@cubic-dev-ai cubic-dev-ai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

1 issue found across 2 files

Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="ghostty">

<violation number="1" location="ghostty:1">
P1: The submodule is pinned to commit `a2f864d61` which is not reachable from `origin/main` and has no corresponding entry in `scripts/ghosttykit-checksums.txt`. CI is failing across multiple jobs (Build GhosttyKit, CI checksum validation, macOS Compatibility). Push this commit to the fork's `main` branch and add the matching checksum entry before merging.</violation>
</file>

Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.

Comment thread ghostty Outdated
@vercel
Copy link
Copy Markdown

vercel Bot commented Apr 18, 2026

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
cmux Ready Ready Preview, Comment Apr 18, 2026 5:41am

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant