Skip to content

feat(ui): switch projects from the panel header - #20

Merged
kshivang merged 7 commits into
mainfrom
feat/project-switcher
Oct 6, 2026
Merged

kshivang merged 7 commits into
mainfrom
feat/project-switcher

Conversation

@kshivang

@kshivang kshivang commented Aug 26, 2026 •

Copy link
Copy Markdown
Contributor

What

The header's first line showed the open project's name as static text. It is now the control that changes the project.

  • Dropdown of recent projects: current one first with a checkmark, parent directory underneath (home collapsed to ~), lazily rendered within a 360dp maximum viewport.
  • Open Project... below a divider, wired to the picker the empty state already used.
  • Selecting a row calls ProjectDataProvider.selectProject, the same host call the top bar's project menu makes, so the switch belongs to the window: top bar name, sidebar panels and project-scoped tabs all follow it. Re-picking the open project is a no-op rather than re-running the host's project-selection flow to land where we already are.
  • BossPopup, not a plain Compose Popup: this panel sits beside browser tabs and Chromium composites over the Compose scene. Placed after the trigger row in a Column so AnchorBounds opens the menu under the header rather than over it.

Two defects on the picker path

Both made a folder chosen from Open Project... appear to do nothing.

The picker ran from inside the menu's own composition. Under HARDWARE rendering the menu is a heavyweight always-on-top AWT window (BossWindow provides LocalHeavyweightOverlays = true), and the host's picker parents its native dialog to whatever window is active:

// DesktopDirectoryPickerProviderImpl.kt
val owner = KeyboardFocusManager.getCurrentKeyboardFocusManager().activeWindow as? Frame
val dialog = FileDialog(owner, "Select Project Directory", FileDialog.LOAD)

Dismissing the menu and opening the picker in one event handed that modal dialog an owner the same frame disposes. The pick request now crosses back into the panel's own composition and runs a frame later, once the menu window is gone.

Trailing-separator paths named every such project "Unknown". macOS returns dialog.directory with a trailing separator when nothing inside the folder was selected, and PathUtils.name of that is "". Such a pick reached the host as ProjectData(name = "Unknown", path = "/dev/BossTerm/") - not the folder's name, and a path that did not match the recents entry for the same folder. New PathUtils.trimTrailingSeparator runs first, and the dropdown's dedupe key shares the helper so one folder cannot occupy two rows.

The picker path is now logged at INFO throughout. It is driven by a native dialog inside the host, so without those lines there is no way to tell a cancelled dialog from a missing provider from a callback the host ignored.

Validation

The PR now integrates the merged Files/Search/Git panel redesign: the project switcher sits in the shared header above all tabs. Project selection and the picker share a lightweight service. Project changes are sampled on IO in bounded 250ms bursts that stop on confirmation, with a 5-second idle poll and serialized updates. Picker completion starts the burst after a valid folder is submitted, including the FILES empty-state action. The UI does not assume that a host confirmation prompt was accepted.

CI=true ./gradlew test buildPluginJar --console=plain -Pkotlin.daemon.jvmargs=-Xmx2g passed against released API 1.0.87 (the declared minimum): 249 tests, zero failures/errors/skips. Regression tests cover path normalization, recents ordering/dedupe, provider failures, polling bounds/cancellation, asynchronous picker completion, and normalized MCP output. Independent review of the final fixes found no blocker. Provider read failures retain the last confirmed path and allow subsequent polling to recover; cancellation propagates. An unchanged initial composition skips the rapid burst.

The native popup's frame-deferred picker ownership, visual geometry, and keyboard interactions are not covered by automated Compose tests. No BOSS app launch, installation, or hot-reload was performed during this merge preparation. Earlier manual validation predates the integrated header.

The header's first line showed the open project's name as static text. It is
now the control that changes the project: a dropdown listing recent projects,
current one first with a checkmark and its parent directory underneath (home
collapsed to ~), plus "Open Project..." below a divider.

Selecting a row calls ProjectDataProvider.selectProject, the same host call the
top bar's own project menu makes, so the switch belongs to the WINDOW rather
than to this panel: the top bar name, the sidebar panels and every
project-scoped tab follow it. Re-picking the open project is a no-op instead of
re-running the host's project-selection flow to land where we already are.

Two defects on the picker path, both of which made a folder chosen from
"Open Project..." appear to do nothing:

- The picker ran from inside the menu's own composition. Under HARDWARE
  rendering the menu is a heavyweight always-on-top AWT window (BossWindow
  provides LocalHeavyweightOverlays = true), and the host's picker parents its
  native FileDialog to KeyboardFocusManager.activeWindow. Dismissing the menu
  and opening the picker in one event therefore handed the modal dialog an owner
  that the same frame disposes. The pick request now crosses back into the
  panel's own composition and runs a frame later, once the menu window is gone.

- The macOS picker returns dialog.directory WITH a trailing separator when
  nothing inside the folder was selected. PathUtils.name of that is "", so such
  a pick reached the host as a project literally named "Unknown", on a path that
  did not match the recents entry for the same folder. New
  PathUtils.trimTrailingSeparator runs first; the dropdown's dedupe key shares
  the same helper, so one folder cannot occupy two rows.

The picker path is logged at INFO throughout. It is driven by a native dialog
inside the host, so without those lines there is no way to tell a cancelled
dialog from a missing provider from a callback the host ignored.

Tests: 19 new (dropdown ordering and matching incl. Windows separators; picker
result handling). slf4j-simple joins the test runtime because BossLogger binds
slf4j in its static init, so any test reaching a log call died without it.
@claude

claude Bot commented Aug 26, 2026

Copy link
Copy Markdown

Review: feat(ui): switch projects from the panel header

Nice piece of work — the write-up is unusually good, and the two defects it documents (heavyweight-popup dialog ownership, trailing-separator paths) are the kind that normally get "fixed" by a retry loop rather than understood. Splitting the ordering/matching rules into a pure ProjectSwitcherEntries so they're testable without a composition is exactly the right seam, and the null-provider handling is textbook for this repo: nullable StateFlow, MutableStateFlow(emptyList()) fallback, warn-and-return on a missing picker and a missing callback, with a test for each.

Conventions check: Compose Multiplatform only (Material 2, consistent with CodebaseContent), no Android APIs, plugin.json correctly left alone, all four new files end with a newline. ✅

Below, roughly in order of how much I'd want them resolved before merge.


1. Please confirm minApiVersion still covers the new API surface

plugin.json declares apiVersion/minApiVersion 1.0.72, and this PR adds hard references to things I can't resolve from the PR alone:

  • ProjectDataProvider.recentProjects (CodebaseDynamicPlugin.kt:66)
  • BossPopup / BossPopupAnchoring (ProjectSwitcher.kt)

If any of them landed after 1.0.72, an install on a 1.0.72 host throws NoSuchMethodError/NoClassDefFoundError. Note where the recentProjects read sits: inside the panel factory lambda, so the failure takes down panel construction, not just the dropdown — the panel disappears rather than degrading. That's the one outcome AGENTS.md rules out.

This repo already has the pattern for API newer than the floor — TreeScanner.supportsHiddenEntries gating the hidden-files toggle. Either bump minApiVersion, or (if you want to keep the floor low) guard the recentProjects read the way supportsShowHidden is guarded.

2. Version not bumped in build.gradle.kts

Still version = "1.5.8". Pushes to main trigger the release workflow and publish to the plugin store, and this is a user-visible feature — looks like it wants a bump (1.6.0?). Flagging in case it was just missed rather than deliberate.

3. The header label didn't get the trailing-separator fix

CodebaseContent.kt:113:

val projectName = projectPath?.let { PathUtils.name(it) }?.ifEmpty { "Project" } ?: ""

pickDirectory and ProjectSwitcherEntries.build both normalize now, but this doesn't. So for a project whose recorded path carries a trailing separator, the header reads "Project" while the dropdown row for that same project correctly reads "BossTerm" — the two labels for one project disagree.

That isn't hypothetical: the "Unknown" / "/dev/BossTerm/" entries the old bug already wrote into users' recents files are exactly this shape, and they'll keep arriving as context.projectPath after a switch. One-liner:

val projectPath = getProjectPath()?.let { PathUtils.trimTrailingSeparator(it) }

4. Selecting a dropdown row still hands the host an un-normalized path

ProjectSwitcherEntry.path is verbatim by design, and ProjectSwitcher passes it straight to viewModel.selectProject(entry.name, entry.path). So the defect this PR fixes on the picker path is still reachable from the menu: click the legacy "/dev/BossTerm/" row → the host re-records the trailing-separator path → it comes back as context.projectPath → §3 fires, and the duplicate shape survives in recents indefinitely. ProjectSwitcherEntriesTest pins this as intended behaviour ("the host's own string back rather than one this file rewrote").

I'd argue the verbatim contract in PathUtils' header is about tree/selection keys and comparisons, not about what crosses into selectProject — and the cleanest place to normalize is one line in CodebaseViewModel.selectProject, which then covers both the picker and the dropdown by construction:

fun selectProject(name: String, path: String) {
    val normalized = PathUtils.trimTrailingSeparator(path)
    ...
}

That also makes the isCurrent match and what the host stores agree permanently, which is what makes the dedupe self-healing rather than cosmetic.

5. trimTrailingSeparator strips whitespace too, and a Windows drive root

val trimmed = path.trim()

Trailing spaces are legal in directory names on macOS and Linux, so /Users/k/My Project — a real folder the picker can legitimately return — becomes a path that doesn't exist, and the panel then loads an empty tree with no error to explain it. The trim() isn't buying much either: the case it's protecting against is a blank result, which reads more honestly at the call site (picked?.takeIf { it.isNotBlank() }) while the helper touches separators only.

Separately, the KDoc's "a filesystem root keeps its single separator" holds for POSIX but not for a Windows drive root: trimTrailingSeparator("C:\\", '\\') returns "C:", and File("C:") is the current directory on C:, not C:\. Unlikely as a project root — mostly worth narrowing the doc claim.

6. Does the panel actually recompose after a switch?

val projectPath = getProjectPath() (CodebaseContent.kt:112) is a plain function call read during composition — nothing subscribes to it. Pre-existing, but this PR is the first thing that makes the panel the cause of a project change, so it becomes newly reachable: switch from the dropdown, and if the host doesn't recreate/recompose the panel, the header name and the file tree both stay on the old project.

In practice the collected recents flow will usually emit (host prepends the new project) and drag a recomposition along with it — but that's an accident, not a mechanism, and it doesn't fire when the selected project is already at position 0 in recents. Your Refreshed open sidebar panels ... panels=1 line suggests the host does recreate panels; worth confirming that's guaranteed, since the fallback is silent staleness.

7. pickPending reset is load-bearing on there being no suspension point

LaunchedEffect(pickPending) {
    if (!pickPending) return@LaunchedEffect
    withFrameNanos { }
    pickPending = false
    onOpenProject()
}

Correct today — the write invalidates the composition, but nothing suspends before onOpenProject(), so the key-change cancellation can't land in between. It's fragile in a way that won't announce itself: add any suspend call between those two lines and the picker silently stops opening, which is the exact symptom this code exists to fix. Reordering removes the dependency entirely:

withFrameNanos { }
try { onOpenProject() } finally { pickPending = false }

8. UI / perf nits

  • The 9-row cap isn't 9 rows. heightIn(max = MenuRowHeight * 9) = 306dp, but ProjectRow is two lines (13sp + 10sp + 10dp padding ≈ 42dp), so heightIn(min = 34.dp) isn't what determines height — you get ~7 rows with the 8th clipped mid-row. It scrolls, so nothing breaks, but given "popup geometry has not been eyeballed on screen" it's worth a look; deriving the cap from the real row height (or LazyColumn with a max height) would make the number mean something.
  • verticalScroll composes every row. Fine for realistic recents sizes, but the comment right above says the host's list "has no bound" — if that's literally true, LazyColumn is the safer read.
  • Keyboard nav. clickable(indication = null) gives the rows no focus and no semantics role, so the menu is mouse-only despite focusable = true on the popup. Not a regression (the header was static text before), just the obvious follow-up.
  • The current row still hovers like it's actionable but is deliberately inert. Dimming it or dropping the hover background would match the "already here" intent.

9. Tests

19 new tests, well-chosen names, and the separator injection means Windows behaviour is actually pinned rather than assumed — that's the right call. Two gaps worth naming out loud:

  • The trickiest part of the PR is the least covered. The frame-deferred pick is the actual fix for defect Theme-aware UI: Operator's Console design system #1, and it's verified only by one manual hot-reload. There's no compose-ui-test dependency in the repo at all, so I'm not asking for a test — just suggesting the PR body say so directly, since "79 passing, 19 new" reads like the popup path is covered when it isn't.
  • ProjectPickTest leaks a ViewModel per test. Each viewModel(...) starts fileWatcher and watchSetSyncJob and never calls dispose(). Harmless (the loops just delay) and consistent with DegradationContractTest, but a try/finally { dispose() } would honour the ViewModel's own contract — the one CodebaseContent's DisposableEffect is careful about.

10. Minor

  • recentProjects has no default on CodebaseComponent/CodebaseContent. = null would keep the signatures source-compatible and make "host without a ProjectDataProvider" the default rather than something each call site has to remember.
  • matchKey normalizes only the platform separator, so a /-separated entry on Windows won't dedupe against its \ twin. Consistent with PathUtils' stated philosophy — noting it only so the dedupe guarantee isn't over-read.
  • slf4j-simple as testRuntimeOnly with the explanatory comment: good, and the right scope.

Nothing here is structural — §1 and §2 are the ones I'd want answered before merge, §3/§4 are a two-line normalization that closes the loop on the bug the PR is already fixing, and the rest is polish.

@kshivang

Copy link
Copy Markdown
Contributor Author

@claude Please review this PR against its latest head commit. Check correctness, regressions, security, and test coverage, and report any actionable findings so they can be fixed before merge.

@claude

claude Bot commented Sep 14, 2026

Copy link
Copy Markdown

Review: feat(ui): switch projects from the panel header

Nice change, and the write-up made reviewing it much easier — the two picker defects are real, and the root-cause notes (native FileDialog owner vs. the heavyweight menu window; PathUtils.name of a trailing-separator path) are the kind of thing that would otherwise be re-debugged from scratch later. Splitting the ordering/dedupe rules into a pure ProjectSwitcherEntries with tests is the right seam.

Convention checks all pass: providers are null-safe on every new path (directoryPickerProvider, onSelectProject, recentProjects each degrade rather than crash), the version moved only in build.gradle.kts, plugin.json is untouched, no Android-only APIs, and every changed file ends with a newline. Note I couldn't run Gradle in this sandbox, so the 79/19 test counts are unverified here.

A few things worth addressing before merge.

1. The switch is invisible for up to 5 seconds

projectPath is sampled by the poll loop (PROJECT_POLL_MS = 5_000L, CodebaseComponent.kt:372). That was fine when the header was static text describing a switch made elsewhere, but the dropdown is now the switch itself: the menu closes instantly, then the header name, the ~/parent line, the checkmark and the FILES tree all keep showing the old project until the next tick. Re-opening the menu inside that window shows the previous project as current, and the one just picked as an ordinary selectable row.

Cheapest fix is to sample optimistically at the call site: in onSelect, set projectPath = PathUtils.trimTrailingSeparator(it.path) and run the same gitViewModel.onProjectChanged() / searchViewModel.clear() reset the poll loop does, then hand off to the host and let the poll merely confirm it. Worth factoring that reset into a local fun applyProject(path: String?) so the two paths can't drift.

2. Clicking the current project does nothing at all

ProjectRow passes enabled = !entry.isCurrent into MenuRow, which forwards it to clickable(enabled = ...) (ProjectSwitcher.kt:455, :528). The click on the current row is therefore swallowed before onSelect runs — which makes the guard in ProjectSwitcher:

expanded = false
if (!entry.isCurrent) onSelect(entry)

unreachable for isCurrent rows, and the menu stays open with no feedback. Clicking the already-checked row is a natural "never mind" gesture in a menu like this. I'd drop enabled from ProjectRow and let the existing expanded = false + if (!entry.isCurrent) guard do exactly what its comment says (dismiss, don't re-run the host flow). Either way, one of the two guards should go.

3. A full CodebaseViewModel as a project-selection service

CodebaseComponent.Content() now builds a second CodebaseViewModel with fileSystemDataProvider = null purely to reach selectProject/pickDirectory (CodebaseComponent.kt:114). Its init unconditionally starts a FileWatcherService poll job and a watchSetSyncJob on the plugin-lifetime scope and allocates FileIndexCache(maxSize = 1000) plus a TreeScanner — none of which can do anything with a null provider, and all of it duplicated alongside the tree VM CodebaseContent already owns. The lifecycle is handled correctly (DisposedWhenGone covers the abandoned-composition path), so this isn't a leak, just machinery that shouldn't need to exist.

Neither method depends on VM state. Extracting them into a small internal class ProjectSelection(private val picker: DirectoryPickerProvider?, private val onSelectProject: ((String, String) -> Unit)?) would let the header hold ~25 lines instead of a tree ViewModel, needs no dispose(), and ProjectPickTest would only lose its viewModel()/@AfterTest scaffolding. CodebaseViewModel can delegate to it so the FILES empty-state button keeps the identical fixed behaviour.

4. Does ProjectDataProvider.recentProjects exist at minApiVersion 1.0.87?

build.gradle.kts compiles against the newest sibling API jar while plugin.json still gates installs at 1.0.87, and the comment at build.gradle.kts:39-41 spells that contract out explicitly ("Compiling needs api >= 1.0.66 …; plugin.json's minApiVersion gates installs to hosts whose runtime api layer has them"). recentProjects is read statically in the panel factory (CodebaseDynamicPlugin.kt:139), so on a host at the declared floor a missing member is a NoSuchMethodError thrown while the panel is being constructed — not a graceful degrade. If it landed after 1.0.87, bump minApiVersion or guard it the way supportsShowHidden guards the hidden-entry overloads. If it predates 1.0.87, ignore this.

5. Smaller items

  • build() runs on every recomposition — ProjectSwitcherEntries.build(recents, projectPath) (CodebaseComponent.kt:234) allocates a LinkedHashMap, a sorted list and fresh ProjectSwitcherEntry instances on every pass, and the new list identity re-diffs the LazyColumn even when nothing changed. remember(recents, projectPath) { ... } makes it free.
  • locationLabel re-implements collapseHome — the home.isEmpty() / == home / isNestedUnder ladder in ProjectSwitcherEntries.locationLabel is behaviourally identical to the existing internal fun collapseHome (CodebaseComponent.kt:353), same package. collapseHome(PathUtils.parent(matchKey(path, separator), separator), separator, homeDirectory) keeps one home-collapsing rule for the panel instead of two that can drift.
  • key(projectPath) looks redundant — CodebaseContent already has LaunchedEffect(projectPath) { clearCache(); loadFileTree(...) } (CodebaseContent.kt:127) and recomposes when the polled state changes. The key additionally discards the VM, its cache, and scroll/expanded/selection state plus any open dialog. If that hard reset is the intent, a one-line comment saying so would stop someone deleting it as dead weight; otherwise it can go.
  • trimTrailingSeparator — val trimmed = path is a no-op alias; use path. Also the drive-root guard is conditioned on separator == '\\', so trimTrailingSeparator("C:/", '/') returns the drive-relative "C:"; dropping the separator condition makes the guard separator-agnostic at no cost.
  • A throwing picker crashes the composition — the LaunchedEffect uses try { onOpenProject() } finally { pickPending = false } (ProjectSwitcher.kt:89), so a host provider that throws propagates out of the composition. A catch that logs would match the never-crash posture the rest of this path already takes.
  • Menu always renders at max width — widthIn(min = 240.dp, max = 420.dp) over rows that fillMaxWidth() pins the surface at 420 dp regardless of content. Fine if intended; Modifier.width(IntrinsicSize.Max) would size it to the longest row.
  • Row cap — the PR text says "capped at 9 rows"; heightIn(max = 360.dp) against a 34 dp minimum row height is closer to 10.
  • Import order — androidx.compose.runtime.key is inserted before .Composable, the ProjectData/StateFlow/collectAsState block lands after com.arkivanov.decompose.ComponentContext, and androidx.compose.ui.semantics.Role sits inside the foundation block in ProjectSwitcher.kt.
  • Test placement — the two pure PathUtils.trimTrailingSeparator cases in ProjectPickTest belong in PathUtilsTest beside the other separator cases. And name of a trimmed path is never empty for a real directory reads more directly as two assertEquals than as assertNull(... .firstOrNull { it.isEmpty() }).
  • Case-insensitive filesystems — dedupe and isCurrent match exactly, so a recents list holding both /dev/Boss and /dev/boss yields two rows on macOS and possibly no current row. Probably not worth handling, but it depends on what the host records.

None of these block the core fixes, which are sound. (1) and (2) are the ones users will actually notice; (4) is worth a quick confirmation because the failure mode there is a panel that won't construct.

@kshivang

Copy link
Copy Markdown
Contributor Author

@claude Please review this PR against its latest head commit. Check correctness, regressions, security, and test coverage, and report any actionable findings so they can be fixed before merge.

@claude

claude Bot commented Sep 14, 2026

Copy link
Copy Markdown

Review: feat(ui): switch projects from the panel header

Nice change — the PR description does a lot of the reviewer's work for it, and the two picker defects are diagnosed properly rather than papered over. Extracting ProjectSelection so the header and the FILES empty state share one boundary, and keeping ProjectSwitcherEntries pure so the ordering/dedupe rules are testable without a composition, are both the right seams. Comments below, roughly by severity.

1. The project poll went 5s → 250ms permanently

CodebaseComponent.kt:362

/** Follow confirmed host changes promptly; selection may be asynchronous or cancelled. */
private const val PROJECT_POLL_MS = 250L

This replaces a constant whose doc comment explicitly argued the other way ("1s only added IPC churn — 5s is indistinguishable in practice"). That rationale was deleted rather than answered, and the cost is real: plugin.json declares "isolationMode": "out-of-process", so getProjectPath() is a boundary call, this is a while (true) loop on the composition dispatcher, and it runs per panel instance per window for the entire life of the panel — 20× the previous rate, forever, to make one user-initiated event feel prompt.

The good news is this PR already wires the push signal it needs. recentProjects is a StateFlow the host mutates on selectProject, so a second effect gets promptness for free:

LaunchedEffect(Unit) {
    recentsFlow.collect { /* host touched projects — re-read now */ refreshProjectPath() }
}

…with PROJECT_POLL_MS back at 5s as the safety net for switches that don't move recents. A burst-poll (250ms for ~5s after ProjectSelection.selectProject/pickDirectory fires, then decay to 5s) would work too. Either way, please keep the IPC-cost note in the constant's KDoc so the next person doesn't re-litigate it.

2. codebase_select_project still has the exact bug this PR fixes

CodebaseMcpTools.kt:121-122

val name = args.string("name") ?: PathUtils.name(path)
p.selectProject(ProjectData(name = name, path = path))

This is the second door into selectProject and it didn't get the fix. An agent passing /dev/BossTerm/ lands name = "" (worse than "Unknown" — an empty row) and the untrimmed path reaches the host, which is precisely the "one folder, two recents entries" the dropdown's dedupe key is working around. Since ProjectSelection now exists, route this through it — or at minimum apply PathUtils.trimTrailingSeparator before both the name fallback and the ProjectData. Normalizing at every entry point is what stops the dropdown from needing the dedupe in the first place.

3. A test doesn't cover the case its name claims

ProjectPickTest.kt

fun `a picker that returns only separators selects nothing`() {
    selection(FakePicker("   "), recorder).pickDirectory()

" " is three spaces, not separators — this exercises the isBlank() guard, not the separator path. The real separator-only case is untested and behaves differently: trimTrailingSeparator("/") returns "/", which isn't blank, so PathUtils.name("/").ifEmpty { path } selects a project literally named "/" rooted at the filesystem root. Either rename this to ...only whitespace... and add the genuine case, or decide the root is something the picker should refuse. Given the whole point of the change is "no more projects named Unknown", a project named / is worth an explicit decision.

4. The one-frame picker deferral is a timing bet (fragility, not a defect)

ProjectSwitcher.kt:111-121. The diagnosis is right and the comment is excellent, but a single withFrameNanos { } assumes the heavyweight overlay window is disposed by the next Compose frame. Two consequences worth weighing:

  • If the panel leaves composition inside that window, the LaunchedEffect is cancelled and the pick is silently dropped — the same "click did nothing" symptom this PR is fixing.
  • If disposal ever takes two frames on some host/renderer combination, the original bug returns with no signal.

You have instrumented the rest of this path at INFO for exactly this reason; the deferred dispatch deserves a line too, so a future "nothing happened" report distinguishes "never dispatched" from "dispatched and cancelled". Gating on observed window state rather than a frame count would be sturdier still, if the API allows it.

5. Smaller things

  • Header name vs. dropdown name disagree. The trigger renders PathUtils.name(projectPath) (CodebaseComponent.kt:277) while the dropdown's current row renders the host's ProjectData.name. If the host ever records a display name that is not the directory basename, the same project shows under two labels a few pixels apart.
  • Stray blank lines before closing braces: ProjectSelection.kt:71 and ProjectSwitcherEntries.kt:79.
  • ProjectPickTest.kt: the separator() helper sits between two @Test functions; move it down with the other helpers.
  • Accessibility: MenuRow sets role = Role.Button but the trigger Row does not, and neither is keyboard-activatable. The PR body already flags keyboard as uncovered — worth a follow-up issue rather than blocking here.
  • CodebaseTooltip wraps the whole header row, which now contains the dropdown trigger and the anchored popup. Worth one manual check that the path tooltip does not appear over the open menu.

What I checked and found fine

  • Version bumped only in build.gradle.kts (1.6.1 → 1.7.0); plugin.json untouched, so the processResources sync stays authoritative. ✅
  • Compose Multiplatform APIs throughout; no Android-only imports. ✅
  • Null-provider handling is genuinely careful, not just present: recentProjects == null degrades to an empty flow so the menu offers only the picker; null picker and null selection callback each log and no-op; all three are covered by tests, alongside providers that throw.
  • key(projectPath) around CodebaseContent is safe — I checked that CodebaseViewModel.dispose() cancels only watchSetSyncJob and the file watcher, not the shared plugin-lifetime scope, and that DisposableEffect(viewModel) fires on the key change. A dispose() that cancelled the shared scope would have broken every switch after the first.
  • trimTrailingSeparator handles the cases that matter: multiple trailing separators, POSIX root, C:\ and C:/// drive roots staying absolute, and significant trailing spaces in directory names.
  • All Kotlin files end with a newline. ✅

Caveats on this review

I could not build or run the suite here — build.gradle.kts resolves the API jar from a sibling ../boss-plugin-api checkout or build/downloaded-deps/, and neither is present in this environment. So the CI=true ./gradlew test buildPluginJar result in the description is taken at its word.

Relatedly, ai.rever.boss.plugin.ui.BossPopup and BossPopupAnchoring are first used in this repo by this PR, and I could not inspect the 1.0.87 jar to confirm they exist at the declared minApiVersion. If CI really does compile against the released 1.0.87 artifact, that is covered; if it resolves latest instead, it is worth a deliberate check before release, since the failure mode is a NoClassDefFoundError on the oldest supported host rather than a build break.

Nothing here is a blocker except (2), which is a two-line fix.

@claude

claude Bot commented Sep 14, 2026

Copy link
Copy Markdown

Review: switch projects from the panel header

Solid work. The two picker defects are well diagnosed, the description explains why BossPopup and the frame-deferred pick are needed rather than just asserting it, and the logic worth pinning (ProjectSwitcherEntries, ProjectSelection, trimTrailingSeparator) was deliberately pulled into pure, testable units. 19 tests for a UI change is the right instinct.

Convention checks all pass:

  • Compose Multiplatform only, no Android APIs.
  • Null-safe providers: recentProjects = projectDataProvider?.recentProjects, and ProjectSelection warns-and-no-ops on both a null picker and a null callback, with a test for each.
  • Version bumped only in build.gradle.kts (1.6.1 -> 1.7.0); plugin.json untouched at 0.0.0-dev.
  • All new/changed .kt files end with a newline.
  • slf4j-simple correctly scoped testRuntimeOnly, so it does not enter the shipped JAR.

Findings below, roughly in the order I would act on them. I could not compile locally (no network for the API jar), so this is from reading rather than running. Nothing blocks; 1, 3 and 4 are what I would fix before merge.

1. The 250 ms burst never exits early, and samples across the process boundary on the UI dispatcher

CodebaseComponent.kt:222-228:

LaunchedEffect(recents, projectRefreshRequest) {
    repeat(20) {
        refreshProjectPath()
        delay(250L)
    }
}

All 20 iterations run unconditionally, including after the path has already changed, so one project switch costs 20 getProjectPath() calls over 5 s even though the answer arrived on iteration 1. The manifest declares "isolationMode": "out-of-process", so getProjectPath() is potentially an IPC hop, and LaunchedEffect runs on the composition dispatcher (the EDT on Compose Desktop). The comment you replaced made exactly this argument in the other direction:

so 1s only added IPC churn - 5s is indistinguishable in practice

4 calls/second on the UI dispatcher is a real step up from that. Two cheap fixes, both worth doing:

fun refreshProjectPath(): Boolean {
    val current = getProjectPath()
    if (current == projectPath) return false
    projectPath = current
    gitViewModel.onProjectChanged()
    searchViewModel.clear()
    return true
}

LaunchedEffect(recents, projectRefreshRequest) {
    repeat(20) {
        if (refreshProjectPath()) return@LaunchedEffect
        delay(250L)
    }
}

plus withContext(Dispatchers.IO) { getProjectPath() } for the sampling itself, so a slow host reply cannot stutter the panel. A fake getProjectPath counting invocations is the cheapest way to pin the bound as this code moves.

2. The picker burst is spent while the dialog is still open

CodebaseComponent.kt:239-241:

onOpenProject = {
    projectSelector.pickDirectory()
    projectRefreshRequest++
},

pickDirectory() returns as soon as the native dialog is up, so the counter bumps immediately and the whole 5 s burst elapses while the user is still browsing folders. It recovers (the recents emission restarts the burst, and the 5 s idle poll backstops it) but the burst at this call site is dead weight today, and it quietly depends on the host adding the pick to recents. Bumping from the result path would make intent match behaviour: an optional onSelected: () -> Unit on ProjectSelection.pickDirectory / selectProject that the panel hooks up.

3. ProjectSwitcher bypasses the panel design tokens

The one I would most like changed, because CodebasePalette documents the exact failure mode the new code walks into (CodebaseUi.kt:122):

and the light themes (Daylight) work, which a hardcoded Color.White.copy(alpha = ...) wash would not

ProjectSwitcher.kt:109 and :280 are that wash, spelled with BorderColor:

BossThemeColors.BorderColor.copy(alpha = 0.45f)

A token already exists for every one of these:

ProjectSwitcher.kt token
BorderColor.copy(alpha = 0.45f) (:109, :280) CodebasePalette.Hover (= BossColors.contextMenuHover)
Surface(color = BossThemeColors.SurfaceColor) (:178) CodebasePalette.Raised (= BossColors.contextMenuBackground)
BorderStroke(1.dp, BossThemeColors.BorderColor) CodebasePalette.BorderStrong (= BossColors.contextMenuBorder)
14.sp (:124), 13.sp (:233, :264), 10.sp (:242) CodebaseMetrics.PrimaryText / SecondaryText / MetaText
MenuRowHeight = 34.dp (:55) CodebaseMetrics.RowHeight (22.dp) / TouchTarget
Modifier.size(18.dp) (:119), 16.dp CodebaseMetrics.Glyph (14.dp)

:124 is also an undocumented visual change: the project name went from CodebaseMetrics.SecondaryText (12.sp, SemiBold, letterSpacing = 0.2.sp) to a bare 14.sp Medium. That is a noticeable bump on the first line of a panel whose list rows are 22 dp, and the description does not mention it. If intentional, say so; otherwise it is an accidental regression in header density.

Smaller point while you are in there: the header row now carries Icons.Rounded.Inventory2 and the switcher Icons.Outlined.FolderOpen side by side. Two container/folder glyphs on one line read as noise; one can go.

4. Dedupe and current-project matching are case-sensitive

ProjectSwitcherEntries.matchKey normalizes trailing separators only. On Windows, and on the default case-insensitive APFS volume on macOS, C:\Dev\Boss and C:\dev\Boss are the same directory, but they occupy two rows and neither gets the checkmark. Same class of bug as the trailing-separator one you just fixed, and build already takes an injectable separator, so it is testable on any OS:

private fun matchKey(path: String, separator: Char): String =
    PathUtils.trimTrailingSeparator(path, separator)
        .let { if (separator == '\\') it.lowercase() else it }

Or normalize once at the host boundary. Either way the existing windows paths dedupe and match on their own separator test is the natural place to pin it.

5. pickPending as a Boolean can leave "Open Project..." permanently dead

ProjectSwitcher.kt:83-97. Cancellation is handled (finally runs) but withFrameNanos { } at :91 suspends until the composition frame clock ticks. If that clock is ever paused while the effect is live, the coroutine neither completes nor cancels, pickPending stays true, and since LaunchedEffect(pickPending) restarts only on a value change, every later click sets true -> true and does nothing, not even a log line. A monotonic token sidesteps the question:

var pickRequest by remember { mutableStateOf(0) }
LaunchedEffect(pickRequest) {
    if (pickRequest == 0) return@LaunchedEffect
    withFrameNanos { }
    logger.info(LogCategory.FILE, "Dispatching deferred project picker after popup dismissal")
    onOpenProject()
}

Low probability, but the failure mode is "the row is dead until the panel is reopened", the exact symptom this PR set out to kill.

6. CodebaseViewModel.selectProject is dead code

CodebaseViewModel.kt:568 has no callers: the header goes through the ProjectSelection in CodebaseComponent, and the FILES empty state only uses pickDirectory(). Drop it, or route the header through the ViewModel and drop the second ProjectSelection.

Relatedly there are now two ProjectSelection instances per panel. It is stateless so nothing breaks, but the empty-state button and the header row reach the same host call by two different paths; hoisting one would make the boundary singular, which was the stated point of extracting the class.

7. Accessibility / keyboard

Acknowledged as untested, so just naming the gaps: menu rows are clickable(indication = null) with no focus ring and no arrow-key traversal, so the menu is mouse-only. And contentDescription = "Switch project" sits on the chevron (:134), so a screen reader announces it on a decorative glyph while the row is the actual button. contentDescription = null on the chevron, plus a semantics label and stateDescription for expanded/collapsed on the trigger Row, would be more correct.

8. Logging volume

Every selection logs an absolute path at INFO. The justification for landing it that way is sound, but once confirmed in the field consider dropping the success-path lines to DEBUG and keeping WARN for the failure branches. INFO on a hot user action fills the host log, and absolute paths are about the most user-identifying strings this plugin handles.

Follow-ups, out of scope here

  • BossPopup / BossPopupAnchoring are new to this plugin and plugin.json keeps minApiVersion: 1.0.87. CI building against the released 1.0.87 jar should cover it, but worth confirming explicitly that both symbols exist at 1.0.87 and not in a later API, since the manifest is what the host gates loading on.
  • By this PR own JxBrowser reasoning, the three Material DropdownMenu call sites in GitTabUi.kt (:682, :793, :1371) carry the same latent draw-behind-the-page bug that motivated BossPopup here. Worth a tracking issue.
  • Geometry note on BossPopup placement: the zero-size probe sits inside the inner Column of the header, between the name row and the path Text. So AnchorBounds anchors below the name, not below the whole header, and the menu should overlap the home-collapsed path line rather than clearing it. If that is what you saw and accepted, fine; if the intent was "under the header", the probe wants to be a sibling of the header Row.

@kshivang

Copy link
Copy Markdown
Contributor Author

Merge-preparation handoff (implementation stopped at user request): current prepared head is 9a589b8af106f42885c19110db4a105dc5c14ac2. The latest known GitHub test run passed. The previously queued local test/JAR-build process has since exited successfully, but its final artifacts/results have not been re-inspected. The last fully inspected minimum-API run was af207c5: 235 tests and JAR build passed against API 1.0.87.

The actionable findings in Claude’s last inspected review were addressed in 9a589b8 (bounded polling, MCP path normalization, root/blank-path tests, consistent labels and deferred-picker logging). Remaining verification: inspect the final local results and any subsequent Claude review output, then manually verify native popup dismissal/picker ownership, tooltip geometry and keyboard interactions. No known additional actionable defect from the completed Codex self-review; no app launch or plugin installation performed. No merge performed.

@claude

claude Bot commented Oct 6, 2026

Copy link
Copy Markdown

Review: feat(ui): switch projects from the panel header

This is a well-scoped PR and the description is very good. Both picker defects (the native dialog getting a disposed popup window as its owner, and trailing separators producing projects named "Unknown") are diagnosed precisely, and both fixes are small and land in the right place. Moving the picker and selection logic into ProjectSelection, and keeping ProjectSwitcherEntries pure, makes the behaviour easy to test. The new tests cover the edge cases that matter.

BOSS conventions ✅

  • Version: bumped only in build.gradle.kts (1.6.1 → 1.7.0). plugin.json is not touched.
  • Null providers: handled. recentProjects falls back to an empty MutableStateFlow. A null picker or null onSelectProject logs a warning and does nothing. Exceptions thrown by the host are caught, and CancellationException is rethrown. Tests cover all of these cases.
  • Compose: only Compose Multiplatform / Material APIs. Using BossPopup with AnchorBounds instead of a plain Popup is correct for a panel that sits next to JxBrowser tabs.
  • Newlines: every new Kotlin file ends with a newline.

Suggestions

1. The header now shows two icons (visual).
CodebaseProjectHeader still draws Icons.Rounded.Inventory2 before the column (CodebaseComponent.kt around line 311). ProjectSwitcher adds its own 18dp Icons.Outlined.FolderOpen inside that column, so the name row reads [box] [folder] Name ⌄. I'd remove the icon from either the header or ProjectSwitcher.

2. ProjectSwitcher doesn't follow the panel's design tokens.
The rest of the redesigned panel uses CodebasePalette and CodebaseMetrics. The old name Text used CodebaseMetrics.SecondaryText and SemiBold. The switcher hard-codes 14.sp, FontWeight.Medium, BossThemeColors.*, an 18dp icon and its own padding. That makes the header taller and visually different from the Files/Search/Git headers below it. Two other details changed too:

  • A missing project used to render the name in CodebasePalette.Muted. Now "No project" uses TextPrimary.
  • The menu rows use BossThemeColors.

I'd pass in CodebaseMetrics.SecondaryText and CodebasePalette.Foreground/Muted instead, or accept them as parameters.

3. The tooltip covers the clickable trigger.
CodebaseTooltip still wraps the whole header row, and that row now contains the dropdown trigger. Hovering the name to open the menu will also show the full-path tooltip, which may overlap the popup when it opens under the header. Consider wrapping only the path line (the second Text) in the tooltip.

4. The confirmation burst can expire while the picker is still open.
onOpenProject runs projectSelector.pickDirectory() and then immediately projectRefreshRequest++. The picker is an asynchronous native dialog, so the 20 × 250ms burst (about 5s) usually ends before the user picks a folder. In practice this is probably covered because the host updates recents after a pick, which restarts the burst through the LaunchedEffect(recents, …) key. If that's what the design relies on, the comment should say so. Otherwise, bump the request from inside the picker callback.

The same applies to the FILES empty-state button (CodebaseContent.kt:182 → viewModel.pickDirectory()). It never bumps projectRefreshRequest, so it relies entirely on recents changing or on the 5s idle poll.

5. Minor issues

  • CodebaseViewModel.selectProject(...) is new but nothing calls it. Consider removing it until something does.
  • codebase_select_project returns "Selected project $name ($path)." with the raw path. Reporting normalizedPath would match what was actually sent to the host.
  • ProjectSelection.selectProject normalizes the path, but pickDirectory has already trimmed it before calling. Trimming twice is harmless. The KDoc on ProjectSwitcherEntry says normalization happens "at the ViewModel boundary", but it now happens in ProjectSelection, so the comment should be updated.
  • trimTrailingSeparator: on Windows, a UNC prefix such as \\\\ collapses to \\. That's an unlikely input, but a one-line test would document the behaviour. Paths using / on Windows are not trimmed, which matches the class note.

Performance

Opening the panel and every recents emission now start a burst of up to 20 getProjectPath() calls on Dispatchers.IO, alongside the 5s idle loop. The Mutex prevents stale writes, and the burst stops at the first change, so the cost is bounded and acceptable. The LazyColumn with a 360dp cap and stable path keys is a good choice for an unbounded recents list.

Security

Nothing of concern. Paths come from the host's picker, the host's recents list, or MCP arguments, and are only handed back to ProjectDataProvider.selectProject. The MCP tool now rejects blank paths, which is an improvement.

Tests

The tests are thorough for the pure logic: trimming, dedupe and ordering, null or throwing providers, MCP normalization, and the bounds and cancellation of the polling loop. Adding slf4j-simple as testRuntimeOnly is the right fix for BossLogger failing to initialise in tests. As the PR says, the one-frame deferral of the picker and the popup geometry have no automated coverage, so they need manual testing on macOS under HARDWARE rendering before merge. They were last tested manually before the integrated header was added.


In short, the logic and tests look correct. I'd fix the duplicate icon and the token/sizing mismatch (items 1–2) before merging, since they are the visible problems in the redesigned header. The rest are polish.

@claude

claude Bot commented Oct 6, 2026

Copy link
Copy Markdown

Review: feat(ui): switch projects from the panel header

This is a solid PR. The pure parts are pulled out well (ProjectSwitcherEntries, pollForProjectChange, ProjectSelection), so the rules that matter can be tested without a composition. The comments explain why things are done, not just what they do. The two picker defects (the heavyweight popup owning the native dialog, and the trailing separator that produced "Unknown") are clearly diagnosed and fixed narrowly.

BOSS conventions ✅

  • Version: bumped only in build.gradle.kts (1.6.1 → 1.7.0). plugin.json is untouched.
  • Null providers: handled throughout. A null projectDataProvider gives a null onSelectProject and an empty recents flow, so the menu shows only "Open Project...". A null picker logs and returns. Exceptions thrown by the host picker or by selection are caught.
  • Compose Multiplatform only: no Android APIs. BossPopup with AnchorBounds is the right call next to JxBrowser tabs.
  • Trailing newlines: none of the changed files is missing one.

Potential issues / suggestions

  1. A 5-second burst runs every time a panel opens, and whenever recents change in any window (CodebaseComponent.kt, LaunchedEffect(recents, projectRefreshRequest)).

    • The effect also runs on first composition. At that point projectPath already equals getProjectPath(), so refresh() never returns true and all 20 reads (one every 250ms) happen for nothing.
    • The recents list is probably host-global. If so, opening a project in window A also starts a burst in every Codebase panel in windows B, C, and so on.
    • It's bounded, so this isn't a blocker. But since getProjectPath can cross IPC (per the new KDoc), consider skipping the first run (e.g. remember the previous recents/request value and return early when neither changed), or only bursting on projectRefreshRequest plus recents changes whose head differs from the current path.
  2. An exception from getProjectPath() would end the idle poll loop and could escape the composition. refreshProjectPath() now runs the getter on Dispatchers.IO inside a mutex, but nothing catches errors. If the getter throws (for example an IPC failure), the exception propagates out of the LaunchedEffect. The old code had the same exposure, but the PR now calls the getter far more often (bursts) and explicitly treats it as IPC-backed. Wrapping the read in a runCatching that logs and returns false would match the "never crash" rule the rest of the PR follows.

  3. key(projectPath) around CodebaseContent recreates CodebaseViewModel on every switch. This works: the existing DisposableEffect(viewModel) disposes the old one, and resetting the tree, selection and dialogs is the intent. Two small notes:

    • CodebaseContent's own LaunchedEffect(projectPath) reload is now mostly redundant with the key.
    • CodebaseContent still reads getProjectPath() directly on each recomposition. The outer key, by contrast, follows the sampled projectPath, which can lag by up to one poll interval. That's harmless, just worth knowing if the two ever disagree briefly.
  4. trimTrailingSeparator edge cases (low priority):

    • The drive-root branch also fires on POSIX for a path like a:/ and returns a:/. That's harmless, but the check could be limited to separator == '\\', or the comment could say it's intentional.
    • On Windows, a host string that uses forward slashes (C:/dev/Boss/) won't be trimmed when separator == '\\'. The class note says separator normalization is out of scope, so this is just a flag in case the host ever emits mixed separators in recents.
  5. Paths are normalized twice on the pick path: pickDirectory trims the path, then selectProject trims it again. It's idempotent and cheap, so this is cosmetic only.

Test coverage

Coverage is good: normalization (including Windows separators, roots and meaningful trailing spaces), ordering and dedupe, the current path missing from recents, poll bounds and cancellation, async picker completion, provider failures, and MCP normalization. Small nits:

  • throwing host selection does not crash caller could also assertFalse(...) on the return value. The refresh gating depends on that false.
  • The trimTrailingSeparator / PathUtils.name cases in ProjectPickTest would sit more naturally in PathUtilsTest.
  • As the description says, the frame-deferred picker, the popup geometry and keyboard handling are untested, so manual QA on macOS under HARDWARE rendering is worth doing before release.

Security

No concerns. The MCP codebase_select_project handler now rejects blank paths and normalizes the rest, which tightens it. Project paths are logged at INFO, which is fine for local diagnostics.

Overall this looks good to merge after a look at #1 and #2. Neither is a correctness blocker.

kshivang added a commit that referenced this pull request Oct 6, 2026
@claude

claude Bot commented Oct 6, 2026

Copy link
Copy Markdown

Review: project switcher in the panel header

This is a well-scoped PR. The two picker-path bugs are diagnosed clearly, and the comments explain why things are done a certain way, not just what the code does. I found no blockers. Notes below, most important first.

✅ BOSS conventions

  • Version: bumped only in build.gradle.kts (1.6.1 → 1.7.0). plugin.json is untouched. ✔️
  • Null-safe providers: onSelectProject is now null when there is no ProjectDataProvider, instead of a lambda that silently did nothing. recentProjects falls back to an empty MutableStateFlow. ProjectSelection logs and returns instead of crashing when the picker or the callback is missing, and it catches exceptions thrown by the host. ✔️
  • Compose Multiplatform only: BossPopup, LazyColumn, material icons. No Android APIs. ✔️
  • Trailing newlines: the diff has no "No newline at end of file" markers. ✔️
  • Lifecycle: key(projectPath) now throws away CodebaseContent on each switch. I checked that the old CodebaseViewModel is still released through the existing DisposableEffect(viewModel) { onDispose { viewModel.dispose() } }, so switching projects does not leak watcher jobs. ✔️

Potential issues / suggestions

  1. Path comparison in refreshProjectPath is not normalized (CodebaseComponent.kt). The check current != projectPath compares raw strings. The dropdown and the selection code both run trimTrailingSeparator now, but the host's getter might still return /a/b/ for a project restored from an older recents entry. In that case key(projectPath) and gitViewModel.onProjectChanged() see a change that isn't one. The current code handles this correctly, but comparing PathUtils.trimTrailingSeparator(...) on both sides would match the dedupe rule used everywhere else.

  2. Bursts fire on any change to recents. LaunchedEffect(recents, projectRefreshRequest) starts up to 20 provider reads, 250 ms apart, whenever recents emits. That includes changes made in other windows, or a recents update that comes with no switch at all. The burst is bounded and stops early on a confirmed change, so it is acceptable. Since the getter may go over IPC, consider triggering only when the recents head changes (recents.firstOrNull()?.path). That is the signal a selection happened.

  3. The one-frame delay is timing-dependent. withFrameNanos {} followed by the host's own invokeLater works today because the heavyweight popup window is disposed within one frame. If BossPopup ever disposes asynchronously, or the host picker stops hopping through invokeLater, the original "pick does nothing" bug comes back with no test failing. The PR description already says this path isn't covered by automated tests. A short note in the plugin API docs or an issue, asking for the host picker to own its parent window (e.g. the main frame, not activeWindow), would be the more durable fix.

  4. CodebaseViewModel.pickDirectory() is effectively unreachable. CodebaseComponent always passes onOpenProject, so the ?: { viewModel.pickDirectory() } fallback in CodebaseContent only runs for other callers. Those callers also miss the refresh burst, so a pick made through that path only updates on the 5 s idle poll. Either remove the fallback or have it accept onSelectionRequested too, so both paths behave the same.

  5. Logging level: ProjectSelection logs every pick and selection at INFO, including the full path. The reason is documented and makes sense for diagnosing the native dialog. Once the picker fix is confirmed in the field, consider dropping the "Opening…" and "Dispatching…" lines to DEBUG so normal use doesn't fill the logs.

  6. Nit (ProjectSwitcher): MenuRow/ProjectSwitcher use indication = null with only a hover background. That means there's no focus indication for keyboard users inside a focusable = true popup. Since the PR already says keyboard interaction isn't validated, a focus highlight (e.g. collectIsFocusedAsState on the same interaction source) would be a cheap improvement.

  7. Nit: ProjectSelection refers to kotlinx.coroutines.CancellationException by its fully qualified name in two places, while ProjectPathPolling.kt imports it. An import would be more consistent.

Performance

  • The mutex-serialized read with withContext(Dispatchers.IO) is a good change. The old loop called getProjectPath() on the composition dispatcher every 5 s.
  • ProjectSwitcherEntries.build is wrapped in remember(recents, projectPath), and the list is lazy and capped at 360dp, which is fine for unbounded recents.

Security

  • MCP codebase_select_project now rejects blank paths and normalizes before deriving a name, which is an improvement. As before, it passes any path to the host without checking that it exists or is a directory. That matches the earlier behavior, but it's worth confirming the host validates it.

Tests

The tests cover the pure logic well: path trimming including Windows drive roots and UNC, recents ordering and dedupe, polling bounds and cancellation, an async picker, and MCP normalization. Adding slf4j-simple as testRuntimeOnly is the right fix for the BossLogger static init. Suggested additions:

  • A case for item 1, where the getter returns a trailing-separator path equal to the current one.
  • trimTrailingSeparator with a UNC share root (\\server\share\) to pin that behavior.

Nice work overall. The header-as-switcher and routing through ProjectDataProvider.selectProject (so the whole window follows the switch) are the right design.

@kshivang
kshivang merged commit 54f2f13 into main Oct 6, 2026
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant