Skip to content

cmux-tui tests: share chat's one action.run is checked on the path the CLI takes (base red from #30550) - #32065

Merged
lawrencecchen merged 1 commit into
feat-cmux-nextfrom
base-red-share-chat
Nov 2, 2026
Merged

lawrencecchen merged 1 commit into
feat-cmux-nextfrom
base-red-share-chat

Conversation

@lawrencecchen

@lawrencecchen lawrencecchen commented Nov 2, 2026 •

Copy link
Copy Markdown
Contributor

Summary

Base red (priority zero). On feat-cmux-next, cmux-tui cli::app::tests::share_chat_waits_for_the_owners_answer (src/cli/app/tests.rs:359) fails with "expected one action.run call". Base job b973957eb8a382f63c3d57c0.

Culprit: ff01dcc (#30550), the commit that added the test. No later commit changed the share chat path: git log ff01dcc58da..origin/feat-cmux-next shows no commit to cli.rs, cli/app.rs, cli/app/ or cli/share_cli.rs. The test asked app::parse for share chat --surface 1, but share is not and never was in APP_SCOPES, so parse answers Ok(None). The test was red from the commit that added it (fleet run at ff01dcc below).

The product is right, so this fixes the test, not the product. cmux share chat is not a mux command, so cli.rs run_app_action_fallback runs it. share_cli::action leaves it as typed (it rewrites only share revoke|kick), and app::run_cli_action sends one action.run for share chat. That run waits PERSON_RUN_TIMEOUT, because run_action takes waiting_run_timeout("share chat"). The test now takes that same path. It checks that parse answers None, that share_cli::action answers None, and that run_action("share chat", ["--surface", "1"], Cli) is one action.run with args.surface = "1" and the person deadline. The waiting_run_timeout assertions stay as they were.

FYI @teamleaderleo: this is your #30550 test.

Testing

cmux-ci run --class isolated --os linux --script scripts/ci/cmux-tui-rust-check.sh --ref SHA --arg test --arg share_chat_waits_for_the_owners_answer:

  • Red at the base 13fd3f48bcb0: FAIL share_chat_waits_for_the_owners_answer, "expected one action.run call". Step 66f5715c3c0e7dd8111fc935.
  • Red at the culprit ff01dcc58da (the commit that added the test): the same FAIL and the same message. Step 010e0c9a4036dd5e5fe2845b (Blacksmith run manaflow-ai/cmuxterm-hq 41421315070). So no later commit broke it.
  • Green at the head a6dbfc45352, 5 of 5: steps 4f5a3a7d9b98c40cd2ff24be, 5ee2d80352be204faf7a7a35, a871d0e5b3593cf69b304a5d, 71275c72030cee25ef3ec2e9, c62b5b8c0a7e879719525869 (each 1 passed).
  • Panic ratchet (--arg panic) at the head: pass, step 7fd6769aa0f14554aeed7d25.

Non-green checks at the head are base findings, not caused by this PR. clippy (changed cmux-tui packages): too_many_arguments on cmux-tui-core snapshot_written, from #31931; fixed by #32092 (merged). defaults-snapshot (cmux-next guards) is a base finding. Its failing step is "Menu rows run registered actions" on AgentPaneSessionInfoContent.swift, from #31930. The same check is red on unrelated open PRs, and #32072 and #32073 fix it.

Observed / Not run

  • Observed: the fleet step logs above (nextest PASS/FAIL lines); python3 scripts/cmux-next/prepush-gate.py passes at the head.
  • Not run: no local build or test on this VM (fleet only). The CLI was not run end to end against a live app (cmux share chat with an app socket); the claim that the product sends one action.run comes from reading cli.rs and cli/app.rs, not from a live run.

Changelog

none

…e CLI takes (base red from #30550)

share_chat_waits_for_the_owners_answer (ff01dcc, #30550) asked
app::parse for `share chat --surface 1`, but `share` was never an app
scope (APP_SCOPES), so parse answers None and the test failed
"expected one action.run call" from the commit that added it; no later
commit changed cli.rs, cli/app.rs, cli/app/ or cli/share_cli.rs.

The product is right: `cmux share chat` is not a mux command, so
cli.rs's run_app_action_fallback runs it; share_cli::action leaves it as
typed (it rewrites only share revoke|kick), and run_cli_action sends one
action.run "share chat" with PERSON_RUN_TIMEOUT. The test now takes that
path: parse answers None, share_cli::action answers None, and
run_action("share chat", ["--surface", "1"], Cli) is one action.run with
the surface argument and the person deadline.
@lawrencecchen lawrencecchen added the risk:low Risk tier (docs/ci/risk-tiers.md) label Nov 2, 2026
@github-actions

github-actions Bot commented Nov 2, 2026 •

Copy link
Copy Markdown
Contributor

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

Review a6dbfc4: 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=pr32065 -f ref=base-red-share-chat -f platform=macos -f backend_mode=local

Shipped in:

@lawrencecchen

Copy link
Copy Markdown
Contributor Author

Review (subagent, correctness first): SHIP at a6dbfc4

Read-only review. Nothing compiled or run on this machine.

What I checked:

  • Product path for cmux share chat --surface 1 (cli.rs run): none of the run_if_requested handlers or remote_host::handles (needs host first) takes share. run_app_scope gets Ok(None) from app::parse because share is not in APP_SCOPES. The mux grammar (parse_command, shorthand, command::parse) does not know share, so it errors and run_app_action_fallback runs. There, agent_view::action returns None (needs agent view). share_cli::action returns None for any verb except revoke and kick. --surface is not a global flag, so it stays in command_args. The fallback then joins the words before the first flag (share chat). new_window_spelling does not match. run_cli_action calls run_action(name, rest, ActionName::Cli) one time, which builds one action.run with wait: true, cli: true and args: {"surface": "1"}. Its timeout is waiting_run_timeout("share chat"), which is PERSON_RUN_TIMEOUT (210 s). call adds one idempotency key and sends that one request. Busy retries happen only when the app says the run never started, and they use the same key. Result: exactly one action.run "share chat" with the person timeout. The product is correct. Only the old test was wrong.
  • The new test follows the same steps: parse returns Ok(None), share_cli::action returns None, then run_action("share chat", &words[2..], ActionName::Cli). It also checks method, action, args and timeout. It fails if share chat leaves PERSON_ACTIONS.
  • Compile, by reading: use super::* brings in parse, run_action, ActionName, AppCommand, PERSON_RUN_TIMEOUT, WAITING_RUN_TIMEOUT, waiting_run_timeout and json! (bare json! is already used at tests.rs:47). super::super::share_cli::action is pub(super) in a private child module of cli, so cli::app::tests can see it. Its return type Option<Result<(String, Vec<String>), String>> has PartialEq and Debug for assert_eq!(.., None). &words[2..] is &[String], which matches run_action. Value has Display for "{params}".
  • History: git log -S' "share",' -- cli/app.rs finds no commit, so share was never in APP_SCOPES. At ff01dcc, APP_SCOPES is the same list as on head. git log ff01dcc58da..HEAD on cli.rs, cli/app.rs, cli/app/run.rs, cli/share_cli.rs, cli/app/tests.rs and cli/app/wait.rs shows only this PR's commit. The claim is true: the test was red when cmux NEXT: a guest link an agent or script asks for waits for the owner's Allow (#30263) #30550 added it and nothing broke it later.

Non-blocking gaps:

  • The test copies the fallback's steps by hand. It does not prove that the mux grammar still rejects share (if a mux share scope is added, the fallback stops running) or that the word split before the first flag gives share chat. run_app_action_fallback needs a socket, so a unit test cannot call it directly. This is acceptable for a base-red repair.

@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 2, 2026
@lawrencecchen

lawrencecchen commented Nov 2, 2026 •

Copy link
Copy Markdown
Contributor Author

Non-green checks at a6dbfc4, classified:

@lawrencecchen
lawrencecchen merged commit ab738a2 into feat-cmux-next Nov 2, 2026
292 of 316 checks passed
@lawrencecchen
lawrencecchen deleted the base-red-share-chat branch November 2, 2026 10:13
lawrencecchen added a commit that referenced this pull request Nov 2, 2026
…est drops a path qualification (clippy base reds)

Two cmux-tui clippy errors on feat-cmux-next (seen on #31921, hq run 41428596319):
- clippy::too_many_arguments (8/7) on link::serve, from #28950, which added the
  native broker as an eighth argument. The broker and the credential relay
  socket are the two ways a serve resolves cloud hosts, and run_serve already
  refuses both at once, so they become one CloudAccess enum
  (Broker(Bound) | Relay(Option<PathBuf>)). The branches are the same as before:
  a broker with an identity is native and gives the config, key and route;
  otherwise the relay socket (or none: unavailable) serves cloud hosts.
- unused_qualifications at cli/app/tests.rs:352, from #32065: Path is in scope
  through super::*.
@lawrencecchen

Copy link
Copy Markdown
Contributor Author

This PR left a cmux-tui clippy red on feat-cmux-next: unused_qualifications at cmux-tui/crates/cmux-tui/src/cli/app/tests.rs:352 (std::path::Path::new). Fixed forward in #32128.

cmux-bot Bot pushed a commit that referenced this pull request Nov 2, 2026
…hecked slices, JsonSet, one CloudAccess) (#32128)

* fix(cmux-tui): link broker ID checks use strip_prefix and a slice pattern, not index or string slices (panic ratchet)

Base red at 8ea6e45 from #28950: the Rust panic ratchet found cmux-tui
clippy::indexing_slicing 835 > baseline 831 (+4) and clippy::string_slice
84 > baseline 82 (+2). The hits are in #28950's production code:
- cloud_broker.rs mint_token: services[0] == services[1] (2 indexing);
- native_broker.rs read_identity: bootstrap.team[5..] (1 indexing, 1 string slice);
- native_broker.rs native_id: value[prefix.len()..] (1 indexing, 1 string slice).

Each check keeps its result: matches!(services, [first, second] if first == second)
is true for the same inputs as len() == 2 && [0] == [1]; strip_prefix plus a
length check on the rest accepts the same strings as starts_with, the total
length and the slice after the prefix. Invalid input still fails closed
(Forbidden, refused()).

* fix(cmux-tui): link-ready line sets its native fields with JsonSet, not Value index writes (panic ratchet)

The panic step at f8a56d7 still found cmux-tui clippy::indexing_slicing
833 > 831: the other 2 hits from #28950 are the serde_json IndexMut writes
ready_line["native"] and ready_line["wg_public_key"] in link/mod.rs.
cmux_json::JsonSet::set does the same insert on the object without a
panicking index (the repo's form for string-key JSON writes).

* fix(cmux-tui): link serve takes one CloudAccess, and the share-chat test drops a path qualification (clippy base reds)

Two cmux-tui clippy errors on feat-cmux-next (seen on #31921, hq run 41428596319):
- clippy::too_many_arguments (8/7) on link::serve, from #28950, which added the
  native broker as an eighth argument. The broker and the credential relay
  socket are the two ways a serve resolves cloud hosts, and run_serve already
  refuses both at once, so they become one CloudAccess enum
  (Broker(Bound) | Relay(Option<PathBuf>)). The branches are the same as before:
  a broker with an identity is native and gives the config, key and route;
  otherwise the relay socket (or none: unavailable) serves cloud hosts.
- unused_qualifications at cli/app/tests.rs:352, from #32065: Path is in scope
  through super::*.

* fix(cmux-tui): CloudAccess boxes its broker variant (clippy large_enum_variant)

The Bound carries the native identity (296 bytes) and the relay variant
24 bytes; boxing the broker keeps the enum small. One allocation per serve.

This branch was previously deployed

3 inactive deployments
ci-routing-agent — a6dbfc45 Deployed Nov 2, 2026 by github-actions[bot] via cmux-next macOS placement #62280
artifacts — a6dbfc45 Deployed Nov 2, 2026 by lawrencecchen via publish tree-addressed cmux-tui #100460
sentry-dsyms — a6dbfc45 Deployed Nov 2, 2026 by lawrencecchen via upload cmux-tui dSYMs to Sentry #100460
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:low Risk tier (docs/ci/risk-tiers.md)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant