Skip to content

Preserve wide-character invariants during no-reflow clear resize#12244

Open
ChanlerDev wants to merge 1 commit into
warpdotdev:masterfrom
ChanlerDev:codex/row-iterator-shrink-wide-char-fix
Open

Preserve wide-character invariants during no-reflow clear resize#12244
ChanlerDev wants to merge 1 commit into
warpdotdev:masterfrom
ChanlerDev:codex/row-iterator-shrink-wide-char-fix

Conversation

@ChanlerDev
Copy link
Copy Markdown

Description

Fixes a RowIterator::next crash that can occur after FullGridClearBehavior::Clear resizes the active grid without reflow.

In that path, GridStorage::shrink_cols(reflow=false) can truncate a valid wide-character pair between the WIDE_CHAR cell and its WIDE_CHAR_SPACER, leaving an orphaned final-column WIDE_CHAR. If that row later enters flat storage, materialization can attempt to write the missing spacer beyond the end of the row and panic.

This PR fixes the producer side of the issue: after no-reflow shrink, if the new final cell is WIDE_CHAR and the first discarded cell was its matching WIDE_CHAR_SPACER, the final cell is no longer marked as double-width. This preserves the row invariant after truncation and prevents FlatStorage::pop_rows / RowIterator::next from materializing an impossible spacer.

Scope note: this is intentionally a source-side fix for the confirmed FullGridClearBehavior::Clear + shrink_cols(reflow=false) producer. It does not claim to be a general hardening layer for every possible malformed flat-storage row.

Linked Issue

Fixes #12243

  • The linked issue is labeled ready-to-implement.
  • Where appropriate, screenshots or a short video of the implementation are included below (especially for user-visible or UI changes).

Note: this is a non-visual crash fix. The linked issue includes sanitized crash-report and Warp-log excerpts plus the deterministic regression shape. The issue has already received automated triage labels triaged and repro:high, but it has not yet received ready-to-implement; please apply readiness during triage if appropriate.

Testing

Automated regression coverage:

cargo test -p warp -- test_full_grid_clear_shrink_cols_does_not_orphan_wide_char_at_boundary -- --nocapture
# 1 passed; 0 failed

cargo test -p warp -- test_full_grid_clear_resize -- --nocapture
# 3 passed; 0 failed

./script/format --check
# passed

./script/presubmit status:

./script/presubmit
# Summary [1038.785s] 7227 tests run: 7220 passed (4 slow, 1 leaky), 6 failed, 1 timed out, 85 skipped

Observed presubmit failures:

FAIL integration::integration shell_integration_tests::test_legacy_ssh_into_bash
FAIL integration::integration shell_integration_tests::test_legacy_ssh_into_zsh
FAIL integration::integration shell_integration_tests::test_ssh_into_ash
FAIL integration::integration shell_integration_tests::test_ssh_into_sh
FAIL integration::integration ui_tests::test_ssh_with_shell_override
FAIL warp ai::agent_sdk::driver::harness::codex::tests::resolve_openai_api_key_returns_value_from_resolved_map
TIMEOUT warp editor::view::model::buffer::tests::test_random_concurrent_operations

None of the observed failures are in the modified grid storage / row invariant tests. The focused regression tests for this PR pass, but the full presubmit run did not complete green in this local environment.

Regression red/green proof:

# Before the fix, at commit 6dd1dcf4:
cargo test -p warp -- test_full_grid_clear_shrink_cols_does_not_orphan_wide_char_at_boundary -- --nocapture

running 1 test
test terminal::model::grid::grid_handler::tests::test_full_grid_clear_shrink_cols_does_not_orphan_wide_char_at_boundary ... FAILED

thread 'terminal::model::grid::grid_handler::tests::test_full_grid_clear_shrink_cols_does_not_orphan_wide_char_at_boundary' panicked at app/src/terminal/model/grid/grid_handler_tests.rs:1518:13:
Orphaned WIDE_CHAR at column 116: next cell is not a WIDE_CHAR_SPACER.

test result: FAILED. 0 passed; 1 failed; 0 ignored; 0 measured; 4829 filtered out

The new regression test exercises the relevant producer path:

  • Creates a 155-column GridHandler.
  • Enables FullGridClearBehavior::Clear.
  • Builds a valid wide-character pair at the shrink boundary: WIDE_CHAR@116, WIDE_CHAR_SPACER@117.
  • Resizes through the real grid.resize(...) API to 117 columns.
  • Asserts the row has no orphaned wide-character flags.
  • Pushes/pops the row through flat storage to verify materialization no longer panics.

Manual testing:

  • I have manually tested my changes locally with ./script/run

Manual testing note: ./script/run built, bundled, signed, and launched WarpOss.app locally. The original user-visible crash depends on a narrow timing/layout condition involving command enter, clear hook handling, resize, and a wide-character boundary, so the deterministic GridHandler regression test is the primary proof for the confirmed producer-side corruption path.

Screenshots / Videos

This is a non-visual crash fix. A screenshot is not meaningful for the code change itself. If maintainers request manual evidence, I can attach a short local smoke-test recording showing Warp running through resize/clear activity without crashing.

Agent Mode

  • Warp Agent Mode - This PR was created via Warp's AI Agent Mode

CHANGELOG-BUG-FIX: Fixed a crash that could occur after resizing CLI-agent terminal output containing wide characters.

Co-Authored-By: Warp agent@warp.dev

@cla-bot
Copy link
Copy Markdown

cla-bot Bot commented Jun 5, 2026

Thank you for your pull request and welcome to our community. We require contributors to sign our Contributor License Agreement, and we don't seem to have the users @ChanlerDev on file. In order for us to review and merge your code, each contributor must visit https://cla.warp.dev to read and agree to our CLA. Once you have done so, please comment @cla-bot check to trigger another check.

@github-actions github-actions Bot added the external-contributor Indicates that a PR has been opened by someone outside the Warp team. label Jun 5, 2026
@ChanlerDev ChanlerDev marked this pull request as ready for review June 7, 2026 17:56
@oz-for-oss
Copy link
Copy Markdown
Contributor

oz-for-oss Bot commented Jun 7, 2026

@ChanlerDev

Every PR must be linked to a same-repo issue before Oz can review it.

This PR is linked to #12243, but no linked issue is marked ready-to-implement yet. Only repository maintainers apply that label, so please wait for a maintainer to mark the issue. Once it is marked, push a new commit or comment /oz-review to re-trigger review.

See the contribution guidelines for the full readiness model.

Powered by Oz

Copy link
Copy Markdown
Contributor

@oz-for-oss oz-for-oss Bot left a comment

Choose a reason for hiding this comment

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

@ChanlerDev

Every PR must be linked to a same-repo issue before Oz can review it.

This PR is linked to #12243, but no linked issue is marked ready-to-implement yet. Only repository maintainers apply that label, so please wait for a maintainer to mark the issue. Once it is marked, push a new commit or comment /oz-review to re-trigger review.

See the contribution guidelines for the full readiness model.

Powered by Oz

@ChanlerDev
Copy link
Copy Markdown
Author

@cla-bot check

@cla-bot cla-bot Bot added the cla-signed label Jun 7, 2026
@cla-bot
Copy link
Copy Markdown

cla-bot Bot commented Jun 7, 2026

The cla-bot has been summoned, and re-checked this pull request!

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

Labels

cla-signed external-contributor Indicates that a PR has been opened by someone outside the Warp team.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Warp Preview can crash in RowIterator::next after clear resize truncates a wide-character spacer

1 participant