Skip to content

Comments

feat(cli): wire CLI commands to mofa-kernel/foundation backend (issue #146)#2

Closed
Copilot wants to merge 1 commit intofeature/cli-backend-connections-146-rebasedfrom
copilot/review-pr-219-for-issue-146
Closed

feat(cli): wire CLI commands to mofa-kernel/foundation backend (issue #146)#2
Copilot wants to merge 1 commit intofeature/cli-backend-connections-146-rebasedfrom
copilot/review-pr-219-for-issue-146

Conversation

Copy link

Copilot AI commented Feb 22, 2026

📋 Summary

All mofa-cli command handlers were empty stubs returning Ok(()). This PR replaces every stub with real backend integration — agent lifecycle through AgentRegistry, sessions through SessionManager, plugins through SimplePluginRegistry, and tools through ToolRegistry — plus file-backed persistence so state survives process restarts.

🔗 Related Issues

Related to mofa-org#146


🧠 Context

CLI commands called // TODO: Implement and returned immediately. No connection existed to the kernel/foundation service layer. A previous attempt built a parallel JSON-file backend that didn't touch existing registries; this PR wires directly to the established crates instead.


🛠️ Changes

  • store.rs (new) — Generic PersistedStore<T> writing per-item .json files; safe filename sanitization prevents path traversal
  • context.rsCliContext now owns AgentRegistry, SessionManager, SimplePluginRegistry, ToolRegistry, and three PersistedStore instances; registers CliBaseAgentFactory; seeds and replays plugin/tool specs on startup; migrates legacy nested session dirs
  • agent/start.rscreate_and_register() via selected factory (--type flag added); atomic: rolls back in-memory registration if persistence fails
  • agent/stop.rsagent.shutdown() → persist "Stopped"registry.unregister(); restores persisted state if unregister fails
  • agent/restart.rs — delegates to stop::run + start::run
  • agent/list.rs — merges live registry entries with agent_store so stopped-but-persisted agents appear
  • plugin/uninstall.rs — marks spec enabled=false in plugin_store before unregistering; rollback on failure
  • session/export.rs, show.rs, list.rs — switched from get_or_create() to new SessionManager::get() that returns Option without side-effects
  • mofa-foundation/session.rs — added SessionManager::get(); fixed list() to read canonical key from JSONL header (preserves _ in keys, no more _: corruption)

🧪 How you Tested

  1. cargo test -p mofa-cli — 37 unit + 4 integration tests, all pass
  2. cargo test -p mofa-foundation — 203 tests pass
  3. Key scenarios covered by tests: start→stop→restart chain, duplicate-start error, unknown factory type, persist-across-context-instances, disabled plugin not replayed, legacy session migration, session list without phantom creation

📸 Screenshots / Logs (if applicable)

running 37 tests
test commands::agent::start::tests::test_start_succeeds_with_default_factory ... ok
test commands::agent::stop::tests::test_stop_updates_state_and_unregisters_agent ... ok
test commands::agent::restart::tests::test_restart_chain_start_stop_restart_list ... ok
test context::tests::test_agent_store_persists_across_context_instances ... ok
test context::tests::test_plugin_and_tool_specs_replayed_on_startup ... ok
...
test result: ok. 37 passed; 0 failed

⚠️ Breaking Changes

  • No breaking changes

If breaking:


🧹 Checklist

Code Quality

  • Code follows Rust idioms and project conventions
  • cargo fmt run
  • cargo clippy passes without warnings

Testing

  • Tests added/updated
  • cargo test passes locally without any error

Documentation

  • Public APIs documented
  • README / docs updated (if needed)

PR Hygiene

  • PR is small and focused (one logical change)
  • Branch is up to date with main
  • No unrelated commits
  • Commit messages explain why, not only what

🚀 Deployment Notes (if applicable)

On first run after upgrade, CliContext::new() creates ~/.local/share/mofa/{agents,plugins,tools}/ directories and seeds default plugin/tool specs. Existing session files under the legacy sessions/sessions/ nesting are migrated automatically.


🧩 Additional Notes for Reviewers

  • PersistedStore sanitizes IDs to [a-zA-Z0-9\-_.] before constructing file paths — no additional validation needed at call sites
  • Rollback paths in start/stop/uninstall are best-effort; failure messages distinguish "operation failed" from "rollback also failed"
  • SessionManager::get() is the canonical non-mutating lookup — callers that previously used get_or_create() and then checked is_empty() now get a proper None instead

💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.

Copilot AI changed the title [WIP] Check PR #219 for issue #146 resolution feat(cli): wire CLI commands to mofa-kernel/foundation backend (issue #146) Feb 22, 2026
Copilot AI requested a review from Rahul-2k4 February 22, 2026 09:30
@Rahul-2k4 Rahul-2k4 deleted the branch feature/cli-backend-connections-146-rebased February 23, 2026 08:54
@Rahul-2k4 Rahul-2k4 closed this Feb 23, 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.

2 participants