Skip to content

base red: panic ratchet: cmux-rd-link's 7 and cmux-tui's 5 new index/slice hits - #28958

Closed
teamleaderleo wants to merge 6 commits into
feat-cmux-nextfrom
lane/panic-ratchet-rd-link-tui
Closed

teamleaderleo wants to merge 6 commits into
feat-cmux-nextfrom
lane/panic-ratchet-rd-link-tui

Conversation

@teamleaderleo

@teamleaderleo teamleaderleo commented Oct 31, 2026 •

Copy link
Copy Markdown
Collaborator

HOLD: superseded by #29024 (merged as 0d38b5c): it made the same panic-ratchet fixes, so this PR's diff against feat-cmux-next is now empty (head tree == base tree). Nothing left to land.

Base red fix (priority zero): the Rust panic ratchet is RED on feat-cmux-next at 3465552:

  • cmux-rd-link: clippy::indexing_slicing 7 > new crate (must be 0) (3b9cf69, 6d04ce1, ae8cd21)
  • cmux-tui: clippy::indexing_slicing 835 > baseline 831 (49394d0 group create, cf3d7a3 chief acp/pipe, d13d17a apps info)

It blocks area PRs such as #28757. #28808 and #28847 fixed the cmux-tui-core and apps hits; these are the rest.

Each hit is rewritten without a panic (get/get_mut, truncate, pushes into a Vec). The baseline is not raised.

  • replay.rs: the slot word is always < WORDS, so get never misses.
  • seal / write_handshake: n is at most the buffer handed to snow.
  • cmux-tui: an empty-slice default replaces what would have panicked.

risk:low (no behaviour change).

Observed / Not run

Changelog

none

Agent session: agent session

🤖 Generated with Claude Code

…slice hits

cmux-rd-link (new crate, must be 0) had 7 indexing_slicing hits from its
secure-link and replay-window commits (3b9cf69, 6d04ce1, ae8cd21);
cmux-tui went 831 -> 835 from 49394d0 (group create), cf3d7a3 (chief
acp, pipe) and d13d17a (apps info). Rewritten with get/get_mut,
truncate and pushes; the baseline is not raised. Behaviour is the same:
the replay slot index is always < WORDS, n never exceeds the buffer the
noise call was given, and the empty-slice defaults are the cases the old
code would have panicked on.

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 31, 2026 •

Copy link
Copy Markdown
Contributor

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

Review 03e24e5: 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=pr28958 -f ref=lane/panic-ratchet-rd-link-tui -f platform=macos -f backend_mode=local

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

blacksmith-sh Bot commented Oct 31, 2026 •

Copy link
Copy Markdown

Found 3 test failures on Blacksmith runners:

Failures

Test View Logs
test_a_picked_signing_label_has_a_rerun_alternative (main.RunsOnFallbackTests.test_
a_picked_signing_label_has_a_rerun_alternative)/
test_a_picked_signing_label_has_a_rerun_alternative (main.RunsOnFallbackTests.test_
a_picked_signing_label_has_a_rerun_alternative)
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_no_runs_on_chain_ends_on_the_reserved_side_label (main.RunsOnFallbackTests.tes
t_no_runs_on_chain_ends_on_the_reserved_side_label)/
test_no_runs_on_chain_ends_on_the_reserved_side_label (main.RunsOnFallbackTests.tes
t_no_runs_on_chain_ends_on_the_reserved_side_label)
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

Review (codex gpt-6-astra high fast, via reach review-router, diff-only prompt): SHIP at 96299c6
Follow-ups: none

Ratchet at this head: RATCHET_RC=0 (cmux-ci run https://github.com/manaflow-ai/cmux/actions/runs/41172450698).

teamleaderleo and others added 2 commits October 31, 2026 05:45
e4ef410 (#28170, peer waits say whether they settled) indexed serde
Values in host_peering's settle, Hub::wait_for_peer and with_wait.
Reads go through pointer/get with Null as the missing value (the same
value Index gave), writes through JsonSet::set. The baseline is not
raised.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Agent-Session: https://cmux.com/deeplink/agent?id=aid-d9e7e9f7fb2b87a57d5f
…pe has tracing's Value trait)

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Agent-Session: https://cmux.com/deeplink/agent?id=aid-d9e7e9f7fb2b87a57d5f
lawrencecchen referenced this pull request Oct 31, 2026
…aled datagrams, reliable control)

A pure crate (no I/O, injected clock) for Mac Screen on cmux.rd/1
(plans/cmux-next/remote-desktop-mac-host.md section 7):
- Noise_XX_25519_ChaChaPoly_BLAKE2s with the hub session id as prologue. The link
  accepts only the remote session key the caller expects (the device hub vouches for it:
  each side signs its session key with its device key, checked before the link is built).
  The initiator checks the responder's key on message 2, before it reveals its own.
- Sealed packets carry an explicit 64-bit nonce (stateless transport), so loss, reorder and
  duplicates on any path are fine; a 2048-nonce sliding window drops replays.
- Inside the seal: rd datagrams (best effort), an ordered reliable control channel (window
  128, RTO with backoff; hello, start and stats leave the TCP socket so the session works
  through routers and relays), path probes and replies, and close.
- Handshake retries every 250 ms and gives up after 10 s.
- Packet tags 0xD0..0xD7 sit outside every RFC 7983 range, so one UDP socket also carries
  STUN and TURN.

Tests (behavior at the crate boundary, two links over an in-memory network): handshake,
control and datagrams across 20 % loss with duplication and reordering; a peer the hub did
not vouch for is refused; another session's handshake does not complete; tampered and
replayed packets open nothing; an unanswered handshake times out. rbx (lane mac-screen-rd):
cargo fmt, clippy --locked -D warnings, 5 tests passed.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@teamleaderleo

Copy link
Copy Markdown
Collaborator Author

Review (codex gpt-6-astra high fast, via reach review-router, diff-only prompt): SHIP at 5033a36
Follow-ups: none

Round 2 (SHIP at e9303c1) covered the acpmux rewrites of #28170's 8 hits; round 3 covers the compile fix (serde_json::Value inside tracing::warn!).
Workspace clippy and the panic ratchet at this head: WS_CLIPPY_RC=0, RATCHET_RC=0 (https://github.com/manaflow-ai/cmux/actions/runs/41177361809).

lawrencecchen added a commit that referenced this pull request Oct 31, 2026
The panic ratchet is also red on two more rows on feat-cmux-next:
acpmux indexing_slicing 692 > baseline 686 (from #28170) and cmux-tui
836 > baseline 831. Each hit is rewritten with checked access (pointer/get,
JsonSet::set, get(n..).unwrap_or_default()); no behavior change: an empty
slice or a null stands where the old code would have panicked. The baseline
is not raised.

These are Leo's hunks from #28958 (e9303c1, 5033a36, 96299c6),
including his compile fix for the Value name inside tracing::warn!.

Co-authored-by: Leo Li <cheerleaderleo@outlook.com>
@lawrencecchen

Copy link
Copy Markdown
Contributor

Note from the base-red lane: #29024 now fixes all three panic-ratchet rows on feat-cmux-next: cmux-rd-link indexing_slicing 7 > 0, acpmux 692 > 686 (#28170) and cmux-tui 836 > 831. It uses your hunks from this PR (credited with Co-authored-by), including your serde_json::Value compile fix in hub/peers.rs. It adds one more cmux-tui hit that came after your count: args[i] in take_target in cli/automation.rs (from 7b3b9f7, cmux automation --target cloud), so the cmux-tui count is now 836, not 835.

You can close this PR or keep it. Whichever PR merges first, the other one becomes a no-op. If this one merges first, #29024 will be closed with a comment.

…::* (unused-imports)

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

Copy link
Copy Markdown
Collaborator Author

Review (codex gpt-6-astra high fast, via reach review-router, delta-only per acpmux-owner): SHIP at dc34d45
Follow-ups: none

Delta over r3 (SHIP at 5033a36): removes the unused cmux_json::JsonSet import in acpmux server/requests.rs that clippy --all-targets flagged.
At this head: clippy --all-targets -p acpmux -p cmux-rd-link -p cmux-tui -D warnings RC=0; workspace clippy RC=0; panic ratchet RC=0 (https://github.com/manaflow-ai/cmux/actions/runs/41180193826).

lawrencecchen added a commit that referenced this pull request Oct 31, 2026
…w index/slice hits (#29024)

* base red: panic ratchet: cmux-rd-link's 7 index/slice hits

The Rust panic ratchet is red on feat-cmux-next since the cmux-rd-link crate
landed by direct push (3b9cf69, 6d04ce1, ae8cd21): a new crate
has a baseline of 0, and it had 7 clippy::indexing_slicing hits.

- lib.rs write_handshake: truncate the buffer to the length Noise wrote
  instead of slicing it.
- lib.rs seal: build the packet with push/extend/resize and hand Noise the
  body through get_mut, not three index writes.
- replay.rs: the window word is read and written through get/get_mut; slot()
  always gives a word < WORDS, so the miss branch never runs.

No behavior change. The baseline is not raised and no allow is added.
These are the same hunks as #28958 (cmux-rd-link part), so that PR merges
over this one with no conflict.

Co-authored-by: Leo Li <cheerleaderleo@outlook.com>

* base red: panic ratchet: acpmux's and cmux-tui's new index/slice hits

The panic ratchet is also red on two more rows on feat-cmux-next:
acpmux indexing_slicing 692 > baseline 686 (from #28170) and cmux-tui
836 > baseline 831. Each hit is rewritten with checked access (pointer/get,
JsonSet::set, get(n..).unwrap_or_default()); no behavior change: an empty
slice or a null stands where the old code would have panicked. The baseline
is not raised.

These are Leo's hunks from #28958 (e9303c1, 5033a36, 96299c6),
including his compile fix for the Value name inside tracing::warn!.

Co-authored-by: Leo Li <cheerleaderleo@outlook.com>

* base red: panic ratchet: cmux automation's take_target walks the words with an iterator (cmux-tui 836 > 831)

7b3b9f7 (cmux automation --target cloud) added args[i] in take_target,
the sixth new cmux-tui indexing_slicing hit. The loop now takes the words
from an iterator and the value after --target from the same iterator; the
words, the target and the missing-value usage error are the same. A unit test
pins both flag forms, the default, a missing value and a bad target.

* acpmux: drop the JsonSet import requests.rs already has through super::* (unused-imports)

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

---------

Co-authored-by: Leo Li <cheerleaderleo@outlook.com>
Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
@teamleaderleo

Copy link
Copy Markdown
Collaborator Author

Red classification at dc34d45: cmux-tui artifacts / build-windows / build x86_64-pc-windows-gnu is a base red, not this PR's.

The errors are all in cmux-automations:

  • std::os::unix and std::os::fd imports in exec.rs and store.rs
  • libc::localtime_r in clock.rs

That crate is unix-only code from ee37179, and this PR changes no file in it. The Windows build ran here only because the PR touches cmux-tui.

#29075 ("base red: cmux-tui builds for Windows again (cmux-automations unix-only)") is the open fix. release-notary and Guard status are the known base guard reds.

@cmux-bot
cmux-bot Bot temporarily deployed to ci-routing-agent October 31, 2026 10:22 Inactive
@github-actions github-actions Bot added risk:low Risk tier (docs/ci/risk-tiers.md) and removed risk:normal Risk tier (docs/ci/risk-tiers.md) labels Oct 31, 2026
@cmux-bot
cmux-bot Bot temporarily deployed to ci-routing-agent October 31, 2026 10:27 Inactive
@teamleaderleo

teamleaderleo commented Oct 31, 2026 •

Copy link
Copy Markdown
Collaborator Author

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

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

Superseded by #29024 (0d38b5c), which landed the same panic-ratchet fixes. At head 03e24e5 this PR's tree equals the feat-cmux-next tree it merged (a93c5a13), so it would land an empty change. Holding it; it will be closed.

@teamleaderleo

Copy link
Copy Markdown
Collaborator Author

Owned by an agent session · session

@teamleaderleo

Copy link
Copy Markdown
Collaborator Author

Superseded by #29024, which landed the same panic-ratchet fixes (merged as 0d38b5c). This PR's diff against feat-cmux-next is empty, so there is nothing left to land.

This branch was previously deployed

3 inactive (2 outdated) deployments
ci-routing-agent — 03e24e58 Deployed Oct 31, 2026 by cmux-bot[bot] via wait for same-key tree publisher #89186
artifacts — dc34d45b Deployed Oct 31, 2026 by teamleaderleo via publish tree-addressed cmux-tui #88148
sentry-dsyms — dc34d45b Deployed Oct 31, 2026 by teamleaderleo via upload cmux-tui dSYMs to Sentry #88148
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants