Repository navigation
Take the Engine and the Harness from runvil-harness - #213
Merged
Merged
Conversation
The Engine and the Harness have their own repository now, so this workspace no longer holds their source. The root `Cargo.toml` pins `runvil`, `harness-gateway-client`, `plugin-web`, and `plugin-mcp` from `https://github.com/cppalliance/runvil-harness` at one `rev`, and the crates under `crates/runvil`, `crates/runvil-engine`, `crates/runvil-plugin`, `crates/runvil-internal`, `crates/harness-internal`, `crates/harness-gateway-client`, `crates/plugin-web`, and `crates/plugin-mcp` are removed together with `prompts/`. `build-xtask`, `ci.yml`, and `docs-claims.mjs` lose the checks that read those crates. - `default-members` lists every remaining crate except the desktop app, the server it embeds, and the server's facade, so plain `cargo build` continues to work with no Node install. - `build-xtask` loses `cargo xtask api` and the modules `engine_guards`, `engine_deps`, `engine_clippy`, `facade_shape`, `harness_bans`, `doc_hidden`, and `test_support_leak`. The product matrix keeps the desktop-app boundary and container privacy. - `ci.yml` loses the `api-surface` and `msrv` jobs, and `rust-version` leaves `[workspace.package]` because no crate here inherits it. - `[workspace.dependencies]` loses the keys that no remaining crate names. `Cargo.lock` loses `criterion`, `rustdoc-types`, and their dependencies, and no registry package changes version. - `deny.toml` adds the repository to `allow-git`. - `cargo xtask site` continues to document the crates in `RUSTDOC_SITES`, now at the pinned commit. - No check in this commit stops a member from naming a different crate of that repository.
The crates from runvil-harness are git dependencies, which the product matrix cannot see, so no check stopped a crate here from naming a private crate of the Engine or the Harness. The new `harness_pin` module reads the root `[workspace.dependencies]` and each member manifest. `all_violations` runs it, so `cargo test -p build-xtask` and `cargo xtask tidy` report a breach. - The check has four rules. An entry from the repository names a crate in `HARNESS_PUBLIC`. Each entry has a `rev`; a `branch` or a `tag` does not count. All entries name one `rev`. No member names the repository in its own manifest, in any dependency table. - `CrateInfo` gets a `manifest` field and `DEP_KINDS` becomes `pub(crate)`, so the check uses the crate walk in `product` and `dependency_tables`. - The URL of the repository matches with or without a trailing `/` or `.git`, in any ASCII case. - A root manifest that cannot be read or parsed is a violation. - Ten tests: one runs the check on this workspace, and the fixtures break each rule in turn. - The check compares `rev` strings exactly, so a short hash and a full hash of one commit count as two commits. It does not read patch sections.
|
Important Review skippedToo many files! This PR contains 773 files, which is 623 over the limit of 150. To get a review, reduce the PR to 150 files or fewer by splitting it into smaller PRs or changing its base branch. Upgrade to Team to raise the limit. Usage-priced reviews support at most 300 files.
 |
 [Medium impact] The PR changes how Workshop builds and updates its Engine and Harness dependencies. The PR appears safe to merge, with a non-blocking gap in the new pin check. Findings
Fix with agent prompt### Issue 1
crates/build-xtask/src/harness_pin.rs:89-91
`harness_pin` accepts any string in `rev`, so setting all four entries to `rev = "master"` passes. Git branch and tag names are valid revision values, but they can move. A later lockfile update could therefore change the Engine and Harness without a manifest change.
Require a full commit hash and add a fixture that rejects a branch name supplied through `rev`.
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.
|
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
Comment on lines
+89
to
+91
| match entry.get("rev").and_then(toml::Value::as_str) { | ||
| Some(rev) => { | ||
| revs.insert(rev); |
There was a problem hiding this comment.
harness_pin accepts any string in rev, so setting all four entries to rev = "master" passes. Git branch and tag names are valid revision values, but they can move. A later lockfile update could therefore change the Engine and Harness without a manifest change.
Require a full commit hash and add a fixture that rejects a branch name supplied through rev.
Prompt To Fix With AI
This is a comment left during a code review.
Path: crates/build-xtask/src/harness_pin.rs
Line: 89-91
Comment:
**Moving revisions pass the check**
`harness_pin` accepts any string in `rev`, so setting all four entries to `rev = "master"` passes. Git branch and tag names are valid revision values, but they can move. A later lockfile update could therefore change the Engine and Harness without a manifest change.
Require a full commit hash and add a fixture that rejects a branch name supplied through `rev`.
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.
vinniefalco
temporarily deployed
to
github-pages
November 12, 2026 19:07 — with 
GitHub Actions
Inactive
GitHub Actions
Inactive
This branch was previously deployed
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
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.
The Engine and the Harness now live in cppalliance/runvil-harness. This pull request removes their source from this repository and pins them from there instead. What stays here is Workshop and the tooling that builds its installers.
What changed
Cargo.tomltakes four crates fromrunvil-harnessat commit73242996:runvil,harness-gateway-client,plugin-web, andplugin-mcp. Those are the only four crates Workshop named before, so no Workshop source file changes.crates/runvil,crates/runvil-engine,crates/runvil-plugin,crates/runvil-internal,crates/harness-internal,crates/harness-gateway-client,crates/plugin-web, andcrates/plugin-mcpare gone, along withprompts/. Their history is inrunvil-harness: the last commit here that touched them,c64f85634, is93d2871athere.build-xtask. The checks that read Engine and Harness source are removed (cargo xtask api, the Engine guards, the facade shape check, the Harness bans, thedoc(hidden)ban, thetest-supportleak guard). They run inrunvil-harnessnow. The Workshop checks stay.harness_pin(second commit). Git dependencies are invisible to the product matrix, so nothing would have stopped a crate here from naming a private Engine or Harness crate. The check reads the manifests and requires that only the four public crates come fromrunvil-harness, that each has arev, that all name onerev, and that no member names the repository directly. It runs incargo test -p build-xtaskandcargo xtask tidy.api-surfaceandmsrvjobs and the three Engine and facade doc steps are removed.runvil-harnesshas its own copies of all of them.ci-greenno longer needs the two jobs.[workspace.dependencies]keys that no remaining crate uses are removed, as isrust-version, which only the Engine crates inherited.Cargo.locklosescriterion,rustdoc-typesand their dependencies, and no registry package changes version.deny.tomlallows the new git source.AGENTS.md, both READMEs,vibelight.toml, and the wording scan are updated to match.Things worth a look
default-members. It was a short list of Engine crates. It is now every crate except the desktop app,workshop-server, andworkshop-server-api, which keeps the old promise that plaincargo buildworks on a fresh clone with no Node install and no Tauri system packages. This is a judgment call, so say so if you would rather have a different default.revon the four lines in the rootCargo.toml.harness_pinfails if they drift apart. Therunvil-plugin-sdkpin must keep naming the Plugin contract commit thatrunvil-harnessuses, becauserunvil-plugin-syssetslinksand Cargo refuses two commits of it in one build.runvil-harnessnow.cargo xtask sitestill documents the same five crates. It builds them from the pinned commit, so the published API docs follow the pin and not the tip ofrunvil-harness.Cargo.toml,Cargo.lock, andsite.rschanged.Verification
Run locally on Windows, all passing:
cargo fmt --all --checkcargo clippyon both partitions, withCARGO_BUILD_WARNINGS=denycargo nextest run --lockedon both partitions: 795 passed and 347 passed. The 5 skipped tests are thevalidated_gateway_fixture_processhelpers, which are ignored on purpose.cargo docon the workspace partition withRUSTDOCFLAGS="-D warnings", andworkshop-serverwith private itemscargo dylint --allcargo xtask tidy, andcargo nextest run --locked -p build-xtask(120 passed). The first commit was also checked on its own (110 passed).node --test crates/workshop/ui/test/docs-claims.mjscargo deny checkandcargo auditringcheck from thesupply-chainjob:ringis still absent from the Workshop binarycargo doc -p <crate> --no-depsfor each of the five docs-site crates, to confirm they still build as git packagesNot run locally: the Linux jobs, the
uijob's npm tests, a fullcargo xtask sitebuild with the books, and the installer builds. CI covers those.