Repository navigation
Conversation
![kilo-code-bot[bot]](https://avatars.githubusercontent.com/in/2193792?s=60&v=4){kind=small}
| /// Register the file-manager integration for this user. On Linux this | ||
| /// writes the hidden `mdo.desktop` entry and its icon through a pure | ||
| /// planning step and a thin executor; on Windows it shells out to | ||
| /// `reg.exe` (see [`install_windows_plan`] for the registry shape). |
There was a problem hiding this comment.
SUGGESTION: Intra-doc link won't resolve on Linux doc builds
This doc comment is on the #[cfg(target_os = "linux")] install, but it links to install_windows_plan, which is #[cfg(target_os = "windows")] and private. On Linux cargo doc builds the target does not exist, so rustdoc emits a broken_intra_doc_links warning (no runtime impact). A plain-text reference keeps the doc accurate on both platforms. The same defect is at line 52 (LinuxInstallPlan doc).
Suggested change
| /// `reg.exe` (see [`install_windows_plan`] for the registry shape). | |
| /// `reg.exe` (see the Windows-only `install_windows_plan` planning step for the registry shape). |
Reply with @kilocode-bot fix it to have Kilo Code address this issue.
| /// Data-first description of a Linux integration install: everything the | ||
| /// executor touches, decided without disk or environment access so the | ||
| /// planning step stays a pure function (unit-tested without spawn-capture), | ||
| /// mirroring the registry's [`install_windows_plan`]. |
There was a problem hiding this comment.
SUGGESTION: Intra-doc link won't resolve on Linux doc builds
LinuxInstallPlan is #[cfg(target_os = "linux")] but links to install_windows_plan, which is #[cfg(target_os = "windows")] and private. On Linux cargo doc builds the link is unresolved (broken_intra_doc_links warning; harmless at runtime). Referring to the planner as plain text avoids the cross-cfg link.
Suggested change
| /// mirroring the registry's [`install_windows_plan`]. | |
| /// mirroring the registry's Windows-only `install_windows_plan` step. |
Reply with @kilocode-bot fix it to have Kilo Code address this issue.
Code Review SummaryStatus: 2 Issues Found | Recommendation: Address before merge Overview
Issue Details (click to expand)SUGGESTION
Files Reviewed (5 files)
Fix these issues in Kilo Cloud Previous Review Summary (commit fce849f)Current summary above is authoritative. Previous snapshots are kept for context only. Previous review (commit fce849f)Status: 2 Issues Found | Recommendation: Address before merge Overview
Issue Details (click to expand)SUGGESTION
Files Reviewed (1 files)
Reviewed by efficient · Input: 44.1K · Output: 8.2K · Cached: 216.3K |
C2 / Beads mdo-6w5.3 (epic mdo-6w5, PR order 3 of 6; stacked on the C6 branch). Watch mode's mechanics were interleaved with main.rs's CLI plumbing: parent-dir registration rationale, event relevance, the trailing-edge debounce drain, channel death, and the render-failure continuation all lived inline around ~180 lines with only two spawned-binary e2e tests as coverage, both unix-gated. New src/watch.rs in the lib crate owns all three mechanics behind one interface: - watch::run(input, render) - one deep watch entry point. Registers the watch BEFORE rendering (a save landing during the initial render is queued, not lost), runs the render callback once immediately, then once per settled burst. A render callback Err (transient filesystem state mid-rename) never ends watch mode; channel death ends it gracefully with Ok(()). - The debounce timing and the loop clock are injectable at pump level: unit tests script a fake event channel (relative arrival times) against a virtual clock, so the semantics are covered cross-platform by construction: burst collapse, bursts running longer than the window, events after the window, sibling noise, watcher error storms, channel death during a drain, callback errors, and event-relevance gates. - watch-mode UX lines (Watching / File changed, re-rendering / Watcher error) are the module's; rendering and its reporting stay with the CLI via the callback. main.rs keeps the CLI-local helpers (render_and_report, launch_open_- output) and its exit-code policy; the -watch -open launch hook stays a one-time duty consumed exactly on the first callback, as before the module existed. tests/cli.rs keeps one spawned-binary e2e smoke test (watch_survives_atomic_rename); the watch_ignores_sibling_changes e2e moves into the module unit tests. Gates: fmt --check, clippy --all-targets --all-features -D warnings, and cargo test --all-targets --all-features --locked all green (12 new watch unit tests; byte-identity harness against the pre-C1 binary still passes for the once-mode CLI surface). Beads-Ref: mdo-6w5.3 Agent-Signature: kilo-kilo-auto-efficient on behalf of matt
C3 / Beads mdo-6w5.4 (epic mdo-6w5, PR order 4 of 6; stacked on the C2 branch). The fact "the registered handler runs mdo --open <file> and siblings are found beside the running exe" was spelled six times: the Linux desktop Exec template, the Windows handler/registry command variants, mdo-open's forwarding argv per cfg-branch, mdo-setup's two sibling_binary copies, and windows setup's sibling resolution. New src/handler.rs in the lib crate owns the contract: - OPEN_FLAGS + open_argv(): the --open argv prefix spelled once; the file argument itself stays platform presentation (%f placeholder, %1 in a registry command, real path when spawned). - sibling_binary(exe, name): one discovery function, one error mode - a NotFound io error naming BOTH the anchor executable and the path that was checked - so callers print it as-is instead of inventing their own not-found story. - Desktop-entry template, Windows registry command builders, and mdo-open's forward become thin adapters consuming it. Windows-only emission tests now derive their expectation from the contract instead of restating it (2 emitted-form tests: Linux Exec line + Windows registry command), so Linux and Windows cannot drift apart. - mdo-setup's two sibling_binary copies are deleted; the eager is_file check Windows onboarding used to spell inline now applies everywhere through the shared error. In-pass: mdo-open no longer exits silently. current_exe() failure, a missing sibling target in any branch, and spawn failures all print a message before the non-zero exit (a GUI-launched binary may still have no stderr to show it on Windows, but silence is worse everywhere). exit_code_from_status is split into exit_code_from_parts(success, code) so the truncation semantics are unit-testable portably: a failure whose low byte is zero (e.g. 0x100) must not read as success. Covered by four portable tests plus a Windows-only end-to-end (only Windows can produce such a status; the repo's CI runner exercises it). Gates: fmt --check, clippy --all-targets --all-features -D warnings, cargo test --all-targets --all-features --locked all green; the byte-identity harness against the pre-C1 binary still passes for the once-mode CLI surface. Beads-Ref: mdo-6w5.4 Agent-Signature: kilo-kilo-auto-efficient on behalf of matt
C5 / Beads mdo-6w5.5 (epic mdo-6w5, PR order 5 of 6; stacked on the C3 branch). Mirror image for the Linux side of the Windows shape the code already has (pure plan + thin executor + typed results): - linux_install_plan(data_home, current_exe, set_default) is a PURE function: the desktop-file/icon targets, the entry content (which now shows it carries the handler invocation contract), the default mimes, and the legacy-cleanup root - unit-testable without spawn-capture. - apply_linux_install_plan(plan) is the only file-touching step; it returns LinuxInstallReport (paths written + LinuxDefaultOutcome: Applied / NeedsManualHint / NotRequested) instead of reporting through stdout mid-apply. - print_linux_install_report(report) is the executor-caller's CLI surface; the exact lines mirror the pre-plan flow byte for byte (verified: install output diffed against the pre-epic binary modulo the fixture paths; both exit 0). - The three _impl trampolines (install/uninstall/integration_installed across linux/windows/not-any cfg bodies) are collapsed into the pub fns directly, on every platform including the not(any) fallbacks. - Dead pubs removed: install_linux_for_exe and install_windows_for_exe had zero external callers; the Windows body moves into install() verbatim (only the handler locals rename to avoid the new `handler` module import shadow). Doc references updated. SCOPE LINE per ADR-0004 (Deferred): strictly the v0.6 minimal owned-registration check - no repair flows, no remnant cleanup, no partial-state classification; reopening ADR-0004 is out of scope. The scripts/uninstall-*.legacy helper scripts are a known duplication but explicitly out of scope; a retirement decision would be its own ADR. Gates: fmt --check, clippy --all-targets --all-features -D warnings, cargo test --all-targets --all-features --locked all green (3 new Linux plan/executor tests; Windows-side tests unchanged and still asserting the registry plan byte-exactly). Beads-Ref: mdo-6w5.5 Agent-Signature: kilo-kilo-auto-efficient on behalf of matt
maphew
force-pushed
the
agent/6w5-c3-handler-contract
branch
from
October 7, 2026 17:06
9eea29c to
5f2bdf1
Compare
maphew
force-pushed
the
agent/6w5-c5-linux-plan
branch
from
October 7, 2026 17:06
fce849f to
2239ee7
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
C5: Shape Linux integration install like the Windows plan/executor
Beads mdo-6w5.5 · parent epic mdo-6w5 (PR order 5 of 6; stacked on C3
agent/6w5-c3-handler-contract→ C2 → C6 → C1)What
The Linux install flow gets the mirror image of the Windows shape (pure plan + thin executor + typed results):
linux_install_plan(data_home, current_exe, set_default)— pure, unit-tested without spawn-capture: desktop-file/icon targets, entry content (carrying the C3 handler-invocation contract), default mimes, legacy-cleanup root.apply_linux_install_plan(plan)— the only file-touching step; returnsLinuxInstallReport(Applied/NeedsManualHint/NotRequested) instead of reporting through stdout mid-apply.print_linux_install_report(report)— the executor-caller's CLI surface; lines mirror the pre-plan flow byte for byte (verified against the pre-epic binary: identical exit codes and output modulo the fixture paths)._impltrampolines (install/uninstall/integration_installedacross linux/windows/not-any) are collapsed into the pub fns on every platform, including thenot(any)Unsupported fallbacks.install_linux_for_exe,install_windows_for_exe(zero external callers); the Windows body moved intoinstall()verbatim. Doc references updated.Scope line (per ADR-0004, Deferred)
Strictly the v0.6 minimal owned-registration check — no repair flows, no remnant cleanup, no partial-state classification; reopening ADR-0004 is out of scope.
scripts/uninstall-*.sh/ps1legacy helpers are a known duplication but explicitly out of scope; a retirement decision would be its own ADR. Status stays infile_manager; dialogs and terminal spawning stay inmdo-setup.Gates
fmt --check ✓ · clippy -D warnings ✓ ·
cargo test --all-targets --all-features --locked✓ (77 lib tests incl. 3 new plan/executor tests; Windows registry-plan tests unchanged, still byte-exact) · render byte-identity harness ✓ · install-flow byte-diff ✓Signed: kilo-kilo-auto-efficient on behalf of matt (Beads mdo-6w5.5)