Skip to content

feat(schema): --schema on every command, plus response shapes - #79

Merged
dspangen merged 5 commits into
mainfrom
feat/schema-everywhere
Aug 25, 2026
Merged

feat(schema): --schema on every command, plus response shapes#79
dspangen merged 5 commits into
mainfrom
feat/schema-everywhere

Conversation

@dspangen

@dspangen dspangen commented Aug 24, 2026

Copy link
Copy Markdown
Contributor

--schema now works on every API command — GETs, DELETEs, and the hand-written ones included — and the emitted document gained a response section, so response shapes are no longer invisible. Previously --schema on a bodyless command was an unknown flag error (13 audited incidents), and nothing could tell you that e.g. models validate returns a bare array.

  • Bodyless ops describe their positional args and queryParams, with an explicit "body": null.
  • response shows the lowest declared 2xx schema (wildcard 2XX accepted), rendered with the same --depth handling as bodies.
  • Body-op output is backward compatible — the new sections are additive.
  • Still zero-friction: no args, no token, no network call.
Details: flag-collision policy, response capture rules, trade-offs, and verification

What changed

  • --schema (and --field/--depth) on every generated command. Registration and the Args/RunE short-circuit moved into a shared openapi.RegisterSchemaFlag helper.
  • Bodyless operations describe themselves. The document carries args (name, placeholder, type, description) and queryParams (flag, name, type, enum, required, description), and an explicit "body": null so "takes no body" is unambiguous rather than merely absent.
  • response section for all operations. Captured in operationInfo and rendered through the existing describer.simplify, honoring --depth. A contentless status (e.g. 204) reports its status with a null schema.
  • --field still drills the request body only — documented in the flag help, and it returns a clear error instead of silently doing nothing when the operation has no body schema.
  • Hand-written commands too: models create-branch and users set-attributes emit the same document shape from a static description of the body the CLI assembles, so agent-help's "every API command" claim holds.
  • omni agent-help documents the four sections and that --schema works everywhere.

Flag-collision policy

A spec parameter always keeps its own flag name — it is the operation's contract, and silently reinterpreting an endpoint's --depth query param as a discovery control would corrupt requests. When a parameter owns schema, field or depth, that discovery flag is registered under a deterministic fallback instead: --schema-doc, --schema-field, --schema-depth (and -2, -3… if those are taken too). The rename is announced in the flag's --help text ("named --schema-doc here because --schema is a parameter of this endpoint") and echoed in a schemaFlags section of the emitted document. Discovery is therefore never unavailable, only occasionally renamed. The current spec has no such collisions; the behavior is covered by a synthetic spec whose GET declares query params named schema, field and depth.

Response capture

The success response is the lowest declared 2xx status, falling back to an OpenAPI range key (2XX) when only that is declared — a specific code always beats the wildcard. Media-type selection prefers application/json with a schema, then the first type that carries a schema; a media type declared without a schema is still reported by name with a null schema, since "comes back as text/csv, shape undocumented" beats implying an empty response. requestBodySchema shares the same picker so request and response selection cannot drift.

Trade-off worth knowing

Response schemas make some documents large — documents v2-* now dumps ~165 KB at the default depth of 8, the same order as its request body already did. --depth N and --field PATH remain the way to narrow that, and agent-help points at them.

Verification

  • make build && make test — all packages pass.
  • New unit tests (including the review follow-ups: flag-name collisions, 2XX-only and 2XX-vs-specific responses, media types without a schema, and freeFlagName exhausting its fallbacks): --schema on a GET with path/query params (including required + enum), bare-array 200 response, contentless 204, --depth truncating the response, --field on a bodyless op erroring, body-op backward compatibility, --schema registered on every command generated from the real spec, --schema skipping arg validation, plus create-branch --schema / set-attributes --schema and their --field rejection.
  • Swept the built binary: --schema --compact succeeds on all 210 API subcommands (the only non-accepting commands are completion and config, which make no API call); every operation reports a response section.
  • Sanity: ./omni models list --schema, ./omni query run --schema, ./omni models validate --schema (previously an unknown-flag error).

🤖 Generated with Claude Code

https://claude.ai/code/session_014TwwKSAsAGPBToNb4iUe5s

dspangen added a commit that referenced this pull request Aug 24, 2026
…nse capture

Review of #79 found two ways a valid spec could silently lose information.

1. Parameter collisions disabled discovery. RegisterSchemaFlag bailed out when a
query param already owned the "schema" flag name, so --schema quietly vanished
from that command — exactly the wasted-call trap this branch set out to fix —
and params named "field" or "depth" would have been reinterpreted as discovery
controls, corrupting requests. The policy is now explicit: a spec parameter
always keeps its own name, and the discovery flag moves to a deterministic
fallback (--schema-doc / --schema-field / --schema-depth, then -2, -3… if those
are taken too). The fallback names are reported in the flag help and in a
schemaFlags section of the document, and the resolved names are threaded through
the emit callback so nothing reads a hardcoded name.

2. Success responses could be dropped. successResponse ignored OpenAPI range
keys, so an operation declaring only "2XX" looked like it had no response at
all; and a media type declared without a schema was skipped entirely, making the
response look contentless. Range keys are now accepted (a specific 2xx code
still wins), and a schema-less media type is reported by name with a null
schema. requestBodySchema shares the same media-type picker so the two cannot
drift.

Constraint: a spec parameter must never be shadowed or reinterpreted by a CLI-only flag
Constraint: --schema discovery must never be silently unavailable
Rejected: skipping registration on collision | that is the bug, just moved
Rejected: forcing the discovery flag to win and renaming the spec param | breaks the operation's documented contract and any existing script
Confidence: high
Scope-risk: narrow
Directive: read the discovery flags through the SchemaFlags names passed to the emit callback; never hardcode "schema"/"field"/"depth"
Not-tested: a spec declaring both "2XX" and "200" with different media types (covered separately for status and for media-type selection)

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_014TwwKSAsAGPBToNb4iUe5s
dspangen and others added 3 commits August 24, 2026 12:23
--schema was only registered for operations with a request body, so running
it on a GET/DELETE (or the hand-written models create-branch) failed with
"unknown flag: --schema" — a wasted call every time, since agents are told to
reach for --schema first. Separately, response shapes were invisible across all
~180 subcommands, so a caller had to make the call to learn (for example) that a
payload is a bare top-level array.

Every generated command now registers --schema/--field/--depth via the shared
RegisterSchemaFlag helper, which keeps the existing short-circuit of Args/RunE
so the flag needs no positional args, no token and no network. The emitted
document gains args, queryParams and a response section (lowest declared 2xx,
application/json preferred, rendered through the existing describer and capped
by --depth); bodyless operations report "body": null explicitly. Body operations
keep their existing method/path/required/body/example fields unchanged.

The hand-written models create-branch and users set-attributes commands emit the
same document from a static description of the body they assemble, so the
"works on every API command" promise in agent-help holds.

Constraint: --schema must not require args, auth or a network call
Constraint: existing --schema consumers must keep seeing method/path/required/body/example
Rejected: registering --schema only on generated commands | leaves create-branch and set-attributes as the same wasted-call trap
Rejected: drilling --field into the response too | one flag with two roots is ambiguous; response is capped by --depth instead
Confidence: high
Scope-risk: moderate
Directive: --field drills the request body only; if you extend it, do not overload the same flag for the response
Not-tested: operations whose 2xx declares a body with no schema (none exist in the current spec; handled with an explicit note)

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_014TwwKSAsAGPBToNb4iUe5s
…nse capture

Review of #79 found two ways a valid spec could silently lose information.

1. Parameter collisions disabled discovery. RegisterSchemaFlag bailed out when a
query param already owned the "schema" flag name, so --schema quietly vanished
from that command — exactly the wasted-call trap this branch set out to fix —
and params named "field" or "depth" would have been reinterpreted as discovery
controls, corrupting requests. The policy is now explicit: a spec parameter
always keeps its own name, and the discovery flag moves to a deterministic
fallback (--schema-doc / --schema-field / --schema-depth, then -2, -3… if those
are taken too). The fallback names are reported in the flag help and in a
schemaFlags section of the document, and the resolved names are threaded through
the emit callback so nothing reads a hardcoded name.

2. Success responses could be dropped. successResponse ignored OpenAPI range
keys, so an operation declaring only "2XX" looked like it had no response at
all; and a media type declared without a schema was skipped entirely, making the
response look contentless. Range keys are now accepted (a specific 2xx code
still wins), and a schema-less media type is reported by name with a null
schema. requestBodySchema shares the same media-type picker so the two cannot
drift.

Constraint: a spec parameter must never be shadowed or reinterpreted by a CLI-only flag
Constraint: --schema discovery must never be silently unavailable
Rejected: skipping registration on collision | that is the bug, just moved
Rejected: forcing the discovery flag to win and renaming the spec param | breaks the operation's documented contract and any existing script
Confidence: high
Scope-risk: narrow
Directive: read the discovery flags through the SchemaFlags names passed to the emit callback; never hardcode "schema"/"field"/"depth"
Not-tested: a spec declaring both "2XX" and "200" with different media types (covered separately for status and for media-type selection)

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_014TwwKSAsAGPBToNb4iUe5s
…after rebase

Main's 99820ce added shorthandFieldType calling resolveField with the old
two-argument signature; this branch parameterized the flag name for renamed
discovery flags. Pass "field" to keep the pre-existing error wording for
shorthand registration.

Confidence: high
Scope-risk: narrow

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_014TwwKSAsAGPBToNb4iUe5s
@dspangen
dspangen force-pushed the feat/schema-everywhere branch from 48e9217 to e65e86d Compare August 24, 2026 16:24
@dspangen
dspangen requested a review from n8agrin August 24, 2026 16:48
@n8agrin
n8agrin requested a balanced review from Copilot August 24, 2026 22:18

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Extends schema discovery across all API commands and adds response-shape metadata.

Changes:

  • Registers collision-safe schema discovery flags on every generated command.
  • Adds arguments, query parameters, request bodies, and success responses to schema output.
  • Supports hand-written API commands and updates agent guidance and tests.

Reviewed changes

Copilot reviewed 11 out of 11 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
internal/openapi/schema.go Expands schema documents and shared flag handling.
internal/openapi/schema_test.go Tests responses, bodyless operations, and collisions.
internal/openapi/generate.go Captures responses and registers schema flags globally.
internal/openapi/generate_test.go Tests universal registration and validation bypass.
internal/openapi/body_shorthand.go Adapts field resolution API.
internal/openapi/body_shorthand_test.go Updates shorthand field checks.
cmd/omni/user_commands.go Adds static schema output for set-attributes.
cmd/omni/user_commands_test.go Tests set-attributes schema output.
cmd/omni/branch_commands.go Adds static schema output for create-branch.
cmd/omni/branch_commands_test.go Tests create-branch schema behavior.
cmd/omni/agent_help.go Documents expanded schema discovery.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread cmd/omni/user_commands.go Outdated
Comment on lines +103 to +107
openapi.RegisterSchemaFlag(cmd, func(c *cobra.Command, names openapi.SchemaFlags) error {
if field, _ := c.Flags().GetString(names.Field); field != "" {
return fmt.Errorf("--%s is not supported for set-attributes; its request body is assembled by the CLI and shown in full", names.Field)
}
return openapi.EmitSchemaDoc(c, setAttributesSchemaDoc())
Comment thread cmd/omni/branch_commands.go Outdated
Comment on lines +99 to +103
openapi.RegisterSchemaFlag(cmd, func(c *cobra.Command, names openapi.SchemaFlags) error {
if field, _ := c.Flags().GetString(names.Field); field != "" {
return fmt.Errorf("--%s is not supported for create-branch; its request body is assembled by the CLI and shown in full", names.Field)
}
return openapi.EmitSchemaDoc(c, createBranchSchemaDoc())

@n8agrin n8agrin left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some initial Claude findings:

Review complete for [PR #79](https://github.com/exploreomni/cli/pull/79) — 15 findings reported, ranked most severe first.

Top issues (confirmed correctness bugs):

--depth is a silent no-op on both hand-written commands — branch_commands.go:99 and user_commands.go:103. RegisterSchemaFlag registers --depth with help text promising it truncates nesting, but create-branch --schema and set-attributes --schema ignore it entirely (verified by building the binary and diffing --depth 0 vs --depth 8 output — byte-identical).
--schema output can be corrupted by cobra's deprecation notice — schema.go:132. On any deprecated command (3 exist today: documents put/update/transfer-ownership), --schema --compact 2>&1 prints a plain-text warning before the JSON, breaking the "clean JSON, no side content" contract for any caller that merges stdout/stderr.

Maintainability (confirmed): both hand-written commands' --schema docs are fully hand-transcribed copies of the real body-assembly code and the spec's response schema, with no shared source of truth and no test that would catch drift (branch_commands.go:109, user_commands.go:129). Also duplicated: a --field-rejection closure and str() helper across both files, and two near-identical test helpers in schema_test.go.

Efficiency: describeBody computes the (potentially ~165KB) response schema before validating --field, wasting that work whenever --field is invalid (schema.go:254).

Conventions: two new doc comments in schema.go (lines 48 and 92) exceed your global CLAUDE.md's "1–2 sentences, 3 max" rule for code comments.

dspangen and others added 2 commits August 24, 2026 21:49
…ON clean

Review follow-ups on the schema-everywhere work:

- --depth was registered but ignored by the two hand-written commands
  (models create-branch, users set-attributes): their static documents were
  emitted verbatim. StaticSchemaEmitter now runs them through limitDocDepth,
  which truncates with the same placeholder the spec describer uses.
- Cobra prints a command's Deprecated notice at the top of execute(), before
  flags are parsed, so plain text preceded --schema's JSON on the three
  deprecated document commands. RegisterSchemaFlag now takes ownership of the
  notice: it clears cmd.Deprecated (silencing cobra), sets cmd.Hidden to keep
  the command out of help listings exactly as Deprecated did, and re-prints the
  same notice to stderr on real (non---schema) runs only.
- The hand-transcribed static documents had no drift guard. Their response
  sections are now asserted equal to what the describer produces for the
  operation each command wraps, and their body fields against the assembling
  code (create-branch's body assembly is extracted into buildCreateBranchBody
  so the test can call it).
- Deduped the --field rejection closure and the str() schema helper across the
  two commands, and collapsed four near-identical schema test helpers onto one
  execSchema core.
- describeBody validates --field before building the response schema, which can
  run to hundreds of kilobytes.

Constraint: a spec parameter always keeps its own flag name, so discovery flags
are read through the resolved SchemaFlags names, never hardcoded
Rejected: suppressing the deprecation notice inside RunE | cobra emits it in
execute() before flags are parsed, so RunE is too late
Rejected: dropping cmd.Deprecated without setting Hidden | deprecated commands
would reappear in help listings
Confidence: high
Scope-risk: narrow
Directive: limitSchemaDepth must keep matching describer.simplify's nesting
rules (properties/items/additionalProperties/oneOf/anyOf) and depthNote — the
static documents' --depth output is compared against the generated one
Not-tested: a deprecated command whose args fail validation no longer prints the
notice (cobra errors before RunE); --help on a deprecated command likewise

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_014TwwKSAsAGPBToNb4iUe5s
@dspangen
dspangen merged commit a369e1e into main Aug 25, 2026
2 checks passed
@dspangen
dspangen deleted the feat/schema-everywhere branch August 25, 2026 15:48
dspangen added a commit that referenced this pull request Aug 25, 2026
Main landed flag normalization (#77) and --schema everywhere (#79), both of
which overlap this branch's generate.go work.

Resolutions:
- Query/path flag naming: main's canonicalName + SetNormalizeFunc +
  resolveQueryFlags replaces this branch's cliFlagName/queryFlagValue and the
  hidden deprecated legacy aliases. Main's approach covers the same spellings
  (--modelid, --modelId, --model_id) via pflag normalization, so the alias
  flags and their conflict check are no longer reachable behavior.
- operationInfo: kept both sides' fields (BodyMediaType/BodyFields from here,
  Response from main).
- requestBodySchema: dropped in favor of this branch's requestBodyMediaType,
  which also returns the MediaType so multipart encodings stay reachable.
- agent-help / README: kept both sides' text (--body @file and multipart from
  here, flag-spelling and --schema-everywhere notes from main).
- Removed TestBuildCommand_CamelCaseQueryFlags: it asserts the legacy-alias
  behavior main replaced. Main's TestBuildCommand_AlternateFlagSpellings and
  TestBuildCommand_CamelCaseQueryParamBecomesKebabFlag cover the successor.

Rejected: keeping legacy alias flags alongside main's normalization | duplicate
mechanisms for one behavior, and the aliases would shadow the normalize func
Confidence: high
Scope-risk: moderate
Directive: multipart FlagName now uses canonicalName; keep it in step with
resolveQueryFlags so upload flags and query flags stay spelled alike
Not-tested: multipart upload against a live API after the flag-name change
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.

3 participants