Filter Linux workspace open targets by XDG category - #2494
Conversation
|
@slopcop review |
|
🚨 SLOP COP 🚨 · I am the Slop Cop. I am reviewing this pull request for security, code quality, architecture, and performance. I will also check whether a safe end-to-end test applies. |
| ]; | ||
| } | ||
|
|
||
| const LINUX_WORKSPACE_APPLICATION_CATEGORIES = new Set([ |
There was a problem hiding this comment.
🚨 slopcop/review — The allowlist excludes standard IDE entries.
The desktop menu standard defines IDE as a category. GNOME Builder uses Categories=GNOME;GTK;Development;IDE;, and this repository has no explicit GNOME Builder adapter. This change removes that useful workspace target. Add IDE to the allowlist and cover it with a fixture.
|
|
||
| const LINUX_WORKSPACE_APPLICATION_CATEGORIES = new Set([ | ||
| "FileManager", | ||
| "TerminalEmulator", |
There was a problem hiding this comment.
🚨 slopcop/review — Terminal entries do not receive terminal-specific path arguments.
A real GNOME Terminal entry uses Exec=gnome-terminal. The current invocation code appends the workspace path, which creates gnome-terminal /workspace. GNOME Terminal uses --working-directory=DIR for this operation. Route these entries through the existing terminal builder, or omit TerminalEmulator until the launcher supports them. Add a fixture with the real command shape.
| ]); | ||
|
|
||
| function parseDesktopEntryList(value: string | undefined): string[] { | ||
| return value?.split(";").filter(Boolean) ?? []; |
There was a problem hiding this comment.
🚨 slopcop/review — The list parser treats an escaped semicolon as a separator.
The desktop-entry standard permits escaped semicolons in string(s) values. For example, Categories=X-Foo\;TextEditor; is one extension category. This code splits it into two values and accepts it as TextEditor. Parse the list escape rule before the allowlist check, and add a focused test.
There was a problem hiding this comment.
🚨 SLOP COP 🚨 · review
Plain English summary: This change makes Linux open menus smaller. It keeps file managers, terminals, and text editors.
I found three correctness gaps:
- The allowlist excludes the standard
IDEcategory. This removes GNOME Builder and other useful IDEs without explicit adapters. - Terminal desktop entries use the generic launcher. GNOME Terminal needs
--working-directory, but BB adds the workspace path as a program. - The list parser splits escaped semicolons. An extension category can therefore pass the
TextEditorcheck by mistake.
I left line comments with fixes and tests. The terminal path should reuse the existing terminal command builder.
I found no duplicate desktop-entry parser. The category policy belongs in the current parser.
I found no new security issue. Category text controls selection only and never enters process arguments.
I found no performance issue. The new work is linear, and existing file reads remain the main cost.
All current GitHub checks pass. I used static reads because this pull request comes from an untrusted fork.
I did not execute the fork code. I did not run Doobie because this daemon library has no browser path.
The new package test covers category selection. It uses an artificial terminal command, so it misses the real terminal failure.
References: desktop category list, desktop list grammar, GNOME Builder entry, and GNOME Terminal entry.
Summary
CategoriesvaluesFileManager,TextEditor, andTerminalEmulatorRoot cause
PR #403 added generic Linux desktop-application discovery for workspace open
targets. It currently includes every visible
.desktopentry withType=Application, anExeccommand, and neitherHidden=truenorNoDisplay=true.As a result, BB's Open in/Open with menus can contain nearly every installed
desktop application on Linux, including media players, settings utilities,
messaging clients, and other software that is not a useful workspace target.
This change additionally requires a relevant standard XDG category:
FileManager,TextEditor, orTerminalEmulator. Existing explicit BB launchadapters remain unaffected.
Validation
pnpm exec turbo run typecheck test --filter=@bb/local-open-targets --forcegit diff --check