fix(cli): address 5 review findings from PR #234 — legacy flag normalization gaps, hidden alias, robust JSON parsing, portable workspace root

[Copilot]

Five issues identified in the review of PR mofa-org#234 — two blocking regressions in normalize_legacy_output_flags plus three minor robustness gaps.

📋 Summary

Fixes two silent breakages in the legacy output-flag normalizer (--output=json equals form never rewritten; mofa session list -o json never normalized), plus three minor improvements to CLI alias visibility, JSON payload parsing, and workspace-root detection.

🔗 Related Issues

Related to #

---

🧠 Context

normalize_legacy_output_flags was introduced in PR mofa-org#234 to rewrite -o/--output → --output-format for backward compat. Two cases were missed:

1. Equals-sign form (--output=json) — clap receives it as-is; output_legacy is global = false, so clap errors with no helpful message.
2. session list — session was excluded from allows_global_after_command entirely (intentional for show/export which own a local -o), but that exclusion silently broke session list -o json, which has no local -o flag.

---

🛠️ Changes

• Fix 1 — equals-sign form (main.rs): the normalizer now rewrites --output=json / -o=json (single-token) in addition to the space-separated form, with the same positional and subcommand guards.
• Fix 2 — session list (main.rs): replaced the flat boolean allows_global_after_command for session with subcommand-aware logic — show and export still skip normalization; list and any future session subcommands without a local -o are normalized.
• Fix 3 — visible alias (cli.rs): visible_short_alias = 'o' on session show --format changed to short_alias = 'o' (hidden); -o still works, only -f appears in --help.
• Fix 4 — JSON parsing (lib.rs): extract_json_payload now uses serde_json::Deserializer::from_str(...).into_iter() to consume exactly one JSON value, tolerating trailing text or newlines that from_str rejects.
• Fix 5 — workspace_root() (lib.rs): replaces ancestors().nth(2) with an upward scan for the outermost Cargo.toml line-matching [workspace]; adds MOFA_WORKSPACE_ROOT env override; original depth used as fallback.
• Refactor: extracted is_output_format_value(s) helper to deduplicate the "text" | "json" | "table" pattern that appeared twice in the normalizer.

---

🧪 How you Tested

1. cargo test -p mofa-cli — 59 unit + 4 integration tests pass (9 new tests added for fixes 1 & 2).
2. cargo clippy -p mofa-cli --no-deps -- -D warnings — zero warnings.
3. cargo check --manifest-path examples/Cargo.toml -p cli_production_smoke — compiles clean.

---

📸 Screenshots / Logs (if applicable)

test result: ok. 59 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out
test result: ok. 4 passed;  0 failed; 0 ignored; 0 measured; 0 filtered out

New tests exercising the fixes:

tests::test_normalize_equals_form_before_command            ... ok
tests::test_normalize_short_equals_form_before_command      ... ok
tests::test_normalize_equals_form_after_agent_command       ... ok
tests::test_do_not_normalize_equals_form_unknown_value      ... ok
tests::test_normalize_session_list_short_output_flag        ... ok
tests::test_normalize_session_list_long_output_flag         ... ok
tests::test_normalize_session_list_equals_form              ... ok
tests::test_do_not_normalize_session_show_equals_form       ... ok
tests::test_do_not_normalize_session_export_equals_form     ... ok

---

⚠️ Breaking Changes

• No breaking changes

---

🧹 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

---

🧩 Additional Notes for Reviewers

• Fix 2 subcommand detection reads args[cmd_idx + 1] and skips it if it starts with -, so flag-before-subcommand patterns (mofa session --verbose list -o json) are handled correctly.
• The [workspace] line-level match (line.trim() == "[workspace]") avoids false positives from workspace = true member entries and comments; no new dependencies required.

Warning

Firewall rules blocked me from connecting to one or more addresses (expand for details)

I tried to connect to the following addresses, but was blocked by firewall rules:

• cdn.pyke.io
  • Triggering command: /home/REDACTED/work/mofa/.codeql-scratch/dbs/rust/working/target/debug/build/ort-sys-e05be62975cad138/build-script-build /home/REDACTED/work/mofa/.codeql-scratch/dbs/rust/working/target/debug/build/ort-sys-e05be62975cad138/build-script-build ebug/build/eyre--Wl,--version-script=/tmp/rustcYHTyQv/list ebug/build/eyre--Wl,--no-undefined-version cc 72c9�� (dns block)

If you need me to access, download, or install something from one of these locations, you can either:

• Configure Actions setup steps to set up my environment, which run before the firewall is enabled
• Add the appropriate URLs or hosts to the custom allowlist in this repository's Copilot coding agent settings (admins only)

---

💡 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.