Skip to content

base red: panic ratchet: remove the apps series' 14 indexing and 7 unwrap hits - #28847

Merged
lawrencecchen merged 2 commits into
feat-cmux-nextfrom
base-red/panic-ratchet-apps-deploy
Oct 31, 2026
Merged

lawrencecchen merged 2 commits into
feat-cmux-nextfrom
base-red/panic-ratchet-apps-deploy

Conversation

@lawrencecchen

Copy link
Copy Markdown
Contributor

What

A base red, priority zero. The Rust panic ratchet in cmux-tui spec / clippy (changed cmux-tui packages) is red again on feat-cmux-next after #28808. The apps series reached the tip at 03:15Z (9d1b1f9, 045f9e0, e62b402, 0012704, 8024f86, all direct pushes), after #28808's green run:

cmux-tui: clippy::indexing_slicing 841 > baseline 831
cmux-tui-core: clippy::unwrap_used 1042 > baseline 1035
cmux-tui-core: clippy::indexing_slicing 857 > baseline 853

Red: #28808's own ratchet run at ddd1753 on a merge ref that held the apps series (job 124426587561). Before the series, the same job passed on 9e0d6b0 (job 124422331085). I diffed the clippy JSON artifacts of the two runs, and these 21 new sites are the whole difference:

Crate, lint Sites Fix
cmux-tui indexing (10) cli/apps_list.rs: reads of an owned reply Value and scope, and args["query"] = ... get(..).and_then(Value::as_str/as_bool), which gives the same result for every input; JsonSet::set for the write
cmux-tui-core indexing (4) apps/deploy.rs manifest["deploy"]["server"|"targets"], apps/store.rs result["instance"] = ..., apps/supervisor.rs row["instance"] = ... JsonFields::field, JsonSet::set
cmux-tui-core unwrap (7) apps/deploy.rs .lock().unwrap() RecoverPoison, as the rest of the crate does

The baseline is not raised. Behavior does not change: a missing key was Null before and is None now, and both give the same default. The writes go to objects, where set does what IndexMut did.

Pass on the head: this PR's own clippy (changed cmux-tui packages) run (the result goes in a comment).

Changelog

none

Observed / Not run

  • Observed: the hit diff of the two CI clippy JSON artifacts. python3 scripts/cmux-next/prepush-gate.py: no new violations. rustfmt --check (pinned 1.95.0) on the 4 files: clean.
  • Fleet: cmux-tui-rust-check.sh clippy and the existing tests rows_join_install_state_and_filter_installed, parses_list_and_info and server_spec_refuses_shell_characters. The results go in a comment.
  • Not run: no Rust compile on this VM (the rules do not allow it).

lawrencecchen and others added 2 commits October 31, 2026 03:26
…c ratchet)

The panic ratchet is red on feat-cmux-next again after #28808: the apps
series reached the tip at 03:15Z (9d1b1f9, 045f9e0, e62b402,
0012704, 8024f86), after #28808's green run. New hits, found by
diffing the CI clippy JSON of #28808's runs before and after:

- cmux-tui indexing_slicing 841 > 831: cli/apps_list.rs reads of an
  owned reply Value (value["id"], ["ok"], ["data"], ["error_code"],
  ["error"], scope["scope"|"risk"|"optional"|"reason"]) -> get(...) with
  the same result for every input, and args["query"] = ... -> JsonSet::set
- cmux-tui-core indexing_slicing 857 > 853: apps/deploy.rs
  manifest["deploy"]["server"|"targets"] -> JsonFields::field; store.rs
  result["instance"] = ... and supervisor.rs row["instance"] = ... ->
  JsonSet::set
- cmux-tui-core unwrap_used 1042 > 1035: apps/deploy.rs seven
  .lock().unwrap() -> RecoverPoison, as the rest of the crate does

The baseline is not raised. Reads give the same values (a missing key
was Null before and is None now; both give the same default). The
writes are on objects, where set does what IndexMut did.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@lawrencecchen lawrencecchen added the risk:normal Risk tier (docs/ci/risk-tiers.md) label Oct 31, 2026
@lawrencecchen

lawrencecchen commented Oct 31, 2026 •

Copy link
Copy Markdown
Contributor Author

Review (Opus 5.5, review subagent): SHIP at 070aa32

I found no correctness defect. I read the diff origin/feat-cmux-next...070aa32affc (base db183fa, which is the current tip, so the branch is current) and the PR body. I did not compile anything locally.

CI evidence. The clippy (changed cmux-tui packages) job ran on this head (job 124428847374, head_sha 070aa32) and it passed. The log says Finished dev profile and rust-panic-ratchet: 50 crates checked, 17 at deny. There is no RED line, and no indexing_slicing or unwrap_used line is over its baseline. The other lines are warn-tier ##[warning]s that are also on the base (arithmetic_side_effects and cast_possible_truncation in acpmux, cmux-automations, cmux-tui, cmux-tui-core and cmux-wg), plus "counts went down in acpmux, cmux-app-host". None of them comes from this PR.

Same result for every input (I checked each one against serde_json's semantics):

  • cmux-tui/crates/cmux-tui/src/cli/apps_list.rs:130-137, :200-209: Value: PartialEq<bool> is as_bool().map_or(false, |b| b == other) and PartialEq<&str> is as_str().map_or(false, ..). So scope["optional"] == true is the same as get(..).and_then(as_bool) == Some(true), and value["id"] != "apps-list-1" is the same as get(..).and_then(as_str) != Some("apps-list-1"). Index<&str> gives Null for a missing key or a non-object, and get gives None there, so the unwrap_or defaults are the same. value["data"].clone() is the same as get("data").cloned().unwrap_or(Value::Null).
  • apps_list.rs:218, apps/store.rs:472, apps/supervisor.rs:376: JsonSet::set (cmux-json/src/lib.rs:50-56) turns Null into {} and inserts, which is what IndexMut does. For a non-null non-object, IndexMut panicked and set now does nothing. All three targets are objects in practice: json!({"limit":200}), the store_install result, and entry(..). set's bool result is not #[must_use].
  • apps/deploy.rs:122, :394-397: JsonFields::field is get(key).unwrap_or(&JSON_NULL) (cmux-tui-core/src/panic_free.rs:38-40), which is the same as the Index read. It still borrows as &Value.
  • apps/deploy.rs: seven .lock().unwrap() become .recover_poison(), which is unwrap_or_else(PoisonError::into_inner). The only change is on a poisoned lock: the code now goes on with the data instead of panicking again, as the rest of the crate does. deploy_remove still passes &MutexGuard to &BTreeMap by deref coercion.

Compiles in principle (CI also confirms it for --lib --bins): cmux-json is a dependency of both crates (cmux-tui/Cargo.toml:53, cmux-tui-core/Cargo.toml:14). crate::panic_free is declared at cmux-tui-core/src/lib.rs:63 and its traits are pub(crate). supervisor.rs already imports JsonSet as _ at :25. The as _ imports leave no unused names. JsonFields::insert does not clash with anything: the only .insert in deploy.rs (:520) is on MutexGuard<BTreeMap<..>>, and the inherent BTreeMap::insert resolves first after deref. No .insert is called on a Value in deploy.rs or in its use super::* tests.

Remaining hits from the apps series: none for the deny lints. The series still has Value[...] reads on &Value receivers: apps_list.rs:56-121 (catalog, listing, row, detail) and deploy.rs:126,133,138 (server). As the clippy JSON diff and this green run show, clippy's indexing_slicing does not count indexing through a reference receiver here, and an Index read on Value never panics. A follow-up can change them for consistency, but they do not hold this PR.

@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 Oct 31, 2026
lawrencecchen referenced this pull request Oct 31, 2026
Reads the App Store from the session daemon's supervisor (apps-store
catalog.list / catalog.get) with no Mac app, so a self-hosted Linux host lists
its apps too. list joins each listing with its install state (--installed,
--query, --json); info shows the scopes with their risk class.
lawrencecchen referenced this pull request Oct 31, 2026
…ud VM

Manifest v2 gains deploy {server: {kind: container, image, port, health},
tenancy, targets} and implements[cmux.pane/1].remote = instance; billing
{model} is free only (Lawrence 2026-10-30). The validator refuses a remote
pane without deploy and a local target for a third-party app; the schema
refuses shell characters in image and health.

cmux.apps.install takes deploy {target}: self-hosted:<ssh host> (the
customer's own machine over SSH or Tailscale SSH, no cmux Cloud call) or
cloud-vm:<machine> (cmux cloud exec). One POSIX runner for both: docker or
podman run bound to the tailnet address (else 127.0.0.1, never every
interface), wait for the health path, print the URL. The install commits and
answers with the instance deploying; a worker records running + URL, or rolls
the install back (container removed) with the reason in the app log. Remove
removes the container. Instances persist in <state>/apps-instances.json and
show in apps-list and installed.list.
@lawrencecchen

Copy link
Copy Markdown
Contributor Author

@lawrencecchen
lawrencecchen merged commit 6831eb6 into feat-cmux-next Oct 31, 2026
95 of 113 checks passed
@lawrencecchen
lawrencecchen deleted the base-red/panic-ratchet-apps-deploy branch October 31, 2026 03:40
@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 070aa32: 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=pr28847 -f ref=base-red/panic-ratchet-apps-deploy -f platform=macos -f backend_mode=local

red: cmux-next checks (god files, concurrency, crash safety, l10n); base red: the same failure on the base db183fa; does not block

Shipped in:

This branch was previously deployed

3 inactive deployments
artifacts — 070aa32a Deployed Oct 31, 2026 by lawrencecchen via publish tree-addressed cmux-tui #87106
sentry-dsyms — 070aa32a Deployed Oct 31, 2026 by lawrencecchen via upload cmux-tui dSYMs to Sentry #87106
ci-routing-agent — 070aa32a Deployed Oct 31, 2026 by lawrencecchen via cmux-tui tree preflight #87106
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