Repository navigation
fix(next): drop the notify subtitle parameter #23286 made dead, and the comment describing it - #23320
Conversation
…ent that described it #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`. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Agent-Session: https://cmux.com/deeplink/agent?harness=claude&session=01a15cb8-bbf0-7010-8d04-f9f2c0cb55df&host=big-red
|
Owned by chat-open-perf-6c on big-red · session |
|
cmux status (one comment, edited in place; each line is updated by the job that owns it) Review App of this head (app.zip, kept 1 day)
Not verified: macOS jobs did not run: compile and app tests were skipped on CI passes on Written by |
|
Bead |
|
Shipped in:
|
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") passedsubtitle:topost, which had no such parameter, soCmuxNextAppdid not compile. Two lanes fixed it:bc661dae0e771) changed the caller to pass the existingworkspaceName:. The formatter then takes its workspace branch: banner titlebuild, subtitleDeploy done. It flippedWorkspaceNotificationArrivalTeststo those expectations.postthesubtitle: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
git grep "subtitle:"in that file now finds only the declaration and the unrelateddesktop.post(subtitle:). Sosubtitle ?? notification.shownSubtitlealways took the right-hand side.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:--subtitlebuildDeploy donebuildapibuildDeploy doneapi— workspace unnamedReachable, not theoretical:
cmux-tui/spec/cli.md:558allows--subtitlewith--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.WorkspaceNotificationArrivalTestsasserts fix(next): workspace notification banner compiles against the where/what banner (base compile red) #23286's expectations (titlebuild, subtitleDeploy 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 grepfor everysubtitle:in the file (declaration + the unrelateddesktop.postonly); the formatter's two branches read line by line (NotificationBannerFormatter.content), confirmingworkspaceNamenon-nil ⇒ title is the workspace and the subtitle is[state, poster].joined(" · ");git logestablishing 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.contentcall with the same arguments before and after — the removedsubtitle ??always resolved tonotification.shownSubtitle, which is what it now reads directly. The banner behavior a capture would show is #23286's, already asserted byWorkspaceNotificationArrivalTestson base.Changelog
none
🤖 Generated with Claude Code
Agent session: big-red
Need help on this PR? Tag
@codesmith-botwith what you need.Summary by cubic
Removes the
subtitle:parameter fromNotificationCenterService.postand the doc comment describing it, which PR #23286 made dead by routing workspace notifications through the existingworkspaceName:parameter instead. The parameter had no callers, so thesubtitle ?? notification.shownSubtitlefallback 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.Written for commit 98327b6. Summary will update on new commits.