[ragnorc]

The branching guide said only that publishing an unpublished commit fails once an ordinary write moves the target head past the commit's parent. Users were left to discover the rest of the expiry rule by hitting not found.

Change

The "Unpublished commits" section now states the whole rule.

• An unpublished commit expires once its target head moves past the commit's parent, or when its target branch is deleted or recreated.
• An expired commit can only be dropped.
• A publish is refused; a moved head fails its precondition.
• Reads at its id (--snapshot, commit show, commit changes) answer not found.
• It leaves the --unpublished listing.

The rule is checked against the code:

• UnpublishedCommit::require_readable refuses every read path that resolves a commit id. Both GraphCoordinator::resolve_target and resolve_commit go through it.
• judge_lifetime decides expiry: HeadAdvanced, or IncarnationRetired when the recorded branch ref no longer resolves.
• require_droppable admits a drop of an expired, unpublished commit.
• The --unpublished listing keeps only live proposals (is_live()).

Tests

unpublished_commit_expired_drop_allowed_read_refused.gqt already pinned the refused --snapshot read and the drop, on the engine and served targets. data_routes::commit_routes_list_publish_and_drop_unpublished_commits now also covers a proposal expired by an ordinary write on main. It checks that the proposal:

• answers 404 to GET /commits/{id} and GET /commits/{id}/changes;
• is absent from GET /commits?unpublished=true;
• answers 412 to publish;
• still drops with 204.

No test covers expiry by deleting or recreating the target branch for an unpublished commit; that sentence comes from judge_lifetime.

Run locally:

• cargo test -p omnigraph-server --test integration -- data_routes::commit_routes: 2 passed.
• cargo clippy -p omnigraph-server --all-targets -- -D warnings.
• cargo fmt --all --check, scripts/check-docs.py and typos.

[ragnorc]

Recommendation: approve e6f721333fd1ce1967e247b735772bde95a1473a. I found no blocking defect or optional implementation change to request. This is a COMMENT review because the authenticated account is the author.

This PR explains why a previously readable proposal can return not found. An unpublished commit stays available while its recorded target authority remains live. Once the target head advances, or its branch is deleted or recreated, the proposal expires. The guide now states the consequences for reads, listing, publication, and drop. The added HTTP assertions verify the head-advance case. Production behavior does not change.

The wording matches the owner of the contract. judge_lifetime compares the recorded native branch and parent with the live target. require_readable, require_publishable, and require_droppable apply that verdict consistently. Expired proposals refuse new reads. Head advancement produces a publication precondition failure. A retired incarnation refuses publication as not found. Both allow drop, subject to policy. Already-published commits retain their separate repeat-publish and drop rules.

I traced snapshot resolution, commit metadata, changes, listing, and control operations. GraphCoordinator applies the same read gate to proposal snapshots and commit lookup. The listing selects only live proposals. CLI direct access and HTTP use these engine paths. The HTTP test appropriately checks 404, 412, and 204 at the transport boundary.

I read the current repository guides and relevant upstream branch, tag, versioning, and retention documentation. I checked the pinned Lance 11.0.0 implementation. Lance tags name physical versions, and its tag operations do not own OmniGraph proposal lifetime. OmniGraph's logical branch retirement and proposal gate explain why expiry can precede physical cleanup. No cloud-specific behavior is needed for this rule.

Tradeoffs and liability. Proposal review has a limited lifetime on a busy target. This clarification lets callers handle that limit explicitly. It adds no runtime work, API, state, or storage invariant. It removes an incomplete explanation and extends the existing route-test fixture. The maintenance cost is keeping one guide paragraph and these assertions aligned with the shared lifetime rule. Five similar clarifications can use their existing documentation and test owners without adding parallel mechanisms.

I surveyed the other open PRs. This clarification directly supports the branch-review work in #964 and #966: an unpublished proposal is not a durable review artifact after its target moves. Future live-branch schema work (#963) and actor views (#951) must preserve lifetime and policy checks. Future Lance versions still need compatibility checks for tag lookup, removal, and native branch identity. A dependency upgrade alone should not redefine proposal eligibility.

Validation. The existing expiry GQT case already covers the observable snapshot refusal and permitted drop. It passed at the exact head through both HTTP and the engine. GQT does not assert the raw HTTP status codes, so the route test is the appropriate additional owner.

cargo test --locked -p omnigraph-server --test integration -j 3 data_routes::commit_routes -- --nocapture
cargo test --locked -p omnigraph-gqt-served --test gqt_served_conformance -j 3 unpublished_commit_expired_drop_allowed_read_refused -- --nocapture

The server command passed both tests at the reviewed head. With only the PR's test diff temporarily applied, it also passed both tests at the parent, 9556daa2fb55f96d17f921071867ac1dd9d2efd6, and current main, 2d8bab7ad06be57ce13a7cab3e9150495db95118. This is evidence of an existing contract, not a fail-before/pass-after behavior fix. The GQT command ran one case, with served=ok and in-process=ok. All temporary edits were restored.

Commands used Rust 1.97.1, RUST_MIN_STACK=16777216, RUSTC_WRAPPER=, CARGO_TARGET_DIR=/tmp/review-928/target, and RUSTFLAGS='--cfg tokio_unstable --cfg tokio_unstable'. Documentation, AGENTS links, formatting, and diff checks passed.

CI and limits. Workspace CI reports 4,341 tests passed and 32 skipped, plus two passing doctests. CI used merge commit 3bca3740d1c652d0dbf93591731dcad8e7adbdf8, which includes main's count-distinct change. Its tree differs from this head, so it is separate evidence from the local exact-head runs. The new route assertions exercise head advancement. I verified deletion and recreation through code inspection, without running a proposal scenario for those two cases. I did not run the full workspace or live S3/Azure tests locally. This PR makes no performance claim.