Repository navigation
feat(windows): add Explorer conversion menus with desktop progress - #7
leoisadev1 merged 15 commits into
Conversation
CNV-11 Add the Explorer right-click menu on Windows
ProblemOn Windows, right-clicking a file shows no convt menu. Goal"Convert with convt" in the Explorer context menu on Windows 11 (the new top-level menu) and Windows 10 (the classic menu), with the same targets as Acceptance criteria
How to verifyInstall the MSI on Windows 11, right-click a PNG, a MOV and a PDF in Explorer, pick a target, and confirm the app converts it and the output appears next to the input. Uninstall and confirm the menu is gone. |
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
![graphify-labs[bot]](https://avatars.githubusercontent.com/in/4356270?s=60&v=4){kind=small}
There was a problem hiding this comment.
Graphify reviewed this change.
Worth a look — the grounded gate found no coupling regressions or blocking issues, but 3 advisory finding(s) below merit a look before merge.
Graphify review — findings
Adds a Windows Explorer context-menu handler (convt-shell, an IExplorerCommand COM DLL) that offers only the targets every selected file supports, in menu order, and hands the selection to convt-app with --show-progress. When show_progress is set, route brings up the main window on the Activity page while the job converts beside the input; if the silent conversion can't run, it still falls back to Quick convert. On Windows the app now builds as a GUI-subsystem binary and spawns ffmpeg and other tools with CREATE_NO_WINDOW, so no console windows flash.
Worth a look
- Probe timeout or failure is cached as 'no targets' for 30s, hiding the menu —
crates/convt-shell/src/windows.rs:105· Escalate · medium- agreed by 2 of 2 members but NOT verified (no proof, no reproducing execution) — consensus is not a verdict; needs human review
- Probe can block past its deadline because reader.join() has no timeout —
crates/convt-shell/src/windows.rs:141· Escalate · medium- agreed by 2 of 2 members but NOT verified (no proof, no reproducing execution) — consensus is not a verdict; needs human review
- windows_subsystem="windows" silently breaks CLI output (--help, --version, errors) on Windows —
crates/convt-app/src/main.rs:4· Escalate · medium- agreed by 2 of 2 members but NOT verified (no proof, no reproducing execution) — consensus is not a verdict; needs human review
Review partial — this diff was larger than one review pass covers, so later files were not reviewed; some findings may be missing.
Analysis details — impact, health, verification
Impact & health
Graphify review
Impact — 295 functions depend on the 292 functions this change touches.
Health — this change adds coupling hotspots:
- new:
parse_url()— 7 callers, 3 callees - new:
parse_args()— 5 callers, 3 callees - new:
run()— 1 callers, 10 callees - new:
route()— 1 callers, 6 callees - new:
a_document_offers_the_pack_and_downloads_only_after_the_click()— 0 callers, 10 callees - new:
a_covered_update_shows_and_opens_the_download_page()— 0 callers, 9 callees - new:
an_ended_trial_stops_conversions_until_a_license_is_entered()— 0 callers, 9 callees - new:
a_license_older_than_the_build_says_so()— 0 callers, 8 callees - …and 28 more — each is listed as a finding
Verification — 295 functions in the blast radius were not formally verified this run (proofs are advisory here).
Health delta baseline: last indexed commit 0fdb80f (diverged from this PR's base — delta is approximate).
Gate & verification
graphify gate
PASS — objectively clean (no health regressions, tests not run — proofs not run this pass (advisory)). Grounded, not self-assessed.
Advisory (not blocking):
- verification_scope: 295 function(s) in the blast radius were not formally verified this run
Docs that may be stale (advisory)
.agents/skills/test-convt-cli/SKILL.md§ Drive (lines 37-73): references changed symbolstargets.agents/skills/test-convt-cli/SKILL.md§ Licensing (lines 74-102): references changed symbolstargets.agents/skills/test-convt-cli/SKILL.md§ Evidence (lines 103-113): references changed symbolserror.agents/skills/test-convt-cli/SKILL.md§ Linux file-manager menus (lines 114-142): references changed symbolsfiles,targets.agents/skills/test-convt-cli/SKILL.md§ Real file managers without root (lines 143-199): references changed symbolsnew,root.agents/skills/test-convt-cli/SKILL.md§ Linux bundle and document pack (lines 238-247): references changed symbolsstatus.agents/skills/test-convt-desktop/SKILL.md§ Test the convt desktop app (lines 6-26): references changed symbolsfiles,status.agents/skills/test-convt-desktop/SKILL.md§ Sign-in and renewal (lines 27-34): references changed symbolserror,status.agents/skills/test-convt-desktop/SKILL.md§ Update check (lines 35-42): references changed symbolsstatus.agents/skills/test-convt-desktop/SKILL.md§ Headless tests first (lines 43-65): references changed symbolsnew
…and 10 more.
· 1 grounded finding(s) anchored inline below; 35 more finding(s) on lines outside this diff (see the check run).
 |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
d2f41c7 to
bb5dd6b
Compare
5b92899 to
5590800
Compare
| # Windows Explorer menu | ||
|
|
||
| Windows 11 shows third-party items in the new compact context menu only through an `IExplorerCommand` COM handler registered by a packaged app. Classic registry verbs (`HKCR\*\shell`) still work, but only under "Show more options". | ||
| The per-user MSI installs **Convert with convt** in Explorer. Windows 11's compact menu uses a sparse MSIX identity and the Rust `convt-shell` COM handler. Windows 10 and **Show more options** use the same handler through classic HKCU registry verbs. No administrator privileges are needed for those registry entries. |
There was a problem hiding this comment.
Following the documented MSI installation does not enable the new Explorer menu. build.ps1 still builds and copies only convt.exe and convt-app.exe. convt.wxs contains no Explorer or COM registration, and the checkout has no sparse package manifest or registration helper.
Add the DLL build, payload copy, and registration steps rather than leaving them only in this description.
Confidence this needs a change: 5/5.
| let bytes = std::fs::read(&path); | ||
| let _ = std::fs::remove_file(&path); | ||
| match bytes.and_then(|bytes| serde_json::from_slice(&bytes).map_err(std::io::Error::other)) { | ||
| Ok(request) => on_request(request), |
There was a problem hiding this comment.
If another process holds the request JSON file open without allowing deletion, the Windows listener still reads and dispatches it after remove_file fails. The same file is then read and run again every 50 milliseconds, so one menu click can queue repeated conversions.
Claim each file once and require successful removal or a completed marker before dispatching it.
Confidence this needs a change: 5/5.
| fn request_dir() -> PathBuf { | ||
| std::env::var_os("CONVT_RUNTIME_DIR") | ||
| .map(PathBuf::from) | ||
| .unwrap_or_else(|| std::env::temp_dir().join("convt")) | ||
| } |
There was a problem hiding this comment.
If XDG_RUNTIME_DIR is set on Windows and CONVT_RUNTIME_DIR is unset, request_dir and the app's runtime_dir choose different folders. The app reads $XDG_RUNTIME_DIR/convt, but Explorer writes to the temporary directory. Choosing a menu target therefore never delivers the conversion to the app.
Use the same folder rules on both sides.
Confidence this needs a change: 5/5.
| fn request_dir() -> PathBuf { | |
| std::env::var_os("CONVT_RUNTIME_DIR") | |
| .map(PathBuf::from) | |
| .unwrap_or_else(|| std::env::temp_dir().join("convt")) | |
| } | |
| fn request_dir() -> PathBuf { | |
| if let Some(dir) = std::env::var_os("CONVT_RUNTIME_DIR") { | |
| return dir.into(); | |
| } | |
| if let Some(dir) = std::env::var_os("XDG_RUNTIME_DIR").filter(|d| !d.is_empty()) { | |
| return PathBuf::from(dir).join("convt"); | |
| } | |
| std::env::temp_dir().join("convt") | |
| } |
| let lists: Vec<_> = paths.iter().map(|p| targets(p)).collect(); | ||
| let targets = crate::common_targets(&lists); |
There was a problem hiding this comment.
Menu construction probes every selected path before checking whether any shared targets remain. Failed probes are not cached. If CLI startup repeatedly reaches the two-second deadline, selecting 256 PNG files can launch the same failing probe 256 times and make one menu call take roughly eight minutes. EnumSubCommands has the same loop.
Keep probe results for the current selection, stop when the intersection is empty, and bound the whole call.
Confidence this needs a change: 5/5.
| handoff(&paths, target)?; | ||
| if !app_running() { | ||
| Command::new(install_dir()?.join("convt-app.exe")) | ||
| .creation_flags(CREATE_NO_WINDOW.0) | ||
| .stdin(Stdio::null()) | ||
| .stdout(Stdio::null()) | ||
| .stderr(Stdio::null()) | ||
| .spawn() | ||
| .map_err(|_| error())?; |
There was a problem hiding this comment.
If starting convt-app.exe fails, Invoke leaves the completed request file behind. A later app launch scans and runs that old request without an age check, so a failed menu conversion can unexpectedly run later.
Return the request path from handoff and clean it up when launch fails.
Confidence this needs a change: 5/5.
| let f = Fixture::new(cx); | ||
| let png = f.png("explorer selection.png"); | ||
| let mut request = cli(vec![png], Some("jpeg"), None); | ||
| request.show_progress = true; | ||
| cx.update(|cx| super::route(request, cx)); | ||
| assert!(window_of::<MainView>(cx).is_some()); | ||
| cx.read(|cx| assert!(!f.app.read(cx).quit_when_idle)); |
There was a problem hiding this comment.
This test never opens an existing window on Automations and never asserts that the page changes to Activity. A fresh MainView already starts on Activity, so removing the new set_page call would still pass the test. The reused-window progress bug could return unnoticed.
Add the reused-window case and assert its page to protect the regression described in the PR.
Confidence this needs a change: 5/5.
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
a13f94d to
304d321
Compare
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Use the windows-sys CloseHandle and a null-pointer check in convt-app, SYNCHRONIZE access for the shell's mutex probe, and keep the primary's mutex for the life of the process. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
c814e1c to
279f9bf
Compare
| if let Some((handle, view)) = &opened { | ||
| let _ = handle.update(cx, |_, window, _| window.activate_window()); | ||
| view.update(cx, |view, cx| { | ||
| view.set_page(main_window::Page::Activity, cx) |
There was a problem hiding this comment.
Reopening the app switches an existing Automations window to Activity, even without an Explorer request. app.on_reopen(ui::show_main) reaches this unconditional set_page on Linux and macOS too. Move the page change into the request.show_progress branch so normal reopen keeps the user's page.
Confidence: 5/5.
| } | ||
| // Deliberately do not FreeLibrary: COM references still own DLL code. | ||
| drop(OsString::from_wide(&[])); | ||
| CoUninitialize(); |
There was a problem hiding this comment.
The --registered probe calls CoUninitialize while root still holds the remote COM command and items is still alive. Rust drops both after this block, so their final Release calls run after COM has shut down. This can crash cleanup after otherwise successful checks. Drop these references before CoUninitialize, or use a guard that is dropped last.
Confidence: 4/5.
| CoUninitialize(); | |
| drop(items); | |
| drop(root); | |
| CoUninitialize(); |
Summary
Windows had no Explorer conversion menu. This adds a per-user Rust COM submenu for Windows 11 through a sparse MSIX identity, with classic HKCU registration for Windows 10 and Show more options. Targets come from the installed CLI registry; choosing one opens the desktop Activity view and converts beside the input without a console window.
flowchart LR Explorer --> Handler[ExplorerCommand DLL] Handler --> Targets[convt targets --menu] Handler --> Request[Desktop progress request] Request --> Activity[Activity view and conversion] MSI --> HKCU[Classic registration] MSI --> MSIX[Sparse package identity]Evidence
Get-AppxPackage Convt.Desktopreturned the identity, and direct DLL plus registered COM surrogate activation returned the same targets as the installed CLI for PNG, MOV, PDF, unsupported files, and mixed selections.bun run rs:check, Windows cross-target clippy, and real SVG → PNG output signature/dimensions. Independent GPT-6.1 Sol review findings were fixed and verified.Merge Danger
Door: two-way. Reverting the change and uninstalling the MSI removes its registration; the tested uninstall removes both menu paths.
Blast radius: Windows Explorer, the per-user installer, and Windows desktop conversion launch. The handler bounds CLI probes to two seconds and preserves COM apartment affinity. Linux/Finder request defaults remain unchanged. This stacks on PR #6 and must follow it.
A public Windows 11 release needs a real signing certificate matching
CN=Convt. The locally generated verification certificate and trust entries were removed after testing; unsigned builds install the classic menu only.Fixes CNV-11
Created with GPT-6.1 Sol in T3 Code.