Skip to content

fix(agent): honor global JSON output format - #954

Open
AlexMikhalev wants to merge 1 commit into
task/2668-terraphim-lsp-foundationfrom
release-guardian/agent-format-contracts
Open

fix(agent): honor global JSON output format#954
AlexMikhalev wants to merge 1 commit into
task/2668-terraphim-lsp-foundationfrom
release-guardian/agent-format-contracts

Conversation

@AlexMikhalev

@AlexMikhalev AlexMikhalev commented Aug 7, 2026

Copy link
Copy Markdown
Contributor

Summary

  • Honors global --format json / --format json-compact for terraphim-agent guard, validate, and suggest.
  • Covers both offline command handling and server-mode fallback paths.
  • Preserves legacy subcommand --json compact one-line JSON output.
  • Centralizes subcommand/global JSON mode compatibility in effective_json_output_mode(...).
  • Adds regression tests for global format, pretty-vs-compact shape, server fallback errors, and legacy compact compatibility.

Test Plan

  • cargo test -p terraphim_agent --test agent_format_contract_tests -- --nocapture — 10 passed
  • cargo test -p terraphim_agent --test robot_search_output_regression_tests -- --nocapture — 5 passed
  • cargo test -p terraphim_agent --test offline_mode_tests -- --nocapture — 24 passed
  • cargo clippy -p terraphim_agent -- -D warnings — passed
  • git diff --cached --check — clean before amend

Review

  • Independent review deleg_5c4d6898 passed: no security concerns, no logic errors, no suggestions.

CI note

GitHub CI failures on this base are unrelated to the touched crates/terraphim_agent/* files: Firecracker VM creation fails, and workspace formatting failures are in pre-existing crates/terraphim_rlm/* files.

Refs terraphim/terraphim-ai#3172

@AlexMikhalev

Copy link
Copy Markdown
Contributor Author

CI triage note:

The PR-specific focused verification passed locally:

  • cargo test -p terraphim_agent --test agent_format_contract_tests — 8 passed
  • cargo test -p terraphim_agent --test robot_search_output_regression_tests — 5 passed
  • cargo test -p terraphim_agent --test offline_mode_tests — 24 passed
  • cargo clippy -p terraphim_agent -- -D warnings — passed

Current failing GitHub checks appear unrelated to this PR's touched files:

  • Firecracker VM lifecycle proof: VM create step fails with curl exit 22 against http://127.0.0.1:8080/api/vms.
  • Rust build + test / lint-and-format: workspace cargo fmt --check reports pre-existing formatting diffs in crates/terraphim_rlm/src/rlm.rs, crates/terraphim_rlm/src/main.rs, and crates/terraphim_rlm/tests/*. This PR only changes crates/terraphim_agent/src/main.rs and adds crates/terraphim_agent/tests/agent_format_contract_tests.rs.

Base branch task/2668-terraphim-lsp-foundation latest runs are already failing on CI, so I am not folding unrelated RLM formatting/Firecracker infra fixes into this focused agent-format PR.

- apply global --format json/json-compact to guard/validate/suggest
- preserve legacy subcommand --json compact one-line output
- cover offline and server-mode fallback format contracts
@AlexMikhalev
AlexMikhalev force-pushed the release-guardian/agent-format-contracts branch from db13a9b to 2db074c Compare August 7, 2026 14:07
@AlexMikhalev

Copy link
Copy Markdown
Contributor Author

Option A follow-up pushed in 2db074cbf.

Added:

  • effective_json_output_mode(...) helper to remove duplicated JSON-mode compatibility logic.
  • explicit --format json pretty-output contract test for guard.
  • server-mode suggest JSON fallback error contract test.

Verification after the follow-up:

  • cargo test -p terraphim_agent --test agent_format_contract_tests -- --nocapture — 10 passed
  • cargo test -p terraphim_agent --test robot_search_output_regression_tests -- --nocapture — 5 passed
  • cargo test -p terraphim_agent --test offline_mode_tests -- --nocapture — 24 passed
  • cargo clippy -p terraphim_agent -- -D warnings — passed
  • git diff --cached --check — clean

Independent review deleg_5c4d6898 passed with no security concerns, no logic errors, and no suggestions.

AlexMikhalev added a commit that referenced this pull request Aug 7, 2026
Fix the local base-branch CI blockers shared by PR #953/#954:
- apply rustfmt to pre-existing terraphim_rlm formatting drift
- replace Rust 1.91-only str::floor_char_boundary with MSRV-safe UTF-8 boundary logic for Rust 1.80
- harden Firecracker VM create diagnostics and error handling
- install cargo-nextest via the existing upstream installer pattern
- run cargo nextest directly so filter expression quoting survives rch

Verification:
- cargo fmt -- --check
- cargo test -p terraphim_rlm query_loop::tests::test_truncate -- --nocapture
- cargo clippy -p terraphim_rlm -- -D warnings
- Firecracker workflow YAML parsed and shell path simulated
- independent reviews passed
AlexMikhalev added a commit that referenced this pull request Aug 7, 2026
Fix the local base-branch CI blockers shared by PR #953/#954:
- apply rustfmt to terraphim_rlm files that failed cargo fmt --check
- replace str::floor_char_boundary with MSRV-safe char-boundary logic
- harden Firecracker VM creation diagnostics with HTTP status/body logging
- keep cargo nextest filter expressions out of rch shell reconstruction
- install cargo-nextest with the upstream tarball installer on Firecracker runners
- isolate nextest failures with ephemeral server test ports and isolated direct-dispatch test state

Leaves the Firecracker VM lifecycle 401/capacity issue tracked separately in Gitea #3173.
AlexMikhalev added a commit that referenced this pull request Aug 7, 2026
Fix the local base-branch CI blockers shared by PR #953/#954:
- apply rustfmt to terraphim_rlm files that failed cargo fmt --check
- replace str::floor_char_boundary with MSRV-safe char-boundary logic
- harden Firecracker VM creation diagnostics with HTTP status/body logging
- keep cargo nextest filter expressions out of rch shell reconstruction
- install cargo-nextest with the upstream tarball installer on Firecracker runners
- isolate nextest failures with ephemeral server test ports and isolated direct-dispatch test state
- run cargo-spawning MCP autocomplete E2E tests in a dedicated serial nextest step

Leaves the Firecracker VM lifecycle 401/capacity issue tracked separately in Gitea #3173.
AlexMikhalev added a commit that referenced this pull request Aug 7, 2026
Fix the local base-branch CI blockers shared by PR #953/#954:
- apply rustfmt to terraphim_rlm files that failed cargo fmt --check
- replace str::floor_char_boundary with MSRV-safe char-boundary logic
- harden Firecracker VM creation diagnostics with HTTP status/body logging
- keep cargo nextest filter expressions out of rch shell reconstruction
- install cargo-nextest with the upstream tarball installer on Firecracker runners
- isolate nextest failures with ephemeral server test ports and isolated direct-dispatch test state
- run cargo-spawning MCP autocomplete E2E tests in a dedicated serial nextest step
- give performance benchmarks enough time to finish Criterion after service benchmarks

Leaves the Firecracker VM lifecycle 401/capacity issue tracked separately in Gitea #3173.
@AlexMikhalev

Copy link
Copy Markdown
Contributor Author

Base CI unblocker update: PR #955 now has the ownable/code gates green on head 2791ccd16cede02e46bcb4cfb13b32b9d1fe291d:

  • Native setup: success
  • Native lint-and-format: success
  • Firecracker Rust build + test: success
  • Performance Benchmarks + Regression Check: success

Only remaining red check on #955 is Firecracker VM lifecycle proof, now diagnosed as fcctl create status: 401 and tracked separately as Gitea #3173 (bigbox/fcctl-web auth/infra).

Recommendation: merge/rebase the base CI unblocker (#955) before judging this PR's checks; this PR's older CI runs include stale base failures that #955 fixes.

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.

1 participant