Skip to content

Base red: acpmux panic ratchet (string_slice 98 > 94, indexing_slicing 703 > 693): rewrite the new hits without panics - #28134

Merged
teamleaderleo merged 10 commits into
feat-cmux-nextfrom
lane/acpmux-string-slice-ratchet
Oct 30, 2026
Merged

teamleaderleo merged 10 commits into
feat-cmux-nextfrom
lane/acpmux-string-slice-ratchet

Conversation

@teamleaderleo

@teamleaderleo teamleaderleo commented Oct 30, 2026 •

Copy link
Copy Markdown
Collaborator

Base-red fix: the Rust panic ratchet step of cmux-tui.yml is red on feat-cmux-next for acpmux: clippy::string_slice 98 > baseline 94 (since 7acff13) and clippy::indexing_slicing above 693 (700 at e863077, 703 by 7f97b84). Every new acpmux hit in production code is rewritten so it can't panic and behaves the same.

Where the hits came from (blame), and what replaced them

commit file hits now
84705bd (#26984) hub/restart_resume.rs 4 string_slice, 1 indexing_slicing char_prefix helper with str::get; get(..i); BoundedJsonWriter copies with take(remaining)
72c0200 (#28046) cli/ls_table.rs, cli/run/ls.rs 6 indexing_slicing get / get_mut in table() and render(); JsonSet::set for the ls params and sessions
95243d1 (#27685) hub/cancel.rs 1 set("restarted", ..)
e863077 (#27809) hub/remote_rows.rs 4 mark_peer_stale reads status, stale and lastSeen with get (a missing field compares the same as the Null that indexing returned)
9d318f9 (#28000) cli/resolve.rs, hub/key_route.rs 4 choice_commands uses enumerate + checked_sub / get; key_route reads name and status with get
65b5de8 (#28016) cli/orchestrate.rs 1 set(HARNESS_FROM_FIELD, ..)
7050ae3 (#27038) hub/message_events.rs, hub/mod.rs, prompt_provenance.rs, server/outbox.rs, server/peer_forward.rs 7 writes via set; outbox reads via pointer (missing reads as Null, as before)
e0f3350 (#27614) hub/paging.rs 2 the moot slot is read and cleared with get / get_mut
4a0b8f0 (#28172) server/requests/briefs.rs 1 set("peersFailed", ..)
0a088aa (#28154) hub/turns.rs 2 the stop reason and the stopped answer are read with get

JsonSet::set (cmux-json) inserts on an object, turns Null into an empty object first (as IndexMut does), and leaves any other value alone. IndexMut would have panicked on those. Every value written here is an object it just built.

Baseline

acpmux measured at 339e742 (fast-checks run 41057101016): indexing_slicing 688, string_slice 94, unwrap_used 143, all at or below the baselines (693 / 94 / 144). The baseline file is unchanged. New acpmux hits landed on base three times while this PR was open, so a baseline pinned to today's exact count would turn base red as soon as another one lands before this merges. A follow-up can lower it once base is quiet.

Tests

  • restart_resume::multibyte_text_is_cut_on_char_boundaries: multibyte text through each of the four string cuts. It includes a 4096-byte scan cut inside a 3-byte char, a truncated tail cut at U+3000, and CJK text before PASSWORD=.
  • ls_table_tests::a_short_or_empty_row_pads_without_panicking: an empty table, rows shorter than the header (they panicked before), and a row longer than the header.

Observed / Not run

  • At 339e742 (run 41057101016), gated: cargo clippy --locked --keep-going -p acpmux -p cmux-tui --all-targets -- -D warnings passes (365994d had three unused import: cmux_json::JsonSet errors from this PR, fixed in the last commit). acpmux counts are 688 / 94 / 143, and cargo test -p acpmux --lib 2184 passed, 0 failed.
  • The ratchet step is still RED at that run for a different crate, cmux-tui: clippy::indexing_slicing 861 > baseline 833, which this PR does not touch. A separate PR is fixing it.
  • Earlier, at a0bd7c6: the ratchet step as written in the workflow, plus 26 restart_resume tests, all green (run 41023303857).
  • Not run: macOS builds (no Swift change).

Review (gpt-6.1-sol, high, fast): SHIP at 339e742 (the import fix), and SHIP at 365994d for everything before it, with no findings (earlier SHIPs: a0bd7c6, 08fb305, 84671eb).

Changelog

none

risk:low (panic-free rewrites with the same output, tests, and a lower baseline).

Agent session: agent session

🤖 Generated with Claude Code

…lice hits from #26984)

The Rust panic ratchet is red on feat-cmux-next: acpmux string_slice 98 > baseline 94.
The four new hits are in hub/restart_resume.rs, added by 84705bd (#26984):
input_prefix and masked_scan cut at a char boundary with `[..end]`,
mask_input_assignments with `[..assignment.end()]`, and safe_tool_text with `[..at]`.

The two char-boundary cuts share one `char_prefix` helper that reads with `str::get`;
the regex-end and rfind cuts use `get` too. Every index was already a char boundary,
so behaviour is unchanged; the unreachable None arms keep the redacting side
("[secret]" alone, or an empty prefix). A new test cuts multibyte text at each spot.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Agent-Session: https://cmux.com/deeplink/agent?id=aid-d9e7e9f7fb2b87a57d5f
@github-actions

github-actions Bot commented Oct 30, 2026 •

Copy link
Copy Markdown
Contributor

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

Review 339e742: HQ feed card · gallery before/after runs · checks

App of this head (app.zip, kept 1 day)

gh workflow run reload-build.yml -R manaflow-ai/cmux --ref feat-cmux-next -f tag=pr28134 -f ref=lane/acpmux-string-slice-ratchet -f platform=macos -f backend_mode=local

red: cmux-next checks (god files, concurrency, crash safety, l10n); NEW: 3 commit(s) change the sidebar's drawing code without a Sidebar-Look: trailer or an entry in plans/cmux-next/sidebar-owner.md is not red on the base 6607ecd; do not merge until green; see lane-rules

@github-actions github-actions Bot added the risk:normal Risk tier (docs/ci/risk-tiers.md) label Oct 30, 2026
@teamleaderleo

teamleaderleo commented Oct 30, 2026 •

Copy link
Copy Markdown
Collaborator Author

Review (the owner lane's independent review, at its land request): SHIP at a0bd7c6

Recorded by lane-gh-broker from this PR's owner lane, which asked to land this exact head after its independent review. next-autoland lands it once every red is the base's; a BLOCK at this head, a HOLD line or a hold label still stops it.

@blacksmith-sh

blacksmith-sh Bot commented Oct 30, 2026 •

Copy link
Copy Markdown

Found 6 test failures on Blacksmith runners:

Failures

Test View Logs
main.DeveloperIdAppTests.test_download_prints_the_active_profile_only/
test_download_prints_the_active_profile_only
View Logs
stubs: /System/Library/Frameworks/Security.framework/Versions/A/Security has no stub/
Security has no stub
View Logs
stubs: CoreFoundation.tbd: install-name /System/Library/Frameworks/
CoreFoundation.framework/Versions/A/CoreFoundation, missing 1: ['_CFRetain']/
CoreFoundation, missing 1: ['_CFRetain']
View Logs
stubs: CoreFoundation.tbd: install-name /System/Library/Frameworks/
CoreFoundation.framework/Versions/A/CoreFoundation, missing 1: ['_log2']/
CoreFoundation, missing 1: ['_log2']
View Logs
test_every_listed_hook_is_one_glaeda_released_with_the_check (main.SigningHooksInSt
epWithGlaedaTests.test_every_listed_hook_is_one_glaeda_released_with_the_check)/
test_every_listed_hook_is_one_glaeda_released_with_the_check (main.SigningHooksInSt
epWithGlaedaTests.test_every_listed_hook_is_one_glaeda_released_with_the_check)
View Logs
test_the_repository_highlights_validate (main.DigestTests.test_the_repository_highl
ights_validate)/
test_the_repository_highlights_validate (main.DigestTests.test_the_repository_highl
ights_validate)
View Logs

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

@teamleaderleo

Copy link
Copy Markdown
Collaborator Author

Base merge 0185b3d (clean; the PR diff is unchanged from a0bd7c6, and base touched neither restart_resume.rs nor the panic baseline), so the gpt-6.1-sol SHIP at a0bd7c6 carries. This also reruns 'cmux-tui artifacts / publish to R2', which failed only on the attestation step's API rate limit (infra, not this diff).

…rom #28046)

table() read each row's cell and each column's width by index, and render()
bumped a hidden count by index. They now use get / get_mut. A row shorter than
the header used to panic in table(); now its missing cells are left out. A
row longer than the header has its extra cells padded to no width.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Agent-Session: https://cmux.com/deeplink/agent?id=aid-d9e7e9f7fb2b87a57d5f
…ts from #28046, #27685, #27809, #26984)

- cli/run/ls.rs (72c0200, #28046): the ls request params and the
  filtered sessions write through JsonSet::set.
- hub/cancel.rs (95243d1, #27685): forced_answer's `restarted` too.
- hub/remote_rows.rs (e863077, #27809): mark_peer_stale reads status,
  stale and lastSeen with get. A missing field compares the way the Null
  that indexing returned did.
- hub/restart_resume.rs (84705bd, #26984): BoundedJsonWriter copies at
  most `remaining` bytes with take, not a slice.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Agent-Session: https://cmux.com/deeplink/agent?id=aid-d9e7e9f7fb2b87a57d5f
@teamleaderleo teamleaderleo changed the title Base red: acpmux panic ratchet string_slice 98 > 94 (restart tool previews cut without slicing) Base red: acpmux panic ratchet (string_slice 98 > 94, indexing_slicing 703 > 693): rewrite the new hits without panics Oct 30, 2026
teamleaderleo and others added 2 commits October 30, 2026 12:04
…ts from #28154); keep the acpmux baseline at 693

hub/turns.rs (0a088aa, #28154): is_stopped_before_prompt and the
stopped answer read with get, not index. A missing reason compares like Null
did, and stopReason is always present in stopped_answer().

The acpmux baseline goes back to base's 693 / 144. acpmux hits landed three
times while this PR was open, and a baseline pinned to today's exact count
would turn base red the next time one lands before this PR merges.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Agent-Session: https://cmux.com/deeplink/agent?id=aid-d9e7e9f7fb2b87a57d5f
@teamleaderleo

teamleaderleo commented Oct 30, 2026 •

Copy link
Copy Markdown
Collaborator Author

Review (the owner lane's independent review, at its land request): SHIP at 365994d

Recorded by lane-gh-broker from this PR's owner lane, which asked to land this exact head after its independent review. next-autoland lands it once every red is the base's; a BLOCK at this head, a HOLD line or a hold label still stops it.

…lippy unused_imports)

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Agent-Session: https://cmux.com/deeplink/agent?id=aid-d9e7e9f7fb2b87a57d5f
@teamleaderleo

teamleaderleo commented Oct 30, 2026 •

Copy link
Copy Markdown
Collaborator Author

Review (the owner lane's independent review, at its land request): SHIP at 339e742

Recorded by lane-gh-broker from this PR's owner lane, which asked to land this exact head after its independent review. next-autoland lands it once every red is the base's; a BLOCK at this head, a HOLD line or a hold label still stops it.

@teamleaderleo

Copy link
Copy Markdown
Collaborator Author

Red checks at 339e742, classified:

  • clippy (changed cmux-tui packages): green now. At 365994d this PR caused it (3 unused JsonSet imports), and 339e742 fixed it.
  • protocol contract / inventory › "Check public resource API boundary": base red. All 5 diagnostics are in crates/cmux-tui/src/cli.rs:767-771 (list-surfaces, new-surface and the others). The cmux-tui crate in this PR is byte-identical to origin/feat-cmux-next, and running the script locally on this head gives the same 5 lines.
  • cmux-next checks (String tables) and release-notary: base reds this PR does not touch (no Swift, l10n or release-notes changes).

@teamleaderleo
teamleaderleo merged commit 15d370b into feat-cmux-next Oct 30, 2026
180 of 188 checks passed
@teamleaderleo
teamleaderleo deleted the lane/acpmux-string-slice-ratchet branch October 30, 2026 13:31
@github-actions

github-actions Bot commented Oct 30, 2026 •

Copy link
Copy Markdown
Contributor

teamleaderleo added a commit that referenced this pull request Oct 30, 2026
…ing_slice 97 > 94): rewrite the new hits without panics (#28386)

* acpmux: inbox, send and outbox status JSON writes without indexing

Part of the acpmux panic ratchet base red that came back after #28134
(indexing_slicing 703 > 693, string_slice 97 > 94). These writes go through
JsonSet::set instead of IndexMut.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Agent-Session: https://cmux.com/deeplink/agent?id=aid-d9e7e9f7fb2b87a57d5f

* acpmux: ls globs, dotted fields, inbox threads and match counts without indexing (10 hits from #28012, #27686, #27791, #27970)

- cli/select.rs (911f25f, #28012): field() cuts the dotted path with
  str::get; glob() compares and tails with slice get. Every index was in
  bounds, so the output is unchanged.
- hub/inbox.rs (cbe8e60, #27686): first_pr reads the number after the
  marker with str::get. inbox_unsend matches on (index, noticed, notice is
  none), with the same three outcomes in the same order.
- hub/mod.rs (fe3152a, #27791): local_match_counts bumps its slot with
  get_mut.
- hub/queue.rs (46ed428, #27970): the transfer id is read with get.

Tests add multibyte names, keys and PR markers.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Agent-Session: https://cmux.com/deeplink/agent?id=aid-d9e7e9f7fb2b87a57d5f

* acpmux: ACP session/list paging without indexing (5 indexing_slicing hits from #28161)

#28161 (4543835) landed while this PR was in review:
- hub/session_pages.rs: byte_page's sort key reads each key with get. Every
  index comes from enumerate, so the order is unchanged.
- server/list_reply.rs: sessions and updatedAt are read with get (a missing
  value is Null, as before), and nextCursor is written via JsonSet::set.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Agent-Session: https://cmux.com/deeplink/agent?id=aid-d9e7e9f7fb2b87a57d5f

* panic ratchet: lower acpmux to the measured counts (indexing_slicing 731 -> 686, string_slice 97 -> 94, unwrap_used 144 -> 143)

#28169 raised the acpmux baseline to the counts base had then, so the hits
fixed here could come back unnoticed. Measured at fdb6218 (fast-checks run
41080915558). PR CI now runs the ratchet too (#28169), so a PR that adds a
hit is caught before it lands.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Agent-Session: https://cmux.com/deeplink/agent?id=aid-d9e7e9f7fb2b87a57d5f

* acpmux: read ls row tags with get (the hit #28369 added to the panic ratchet)

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Agent-Session: https://cmux.com/deeplink/agent?id=aid-d9e7e9f7fb2b87a57d5f

---------

Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>

This branch was previously deployed

3 inactive deployments
artifacts — 339e7420 Deployed Oct 30, 2026 by teamleaderleo via add the Windows daemon to the tree #83073
sentry-dsyms — 339e7420 Deployed Oct 30, 2026 by teamleaderleo via upload cmux-tui dSYMs to Sentry #83073
ci-routing-agent — 339e7420 Deployed Oct 30, 2026 by teamleaderleo via cmux-tui tree preflight #83073
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

risk:normal Risk tier (docs/ci/risk-tiers.md)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant