Skip to content

base red: panic ratchet rows from #34798 (link sync history slices) - #34941

Merged
lawrencecchen merged 1 commit into
feat-cmux-nextfrom
base-red/panic-rows-2
Nov 4, 2026
Merged

lawrencecchen merged 1 commit into
feat-cmux-nextfrom
base-red/panic-rows-2

Conversation

@lawrencecchen

@lawrencecchen lawrencecchen commented Nov 4, 2026 •

Copy link
Copy Markdown
Contributor

The feat-cmux-next panic ratchet is red at the tip (tip watcher, fleet job d902ffbfaf0fcbf76b13aeb2 at baa2e73 and 85dacc757ba1e7df1b5c27f5 at 6fef0f8):

  • cmux-tui: clippy::indexing_slicing 835 > baseline 831
  • cmux-tui: clippy::string_slice 83 > baseline 82
  • cmux-vault-egress: indexing_slicing 2, string_slice 5 > new crate (must be 0)

This PR removes the cmux-tui rows. The cmux-vault-egress rows are already fixed on feat by 17f9f49 (#34921).

Culprits

Rows Commit Sites Fix
cmux-tui indexing_slicing +4, string_slice +1 c92b4ad (#34798, link sync undo/history/restore) link/sync_cli/history.rs: four &buffer[..read] in the copy loops of keep_blob_inner and keep_file, one &sha256[..2] in blob_path this PR: buffer.get(..read) (error if a read says more than the buffer holds), sha256.get(..2) behind the same SHA-256 check
cmux-vault-egress indexing_slicing 2, string_slice 5 a664e6a (#34708, vault egress proxy, new crate) substitute.rs already on feat: 17f9f49 (#34921)

How the culprit was found (panic rows of the tip watcher per tip): cf86f55 had cmux-tui 836 / 82 (job 8430b3923f14690597d98d7c; 5 of these were the rows #34735 fixed). c92b4ad (#34798) had 840 / 83 (job be282ef707053535784f889f): +4 and +1. #34735 (9a1aa2f) then removed 5, so da8ca9d had 835 / 83 (job 505d70d3d95173d207fdb61c). #34634, #34782 and 93703fe add no rows. The stale push 164d65f took back #34798 and #34735 for a time (tips bd14c57..f291b6c showed acpmux 689 > 686 again); 93703fe put both back, and the seven #34735 files on feat are identical to 9a1aa2f.

Behavior is unchanged: Read::read never returns more than the buffer length, and blob_path already refuses anything but 64 hex digits, so get(..2) always has a value.

Verification

  • Panic ratchet (Linux, the tip watcher's step cmux-tui-rust-check.sh panic) at b9171a2: job 54bcb0bfee7258f1e6396abd: pass (89 crates checked; no row over baseline, cmux-vault-egress at 0)
  • Clippy (Linux, -D warnings, all targets) at b9171a2: job 1d5ed720fe12d1fb1de84c84: red on a base finding, not this PR: clippy::redundant_clone in crates/cmux-git/tests/sync_barrier.rs:108 (from c30af19 fix(cloud): wait for repo sync before native agent tools #34782; the same on the feat tip). The build stopped there, so this run gives no clippy result for cmux-tui; the panic step's clippy --lib --bins covers the changed file
  • nextest undo (the five tests of tests/link_sync_history.rs, owned Mac) at b9171a2: job c443db895433e13e4e7982e8: pass (72 tests run, 72 passed, among them all 5 cmux-tui::link_sync_history tests)
  • python3 scripts/cmux-next/prepush-gate.py: no new violations.

Observed / Not run

Observed: panic ratchet green and nextest undo green at the head (jobs above); the red rows and their per-tip counts in the tip watcher's fleet jobs named above; the diff of #34798 (the five sites); the #34735 files on feat compared with 9a1aa2f (no diff); the prepush gate.
Not run: no regression test is added. The change takes a panic path that cannot happen (a read past its buffer, a short SHA-256) and returns an error there instead; no input reaches it. The existing link sync history tests cover the copy loops.

Changelog

none

… get (panic ratchet, from #34798)

c92b4ad (#34798, link sync undo/history/restore) added four
`buffer[..read]` slices and one `sha256[..2]` str slice in
link/sync_cli/history.rs: cmux-tui clippy::indexing_slicing 835 > 831 and
clippy::string_slice 83 > 82 on feat-cmux-next. The copy loops now take
`buffer.get(..read)` and return an error if a read ever says more than the
buffer holds; the blob shard is `sha256.get(..2)` behind the same
SHA-256 check. Behavior is unchanged: Read::read never returns more than
the buffer length, and the 64-hex check already holds for the shard.
@lawrencecchen lawrencecchen added the risk:normal Risk tier (docs/ci/risk-tiers.md) label Nov 4, 2026
@lawrencecchen

Copy link
Copy Markdown
Contributor Author

Base finding, not from this PR: the clippy step 1d5ed720fe12d1fb1de84c84 at b9171a2 is red on clippy::redundant_clone in cmux-tui/crates/cmux-git/tests/sync_barrier.rs:108 (let target = repo.clone();, from c30af19 #34782). This PR does not touch that file, and the file is the same on the feat-cmux-next tip. The build stopped at that error, so this clippy run gives no result for cmux-tui. The panic step (clippy --lib --bins) and nextest still check the changed code.

@blacksmith-sh

blacksmith-sh Bot commented Nov 4, 2026

Copy link
Copy Markdown

Found 6 test failures on Blacksmith runners:

Failures

Test View Logs
/tmp/tmp_bmhxuny/summary-missing.json (live):/summary-missing.json (live): View Logs
/tmp/tmpzhzjgxvj/summary-fail.json (live):/summary-fail.json (live): View Logs
[new_workspaces] debug.new_workspace timed 0 of 50 new workspace rows/
[new_workspaces] debug.new_workspace timed 0 of 50 new workspace rows
View Logs
[new_workspaces] the window draws no frames/[new_workspaces] the window draws no frames View Logs
[tabs] debug.tab_switch timed 0 of 40 switches/
[tabs] debug.tab_switch timed 0 of 40 switches
View Logs
[tabs] the window draws no frames/[tabs] the window draws no frames View Logs

Fix with [code]smith
Need help on this PR? Tag @codesmith-bot with what you need.

@github-actions

github-actions Bot commented Nov 4, 2026 •

Copy link
Copy Markdown
Contributor

cmux status (one comment, edited in place; each line is updated by the job that owns it)

red: cmux-next checks (god files, concurrency, crash safety, l10n); NEW: red check: Base-red classifier (per test) (+1 more) is not red on the base aa2301b; do not merge until green; see lane-rules

Shipped in:

@lawrencecchen

Copy link
Copy Markdown
Contributor Author

Review (claude-review-subagent): SHIP at b9171a2

Checked (reading only, no local build):

  • blob_path: the 64-hex-digit check still runs first, so sha256.get(..2) always has a value. The else branch gives the same bail! message. The path is unchanged.
  • keep_blob_inner (anyhow::Result): buffer.get(..read).context(...) works with anyhow::Context as _, which the file already imports. One chunk goes to both hasher.update and write_all, as before.
  • keep_file (closure returns std::io::Result<()>): ok_or_else(|| std::io::Error::other(...))? has the correct type. io::Error::other is stable since Rust 1.74, and the pinned toolchain is 1.95.0. An error here sets copied to Err, the same result as any other read error.
  • Read::read never returns more than buffer.len(), so neither new error path can run. There is no new panic and no behavior change.
  • No [..] slice is left in history.rs. Fleet job 54bcb0bfee7258f1e6396abd (panic ratchet at this head): pass, with no indexing_slicing or string_slice row over baseline for cmux-tui. The remaining warnings (arithmetic_side_effects, cast_possible_truncation) are advisory lints that are not at deny.
  • Fleet job c443db895433e13e4e7982e8: 72/72 pass, including all 5 cmux-tui::link_sync_history tests.
  • The new single-line let ... else matches rustfmt use_small_heuristics = "Max" and the existing style in the file.
  • The PR description has risk:normal, Changelog none, and Observed / Not run. It explains why there is no regression test (the paths cannot be reached), and I agree with that reason.

@lawrencecchen lawrencecchen added the ready-to-land Reviewed at this head; next-autoland merges it into feat-cmux-next when its only reds are base reds label Nov 4, 2026
@lawrencecchen
lawrencecchen merged commit 89c6b93 into feat-cmux-next Nov 4, 2026
249 of 264 checks passed
@lawrencecchen
lawrencecchen deleted the base-red/panic-rows-2 branch November 4, 2026 06:55

This branch was previously deployed

3 inactive deployments
artifacts — b9171a28 Deployed Nov 4, 2026 by lawrencecchen via publish tree-addressed cmux-tui #110046
sentry-dsyms — b9171a28 Deployed Nov 4, 2026 by lawrencecchen via upload cmux-tui dSYMs to Sentry #110046
ci-routing-agent — b9171a28 Deployed Nov 4, 2026 by lawrencecchen via cmux-tui tree preflight #110046
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ready-to-land Reviewed at this head; next-autoland merges it into feat-cmux-next when its only reds are base reds risk:normal Risk tier (docs/ci/risk-tiers.md)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant