Skip to content

fix(next): workspace notification banner compiles against the where/what banner (base compile red) - #23286

Merged
teamleaderleo merged 1 commit into
feat-cmux-nextfrom
fix/next-notify-workspace-banner-compile
Oct 21, 2026
Merged

teamleaderleo merged 1 commit into
feat-cmux-nextfrom
fix/next-notify-workspace-banner-compile

Conversation

@teamleaderleo

@teamleaderleo teamleaderleo commented Oct 21, 2026 •

Copy link
Copy Markdown
Collaborator

feat-cmux-next does not compile CmuxNextApp right now:

Sources/CmuxNextApp/Notifications/NotificationCenterService.swift:399:59: error: extra argument 'subtitle' in call

d168615 (cmux notify --workspace) posts a workspace notification's banner with post(..., subtitle:). 9aa00c6 (banners say where, who and what) had already removed that parameter: post now builds title and subtitle with NotificationBannerFormatter (title = where, subtitle = the headline and the poster's subtitle). The two landed close together and the merge broke. Fleet cmux-ci compile-check --app fails on base 66a1a07 (job 58034821f3e8efd722a6dba8) and on 69b5399 (job 1355cfaf27521d9eac5bc916).

Fix: pass the workspace's name as workspaceName. The banner's title names the workspace and the subtitle is the headline, the same layout as a terminal's notification. WorkspaceNotificationArrivalTests expected the old layout (title = headline, subtitle = workspace); it now expects title build, subtitle Deploy done. It still checks that the banner names its workspace.

Red fix: merges once its own compile passes and an independent review passes.

No capture: the only UI effect is the title/subtitle order of a notify --workspace banner, matching every other banner.

Changelog: none

Agent session: cosmic-gray

🤖 Generated with Claude Code


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


Summary by cubic

Fixes a compile error on feat-cmux-next where a workspace notification still passed subtitle: to post, but that parameter was removed when banners moved to NotificationBannerFormatter.

Passes the workspace name as workspaceName instead, so the banner's title names the workspace and the subtitle shows the headline, matching every other banner. Updates WorkspaceNotificationArrivalTests to expect that layout.

Written for commit 513b5fe. Summary will update on new commits.

View guided diff Turn on auto-fix

…re/what banner (base compile red)

d168615 (notify --workspace) called post(..., subtitle:), which
9aa00c6 (banners say where, who and what) had removed: post builds
the title and subtitle with NotificationBannerFormatter. feat-cmux-next
fails to compile CmuxNextApp at NotificationCenterService.swift:399.

The workspace's name now goes in as workspaceName, so the banner's title
names the workspace and the subtitle is the headline, as for a terminal's
notification. The arrival test expects that layout.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Agent-Session: https://cmux.com/deeplink/agent?harness=claude&session=01a15cfc-663f-7650-b9f7-64d198c6b88b&host=cosmic-gray
@teamleaderleo teamleaderleo added the risk:low Risk tier (docs/ci/risk-tiers.md) label Oct 21, 2026
@teamleaderleo

Copy link
Copy Markdown
Collaborator Author

Owned by orphan-sweep on cosmic-gray · session

@teamleaderleo

Copy link
Copy Markdown
Collaborator Author

Owned by orphan-sweep on cosmic-gray · session

@github-actions

github-actions Bot commented Oct 21, 2026 •

Copy link
Copy Markdown
Contributor

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

Not verified: macOS jobs did not run: compile and app tests were skipped on 513b5fe3ea; ci-status is green without them.

CI passes on 513b5fe3ea (run 39580802897 attempt 1).
macOS jobs did not run: compile and app tests were skipped: Fast static checks skipped (the macos job needs both to succeed). Re-run CI once they pass, before merging.

Written by scripts/ci/classify_failures.py (ci-failure-attribution.yml); signatures are its SIGNATURES table. A machine verdict is the runner's fault, not this PR's; yours means the failing file is one this PR changes, also red on main that main's latest full suite fails the same way, seen on other PRs that it failed on another pull request's run lately.

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

@blacksmith-sh

blacksmith-sh Bot commented Oct 21, 2026 •

Copy link
Copy Markdown

Found 4 test failures on Blacksmith runners:

Failures

Test View Logs
/tmp/tmpoal17mex/summary-fail.json (live):/summary-fail.json (live): View Logs
/tmp/tmpwna7f2xp/summary-missing.json (live):/summary-missing.json (live): View Logs
convention violations found (ERROR lines above)./
convention violations found (ERROR lines above).
View Logs
test_every_source_and_test_directory_is_a_target (main.GraphCoversThePackage.test_e
very_source_and_test_directory_is_a_target) (directory='CmuxNextHomeEngineTests')/
test_every_source_and_test_directory_is_a_target (main.GraphCoversThePackage.test_e
very_source_and_test_directory_is_a_target) (directory='CmuxNextHomeEngineTests')
View Logs

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

@teamleaderleo

Copy link
Copy Markdown
Collaborator Author

Review (independent-subagent): SHIP at 513b5fe

Read with git against origin/feat-cmux-next (dfb5330; nothing on base has touched these files since the merge base 0e51a06, so base has not fixed this already).

  • Compile: post(_:tab:workspace:workspaceName:sound:program:headline:owner:) on base has no subtitle: parameter, so line 399 on base does not compile. The new call post(notification, tab: nil, workspace: workspace?.id, workspaceName: workspace?.displayName, sound: decision.sound) uses the labels in declaration order. The only subtitle: call left in CmuxNext is desktop.post(... subtitle: content.subtitle ...), which is valid (DesktopNotifier.post has subtitle: String? = nil). The fleet compile-check --app at this head agrees.
  • Banner: for cmux notify --workspace with headline "Deploy done", no poster subtitle, workspace "build", no tab and no host, NotificationBannerFormatter.content gives title "build" and subtitle "Deploy done". That matches the terminal path (where in the title, what in the subtitle). The poster's own subtitle is not lost: the formatter adds it to the subtitle as headline · poster. If the workspace is not found, the headline stays the title, as before.
  • Test: service.desktop is the real DesktopNotifier, which appends to posted before it checks for the system center. withApproval with tab: nil calls post(nil) synchronously, so posted.last is set when the test reads it. The new expectations (title "build", subtitle "Deploy done") match the formatter. The muted test is unchanged. Not compiled here: the test target was outside the compile-check.

Nit, not blocking and from before this PR: the doc comment on DesktopNotifier.Posted.subtitle ("The workspace a terminal program's banner came from") no longer matches the where/what layout.

@teamleaderleo
teamleaderleo merged commit bc661da into feat-cmux-next Oct 21, 2026
@teamleaderleo
teamleaderleo deleted the fix/next-notify-workspace-banner-compile branch October 21, 2026 10:00
teamleaderleo added a commit that referenced this pull request Oct 21, 2026
Picks up #23286, the base compile fix this head's compile needs.

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

Copy link
Copy Markdown
Contributor

Shipped in:

teamleaderleo added a commit that referenced this pull request Oct 21, 2026
…ent that described it (#23320)

#23281 and #23286 fixed the same base compile break (`d168615bffca0` passed
`subtitle:` to `post`, which had no such parameter) by opposite routes, and
#23286 landed first:

- #23286 changed the caller to pass the existing `workspaceName:`, so the
  formatter takes its workspace branch — banner title `build`, subtitle
  `Deploy done` — and flipped `WorkspaceNotificationArrivalTests` to match.
- #23281 (mine, 58 minutes later) gave `post` the `subtitle:` parameter the
  original caller had asked for.

They merged without a conflict, because they touch different lines, and the
result compiles and passes. But nothing passes `subtitle:` any more, so the
parameter is dead and `subtitle ?? notification.shownSubtitle` always takes the
right-hand side. Worse, the doc comment I added with it says a tab-less banner
"keeps the poster's headline as the title and names its workspace there", which
is no longer what this code does at all: #23286's caller makes the workspace the
title. A false comment on a parameter no one calls is the part worth removing.

`post`'s signature is now identical to its pre-feature form, and the formatter
call is byte-for-byte what it was. #23286's design stands on its own.

It also closes, by a better route than I had planned, the gap an independent
review of #23281 found in that design: with `subtitle:` the workspace went into
the poster-owned subtitle slot, so `cmux notify --workspace ws_x --subtitle api`
(both flags are allowed together, cmux-tui/spec/cli.md:558) named the workspace
nowhere. Under #23286 the workspace is the title, so it is always named, and the
poster's subtitle joins the headline as `Deploy done · api`.


Agent-Session: https://cmux.com/deeplink/agent?harness=claude&session=01a15cb8-bbf0-7010-8d04-f9f2c0cb55df&host=big-red

Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

risk:low Risk tier (docs/ci/risk-tiers.md)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant