Skip to content

Deepen mdo C6: retire divergent test mirror and dead pub surface - #43

Closed
maphew wants to merge 2 commits into
agent/6w5-c1-convert-requestfrom
agent/6w5-c6-test-mirror
Closed

maphew wants to merge 2 commits into
agent/6w5-c1-convert-requestfrom
agent/6w5-c6-test-mirror

Conversation

@maphew

@maphew maphew commented Oct 7, 2026

Copy link
Copy Markdown
Owner

C6: Retire the divergent test mirror and dead public surface

Beads mdo-6w5.2 · parent epic mdo-6w5 (PR order 2 of 6; stacked on the C1 branch agent/6w5-c1-convert-request)

What

  • tests/cli.rs:618-633 re-implemented temp_output_for as a mirror that had already lost stem sanitization. The four --open call sites now call mdo_cli::temp_output_for directly, so the tests compute precisely the paths production computes. Mirror deleted (plus the Hash/Hasher imports only it needed).
  • derived_output alias (src/lib.rs:137, one caller) deleted; main.rs calls the real target input.with_extension("html").
  • SETUP_SAMPLE_FILE_NAME / SETUP_SAMPLE_MARKDOWN (zero external callers) made private.
  • wrapper.rs's empty_path_dir duplication deliberately untouched — recorded in the issue as having an honest reason.

Gates

fmt --check ✓ · clippy -D warnings ✓ · cargo test --all-targets --all-features --locked ✓ (90 tests, including the Linux --open suite that now exercises the real path contract)

Recorded byte-identity work from C1 is preserved: no CLI-visible strings changed in this PR.

Signed: kilo-kilo-auto-efficient on behalf of matt (Beads mdo-6w5.2)

C6 / Beads mdo-6w5.2 (epic mdo-6w5, PR order 2 of 6; stacked on the C1
branch).

- tests/cli.rs re-implemented temp_output_for as temp_output_for_test and
  had already lost stem sanitization (diverged from production). The four
  call sites now call mdo_cli::temp_output_for directly, so tests compute
  the same per-source private paths production does, sanitization included.
  The mirror is deleted, along with the Hash/Hasher imports it alone needed.
- The one-line derive_output alias had a single caller; main.rs now calls
  the real target, input.with_extension("html"), and lib no longer exposes
  the alias.
- SETUP_SAMPLE_FILE_NAME / SETUP_SAMPLE_MARKDOWN had zero external callers;
  both are now private. (Windows SetupSample? not referenced: mdo-setup and
  mdo-open never touch them.)
- wrapper.rs's own empty_path_dir duplication is intentionally untouched:
  recorded in the issue as having an honest reason.

Gates: fmt --check, clippy -D warnings, cargo test --all-targets
--all-features --locked (90 tests) all green, including the Linux --open
tests that exercise the corrected temp-output path contract.

Beads-Ref: mdo-6w5.2

Agent-Signature: kilo-kilo-auto-efficient on behalf of matt
Comment thread tests/cli.rs
use std::process::Command;
use std::time::{SystemTime, UNIX_EPOCH};

use mdo_cli::temp_output_for;

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

WARNING: Unused import on non-Linux test builds - cargo clippy --all-targets -D warnings fails on Windows/macOS

Every caller of temp_output_for in this integration-test file (open_output_refuses_precreated_symlink, open_launch_failure_exits_nonzero_and_reports_path, open_success_exits_zero, open_reports_failure_when_opener_exits_nonzero) is #[cfg(target_os = "linux")], so on any non-Linux target this import is unused and -D warnings turns the warning into an error. The deleted Hash/Hasher import directly below this line was cfg-gated the same way, which is this file's convention (lines 8-12).

Suggested change
use mdo_cli::temp_output_for;
#[cfg(target_os = "linux")]
use mdo_cli::temp_output_for;

Reply with @kilocode-bot fix it to have Kilo Code address this issue.

@kilo-code-bot

kilo-code-bot Bot commented Oct 7, 2026 •

Copy link
Copy Markdown

Code Review Summary

Status: No Issues Found | Recommendation: Merge

Files Reviewed (3 files)
  • src/lib.rs
  • src/main.rs
  • tests/cli.rs
Previous Review Summary (commit 9ca6598)

Current summary above is authoritative. Previous snapshots are kept for context only.

Previous review (commit 9ca6598)

Status: 1 Issue Found | Recommendation: Address before merge

Fix these issues in Kilo Cloud

Overview

Severity Count
CRITICAL 0
WARNING 1
SUGGESTION 0
Issue Details (click to expand)

WARNING

File Line Issue
tests/cli.rs 6 use mdo_cli::temp_output_for; is unconditional, but all four call sites are #[cfg(target_os = "linux")], so non-Linux cargo clippy --all-targets -D warnings builds fail on the unused import; gate the import #[cfg(target_os = "linux")] like the removed Hash/Hasher import (the file's convention).
Files Reviewed (3 files)
  • src/lib.rs - 0 issues
  • src/main.rs - 0 issues
  • tests/cli.rs - 1 issue

Reviewed by efficient · Input: 24.7K · Output: 2.8K · Cached: 112.6K

The import was only used in #[cfg(target_os = linux)] tests but was unconditional,
causing clippy -D warnings to fail on macOS/Windows with unused_imports.
This is a follow-up to Beads mdo-6w5.2 (C6).

Agent-Signature: kilo-kilo-auto-efficient on behalf of matt
@maphew
maphew deleted the branch agent/6w5-c1-convert-request October 7, 2026 16:43
@maphew maphew closed this Oct 7, 2026
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