Skip to content

feat(cli): serve a target for you on loopback - #1083

Closed
ragnorc wants to merge 88 commits into
ModernRelay:mainfrom
ragnorc:feat/cli-serve
Closed

ragnorc wants to merge 88 commits into
ModernRelay:mainfrom
ragnorc:feat/cli-serve

Conversation

@ragnorc

@ragnorc ragnorc commented Oct 19, 2026 •

Copy link
Copy Markdown
Contributor

Disposition — 2026-10-20

The second serving model is discontinued. OmniGraph retains one cluster-backed
server model, including on localhost: the ordinary applied state, authentication,
policies, admission, routes and deployment lifecycle.

This PR is closed without merging. The standalone/local serving APIs, generated
local policies and credentials, and local-server discovery/election/reuse
registry are not proceeding. This decision supersedes the implementation and
sequencing proposed below; draft #1017 is being revised accordingly. No part of
this serving stack has shipped. General improvements to the existing server
may be proposed independently without introducing another serving mode.

Historical proposal

Why

RFC #1017 phase 3: a local server for you. A same-account client (an editor, a notebook, Studio) should reach a direct target over the HTTP API without running a deployment.

What

omnigraph serve [--target NAME] [--reuse] [--port PORT] [--as ACTOR] [--json] serves one direct target on 127.0.0.1:

  • Scope: a standalone graph or a cluster root.
  • Access: read-only, as the --as/operator.actor actor, behind a bearer token generated for the run. --write refuses with local_server_read_write_unavailable until a write whose outcome is unknown no longer stops the server (bug: one storage write with an unknown outcome stops the server until a manual unlock #1067); that gate is the server's, from feat(server): serve one standalone graph to the same account on loopback #1075.
  • Ready line: with --json, stdout carries one line (protocol 1) once the server handles signals. It includes the token only when stdout is not a terminal.
  • Records: an owner-only record in a per-account directory (macOS ~/Library/Application Support/OmniGraph/servers, Linux ~/.local/state/omnigraph/servers, or OMNIGRAPH_LOCAL_SERVERS_DIR). A liveness lock held by the process means a killed server's record is ignored and reaped. Platforms without Unix permissions refuse, since a record holds a token.
  • One server per destination (scope plus canonical root). Concurrent starts are elected; a second start refuses with local_server_exists, or with --reuse prints the live server's ready line, carrying the requesting target's graph. Reuse refuses on an access, actor or HTTP-contract mismatch.
  • Loopback guard, outside every server layer: Host must name this server's loopback address, any Origin refuses, CORS headers are stripped, and every response carries Cross-Origin-Resource-Policy: same-origin.
  • Listing: target list|show name each direct target's live local servers. version --json reports protocols.serve and protocols.local_server_record.

How

serve binds the listener, opens AppState::local through LocalBinding::local with the destination's canonical path, and installs the lifecycle with start_local before announcing. A failure after that (holding, writing or announcing the record) ends the server with LocalServing::release, so a start never strands a cluster's admission.

Stack

This branch contains #1081 (and through it #1073, #1074, #1076, #1078, #1080) and #1077 (and #1075). The delta reviewed here is crates/omnigraph-cli/src/commands/serve/, crates/omnigraph-cli/src/local_servers.rs, the serve wiring, docs/user/cli/serve.md and cli_local_servers. Merge after those.

Tests

  • Process level, in cli_local_servers:
    • a read-only server behind the guard (Host, Origin, token, a write refused before its body);
    • reuse and its refusals, the listing, and SIGTERM exiting 0 with the record removed;
    • concurrent starts electing one server;
    • a killed server leaving no live record;
    • a cluster root served under its policy, including reuse from another target of the same root;
    • the read-write gate;
    • refusals before opening anything.
  • Unit tests for records and the guard.
  • CLI, server and policy suites, clippy, fmt, docs, typos and dependency-source checks pass locally. A Codex Astra review of the serve commit found four issues; they are fixed in f5136d4 and d905660.

A request carrying omnigraph-expect-actor is refused with 412
actor_mismatch, after authentication and the contract check and before
graph lookup, body work or effects, when its credential resolves to
another actor. A match changes nothing, so the header only narrows a
request and never supplies an actor. Every protected router and /mcp
check it.

The CLI sends it for --as on served writes and cluster --server
commands, which no longer refuse --as. A mismatch is a not-started
refusal and exits 4, like a failed graph-commit precondition.
AppState::local opens a standalone graph read-only or read-write behind a
generated bearer token and a Cedar policy generated for one actor, which
it installs on the engine as well as at the HTTP layer. Every policy
action has a local-server class, so an action added later is never
granted until it is given one. A read-only local server refuses every
write-class route in ingress before reading its body. No local server
mounts the cluster deployment routes; each enables MCP ad-hoc reads and
lists write tools only when it writes. A root inside a cluster is
refused.

serve_local runs the deployment's serving lifecycle, now one private
type both entry points share, on a loopback listener the caller bound.
It writes nothing to stdout, disarms the shutdown watchdog when it
returns, and reports an uncertain outcome instead of exiting.
install_log_subscriber puts the server's restricted log filter on
another writer.
LocalScope::Cluster serves a cluster's storage root from its applied
state for one actor, with the cluster's own policies, stored queries,
embedding-provider bindings and external-Blob rules. A read-only one
captures one ledger revision without admission, opens every graph
read-only and validates it against that revision, and forbids every
write action in each policy it loads, so policy answers report the
refusal. A read-write one takes the cluster's serving admission and
refuses while another owner holds it; serve_local releases it after a
clean drain through the release the deployment runs, and keeps it when
a completion is uncertain.

PolicyEngine::forbidding adds a Cedar forbid for a set of actions, and a
prepared graph open now names its access: writable within a storage
scope, or read-only against a captured ledger revision.
Every error body carries an outcome object (execution, effects, action)
that the server computes for the request it answered: not_started with
no effects when it refused before submitting a write, or refused the
write's one publication with a typed reason, which an engine gives only
before it publishes; unknown otherwise, including a typed refusal after
a load forked its branch. The next step a caller takes is retry,
refresh, recover or reconcile. A write whose completion is uncertain
answers outcome_unknown instead of internal. Errors that carried a
reason only in their details gain registry codes: precondition_failed,
recovery_required, external_blob_source and change_feed_gap.

CLI data-write failures print the same object as outcome, in place of
command_outcome, and take the server's when its request was the
command's only effectful one.
The exit follows from the failure's outcome: a command that did not
start exits 1, one whose effects are unknown 2, one refused for a
condition the caller set 4, and one refused before starting with retry
75. A refused command line exits 64 (EX_USAGE) instead of Clap's 2.

Read commands report the same failure envelope as data writes, always
not_started with no effects, so a failed read prints its outcome too.

An embedded engine write's own error now carries the engine's
completion evidence, as a server's does: a typed refusal before any
request, outside a load that forks its branch, did not start, so
embedded load conflicts and failed conditions exit as served ones
instead of claiming unknown effects. Any later error, such as one
rendering a committed receipt, stays unknown. Embedded writes refuse a
noncanonical branch name and read their data file before opening
storage.

Managed commands keep their own exits until the managed relay lands.
GET /graphs/{graph_id}/access and the MCP access tool return the
caller's resolved actor, the actions its credential allows on the whole
graph, and, for a named branch and target branch, the actions allowed
there. Each action is evaluated through authorize with the branch and
target branch shaped as the route that performs it shapes them, so the
answer matches those routes' decisions. The route needs no action of
its own, reads no graph data and judges branches by name.
A target is one complete binding in ~/.omnigraph/config.yaml: a direct
graph or cluster root, or a server with its credential owner or
anonymous access. --target NAME, else defaults.target, addresses a
command; it refuses every other address and --profile beside it, and
resolves through the scope resolver profiles use. A server target's
owner keys the existing token chain and is bound to the target's
endpoint, so its token goes only to URLs under it; the file refuses an
owner at two endpoints, two owners for one endpoint and an anonymous
target beside a keyed one.

target list|show|add|remove lists the targets without secrets or
network, with a binding digest, and edits one entry at a time: an edit
holds config.yaml.lock, changes only that entry's lines, proves by
parsing that every other key is unchanged, and replaces the file by
rename. login/logout --target store under the target's owner, and
credential get --target hands a server target's token to a pipe, never
a terminal. Refusals carry registry codes (target_unknown,
target_invalid, target_exists, target_is_default, config_not_editable,
target_kind_unsupported, credential_terminal, credential_anonymous,
credential_missing). version --json reports protocols.target and
protocols.credential. Existing servers, profiles and their flags keep
working.
# Conflicts:
#	crates/omnigraph-server/tests/integration/auth_policy.rs
omnigraph access names the caller's actor and the actions the graph's
policy grants it: graph-wide, on --branch, and from it to
--target-branch for branch_create and branch_merge. A server target
asks the server's access route with the target's token. For direct
storage the CLI decides for the actor --as or operator.actor names: a
cluster graph by its applied policy, which allows nothing without an
actor, and a standalone graph, which has no policy, allows every
action, as direct writes do.

The decision shape moves into api-types (AccessOutput::evaluate and the
AccessAction to PolicyAction mapping), so the route and the CLI decide
alike. version --json reports protocols.access.
# Conflicts:
#	crates/omnigraph-server/tests/integration/auth_policy.rs
The shutdown listener registered its SIGTERM and SIGINT handlers only
when its spawned task first ran, and `serve_local` installed the
lifecycle only after the caller had announced readiness. A signal in
either window met the default disposition and ended the process without
the drain.

The lifecycle now registers both handlers before it returns, and
`start_local` installs it so a caller announces between `start_local`
and `LocalServing::serve`. `serve_local` remains the two calls in one.
…server

# Conflicts:
#	crates/omnigraph-server/src/lib.rs
#	crates/omnigraph-server/tests/boot_settings.rs
#	docs/dev/control-plane.md
`omnigraph serve` serves one direct target, a standalone graph or a
cluster root, on 127.0.0.1 to clients of the same account. It is
read-only unless `--write`, acts as the `--as` or `operator.actor`
actor, and answers behind a token generated for the run.

It keeps an owner-only record a client attaches by reading, held by a
lock its process owns, so a killed server's record is ignored and
removed by the next reader. Starts of one destination are elected: one
serves and the others refuse with `local_server_exists`, or print the
live server's ready line with `--reuse` when its access, actor and HTTP
contract fit.

A guard outside every server layer admits only requests whose Host
names this server's loopback address, refuses any Origin, strips CORS
headers and marks every response same-origin. The ready line follows
`start_local`, so a signal after it drains the server.

`target list` and `target show` name each target's live local servers,
and `version --json` reports `protocols.serve` and
`protocols.local_server_record`.
…e records

A reuse from another target of the same root now reports that target's
graph. A platform without Unix permissions refuses to keep a record,
since it holds a bearer token, and a target listing reads records
without creating their directory or failing on it. The listener binds
before a read-write cluster open takes the cluster's admission.
…es it

A server built before `omnigraph-expect-actor` speaks the same HTTP
contract and ignores the header, so `--as` on a served write would run
as the credential's actor. Every response now lists the assertions its
server enforces in `omnigraph-http-api-assertions`, and the CLI refuses
before sending when discovery does not list `expect-actor`.

A served `cluster plan` or `cluster apply` refused for another actor
now exits 4 as a data write does: its requests are observed, and a
refused assertion with nothing effectful sent before it is a condition,
with or without `--json`.
`main` returned an undelivered failure as an error, so the process
exited 1, the status that says nothing changed, after a write whose
receipt met a closed stdout. It now reports the error as before and
exits with the code the outcome chose.

`alias` is a read, as `query` is, so its failures report an outcome
and exit by it. Command classification is now an exhaustive match, so
a new command declares how its failure reads.
…s everywhere

Credential lookup now reads the same validated bindings as the
ownership checks, so a server target that fails validation can no
longer win a request's credential and send another owner's token to
its endpoint.

`queries` and `policy` resolve `--target` and `defaults.target` like
every other command: a target is a complete address that excludes
`--cluster` and `--profile`, only a cluster-root target names a
cluster, and its default graph applies.

`login` and `logout` check after parsing that exactly one of a server
name, `--api` and `--target` names the credential, so a global
`--target` before the subcommand works and a conflict refuses.

A configuration refusal keeps its code through the invocation's
captured configuration, and a target edit is a local write: a failure
after its rename no longer reports that nothing changed.
# Conflicts:
#	crates/omnigraph-cli/src/command_outcome.rs
# Conflicts:
#	crates/omnigraph-cli/src/command_outcome.rs
A quarantined graph's policy bundle is withheld from the serving
snapshot and an unknown graph has none, so `access` read either as a
graph without a policy and reported every action allowed. It now
checks the snapshot first: a graph that is not applied refuses with
`not_found`, naming the applied graphs, and a quarantined one with
`graph_unavailable` and the cluster's reason, as a server refuses them.
…tests off the account's records

A records directory that does not exist means no local server has run,
so a direct target lists none; one that cannot be read safely leaves
`local_servers` absent. CLI tests point the records directory at a path
that does not exist, so no test reads the account's own records. `serve`
reports its own failures and takes no outcome kind.
The cluster skill page still said `cluster apply --server` refuses
`--as`; it now names the assertion `cluster plan` and `cluster apply`
send and its exit 4. The release note moves to `breaking`, since a
matching `--as` now runs a write the CLI used to refuse. The server
guide says `/mcp` has no contract check, an empty assertion joins the
malformed-header test, and stale comments name the current behaviour.
…ne's policy

- A relative or URL-special local root now names its cluster.
  `LocalBinding::from_root` makes a local root absolute, and the cluster
  storage URI is built with `Url::from_file_path`, so `./cluster` no longer
  becomes `file://./cluster`, which the cluster store refused in both modes.
- A ledger revision that moves between a read-only open's capture and its
  validation refuses the whole server. `GraphOpenFailure` carries the
  cluster's refusal from `validate_opened`, and the read-only open turns it
  into `LocalOpenError::Cluster`. A graph whose schema drifted from its
  achieved contract is still blocked alone, as normal serving blocks it.
- A user rule can share an id with the generated forbid. Cedar policy ids
  are namespaced `permit:` and `forbid:`, so the two can no longer collide
  and fail `PolicySet::add`. A matched rule still reports the bare rule id.
- `LocalServing::release` stops a local server that never served. It closes
  request admission, drains the logical owners, releases a read-write
  cluster's serving admission as `serve` does after a clean drain, and
  disarms the watchdog. Before this, a caller that failed after
  `start_local` could only drop it, which keeps the admission lock.
- `/readyz` and `/graphs` report the ledger revision served. Both access
  modes install the boot witness from the applied snapshot instead of the
  default revision 0.
- A local cluster server refuses when no graph opens, as a deployment's
  startup does. Read-only now applies the same check as `finish_graph_startup`,
  and both modes honour `OMNIGRAPH_REQUIRE_ALL_GRAPHS` through the config's
  `require_all_graphs`, not a hard-coded `false`.
- A graph the cluster binds no policy to gets `read` alone for the actor,
  which is what a deployment's routes grant without a bundle. The grant is
  installed on the engine too, so an embedder holding the handle can no
  longer write through an engine with no policy.
- Read-write refusals before any graph opens release the admission, as a
  refused settings preflight does. The catalogue policy now loads before
  graph preparation. A failed startup after a graph open keeps the admission
  and names its lock in `LocalOpenError::AdmissionRetained`.
- The read-only catalogue policy passes through the same write forbid as
  every other policy it loads.
The access report's branch slots were lists in the evaluator, so a new
policy action compiled and stayed out of the report. One exhaustive
match on `PolicyAction` now gives each action its slot (source, target,
transition, graph-wide or unreported), shared by the access route and
`omnigraph access`.
A write whose outcome is unknown still stops the server that ran it, so
`AppState::local` refuses `LocalAccess::ReadWrite` with
`LocalOpenError::ReadWriteUnavailable` until it no longer does. The
read-write path stays and stays tested: the `test-util` feature's
`with_local_read_write` scope opens it for tests alone, enabled from
dev-dependencies only (this crate's own included) and checked by the
test-util guard, and a build with the feature still refuses outside the
scope.

`LocalBinding::local(path)` replaces the string constructor. It takes a
root on this machine's file system, made absolute, and refuses a URI: a
remote root's endpoint and credentials would come from the process
environment when it opens, which a captured binding must not depend on.

A read-only local server's engine refuses a direct write, now tested on
the handle. The developer guide says what the read-write gate is, that
an embedder a browser can reach adds the loopback guard in `extend`, and
that Tokio keeps the signal handlers for the rest of the process. The
invariants say the library serves one standalone graph on loopback
while the binary stays cluster-only, and the release notes no longer
claim read-write or a wider signal window than the code covers.
Brings in ModernRelay#1075's fix through ModernRelay#1077: the read-only refusal names no
--write, and a local binding folds `..` before it is stored.
The engine re-exports the storage crate's lexical absolutizer for the
local server's binding; the split-crate re-export guard lists every such
item by name, so it now names this one.
The key-conflict description said the refused rows wrote nothing; the
OpenAPI vocabulary guard keeps graph vocabulary in the published
description, so it now says the refused write changed nothing.
Brings in the engine's pinned re-export of absolutize_lexically.
Brings in the key-conflict description in graph vocabulary.
Brings in the key-conflict description in graph vocabulary.
Brings in the key-conflict description in graph vocabulary.
Brings in the key-conflict description in graph vocabulary.
Brings in the engine's pinned re-export of absolutize_lexically.

ragnorc commented Oct 19, 2026

Copy link
Copy Markdown
Contributor Author

Recommendation: approve.

Why, what, how, impact

omnigraph serve starts one loopback server for one direct target and one OS account. A client of that account attaches by the record, or by serve --reuse --json.

This re-review covers d6702d1e..6ee0c3d1. The serve commits in that range are 4c0eb1da and 5c075c2f. Later merges bring in feat/cli-access (fb2ab08b, #1081) and feat/local-cluster-server (b296337b, #1077). Those merges keep the serve fixes. Where a file differed, the merge kept the copy that already had the newer HTTP contract and the lexical path fold.

The blocking item from the review at d6702d1e is fixed. A missing or blank host id makes Records::open and Records::existing return local_server_records_unsafe. Nothing writes machine: "unknown".

Previous findings

  1. Blocking, machine_id fails open. Resolved. host_id returns None when the platform gives no id (local_servers.rs). this_machine refuses a missing or blank value. open and existing use that function. records_refuse_a_machine_without_a_host_identifier covers None, "", and whitespace, and it leaves a foreign record in place.

  2. targets.md "empty today". Resolved. The listing section says local_servers names live servers and is absent when the records directory is not safe.

  3. Skill and help. Partly resolved. SKILL.md, references/commands.md, references/cluster.md, and the serve help say that serve is a loopback server for this account and that a deployment remains omnigraph-server. references/server-policy.md line 17 still says "There is no single-graph server mode." The RFC names that file. See optional 2.

  4. Listing regression. Resolved. serve_refuses_what_it_cannot_serve_before_opening_anything checks a missing directory (local_servers: []) and a mode 0755 directory (the field is omitted, then serve refuses with local_server_records_unsafe and an outcome).

  5. Refusal outcome. Resolved. with_refusal_outcome sets not_started / none, with retry for local_server_starting and refresh otherwise. A wrapped unclean release carries no outcome. Exit stays 1. The exit-75 gap is in the RFC section below.

  6. Storage format on reuse. Resolved. The record's cli.internal_schema comes from the same source as version --json. --reuse refuses on any difference, or on a missing field, with local_server_cli_incompatible. The record format stays 1. The reuse test edits a live record for an HTTP-contract mismatch, a different writes value, and a missing field.

  7. Lock read path. Resolved. live_records opens {id}.lock with create: false. A record without its lock stays on disk and is not live. The liveness unit test covers that.

  8. #[cfg(unix)]. Resolved. tests/integration/main.rs registers #[cfg(unix)] mod cli_local_servers;. That file is not compiled on other platforms.

Blocking

None.

Optional

  1. Inline on crates/omnigraph-cli/src/local_servers.rs line 204 (return Some(id.to_string()) in host_id). host_id accepts any non-empty string. On Linux, machine-id(5) says a machine id is 32 lowercase hex characters and is not all zeros. The string uninitialized is the first-boot marker. systemd writes that string to /etc/machine-id before it overmounts the real id. Two hosts that both read uninitialized, or 000…0, compare equal. live_records can then reap or reuse the other host's record and token. That is the same shared-home hazard. Accept only a 32-hex id that is not all zeros on Linux, and a non-zero 16-byte id from gethostuuid on macOS. Refuse every other value with local_server_records_unsafe. A cloned valid id remains the limit the RFC chose.

  2. In skills/omnigraph/references/server-policy.md line 17, replace "There is no single-graph server mode." Say that omnigraph serve is a loopback server for this account, and that omnigraph-server remains the deployment. The line is outside this pull request's diff hunk, so this note is in the body.

Where the code and RFC #1017 differ

RFC head e15d2f5b85a759b053dda73082a5c9e7fff8e121. The 2026-10-19 decision log records the embedding API, the cluster-graph refusal, and internal_schema on the record sample. These rows still differ. This branch merges after that RFC is accepted, so each row needs a decision in the RFC or in the code.

  • The platform table says serve is Supported on Windows, with FOLDERID_LocalAppData, an ACL, and MachineGuid. The code and serve.md refuse every non-Unix platform with local_server_records_unsafe. Keep the refusal until the ACL check exists. Change the RFC row.
  • The ready-line sentence says binding is a digest of the resolved destination. The code and serve.md put TargetBinding::digest() on the line. The record's destination field is the scope-and-root digest. Change the RFC sentence to the target binding digest.
  • The code table says local_server_records_unsafe includes target list and target show. The code omits local_servers and still lists the other targets. serve refuses. serve.md describes the omission. Change the code table. The listing prose that says a direct target always carries local_servers needs the same sentence.
  • An s3:// or az:// target fails as target_invalid in the target resolver, before Destination::of. Destination::of returns local_server_destination_unknown for a non-file URI. serve.md documents target_invalid. Say that in the RFC.
  • A direct graph whose path is inside a cluster becomes target_invalid (LocalOpenError::ClusterGraph). The decision log and the embedding API say the caller names the cluster scope. The user table still says "A direct graph target inside a cluster root serves its cluster." The code follows the decision log. Make the user table match.
  • local_server_starting carries outcome.action: retry. The RFC exit table maps that pair to exit 75. This command exits 1, and serve.md says exit 1. Command::Serve stays outside command_outcome::kind, so a later error is not labeled not_started. Give this refusal exit 75 when the exit table reaches serve, or write exit 1 in the RFC.
  • --reuse refuses --write against a read-only server. A read-only --reuse of a future read-write server would receive the write token. Decide that direction in the RFC before --write can succeed.

Stack

Leave these on their own pull requests.

origin/main is 9e2425d2, two commits ahead of this head. git merge-tree reports no conflict.

Tradeoffs and liability

The records directory, the JSON record, the liveness lock, and the election file stay the four primitives. The host id is now a closed input: no id means no records. That removes the shared-home fail-open for a missing file. It adds a refusal on a host that has no id.

The optional format check is that same rule for a file that is present and is not an id. Further record readers should call this_machine.

internal_schema on format 1 lets an older reader still see the server. A format bump would hide that server and allow a second start. RECORD_FORMAT states that rule.

Exit 1 for local_server_starting keeps one exit path for serve. The JSON action says retry. A caller that retries only on exit 75 will not retry. That is the RFC gap above.

The 100 ms election poll blocks one worker for at most 60 seconds. This process does no other work during the wait. The bound is in serve.md.

Validation

Reviewed head: 6ee0c3d10154f1f342fc37dbbde5fdce8a74f3f9. RFC head: e15d2f5b85a759b053dda73082a5c9e7fff8e121. origin/main: 9e2425d26a03b1b5628030a359ee51f096d3ca1c.

A .gqt case cannot express a process, an exit code, a record file, flock, or a host id. The owner is omnigraph-cli: local_servers and commands::serve in the library, and tests/integration/cli_local_servers.rs.

Local, exact head, RUST_MIN_STACK=16777216:

cargo test -p omnigraph-cli --locked --lib --test integration -- \
  local_servers:: commands::serve:: cli_local_servers:: \
  version_command_prints_cli_version_http_contract_and_storage_formats_issue_977 \
  --test-threads=4

Library: 6 passed, 0 failed (0.21s). Integration: 7 passed, 0 failed (0.33s).

Regression proof for the host-id fix. At d6702d1e, machine_id returns the string unknown when the id cannot be read. The parent of the fix is 306356b0. The new test calls Records::open_at, which does not exist at that commit. The stand-in, applied on this head and then restored, makes this_machine return "unknown" for a missing or blank host.

  • At 6ee0c3d1, cargo test -p omnigraph-cli --locked --lib local_servers::tests::records_refuse_a_machine_without_a_host_identifier -- --exact passed.
  • With the stand-in, the same command failed. open_at returned success. The test panicked at a start without a host identifier refuses (local_servers.rs in the test).
  • After git checkout -- crates/omnigraph-cli/src/local_servers.rs, the same command passed. The worktree is clean. Nothing was pushed.

CI on this SHA: run 39277633407 (CI) succeeded, including Test Workspace. Run 39277633168 (GQ Logic Tests) failed in GQT (dst): case::load_csv_integer_endpoint_memory.gqt exceeded its 10s wall-time budget (worker budget 2.06s). GQT (ordinary) was cancelled. That case is outside the serve surface. I did not rerun the corpus.

Limitations

The terminal path that omits token is from stdout().is_terminal() in app.rs. I did not run a pseudo-terminal. I did not run Windows. I did not run the full workspace suite locally. No performance claim.

A host identifier was any non-empty string, so two machines that both
read systemd's `uninitialized` first-boot marker, or an all-zero id,
compared equal and could reap or reuse each other's records and tokens
in a shared home. A machine id is now 32 hex digits and not all zeros,
which both sources produce (a Linux machine-id and macOS's 16-byte host
UUID); anything else refuses with `local_server_records_unsafe`. The
server-policy reference no longer says there is no single-graph server
mode: `omnigraph serve` serves one direct target on loopback.
@ragnorc

ragnorc commented Oct 20, 2026

Copy link
Copy Markdown
Contributor Author

Both optional findings addressed at 3480267:

  1. Machine id shape. A host identifier is accepted only as 32 hex digits and not all zeros, the shape a Linux machine-id and macOS's 16-byte host UUID both produce. systemd's uninitialized marker, an all-zero id, a short id and a non-hex id refuse with local_server_records_unsafe. records_refuse_a_machine_without_a_host_identifier covers each, and the test machines now use valid ids.
  2. server-policy.md. The line now says omnigraph serve serves one direct target on loopback to this account, read-only for now, and omnigraph-server remains the deployment.

local_servers:: and the skill tests pass, as do the six cli_local_servers integration tests and clippy with -D warnings.

@cursor

cursor Bot commented Oct 20, 2026

Copy link
Copy Markdown

Recommendation: approve.

Why, what, how, impact

omnigraph serve starts one loopback server for one direct target and one OS account. A client of that account attaches by the record, or by serve --reuse --json.

This re-review covers 6ee0c3d1..34802678. That range is one commit, 34802678 (fix(cli): accept only a real machine id for local server records). It changes crates/omnigraph-cli/src/local_servers.rs and skills/omnigraph/references/server-policy.md.

The machine field is the fact that keeps one machine from reaping or reusing another machine's record when a home directory is shared. The old check treated every non-empty string as an id. Two hosts that both read systemd's uninitialized marker, or an all-zero id, compared equal. this_machine now accepts one shape: 32 hex digits, after trim and ASCII lowercasing, and not all zeros. Records::open and Records::existing both use that function. A refusal is local_server_records_unsafe, and open returns before it creates the directory. live_records compares the stored machine to that same value. No other caller reads host_id.

Previous optional findings

  1. Host id accepted uninitialized and all zeros. Resolved. The check is the grammar above, not a special case for one sentinel. records_refuse_a_machine_without_a_host_identifier now refuses None, "", whitespace, uninitialized, 32 zeros, 16 hex digits, and a 32-character non-hex string. The test machines are valid ids (0123456789abcdef0123456789abcdef and fedcba9876543210fedcba9876543210).

  2. server-policy.md said there is no single-graph server mode. Resolved. The sentence now says omnigraph serve serves one direct target on loopback to this account, read-only for now, and omnigraph-server remains the deployment. docs/user/operations/server.md still says omnigraph-server has no single-graph boot mode. That sentence is about the deployment binary's boot, and it remains true.

Is the 32-hex rule right?

Yes for every host id this code reads. Refusing every other value is the correct fail-closed behavior.

Linux. machine-id(5) defines /etc/machine-id as one newline-terminated, lowercase, 32-character hex id, and not all zeros. The same page says the D-Bus id at /var/lib/dbus/machine-id uses that format. host_id trims the file and tries /etc/machine-id first, then the D-Bus file when the first file is missing or empty. this_machine trims again and lowercases, so a valid uppercase file still matches. uninitialized is the first-boot marker. systemd writes that string and then overmounts the real id. A normal read of /etc/machine-id after that overmount returns the real id, and the grammar accepts it. A read that still sees uninitialized refuses. That matches the marker: it is not an id, and two machines can both have it. The code does not skip a present non-id and use the D-Bus file. systemd treats uninitialized as first boot, not as an empty file. An empty /etc/machine-id still falls through to D-Bus before the grammar runs. A cloned copy of a real id remains possible. The RFC already accepts that limit.

macOS. host_id calls gethostuuid with a one-second wait. The man page says success writes a 16-byte uuid_t. The code formats each byte as two uppercase hex digits. That string has length 32 and has no dashes. this_machine lowercases it and accepts it when any byte is not zero. A failed or timed-out call returns None, and records refuse. The code does not read the IOPlatformUUID string, and it does not delete dashes from that string. The dashed form is 36 characters and would refuse. The stored value is the hex of the 16 bytes gethostuuid wrote, in that order.

Other platforms. host_id returns None. this_machine refuses. open hits that refusal before the non-Unix directory check. There is no Windows MachineGuid reader in this tree. A later Windows source must turn that registry string into 16 bytes inside host_id before this grammar. The registry text contains dashes. Passed through unchanged, it would refuse every Windows host.

Blocking

None.

Optional

  1. In docs/user/cli/serve.md, the records section still says a machine refuses when its host identifier cannot be read. The same words are in changelog.d/cli-serve.added.md. A present file can now refuse too: uninitialized, all zeros, a short id, or a non-hex id. The error text already says the id is missing or is not a machine id. Add that sentence to the guide and the release note, beside the two Linux paths and gethostuuid.

Where the code and RFC #1017 differ

RFC head e15d2f5b85a759b053dda73082a5c9e7fff8e121. The rows from the review at 6ee0c3d1 are unchanged. This commit adds one row.

  • The machine sentence says the field is gethostuuid, /etc/machine-id, or Windows MachineGuid. The code stores the lowercased 32-hex form and refuses every other value. The platform table still says Windows is supported. The code refuses every non-Unix platform, and it has no MachineGuid reader. Update those RFC sentences with this grammar.

Stack, and the rebase onto main

origin/main is b8e82abf. git merge-tree --write-tree of that commit and this head exits 1. The one content conflict is crates/omnigraph-cli/src/client.rs, in embedded branch_create. Main's #1016 passes a ReadTarget and uses the returned value. This branch still calls branch_create_from_as(ReadTarget::branch(from), ...) and drops the result with engine_write(). The same merge against 9e2425d2 exits 0. This conflict is not in 34802678. Rebase or merge main before landing this pull request.

Sibling heads today:

  • #1075 c4f844cb, #1076 78963e94, #1077 b296337b, and #1078 0a11b0da are ancestors of this head.
  • #1074 7474e61a is not. It renames the actor header to Omnigraph-If-Actor and merges main through 9e2425d2. This head still sends omnigraph-expect-actor. Rebase onto that head, or a later merge can put the old header back.
  • #1081 3a3f8375 is one commit past the copy in this head (fix(cli): read a store's cluster ledger where its storage is). That commit is not in this head.
  • #1084 24be23f4 contains 6ee0c3d1 and does not contain 34802678. It needs this commit before it relies on the machine-id rule.
  • #1090 is a separate CLI rewrite. Its server-policy.md still says there is no single-graph server mode. It does not contain omnigraph serve. Expect text conflicts if both land. It does not change this grammar.
  • #1091 changes branch deletion beside static routes. It does not touch local server records.
  • Draft #1089 (HTTP API conventions) and draft #1093 (storage outcomes) do not define the host id. Read-only serve still waits on a write whose outcome is unknown.

Tradeoffs and liability

The records directory, the JSON record, the liveness lock, and the election file stay the four primitives. The host id now has one grammar, at the only gate that opens records. That removes the shared-home fail-open for a sentinel or an all-zero id. It adds a refusal on a host that has no real machine id yet, including a first boot that still shows uninitialized. A bearer token in a shared home is the higher cost. Availability of serve on that host is the lower cost.

A further platform source should normalize to 16 bytes in host_id and then use this grammar. A special case per sentinel would bring the old hole back. A cloned valid id can still match. That limit stays with the RFC.

The check is one predicate on a short string. It does not change publication, snapshots, or Lance.

Validation

Reviewed head: 34802678db51f4cb721898656ef80987e3633743. Previous head: 6ee0c3d10154f1f342fc37dbbde5fdce8a74f3f9. origin/main: b8e82abfbaa02a0e14b65e3bb034d1a81f281864. RFC head: e15d2f5b85a759b053dda73082a5c9e7fff8e121.

A .gqt case cannot express a process, a record file, or a host id. The owner is omnigraph-cli: local_servers in the library, and tests/integration/cli_local_servers.rs.

Regression proof for the host-id grammar. The test committed at 6ee0c3d1 does not include the new inputs, so that binary would stay green. On 6ee0c3d1 I added the head's four inputs to records_refuse_a_machine_without_a_host_identifier, ran it, and restored the file. Nothing was pushed.

# at 6ee0c3d1, with those four inputs added temporarily
RUST_MIN_STACK=16777216 cargo test -p omnigraph-cli --locked --lib \
  local_servers::tests::records_refuse_a_machine_without_a_host_identifier -- --exact

Failed. open_at accepted uninitialized, 32 zeros, 0123456789abcdef, and g123456789abcdef0123456789abcdef. None, "", and whitespace still refused. The old filter kept every non-empty trimmed string. That is the predicted reason.

# at 34802678, the committed test
RUST_MIN_STACK=16777216 cargo test -p omnigraph-cli --locked --lib local_servers:: -- --test-threads=4
RUST_MIN_STACK=16777216 cargo test -p omnigraph-cli --locked --test integration cli_local_servers:: -- --test-threads=4
RUST_MIN_STACK=16777216 cargo test -p omnigraph-skill --locked --lib -- --test-threads=4
RUST_MIN_STACK=16777216 cargo test -p omnigraph-cli --locked --lib skill:: -- --test-threads=4
cargo clippy -p omnigraph-cli -p omnigraph-skill --all-targets --locked -- -D warnings

local_servers::: 4 passed, 0 failed. cli_local_servers::: 6 passed, 0 failed. Skill library: 1 passed. CLI skill::: 6 passed. Clippy exit 0.

This machine's /etc/machine-id and /var/lib/dbus/machine-id are the same 32-hex id, so the integration tests take the accept path of the real host_id.

CI on this SHA: run 39359862315 (CI) succeeded, including Test Workspace and Lint (clippy). Run 39359862119 (GQ Logic Tests) succeeded, including GQT ordinary and GQT dst. Four release-tag jobs were skipped. I did not rerun the corpus locally.

Limitations

I did not run macOS, so gethostuuid is from the man page and this source. I did not run Windows. I did not run the full workspace suite locally. No performance claim.

…uses

The serve guide and its release note said a machine refuses to keep
records only when its host identifier cannot be read. Since records
accept only 32 hexadecimal digits that are not all zeros, a present
identifier refuses too: systemd's first-boot `uninitialized` marker,
all zeros, a short id or a non-hexadecimal one.
@cursor

cursor Bot commented Oct 20, 2026

Copy link
Copy Markdown

Recommendation: approve.

Why, what, how, impact

omnigraph serve starts one loopback server for one direct target and one OS account. A client of that account attaches by the record, or by serve --reuse --json.

This re-review covers 34802678..7175120b. The range is one commit, 7175120b (docs(cli): say a present host identifier that is not a machine id refuses). It changes changelog.d/cli-serve.added.md and docs/user/cli/serve.md. It changes no Rust file.

The optional finding from the review at 34802678 is resolved. The guide and the release note now match this_machine in crates/omnigraph-cli/src/local_servers.rs.

Previous optional finding

The guide named only a missing host identifier. Resolved.

docs/user/cli/serve.md says the host identifier refuses with local_server_records_unsafe when it cannot be read or is not a machine id. The sources are gethostuuid on macOS, /etc/machine-id on Linux, and /var/lib/dbus/machine-id on Linux. A machine id is 32 hexadecimal digits and is not all zeros. The first-boot marker uninitialized is not a machine id.

this_machine trims the string, converts it to lowercase ASCII, and accepts it only when all three checks pass:

  • the length is 32
  • every byte is an ASCII hex digit
  • at least one byte is not 0

Records::open and Records::existing both call this_machine. open returns before it creates the directory. The refusal is unsafe_records, which uses ErrorCode::LocalServerRecordsUnsafe. That enum serializes as local_server_records_unsafe.

The same rule refuses each case in records_refuse_a_machine_without_a_host_identifier:

  • None (the platform returns no id)
  • "" and whitespace
  • uninitialized
  • 32 zeros
  • 16 hex digits
  • a 32-character string that is not hex

A newline after a valid id is removed by trim, so the id is accepted. Uppercase A-F is accepted after the lowercase conversion. The guide does not say those forms refuse.

On Linux, host_id returns the first non-empty file. A present uninitialized in /etc/machine-id does not continue to the D-Bus file. The guide does not say that it continues.

Records::existing returns no records when the directory is missing, and it does not check the host id in that case. target list then keeps local_servers: []. When the directory exists and the id is not a machine id, existing returns the same error and the listing omits local_servers. The guide still says the field is absent when the directory cannot be read safely. The new sentence is the serve refusal.

The release note says a machine refuses to keep records when the identifier cannot be read, or is not 32 hexadecimal digits other than all zeros. That is the same set of values. The guide names the error code. The release note does not repeat the code.

Blocking

None.

Optional

None.

Rebase onto main

This pull request still conflicts with b8e82abf. A rebase or a merge of that commit is still required before it can land.

git merge-tree --write-tree b8e82abf 7175120b exits 1. The one content conflict is crates/omnigraph-cli/src/client.rs. The other changed files merge cleanly.

The conflict is the embedded branch create from #1016. Main's function is branch_create(start: ReadTarget). It keeps the value from db.branch_create_from_as(start, name, actor) and returns created.from and created.from_commit_id. This branch still calls branch_create_from_as(ReadTarget::branch(from), ...) and drops the value with engine_write(). In the merged function, from is not in scope. Keep main's line.

7175120b does not change client.rs. The resolution does not change serve, the record format, or the host-id check.

Stack

This docs commit does not change the stack notes from the review at 34802678, except where a head moved.

Tradeoffs and liability

The commit adds no code, no record field, and no new gate. The host id stays one predicate, and both record doors call it. The guide was the remaining copy that described only a missing file. That copy could tell an operator that uninitialized or an all-zero id is safe to share. Two machines with that file compare equal, and one can reap or reuse the other's token. The new sentences remove that reading. They add no runtime cost.

A further platform source should still normalize to 16 bytes inside host_id and then use this grammar. The docs now say the grammar, so a later source that passes a dashed MachineGuid through unchanged would refuse every host on that platform. The RFC's Windows row still needs that decision. This commit does not make the Windows refusal worse.

The check does not change publication, snapshots, or Lance. A cloned valid id can still match. That limit stays with the RFC.

Validation

Reviewed head: 7175120bad707548197b4ea2f286c8dad957d537. Previous head: 34802678db51f4cb721898656ef80987e3633743. Main: b8e82abfbaa02a0e14b65e3bb034d1a81f281864.

git diff --name-only 34802678..7175120b lists only the two documentation files. The Rust tree is the tree reviewed at 34802678. A .gqt case cannot express a process, a record file, or a host id. No new regression test is required, and I added none.

Local, on a detached worktree at the exact head:

python3 scripts/check-docs.py

Documentation OK (354 Markdown files). Exit 0. Dependencies were markdown-it-py==4.0.0 and mdurl==0.1.2. python3 -m venv could not create .venv-docs because python3-venv is not installed.

RUST_MIN_STACK=16777216 cargo test -p omnigraph-skill --locked --lib -- --test-threads=4

1 passed, 0 failed.

RUST_MIN_STACK=16777216 cargo test -p omnigraph-cli --locked --lib skill:: -- --test-threads=4

6 passed, 0 failed. 259 other library tests were filtered out. The test profile compiled in 16m 29s.

On this SHA, GitHub run 39367814597 (Fix Regression Gate) succeeded and run 39367814483 (PR Title) succeeded. The workspace tests and the GQT jobs did not run for this commit. They succeeded on 34802678 in runs 39359862315 and 39359862119. I did not rerun local_servers:: or cli_local_servers::, because this commit does not change them.

Limitations

I did not run macOS, so the gethostuuid path is from this source and from the prior review of the same function. I did not run Windows. I did not run the full workspace suite on this SHA. No performance claim.

@aaltshuler aaltshuler closed this Oct 20, 2026
ragnorc added a commit to ragnorc/omnigraph that referenced this pull request Oct 20, 2026
Brings in main through ModernRelay#1083, including branch creation at a commit,
the disposition, the actor-mismatch fix expectations and the machine id
grammar. `mcp_target_lost` joins the error-code registry as a CLI-only
code that asks the caller to fix the target: the bridge has already
resolved it again once.
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