Skip to content

base red: the Needs you banners and the system notifier move out of the App (#32213 app_lines) - #32438

Merged
lawrencecchen merged 2 commits into
feat-cmux-nextfrom
fix/base-red-banner-arch
Nov 2, 2026
Merged

lawrencecchen merged 2 commits into
feat-cmux-nextfrom
fix/base-red-banner-arch

Conversation

@lawrencecchen

@lawrencecchen lawrencecchen commented Nov 2, 2026 •

Copy link
Copy Markdown
Contributor

Summary

Fixes the tip arch red from #32213 (c9835aa, the agent banner): over its parent it added app_lines +278 to CmuxNextApp. After #32414 lands (the #32127 fix), this is the one rise left on the tip.

Takes back #32213: only where its code lives. Its Needs you banner rules leave the App for a package; the feature stays the same for the person.

The lines #32213 put in the App are mostly wiring that has to stay, so its rules move together with the system notifier they post through. A new CmuxNextNotifications target (layer ui) holds:

The strings move with their code, every locale included (the App catalogs only lose those keys). Tests now call service.attention.… instead of the old service.attention… methods.

Testing

  • python3 scripts/cmux-next/arch-ratchet.py --base c9835aa239ce^ --head 9a043f986d2: app_lines was +193 on the base tip (cmux NEXT: a banner with Show for a tab an agent marked (carry of #24345) #32213 alone: +278); with this PR it is -324. This commit alone is app_lines -517. No other row rises.
  • python3 scripts/cmux-next/prepush-gate.py: no new violations. arch_rules/layer_gate.py: ok. scripts/lint-ios-package-conventions.sh: ok. scripts/lint-xcstrings.py on the three changed catalogs: passed.
  • Fleet cmux-ci compile-check --ref 8522f32f328 --app: passed at 8522f32 (job adc1765dc79f44a33cc2147a).
  • Fleet suites (package-test-lane.sh suite Packages/macOS/CmuxNext ...): all 9 suites passed at 8522f32 (job 856dfeed60965ef8eca2b27f, aws-m4pro-8): AttentionBannerTests 11 tests, AttentionSnapshotTests 8, AttentionTabsTests, AgentPaneAttentionTabsTests 5, DesktopNotifierSystemCenterTests 5, IncognitoSurfacesTests, NotificationBannerContentTests, NotificationPreviewTests, NotificationsControlTests.

The head 9a043f9 differs from 8522f32 only in plans/cmux-next/swift-linux-inventory.md, put back to the base content (a generated file the "no new generated files" guard refuses in a PR; no check needs it current). The fleet runs above therefore cover the head's Swift.

Observed / Not run

Observed: the ratchet and gate runs, the fleet app compile, and the fleet suites above (#32213's own: AttentionBannerTests, AttentionSnapshotTests, AttentionTabsTests, AgentPaneAttentionTabsTests; and the moved notifier's: DesktopNotifierSystemCenterTests, IncognitoSurfacesTests, NotificationBannerContentTests, NotificationPreviewTests, NotificationsControlTests). Not run: a tagged build with a real Needs you banner and its Show button. That path is read, not run.

Changelog

none

Proof

No capture: no UI change; code moves between targets and keeps its behavior.

…he App (#32213 app_lines)

Fixes the tip arch red from #32213 (c9835aa): it added 278 CmuxNextApp
lines. A new CmuxNextNotifications target now holds:

- AttentionBanners: the Needs you banner rules #32213 put in a
  NotificationCenterService extension (marks changed, launch
  reconciliation, the policy arrival and decision, Show). The service is
  its host (AttentionBannerHost) and keeps only the app's facts: its
  machines, windows and the marked-tab list.
- DesktopNotifier (with its incognito strings), whose attention category
  and retained-banner read #32213 added, and the notification permission
  types it answers `notifications.status` / `notifications.request` with.
  NotificationsControl's control methods stay in the App.

Behavior is the same; the strings move with their code in every locale.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@lawrencecchen lawrencecchen added the risk:normal 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 9a043f9: 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=pr32438 -f ref=fix/base-red-banner-arch -f platform=macos -f backend_mode=local

Shipped in:

…generator

The inventory is a generated file no manifest lists, so a pull request that
edits it is red in "cmux-next no new generated files". Its freshness has no
check, and it is already stale on the base.

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

Copy link
Copy Markdown
Contributor Author

Review (claude-review-subagent): SHIP at 9a043f9

Reviewed read-only: 4a1c444..9a043f9 against the culprit c9835aa. 9a043f9 differs from 8522f32 only in plans/cmux-next/swift-linux-inventory.md, and that file is again the same as the base. So the fleet compile (adc1765dc79f44a33cc2147a) and the 9 suites (856dfeed60965ef8eca2b27f) at 8522f32 cover the head's Swift.

Findings (none blocking):

  • Behavior is the same. AttentionBanners.marksChanged / decision / arrival / post / show have the same logic as the removed NotificationCenterService extension. Only these changed: services -> host witnesses (attentionTabs, attentionDaemons, isIncognito, which read the same services paths), attentionSeen -> seen, and attentionBanners -> posted. The reconcile Task is the same, with [weak self, desktop] and it reads posted after the await. Before, the service deinit cancelled it. Now the AttentionBanners deinit cancels it, and that runs when the service is released, so the lifetime is the same. decision returns nil only when the host is gone. post() binds a strong host first, so that nil path cannot occur there.
  • No retain cycle. The service holds the lazy attention strongly, the back reference is weak, and the Task captures self weakly and only desktop strongly.
  • DesktopNotifier.swift is byte-identical to the base copy apart from the package keywords. Its package API names only same-module types or types from package import AppKit / UserNotifications. AttentionBannerHost names only types from its package imports (AgentPane, Daemon, DaemonService, Feed, Settings). The internal NotificationCenterService witnesses are sufficient for the package protocol.
  • Strings: a JSON comparison shows 8 Localizable keys and 2 NotificationsPanel keys moved, with identical content, into the new target's tables of the same name. App catalogs: no other key was added, removed or changed. Every String(localized:) in CmuxNextNotifications resolves in its own catalogs, and the incognito strings are in the NotificationsPanel table. No App or test source still uses a moved key.
  • git grep finds no remaining references to the removed names. Every App or test file that names DesktopNotifier / NotificationPermission* imports CmuxNextNotifications.
  • Only CmuxNextApp and CmuxNextAppTests depend on CmuxNextNotifications. Its dependencies cannot reach the App, so there is no cycle. layer_gate: ok. arch-ratchet at --base c9835aa^: app_lines -324 and no row rises. At the PR base: -517. prepush-gate: no new violations. lint-ios-package-conventions: ok (NotificationPermissionStrings is internal, so the struct form is fine).
  • Notes: the guardrail asks you to tell PL-arch about the architecture-layers.json row. The ratchet suggests lowering arch-ratchet.json (--update), which this PR does not need. Not run: a tagged build with a real Needs you banner and its Show button. Also, ci-target-graph.json does not yet list the new target, but routing for an unmapped package path falls back to every tier, so no coverage is skipped.

@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 merged commit 73eaec7 into feat-cmux-next Nov 2, 2026
188 of 196 checks passed
@lawrencecchen
lawrencecchen deleted the fix/base-red-banner-arch branch November 2, 2026 16:38
lawrencecchen added a commit that referenced this pull request Nov 3, 2026
…ic red since 25c69ee (#33986)

* swift-logic-classes: classify the false counts of the tip's swift-logic red since 25c69ee

The tip watcher's swift_logic_lines row has been red since 2026-11-02
05:38Z (last green 25c69ee: 415218). Part of the rise is not new
Swift logic: declarations whose reviewed row did not follow a module
move, and view, strings, tunables and debug code that the default rule
counts in full because no row lists it. This commit corrects the table
only (no code change). Each row carries its reason.

- 24 rows follow their declaration's module move (class, shares and
  reason kept): DesktopNotifier and NotificationPermission* to
  CmuxNextNotifications (#32438), LinkHintController to CmuxNextBrowser
  (#32537), FirstFrameGate, LinkDragItem, LinkDropPlan, StaticURL,
  PageInstallCounter, AgentDialogLine, NewTabLayoutChoice,
  RunningPrograms, AgentChatMute, CloudAppOp*, PaneFullWidthTabs,
  PaneTabBarChoices, WindowRegistryStore. LOGIC rows stay LOGIC.
- 3 stale rows follow a move that also changed the kind (enum to
  struct): WhatsNewTunables, NewTabTunables, NotificationPermissionStrings;
  the stale CloudMoveStrings enum row is replaced by its struct.
- 48 new PLATFORM rows: the vertical tab list's AppKit drawing, pointer,
  accessibility and pane extensions (#32127; its row layout math,
  VerticalTabListLayout, stays counted), Core Animation avatar looks
  (#33395), view hosts (BrowserPageBars, TerminalStatusLabel,
  SidebarContainerView+PaneReach, AgentPaneView+MoveCover), the person
  click check of consent prompts, the glyph bitmap cache, the thin Swift
  side of the Rust Cloud start rules (CloudStartCore), the at-cap dialog
  spec, localized strings, Debug Settings tunables, and DEBUG, bench or
  DEV playground and fixture code.

Not changed: every real rise stays counted (for example
NativeDeviceEnrollment, NativeCloudLinkChild, GitHubConnections,
VerticalTabListLayout, DisplayBackdropBinding, CreationInputBuffer);
REWRITE R-SWIFT-MIRRORS names the wave that removes each. The green
reference 415218 is not changed.

swift_logic_lines at 2b3d7cd (python3 -I scripts/cmux-next/
swift-logic-lines.py --rev 2b3d7cd --json, then with --table of
this commit): 431786 -> 427957 (-3829). mirror_lines 71688 unchanged.
table_rows_missing 36 -> 8 (stale rows of deleted declarations);
table_rows_invalid 0. scripts/cmux-next/tests/test_swift_logic_lines.py:
16 tests OK.

* swift-logic-classes: the review's three rows count their rules; reasons say what the code is

Review of #33986 (FIX at cb155fb, comment 6610083921): three rows
put a rule under PLATFORM. Each now counts it:

- RefusalStrings+Spaces: MIXED 0/46. Its spaceDelete (lines 14-19) is a
  Swift copy of the daemon's own rule (cmux-tui-core
  state/room_delete.rs:148 and personal_mutations.rs:241: the default
  room cannot be deleted), so its share is MIRROR and it counts in
  mirror_lines; the localized strings stay platform.
- TerminalStatusLabel: MIXED 65/0. Lines 17-23, 46-47, 49-76 and 88-131
  (81 of 125) are the reveal-wait state machine.
- LinkHintController: MIXED 35/0. #32537 moved and rewrote it; who gets
  each key while the page answers (hold, release, replay marks, key
  kinds, the 500 ms answer bound, outcome handling: about 87 of 251
  lines) is logic for REWRITE R-INPUT.

Text: the reasons of the strings and tunables rows say what the code is
and cite the table's own precedent rows, not a "table rule" no document
has; the AgentPaneView #1 row of AgentPaneView+MoveCover describes the
Chromium move it holds; LinkHintStrings moved to its own file, not to
another module; CloudStartCore's reason names its one hand-copied
fallback line. Three stale rows follow their moved code:
PersonalGroupLifecycle (LOGIC, now a struct in CmuxNextAppCore), the
NSResponder extension (PLATFORM, now in CmuxNextFocusUI), and
DiffTabReuse (LOGIC, now enum DiffTabChange and an extension of
DiffOpenSource in CmuxNextDiff; both rows LOGIC).

swift_logic_lines at 2b3d7cd with this table: 428126 (base table
431786; the first version of this PR 427957); mirror_lines 71694.
At feat f07e8d9: 433351 -> 429651. table_rows_missing 5,
invalid 0; scripts/cmux-next/tests/test_swift_logic_lines.py: OK.

* swift-logic-classes: LinkHintController counts the key routing its reason names; textInputHasKeyboard is listed

Re-review of #33986 (FIX at 791c494, comment 6611048817):

- LinkHintController: MIXED 35/0 -> 57/0. The reason already named
  hold, release, replay marks, key kinds, the answer bound and outcome
  handling as logic; those are lines 22-24, 28-31, 34-35, 45-46,
  52-77, 79-85, 98-105, 107-130, 132-140, 142-149, 180-200, 202-210
  and 217-237: 144 of 251 lines. The reason now lists them.
- extension NSWindow in CmuxNextFocusUI/TypedTextInput.swift (#31865):
  new LOGIC row. textInputHasKeyboard is the rule which input has the
  person's keyboard; the view-shaped default (an NSWindow extension)
  counted its 13 lines as 0.
- Reasons cite repository facts only (crates, files, PRs), not plan
  names outside the repository; the strings rows no longer quote a
  row count that changes.

swift_logic_lines at 2b3d7cd with this table: 428194 (base table
431786). At feat f07e8d9: 429719. mirror_lines 71694 / 71698.
table_rows_missing 5, invalid 0. scripts/cmux-next/tests/
test_swift_logic_lines.py: OK.

This branch was previously deployed

1 inactive deployment
ci-routing-agent — 9a043f98 Deployed Nov 2, 2026 by lawrencecchen via cmux-tui tree preflight #101955
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