[ragnorc]

Fixes #1023

Problem

commit publish and commit drop are writes: the CLI dispatches both through GraphClient::resolve_with_policy with cli.as_actor, and the engine runs them through publish_commit_as / drop_commit_as. The plane guard in crates/omnigraph-cli/src/planes.rs classified every Command::Commit as a read and refused --as, so on a direct graph these two commands could only take operator.actor, and the --as path was unreachable. The GQ spelling (mutate -e 'commit publish "<id>"' --as X) already accepted it.

Change

• The --as row of the guard now matches the commit subcommands exhaustively: publish and drop take --as; list, show and changes still refuse it. A new commit subcommand will not compile until it is classified.
• command_label descends into commit, so guard errors name the subcommand (`commit list` is a data command; --as … does not apply).
• The comment that called all of commit a read is corrected, as is the parity harness's read list in tests/support/mod.rs, which mirrors the guard.
• A served publish or drop still refuses --as in resolve_with_policy_scope before any request; the test verifies this.

What --as does here

A publication records no publisher. The published commit keeps the actor_id and created_at of the write that created it, as the unpublished-commits RFC specifies, and a drop records no actor. On these commands --as names the actor for the engine's commit_publish / commit_drop policy check. A direct CLI open installs no policy, so nothing about the commit changes on a direct graph. The branching guide now states that a published commit keeps its original actor.

Tests

• planes::tests::scope_flag_matrix_matches_capabilities_issue_1023 (renamed owner): rows for commit list, show, publish and drop.
• cli_data::commit_publish_and_drop_take_as_on_a_direct_graph_issue_1023:
  • proposes a commit with --as act-author and publishes it with --as act-reviewer; the published commit is on main with actor_id act-author;
  • drops a second proposal with --as; it no longer resolves and the unpublished listing is empty;
  • commit list/show/changes --as are refused;
  • served commit publish/drop --as are refused before any request.
• Both tests failed before the fix with `commit` is a data command; --as … does not apply.

Run locally:

• RUST_MIN_STACK=16777216 cargo test -p omnigraph-cli -p omnigraph-server --bin omnigraph: 165 passed.
• The new test, the --as/Blob tests in cli_data, and all 37 parity_matrix rows pass. Under heavy machine load some parity rows first hit the 30 s server-readiness deadline; each passed on rerun.
• cargo fmt --all --check, cargo clippy -p omnigraph-cli --all-targets --locked -- -D warnings -W clippy::dbg_macro, scripts/check-docs.py and typos are clean.

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

[cursor]

Review: fix(cli): accept --as on commit publish and drop (head 2d256564)

Recommendation: approve. It can merge when ready. I found 0 blocking and 4 optional findings. This is a comment because the tool I used can only post comments, not an approval.

What the change does

• The --as row in the plane guard now splits commit exhaustively. publish and drop accept --as. list, show and changes still refuse it (crates/omnigraph-cli/src/planes.rs:200-220).
• Guard errors now name the subcommand (`commit list`), not only `commit` (planes.rs:361-367).
• The parity harness read list, the CLI reference, the branching guide and a changelog note are updated to match.

Contract check (correctness)

• Direct path: the guard passes, then resolve_with_policy, then resolve_cli_actor (--as, else operator.actor), then GraphClient::Embedded { actor }, then publish_commit_as / drop_commit_as, then enforce_commit_control (client.rs:1620-1626, client.rs:1655-1660, omnigraph.rs:3874, omnigraph.rs:3947). The --as path in main.rs:957-1015 is now reachable, which closes CLI: commit publish and commit drop reject --as although they write #1023 as written.
• Served path: the guard now passes, so the refusal moves to resolve_with_policy_scope. It refuses --as before any request (client.rs:482-491). The new test proves no request is sent. Invariant 10 (trust at the boundary) holds: a client still cannot supply a trusted actor to a server.
• Reads: commit list/show/changes --as still fail. The test checks all three.
• Matches existing behaviour: branch create and branch delete already accept --as on a direct graph and use the actor only for the _as policy gate (omnigraph.rs:3481, omnigraph.rs:3641). The GQ form mutate -e 'commit publish "<id>"' --as X also accepts it. After this PR, the CLI verb and the GQ statement follow the same rule.

Tradeoff, stated plainly

Today, on a direct graph, --as on publish or drop has no observable effect:

• A direct CLI open installs no policy (client.rs:589, Omnigraph::open), so enforce returns before it reads the actor (omnigraph.rs:1053).
• Publish and drop record no actor. The unpublished-commits RFC says "This release does not add durable approval or publisher provenance."

The PR body says this openly. The guard's own comment says to reject --as where it is a silent no-op, so there is some tension. I accept the PR's position: the deciding question is whether the command is a write that goes through an _as entry point, which is the same rule branch create/delete already use. The other fix (keep refusing --as and delete the dead path in main.rs) would make the verb and the GQ statement disagree. It would also have to be undone when a direct open installs a policy or when publisher provenance is added.

Liability: low. There is no format, protocol or storage change, and no Lance domain is involved. The exhaustive match lowers future drift for commit.

Regression proof (my runs)

Environment: worktree at 2d2565648975acff1ead7406f948139d40f183a1 (base 9c2b3679), Rust 1.97.1, protoc 3.21.12, RUST_MIN_STACK=16777216.

1. Before the fix, the test fails for the predicted reason. I restored only crates/omnigraph-cli/src/planes.rs from 9c2b3679 and kept the PR's test.
   cargo test -p omnigraph-cli --locked --test integration -- cli_data::commit_publish_and_drop_take_as_on_a_direct_graph_issue_1023 --exact
   Result: FAILED at the commit publish --as step, with stderr `commit` is a data command; --as sets the actor for a direct-engine or actor-bound cluster operation and does not apply.
2. Mutation check on the unit row. On the head, I changed Publish | Drop => true to false.
   cargo test -p omnigraph-cli --locked --bin omnigraph planes::tests::scope_flag_matrix_matches_capabilities_issue_1023
   Result: FAILED with As on 'commit publish' (Any), left: false, right: true.
3. On the exact head (restored, git diff --quiet HEAD clean):
   • cargo test -p omnigraph-cli --locked --bin omnigraph planes:: gives 4 passed.
   • cargo test -p omnigraph-cli --locked --test integration -- cli_data::commit_publish_and_drop_take_as_on_a_direct_graph_issue_1023 cli_data::as_on_a_served_write_is_rejected cli_data::optimize_with_as_flag_errors plus 7 cli_data::blob* tests gives all passed.
   • Parity rows that pass through the edited harness list: parity_commit_list, parity_commit_list_branch, parity_commit_changes, parity_commit_changes_filtered, parity_blob_put_and_clear_share_receipts_and_refusals, parity_blob_stat_preserves_exact_managed_and_external_metadata, parity_branch_create_delete give 7 passed.
4. Manual probe of the new doc claim. I proposed with --as act-author, waited 1.2 s, then ran commit publish --as act-reviewer. actor_id stayed act-author and created_at stayed the same (1792198816972432) in the publish answer and in commit show.

A .gqt case cannot express this defect (CLI flag classification), so the in-source matrix row plus the CLI integration test are the right tiers.

Limits:

• I did not run the full workspace gate, clippy or fmt locally. CI on this head shows 28 pass, 5 skipped, 0 fail.
• No test can show that the --as value reaches publish_commit_as on a direct graph, because nothing there reads it. I checked that by reading the code.
• My first parity attempt failed because the harness could not start omnigraph-server: the binary was not built and stderr was empty. After I built it, the rows passed. The PR did not cause this.

Optional findings

1. Test covers half of a new doc claim (inline at cli_data.rs:3095). Assert created_at as well as actor_id.
2. Test ownership (inline at cli_data.rs:3131). The served refusal could be two rows in as_on_a_served_write_is_rejected instead of a parallel block.
3. Follow-up: negative list (inline at planes.rs:209). The other families still default to accepting --as, and the harness keeps a string copy of the read list. One exhaustive "consumes actor" classifier would remove both risks.
4. Doc nit. docs/user/branching/index.md:149 was not rewrapped after the edit (118 characters, while the paragraph wraps at about 78).

Other open PRs and direction

• feat(commits): page a branch's commit list #1013 rewrites the same commit list/show/changes/publish/drop row in docs/user/cli/reference.md, so whichever PR lands second gets a small text conflict.
• fix(cli)!: send a server token only to the server it is keyed to #1030 changes how run_both in tests/support/mod.rs addresses the server, but not its read list. Expect a text merge only.
• feat(server,cli): report each branch's head, start and times in the branch listing #1012 records the branch creator on branch create, which makes --as observable there. If publisher provenance is added later, this PR's path already carries the actor to the engine.
• RFC rfc: graph access and cross-process writes #1017 (local sessions) keeps the same direct actor rule (--as, else operator.actor). This PR fits that direction.
• main is ahead of the base (now 0b5e17fa). GitHub reports the PR as mergeable (CLEAN).

[cursor]

Re-review: fix(cli): accept --as on commit publish and drop (head 030b5870)

Recommendation: approve. It can merge when ready. I found 0 blocking findings and 2 optional ones. One optional finding is still open from the first review; the other is new and small. I post this as a comment because my tool can only post comments, not approvals.

This re-review covers the one new commit since my review at 2d256564 (first review), and its effect on the whole PR.

Why, what, how, impact

• Why: On a direct graph, commit publish and commit drop are writes, but the CLI guard refused --as on every commit subcommand (CLI: commit publish and commit drop reject --as although they write #1023).
• What: The guard now matches the commit subcommands exhaustively: publish and drop accept --as, and list, show and changes still refuse it. 030b5870 changes only tests and one doc paragraph.
• How (the delta):
  • The issue test keeps the whole propose answer and now asserts that the published created_at equals the proposal's created_at (cli_data.rs:3104-3107).
  • The parallel served-refusal block is removed. commit publish <id> and commit drop <id> are now two more write rows in the existing owner as_on_a_served_write_is_rejected. Each row runs with and without --graph, and the owner still requires zero requests (cli_data.rs:3024-3055).
  • The branching guide paragraph is rewrapped. I diffed the word sequence: the change is whitespace only, and no line is longer than 78 characters.
• Impact: There is no change to production code since 2d256564. The tests now pin the full doc claim ("actor and timestamp"), and the served refusal has one owner. This removes one parallel fixture.

Status of the first review's findings

#	Finding	Status
1	Test pinned actor_id but not created_at	Resolved in 030b5870. A mutation check shows the assertion is effective (see Validation).
2	Served refusal had a parallel setup	Resolved. The rows are now in as_on_a_served_write_is_rejected.
3	Other command families still use a negative --as list, and the harness keeps a string copy	Still open, deferred by agreement. I accept the deferral. No follow-up issue exists yet (see inline reply).
4	Doc paragraph not rewrapped	Resolved (whitespace only).

Findings at this head

Blocking: none.

Optional:

1. Track the deferred classifier follow-up (reply on the planes.rs:209 thread). The author said this goes to a separate follow-up, but I found no open issue for it. Please file one before merge so the debt does not depend on memory.
2. Changelog wording (inline at changelog.d/commit-publish-drop-accept-as.fixed.md:1). The guide and the test now say the published commit keeps the actor and timestamp. The changelog still says only "keeps the actor". This is a one-phrase change for consistency, and the current text is not wrong.

Contract and liability

• Contract: This is unchanged since my first review. On a direct graph, --as reaches publish_commit_as / drop_commit_as. A served publish or drop still refuses --as in resolve_with_policy_scope before any request. The trusted actor stays at the server boundary (invariant 10). A publication still records no publisher; the RFC says this release adds no publisher provenance.
• Tradeoff: This is the same as before. On a direct open, --as on publish or drop has no observable effect today, because no policy is installed and nothing records the publisher. The deciding rule is still "a write that goes through an _as entry point accepts --as", which matches branch create/delete and the GQ commit publish statement.
• Liability: The delta lowers it. It removes one IntentApiFixture setup (net −5 lines in tests) and adds one assertion that guards a documented behavior. It adds no new API, state or format. No Lance surface is involved. created_at is OmniGraph lineage metadata that publish copies from the proposal record (crates/omnigraph-catalog/src/unpublished.rs:761), not Lance tag metadata. A future Lance version cannot change this contract.

Validation (my runs)

Environment: isolated worktree at 030b587065c5b09d746540bbf3cc4e7dac53feaf (base 9c2b3679), Rust 1.97.1, protoc 3.21.12, RUST_MIN_STACK=16777216, separate CARGO_TARGET_DIR.

1. Exact head (clean tree, git diff --quiet HEAD):
   • cargo test --locked -p omnigraph-cli --bin omnigraph planes:: gives 4 passed.
   • cargo test --locked -p omnigraph-cli --test integration -- cli_data::commit_publish_and_drop_take_as_on_a_direct_graph_issue_1023 cli_data::as_on_a_served_write_is_rejected cli_data::optimize_with_as_flag_errors --exact gives 3 passed.
   • Parity rows that go through the edited harness read list (parity_commit_list, parity_commit_list_branch, parity_commit_changes, parity_commit_changes_filtered, parity_branch_create_delete, parity_blob_put_and_clear_share_receipts_and_refusals, parity_blob_stat_preserves_exact_managed_and_external_metadata) give 7 passed.
   • cargo fmt --all --check is clean. cargo clippy -p omnigraph-cli --all-targets --locked -- -D warnings -W clippy::dbg_macro is clean.
2. Before the fix: the same tests fail for the predicted reason. I restored only crates/omnigraph-cli/src/planes.rs from 9c2b3679 and kept the head's tests. Then I ran the two cli_data tests above. Result: 2 FAILED.
   • as_on_a_served_write_is_rejected fails on the new row ["commit", "publish", "some-commit"] None. Stderr shows `commit` is a data command; --as sets the actor … does not apply, not the served refusal. So the moved served rows are real regression coverage.
   • The issue test fails at the commit publish --as step with the same message.
3. Mutation check for the new created_at assertion. I changed unpublished.rs:761 to created_at: property.created_at + 1. The issue test FAILED at cli_data.rs:3104 with left: 1792221573082002, right: 1792221573082001. A one-microsecond re-stamp on publish is caught.
4. Not vacuous. A manual run showed the proposal and publish JSON both carry the same non-null created_at (1792221524507173) across a 1.1 s gap. created_at is a required i64 in the API commit type, so Null == Null cannot occur.
5. I restored every temporary edit. After the restore, the head reran green and git status was empty. I pushed nothing.
6. CI on 030b5870: 27 checks pass, including Test Workspace, Fix Regression Gate, Lint (clippy) and GQ Logic Tests; 5 release-only jobs are skipped; 0 fail.

A .gqt case cannot express this contract. The defect is CLI argument classification and CLI JSON output, and GQT does not run the CLI parser. So the in-source matrix row and the CLI integration owner are the right tiers.

Limits:

• I did not run the full workspace gate locally; CI covers it at this head.
• My first parity attempt failed with "server did not become ready" after 30 s. The harness looks for <checkout>/target/debug/omnigraph-server, and with a separate target directory it falls back to cargo run, which builds the server inside the readiness window. With CARGO_BIN_EXE_omnigraph-server set, all 7 rows passed. This is local setup, not the PR.
• As before, no test can show that --as changes anything on a direct publish or drop, because nothing reads it there today. I checked the path by reading the code.

Other open PRs and direction

• fix(cli)!: send a server token only to the server it is keyed to #1030 also edits cli_data.rs and tests/support/mod.rs, but not as_on_a_served_write_is_rejected. Its head (bb0b308e) keeps IntentApiFixture::new, so the folded rows still compile after both merge. Expect a text merge only.
• feat(commits): page a branch's commit list #1013 and feat(branch): start a branch at a graph commit #1016 add text near line 126 of docs/user/branching/index.md. The rewrap here is at lines 146-154, so the hunks are near each other but do not overlap.
• main is 4 commits ahead of the base. None of them touch this PR's files, and GitHub reports CLEAN / MERGEABLE.