feat(macOS): show terminal controls in native titlebar

[kshivang]

Keep MCP, sharing and calling controls in BossConsole's macOS native titlebar whenever the terminal plugin is enabled, including browser tabs and windows without an open terminal. A window-owned plugin composition supplies live actions and dialogs. Use BossTerm's native sharing/phone symbols, retain its MCP icon, and group Sharing/Call/MCP into one native toolbar bubble, with Search/Tools/Toolbox in a second bubble. Grouped status selection follows live service state, including MCP on/off; opening a menu does not toggle it. Unregistering the provider removes its controls. Unsupported titlebars retain existing behavior.

The live voice-call panel is hosted by BossConsole at the bottom-right of the window across all selected tabs. It uses the existing content-sized overlay above heavyweight browser surfaces, with meter, Mute and End controls; terminal panes suppress their duplicate. Call content is isolated by window and removed when its provider is disposed.

Requires Plugin API 1.0.95 from risa-labs-inc/boss-plugin-api#65. Keep this draft until that release exists. Terminal controls additionally require the terminal-tab integration and BossTerm export kshivang/BossTerm#438. Native titlebar integration remains macOS-only. The bottom status bar now defaults to off on all desktop platforms, including macOS and Windows; the View menu still toggles it.

Validation: desktop compilation, ktlintCheck, detekt, TerminalTitleBarBridgeTest and ApiPackageDivergenceTest pass against the locally built API. Live native UI verification remains pending.

The shared bridge now uses deterministic identity-based owner arbitration and keyed call-panel composition, matching API #65. Unchanged publications preserve ownership; disposal restores the prior eligible owner. The public lifecycle facade has a documented targeted Detekt suppression. Release this host integration before terminal-tab #114, whose minBossVersion is 9.5.30; reconfirm that version when publishing.

[supabase]

This pull request has been ignored for the connected project pcnwqamqdnsadranufjv because there are no changes detected in supabase directory. You can change this behaviour in Project Integrations Settings ↗︎.

---

Preview Branches by Supabase.
Learn more about Supabase Branching ↗︎.

[github-actions]

Claude diff review of 1563369 (no tests executed).

Review of PR #1754 @ 1563369

Scope note: this is a diff-only review. I have not run the build or tests, and several call sites (makeItem, dispatchSafely, NativeTitleBarAction, OverlayCorner, defaultWindowAppearanceSettings, WindowAppearanceMigrations) are referenced but not shown, so anything depending on them is flagged as uncertain.

---

Confirmed defects

1. TerminalOwners.put's equality guard cannot work for Entry → recomposition feedback loop

plugin-ui-core/.../TerminalTitleBarBridge.kt, TerminalOwners.put (~L130):

val index = values.indexOfFirst { it.owner === owner }
if (index >= 0 && values[index].value == value) return
val key = if (index >= 0) values.removeAt(index).key else Any()
values.add(Owned(owner, value, key))

The comment above the class claims "Unchanged effects do not invalidate composition." That guarantee does not hold for Entry:

• Entry is a data class whose actions: List<TerminalTitleBarAction> compares element-wise;
• TerminalTitleBarAction (~L15) is a plain class with no equals, so element comparison is identity.

Therefore any publisher that constructs its action list per composition (the natural shape, since active must change) always fails the guard and mutates values, a mutableStateListOf. The readers of that state are composables: actions() is read by nativeTerminalTitleActions (NativeTerminalTitleActions.kt L9), which is now reached from sidebarTitleActions (BossAppScaffold.kt L1160), which lives in the same composable that hosts TerminalTitleBarBridge.Content(windowId) (NativeTerminalTitleActions.kt L25). So: provider SideEffect → publish → state write → SidebarTitleBar invalidated → Content recomposes → provider SideEffect → publish … i.e. an unbounded recomposition loop unless the plugin memoizes the exact TerminalTitleBarAction instances.

The kdoc ("Publish from effects with stable callbacks", L~32) documents the invariant, but nothing enforces it and the guard silently gives a false sense of safety. Making TerminalTitleBarAction a data class (or giving it equals/hashCode) would make the existing guard actually effective; note that would also change the ABI contract you explicitly froze at L12–14 ("Preserve this constructor"), so it is better done now than after 1.0.95 ships.

The supplied test cannot catch this: TerminalTitleBarBridgeTest creates a Recomposer but never calls runRecomposeAndApplyChanges(), so only the initial setContent pass runs.

2. TerminalCallOverlay rebuilds the call bar subtree on every window-focus change

composeApp/.../app/TerminalCallOverlay.kt L17–29:

if (!LocalWindowInfo.current.isWindowFocused) {
    Box(Modifier.align(Alignment.BottomEnd).padding(12.dp)) { TerminalTitleBarBridge.CallBarContent(windowId) }
    return
}
OverlayCorner(alignment = Alignment.BottomEnd, initialSize = DpSize(480.dp, 56.dp)) { ... }

The two branches are structurally different call sites, so toggling focus (alt-tabbing away from the window, clicking another app) disposes and re-creates the entire CallBarContent subtree rather than moving it. Every remember/DisposableEffect inside the plugin's call bar is torn down and re-run on each focus transition — for an in-call UI (timers, mute state, media surfaces) that is a visible regression. Additionally, OverlayCorner's user-adjusted position/size will reset each time the branch flips back, since the overlay composable itself is re-created.

If the intent is only to disable dragging while unfocused, that should be a parameter to a single OverlayCorner call site, not two branches.

3. showBottomBar default flip silently removes the status bar for every existing install, with no migration in this diff

composeApp/.../window/WindowAppearanceSettings.kt L63:

-    val showBottomBar: Boolean = true,
+    val showBottomBar: Boolean = false,

By the file's own persistence rule, retained at L48+ ("a value equal to the default is never stored, so a file that does not mention a bar picks the new default up"), no existing install has showBottomBar written, because true was the default. Every existing user therefore loses the status bar on upgrade. The diff deletes the paragraph that argued this specific bar should stay:

-     * The status bar is the exception because nothing replaces it. It is the only always-on
-     * readout of what the app is doing - the current URL, memory, transient status messages - and
-     * none of that is reachable from a menu or a launcher.

and replaces it with "the View menu can restore any bar" — but the deleted text's claim was about content (URL, memory, transient status) having no other surface, which the View menu does not address and which nothing else in this diff replaces. The diff also does not touch WindowAppearanceMigrations, which the surrounding comment still cites as the mechanism for moving existing installs. At minimum this needs an explicit decision recorded, and a check that a migration is (or deliberately is not) required.

Related: WindowAppearanceSettings is in commonMain, so the default also changes on any non-desktop target, while the new doc text says "off by default on desktop" (L40). The only new coverage (ChromeDensityControlsTest) exercises defaultWindowAppearanceSettings on desktop only.

4. TerminalTitleBarBridge is duplicated in the host and in the published API JAR

TerminalTitleBarBridge.kt L9: "Host mirror omits the API JAR's documentation-only HostImplemented marker to avoid an API dependency cycle." combined with gradle/libs.versions.toml L184 (boss-plugin-api = "1.0.95") means the exact FQN ai.rever.boss.plugin.ui.TerminalTitleBarBridge / TerminalTitleBarAction will exist in two artifacts on the same classpath. Which one wins is classpath-order dependent, and any signature drift between the mirror and the released JAR becomes a runtime NoSuchMethodError/NoClassDefFoundError in plugins rather than a compile error. Nothing in this diff asserts parity between the two declarations. (The BrowserTitleBarBridge precedent referenced in the toml comment presumably has the same shape — if there is an existing parity check, this new pair should be added to it.)

Also stale: the comment block at libs.versions.toml L181–183 still only documents 1.0.94/BrowserTitleBarBridge while bumping to 1.0.95; the "Release the API before merging this pin" gate now applies to an API version the comment never mentions.

---

Uncertain observations (need verification against code not in the diff)

• Content() ignores windowId when selecting a provider (TerminalTitleBarBridge.kt L68–71): providers.last() returns the most recently registered provider globally. With two registered providers only one ever renders, in any window, silently. The kdoc says the terminal plugin registers exactly one, but nothing enforces or surfaces the violation.
• hostWindow is a boolean claim, not a refcount (L~88). If two composables ever claim the same windowId (e.g. during a sidebar relayout where old and new SidebarTitleBar overlap), the first onDispose clears the surviving claim and the call bar/actions disappear. NativeTerminalHostAvailability keys its DisposableEffect on (windowId, ready), which handles the single-owner case but not overlap.
• First-frame ordering: in BossAppScaffold.SidebarTitleBar, NativeSidebarTitleBar(title, actions) is evaluated before NativeTerminalHostAvailability sets the claim (L1122–1124), and Content() only composes when isHosted is already true. So the first native toolbar build necessarily lacks terminal actions and the toolbar is rebuilt on the next recomposition. Likely just a pop on window open, but worth confirming the AppKit rebuild is idempotent.
• "mcp" used as a cross-API sentinel (NativeTerminalTitleActions.kt L12–13): symbol.takeUnless { it == "mcp" } / icon = if (it.symbol == "mcp") it.icon else null. This is an undocumented, untested protocol between the plugin's symbol field and the host; it also depends on NativeTitleBarAction.symbol/icon being nullable, which I cannot confirm from the diff. A dedicated nullable symbol or an explicit enum would be safer than a magic string.
• Hard-coded id coupling: MacToolbarGroups.members (L12–15) hard-codes terminal_sharing/terminal_call/terminal_mcp, which only match if the plugin publishes ids exactly sharing/call/mcp (prefix added at NativeTerminalTitleActions.kt L11). A renamed or added plugin id degrades to an ungrouped standalone toolbar item with no warning. Also note the group forces members-declared order, overriding whatever order the plugin publishes.
• Cross-thread map mutation (MacSidebarToolbar.kt): groupedItems/items are plain mutableMapOf. MacToolbarGroups.update now writes groupedItems both from the delegate path inside makeItem (AppKit callback thread) and from update()/the invokeLater block (EDT). Whether this is actually racy depends on which thread the JNA delegate callbacks land on and on dispatchSafely's semantics — neither is visible here. The pre-existing items map already has the same exposure, so this may be accepted practice in this file, but the new write set is larger.
• Selection re-sync placement (MacSidebarToolbar.kt L208–218): the re-sync runs asynchronously on invokeLater after AppKit has already flipped the clicked segment's selection, so there is a one-frame flash of the wrong selection before it is restored. It also calls setSelected:atIndex: on utility_controls, which is configured Momentary (MacToolbarGroups.create L27) — probably a harmless no-op, but the comment ("Opening a status menu is not a service toggle") only justifies the terminal group.
• AppKit constants look correct to me (SelectAny = 1, Momentary = 2, ControlRepresentation.Expanded = 1), but they are raw literals with no named constants; worth a second pair of eyes.
• Autorelease/ownership: MacToolbarGroups.create (L21–24) does alloc + initWithItemIdentifier: and never releases; update (L43) uses [NSMutableArray array] (autoreleased) from a JNA thread whose autorelease pool state is unclear. Consistent with the surrounding file as far as I can tell from the diff, but flagging since it is new native allocation.

---

Test gaps

1. MacToolbarGroups has no tests at all. identifiers() (L18) is pure and trivially testable: mapping + distinct(), interleaving of grouped and ungrouped ids, and its interaction with the index > 0 && id != "split_horizontal" spacer rule in MacSidebarToolbar.kt L88. update()'s previous[id] != present cache and the setSelected:atIndex: index alignment (which silently misaligns if present and the selection loop ever disagree) are also untested.
2. nativeTerminalTitleActions is untested — specifically the "mcp" symbol/icon swap and the terminal_ prefixing that MacToolbarGroups.members depends on. A rename on either side breaks the grouping with no failing test.
3. TerminalCallOverlay focus branching is untested, including the defect in §2.
4. Documented bridge behaviour is untested: "The most recently changed registration/publication wins when owners overlap; removing it restores the previous owner" (kdoc L~33) has no test with two overlapping owners on the same window, nor a test that remove of the top owner restores the previous one. TerminalTitleBarBridgeTest only tests two owners on different windows.
5. Recomposer is created but never run (TerminalTitleBarBridgeTest.kt L20, 24): only the initial composition pass executes, so no test covers re-publication, invalidation, or the loop risk in §1.
6. Test uses a process-global singleton without isolation. TerminalTitleBarBridge is an object; both tests mutate shared state keyed by arbitrary window ids. Tests are order- and parallelism-sensitive. The first test's finally (L~48–55) cleans up unregisterProvider, remove, hostWindow but omits removeCallBar(actionsOwner) — asymmetric with the publishCallBar at L33. It happens to be covered by composition.dispose() running the inner onDispose, but if the composition ever fails to dispose the call bar leaks into subsequent tests.
7. File/package placement: composeApp/src/desktopTest/kotlin/ai/rever/boss/components/sidebar/TerminalTitleBarBridgeTest.kt declares package ai.rever.boss.components.sidebar while testing ai.rever.boss.plugin.ui.TerminalTitleBarBridge from a different Gradle module. The directory, the package and the subject all disagree, and plugin-ui-core's own test source set gets no coverage of the type it ships.
8. ChromeMetricsTest update looks arithmetically consistent (62.dp → 31.dp, i.e. dropping the 31dp bottom bar), but only one assertion was updated. Any other test or golden that depends on showBottomBar = true as the default (persistence round-trips, migration tests, layout snapshots) would need the same change and is not visible in this diff.

[github-actions]

Claude diff review of 1563369 (no tests executed).

Review of PR #1754 @ 1563369

Scope note: this is a diff-only review. I have not run the build or tests, and I can't see the plugin side that actually calls TerminalTitleBarBridge.publish(...), nor the bodies of MacToolbarRuntime.send/string, dispatchSafely, OverlayCorner, or NativeTitleBarAction. Findings that depend on those are marked uncertain.

---

Confirmed defects

1. TerminalOwners.put's change-detection guard cannot work for the payload it guards

plugin-ui-core/.../TerminalTitleBarBridge.kt

class TerminalTitleBarAction(        // NOT a data class, no equals/hashCode
    val id: String, val label: String, val symbol: String,
    val icon: ImageVector, val active: Boolean, val onClick: () -> Unit,
)
...
private data class Entry(val windowId: String, val active: Boolean, val actions: List<TerminalTitleBarAction>)
...
fun put(owner: Any, value: T) {
    val index = values.indexOfFirst { it.owner === owner }
    if (index >= 0 && values[index].value == value) return   // <- guard
    val key = if (index >= 0) values.removeAt(index).key else Any()
    values.add(Owned(owner, value, key))
}

Entry is a data class, but its actions list compares element-wise using TerminalTitleBarAction.equals, which is reference identity. The class doc explicitly says "Identity-owned, explicitly ordered state. Unchanged effects do not invalidate composition." — that contract only holds if the caller hands back the same TerminalTitleBarAction instances. Any publisher that constructs its action list inline (which is exactly what the documented pattern — "Publish from effects with stable callbacks" — invites) will hit removeAt + add on a mutableStateListOf on every publish.

Consequence chain that looks reachable from this diff alone:
Content(windowId) composes the provider inside NativeTerminalHostAvailability → provider's SideEffect calls publish → state list mutated → nativeTerminalTitleActions(windowId) (a snapshot read in sidebarTitleActions, BossAppScaffold.kt:1160) invalidates → SidebarTitleBar recomposes → provider re-executes → SideEffect publishes again → unbounded recomposition.

The same applies to CallBar(windowId, content: @Composable () -> Unit): data class equality over a lambda is identity unless the compose compiler memoized it at the call site, which is not guaranteed for a lambda passed into a plain (non-@Composable) function like publishCallBar.

Suggested fix: make TerminalTitleBarAction a data class with an equals that deliberately ignores onClick (or compares it), and/or compare structurally inside put on the fields that matter. At minimum, add a unit test asserting that a second publish with an equal-but-distinct action list does not mutate the backing list.

2. Focus toggling destroys and recreates the call-bar subtree

composeApp/.../app/TerminalCallOverlay.kt

if (!LocalWindowInfo.current.isWindowFocused) {
    Box(Modifier.align(Alignment.BottomEnd).padding(12.dp)) { TerminalTitleBarBridge.CallBarContent(windowId) }
    return
}
OverlayCorner(alignment = Alignment.BottomEnd, initialSize = DpSize(480.dp, 56.dp)) {
    Box(Modifier.padding(12.dp)) { TerminalTitleBarBridge.CallBarContent(windowId) }
}

These are two distinct composable call sites. Every window focus change moves the call bar between them, which disposes the entire subtree and re-composes it fresh: all remembered state inside the plugin's call bar (elapsed-timer state, expanded/collapsed, mute animation, any DisposableEffect-held resource) is torn down and rebuilt. For a live call UI, losing focus is the most common event in the app's life. key(entry.key, windowId) inside CallBarContent does not preserve identity across different slot positions.

It also means the bar silently changes affordances on focus change: draggable/resizable in one branch, pinned and non-interactive-chrome in the other, with a visible position jump if the user had moved it. Neither the code nor the comment (/** The live call belongs to the window, not the selected terminal or browser tab. */) explains why unfocused windows must bypass OverlayCorner.

3. showBottomBar default flip is a cross-platform change described as desktop-only, and the diff deletes the rationale without replacing the capability

composeApp/commonMain/.../window/WindowAppearanceSettings.kt

-    val showBottomBar: Boolean = true,
+    val showBottomBar: Boolean = false,

Three separate concerns:

• The new doc says "off by default on desktop", but this is the commonMain data-class default, so every platform that constructs WindowAppearanceSettings() without an explicit value (Android/iOS/web, plus any test fixture) also loses the bar. The only defaults function updated in tests is defaultWindowAppearanceSettings(isMacOs = …) (ChromeDensityControlsTest.kt), which does not cover non-desktop construction paths.
• The deleted paragraph made a specific, still-true argument: the status bar is "the only always-on readout … the current URL, memory, transient status messages — and none of that is reachable from a menu or a launcher." The replacement text ("the View menu can restore any bar") answers a different question — restoring the bar is not the same as the content remaining reachable. If transient status messages are the app's only error/progress surface, hiding it by default is a user-visible regression. Nothing in this diff relocates that content.
• The surviving doc still says the decode default is what moves existing installs, and references WindowAppearanceMigrations, but no migration entry is added here. Users who never touched the setting silently lose the bar on upgrade. If that's intended, please say so in the KDoc; if a migration is required (as it apparently was for the other three bars), it's missing.

---

Likely problems / needs verification

4. Group selection is resynced from stale state before the action runs

composeApp/desktopMain/.../window/MacSidebarToolbar.kt, in handleAction:

SwingUtilities.invokeLater {
    if (!closed) {
        dispatchSafely {
            MacToolbarGroups.members.keys.forEach { groupId ->
                items[groupId]?.let { MacToolbarGroups.update(groupId, it, actions, groupedItems, ::makeItem) }
            }
        }
        onAction(id)
    }
}

actions here is the pre-click snapshot. For a genuine toggle (e.g. terminal_sharing), AppKit has already flipped the segment visually; this code immediately flips it back to the old value, and correctness then depends entirely on the host recomposing and calling setActions afterwards — which is precisely the case the comment says it is protecting against ("even when the action does not recompose the host"). If onAction is async (a service handshake), you get a visible flip-back-then-flip. Consider resyncing after the host state settles, or scoping the resync to the momentary/menu group only.

Also note this loop runs for every toolbar action id, including ids that belong to no group.

5. Reusing top-level NSToolbarItem pointers as group subitems

MacToolbarGroups.update builds subitems via present.mapNotNull { makeItem(string(it)) }, and makeItem returns pointers out of MacSidebarToolbar.items, which is a long-lived cache. search, tools and toolbox existed as top-level toolbar items before this change, so on the first setActions after upgrade the same NSToolbarItem instance may be installed both in the toolbar's item list and as a subitem of utility_controls. AppKit generally does not tolerate one item living in two places. Worth verifying on a real macOS run that the toolbar is fully rebuilt (identifiers changed → old items removed) before the group is populated.

6. NSToolbarItemGroup is alloc/init'd and never released

MacToolbarGroups.create: pointer(pointer(clazz("NSToolbarItemGroup"), "alloc"), "initWithItemIdentifier:", string(id)) produces a +1 retained object stored in items forever. The NSMutableArray in update is autoreleased (fine), but each group leaks per toolbar/window. This mirrors whatever MacSidebarBoundary.create does, so it may be an accepted pattern — flagging so it's a conscious choice.

7. Window claim is a boolean, not a refcount

TerminalTitleBarBridge.hostWindow(windowId, enabled) does windows.remove(windowId) on false. NativeTerminalHostAvailability's onDispose { hostWindow(windowId, false) } therefore clears the claim for the whole window id even if some other composable also claimed it. Today there appears to be one call site (BossAppScaffold.kt:1125), but the API shape makes the second call site a silent bug. An owner-keyed set would be safer and would match how providers/entries/callBars already work.

Related ordering wrinkle: NativeTerminalHostAvailability composes Content(windowId) before the DisposableEffect sets the claim, so the provider is guaranteed not to render on the first pass and needs an extra recomposition to appear. Benign but worth a comment.

8. Hard-coded "mcp" sentinel and hard-coded group member ids

NativeTerminalTitleActions.kt:

symbol = it.symbol.takeUnless { symbol -> symbol == "mcp" },
icon = if (it.symbol == "mcp") it.icon else null,

This encodes "no SF Symbol exists for this one" as the magic string "mcp". Any future vector-only action from the plugin will be handed to AppKit as a bogus SF Symbol name and (presumably) render blank, because icon is dropped. The API already carries both symbol and icon; making symbol nullable in TerminalTitleBarAction (you control this class — it's new in this diff) removes the sentinel entirely.

Separately, MacToolbarGroups.members hard-codes "terminal_sharing", "terminal_call", "terminal_mcp", which must match the plugin's action ids prefixed with terminal_ by NativeTerminalTitleActions. That's a cross-repo string contract with no test and no assertion; a renamed plugin id silently degrades to ungrouped buttons.

9. Mirrored public type vs. the published API JAR

gradle/libs.versions.toml bumps boss-plugin-api to 1.0.95, but the comment immediately above still reads "1.0.94 adds the shared BrowserTitleBarBridge. Release the API before merging this pin." — please update it to state what 1.0.95 adds and whether it is actually published.

More importantly, plugin-ui-core/.../TerminalTitleBarBridge.kt declares ai.rever.boss.plugin.ui.TerminalTitleBarBridge / TerminalTitleBarAction in the host while the 1.0.95 JAR presumably declares the same FQNs (the file's own comment says "Host mirror omits the API JAR's documentation-only HostImplemented marker"). This works only under strict parent-first loading and only while the two declarations stay binary-identical. Nothing in the diff enforces that — a field reorder or an added default parameter on either side becomes a NoSuchMethodError at plugin load. A signature/ABI check (or generating the mirror) would be worth adding.

10. sidebarTitleActions became @Composable

BossAppScaffold.kt:1139. The caller isn't in the diff. Any other invocation from a non-composable context is now a compile error, and the composable calls now happen inside buildList { … }. I believe composable calls inside buildList's inline lambda are legal, but I can't verify compilation from the diff — worth confirming CI actually built this module.

---

Test gaps

• MacToolbarGroups.identifiers is pure Kotlin and completely untested. It's the only new logic that needs no JNA/AppKit: dedup, ordering when members are interleaved with non-members, passthrough of unknown ids, behaviour when a group has zero present members (which still leaves a stale entry in items/groupedItems), and interaction with the split_horizontal spacing special case at MacSidebarToolbar.kt:88. This is the cheapest high-value test in the PR.
• No test for the defect in §1. TerminalTitleBarBridgeTest reuses a single val action instance throughout, so the put dedup path is never exercised with an equal-but-distinct payload — exactly the case that fails.
• No test for nativeTerminalTitleActions' "mcp" mapping (symbol/icon inversion), despite it being branchy, string-keyed logic.
• No test for TerminalCallOverlay's two branches, including the focus-driven subtree swap in §2.
• Bridge test is in the wrong module. The type lives in plugin-platform/plugin-ui-core/src/commonMain, but the test is in composeApp/src/desktopTest/.../components/sidebar/TerminalTitleBarBridgeTest.kt — a package unrelated to the subject. The owning module gets no coverage, and the multiplatform (commonMain) API is only tested on desktop.
• Shared-singleton test hygiene. TerminalTitleBarBridge is a global object; both tests mutate providers/entries/callBars/windows. They use distinct window ids, but providers.last() is global, so any parallel or interleaved execution is order-sensitive. Also, the first test's finally calls remove(actionsOwner) but not removeCallBar(actionsOwner) — it relies on composition.dispose() running the onDispose, which is true only as long as that line is first in the finally. Make the cleanup symmetric, or add an @AfterTest reset hook on the bridge.
• No coverage for the showBottomBar default change beyond arithmetic. ChromeMetricsTest and ChromeDensityControlsTest were updated to assert the new numbers, but there's no test that an existing settings file with an explicit showBottomBar = true still decodes to true (the migration concern in §3), and none that non-desktop targets are unaffected.

[github-actions]

Claude diff review of 1563369 (no tests executed).

Review of PR #1754 @ 1563369

Scope: a new TerminalTitleBarBridge (plugin-facing singleton), macOS toolbar grouping, a window-level call overlay, and a flip of the showBottomBar default. Comments below are from the diff text only; I have not run the build or tests and cannot see the provider implementation in BossConsole, SidebarTitleBar's outer conditions, makeItem's non-group branch, dispatchSafely, or WindowAppearanceMigrations.

---

Likely defects

1. TerminalOwners.put can drive an unbounded recomposition loop (high risk)

plugin-platform/.../TerminalTitleBarBridge.kt:141-144:

val index = values.indexOfFirst { it.owner === owner }
if (index >= 0 && values[index].value == value) return
val key = if (index >= 0) values.removeAt(index).key else Any()
values.add(Owned(owner, value, key))

The short-circuit depends on value ==. For entries, value is Entry (a data class) whose actions: List<TerminalTitleBarAction> compares element-wise, and TerminalTitleBarAction is a plain class (:15), so element equality is reference equality. For callBars, CallBar.content is a @Composable () -> Unit lambda — also reference equality.

Consequences if a provider allocates fresh actions/lambdas per recomposition (the normal case when onClick captures state):

1. SidebarTitleBar → nativeTerminalTitleActions reads entries (BossAppScaffold.kt:1160, NativeTerminalTitleActions.kt:9) and therefore subscribes to the state list.
2. NativeTerminalHostAvailability composes TerminalTitleBarBridge.Content(windowId) in the same subtree (NativeTerminalTitleActions.kt:26).
3. The provider's effect calls publish(...) with new instances → removeAt + add → state-list write → invalidates the reader in (1) → recompose → (2) → (3).

The KDoc at :31 (“Publish from effects with stable callbacks”) acknowledges the hazard, but nothing in the host enforces it and the failure mode is a spinning UI thread, not a visible error. The supplied test hoists a single action val and publishes it repeatedly, so it deliberately exercises the stable path only and cannot catch this.

Suggested fix: compare on value-bearing fields (id, label, symbol, active, and icon) and ignore onClick, or make TerminalTitleBarAction a data class and require callers to remember the lambda — the former is the only one the host can guarantee.

2. TerminalCallOverlay rebuilds the call bar on every window focus change

composeApp/.../app/TerminalCallOverlay.kt:19-29:

if (!LocalWindowInfo.current.isWindowFocused) {
    Box(Modifier.align(Alignment.BottomEnd).padding(12.dp)) { TerminalTitleBarBridge.CallBarContent(windowId) }
    return
}
OverlayCorner(alignment = Alignment.BottomEnd, ...) { Box(...) { TerminalTitleBarBridge.CallBarContent(windowId) } }

These are two structurally distinct call sites. Toggling window focus therefore disposes one subtree and composes the other, discarding all remember/DisposableEffect state inside the plugin's call bar — for a live call widget (timers, animations, media handles, drag position) that is a user-visible regression on every alt-tab. The key(entry.key, windowId) guard inside CallBarContent (TerminalTitleBarBridge.kt:92) preserves identity across republishes but does nothing across two different call sites.

Prefer a single CallBarContent call with the focused/unfocused difference expressed as a modifier/wrapper, or hoist the state.

3. showBottomBar default flip silently removes the status bar from existing installs

composeApp/.../window/WindowAppearanceSettings.kt:63 changes showBottomBar = true → false, and the retained KDoc (:48-52 region) states that “a value equal to the default is never stored, so a file that does not mention a bar picks the new default up.” Every existing user who left the bottom bar at its default therefore loses it on upgrade, and the diff adds no WindowAppearanceMigrations entry (that file is not touched) even though the surviving comment points at it as the mechanism for exactly this situation.

The removed paragraph (-“The status bar is the exception because nothing replaces it … the current URL, memory, transient status messages — and none of that is reachable from a menu or a launcher”) is replaced by the claim that “the View menu can restore any bar” (:41), which answers discoverability but not the original argument that the content has no other home. Two follow-ups:

• Confirm a migration pins the old value for existing profiles, or state explicitly that the silent change is intended.
• ChromeDensityControlsTest now asserts the hidden default for isMacOs = false as well (ChromeDensityControlsTest.kt:16-23). On non-macOS there is no native toolbar, so browser_url never renders there; with both the top bar and the bottom bar hidden by default, I could not determine from the diff where a Windows/Linux user reads the current URL on a fresh install. Worth verifying.

Also note this default change is unrelated to the terminal-bridge feature; bundling it makes the behavioural regression easy to miss in review and hard to revert independently.

---

Correctness concerns (lower confidence)

4. The call overlay is gated on native title-bar availability

TerminalCallOverlay.kt:18 returns early unless isHosted(windowId), and the only writer of that flag is NativeTerminalHostAvailability (NativeTerminalTitleActions.kt:27-30), which is invoked from the nativeReady branch of SidebarTitleBar (BossAppScaffold.kt:1125). So the window-level call bar appears only when the macOS native sidebar title bar is ready. Any window where the native header is unavailable (non-macOS, title bar hidden, presumably focus mode) gets no overlay. If the plugin's own floating controls cover that case this is by design (TerminalTitleBarBridge.kt:27), but the coupling is implicit and the diff contains no assertion of it.

Related ordering nit: Content(windowId) is called before the DisposableEffect that sets hostWindow(windowId, true), so the provider is not composed on the first pass and only appears after the snapshot write triggers a second composition. Harmless but a guaranteed extra frame.

5. hostWindow has no reference counting

TerminalTitleBarBridge.kt:96-101 stores a plain boolean. If two compositions ever claim the same windowId (a transient overlap while a title bar moves between layouts, where the new node's effect can run before the old node's onDispose), the stale onDispose clears a live claim and the overlay/actions vanish until something recomposes. A counter or owner-keyed set would be robust; as written the invariant “exactly one host composition per window id” is undocumented and unenforced.

6. MacToolbarGroups details

• identifiers() (MacToolbarGroups.kt:17-18) maps each member to its group and distinct()s. If group members are interleaved with other trailing actions, the group is hoisted to the position of its first member and the other members' positions are silently dropped — toolbar order can change in ways the caller does not express. The spacer rule in MacSidebarToolbar.kt:88-91 (if (index > 0 && id != "split_horizontal")) now indexes the collapsed list, so spacing around split_horizontal changes whenever grouping collapses a preceding item.
• utility_controls uses setSelectionMode: 2 (momentary, MacToolbarGroups.kt:28), but update() still calls setSelected:atIndex: for every present member (:48-51) including momentary groups. Likely a no-op, but it is dead work and, if AppKit ever asserts on it, a crash inside a JNA callback.
• create() uses checkNotNull(...) (:22) — throwing from inside the AppKit itemForItemIdentifier callback path (MacSidebarToolbar.kt:137-140) is not a graceful failure mode; the surrounding code returns null for unknown identifiers instead.
• alloc/init at :21-24 with no release, and pointer(clazz("NSMutableArray"), "array") at :43 relies on an active autorelease pool. Both may match existing conventions in this file (the pre-existing items map has the same lifetime story), but the array case is new and is now also reached from SwingUtilities.invokeLater (MacSidebarToolbar.kt:208-217), where the enclosing pool depends on which thread the EDT actually is under JBR. Worth a second look from someone who knows this file's threading model — the items/groupedItems plain HashMaps are read from the AppKit callback and written from setActions.
• If pointer(clazz("NSMutableArray"), "array") returns null, setSubitems: is sent nil (:45); present is then recorded in previous anyway (:46), so the bad state is cached and never retried.

7. update() caching ignores subitem content

MacToolbarGroups.kt:41-47 rebuilds subitems only when the set of ids changes. That is correct only if the cached NSToolbarItems are refreshed elsewhere — MacSidebarToolbar.setActions does actions.forEach { (id, action) -> items[id]?.let { updateItem(it, action) } } (:68) before the group loop (:69-71), which appears to cover it, but only for ids present in items. If a group's membership first becomes non-empty after the toolbar was built, nothing in this diff shows the toolbar being told to re-query toolbarDefaultItemIdentifiers:, so a terminal opening mid-session may not surface terminal_controls until some other event forces a rebuild. Please confirm the existing dynamic-browser_* path covers this.

8. Icon fallback is dropped for all non-mcp terminal actions

NativeTerminalTitleActions.kt:11-13:

symbol = it.symbol.takeUnless { symbol -> symbol == "mcp" },
icon = if (it.symbol == "mcp") it.icon else null,

The ImageVector the plugin supplies is discarded for every action except the one whose symbol string is literally "mcp". Any renderer that falls back to icon (or any macOS version lacking that SF Symbol) shows nothing. The magic string is also an undocumented contract between host and plugin, repeated twice in one expression.

9. Cross-plugin takeover semantics

Content composes only providers.last() (TerminalTitleBarBridge.kt:69) and actions() only the last matching entry (:118-123). Any plugin that calls registerProvider later silently takes over the terminal title-bar surface and the window-level call overlay for every window, and the host composes that plugin's arbitrary composable inside its own title-bar composition with no runCatching guard — an exception from a plugin takes down the window's composition. That may be acceptable under this codebase's plugin trust model, but the bridge is a new public surface and the behaviour is only described as “most recently changed … wins” (:32).

10. Version pin comment is stale / release ordering

gradle/libs.versions.toml:181-184 bumps boss-plugin-api to 1.0.95 while the comment above still reads “1.0.94 adds the shared BrowserTitleBarBridge. Release the API before merging this pin.” Update it to describe 1.0.95 (TerminalTitleBarBridge), and confirm 1.0.95 is actually published — otherwise this merge breaks dependency resolution for everyone. Also worth confirming the host mirror at plugin-platform/.../TerminalTitleBarBridge.kt and the API JAR declare the same FQN without a duplicate-class hazard (the file comment at :9 acknowledges the mirror but only explains the omitted marker annotation).

---

Test gaps

1. No coverage of the recomposition-loop path (§1). Add a test that publishes two equal-valued but distinct TerminalTitleBarAction instances for the same owner and asserts no observable change (e.g. via a snapshot observer / invalidation counter). As written, TerminalTitleBarBridgeTest only ever publishes the same hoisted instance.
2. No coverage of MacToolbarGroups.identifiers — it is pure Kotlin and trivially testable: ordering, distinct() collapsing of interleaved members, empty-membership behaviour, and the split_horizontal spacer interaction in MacSidebarToolbar.kt:88-91.
3. No test pins the "terminal_" prefix contract. NativeTerminalTitleActions.kt:11 produces terminal_${id} and MacToolbarGroups.members (:13) hardcodes terminal_sharing/terminal_call/terminal_mcp. Renaming either side silently un-groups the controls with no failure.
4. No test for the "mcp" symbol/icon special case (§8).
5. No test for the showBottomBar default flip on an existing settings file. ChromeMetricsTest (:97-102) and ChromeDensityControlsTest (:16-23) only assert the new default for a fresh object; neither exercises decode-of-an-old-file or a migration.
6. No test for TerminalCallOverlay — in particular the focused/unfocused branch swap (§2) and the isHosted gate (§4).
7. Bridge tests leak global singleton state. The finally in the first test (TerminalTitleBarBridgeTest.kt, the unregisterProvider/remove/hostWindow block) omits removeCallBar(actionsOwner), which is only cleaned by the DisposableEffect succeeding. Combined with the second test's opening assertFalse(isHosted("test-first")), these tests are order- and parallelism-sensitive against a process-wide object. Consider an @BeforeTest/@AfterTest reset hook on the bridge (test-only) rather than per-test finally blocks.
8. recomposer.cancel() without join()/close() in the first test may leave the recomposer coroutine outstanding within runTest; worth checking it does not produce flaky "job did not complete" failures.

---

Minor

• sidebarTitleActions becoming @Composable (BossAppScaffold.kt:1139) changes its call contract; verify there is no other (non-composable) caller.
• MacSidebarToolbar.kt:208-217 re-applies group selection before invoking onAction, so a click that genuinely toggles a service will visually revert and then re-select one recomposition later. The comment explains the menu case but not this flicker.
• TerminalTitleBarBridge is documented as “UI-thread-only” (:25) but nothing enforces it; unregisterProvider/remove are plausible plugin-unload callbacks that could run off the UI thread, where indexOfFirst + removeAt in put/remove (:141-149) is a racy read-modify-write on the snapshot list.