refactor(cli): demote check from visible_alias to deprecation shim

`omnigraph check` was a clap `visible_alias` on `lint`, advertised in
`--help` as an equivalent canonical name. Per MR-981 §6 (long-form
flags as canonical, short forms as visible aliases), visible aliases
on subcommand names hurt agent CX: agents emit either spelling
depending on training-data drift, and there's no length signal
pointing at the canonical name.

Changes:

* Remove `#[command(visible_alias = "check")]` from the `Lint` variant.
  `omnigraph --help` now shows only `lint`.
* Add bare `check` to `rewrite_deprecated_argv` so `omnigraph check
  <args>` still works — it rewrites to `omnigraph lint <args>` and
  emits a one-line stderr deprecation warning, matching the existing
  pattern for `read` / `change` / `query lint` / `query check`.
* Fix the nested `query check` shim to substitute `check` -> `lint` in
  the rewritten argv (previously it relied on `check` being a
  visible_alias to reach the `Lint` variant).
* New test `deprecated_check_top_level_rewrites_to_lint` covers: bare
  `check` produces identical stdout to `lint`, emits the deprecation
  warning, and `check` does NOT appear as an alias in `omnigraph
  --help`.
* Release notes updated to reflect the deprecation-shim treatment and
  cross-reference MR-981 §6 reasoning.

Cargo / Go users typing `check` still work indefinitely; one stderr
nudge per invocation teaches the canonical name. Agents see only
`lint` in `--help --json` so they emit one canonical form.

67/0 omnigraph-cli tests pass; 39 workspace test suites green.
This commit is contained in:
Ragnor Comerford 2026-05-29 13:17:20 +02:00
parent ba1c5d7530
commit 7f28e1a5ed
No known key found for this signature in database
3 changed files with 112 additions and 12 deletions

View file

@ -130,10 +130,15 @@ enum Command {
},
/// Validate queries against a schema (offline) or repo (repo-backed).
///
/// Replaces `omnigraph query lint` / `omnigraph query check`, which
/// are kept as deprecated argv-level shims (a one-line warning is
/// printed and the invocation is rewritten to `omnigraph lint`).
#[command(visible_alias = "check")]
/// Canonical name is `lint` (matches the `omnigraph_compiler::lint`
/// module and the `OG-XXX-NNN` lint-code vocabulary). Replaces the
/// deprecated `omnigraph query lint` / `omnigraph query check` /
/// `omnigraph check` invocations — each is kept as an argv-level
/// shim that prints a one-line stderr warning and rewrites to
/// `omnigraph lint`. Aliases are deliberately *not* exposed via
/// clap's `visible_alias` because that would advertise two
/// equivalent canonical names, which agents emit interchangeably
/// (see MR-981).
Lint {
/// Graph URI
uri: Option<String>,
@ -1776,11 +1781,19 @@ async fn execute_export_remote_to_writer<W: Write>(
/// Rewrite deprecated CLI invocations into their canonical form.
///
/// The current rename pass moves three subcommands:
/// - `omnigraph read` -> `omnigraph query` (visible_alias handles parsing; we just warn)
/// - `omnigraph change` -> `omnigraph mutate` (visible_alias handles parsing; we just warn)
/// The current rename pass moves four subcommands:
/// - `omnigraph read` -> `omnigraph query` (clap `visible_alias` handles parsing; we warn)
/// - `omnigraph change` -> `omnigraph mutate` (clap `visible_alias` handles parsing; we warn)
/// - `omnigraph check` -> `omnigraph lint` (rewrite required; no visible_alias by design)
/// - `omnigraph query lint` -> `omnigraph lint` (rewrite required; `query` is now the read-runner)
/// - `omnigraph query check` -> `omnigraph lint` (`check` is still a visible alias on `lint`)
/// - `omnigraph query check` -> `omnigraph lint` (rewrite required)
///
/// `check` is *not* a clap visible_alias on `lint` even though they're
/// semantically equivalent. Visible aliases create two canonical names
/// that agents emit interchangeably depending on training-data drift
/// (see MR-981 §6 for the policy). The argv-shim + stderr warning
/// pattern preserves back-compat for human users while pointing every
/// caller at the single canonical name in `--help`.
///
/// Returns the (possibly rewritten) argv that clap should parse.
fn rewrite_deprecated_argv(args: Vec<OsString>) -> Vec<OsString> {
@ -1790,12 +1803,17 @@ fn rewrite_deprecated_argv(args: Vec<OsString>) -> Vec<OsString> {
if sub == Some("query") && matches!(sub2, Some("lint") | Some("check")) {
let suffix = sub2.unwrap();
eprintln!(
"warning: `omnigraph query {suffix}` is deprecated; use `omnigraph lint` (alias: `omnigraph check`) instead"
"warning: `omnigraph query {suffix}` is deprecated; use `omnigraph lint` instead"
);
// Drop the leading `query` token, leaving e.g. `lint --query ./foo.gq`.
// Drop the leading `query` token AND normalize `check` -> `lint`.
// `check` is no longer a clap visible_alias (MR-981 §6), so the
// rewritten argv must reach the canonical `lint` subcommand
// directly. Result for `omnigraph query check --query foo.gq`:
// `omnigraph lint --query foo.gq`.
let mut out = Vec::with_capacity(args.len() - 1);
out.push(args[0].clone());
out.extend(args[2..].iter().cloned());
out.push(OsString::from("lint"));
out.extend(args[3..].iter().cloned());
return out;
}
}
@ -1807,6 +1825,17 @@ fn rewrite_deprecated_argv(args: Vec<OsString>) -> Vec<OsString> {
"change" => eprintln!(
"warning: `omnigraph change` is deprecated; use `omnigraph mutate` instead"
),
"check" => {
eprintln!(
"warning: `omnigraph check` is deprecated; use `omnigraph lint` instead"
);
// Rewrite the top-level subcommand to `lint`; pass through the rest.
let mut out = Vec::with_capacity(args.len());
out.push(args[0].clone());
out.push(OsString::from("lint"));
out.extend(args[2..].iter().cloned());
return out;
}
_ => {}
}
}

View file

@ -714,6 +714,77 @@ query list_people() {
);
}
/// Bare `omnigraph check` is NOT a clap `visible_alias` on `lint` (MR-981 §6:
/// visible aliases give agents two canonical names to emit interchangeably).
/// It's an argv-level shim: rewrites to `omnigraph lint`, prints a one-line
/// stderr deprecation warning, and produces identical stdout to the canonical
/// invocation. Cargo/Go users typing `check` keep working; help text shows
/// only `lint`.
#[test]
fn deprecated_check_top_level_rewrites_to_lint() {
let temp = tempdir().unwrap();
let schema_path = temp.path().join("schema.pg");
let query_path = temp.path().join("queries.gq");
write_file(
&schema_path,
r#"
node Person {
name: String
}
"#,
);
write_query_file(
&query_path,
r#"
query list_people() {
match { $p: Person }
return { $p.name }
}
"#,
);
let canonical = output_success(
cli()
.arg("lint")
.arg("--query")
.arg(&query_path)
.arg("--schema")
.arg(&schema_path)
.arg("--json"),
);
let deprecated_check = output_success(
cli()
.arg("check")
.arg("--query")
.arg(&query_path)
.arg("--schema")
.arg(&schema_path)
.arg("--json"),
);
assert_eq!(stdout_string(&canonical), stdout_string(&deprecated_check));
let check_stderr = String::from_utf8(deprecated_check.stderr).unwrap();
assert!(
check_stderr.contains("`omnigraph check` is deprecated")
&& check_stderr.contains("`omnigraph lint`"),
"expected `omnigraph check` deprecation warning pointing at `omnigraph lint`; got: {check_stderr}"
);
// `check` must NOT appear in the canonical `omnigraph --help` output —
// agents copy the surface from help text and would otherwise emit both
// names interchangeably.
let help = cli().arg("--help").output().unwrap();
let stdout = String::from_utf8(help.stdout).unwrap();
let check_aliased = stdout
.lines()
.any(|line| line.trim_start().starts_with("lint") && line.contains("check"));
assert!(
!check_aliased,
"`check` must not be advertised as a visible alias of `lint`; help output: {stdout}"
);
}
/// `omnigraph read` and `omnigraph change` are kept as visible clap
/// aliases for the new canonical `query` / `mutate` subcommands, plus an
/// argv-level deprecation warning. The warning is emitted to stderr; the

View file

@ -35,7 +35,7 @@ Runtime add/remove (`POST /graphs`, `DELETE /graphs/{id}`, `omnigraph graphs cre
### Query / mutation rename
- **`ChangeRequest` field rename**: `query_source``query`, `query_name``name`. Both legacy names continue to deserialize via `#[serde(alias = "...")]`, so existing clients sending the old JSON keys keep working. CLI remote calls against `/change` still emit the legacy keys verbatim through the `legacy_change_request_body` helper so a newer CLI talking to an older server keeps working byte-for-byte.
- **CLI `omnigraph query lint` / `omnigraph query check`** are now top-level `omnigraph lint` / `omnigraph check`. The legacy invocations remain as argv-level shims that rewrite to the canonical form and print a one-line deprecation warning to stderr.
- **CLI `omnigraph query lint` / `omnigraph query check`** are now top-level — canonical name is **`omnigraph lint`**. The three deprecated invocations (`omnigraph query lint`, `omnigraph query check`, and bare `omnigraph check`) remain as argv-level shims that rewrite to `omnigraph lint` and print a one-line stderr deprecation warning. `check` is deliberately **not** a clap `visible_alias` on `lint` — two equivalent canonical names would split agent emissions between them depending on training-data drift, so the deprecation pattern (rewrite + warn) gives one unambiguous canonical name in `omnigraph --help`.
## New