Embed the corgea skill in the binary and expose it via corgea skill show - #152
Open
leenk7991 wants to merge 4 commits into
Open
Embed the corgea skill in the binary and expose it via corgea skill show#152leenk7991 wants to merge 4 commits into
leenk7991 wants to merge 4 commits into
Conversation
An agent reading the skill from a branch or a registry can be told about flags the installed CLI does not accept. Compiling skills/corgea/SKILL.md into the binary makes the reference pinned to the version being driven, by construction. corgea skill show prints it verbatim. It is dispatched without the token check that guards install, because reading a compiled-in string cannot need a login, and the point is that it still answers when the registry does not. install --local writes the same content, refusing a name other than corgea or an explicit version since neither is something the binary can honour. The registry path is unchanged and still serves company-authored skills. Co-authored-by: Cursor <cursoragent@cursor.com>
Config::load() creates ~/.corgea/config.toml and panics when it cannot, so `corgea skill show` exited 101 with a backtrace in a sandbox with a read-only home. That is the environment the command is most useful in, and printing a string compiled into the binary should not depend on the filesystem, so it is now dispatched before the config loads. Writing via write_all also lets `corgea skill show | head` exit quietly. Rust ignores SIGPIPE, so print! would have panicked once the skill grew past the pipe buffer. Also anchors the frontmatter test to the frontmatter block rather than matching anywhere in the file. Co-authored-by: Cursor <cursoragent@cursor.com>
4 tasks
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Automated review risk: 3/5.
The embedded display path is robust, but local installation still unnecessarily depends on a writable HOME. Process-level guarantees also lack integration coverage.
Critical or high-priority changes must be addressed.
Automatic approval was not submitted: automated review found critical or high-priority findings.
The early return added for `skill show` only covered that one command, so `skill install corgea --local --dir <writable-dir>` still reached the unconditional `Config::load()` and panicked creating ~/.corgea/config.toml. The embedded path needs neither a registry nor a token, so the local installer was unusable in the same sandbox `skill show` was fixed for. Replace the special case with one rule: `Config::load_or_defaults()` falls back to in-memory defaults, and `tolerates_unusable_home` decides which commands get it. Only the two that serve the compiled-in skill qualify; everything else still fails loudly, because it needs a token or somewhere to save one. The existing tests only read the embedded constant, so they survived every regression this PR guards against. Add binary-level tests that drive the real executable with no token under an unusable HOME and compare the bytes it prints and writes against skills/corgea/SKILL.md. Reverting the config guard fails four of them; removing the --local auth bypass fails five.
There was a problem hiding this comment.
Automated review risk: 3/5.
The offline commands are well tested for an uncreatable HOME, but config loading still panics on malformed configuration and mutates fresh homes. Both contradict the core no-config/no-persistence behavior.
Critical or high-priority changes must be addressed.
Automatic approval was not submitted: automated review found critical or high-priority findings.
`load_or_defaults` called `load()`, which creates ~/.corgea and writes config.toml as a side effect of resolving its path. So `skill show` began persisting state on a writable home -- a regression against the early return it replaced, and a contradiction of its own comment. It also inherited the `.expect()` on the TOML parse, so a config the user had merely mistyped took down the commands that exist to work when nothing else does. Read the file directly instead, through a `config_path_readonly` that creates nothing, falling back to the defaults when it is absent, unreadable or unparseable. `load()` now returns a parse failure as an `io::Error` naming the file rather than panicking, and `main` prints that and exits 1: a file the user can edit is their problem to fix, not a crash with a backtrace note. The tolerance is scoped to the embedded path. A command that needs the config still refuses to run on one it cannot parse.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Makes the CLI binary the single source of truth for its own agent skill, per Juan's design: the skills repo points at the CLI rather than keeping a second copy that drifts.
An agent reading the skill from a branch or a registry can be told about flags the installed CLI does not accept. Compiling
skills/corgea/SKILL.mdinto the binary makes the reference pinned to the version being driven, by construction.EMBEDDED_SKILLviainclude_str!, following the existing pattern insrc/vuln_api/mod.rscorgea skill showprints it verbatim to stdoutcorgea skill install corgea --localwrites the same content without touching the registryNeither path is behind the auth gate.
verify_token_and_exit_when_failstill guards the registry install, but reading a string compiled into the binary cannot need a login, and the point is that it still answers when the registry does not.showis dispatched beforeConfig::load()for the same reason — that call creates~/.corgea/config.tomland panics when it cannot, which made the command exit 101 in a read-only sandbox.--localrefuses any name other thancorgeaand refuses an explicit@version, since neither is something the binary can honour. The registry path is unchanged and still serves company-authored skills.This also removes the awkward coupling in the earlier plan:
tests/cli_deps_skill.rsandexamples/deps_skill.rshardcodeskills/corgea/SKILL.md, and because that file stays put and becomes the embedded artifact, the generated deps block stays authoritative with no cross-repo sync.Follow-up
Corgea/skillshas a stacked PR slimmingcorgea-scaninto a pointer atcorgea skill show. It should merge only after this ships in a release, or it points users at a command their binary does not have.Cargo.tomlis already at 1.10.0 against a latest tag of v1.9.3, so no version bump is needed.Test plan
cargo fmt --check,cargo clippy --all-targets -- -D warningscleancargo testsuite greencorgea skill showoutput is byte-identical toskills/corgea/SKILL.mdcorgea skill showexits 0 with an unwritable$HOME(previously panicked, exit 101)corgea skill show | headexits quietly rather than panicking onEPIPEinstall corgea --localwrites the identical file with no token setinstall some-other-skill --localandinstall corgea@1.2.3 --localboth exit 1 with a clear message--localstill hits the auth gate and the registryMade with Cursor
Linear ticket: Ship CLI-pinned Corgea skill and synchronize public skills/docs releases