Skip to content

base red: acpmux's login-env watcher test module is Linux-only, so macOS has no unused import (clippy, after #33406) - #33690

Merged
lawrencecchen merged 3 commits into
feat-cmux-nextfrom
base-red/acpmux-watch-import-33406
Nov 3, 2026
Merged

lawrencecchen merged 3 commits into
feat-cmux-nextfrom
base-red/acpmux-watch-import-33406

Conversation

@lawrencecchen

@lawrencecchen lawrencecchen commented Nov 3, 2026 •

Copy link
Copy Markdown
Contributor

Fixes the macOS half of the clippy (changed cmux-tui packages) red from #33406 (3534e7b): error: unused import: super::* at acpmux/src/login_env/watch.rs:229 in the acpmux lib test build on macOS. The Linux half (unused import: notify::Watcher) was fixed by #33664 while this PR was open; this PR now has only the macOS fix.

Takes back #33406: only the test module's cfg changes; no code changes.

Cause

The test module's one test is Linux-only (#[cfg(target_os = "linux")]), so the macOS test build has an empty mod tests whose use super::*; is unused, and clippy -D warnings refuses it.

Fix

The test module gets #[cfg(all(test, target_os = "linux"))], and the cfg on its one test moves to the module. No test is lost on any OS.

Red at the base, green at the head

Command (fleet): cmux-ci run --class isolated --script scripts/ci/cmux-tui-rust-check.sh --arg clippy --os macos --ref SHA

  • Red at 3534e7b: step 77bd03ed79cbc2e7cc278152 (unused import: super::*). Green at the earlier head f4614c2: step 337633afdd8bfc8675e1a086.
  • At the current base and head: keys watch3-mac-1ac26c7853 and watch3-mac-675e27deda; results in a comment.

Changelog

none

Observed / Not run

Observed: the clippy steps above; rustfmt --check passed.
Not run: the Linux test itself (its cfg only moved; CI runs it).

…re it is used (clippy unused import on Linux, after #33406)

#33406 (3534e7b) made Linux watch through inotify, so the only
Watcher::watch call is in the non-Linux source(). The file-level
'use notify::Watcher as _;' is unused on Linux, and clippy -D warnings
refuses it there: 'clippy (changed cmux-tui packages)' is red for every
acpmux change. The import now has the same cfg as its one use.
…its import is used (clippy unused super::* on macOS)

With the module's one test Linux-only, the macOS test build kept an empty
'mod tests' whose 'use super::*' is unused: clippy -D warnings fails the
acpmux lib test build on macOS. The cfg moves from the test to the module.
@lawrencecchen lawrencecchen added the risk:normal Risk tier (docs/ci/risk-tiers.md) label Nov 3, 2026
@github-actions

github-actions Bot commented Nov 3, 2026 •

Copy link
Copy Markdown
Contributor

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

Review 675e27d: 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=pr33690 -f ref=base-red/acpmux-watch-import-33406 -f platform=macos -f backend_mode=local

web and size checks: green

Shipped in:

@github-actions github-actions Bot added risk:high Risk tier (docs/ci/risk-tiers.md) and removed risk:normal Risk tier (docs/ci/risk-tiers.md) labels Nov 3, 2026
@lawrencecchen

Copy link
Copy Markdown
Contributor Author

Review (independent subagent): SHIP at f4614c2

Read the PR body, the diff and cmux-tui/crates/acpmux/src/login_env/watch.rs at the head. No build or test run by this review.

  1. Import. The only Watcher trait method call in the file is watcher.watch(dir, NonRecursive) at line 72, inside the #[cfg(not(target_os = "linux"))] fn source. There is no .unwatch( or .configure(. The Linux path calls quiet::watch, a free function, not the trait. So on Linux the import is unused and the new #[cfg(not(target_os = "linux"))] removes it. On macOS and Windows (both not(linux)), the import and the non-Linux source compile as before. mod watch; in login_env.rs has no cfg of its own.
  2. Test module. At the base the module held one item, the Linux-only test a_read_in_a_watched_folder_sends_nothing_and_a_write_arrives, plus use super::*;. The head moves the cfg to the module (all(test, target_os = "linux")). No test is lost on any OS: the test ran only on Linux before and still runs only there. On Linux, use super::* is still used (source, touches, Duration).
  3. Comments. Both comments are accurate: the non-Linux source is the only caller of Watcher::watch, and off Linux the test module was empty, so its import was unused.
  4. Fleet evidence. I checked each named step with cmux-ci status, and each one matches the body:
    • a9b22739eb76034e0c4cede7: Linux clippy at 3534e7b, failed, exit 1.
    • 53459d04dbc05a7f16cb4055: Linux clippy at the head, done, exit 0.
    • 77bd03ed79cbc2e7cc278152: macOS clippy at 3534e7b, failed, exit 101.
    • 337633afdd8bfc8675e1a086: macOS clippy at the head, done, exit 0.
  5. Other checks. The change touches only cfg attributes and two comments, with no behavior change. feat-cmux-next has no later commits to login_env/ that this change could conflict with. The body has the Takes back #33406: line for the one-week take-back hold. risk:normal is correct.

@blacksmith-sh

blacksmith-sh Bot commented Nov 3, 2026 •

Copy link
Copy Markdown

Found 6 test failures on Blacksmith runners:

Failures

Test View Logs
/tmp/tmpd0vq8io_/summary-fail.json (live):/summary-fail.json (live): View Logs
/tmp/tmpl6kfu8b8/summary-missing.json (live):/summary-missing.json (live): View Logs
[new_workspaces] debug.new_workspace timed 0 of 50 new workspace rows/
[new_workspaces] debug.new_workspace timed 0 of 50 new workspace rows
View Logs
[new_workspaces] the window draws no frames/[new_workspaces] the window draws no frames View Logs
[tabs] debug.tab_switch timed 0 of 40 switches/
[tabs] debug.tab_switch timed 0 of 40 switches
View Logs
[tabs] the window draws no frames/[tabs] the window draws no frames 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 3, 2026
…pmux-watch-import-33406

# Conflicts:
#	cmux-tui/crates/acpmux/src/login_env/watch.rs
@lawrencecchen lawrencecchen removed 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 3, 2026
@lawrencecchen lawrencecchen changed the title base red: acpmux's login-env watcher imports only what each OS uses (clippy unused imports from #33406) base red: acpmux's login-env watcher test module is Linux-only, so macOS has no unused import (clippy, after #33406) Nov 3, 2026
@lawrencecchen

Copy link
Copy Markdown
Contributor Author

macOS clippy (cmux-tui-rust-check.sh clippy --os macos): red at the base 1ac26c7, step 649c60fd6b2a09102a48fe19 (unused import: super::*, exit 101); green at this head 675e27d, step 5b8200ffb625fba66afe494e (exit 0).

@lawrencecchen

Copy link
Copy Markdown
Contributor Author

Review (independent subagent): SHIP at 675e27d

Read the PR body and comments, the diff, and cmux-tui/crates/acpmux/src/login_env/watch.rs at the head and at origin/feat-cmux-next. No build or test was run by this review.

  1. Scope. The merge-base with feat-cmux-next is 1ac26c7 (the branch merged the base). The three-dot diff and the two-dot diff for this file both touch only watch.rs, +2/-2. The changes: #[cfg(test)] on mod tests becomes #[cfg(all(test, target_os = "linux"))], the #[cfg(target_os = "linux")] on the one test is removed, and one comment line is added. No change to non-test code. The base red: the notify Watcher import is macOS-only (unused on Linux after #33406) #33664 fix (#[cfg(not(target_os = "linux"))] use notify::Watcher as _;) is already on the base and stays unchanged.
  2. No test is lost. At the base, mod tests holds only use super::*; and one test, a_read_in_a_watched_folder_sends_nothing_and_a_write_arrives, which was already #[cfg(target_os = "linux")]. Before and after, that test compiles and runs only on Linux. Off Linux (macOS, Windows), the module was empty except for the import, which is the cause of the unused-import error. On Linux, use super::* is still used: the test calls source (the Linux #[cfg(target_os = "linux")] variant), touches (no cfg) and Duration (imported at the top of the file).
  3. Fleet evidence. I checked both steps from the PR comment with cmux-ci status and cmux-ci log:
    • 649c60fd6b2a09102a48fe19: ref 1ac26c7 (base), macos-26, state failed, exit 101. The log shows error: unused import: super::* at crates/acpmux/src/login_env/watch.rs:228:9 and could not compile acpmux (lib test).
    • 5b8200ffb625fba66afe494e: ref 675e27d (head), macos-26, state done, exit 0. The step is cargo clippy --workspace --exclude cmux-holder-v3 --all-targets --locked --keep-going -- -D warnings; acpmux compiles, and the summary is clippy pass.
      The Linux side is not changed in behavior: the same test exists under the same effective cfg.
  4. Title and body. Both match the change. The body says correctly that base red: the notify Watcher import is macOS-only (unused on Linux after #33406) #33664 fixed the Linux half and that this PR now has only the macOS fix. It also has the Takes back #33406: line. The comment is accurate.

Note: the PR has the risk:high label. A cfg change to tests only would normally be risk:low. This receipt on the exact head covers the high-tier review requirement in either case.

This branch was previously deployed

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant