fix(next): drop the notify subtitle parameter #23286 made dead, and the comment describing it

Merged
TE
16d ago
+3-8

Two PRs fixed the same base compile break by opposite routes within an hour, and the one that landed second (mine) left a dead parameter and a comment that is now false. This removes both.

What happened

d168615bffca0 ("cmux notify --workspace ws_… scopes the notification and its clear to that workspace") passed subtitle: to post, which had no such parameter, so CmuxNextApp did not compile. Two lanes fixed it:

  • #23286 (03:00Z, bc661dae0e771) changed the caller to pass the existing workspaceName:. The formatter then takes its workspace branch: banner title build, subtitle Deploy done. It flipped WorkspaceNotificationArrivalTests to those expectations.
  • #23281 (03:58Z, mine) gave post the subtitle: parameter the original caller had asked for, leaving that caller as written.

They merged cleanly, because one touches the call site and the other the declaration, and base compiles and the test passes. But the two fixes are alternatives, and #23286's won: nothing passes subtitle: any more.

What this removes

-                      sound: String?, subtitle: String? = nil, program: ProgramStatusNotification? = nil,
+                      sound: String?, program: ProgramStatusNotification? = nil, headline: String? = nil,
-                headline: headline, posterSubtitle: subtitle ?? notification.shownSubtitle,
+                headline: headline, posterSubtitle: notification.shownSubtitle,
  • The parameter has no caller: git grep "subtitle:" in that file now finds only the declaration and the unrelated desktop.post(subtitle:). So subtitle ?? notification.shownSubtitle always took the right-hand side.
  • The doc comment I added with it said a tab-less banner "keeps the poster's headline as the title and names its workspace there". Under #23286 that is simply not what the code does — the workspace is the title. A false comment attached to a parameter nobody calls is the part actually worth deleting.

After this, post's signature is identical to its pre-feature form and the formatter call is byte-for-byte what it was, so #23286's design stands alone.

It also closes a gap a review found in my version

An independent review of #23281 found that routing the workspace name through subtitle: put it in the poster-owned slot, so when the poster supplied its own subtitle the workspace was named nowhere:

--subtitleworkspacetitlesubtitle
(none)buildDeploy donebuild
apibuildDeploy doneapi — workspace unnamed

Reachable, not theoretical: cmux-tui/spec/cli.md:558 allows --subtitle with --workspace. I had planned a follow-up that joined the two. #23286's route makes that unnecessary — the workspace is the title, so it is always named, and the poster's subtitle joins the headline (Deploy done · api) through the [state, poster].joined(" · ") the formatter already uses. Keeping the better design and deleting mine is the smaller change.

Verification

  • python3 scripts/cmux-next/prepush-gate.py — no new violations, exit 0.
  • The diff is one file, 3 insertions / 8 deletions, no generated file.
  • No test changes: WorkspaceNotificationArrivalTests asserts #23286's expectations (title build, subtitle Deploy done) and is untouched by this, because removing an uncalled parameter cannot change what any caller produces.

Observed / Not run

Observed: the gate above; git grep for every subtitle: in the file (declaration + the unrelated desktop.post only); the formatter's two branches read line by line (NotificationBannerFormatter.content), confirming workspaceName non-nil ⇒ title is the workspace and the subtitle is [state, poster].joined(" · "); git log establishing that #23286 landed before #23281.

Not run: no Swift compiled or run on this host (big-red); this change removes a defaulted parameter with no callers, so no fleet lane was dispatched for it. I did not run the app and took no captures.

Captures

No capture: this deletes an uncalled parameter and a comment, and changes no banner any person can see. Every banner's title, subtitle and body come from the same NotificationBannerFormatter.content call with the same arguments before and after — the removed subtitle ?? always resolved to notification.shownSubtitle, which is what it now reads directly. The banner behavior a capture would show is #23286's, already asserted by WorkspaceNotificationArrivalTests on base.

Changelog

none

🤖 Generated with Claude Code

Agent session: big-red


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


Summary by cubic

Removes the subtitle: parameter from NotificationCenterService.post and the doc comment describing it, which PR #23286 made dead by routing workspace notifications through the existing workspaceName: parameter instead. The parameter had no callers, so the subtitle ?? notification.shownSubtitle fallback always resolved to the right-hand side; the comment described behavior no longer true under #23286's design. After this change, post's signature returns to its pre-feature form and the formatter call matches what it was before the conflicting fixes, leaving #23286's design standing alone.