Skip to content

fix(cli)!: refuse --as wherever a command records no actor - #1059

Closed
ragnorc wants to merge 3 commits into
ModernRelay:mainfrom
ragnorc:claude/cli-actor-classifier
Closed

ragnorc wants to merge 3 commits into
ModernRelay:mainfrom
ragnorc:claude/cli-actor-classifier

Conversation

@ragnorc

@ragnorc ragnorc commented Oct 17, 2026 •

Copy link
Copy Markdown
Contributor

Superseded by merged PR #1090 (2c9eb1e051ef9366ff996c5b963d4773bb208b44). Its exhaustive actor classification, precise branch labels, operator-config parity actor and recorded-actor assertions are incorporated into the new target CLI and verified by the full CI and merge-queue gates.


Follow-up to #1029, from the review on its planes.rs thread.

Problem

The plane guard's --as row for data commands was a list of reads to refuse. Any data verb missing from the list accepted --as and then ignored it. snapshot, export, schema show and branch list all did this: they dispatch through the read resolver, which records no actor. The guard exists to refuse --as where it is a silent no-op, and a new read subcommand would have joined the gap by default.

The parity harness (run_both in tests/support/mod.rs) kept its own string copy of that list to decide where to pass --as, and the two copies could drift apart.

Change

  • One classifier. consumes_actor(cmd) in crates/omnigraph-cli/src/planes.rs answers for every command. --as applies to engine _as writes and to actor-bound cluster operations: cluster plan checks the intended actor, and apply and upgrade-ledger attribute it. Everything else refuses it. The guard's --as row is now ScopeFlag::As => consumes_actor(cmd).
  • Exhaustive where it matters. Every family that has both answers (branch, blob, commit, schema, cluster) is matched down to the subcommand, so a new command or subcommand does not compile until it is classified. A family that only reads answers as a whole, so a subcommand added to it refuses --as until someone classifies it.
  • Nothing else moves. Only the four verbs above change answer. Direct maintenance, control and served commands give the same answer as before.
  • Error labels. command_label now names the branch subcommand, e.g. `branch list` is a data command; --as … does not apply.
  • Harness. The parity harness's direct arm now takes act-parity from operator.actor in its own operator home, instead of passing --as. Every write resolves operator.actor and every read ignores it, so the harness no longer keeps a read list, and an explicit --as on a read stays the guard's error. The parity rows still assert the direct arm's actor_id on writes (blob put/clear, branch create/merge/delete), which shows the actor still arrives.

Compatibility

This is breaking for scripts that pass --as to snapshot, export, schema show or branch list: those commands now refuse it, and the fix is to drop the flag. See changelog.d/as-refused-on-every-read.breaking.md. The CLI reference's --as row now says every read command refuses it.

Tests

  • planes::tests::scope_flag_matrix_matches_capabilities_issue_1023 adds rows for every data verb's --as answer: mutate, load, blob clear, the branch writes and reads, snapshot, export, schema show and diff.
  • New planes::tests::as_refusal_names_the_subcommand pins the exact refusal text for branch list, branch diff, commit show, snapshot, export and schema show.
  • Before the classifier, both failed as predicted: the matrix on branch list accepting --as, and the label test because branch list --as passed the guard.

Run locally on this head (RUST_MIN_STACK=16777216, cargo test -p omnigraph-cli -p omnigraph-server):

  • in-source CLI tests: 166 passed;
  • all 37 parity_matrix rows, the cli_schema_config suite and the cli_data --as owners: 66 passed;
  • cargo fmt --all --check, scripts/check-docs.py, typos and scripts/check-agents-md.sh: clean.

Before the rebase onto main, the full CLI integration binary (323 passed) and cargo clippy -p omnigraph-cli --all-targets --locked -- -D warnings -W clippy::dbg_macro were also clean on the identical change. I did not rerun clippy after the rebase because the machine was low on disk; CI's Lint job covers it.

No Lance domain is involved: the change is CLI argument classification only.

The plane guard's `--as` row listed the data reads to refuse, so any data
verb missing from the list accepted `--as` and dropped it: `snapshot`,
`export`, `schema show` and `branch list` did. One exhaustive classifier,
`consumes_actor`, now answers for every command: engine `_as` writes and
actor-bound cluster operations take `--as`, everything else refuses it. A
family that holds both answers is matched to the subcommand, so a new
command or subcommand does not compile until it is classified. Guard
errors name the `branch` subcommand.

The parity harness kept its own copy of the read list to decide where to
pass `--as`. Its direct arm now takes the parity actor from
`operator.actor`, which every write resolves and every read ignores, so
the copy is gone.
@ragnorc
ragnorc requested a review from aaltshuler as a code owner October 17, 2026 23:23
…sifier

# Conflicts:
#	crates/omnigraph-cli/src/planes.rs
#	crates/omnigraph-cli/tests/integration/parity_matrix.rs
#	crates/omnigraph-cli/tests/support/mod.rs

ragnorc commented Oct 18, 2026

Copy link
Copy Markdown
Contributor Author

Recommendation: approve.

Why

--as names the actor a command records. Four data commands accepted the flag and then dropped it: branch list, snapshot, export, and schema show. A caller could believe the actor was set. The plane guard now refuses --as on every command that records no actor.

What

consumes_actor in crates/omnigraph-cli/src/planes.rs is the one classifier. ScopeFlag::As calls it. Capability no longer selects this flag. The function returns true for an engine _as write and for an actor-bound cluster operation. It returns false for every other command. The parity harness deletes its copy of the read list. The direct arm sets OMNIGRAPH_HOME. operator.actor supplies the actor on a write. A read ignores that value. An explicit --as on a read stays an error from the guard. The CLI reference row for --as matches this rule.

How

The guard runs before dispatch and before a graph open. A false result from consumes_actor produces this error when --as is set:

`{label}` is a data command; --as sets the actor for a direct-engine or actor-bound cluster operation and does not apply.

Mixed families match each subcommand, so a new variant fails to compile. Read-only families use { .. } and refuse --as on a new subcommand until the match gains an arm. That refusal is loud.

Impact

This is a breaking CLI change. branch list, snapshot, export, and schema show exit 1 when --as is present. Drop --as from those calls. The same four commands still succeed when the actor comes only from operator.actor. A direct write still records --as. --as still overrides operator.actor. A served write still refuses --as in the resolver. The server takes the actor from the bearer token. branch merge --dry-run still accepts --as. That path copies the resolved actor into the JSON actor_id field.

Every branch guard error now names the subcommand. branch diff --as reports `branch diff`.

Blocking findings

None.

Optional findings

  1. Inline on crates/omnigraph-cli/tests/support/mod.rs line 1436: The rustdoc on run_both still says the direct arm uses --as and that only read-only Blob commands reject the flag. This function sets OMNIGRAPH_HOME and passes no --as. Update the first paragraph of that rustdoc. A later edit can otherwise restore the deleted read list.
  2. The breaking note names the four commands. It omits the new branch error text. Add one clause if a script matches the old `branch` is a data command string.

Tradeoffs and liability

The old rule was a denylist of data-plane reads under capability Any. A data command absent from that list accepted --as. That breaks the fail-closed rule for a scope flag. It also breaks invariant 10. A client must not believe it set an actor the command does not record. The class is every command that does not pass an actor into an engine _as entry or an actor-bound cluster operation. The plane guard owns that contract.

The classifier matches dispatch on this head. These commands pass cli.as_actor into resolve_with_policy or resolve_cli_actor: mutate, load, rebuild-full-text-indexes, branch create, branch delete, branch merge, blob put, blob clear, commit publish, commit drop, and schema apply. Cluster plan, apply, and upgrade-ledger use resolve_cluster_actor on an unmanaged path. These commands do not pass the flag into an actor-recording call: query, snapshot, export, diff, changes, schema show, schema plan, schema upgrade-system-columns, branch list, branch diff, branch merge-base, commit list, commit show, commit changes, blob get, blob stat, graphs, queries, policy, organization, init, upgrade, optimize, repair, cleanup, lint, alias, embed, login, logout, use, profile, and skill.

The fix removes the silent no-op and the second list in the harness. It adds one handwritten mirror of dispatch. A new command fails to compile until it has an arm. A command that stays true after it stops recording an actor would accept --as again. Five similar commands add arms to this one function. They do not add a new list.

schema upgrade-system-columns still calls upgrade_system_columns_as with None. The guard already refused --as on that command. This change leaves that path in place.

This diff does not call Lance, open a dataset, or change storage. Pinned Lance 11.0.0 stays unchanged. No storage-cost row changes. A later Lance version does not affect this classifier. Open CLI pull requests do not change this flag contract. Pull request 1012 also edits branch-list CLI files, so a rebase will meet that diff.

GitHub reports this head does not merge into current main. The conflict is mechanical: command variants, the run_both signature, and the reference page. When you resolve planes.rs, keep consumes_actor. Current main still records the actor on the same commands.

Validation

Parent a152dd4672dc1f45970b578a547ea0751f28be05. A temporary unit test called guard_addressing for branch list, snapshot, export, and schema show with --as act-reader. Command, from that tree:

cargo test -p omnigraph-cli --bin omnigraph -- planes::tests::reads_that_record_no_actor_refuse_as_issue_1059

Result: panic. All four accepted --as: omnigraph branch list --as act-reader, omnigraph snapshot --as act-reader, omnigraph export --as act-reader, omnigraph schema show --as act-reader. The temporary test was removed. It was not pushed.

Head cda2369e6b699a229fbecb71b8c76782ff768bec. GQT cannot express a global CLI flag. The CLI binary tests own this contract. From the isolated checkout, with the shared CARGO_TARGET_DIR and RUST_MIN_STACK=16777216:

  • cargo test -p omnigraph-cli --bin omnigraph --locked -- planes:: — 5 passed, 170 filtered out. That includes scope_flag_matrix_matches_capabilities_issue_1023 and as_refusal_names_the_subcommand.
  • cargo clippy -p omnigraph-cli --all-targets --locked -- -D warnings -W clippy::dbg_macro — exit 0.
  • cargo fmt --all --check — exit 0.
  • python3 scripts/check-docs.py — exit 0, 318 Markdown files.

Process check of the omnigraph binary built from this head. The four commands with --as exit 1. The message matches the unit test for `branch list`, `snapshot`, `export`, and `schema show`. With OMNIGRAPH_HOME set so operator.actor is act-from-config, mutate records that actor. --as act-flag records act-flag. branch list and snapshot --json with that home and no --as exit 0. Snapshot JSON has no actor field.

Limitations

The parity integration suite did not run. The worktree had no omnigraph-server binary. A server binary in the shared target directory belongs to another commit. Six attempted cases stopped during server startup. That limit is environmental.

CI on this commit ran PR Title and Fix Regression Gate. Both succeeded. This SHA has no lint job and no test job. The Devin Review status says the full review was skipped.

The full CLI integration binary was not rerun here. Clippy on all targets of omnigraph-cli was clean on this head.

The merge with main shortened tuple-variant patterns, so one classifier arm now fits on a line. The breaking note also names the guard's new `branch` subcommand label, which a script matching the old error text would see.
@ragnorc
ragnorc marked this pull request as draft October 20, 2026 03:08
@ragnorc

ragnorc commented Oct 20, 2026

Copy link
Copy Markdown
Contributor Author

Held until #1090 lands. #1090's accepts_actor already refuses --as on snapshot, export, schema show and branch list and replaces the ScopeFlag guard this PR edits. After it merges, this PR will be reduced to what #1090 does not carry: the exhaustive per-subcommand classifier (a new command or subcommand does not compile until classified), the parity harness taking its actor from operator.actor instead of keeping a read list, and the branch subcommand labels in guard errors.

@aaltshuler aaltshuler closed this Oct 20, 2026
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.

2 participants