Remove seven dead modules and correct roadmap citations - #6
Closed
MadTinker wants to merge 2 commits into
Closed
Conversation
MCP speaks JSON-RPC 2.0 over stdout, but the tracing subscriber defaulted to stdout too, so every log line was interleaved into the protocol stream. A client reading the first line for its initialize response got INFO ThreadId(01) tinker: src/main.rs:90: Starting Tinker Workshop... instead of JSON, and failed to parse. This affected any MCP client, including Claude Desktop using the configuration in the readme. Logging was also initialized before the arguments were parsed, so it could not know whether --mcp was set. Parse first, then point the subscriber at stderr when running as an MCP server. Non-MCP output is unchanged. The MCP write loop itself was already correct, and there are no other stdout writers in the tree, so the subscriber was the sole corruption source. This is why tests/mcp_tests.rs had three #[ignore]d tests. The attribute said they were skipped because they require building the binary, but they were failing on the corrupted stream. They pass now and are re-enabled, giving end-to-end protocol coverage that guards this regression. cargo test: 167 passed, 0 failed, 0 ignored (was 164 passed, 3 ignored). Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Pg53xf5xcUV7oc5CkaiCpz
None of these files was declared in lib.rs, main.rs, or any mod.rs, so none had ever compiled: src/browser/navigation.rs 167 lines src/browser/state_manager.rs 199 lines src/browser/window_manager.rs 158 lines src/browser/error.rs 153 lines src/browser/native_ui.rs 137 lines src/platform/macos.rs 124 lines src/browser/menu.rs 50 lines The cluster was self-referential: error.rs was imported only by navigation.rs, window_manager.rs, and state_manager.rs, all themselves dead. A naive "is this imported anywhere?" check finds references and concludes the code is live. Every remaining file under src/browser/ now maps to a declared module. Also corrects two errors in ROADMAP.md introduced when it was written from a static scan rather than a build: - It cited browser/navigation.rs as implementing per-tab navigation history. That file never compiled; the implementation is in browser/tabs.rs. - Its per-module test counts included tests in files that never ran. Verified: cargo build succeeds; cargo test reports 167 passed, 0 failed, 0 ignored — identical to before the removal, confirming the ~21 #[test] functions in the deleted files were never executed. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Pg53xf5xcUV7oc5CkaiCpz
MadTinker
force-pushed
the
claude/mcp-stdout-logging-fix
branch
from
August 22, 2026 18:46
0fb2ec7 to
cbe0af3
Compare
Collaborator
Author
|
Closing — this change has been folded into #4 rather than dropped. The dead-module removal is the same kind of work as #4's M1 cleanup (removing things the repo claimed existed but didn't), so keeping them as separate stacked PRs added review overhead without adding clarity. The commit is now the third of three in #4, and the stack is one level shallower:
Nothing was lost. Verified on the rebased tip: Generated by Claude Code |
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
2 participants
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.
What
Seven files, ~988 lines, none of which was declared in
lib.rs,main.rs, or anymod.rs— so none had ever compiled:#[test]fns (never ran)src/browser/state_manager.rssrc/browser/navigation.rssrc/browser/window_manager.rssrc/browser/error.rssrc/browser/native_ui.rssrc/platform/macos.rssrc/browser/menu.rsCombined with the two removed in #4 (
browser/tests.rs,src/cli/), that's nine dead modules totalling roughly 1,100 lines.Why it stayed hidden
The cluster was self-referential.
error.rswas imported bynavigation.rs,window_manager.rs, andstate_manager.rs— all themselves dead. So a "is this imported anywhere?" check finds real references and concludes the code is live. Only comparing files on disk againstmoddeclarations reveals it.native_ui.rsis the one worth a second look: its tests build real windows viaWindowBuilder, and are#[ignore]d on macOS for main-thread reasons but not elsewhere. Had that file ever been wired up, it would have failed on any CI runner without a display. Removing it now avoids a trap for whoever adds CI.Roadmap corrections
Two errors in
ROADMAP.md, both from writing it against a static scan rather than a build:browser/navigation.rsas implementing per-tab navigation history. That file never compiled — the implementation is inbrowser/tabs.rs.The file now carries a short note recording this, since its stated purpose is that every claim cites a checkable file. Citing files only helps if the citations are checked against what actually builds.
Verification
Identical to before the removal — which is the point. If any of those ~21 tests had been running, the count would have dropped.
Every remaining file under
src/browser/now maps to a declared module.Note
Git history retains all of this if any turns out to be worth reviving. Per discussion, the intent is to rewrite rather than resurrect if the functionality is wanted later —
state_manager.rsandwindow_manager.rsin particular read as an abandoned refactor rather than working code.Generated by Claude Code