Skip to content

base red: panic ratchet, #30921's and #30923's index reads use checked forms (cmux-tui 855 > 831) - #31097

Merged
lawrencecchen merged 2 commits into
feat-cmux-nextfrom
fix/base-red-panic-cmux-tui-855
Nov 1, 2026
Merged

lawrencecchen merged 2 commits into
feat-cmux-nextfrom
fix/base-red-panic-cmux-tui-855

Conversation

@lawrencecchen

@lawrencecchen lawrencecchen commented Nov 1, 2026 •

Copy link
Copy Markdown
Contributor

Fixes #30921
Fixes #30923

The panic ratchet on feat-cmux-next is red since about 14:47Z: cmux-tui: clippy::indexing_slicing 855 > baseline 831 (tip watcher job af1347a18047071fb78a1b9b at db2332c, hq run 41321021625). The new hits come from two direct merges after #30966's base (41d5197, 831):

Each read now uses a checked form. Valid input behaves the same. A missing or non-object field reads as null, the same as serde_json's Index returned:

  • video_tools.rs: the app's answer and error fields go through Value::get (the error code through one error_code helper). The params writes go through cmux_json::JsonSet::set; the params are always an object, so nothing changes.
  • seed_pull.rs export: the job and status fields go through Value::get. Staging::copy takes its chunk through buffer.get(..read), which returns an error if a read reports more bytes than its buffer holds.
  • seed_cli/pull.rs keep: summary.base.{head,index,work} through Value::get.

There are no allow attributes and no baseline change.

Hit table

This table compares the full clippy JSON at this PR's base 1f80a9f (a measurement-only commit lifts the cmux-tui baseline so that the step returns its JSON; that ref was deleted after the run) with the JSON at this head. Exactly 24 hits are gone and none are added. git blame at 1f80a9f names the culprit on every line.

file hits removed culprit
cli/mcp/video_tools.rs (404, 407, 412, 413, 435, 459, 467, 468, 481, 517, 530) 11 #30921 af670df
link/sync_cli/seed_pull.rs (375, 379, 387, 389, 390, 395, 401, 412, 413, 414, 753, 754) 12 #30923 b91b9c7
link/seed_cli/pull.rs (205) 1 #30923 b91b9c7
total 24 (855 to 831)

Clippy did not count the other reads in these files (describe, info_value, the per-frame closures). They were changed to the same checked form so that each function uses one form.

Evidence (fleet, Linux route, scripts/ci/cmux-tui-rust-check.sh)

check ref job result
panic tip db2332c (tip watcher) af1347a18047071fb78a1b9b red, 855 > 831
panic 41d5197 (before #30966) 445bcd7a45940c8b93c8e601 red, 835 > 831 (#30966's 4 hits not fixed yet); no JSON returned
panic tip 762fda7 e74d9073071178e6cbfd1a0b red, 855 > 831
panic head c7de86d 8b943a06e67c8ac1092dcae7 green, cmux-tui indexing_slicing 831
clippy -D warnings head a285f14f8ea5e7d0d49f1d64 green
test video_tools head 01c813e2c05fb95374817930 green, 9 of 9
test pull (seed_pull_journey, seed_cli::pull, link_sync) head 09e24ade054d746c76430314 green, 27 of 27
test bring_back (bring_back_journey, seed_pull_journey) head 0736b1af55ea0d332f909749 green, 4 of 4
panic, hit list 1f80a9f + lifted baseline (measurement) a599ea4075e85735b64efd54 JSON for the hit table

Observed / Not run

Observed: the tip watcher's panic step is red at db2332c (855 > 831). scripts/cmux-next/prepush-gate.py passed on the head. rustfmt --edition 2024 is clean on the three files. Every fleet job in the table above ran on the Linux route, and I read each job's log. An independent review subagent passed the head: Review (subagent, correctness first): SHIP at c7de86da361040eedd91b7842ad453ef727a05a4.

Not run: no local cargo build (the fleet runs it). No macOS run: the panic baseline holds Linux counts, and the changed code has no macOS-only path.

Changelog

none

…d forms

#30921 (af670df, cmux mcp video_frames/video_info) and #30923
(b91b9c7, cmux link seed pull) added clippy::indexing_slicing hits in
cmux-tui production code, a deny lint of the panic ratchet: cmux-tui went
from 831 to 855. Each read now uses a checked form; valid input behaves the
same, and a missing or non-object field reads as it did through serde_json's
Index (null):

- video_tools: the app's answer and error fields through Value::get; the
  params writes through cmux_json::JsonSet::set (the params are always an
  object, as before).
- seed_pull export: the job and status fields through Value::get; the
  bundle copy's chunk through buffer.get(..read), an error if a read ever
  claims more than its buffer.
- seed_cli pull keep: summary.base.{head,index,work} through Value::get.

No allow attributes, no baseline change.
@lawrencecchen lawrencecchen added the risk:normal Risk tier (docs/ci/risk-tiers.md) label Nov 1, 2026
@github-actions

github-actions Bot commented Nov 1, 2026 •

Copy link
Copy Markdown
Contributor

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

Review c7de86d: 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=pr31097 -f ref=fix/base-red-panic-cmux-tui-855 -f platform=macos -f backend_mode=local

Shipped in:

@lawrencecchen

Copy link
Copy Markdown
Contributor Author

Review (subagent, correctness first): SHIP at c7de86d

Every rewritten read matches serde_json's Index semantics: a missing or non-object field reads as Null or None, and the comparisons go through as_str and as_bool as Value's PartialEq does. The JsonSet::set targets are always objects from identity(). buffer.get(..read) cannot fail under the Read contract. The video_tools.rs hits that remain are all in #[cfg(test)]. The PR adds no allow and no baseline change.

@blacksmith-sh

blacksmith-sh Bot commented Nov 1, 2026

Copy link
Copy Markdown

Found 4 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

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

@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 1, 2026
@cursor

cursor Bot commented Nov 1, 2026

Copy link
Copy Markdown

Bugbot needs on-demand usage enabled

Bugbot uses usage-based billing for this team and requires on-demand usage to be enabled.

A team admin can enable on-demand usage in the Cursor dashboard.

@cmux-bot
cmux-bot Bot temporarily deployed to ci-routing-agent November 1, 2026 15:59 Inactive
@lawrencecchen
lawrencecchen merged commit c022cec into feat-cmux-next Nov 1, 2026
190 of 193 checks passed
@lawrencecchen
lawrencecchen deleted the fix/base-red-panic-cmux-tui-855 branch November 1, 2026 16:17

This branch was previously deployed

3 inactive deployments
artifacts — c752b1c1 Deployed Nov 1, 2026 by cmux-bot[bot] via add the Windows daemon to the tree #96807
ci-routing-agent — c752b1c1 Deployed Nov 1, 2026 by github-actions[bot] via cmux-next macOS placement #59434
sentry-dsyms — c752b1c1 Deployed Nov 1, 2026 by cmux-bot[bot] via upload cmux-tui dSYMs to Sentry #96807
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