refactor(mcp_server): extract write-dispatch and audit pipeline into write_dispatch.rs

[M3gA-Mind]

Summary

• Extracts dispatch_write_tool, audit_write, audit_write_rejection, audit_write_rejection_without_config, is_write_tool, summarize_write_args, and 6 private helpers out of the 2,758-line tools.rs into a new focused sibling module mcp_server/write_dispatch.rs
• tools.rs delegates to write_dispatch:: at all call sites; public behavior (tool responses, audit records, logging, correlation fields) is unchanged
• Adds 8 new targeted tests covering the dispatch error path, is_write_tool, now_ms, first_chars, and summarize_write_args edge cases; 7 existing tests moved into the new module

Result

File	Lines before	Lines after
tools.rs	2,758	2,330
write_dispatch.rs	—	598 (new)

All 9,490 Rust lib tests pass. TypeScript, ESLint, Prettier, and cargo fmt all clean.

Test plan

• cargo test -p openhuman --lib -- openhuman::mcp_server — 94 passed, 0 failed
• cargo fmt --all -- --check — clean
• cargo clippy -p openhuman — no errors
• pnpm --filter openhuman-app compile — clean
• pnpm --filter openhuman-app lint — 0 errors (pre-existing warnings only)
• pnpm --filter openhuman-app format:check — clean

Closes #2604

Summary by CodeRabbit

• Refactor
  • Reorganized internal code structure to enhance maintainability of write operations and audit logging functionality.

[coderabbitai]

📝 Walkthrough

Walkthrough

This PR extracts the write-tool dispatch and audit pipeline from the oversized tools.rs module into a focused sibling write_dispatch.rs module. Write-tool calls now delegate configuration loading, policy enforcement, RPC routing, and audit recording to the new module while preserving all existing MCP response and audit behavior.

Changes

Write dispatch module extraction and integration

Layer / File(s)	Summary
Module structure and public interface setup src/openhuman/mcp_server/mod.rs, src/openhuman/mcp_server/write_dispatch.rs	Adds write_dispatch module declaration and establishes module documentation and imports for JSON, configuration, security policy, and audit handling.
Configuration loading and policy enforcement src/openhuman/mcp_server/write_dispatch.rs	Public load_write_config and enforce_write_policy_for_config functions load RPC configuration with timeout handling and enforce per-tool write permissions via SecurityPolicy, rejecting writes on read-only settings. Helper is_write_tool identifies write tool names.
Write dispatch and audit recording src/openhuman/mcp_server/write_dispatch.rs	Core dispatch_write_tool routes RPC calls by fixed method name, invokes registered handlers, extracts document IDs from response envelopes, records success/failure to the audit store, and returns MCP-style payloads wrapping both success and error outcomes. audit_write helper handles async-safe audit row insertion using tokio or fallback thread spawning.
Audit rejection paths src/openhuman/mcp_server/write_dispatch.rs	Two audit rejection entry points: audit_write_rejection records immediately when config is available; audit_write_rejection_without_config spawns async task to load config and record, with graceful fallback logging on runtime unavailability.
Supporting utilities and helpers src/openhuman/mcp_server/write_dispatch.rs	Helpers for argument summarization with field omission and text truncation, document ID extraction from envelope structures (root or nested result.document_id), UTC timestamp generation in milliseconds, and param-key filtering for rejected-write audit records.
Test coverage for write dispatch module src/openhuman/mcp_server/write_dispatch.rs	Comprehensive unit and integration tests covering argument summarization (omissions, truncation, length calculation), policy denial behavior, document ID extraction from envelopes, write-tool identification, audit row recording for success and failure paths with async polling, timestamp freshness, and truncation boundary cases.
Tools.rs integration and visibility changes src/openhuman/mcp_server/tools.rs, src/openhuman/mcp_server/mod.rs	Updates imports to reference write_dispatch module; rewrites write-tool branches in call_tool to delegate configuration, policy, dispatch, and audit to the new module functions; changes tool_success and tool_error to pub(super) to allow sibling module access; removes ~428 lines of extracted implementations, write-audit tests, and document-ID extraction tests.

Sequence Diagram

sequenceDiagram
  participant call_tool
  participant write_dispatch
  participant RPC
  participant AuditStore
  call_tool->>write_dispatch: is_write_tool(name)
  write_dispatch-->>call_tool: true/false
  call_tool->>write_dispatch: load_write_config(tool_name)
  write_dispatch->>RPC: load config with timeout
  RPC-->>write_dispatch: Config or error
  write_dispatch-->>call_tool: Result<Config>
  call_tool->>write_dispatch: enforce_write_policy_for_config(tool_name, config)
  write_dispatch-->>call_tool: Ok() or InvalidParams error
  call_tool->>write_dispatch: dispatch_write_tool(tool_name, params, args, client_info, config)
  write_dispatch->>RPC: invoke mapped RPC method
  RPC-->>write_dispatch: result or handler error
  write_dispatch->>AuditStore: record audit row (success or failure)
  AuditStore-->>write_dispatch: row inserted
  write_dispatch-->>call_tool: Ok(tool response)

Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

• tinyhumansai/openhuman#2566: Implements the MCP write-tool audit pipeline that this PR modularizes into write_dispatch; both refactor write execution and audit recording in tools.rs.
• tinyhumansai/openhuman#2306: Both PRs refactor write-tool routing and dispatch logic in tools.rs, with this PR extracting that logic into a shared write_dispatch module.
• tinyhumansai/openhuman#2316: Both PRs modify the write-tool dispatch path for tree.tag and other write calls; this PR extracts that dispatch logic into the new write_dispatch module.

Suggested labels

working, rust-core

Suggested reviewers

• senamakel

Poem

🐰 A rabbit hops through tangled code,
"Too big!" it cries, "Let's split this load!"
Write dispatch now stands alone and clean,
The finest modular refactor seen.
One tool calls many, audit logs flow true—
Tools.rs breathes, and tests pass through! 🎯

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 22.22% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title accurately describes the main refactoring: extracting write-dispatch and audit pipeline into a new write_dispatch.rs module.
Linked Issues check	✅ Passed	All coding objectives from issue #2604 are met: write dispatch extracted to new module, public behavior unchanged, regression tests added, and diff coverage requirements satisfied.
Out of Scope Changes check	✅ Passed	All changes are directly related to extracting write-dispatch helpers; no unrelated modifications detected in the changeset.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

---

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

🧹 Nitpick comments (1)

src/openhuman/mcp_server/write_dispatch.rs (1)

508-546: ⚡ Quick win

Make this test prove the handler-error branch.

This still passes if openhuman.memory_doc_put is unregistered and dispatch_write_tool takes the None path, so it doesn't actually pin the Some(Err(_)) branch named in the test. Assert on the exact returned/audited error or set up a deterministic failing handler here.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/openhuman/mcp_server/write_dispatch.rs` around lines 508 - 546, The test
currently can pass via the None-path; make it exercise the Some(Err(_)) branch
by explicitly registering a deterministic failing write handler for the
"memory.store" tool before calling dispatch_write_tool (or otherwise ensure
openhuman.memory_doc_put is registered to return Err). Specifically, install a
handler that returns Some(Err("deterministic failure")) for "memory.store", then
call dispatch_write_tool and assert the returned tool_error content and the
audit row from list_writes (via McpWriteListQuery) contains the same
deterministic error message and success=false to prove the handler-error branch
executed.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Nitpick comments:
In `@src/openhuman/mcp_server/write_dispatch.rs`:
- Around line 508-546: The test currently can pass via the None-path; make it
exercise the Some(Err(_)) branch by explicitly registering a deterministic
failing write handler for the "memory.store" tool before calling
dispatch_write_tool (or otherwise ensure openhuman.memory_doc_put is registered
to return Err). Specifically, install a handler that returns
Some(Err("deterministic failure")) for "memory.store", then call
dispatch_write_tool and assert the returned tool_error content and the audit row
from list_writes (via McpWriteListQuery) contains the same deterministic error
message and success=false to prove the handler-error branch executed.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: bf02c5ce-1c3f-4e9c-8c1b-523f50586620

📥 Commits

Reviewing files that changed from the base of the PR and between 0d62e84 and 1c1a2ef.

📒 Files selected for processing (3)

• src/openhuman/mcp_server/mod.rs
• src/openhuman/mcp_server/tools.rs
• src/openhuman/mcp_server/write_dispatch.rs