Repository navigation
fix(plugins): give a plugin's file panel an owner and its own directory mode (#551) - #552
Conversation
|
@claude Please review the current head of this PR against dev for correctness and meaningful regression coverage. Check plugin-context wrapper routing for #548; for #552 check native owner selection, nested directory mode restoration and native-peer disposal. Related #611 is being compared for consolidation by this group; please do not mutate other PRs. |
|
Claude finished @kshivang's task in 6m 50s —— View job Review: #552 head vs
|
| Provider | DefaultPlugin |
TrackingPluginContext |
SandboxedPluginContext |
|---|---|---|---|
filePickerProvider |
:868 |
:406 |
:240 |
directoryPickerProvider |
:893 |
:411 |
:247 |
So nothing is broken, but nothing pins it either, and this exact regression has shipped: docs/release-notes/v9.2.35.md:22-23 records directoryPickerProvider and projectDataProvider falling through to the interface default null in both wrappers, so "Open Project" and Evolve's "Browse" silently did nothing. AGENTS.md records the same failure for mcpToolRegistry, and states the three-wrapper rule as a rule — with no test behind it. TrackingPluginContextMcpTest is the only wrapper test and covers MCP registration, not forwarding.
A reflection test is ~15 lines and is the highest-value missing test in this area: enumerate PluginContext's provider vals and assert each is declared as an override in TrackingPluginContext and SandboxedPluginContext. Today a new provider silently returns null for every tracked and every sandboxed plugin, which is the shape of bug that reaches users as "the button does nothing." Out of scope for this PR, but it belongs to whichever of #548/#552/#611 the group consolidates into.
Regression coverage for #552
What's covered is good and correctly chosen: six focused tests on withDirectoryMode, including the two cases that are actually subtle (absent-stays-absent, reverse unwinding) and a throwing body. Modulo finding 6, they'd catch a regression in the restore logic.
What is not covered, in descending value:
- Nothing fails if someone writes
FileDialog(null as Frame?, ...)again — which is the defect this PR exists to fix. The repo already pins this kind of invariant against source (OsOpenArgumentsTestpins the CLI subcommand list,FileTypeCategoriesTestpins two extension sets, andNativeFileDialogsTestexists specifically to catch a dropped registration line). A test that scanscomposeApp/src/desktopMainforFileDialog(outsideOwnedFileDialog.ktagainst a shrinking allowlist would give the fix real teeth — and would have surfaced findings 1 and 7 on its own. - The owner branches are untested. Deleting
is Dialog ->fromownedFileDialogfails nothing, even though "the active window is aDialogwhenever a BOSS modal is up" is the stated motivation. Extractinginternal fun fileDialogOwner(active: Window?): Window?— pure, taking the window rather than reading the focus manager — makes the classification testable, and makes finding 5 checkable too. AGENTS.md's mutation-verification standard forpluginSectionAbsenceis the bar here. showModal's dispose-in-finallyis untested and can't be from a headless JVM as written, sinceFileDialog's constructor throwsHeadlessException. Fine to leave — but say so in the KDoc rather than letting the reader assume the "each of which has a test" claim coversshowModaltoo. It coverswithDirectoryModeonly.
Disposal summary
| Site | Owner | States mode | Disposes |
|---|---|---|---|
NativeFileDialogs |
yes | yes | yes |
FilePickerProviderFactory (mac) |
fixed here | fixed here | fixed here |
FilePickerProviderFactory (JFileChooser) |
fixed here | n/a | yes (internal to showDialog) |
DesktopDirectoryPickerProviderImpl |
Frame only |
writes, never restores | no |
platform/DesktopFilePicker |
no | writes, never restores | no |
platform/FilePicker ×2 |
no | no | no |
SettingsComponents |
no | no | no |
On #611 and consolidation
Commit 2 (e8f3bf1, "preserve save filters from PR 611") already pulls #611's macOS SAVE-mode filter into FilePickerProviderFactory.kt:110-113, so if both land, #611 is either a no-op or a conflict on those exact lines — the group should pick one carrier. Note the non-mac branch already had save filters, so #611's contribution here is mac-only parity, and it inherits finding 3. I have not touched #611 or any other PR.
What I could not verify
./gradlew is not runnable in this environment (command approval denied), so I could not execute OwnedFileDialogTest, ktlintCheck or detekt myself — I'm taking the PR's report of those at face value. Everything above is from reading the tree. Findings 1, 3, 4 and 6 are static and don't need a display; findings 2 and 5 are reachable by inspection but their user-visible consequence would want the manual check the description already flags as outstanding.
· branch fix/plugin-file-picker-owner
|
@arjun28115 Bounded review handoff for #552, which now carries @JaspreetSingh33's #611 contribution. I preserved the validated original implementation and pushed two review commits: e8f3bf1 integrates Jaspreet's macOS save filter with co-author credit and removes stale extraction comments; the latest follow-up fixes OwnedFileDialogTest.restore, where System.setProperty's nullable return triggered the Elvis clear immediately after restoration, and adds a fixture regression. That fixture defect was in the original #552 submission; the fixes are agent work. Remaining substantive issues and required validation:
Other Claude findings were triaged rather than blindly applied:
Claude review: https://github.com/risa-labs-inc/BossConsole/actions/runs/34736418629. No repeat review request. Marking this open PR draft under the review handoff policy until the two concrete callback/filter cases and required checks are resolved. #611 remains correctly closed as covered by this surviving implementation, with both contributors credited. No PR merged. |
The two shared-provider defects handed back on risa-labs-inc#552. Neither came from the ownership change; both live in the picker this PR touches, and both are silent. 1. `onResult` was called inside the same `try` that catches picker failures, so a plugin whose callback threw received its path and then a `null` immediately after. `null` is indistinguishable from a cancel, so one pick produced two answers and the second one was wrong. Delivery now goes through `deliverOnce`, which sets its flag before invoking, so a throw from the plugin's callback cannot be retried by the catch. The two throws are told apart by when they happen: a picker failure arrives before delivery and still reports the cancel. 2. The API takes `List<String>` and says nothing about the spelling, so plugins pass both `csv` and `.csv`, and every consumer appended its own dot. The macOS filter tested `name.endsWith(".$ext")` and Swing's FileNameExtensionFilter adds the dot itself, so `.csv` was matched as `..csv`. No file is named that, so the filter hid every valid choice instead of narrowing to them. `normalizedExtensions` strips a leading dot and drops blanks, and both platforms and both methods now route through it. Normalising rather than declaring a bare-extension contract, because the contract is not checkable at the plugin boundary and the failure it produces is a panel that lists nothing, which reads as "no matching files here". Both fixes are in Open and Save. The Save path also gains a comment saying what its filter does NOT do: a native SAVE panel narrows what is listed and does not constrain what the user types, so the returned path may still carry any extension or none. The three helpers are `internal` and pure so the rules are testable without an AWT panel on the EDT. The provider calls exactly these, so reverting a call site is a compile error rather than a silently unexercised test. 11 tests cover one delivery per pick, a throwing callback, a failure before delivery, csv/.csv, mixed case, whitespace, blank-only and empty filter lists. ktlint 1.8.0 clean; no line over 120 columns. No local test run is claimed. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
Pushed 1. One result per pick. Delivery now goes through 2. A dotted extension no longer empties the panel. The API takes I normalised rather than declaring a bare-extension contract, for the reason the bug is hard to notice: the contract is not checkable at the plugin boundary, and breaching it produces a panel that lists nothing, which reads as "there are no matching files here" rather than as a defect. Blank entries are dropped, and a blank-only list becomes empty, which every call site already treats as "no filter" rather than as a filter matching nothing. On your last point about SAVE. I have not claimed the filter forces the output extension, and the Save path now carries a comment saying the opposite: a native SAVE panel narrows what is listed and does not constrain what the user types, so the returned path may still carry any extension or none. Testability. The three helpers are Not taken here, matching your triage: the ktlint 1.8.0 is clean on both files and no line exceeds 120 columns. I run the pinned ktlint locally now rather than finding that out from |
…ry mode (risa-labs-inc#551) FilePickerProviderFactory created every panel as FileDialog(null as Frame?, ...) and showed it with a bare isVisible = true. Two consequences, and they are different failures. No owner. The panel has no window to be modal to and none to come forward over, so it can open behind the Compose window. onResult(null) then reaches the plugin looking exactly like a cancel. No directory mode. apple.awt.fileDialogForDirectories is process-wide and read by the native peer at creation, and a modal panel runs a nested event loop on the EDT that keeps dispatching. NativeFileDialogs already recorded this file as one of three that create a LOAD dialog without stating the flag, and the consequence it named: such a panel opened inside the browser's folder panel comes up as a directory chooser. That part is a defect independent of the reported symptom. NativeFileDialogs had already solved both for the browser's own panels, and its comment says the durable fix is to route every FileDialog in the tree through one helper. This extracts that helper rather than adding a third copy of it. NativeFileDialogs now delegates to it and loses its private duplicates, so the two cannot drift. The property discipline is split out as withDirectoryMode so it is testable without a display: an absent property is cleared rather than written back as "false", a present one is restored verbatim, nesting unwinds in reverse, and a throwing body still restores. Seven tests cover it. The owner selection and the panel itself are not covered and cannot be from a headless test. The catch blocks now log at error rather than warn and say that the plugin will see the failure as a user cancel, since onResult(null) is the only channel this API has and the log is the only place the difference exists. 570 tests in the browser package pass, including the existing NativeFileDialogs suite. ktlint and detekt clean. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Consolidate Jaspreet Singh’s save-filter addition into arjun28115’s shared owned-dialog implementation. Remove comments left behind by the helper extraction. Co-authored-by: Jaspreet Singh <jaspreetsinghupkar@gmail.com>
The two shared-provider defects handed back on risa-labs-inc#552. Neither came from the ownership change; both live in the picker this PR touches, and both are silent. 1. `onResult` was called inside the same `try` that catches picker failures, so a plugin whose callback threw received its path and then a `null` immediately after. `null` is indistinguishable from a cancel, so one pick produced two answers and the second one was wrong. Delivery now goes through `deliverOnce`, which sets its flag before invoking, so a throw from the plugin's callback cannot be retried by the catch. The two throws are told apart by when they happen: a picker failure arrives before delivery and still reports the cancel. 2. The API takes `List<String>` and says nothing about the spelling, so plugins pass both `csv` and `.csv`, and every consumer appended its own dot. The macOS filter tested `name.endsWith(".$ext")` and Swing's FileNameExtensionFilter adds the dot itself, so `.csv` was matched as `..csv`. No file is named that, so the filter hid every valid choice instead of narrowing to them. `normalizedExtensions` strips a leading dot and drops blanks, and both platforms and both methods now route through it. Normalising rather than declaring a bare-extension contract, because the contract is not checkable at the plugin boundary and the failure it produces is a panel that lists nothing, which reads as "no matching files here". Both fixes are in Open and Save. The Save path also gains a comment saying what its filter does NOT do: a native SAVE panel narrows what is listed and does not constrain what the user types, so the returned path may still carry any extension or none. The three helpers are `internal` and pure so the rules are testable without an AWT panel on the EDT. The provider calls exactly these, so reverting a call site is a compile error rather than a silently unexercised test. 11 tests cover one delivery per pick, a throwing callback, a failure before delivery, csv/.csv, mixed case, whitespace, blank-only and empty filter lists. ktlint 1.8.0 clean; no line over 120 columns. No local test run is claimed. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
fa2b7d2 to
7fde917
Compare
|
Rebased onto current
Your commits are preserved in order with their authorship. The SHAs changed because a rebase rewrites them; nothing else did. I checked file by file that every file this PR touches is byte-identical to what the branch held before the rebase, so the only intended change is the base. Flagging one thing rather than leaving you to find it: while rebasing #596 earlier I force-pushed over three of your commits on that branch before restoring them. I have since checked each branch's full history before touching it, which is what I should have done first. If you would rather I never rebase a branch you have commits on, say so and I will just tell you when one needs it. |
Two failures, both mine, neither caused by the rebase.
`compileTestKotlinDesktop` failed at FilePickerDeliveryTest:34. `assertEquals`
takes one type parameter for both arguments, and `listOf("...")` against a
`MutableList<String?>` gives it nothing to unify, so inference failed outright.
The expectation is now spelled `listOf<String?>(...)`, matching the two
assertions further down that already were. The `emptyList()` expectations are
spelled `emptyList<String>()` for the same reason, before they hit it too.
`detekt` failed on SpreadOperator at both FileNameExtensionFilter constructions.
The rule is not new and the code is not either: renaming `filters` to
`extensions` moved the element out of whatever suppressed it before. Suppressed
at the two functions with the reasoning written out, matching what
NativeFileDialogs.onOpenFiles in this same PR already does. The spread is over a
plugin's declared filter list, a handful of entries, and FileNameExtensionFilter
is Java varargs with no collection overload to prefer.
Worth saying plainly: a type error is something a compile would have caught and
I shipped it without one. There is no working Gradle path on this machine, so
ktlint is the only gate I can run locally, and it does not type-check.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
The blocker you named is cleared. Current head The dev-integration recheck you listed as the other step is also done, and it needed doing for a different reason: Marking ready is yours under the bounded-review policy, so I have not touched the draft flag. Flagging it because the condition you set has been met rather than leaving it to a poll. |
|
@claude Please review current head |
|
Claude finished @kshivang's task in 5m 2s —— View job Review: current head
|
| Site | Owner | States mode | Writes the flag | Disposes |
|---|---|---|---|---|
providers/DesktopDirectoryPickerProviderImpl.kt:38,48,55 |
Frame only |
no | yes, "false", no finally |
no |
platform/DesktopFilePicker.kt:43,45,51 |
no | no | yes, "false", no finally |
no |
platform/FilePicker.kt:41,99 |
no | no | no | no |
settings/shared/SettingsComponents.kt:875 |
no | no | no | no |
Two consequences worth having written down next to the helper rather than only in a PR thread:
DesktopDirectoryPickerProviderImplhas notry/catcharoundisVisible = true(the onlytryin that function,:22-31, wraps the look-and-feel call). A throw escapes theinvokeLaterbody, so the flag stays"true"for the process andonResultis never called, leaving the plugin waiting with no cancel. That is the sticky state the new picker is immune to and the other three sites are not.- Both writers write
"false"rather than restoring, so after one host directory pick the process-wide property is permanently set.withDirectoryModethen faithfully restores"false", which is functionally the same as absent for the peer, but the "absent stays absent" invariantOwnedFileDialogTest:55pins is defeated process-wide by code outside this PR. Worth a sentence so the next reader does not conclude the test proves more than it does.
4. Two dead imports - NativeFileDialogs.kt:12,14
java.awt.Dialog and java.awt.Frame are now referenced only from a backticked KDoc phrase at :295; the owner when that used them moved to OwnedFileDialog.kt. ktlint's no-unused-imports did not flag it (CI is green, and the rule counts KDoc text), so nothing gates this - it is a one-line tidy while the file is open.
While you are at :290-296: that KDoc still says "Same fix as DirectoryPickerProviderImpl, except that one only handles a Frame owner." It now describes a helper it delegates to, and it is the last in-tree pointer at the unfixed file, so it is the natural place for item 3's list.
5. Carried, and deferred by your triage - not re-arguing, just noting they are unchanged
pickSaveFilestill passessuggestedFileNametodialog.fileraw (:173-175), whereNativeFileDialogs.showSaveusessafePrefill. The suggestion comes from a plugin, andsafePrefillisinternalin the same module. Your "investigate compatibility first" is fair; the compatibility question is narrow, sincesafePrefillonly takes the last path segment and drops control characters."$dir$file"(:120,:186) still depends ongetDirectory()carrying a trailing separator, whichFileDialogdoes not document.File(dir, file).absolutePathis what the rest of the module uses.- The SAVE-filter caveat is now written at
:176-177and matches what you asked for: it narrows what is listed, it does not constrain what is typed. No over-promise anywhere in the file.
6. Coverage, in descending value - unchanged from last pass
- Nothing fails if someone writes
FileDialog(null as Frame?, ...)again, which is the defect this PR exists to remove. A source-shape test scanningcomposeApp/src/desktopMainforFileDialog(outsideOwnedFileDialog.ktagainst a shrinking allowlist of the four sites in item 3 would give the fix teeth and make the record in item 3 self-maintaining. The repo already pins this class of invariant against source (OsOpenArgumentsTest,FileTypeCategoriesTest). - The owner branches are untested. Deleting
is Dialog ->fromownedFileDialogfails nothing. Extractinginternal fun fileDialogOwner(active: Window?): Window?- pure, taking the window rather than reading the focus manager - makes the classification reachable from a headless test. AGENTS.md's mutation-verification standard forpluginSectionAbsenceis the bar. showModal's dispose-in-finallyis untested and cannot be from a headless JVM. The KDoc at:68-76correctly scopes its "each of which has a test" claim towithDirectoryMode, so this is fine as it stands.
Minor: OwnedFileDialogTest mutates a process-wide property, so it would need isolation if desktopTest ever turned on parallel execution within the JVM. Not a problem today.
#548
Wrapper routing is intact on this head, unchanged by the rebase:
| Provider | DefaultPlugin |
TrackingPluginContext |
SandboxedPluginContext |
|---|---|---|---|
filePickerProvider |
:867 |
:406 |
:240 |
directoryPickerProvider |
:892 |
:411 |
:247 |
pluginStorageFactory |
forwards | :388 |
forwards |
Still nothing pins it, and this exact regression has shipped: docs/release-notes/v9.2.35.md:22-23 records directoryPickerProvider and projectDataProvider falling through to the interface default null in both wrappers, so "Open Project" and Evolve's "Browse" silently did nothing. AGENTS.md states the three-wrapper rule as a rule with no test behind it. A ~15-line reflection test asserting every PluginContext provider val is declared override in TrackingPluginContext and SandboxedPluginContext is the highest-value missing test in this area, and it belongs to whichever PR the group consolidates into rather than to this one.
On storage identity specifically: pluginStorageFactory is forwarded by all three wrappers, but the identity a storage instance is created under is decided by the caller, not by the wrapper, and nothing in this head touches it - the diff is five files, all picker. So I can confirm the routing is not broken by this PR and cannot confirm anything about identity correctness from it. If #548 covers storage identity, it needs its own diff to review.
What I could not verify
./gradlew is not runnable here (fetch and build commands are not approved in this environment), so I did not execute OwnedFileDialogTest, FilePickerDeliveryTest, ktlintCheck or detekt myself - I am taking the green CI run on this head as the evidence for those. No native UI verification is claimed or attempted; the manual checks in the PR body (Frame and Dialog ownership, cancellation, a plugin pick nested inside the browser's folder panel, peer release over repeated picks) are still the only way to close items the source cannot answer. Findings 1 through 4 are static and need no display.
· branch fix/plugin-file-picker-owner
|
@arjun28115 Current-head review is complete. Your callback delivery and dotted-extension repairs resolve both substantive handoff items. Downloaded current-head macOS test reports confirm 11 delivery/filter + 8 owned-dialog + 11 native-dialog tests, all passing; all ten hosted checks pass. @JaspreetSingh33’s #611 work and co-author credit remain preserved. Latest Claude review: https://github.com/risa-labs-inc/BossConsole/actions/runs/34804614330. Final triage:
No unresolved substantive correctness/security/regression issue remains in this change. Proceeding with a locked exact-head squash merge into dev after refreshing dev/state/mergeability/checks. No production-code changes by this sweep; the new delivery/filter code and tests are your contributor work, separate from prior agent integration/fixture repairs. |
…ry mode (#551) (#552) * fix(plugins): give a plugin's file panel an owner and its own directory mode (#551) FilePickerProviderFactory created every panel as FileDialog(null as Frame?, ...) and showed it with a bare isVisible = true. Two consequences, and they are different failures. No owner. The panel has no window to be modal to and none to come forward over, so it can open behind the Compose window. onResult(null) then reaches the plugin looking exactly like a cancel. No directory mode. apple.awt.fileDialogForDirectories is process-wide and read by the native peer at creation, and a modal panel runs a nested event loop on the EDT that keeps dispatching. NativeFileDialogs already recorded this file as one of three that create a LOAD dialog without stating the flag, and the consequence it named: such a panel opened inside the browser's folder panel comes up as a directory chooser. That part is a defect independent of the reported symptom. NativeFileDialogs had already solved both for the browser's own panels, and its comment says the durable fix is to route every FileDialog in the tree through one helper. This extracts that helper rather than adding a third copy of it. NativeFileDialogs now delegates to it and loses its private duplicates, so the two cannot drift. The property discipline is split out as withDirectoryMode so it is testable without a display: an absent property is cleared rather than written back as "false", a present one is restored verbatim, nesting unwinds in reverse, and a throwing body still restores. Seven tests cover it. The owner selection and the panel itself are not covered and cannot be from a headless test. The catch blocks now log at error rather than warn and say that the plugin will see the failure as a user cancel, since onResult(null) is the only channel this API has and the log is the only place the difference exists. 570 tests in the browser package pass, including the existing NativeFileDialogs suite. ktlint and detekt clean. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(plugins): preserve save filters from PR 611 Consolidate Jaspreet Singh’s save-filter addition into arjun28115’s shared owned-dialog implementation. Remove comments left behind by the helper extraction. Co-authored-by: Jaspreet Singh <jaspreetsinghupkar@gmail.com> * test(pickers): preserve pre-existing directory flag in teardown * fix(pickers): deliver one result per pick, and accept a dotted extension The two shared-provider defects handed back on #552. Neither came from the ownership change; both live in the picker this PR touches, and both are silent. 1. `onResult` was called inside the same `try` that catches picker failures, so a plugin whose callback threw received its path and then a `null` immediately after. `null` is indistinguishable from a cancel, so one pick produced two answers and the second one was wrong. Delivery now goes through `deliverOnce`, which sets its flag before invoking, so a throw from the plugin's callback cannot be retried by the catch. The two throws are told apart by when they happen: a picker failure arrives before delivery and still reports the cancel. 2. The API takes `List<String>` and says nothing about the spelling, so plugins pass both `csv` and `.csv`, and every consumer appended its own dot. The macOS filter tested `name.endsWith(".$ext")` and Swing's FileNameExtensionFilter adds the dot itself, so `.csv` was matched as `..csv`. No file is named that, so the filter hid every valid choice instead of narrowing to them. `normalizedExtensions` strips a leading dot and drops blanks, and both platforms and both methods now route through it. Normalising rather than declaring a bare-extension contract, because the contract is not checkable at the plugin boundary and the failure it produces is a panel that lists nothing, which reads as "no matching files here". Both fixes are in Open and Save. The Save path also gains a comment saying what its filter does NOT do: a native SAVE panel narrows what is listed and does not constrain what the user types, so the returned path may still carry any extension or none. The three helpers are `internal` and pure so the rules are testable without an AWT panel on the EDT. The provider calls exactly these, so reverting a call site is a compile error rather than a silently unexercised test. 11 tests cover one delivery per pick, a throwing callback, a failure before delivery, csv/.csv, mixed case, whitespace, blank-only and empty filter lists. ktlint 1.8.0 clean; no line over 120 columns. No local test run is claimed. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(pickers): repair what CI caught on the rebased head Two failures, both mine, neither caused by the rebase. `compileTestKotlinDesktop` failed at FilePickerDeliveryTest:34. `assertEquals` takes one type parameter for both arguments, and `listOf("...")` against a `MutableList<String?>` gives it nothing to unify, so inference failed outright. The expectation is now spelled `listOf<String?>(...)`, matching the two assertions further down that already were. The `emptyList()` expectations are spelled `emptyList<String>()` for the same reason, before they hit it too. `detekt` failed on SpreadOperator at both FileNameExtensionFilter constructions. The rule is not new and the code is not either: renaming `filters` to `extensions` moved the element out of whatever suppressed it before. Suppressed at the two functions with the reasoning written out, matching what NativeFileDialogs.onOpenFiles in this same PR already does. The spread is over a plugin's declared filter list, a handful of entries, and FileNameExtensionFilter is Java varargs with no collection overload to prefer. Worth saying plainly: a type error is something a compile would have caught and I shipped it without one. There is no working Gradle path on this machine, so ktlint is the only gate I can run locally, and it does not type-check. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5 <noreply@anthropic.com> Co-authored-by: Shivang <shivang.iitk@gmail.com> Co-authored-by: Jaspreet Singh <jaspreetsinghupkar@gmail.com>
) * test(db): gate the two client-grant shapes that produced #547 three times (#596) * test(db): gate the two client-grant shapes that produced #547 three times `ALTER DEFAULT PRIVILEGES` in 20251023000014_grants.sql hands every later table client grants, and three tables then shipped with a permissive write policy on top of that: plugin_downloads (#488), completed_authentications (#536) and secret_access_log / plugin_api_key_logs (#538). Each was caught by a person reading a migration. identity-disclosure audit. That audit asks who can READ identity. Neither it nor the default-privileges change reaches the shape above: a default-privilege revoke is not retroactive, CHECK 3 filters `cmd in ('SELECT','ALL')` so an INSERT policy is invisible to it, and CHECK 3b filters `not relrowsecurity` while both #538 tables had RLS enabled with a permissive policy. Adds supabase/audit/client_grant_audit.sql with two exact catalog checks: * G1 names any public table a client role can reach without RLS. No allowlist, because all 34 tables in the schema enable RLS today. * G2 names any INSERT/UPDATE/DELETE/ALL policy reachable by anon, authenticated or PUBLIC whose predicate is the literal `true`. G2 keys on the predicate, not on a missing `TO` clause. The missing `TO` is how all three slipped past review, but 101 of the 128 policies in this schema omit it, so gating on that would report most of the schema. Predicate plus write plus client role matches two policies in the whole schema, and both are the ones #538 removes. G2 is asserted as a subset rather than as HEALTHY while those two are still on dev, so the gate passes before and after #538 and still fails on anything new. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(db): exclude restrictive policies from write lint * docs(db): state audit limits and cross-suite regression coverage * docs(db): name the exact companion policy assertion --------- Co-authored-by: Claude Opus 5 <noreply@anthropic.com> Co-authored-by: Shivang <shivang.iitk@gmail.com> * fix(browser): actually warn before downloading an executable file (#484) * fix(browser): actually warn before downloading an executable file warnForExecutables defaults to true and its KDoc promises a warning before a recognized executable finishes downloading, but the branch that was supposed to show it was an empty TODO block ("for now, just proceed") - so the download always proceeded silently regardless of the setting, for every user, with no way to turn the (nonexistent) warning off since there is no Settings UI for it either. confirmExecutableDownload mirrors pickSaveFile's own pattern exactly, since both are called from the same synchronous StartDownloadCallback and both need a blocking answer before it returns: a Swing confirm dialog run via SwingUtilities.invokeAndWait, failing closed (treated as cancel) if the dialog itself cannot be shown, matching how a null save path from pickSaveFile already cancels the same callback. Declining releases the claimed save path and cancels the download the same way the existing parent-directory-failure bail does, just above it in the same callback. * fix(browser): close executable consent and cancellation gaps * fix(browser): bind download consent to a usable window and explicit choice * feat(browser): add a user-facing toggle for the executable download warning Settings > Browser > Downloads > "Warn before downloading executable files", on by default. BrowserSettings.warnForExecutables is persisted through the existing BrowserSettingsManager JSON store alongside the other browser toggles, and FluckEngine reads it live at download time instead of the unpersisted DownloadSettings copy, so a change applies to the next download without restarting BOSS. Disabling it skips only the executable warning; filename checks, save-path handling and cancellation cleanup are unchanged. Addresses the maintainer's follow-up on #484. * fix(settings): index the executable-download-warning toggle for search SettingsSearchIndexDriftTest failed CI on all three platforms: the new "Warn before downloading executable files" toggle (Settings > Browser > Downloads) had no entry in SettingsSearchIndex, so Settings search could not find it. Added a Downloads group under the FLUCK section, extracted into its own downloadsEntries() helper alongside the existing fileAndLinkEntries() to keep browserEntries() under detekt's LongMethod threshold. * fix(browser): pin the warning toggle's persistence and volatility - BrowserSettingsManager: settingsFile is now an internal var with a reloadForTest() seam (same pattern as DefaultAppsSettingsManager), so the persisted-value tests exercise the real saveSettings/load path instead of re-stating the assignment; covers save+reload round-trips, a file predating the key, and a corrupt file - ExecutableDownloadConsentTest: the live-read test re-implemented the call site, so it now pins the StartDownloadCallback argument textually (SettingsSearchIndexDriftTest idiom) - a revert to the unpersisted downloadSettings copy now fails the build - BrowserSettings.warnForExecutables is @volatile: written on the Compose UI thread, read on the JxBrowser download callback thread - DownloadSettings.warnForExecutables removed: no reader left it; the live flag is BrowserSettings.warnForExecutables * test(browser): make the persistence tests measure the reload, not the assignment Addressing the second Claude review of 70a4874: - default test asserted process-global state that the test's own @beforeeach causes to be loaded from the developer's real settings file - it would pass in CI and fail on a machine with the toggle off. It now pins the persisted shape's default; the in-memory direction is carried by the predating-key reload test. - the round-trip test moved the in-memory value away between save and reload, so deleting the load-apply in BrowserSettingsManager now fails it. - the whole persisted shape (all eleven fields, including the four system-property mirrors) is snapshotted in @beforeeach - after the first access that may trigger the manager's real-file load - and restored in @AfterEach, since reloadForTest() rewrites all of them. - reloadForTest KDoc states the deliberate no-reset divergence from DefaultAppsSettingsManager.resetForTest. * fix(browser): test the download dialog's owner resolution and surface it R1: the dialog-owner resolution (active window, else a fallback, else refuse) was the only new decision in the executable-download warning with no test - the branch that silently cancels a user's download had nothing pinning it. Extracted pickDialogOwner(activeWindow, candidates), mirroring resolveActionableWindowIdFrom's own pure-function split, and added cases covering active-window-wins, non-displayable-active falling back, and no-usable-owner refusing. R2: the fallback resolved exactly one window id and refused outright if it wasn't displayable, even when another registered window was open - reachable in the interval where a window has been disposed but its EDT unregister hasn't run yet. pickDialogOwner now takes an ordered list of candidates and scans all of them; WindowFocusManager. candidateWindowsForDialogOwner() supplies that list (the resolved fallback window first, then every other registered window), covered by a test where the first candidate is stale and a later one is usable. R3: nothing called toFront()/requestFocus() on the resolved owner, so a background BOSS could still show this modal somewhere the operator doesn't see, with the JxBrowser callback thread waiting behind it. Already running on the EDT inside confirmExecutableDownloadOnEdt's own Runnable, so this calls AWT directly rather than needing invokeLater. * fix(review): preserve current release metadata in download consent PR * fix(review): tighten executable consent cleanup and default coverage --------- Co-authored-by: Shivang <shivang.iitk@gmail.com> * test: harden source convention scans (#508) Co-authored-by: sgoel2be24-cyber <223222024+sgoel2be24-cyber@users.noreply.github.com> * fix(plugins): scope PluginStorageFactory.createStorage to the calling plugin's own id (#548) * fix(plugins): scope PluginStorageFactory.createStorage to the calling plugin's own id TrackingPluginContext (the only PluginContext a plugin's own code ever holds - every registration path wraps createSandboxedContext in one before calling register()) exposed the raw, unscoped PluginStorageFactory straight through from its delegate. That factory takes an arbitrary pluginId per call and the desktop implementation persists purely off that string (~/.boss/plugin-data/{pluginId}/storage.properties), with no check anywhere that it names the caller. Any installed plugin could reach another plugin's persisted data with nothing more than: context.pluginStorageFactory?.createStorage("victim-plugin-id") - read, overwrite, or clear() it. Added ScopedPluginStorageFactory, which binds createStorage to the context's own pluginId (a construction-time value the plugin does not control) and ignores whatever id the caller passes in - the same pattern DownloadCenterProviderImpl.forPlugin already uses to keep one plugin's transfer ids from colliding with or addressing another's. SandboxedPluginContext is untouched: it already passed downloadCenterProvider through unscoped in the same way, since TrackingPluginContext wraps it and is the only context a plugin ever sees - matching that existing, working precedent rather than adding a second, redundant scoping layer. Added tests/TrackingPluginContextStorageScopeTest.kt: the underlying factory is only ever asked for the context's own plugin id regardless of what a caller requests, and a write made through one plugin's storage reference never lands in another plugin's real storage. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * test(plugins): model shared storage in isolation regression * test(plugins): assert scoped provider identity and document boundary --------- Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com> Co-authored-by: Shivang <shivang.iitk@gmail.com> * fix(browser): validate co-browse control payloads before splicing into JS (#549) Validate remote co-browse control objects before embedding them in page JavaScript. Reject unquoted expressions recursively and preserve valid control messages and escaped string data. Original diagnosis, object guard and six regression tests by Antriksh. Prior review repair adds recursive token validation and four regressions; contributor replayed all three commits cleanly onto dev. All ten hosted checks and ten focused tests pass. Co-authored-by: Antriksh <antrikshsingh850@gmail.com> Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com> * fix(kernel): require verified caller identity for the Supabase proxy and Download bridges (#550) * fix(kernel): require verified caller identity for the Supabase proxy and Download bridges SupabaseServiceBridge is a generic, unrestricted Postgrest proxy backed by the signed-in user's own RLS-scoped Supabase session - its rpc() reaches the same create_secret/update_secret RPCs SecretServiceBridge exists to protect, and select() can read any table RLS exposes. Neither select nor rpc checked caller identity (BossConsole#53), so any local process reaching the kernel's IPC socket had the same reach as a legitimately installed plugin, attributable to nobody. Both now require a verified process identity, matching SecretServiceBridge's established pattern, with audit logging on every call. DownloadServiceBridge.openFile/revealInFolder shell out to the OS's own "open with default application" command for any path that exists on disk, with no check that the path has anything to do with a download - before this, an unauthenticated caller on the IPC bridge could make BOSS execute any file on disk via the OS default-handler mechanism, completely bypassing BossConsole#484's executable-download-warning consent dialog (that path never goes through it at all). Every RPC on the bridge now requires a verified caller identity, and openFile/revealInFolder are additionally confined to a path this provider's own downloads list is currently tracking - canonical-path compared, so a symlink or a `..` cannot walk outside it - turning "open any file that exists" into "open a file BOSS itself downloaded". Both bridges get real gRPC-server-and-client tests (matching PluginUIServiceBridgeTest's pattern): an anonymous caller is refused with PERMISSION_DENIED on every RPC, the underlying provider is asserted never invoked on a refusal, and DownloadServiceBridge additionally covers an authenticated caller naming a path outside the tracked set. * fix(kernel): revalidate download streams and use checked paths * fix(kernel): capture stream identity and reject empty download paths * style(tests): align download regression expression wrapping * fix(kernel): clear code-quality on the download/supabase caller-identity branch detekt's default MaxLineLength (120) rejected a 128-char assertEquals in DownloadServiceBridgeTest that ktlint's wider 140-char .editorconfig limit had let through - wrap it. Also finishes the finding-6 KDoc reword the bot review flagged as ungrammatical ("this is the path does not provide" -> "this path provides no executable-consent step") and reflows that paragraph. No behavior change; addresses the last remaining code-quality blocker noted in the PR's review thread. --------- Co-authored-by: Shivang <shivang.iitk@gmail.com> * fix(plugins): give a plugin's file panel an owner and its own directory mode (#551) (#552) * fix(plugins): give a plugin's file panel an owner and its own directory mode (#551) FilePickerProviderFactory created every panel as FileDialog(null as Frame?, ...) and showed it with a bare isVisible = true. Two consequences, and they are different failures. No owner. The panel has no window to be modal to and none to come forward over, so it can open behind the Compose window. onResult(null) then reaches the plugin looking exactly like a cancel. No directory mode. apple.awt.fileDialogForDirectories is process-wide and read by the native peer at creation, and a modal panel runs a nested event loop on the EDT that keeps dispatching. NativeFileDialogs already recorded this file as one of three that create a LOAD dialog without stating the flag, and the consequence it named: such a panel opened inside the browser's folder panel comes up as a directory chooser. That part is a defect independent of the reported symptom. NativeFileDialogs had already solved both for the browser's own panels, and its comment says the durable fix is to route every FileDialog in the tree through one helper. This extracts that helper rather than adding a third copy of it. NativeFileDialogs now delegates to it and loses its private duplicates, so the two cannot drift. The property discipline is split out as withDirectoryMode so it is testable without a display: an absent property is cleared rather than written back as "false", a present one is restored verbatim, nesting unwinds in reverse, and a throwing body still restores. Seven tests cover it. The owner selection and the panel itself are not covered and cannot be from a headless test. The catch blocks now log at error rather than warn and say that the plugin will see the failure as a user cancel, since onResult(null) is the only channel this API has and the log is the only place the difference exists. 570 tests in the browser package pass, including the existing NativeFileDialogs suite. ktlint and detekt clean. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(plugins): preserve save filters from PR 611 Consolidate Jaspreet Singh’s save-filter addition into arjun28115’s shared owned-dialog implementation. Remove comments left behind by the helper extraction. Co-authored-by: Jaspreet Singh <jaspreetsinghupkar@gmail.com> * test(pickers): preserve pre-existing directory flag in teardown * fix(pickers): deliver one result per pick, and accept a dotted extension The two shared-provider defects handed back on #552. Neither came from the ownership change; both live in the picker this PR touches, and both are silent. 1. `onResult` was called inside the same `try` that catches picker failures, so a plugin whose callback threw received its path and then a `null` immediately after. `null` is indistinguishable from a cancel, so one pick produced two answers and the second one was wrong. Delivery now goes through `deliverOnce`, which sets its flag before invoking, so a throw from the plugin's callback cannot be retried by the catch. The two throws are told apart by when they happen: a picker failure arrives before delivery and still reports the cancel. 2. The API takes `List<String>` and says nothing about the spelling, so plugins pass both `csv` and `.csv`, and every consumer appended its own dot. The macOS filter tested `name.endsWith(".$ext")` and Swing's FileNameExtensionFilter adds the dot itself, so `.csv` was matched as `..csv`. No file is named that, so the filter hid every valid choice instead of narrowing to them. `normalizedExtensions` strips a leading dot and drops blanks, and both platforms and both methods now route through it. Normalising rather than declaring a bare-extension contract, because the contract is not checkable at the plugin boundary and the failure it produces is a panel that lists nothing, which reads as "no matching files here". Both fixes are in Open and Save. The Save path also gains a comment saying what its filter does NOT do: a native SAVE panel narrows what is listed and does not constrain what the user types, so the returned path may still carry any extension or none. The three helpers are `internal` and pure so the rules are testable without an AWT panel on the EDT. The provider calls exactly these, so reverting a call site is a compile error rather than a silently unexercised test. 11 tests cover one delivery per pick, a throwing callback, a failure before delivery, csv/.csv, mixed case, whitespace, blank-only and empty filter lists. ktlint 1.8.0 clean; no line over 120 columns. No local test run is claimed. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(pickers): repair what CI caught on the rebased head Two failures, both mine, neither caused by the rebase. `compileTestKotlinDesktop` failed at FilePickerDeliveryTest:34. `assertEquals` takes one type parameter for both arguments, and `listOf("...")` against a `MutableList<String?>` gives it nothing to unify, so inference failed outright. The expectation is now spelled `listOf<String?>(...)`, matching the two assertions further down that already were. The `emptyList()` expectations are spelled `emptyList<String>()` for the same reason, before they hit it too. `detekt` failed on SpreadOperator at both FileNameExtensionFilter constructions. The rule is not new and the code is not either: renaming `filters` to `extensions` moved the element out of whatever suppressed it before. Suppressed at the two functions with the reasoning written out, matching what NativeFileDialogs.onOpenFiles in this same PR already does. The spread is over a plugin's declared filter list, a handful of entries, and FileNameExtensionFilter is Java varargs with no collection overload to prefer. Worth saying plainly: a type error is something a compile would have caught and I shipped it without one. There is no working Gradle path on this machine, so ktlint is the only gate I can run locally, and it does not type-check. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5 <noreply@anthropic.com> Co-authored-by: Shivang <shivang.iitk@gmail.com> Co-authored-by: Jaspreet Singh <jaspreetsinghupkar@gmail.com> * feat(mcp): trust a whole plugin's tools at once, not one at a time (#542) * feat(mcp): trust a whole plugin's tools at once, not one at a time Adds a "Trust This Plugin" option to the MCP tool approval dialog - a persisted, provider-wide ALLOW covering every tool a plugin contributes, not just the one call being approved. Answers the actual friction: a plugin with several mutating tools currently means approving each one individually, in the same session, one at a time. McpToolPolicyConfig gains a second rule map, providerRules, keyed by providerId rather than tool name. policyFor(toolName, providerId) now resolves both, with DENY - at either level - always winning first and unconditionally: a provider-wide DENY is not "most specific wins" and must not be overridable by a narrower per-tool ALLOW, the same posture DENY already has against session trust everywhere else in this engine. Below that: session trust, then an explicit tool-specific rule (ALLOW or ASK), then the provider's own ALLOW, then the risk-based default. providerId is an optional parameter so an existing caller that only ever checked a tool name keeps compiling. setToolPolicy, the new setProviderPolicy and revokeProviderPolicy now share one persistAndPublish helper for the persist-and-fault-report shape, so a future change to how a write failure is logged or reported cannot update one path and silently miss the others - the mistake a similar policy engine made once already by not sharing this logic. revokeProviderPolicy removes the key rather than rewriting it to some neutral value, for the same reason. McpApprovalDecision.Approved gains a trustProvider flag alongside the existing trustForSession one. Persisting it is a synchronous disk write, so it runs under withContext(Dispatchers.IO) rather than on whatever dispatcher delivered the MCP call - and a failed persist doesn't block the call the operator already approved past; it falls back to session trust for that one tool instead; a new PROVIDER_TRUST_PERSIST_FAILED ledger disposition records that the durable grant did not save. The new button sits in its own row below the main actions, not next to "Approve Once" - it is the broadest grant this dialog can make, so it should take a deliberate reach rather than risk a fast click landing on it by mistake. A caption names the scope. Revocation is a new McpProviderTrustDialog (list + per-row revoke, mirroring the shape an equivalent per-tool dialog would need) reachable from a "Trusted plugins (n)" button in the bottom bar, next to the existing session- trust revoke button - only shown once a provider is actually trusted. Test plan: McpProviderPolicyTest pins policyFor's full precedence table (provider ALLOW/DENY against tool rules, session trust, the policy-unreadable fault) and persistence (round-trips through a real file; revoke removes the key, survives reload; revoking one provider leaves another's trust untouched). McpGovernedInvocationTest adds two integration cases through the real registry: approving one tool with trustProvider lets a sibling tool from the same provider through with no second prompt and no session-trust call, and revoking provider trust makes its tools ask again. No Compose-level test for the dialogs themselves - no test infrastructure exists yet for this UI layer on this branch (no ScrimmedModalSemanticsTest-style harness), noted here rather than left unstated. Verified: full :composeApp:allTests passes except the two pre-existing, environment-specific Windows symlink-privilege failures already known from earlier this week (DesktopFileScannerTest, DiscardDownloadContainmentTest). ktlintCheck/detekt clean. * test(mcp): pin the provider-trust persist-failure fallback A failed provider-wide persist must still run the already-approved call, record PROVIDER_TRUST_PERSIST_FAILED, session-trust only that one tool, and leave the provider's other tools asking. Mirrors the blocked-write setup the per-tool failure case in McpPersistentApprovalTest uses. Agent-side regression coverage added during sweep BOSS_SWEEP_20260911_mcp-policy; not part of the author's submission. * fix(mcp): guard provider-wide trust writes against a stale approval dialog trustProvider ("Trust This Plugin") called setProviderPolicy directly, with none of the revocation/DENY/availability guards validateApproval already applies before setToolPolicy's persisted write. A revoke or DENY landing while the dialog was open (e.g. an operator resetting the tool from "Persisted MCP policies") could be silently overwritten: the call in hand was correctly withheld, but the provider-wide ALLOW still persisted, leaving every other tool that provider contributes AUTO_ALLOWED from an authorization the reset was supposed to invalidate. setProviderPolicy now takes the same preserveDeny/expectedRevocation guards setToolPolicy has, checked under the same lock, plus an optional toolName since the revocation check is inherently per-tool. The trustProvider branch in approvedAuthorization pre-checks isAvailable/revocationVersion/policyFor before writing (mirroring validateApproval exactly) and, on a false return, disambiguates a guard refusal (POLICY_DENIED, call does not run) from a genuine disk failure (PROVIDER_TRUST_PERSIST_FAILED, call still runs - the existing, deliberate fallback for that case). New regression test in McpGovernedInvocationTest reproduces the review's exact scenario (a revokePersistedPolicy landing while a prompt is open) and confirms it fails without the fix. AGENTS.md's governed-MCP section now documents provider-wide trust as its own scope, including the deliberate asymmetry between a stale-dialog refusal and a disk-failure fallback. * fix(mcp): invalidate queued grants when provider trust is reset * fix(mcp): clear code-quality on the provider-scoped-policy branch detekt flagged setProviderPolicy's preserveDeny guard as ComplexCondition (4 boolean/comparison operators inline) and a 121-char assertEquals in McpProviderRevocationTest as MaxLineLength (over detekt's default 120, though under ktlint's wider 140-char .editorconfig limit). Extract the DENY check into isProviderOrToolDenied() - same behavior, named and testable on its own - and wrap the long assertEquals. No behavior change. --------- Co-authored-by: Shivang <shivang.iitk@gmail.com> * fix(keymap): make an explicit Ctrl chord fire on the Control key off macOS (#553) (#559) Make explicit Ctrl bindings fire on Control off macOS, with shared capture/matching rules and platform-aware chord identity for conflict detection and migration. Preserve distinct macOS modifiers and reject unsupported Super capture. Original implementation and contributor follow-up by @arjun28115. Earlier review-agent work integrated dev, rejected unsupported Super captures, and corrected comments; those repairs are separately credited in the PR description. Closes #553. * fix(chrome): restore a minimized window before focusing it (#560) (#561) Restore iconified frames before focusing through either WindowFocusManager entry point, preserving maximized state. Original shared helper and headless mask tests by @arjun28115 (#561). Independent equivalent implementation and missing-target regression by @Sahvendra7 (#580), retained through consolidation. Earlier review-agent work integrated that regression and clarified restore ordering and documentation. Closes #560. Co-authored-by: Sahvendra <shyamajanidhaka@gmail.com> * fix(cli): prevent readiness queue lost commands (#633) * fix(cli): prevent readiness queue lost commands * fix(cli): close initialization queue race and guard deferred logging --------- Co-authored-by: ayush <workside@gamil.com> Co-authored-by: Shivang <shivang.iitk@gmail.com> * fix(cli): queue external command approval prompts * fix(cli): guard queued approvals against double clicks and floods * fix(cli): log command holding only after enqueue succeeds --------- Co-authored-by: arjun28115 <singlaarjun28@gmail.com> Co-authored-by: Claude Opus 5 <noreply@anthropic.com> Co-authored-by: Antriksh <antrikshsingh850@gmail.com> Co-authored-by: Shikhar Goel <sgoel2_be24@thapar.edu> Co-authored-by: sgoel2be24-cyber <223222024+sgoel2be24-cyber@users.noreply.github.com> Co-authored-by: Jaspreet Singh <jaspreetsinghupkar@gmail.com> Co-authored-by: Sahvendra <shyamajanidhaka@gmail.com> Co-authored-by: Ayush Santosh Chhatre <109429245+Ayush04-C@users.noreply.github.com> Co-authored-by: ayush <workside@gamil.com>
Plugin file pickers now use the active Frame or Dialog as their owner on macOS and the active window as the Swing parent on other platforms. The shared native-dialog helper states the directory mode before showing, restores the prior property value through nested dialogs and failures, and disposes the native peer in finally. This addresses the source-established defects in #551; the reported Fluck Agent Attach-button symptom still needs native UI reproduction.
The browser's existing picker implementation now shares that helper. Plugin save pickers also install the supplied extension filter. Native SAVE-panel filtering remains platform behavior to check manually; this does not promise enforcement of an output filename extension.
Contributor provenance:
Validation: the original #552 head 8918bbb passed all nine hosted checks, including all platform builds and ktlint/detekt. Current-head browser-package tests and root ktlintCheck/detekt are queued behind the shared local build lock; current-head CI and Claude review are pending. No native UI verification is claimed.
Manual checks still required: use Fluck Agent Attach file with a hardware-accelerated browser surface; test Open and Save from both a main window and an existing modal; cancel and select a file; open a plugin file picker during a browser folder panel and confirm file mode and restored folder mode; verify native peers are released after repeated picks. Other host picker entry points are outside this change.
Current-head review — 2026-09-14
Current head
e6cc0b05e823f21a3b741416f18f59270f1f15edhas all ten hosted checks passing, including macOS/Linux/Windows builds, root ktlint/detekt and compatibility. Downloaded the macOS test artifact and verified 11 FilePickerDeliveryTest, 8 OwnedFileDialogTest and 11 NativeFileDialogsTest tests with zero failures/errors/skips. These are current-head executed tests, superseding the prior pending-validation handoff. Fresh main includes merged #626; the relevant implementation is not already present in main/dev. No native UI execution is claimed.@arjun28115’s subsequent contributor repairs now provide once-only callback delivery and normalize dotted/blank extensions, with 11 tests. Prior agent fixture repair and @JaspreetSingh33’s #611 save-filter integration remain preserved with explicit credit. This sweep made no production-code changes.
Tests exercise helper behavior; they do not prove provider call-site wiring, native owner selection or peer disposal. The comment claiming that reverting call sites necessarily causes a compile error overstates coverage. Current wiring was reviewed directly. Fluck Attach reproduction and native Frame/Dialog Open/Save, cancellation, nested directory-mode and repeated peer-disposal checks remain manual follow-up. Remaining pre-existing directory-flag writers are outside this fix. #611 stays closed and fully represented here; no other PR was modified.
Current-head Claude review completed: https://github.com/risa-labs-inc/BossConsole/actions/runs/34804614330. It confirms both substantive repairs and fixture regression. MIME/glob support is outside the documented extension-only plugin API; browser HTML accept has different semantics. Callback-failure log wording, import/KDoc tidy and broader owner/wrapper tests are nonblocking follow-ups, with detailed evidence in the final review comment.
Remaining unrouted native sites: components/plugin/providers/DesktopDirectoryPickerProviderImpl.kt and platform/DesktopFilePicker.kt write the directory flag without finally restoration (resetting to false); platform/FilePicker.kt and components/settings/shared/SettingsComponents.kt retain separate dialogs. These pre-existing sites are not fixed by this PR.