wavecli: reconcile schema with live surfaces#1028
Conversation
There was a problem hiding this comment.
Code Review
This pull request enhances the CLI schema registry by introducing fields for output schema ID/version, side effects, positional arguments, and MCP-only flags. It updates several command schemas with new parameters, adds new MCP-only tools (daemon.balance and ark.oor.newaddress), and registers the ark.rounds.join method. Additionally, a new test suite (schema_parity_test.go) is introduced to ensure parity between the schema registry, the Cobra command tree, and the live MCP tools. Feedback on the PR suggests using cmd.Runnable() instead of checking cmd.RunE != nil in the parity test to more robustly identify runnable commands.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| method := strings.ReplaceAll(path, " ", ".") | ||
|
|
||
| if strings.HasPrefix(path, "ark ") { | ||
| return method, cmd.RunE != nil |
There was a problem hiding this comment.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3d76173fb9
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| RequestType: "ListVTXOsRequest", | ||
| ResponseType: "ListVTXOsResponse", | ||
| JSONInput: true, | ||
| MCPTool: true, |
There was a problem hiding this comment.
Align MCP parameters with the advertised schema
Marking ark.vtxos.list as an MCP tool makes the registry advertise its CLI parameters as the tool contract, but the live MCP handler accepts status_filter and min_amount_sat (and does not accept status, min_amount, fields, or ndjson). An agent using wavecli schema to invoke this newly advertised MCP method will therefore send unsupported arguments or silently get unfiltered results; the new test compares only tool names, so it will not catch this drift. Give MCP methods their own parameter schema or validate the live MCP input schema as well.
AGENTS.md reference: cmd/wavecli/waveclicommands/AGENTS.md:L105-L113
Useful? React with 👍 / 👎.
Addresses P0-7 from #997 with the intentionally small reconciliation-and-ratchet approach.
Before:
schema --alladvertised wrong names/types, phantom fields, missing flags/commands, and incorrect MCP exposureAfter:
daemon.balance/ark.oor.newaddress, send wait flags, exit/create/dry-run/in-round parameters, and the missing rounds join entryoutput_schema_id,output_schema_version: 1, andside_effectConcrete consequence: agents can trust the runtime manifest for command construction and safety classification, while future drift fails the ordinary unit suite.
Merge order: this PR must merge last after all selected surface changes (#1021 through #1027), then be rebased once so the ratchet records the final combined surface. In particular, #1022 adds watch bounds and #1026 adds non-interactive input flags that the final manifest must include. It is deliberately a draft until that final rebase.
Validation on current main:
make unit pkg=./cmd/wavecli/...go test ./cmd/wavecli/... -racemake lint-changed-localmake commitmsg-lint range="origin/main..HEAD"