Skip to content

base red: acpmux: macOS clippy dead start_logged; arithmetic ratchet back under its baseline - #28418

Merged
lawrencecchen merged 4 commits into
feat-cmux-nextfrom
base-red/daemon-memory-macos-dead-code
Oct 31, 2026
Merged

lawrencecchen merged 4 commits into
feat-cmux-nextfrom
base-red/daemon-memory-macos-dead-code

Conversation

@lawrencecchen

@lawrencecchen lawrencecchen commented Oct 30, 2026 •

Copy link
Copy Markdown
Contributor

Two acpmux lint base reds on feat-cmux-next.

1. macOS clippy: function start_logged is never used

cargo clippy -D warnings on macOS (job fb8745cdbcf3231af7557c1d, aws-m4pro-4, at #28041's merged head) fails at cmux-tui/crates/acpmux/tests/daemon_memory.rs:82. #27991 (941b7a4) moved Daemon::start onto start_inner, so start_logged has one caller left, retained_session_list_allocation_benchmark, which is #[cfg(target_os = "linux")]. Fix: start_logged is #[cfg(target_os = "linux")] too.

2. Panic ratchet: acpmux arithmetic_side_effects over its baseline 1430

The tracked count grew from the baseline that #28169 (66c3d03) measured (warning now). From the ratchet's clippy JSON at 66c3d03 (cmux-tui-spec run 41076340459) and at a current feat head (run 41086018852), per file:

merge file new hits now
4543835 (#28161) hub/session_pages.rs 3 (len() + 1, used + bytes twice) saturating_add
55df640 (#28353) daemon/unused_exit.rs 2 (clients += 1, arrivals += 1) saturating_add

7a5538d (#27868) removed one in agent_host/host.rs. No value here can reach usize::MAX or u64::MAX, so behavior is the same. The baseline is not lowered here: a lower count needs the Linux measurement of this PR's own ratchet run.

Evidence

run result
macOS clippy before (feat tip 4ab956c) fleet 5937adaa407e3121bf249dd6 (aws-m4pro-2) fails: associated function start_logged is never used
macOS clippy after (c73f1cc, the dead-code fix alone) fleet b62faf44852ef09f28767510 (aws-m4pro-2) passes
Linux clippy after (c73f1cc) fleet bf122b19e08a2d0c4e790703 passes (Linux never had the warning)
ratchet after (head) this PR's cmux-tui-spec run pending

R9: no new cap, limit or timeout.
R10: no hot path changes.

Observed / Not run

  • Observed: the runs above; prepush-gate.py passes.
  • Not run: the ratchet at the head (it runs in this PR's checks).

Changelog

  • none

No capture: no UI change.

lawrencecchen and others added 3 commits October 30, 2026 15:48
…caller

#27991 (941b7a4) moved Daemon::start onto start_inner, leaving
start_logged used only by the Linux-only retained_session_list_allocation_benchmark,
so clippy -D warnings on macOS failed with 'function start_logged is never used'.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
… add without overflow (panic ratchet)

The panic ratchet's tracked arithmetic_side_effects count for acpmux grew past
its baseline 1430: three adds in hub/session_pages.rs (4543835, #28161)
and two in daemon/unused_exit.rs (55df640, #28353). They saturate now;
no value they hold can reach usize::MAX or u64::MAX, so behavior is the same.

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

github-actions Bot commented Oct 30, 2026 •

Copy link
Copy Markdown
Contributor

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

Review 1f73eb4: 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=pr28418 -f ref=base-red/daemon-memory-macos-dead-code -f platform=macos -f backend_mode=local

Shipped in:

@blacksmith-sh

blacksmith-sh Bot commented Oct 30, 2026 •

Copy link
Copy Markdown

Found 2 test failures on Blacksmith runners:

Failures

Test 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_the_repository_highlights_validate (main.DigestTests.test_the_repository_highl
ights_validate)/
test_the_repository_highlights_validate (main.DigestTests.test_the_repository_highl
ights_validate)
View Logs

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

@lawrencecchen

Copy link
Copy Markdown
Contributor Author

Review (independent review subagent, Claude Opus 5.5): SHIP at 6825a75
reviewer-session: rbasered-7b4fbe43
author-session: a1c85faf51fad876b
R9: pass. No new cap, queue, limit or deadline.
perf claim: none

Scope: PR #28418, git diff origin/feat-cmux-next...6825a752c11 (daemon/unused_exit.rs, hub/session_pages.rs, tests/daemon_memory.rs). I read the code, the culprit commits and the fleet logs. I did not compile or run tests.

Findings

  1. Required before landing (merge, not a code defect). GitHub says this head is CONFLICTING. acpmux: a session list too large for one message goes in pages #28182 (49c648c) changed the same lines in hub/session_pages.rs byte_page: the base now has let bytes = size(&value) + 1; and still has used + bytes two times. Merge the base with scripts/merge-main.sh and keep the saturating form with the new size: let bytes = size(&value).saturating_add(1); let total = used.saturating_add(bytes);. After the push, the merge is a new head, so re-add ready-to-land.

  2. Pass (verified). start_logged gets #[cfg(target_os = "linux")]. Its only caller is at daemon_memory.rs:566, inside retained_session_list_allocation_benchmark, which is #[cfg(target_os = "linux")] (line 525). The macOS clippy red is associated function start_logged is never used in 5937adaa407e3121bf249dd6 (4ab956c, aws-m4pro-2). The same clippy passes at c73f1cc in b62faf44852ef09f28767510. This is the root-cause fix: the function and its caller now have the same cfg. No lint is allowed or skipped.

  3. Pass (verified). Each arithmetic change keeps behavior:

    • unused_exit.rs arrive: clients (usize) counts open connections. It cannot reach usize::MAX (each needs an fd and memory). arrivals (u64) increases once per connection; it cannot reach u64::MAX. saturating_add is correct where overflow cannot happen. Drop already uses saturating_sub. The arrivals != seen checks (lines 114, 118, 126) do not change.
    • session_pages.rs byte_page: bytes is a JSON string length plus 1, and used is the sum of rows that fit budget. Both are bounded by memory. total is computed once, and the same value goes into the compare and into used, so the page cut does not change.
      The 5 hits came from 4543835 (acpmux: ACP session/list goes in pages when the whole list is too large to send #28161: len() + 1, used + bytes 2 times) and 55df640 (acpmux: a daemon nobody uses stops after an hour #28353: clients += 1, arrivals += 1). I confirmed both in the culprit diffs.
  4. Pass (ratchet). This PR's clippy (changed cmux-tui packages) run (41087613416) passes the panic ratchet and says "counts went down in acpmux". The PR does not raise the baseline.

  5. Low (non-blocking). The body's evidence table still says the ratchet run is "pending". It passed; update the row.

checks: (gh pr checks 28418) 70 pass, 85 skipping, 4 fail.

…emon-memory-macos-dead-code

# Conflicts:
#	cmux-tui/crates/acpmux/src/hub/session_pages.rs
@lawrencecchen

Copy link
Copy Markdown
Contributor Author

Review (independent review subagent, Claude Opus 5.5): SHIP at 1f73eb4
reviewer-session: rbr3-407362a5
author-session: a1c85faf51fad876b
R9: pass. No new cap, queue, limit or deadline (saturating adds on existing counters and an existing byte budget).
perf claim: none

Scope: PR #28418. The last receipt was SHIP at 6825a75. The new head 1f73eb4 is a merge of feat-cmux-next 45d6f70 into 6825a75, with one resolved conflict in cmux-tui/crates/acpmux/src/hub/session_pages.rs. I checked the merge and the PR's patch after it.

Findings

None.

Checked and found fine

  • The PR's own commits are unchanged: git range-diff of the PR range before and after the merge shows c73f1cce9c9 = c73f1cce9c9 and 6825a752c11 = 6825a752c11.
  • The PR's patch against the new base (git diff 45d6f7093d1 1f73eb4d379) is the same 3 files and 9+/6- as before: daemon/unused_exit.rs (saturating clients/arrivals), hub/session_pages.rs, tests/daemon_memory.rs (#[cfg(target_os = "linux")] on start_logged). Nothing else changed.
  • The conflict resolution (git show --cc) keeps both intents:
  • Fleet: macOS clippy a4a8b0a15237a472ecf7a898 fetched 1f73eb4 and ran cargo clippy --workspace --exclude cmux-holder-v3 --all-targets --locked -- -D warnings: clippy pass, cmux-tui-rust-check: green, exit 0. This is the check the start_logged part of the PR fixes on macOS.

checks

gh pr checks 28418: 75 pass, 82 skipping, 2 fail. clippy (changed cmux-tui packages), rustfmt, cmux-next crash ratchet, cmux-next checks (god files, concurrency, crash safety, l10n), cmux-next fast tier (cargo), tooling-tests, risk evidence, merge freeze, ci-status pass. The 2 fails are a base finding: nightly-guards / workflow-guard-tests / release-notary and its wrapper nightly-guards / Guard status, red on feat-cmux-next until #28441 lands (same on #28416, #28417, #28467).

@lawrencecchen

Copy link
Copy Markdown
Contributor Author

Ready to land at 1f73eb4:

@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 30, 2026
@lawrencecchen
lawrencecchen merged commit 0e12c60 into feat-cmux-next Oct 31, 2026
241 of 244 checks passed
@lawrencecchen
lawrencecchen deleted the base-red/daemon-memory-macos-dead-code branch October 31, 2026 00:25

This branch was previously deployed

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