Files
openhuman/PLAN.md
T
325f019853 refactor(core): move CLI adapters out of src/core/ into per-domain cli modules (#758)
* refactor(voice): move standalone CLI adapter into voice domain

Move the blocking `openhuman voice` / `openhuman dictate` dictation-server
subcommand out of `src/core/cli.rs` and into a new `src/openhuman/voice/cli.rs`
owned by the voice domain. The core CLI dispatcher now routes to
`voice::cli::run_standalone_subcommand` and no longer imports voice internals.

Why this shape: the standalone server blocks forever on the hotkey listener,
which doesn't fit the controller registry's request/response contract. A
domain-owned CLI adapter is the right home for long-lived operational
commands; the controller registry stays for RPC-style capabilities.

Establishes the exposure rule documented in PLAN.md for the rest of the
backend overhaul.

- src/openhuman/voice/cli.rs: new 118-line adapter (flag parsing, tokio
  runtime, run_standalone call)
- src/openhuman/voice/mod.rs: declare `pub mod cli`
- src/core/cli.rs: delete 92-line run_voice_server_command, dispatch only
- PLAN.md: new scope doc + exposure rule

* refactor(text_input): move CLI adapter into domain

Move src/core/text_input_cli.rs to src/openhuman/text_input/cli.rs and
drop the stale `pub mod text_input_cli` from src/core/mod.rs. Core CLI
dispatcher now routes `text-input` to the domain-owned adapter.

The moved file has a mixed shape that validates the exposure rule from
the previous voice commit:
 - `run` starts a long-lived HTTP JSON-RPC server (domain cli.rs shape)
 - `read / insert / ghost / dismiss` are short-lived UX wrappers around
   `text_input::rpc::*` (pretty-print, flag parsing — CLI affordance, not
   transport duplication)

Both shapes legitimately belong in the domain. The controller registry
stays for transport-agnostic capabilities; CLI UX wrappers live next to
their domain RPC.

- git mv src/core/text_input_cli.rs src/openhuman/text_input/cli.rs
- src/openhuman/text_input/mod.rs: declare `pub mod cli`
- src/core/mod.rs: drop `pub mod text_input_cli`
- src/core/cli.rs: update dispatch path

* refactor(tree_summarizer): move CLI adapter into domain

git mv src/core/tree_summarizer_cli.rs src/openhuman/tree_summarizer/cli.rs
and drop the stale `pub mod tree_summarizer_cli` from src/core/mod.rs.
Core CLI dispatcher now routes `tree-summarizer` to the domain-owned
adapter.

All five subcommands (ingest / run / query / status / rebuild) are short
-lived UX wrappers over registry handlers that already exist in
tree_summarizer/schemas.rs — no long-running server, no business logic
duplicated. Straight relocation keeps transport generic.

- git mv src/core/tree_summarizer_cli.rs src/openhuman/tree_summarizer/cli.rs
- src/openhuman/tree_summarizer/mod.rs: declare `pub mod cli`
- src/core/mod.rs: drop `pub mod tree_summarizer_cli`
- src/core/cli.rs: update dispatch path

* refactor(screen_intelligence): move + split CLI adapter into domain

Move src/core/screen_intelligence_cli.rs (699 lines, 7 subcommands) into
a domain-owned cli/ submodule, split by responsibility so no single file
exceeds the project's 500-line soft limit.

Layout:
  screen_intelligence/cli/mod.rs        (204) dispatch + shared helpers
                                              (CliOpts, parse_opts,
                                              bootstrap_engine, logging,
                                              print_help)
  screen_intelligence/cli/server.rs      (79) run_server (long-running
                                              capture+vision loop)
  screen_intelligence/cli/session.rs    (150) status / start / stop
  screen_intelligence/cli/capture.rs    (173) capture / vision (inspect pair)
  screen_intelligence/cli/doctor.rs     (107) readiness diagnostics

Classification before moving:
 - no business-logic duplication vs schemas.rs handlers
 - heavy fns (doctor 103, capture 97, start 89) were 80%+ pretty-printing
   and CLI-side orchestration (polling loop, flag-driven save) — legit
   CLI UX, not extracted domain logic
 - shared helpers stayed pub(super) inside mod.rs; no new domain APIs
   introduced

Core transport changes:
 - delete src/core/screen_intelligence_cli.rs
 - drop `pub mod screen_intelligence_cli` from src/core/mod.rs
 - src/core/cli.rs dispatch routes to domain cli::

After this commit, src/core/cli.rs contains only dispatch lines for all
four migrated domains (voice, text_input, tree_summarizer,
screen_intelligence) — no embedded domain logic.

* refactor(screen_intelligence): address external review — extract capture save + tighten cli visibility

Follow-ups from codex + gemini review of the branch:

 1. Extract CaptureFrame construction + disk save from the CLI.
    `capture.rs --keep` was building a CaptureFrame inline (stamping
    timestamps, copying context fields) and calling
    `AccessibilityEngine::save_screenshot_to_disk` directly. Added
    `AccessibilityEngine::save_capture_test_result(workspace_dir,
    &CaptureTestResult, reason) -> Option<Result<PathBuf, String>>`
    so the CLI only passes the result through and picks a reason
    label. Domain owns the frame shape, not the CLI.

 2. Tighten CLI module visibility across all four migrated domains.
    `pub mod cli` -> `pub(crate) mod cli` and
    `pub fn run_*_command` -> `pub(crate) fn` for voice, text_input,
    tree_summarizer, screen_intelligence. Only src/core/cli.rs needs
    to see these; they're transport plumbing, not domain public API.

No behavior change.

* style: cargo fmt

* address review comments from coderabbitai

- screen_intelligence/cli/server: use effective keep_screenshots (opts.keep
  || config flag) for both SiServerConfig and status output, so the server
  behaves consistently with what the CLI reports.
- voice/cli: surface config load errors with a warning instead of silently
  falling back to Config::default(); reject unknown --mode values instead
  of silently coercing them to Push.
- PLAN.md: narrow the Phase 4 enforcement grep so it forbids domain
  imports/non-dispatcher references in the transport layer while still
  allowing crate::openhuman::<domain>::cli::run_*(...) dispatch calls,
  which this PR legitimately introduces.

* style: cargo fmt voice cli

* style: cargo fmt + bump tauri-cef vendor

* fix(screen_intelligence): narrow save_capture_test_result to pub(crate) (addresses @coderabbitai nitpick on engine.rs:486)

AccessibilityEngine::save_capture_test_result is only called from the
domain's own cli/capture.rs (crate-internal). Making it pub leaks it
into the public API surface unnecessarily — tighten to pub(crate) to
match the visibility-tightening done across the rest of this PR.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

* update

* chore: update .gitignore to include scheduled_tasks.lock in app/.claude

* ran formatter

---------

Co-authored-by: Jwalin Shah <jshah1331@gmail.com>
Co-authored-by: Steven Enamakel <enamakel@tinyhumans.ai>
Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
2026-04-22 14:30:31 -07:00

7.8 KiB
Raw Permalink Blame History

Backend Overhaul — Controller Registry Consolidation

Branch: feat/backend-overhaul Base: main @ 515c4f4b

Why

Project rule (CLAUDE.md): domain functionality exposed to CLI + JSON-RPC only via the controller registry (schemas.rs per domain + src/core/all.rs). Current state is 90% there — 34 domains fully migrated — but six CLI adapters and one JSON-RPC fallback still live in the transport layer. Finishing this lets us delete ~95KB of adapter code and enforce the rule with a lint/grep check.

Goals

  1. Zero domain-specific code in src/core/cli.rs, src/core/jsonrpc.rs, src/rpc/dispatch.rs.
  2. Every CLI command and JSON-RPC method dispatched through the shared registry.
  3. No behavior change — all existing CLI flags and RPC methods preserve names, params, outputs.
  4. Unregistered infrastructure modules documented as intentionally transport-agnostic.

Non-goals

  • No new domain functionality.
  • No changes to skills QuickJS runtime exposure (stays runtime-only).
  • No renames of existing CLI flags or RPC method names.
  • No touching memory-tree domain (sanil-23 has in-flight PRs #732 #733).

Current leakage

File LOC What's there Target
src/core/agent_cli.rs 21.4K agent CLI adapter openhuman/agent/schemas.rs
src/core/memory_cli.rs 15.3K memory CLI adapter openhuman/memory/schemas.rs
src/core/text_input_cli.rs 12.4K text_input CLI adapter openhuman/text_input/schemas.rs
src/core/screen_intelligence_cli.rs 26.5K screen_intelligence CLI adapter openhuman/screen_intelligence/schemas.rs
src/core/tree_summarizer_cli.rs 13.2K tree_summarizer CLI adapter openhuman/tree_summarizer/schemas.rs
src/core/cli.rs (voice block) ~2K run_voice_server_command() openhuman/voice/schemas.rs
src/rpc/dispatch.rs 76 openhuman.security_policy_info fallback openhuman/security/schemas.rs (or about_app)

Total: ~95KB of adapter code to fold into registry.

Exposure rule (decision tree)

Two legitimate shapes for exposing a domain capability. Pick by command shape, not by convenience:

  1. Controller registry (src/openhuman/<domain>/schemas.rs + src/core/all.rs). Use for request/response RPC-style capabilities — take params, do work, return JSON, done. The vast majority of domain methods. CLI access is free via the generic namespace dispatcher (openhuman <domain> <verb> --args).

  2. Domain-owned CLI adapter (src/openhuman/<domain>/cli.rs). Use only for standalone long-running / blocking operational commands — e.g. openhuman voice runs a hotkey listener forever, holds the process, logs to stderr. These don't fit the registry's fire-and-return JSON contract. The adapter lives inside the domain (not in src/core/cli.rs) so transport layer stays generic.

Hard rule: no domain-specific imports or logic in src/core/cli.rs, src/core/jsonrpc.rs, or src/rpc/dispatch.rs. The only allowed reference is the dispatch line that routes args[0] to the domain's CLI function. Everything else lives in the domain.

Don't invent a third shape. If a capability feels like it needs one, it probably belongs in one of the two above with a thin adjustment.

Approach

Each migration is a self-contained diff:

  1. Confirm domain already has schemas.rs with all_<domain>_registered_controllers().
  2. For each CLI subcommand in <domain>_cli.rs:
    • Identify the underlying domain function it calls.
    • Add a handle_<verb> fn in schemas.rs that takes Map<String, Value> and returns ControllerFuture.
    • Register it in all_<domain>_registered_controllers().
  3. Delete the <domain>_cli.rs file.
  4. Remove its import from src/core/cli.rs / mod.rs.
  5. Verify CLI flags still work via generic dispatcher — the registry already exposes them by method name.
  6. cargo check + cargo test -p openhuman per step.

The generic CLI adapter in cli.rs already knows how to route <domain> <verb> --key value to registry method openhuman.<domain>_<verb>. Migration is mostly deletion once handlers exist.

Phases

Phase 1 — Smallest adapter first (shakedown)

voice (~2K inline in cli.rs). Lowest risk: one server command, confirms the approach end-to-end before touching bigger files.

  • Commit: refactor(voice): move CLI adapter to controller registry

Phase 2 — Mechanical migrations (one PR each)

Order = smallest file first, so each PR is reviewable and CI catches issues early.

  1. text_input (12.4K) — likely thin wrappers.
  2. tree_summarizer (13.2K).
  3. memory_cli (15.3K) — coordinate with sanil-23 memory-tree PRs; rebase after #732/#733 merge or pair-review.
  4. agent_cli (21.4K).
  5. screen_intelligence (26.5K) — largest, leave for last.

Each: one commit, one PR, self-contained. No bundling.

Phase 3 — JSON-RPC dispatch fallback

Move openhuman.security_policy_info from src/rpc/dispatch.rs into whichever domain owns security policy (likely about_app or a new security module). Delete rpc/dispatch.rs if it becomes empty, else leave as a thin generic dispatch shim.

Phase 4 — Enforce the rule

Add a build-time check or CI grep. The rule must forbid domain imports/types in the transport layer while still allowing the dispatcher to call the generic ::cli::run_* entrypoints that live inside each domain:

# Forbid `use crate::openhuman::<domain>::…` imports in transport files.
! grep -rE '^\s*use\s+crate::openhuman::[a-z_]+::' \
    src/core/cli.rs src/core/jsonrpc.rs src/rpc/dispatch.rs

# Forbid any other domain reference that is NOT a call into ::cli::run_*(…).
# (The dispatcher is allowed to invoke `crate::openhuman::<domain>::cli::run_<x>(...)`.)
! grep -rE 'crate::openhuman::[a-z_]+::' \
    src/core/cli.rs src/core/jsonrpc.rs src/rpc/dispatch.rs \
  | grep -vE 'crate::openhuman::[a-z_]+::cli::run_[a-z_]+\('

Fails if any domain-specific import or non-dispatcher reference creeps back in. Commit as chore(core): fence domain imports out of transport layer.

Phase 5 — Document unregistered modules

Add a short table to CLAUDE.md listing intentionally-unregistered infra modules (accessibility, approval, context, embeddings, integrations, node_runtime, overlay, providers, routing, tokenjuice, tool_timeout) with one-liner "why not in registry" for each. Also document skills as runtime-only.

Risks

  • agent_cli / screen_intelligence size — large files may hide subtle flag-parsing logic not obvious from a first read. Mitigation: test-drive each CLI subcommand manually after migration; add CLI smoke tests to the registry dispatcher if missing.
  • memory_cli collision with sanil-23 — memory-tree PRs touch adjacent paths. Mitigation: do phase 2.3 last, rebase on top of merged PRs.
  • CLI flag drift — generic adapter parses flags by schema. If current _cli.rs does custom parsing (e.g. positional args, subcommand nesting), need to extend schema before deleting. Mitigation: read each CLI file top-to-bottom before writing handler.
  • Behavior diff at registry boundary — registry handlers return ControllerFuture with JSON; _cli.rs may print formatted text. Mitigation: keep a thin formatter in the generic CLI path, not per-domain.

Verification per step

cargo check --manifest-path Cargo.toml
cargo test -p openhuman --test json_rpc_e2e
# Manual CLI smoke for migrated domain:
./target/debug/openhuman <domain> <verb> --args...

Plus yarn test:rust for the full mock-backed integration suite before each PR.

Milestones

  • M1: Phase 1 done, shakedown merged.
  • M2: Phases 2.12.5 done, all *_cli.rs deleted.
  • M3: Phase 3 done, rpc/dispatch.rs cleaned.
  • M4: Phase 4+5 done, rule enforced, docs updated.

Estimate: M1 = 0.5d, M2 = 34d (one domain/day), M3 = 0.5d, M4 = 0.5d. Total ~1 week of focused work.