refactor(cli): replace the output globals with an explicit Out#322
Conversation
socksy
force-pushed
the
refactor/output-explicit-writer
branch
from
27ff998 to
d34e910
Compare
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
socksy
force-pushed
the
refactor/output-explicit-writer
branch
3 times, most recently
from
a4c7b3a to
2709898
Compare
Where CLI output went was controlled by two mutable globals: an OUTPUT_MODE and a CURRENT_SENDER that the MCP server set and cleared around each tool call. Every output:: function read them to choose between stdout, JSON, and the MCP peer, so a command never said where its own output went. Pass the destination in as a value instead: commands take an Out and the writing functions become methods on it. The CLI builds one over stdout; an MCP tool builds one whose writer forwards each line to that call's channel. The sender and the notification flag live on the value, not in globals.
socksy
force-pushed
the
refactor/output-explicit-writer
branch
from
2709898 to
a0233a2
Compare
| pub fn clean_head_sha(dir: &Path) -> Option<String> { | ||
| if !is_inside_work_tree(dir) { | ||
| debug!("{:?} is not inside a git worktree; skipping idempotency key", dir); | ||
| debug!( |
There was a problem hiding this comment.
unrelated but cargo fmt was failing on this file
konstantinoscs
left a comment
konstantinoscs
left a comment
There was a problem hiding this comment.
LGTM, I wonder if there's a way to actually test the concurrent call scenario (and that the channels never cross-talk
3 participants
Where CLI output went was controlled by two mutable globals: an
OUTPUT_MODEand aCURRENT_SENDERthat the MCP server set and cleared around each tool call. Everyoutput::*function read them to choose between stdout, JSON, and the MCP peer. So a command never said where its output went. That was decided elsewhere, out of sight, and the MCP path had to juggle a global sender to capture anything a command printed.This passes the destination in as a value instead. Commands take an
Out, and the writing functions become methods on it. The CLI builds one over stdout. For an MCP tool, theOutwraps a writer that forwards each line to that call's channel. The sender and the notification flag live on the value now, not in globals.Most of the ~17-file diff is the mechanical
output::foo(x)→out.foo(x)sweep.Outis cloneable so the spawned log and status monitors can share the writer, which is why it sits behind anArc<Mutex>. Tests, build, and fmt pass.