Skip to content

feat(cli): omnigraph skill export ships the version-matched agent skill - #986

Closed
ragnorc wants to merge 12 commits into
ModernRelay:mainfrom
ragnorc:claude/cool-newton-c25ff0
Closed

ragnorc wants to merge 12 commits into
ModernRelay:mainfrom
ragnorc:claude/cool-newton-c25ff0

Conversation

@ragnorc

@ragnorc ragnorc commented Oct 13, 2026

Copy link
Copy Markdown
Contributor

Summary

A consumer of the omnigraph CLI (OmniGraph Studio's agent panel, CI scripts, a coding agent) can now ask the binary it chose for the agent skill written for it, instead of installing whatever skills/omnigraph holds on the default branch.

  • omnigraph skill export <DIR> [--force] [--json] writes the omnigraph skill (SKILL.md and references/) into DIR, creating it. --json reports the files written, skill_version (metadata.version), cli_version and http_contract (the existing omnigraph_api_types::HTTP_API_CONTRACT; omnigraph version is untouched, see CLI: omnigraph version --json reporting the HTTP contract and storage versions #977).
  • omnigraph skill show [PATH] prints SKILL.md, or any other skill file, to stdout.
  • Both are local commands: no graph, server, configuration or network.

Embedding: skills/omnigraph stays the one copy

include_str!("../../../skills/...") from the CLI would not survive cargo package, since a package carries only files below its root. So a new workspace crate, omnigraph-skill, has its package root at skills/, which puts the tree inside the package. Its build.rs walks skills/omnigraph and generates an include_bytes! table, so adding or removing a reference file needs no code change. The crate has no dependencies. cargo package -p omnigraph-skill lists all 14 skill files and its verify build compiles from the extracted package. npx skills add is unaffected: it copies skills/omnigraph/, and the crate's Cargo.toml, build.rs and src/ sit beside that directory, not inside it.

--force and stale files

An export writes .omnigraph-skill.json beside the skill: each written path with the SHA-256 of the bytes written. Without --force, any existing target path refuses the export before it writes anything. With --force, the skill's files are replaced (a symbolic link at a skill path is replaced, not followed). A file the previous record lists and this skill no longer has is removed only if it is still a regular file with the recorded digest; otherwise it is reported under kept (modified or not_a_file). A file the record never listed is never touched, and directories are never removed. A record this binary cannot read refuses the forced export up front: unknown format, another skill, a path that could leave the directory, or a malformed digest.

Drift tests

  • skill::tests::skill_command_index_lists_every_public_command walks the clap tree (Cli::command()). It requires the new ## Command Index table in references/commands.md to list exactly the public top-level commands and subcommands. A command added or removed without updating the skill fails with "Missing from the index" or "Listed but not commands".
  • skill::tests::skill_condensed_reference_names_every_top_level_command requires SKILL.md's condensed CLI reference to name every top-level command, and every entry there that opens with a command to open with a real one.
  • cli_schema_config::skill_export_writes_the_built_in_skill_byte_for_byte runs the binary and compares the exported tree with skills/omnigraph byte for byte. It also checks the JSON fields, the record's digests and skill show.
  • cli_schema_config::skill_export_refuses_existing_files_and_force_removes_only_unchanged_recorded_files covers the refusal and the --force rules above: an unchanged recorded file is removed; an edited one, a symbolic link (Unix) and an unrecorded file are kept; a record with an escaping path refuses.
  • scripts/check-docs.py already holds metadata.version to the CLI's package version. It now also holds the HTTP contract named in the skill's compatibility to HTTP_API_CONTRACT.

Other changes

  • Docs: docs/user/cli/index.md gets a short "Give an agent the matching skill" section; docs/user/cli/reference.md gets a command-table row. The README install line now leads with omnigraph skill export and keeps npx skills add for older CLIs. SKILL.md and references/commands.md mention the command.
  • docs/user/cli/reference.md sits at the checker's 350-line ceiling, so one sentence in it was joined onto one line to make room for the row.
  • Workspace ripple: Cargo.toml members, AGENTS.md's workspace map, docs/dev/architecture.md, the docs/dev/testing.md ownership table, .github/CODEOWNERS, and the (paused) crates.io publish order, with omnigraph-skill before omnigraph-cli.
  • Changelog: changelog.d/skill-export.added.md.

Test plan

  • RUST_MIN_STACK=16777216 cargo test -p omnigraph-cli --locked with the server binary built: 146 in-source (including skill:: and planes::) and 298 integration tests pass
  • The drift tests go red when the index loses repair, gains teleport or profile edit, or SKILL.md's condensed reference drops a command or describes a nonexistent one
  • Merging main brought in diff, branch diff and branch merge-base from feat(changes): graph diff, branch review and merge dry run #966; the drift tests failed with Missing from the index: ["branch diff", "branch merge-base", "diff"] until the skill listed them
  • cargo clippy --workspace --all-targets --locked -- -D warnings -W clippy::dbg_macro before merging main, the same for -p omnigraph-cli -p omnigraph-skill after; cargo fmt --all --check
  • cargo package -p omnigraph-skill (verify build from the extracted package)
  • python3 scripts/check-docs.py (the contract check fails on a skill naming 0.12 or no contract), bash scripts/check-agents-md.sh, python3 scripts/check-dependency-sources.py, python3 scripts/check-change-classes.py, cargo deny --locked check, typos

…kill

The binary now carries `skills/omnigraph` byte for byte. `omnigraph skill
export <dir> [--force] [--json]` writes it where an agent loads skills, and
`omnigraph skill show [<path>]` prints one file, so a consumer gets the skill
written for the CLI it runs.

A new `omnigraph-skill` crate has its package root at `skills/`, keeping the
skill tree its one copy while still packaging; its build script embeds every
file. An export records each written file's digest in `.omnigraph-skill.json`,
so a later `--force` export removes only unchanged files the skill no longer
has and never touches anything else.

In-source tests hold the skill's command index and condensed reference to the
clap tree; integration tests check byte identity and the `--force` rules;
check-docs.py now also holds the skill's declared HTTP contract to the CLI's.
…5ff0

# Conflicts:
#	crates/omnigraph-cli/src/planes.rs
#	docs/user/cli/reference.md
…5ff0

# Conflicts:
#	crates/omnigraph-cli/src/cli.rs
#	crates/omnigraph-cli/src/main.rs
#	crates/omnigraph-cli/src/planes.rs
#	docs/user/cli/reference.md
Comment thread skills/omnigraph/SKILL.md
Comment thread crates/omnigraph-cli/src/skill.rs
Comment thread crates/omnigraph-cli/src/skill.rs
Comment thread skills/Cargo.toml
@cursor

cursor Bot commented Oct 14, 2026

Copy link
Copy Markdown

Review of 343dc4b: request changes

Why. An agent that uses the omnigraph CLI needs instructions for the binary it actually runs, not for the default branch.
What. omnigraph skill export <DIR> writes the skill embedded in the binary. omnigraph skill show [PATH] prints one file. A new omnigraph-skill crate, rooted at skills/, embeds skills/omnigraph at build time.
How. build.rs generates an include_bytes! table. A .omnigraph-skill.json record lets a later --force export remove only unchanged files that it wrote earlier. In-source tests hold the command index and the condensed reference to the clap tree.
Impact. This is a local-plane command. It reads no configuration and uses no graph, server or network. It does not touch __manifest, snapshots, Lance, policy or the server trust boundary.

The design is good. One problem blocks the "version-matched" claim: the copy the binary exports gives the wrong storage formats for that binary.

Blocking

  1. The exported skill misstates this binary's storage formats, and no guard catches it (inline on SKILL.md 29–34 and skill.rs 476–478). SKILL.md says format 14 only, internal-schema 14 (serves v14 to v14) and --to-format 14. The binary reports internal-schema 16 (serves v14 to v16) and refuses --to-format 14 with unsupported_target. The text came before this PR, but this PR makes it the copy the binary ships. Please fix the lines, and add a guard that reads VersionOutput::current(), as you did for the version and the HTTP contract.

Merge prerequisite (not a code finding). GitHub reports a conflict with main. #988 changed the same lines of crates/omnigraph-cli/src/main.rs (module list) and Cargo.lock (CLI dependencies). Keeping both sides resolves it. I did this in a local scratch tree (not pushed), and the skill tests pass there (see Validation).

Not blocking

  1. Recommended: on a case-insensitive file system, a case-only rename between skill versions makes --force delete the file it has just written, and the export still succeeds (inline on skill.rs 168–181, reproduced). No current path triggers it, and the fix is small.
  2. Optional: add include to skills/Cargo.toml so the package carries only what it embeds (inline).
  3. Process: GOVERNANCE.md puts a new CLI surface and a new on-disk format (.omnigraph-skill.json, format: 1) in size L. The PR links no issue or RFC. Maintainers decide how that applies here.

Tradeoffs and liability

  • Source of truth. skills/omnigraph stays the only copy. The binary builds its copy from that directory, and drift tests keep the command index in step with clap. After five more changes like this one there is still one copy, so the liability is low.
  • Crate outside crates/. This is deliberate: a package can only carry files below its root. The cost is one layout exception, which AGENTS.md and docs/dev/architecture.md document. The trade is acceptable.
  • New on-disk record. format: 1 refuses unknown formats and ignores unknown fields. A record that names an escaping path, another skill or a bad digest is refused before any write. This format will live a long time, but it is small and its refusal rules are clear.
  • Every future CLI command must update the skill index. This cost is deliberate and keeps agents current. Expect it on open PRs: feat(cli): add native managed workflows and organization-scoped access #897 adds CLI surface, and Refresh skills and operator guides for 0.13 #892 rewrites references/commands.md (−357 lines; it also still says format 14). If either lands after this PR, the index guard fails until the index is restored. That is the intended effect.
  • Crash behavior. Files are written in place, and the record is written last. After an interrupted export, a rerun with --force converges. That is enough for a local tool.
  • Deprioritized: efficiency. About 180 KiB of text in the binary does not matter. Robustness is applied where it matters: the deletion path.

Future direction

  • feat(server): serve the skill references as MCP best-practices resources #995 stacks on this crate (the server reads omnigraph_skill::file). The small API (FILES, file, NAME, ENTRY) fits both consumers. If the server also needs metadata.version, move skill_version() into omnigraph-skill so that only one place parses the frontmatter.
  • Lance. This PR touches no Lance surface, and the Lance 11.0.0 pin does not change, so no domain in docs/dev/lance.md applies. A Lance or storage-stamp change reaches this feature only through the skill's storage-format text. That is the gap in finding 1.

Validation

Clean isolated worktree at 343dc4b. Current main is 1b514b4; the PR's last merge brought in e4b1dea.

Command Result
cargo build --locked -p omnigraph-server --bin omnigraph-server and cargo test --locked -p omnigraph-cli -p omnigraph-skill --no-run pass
RUST_MIN_STACK=16777216 cargo test --locked -p omnigraph-cli --bin omnigraph 146 passed, 6 ignored (includes 3 skill::tests)
cargo test --locked -p omnigraph-cli --test integration cli_schema_config:: 29 passed (includes both skill_export_*)
Mutation: delete the repair row from the command index, then rerun the in-source suite skill_command_index_lists_every_public_command fails with Missing from the index: ["repair"]; the skill crate re-embedded the edit without a clean build; file restored
cargo package -p omnigraph-skill --locked 20 files (14 skill files); verify build passes
cargo fmt --all --check; cargo clippy --locked -p omnigraph-skill --all-targets -- -D warnings -W clippy::dbg_macro clean
check-docs.py, check-agents-md.sh, check-dependency-sources.py, check-change-classes.py, check-workflow-action-pins.py pass
omnigraph version and omnigraph skill show internal-schema 16 (serves v14 to v16) vs. skill line 19 internal-schema 14 (serves v14 to v14)
omnigraph upgrade file:///tmp/g1 --check --to-format 14 --json on a fresh graph check_failed, unsupported_target
ciopfs case-insensitive mount, crafted record (see inline) references/commands.md deleted, exit 0; ext4 control removes nothing
Scratch merge with main 1b514b4, both conflict sides kept (not pushed): --bin omnigraph and cli_schema_config:: 148 passed (6 ignored) and 29 passed

Limitations. I did not run the full workspace test graph, workspace Clippy, cargo deny or typos locally. CI is green for all of them at this head. The case-insensitive check used ciopfs, not APFS or NTFS. I did not run on Windows.

…claude/cool-newton-c25ff0

# Conflicts:
#	Cargo.lock
#	crates/omnigraph-cli/src/main.rs
…claude/cool-newton-c25ff0

# Conflicts:
#	crates/omnigraph-cli/Cargo.toml
The exported skill now states the storage formats this binary reports, and
an in-source test holds that line to the value `omnigraph version` prints,
through one shared formatter. Elsewhere the skill refers to that line
instead of repeating the numbers.

A `--force` export no longer removes a recorded file that differs from a
current file only in ASCII case, which on a case-insensitive file system is
the file it just wrote. Skill paths and record paths must be ASCII. The
skill crate's package includes exactly what its build embeds. The command
index lists `cluster list` and `cluster pull`.
…5ff0

# Conflicts:
#	.github/workflows/publish-crates.yml
@ragnorc

ragnorc commented Oct 16, 2026

Copy link
Copy Markdown
Contributor Author

Closing as landed: #995 was squash-merged (4102cc85) with this branch's commits merged into it, at head 62a83e79. main now has the omnigraph-skill crate, omnigraph skill export/show, the binary-search file(), and the publish order with omnigraph-skill before omnigraph-server. git diff origin/main 62a83e79 -- skills crates/omnigraph-cli is empty.

@ragnorc ragnorc closed this Oct 16, 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.

1 participant