feat(734): plugin loader skeleton for PEP 1.7.0#758
Conversation
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
|
Warning Rate limit exceeded
You’ve run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
WalkthroughThis PR implements PEP 1.7.0 plugin loading and registry management through a new ChangesPlugin Loader System
Self-Contract Attestations and Protocol Catalog Updates
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related issues
Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Implements §3 (file interface), §4 (JSON-RPC stdio interface), §6 (content-addressing), §7 (CLI flag conventions), §8 (error model), §9 (registry semantics) of the plugin-protocol spec (PR #740). The runtime engine that makes sugar dicts, loss functions, and future plugin kinds loadable as content-addressed JSON files or JSON-RPC sidecars. New crate: provekit-plugin-loader - PluginMemento + PluginEnvelope + PluginHeader + PluginMetadata (§1) - PluginLoadFailureMemento + FailureReasonKind (§8.1) - load_plugin_from_file (§3): parse, shape-validate, CID verify - load_plugin_from_rpc (§4): stdio spawn + JSON-RPC describe dispatch - compute_plugin_cid (§6.1): JCS over header-without-cid, blake3-512 - compute_failure_cid (§8.3): JCS over failure header, blake3-512 - PluginRegistry: BTreeMap<(kind, cid)>; register, lookup, by_kind - PluginRegistryMemento + compute_registry_cid (§9.1 + §9.3) - mint_failure_memento convenience function - stub_rpc_server binary: minimal stdio JSON-RPC sidecar for tests CLI plumbing (provekit-cli): - cmd_plugin.rs: PluginFlags struct + build_registry function - Flags: --plugin, --sugar, --loss-fn, --lifter, --no-default-plugins, --no-default-plugin, --strict-plugins, --plugin-registry-out - BindArgs gains #[command(flatten)] pub plugins: PluginFlags - run() seals registry before pipeline work; logs CID prefix 24 tests (16 unit + 8 integration): - Pinned CID round-trip for file fixture (byte-stability) - CID-delivery-independence for file vs RPC paths - Registry (kind, cid) lookup, deduplication, by_kind, emit_memento - PluginLoadFailureMemento for missing file (deterministic CID) - stdio RPC load via stub_rpc_server binary No concrete plugins in this PR. Spring sugar (#735), comment sugar (#736), JUnit sugar (#737), default loss-function (#738), and demo (#739) follow as separate PRs. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
TSavo
force-pushed
the
feat/734-plugin-loader-skeleton
branch
from
bcc6a51 to
8c665c2
Compare
The plugin loader skeleton (PR #758) added a new crate and modified implementations/rust/Cargo.toml + provekit-cli/Cargo.toml + cmd_bind.rs + main.rs. These source-byte changes propagated into spec-CID recomputation via the catalog-verify tool. This commit re-runs recompute-spec-cids --write to update the stored CIDs that reflect the new source bytes. No semantic content changes; only CID values updated to track the actual source bytes. Two CIDs updated: - agent-plugin-protocol - lift-plugin-protocol Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
|
Catalog-verify gate fixed. Commit 86583b0 pushed to this branch. The
No semantic content changes -- keys, structure, and all other 26 spec CID values are unchanged. |
Review: REQUEST CHANGESWorktree built clean ( Blockers1. §9.1 "load_order": ["blake3-512:zzzz","blake3-512:aaaa","blake3-512:mmmm"]The 2. §9.1 3. §3.1 alias Plain breach of the §3.1 alias table. The remediation is one line: add 4. §3.2 flag-order preservation violated (MEDIUM-HIGH). Non-blocking but must-fix before #735-#738 land
What's solid
Path to mergeFix #1, #2, #3, #4 above; convert the — Reviewed at commit 8c665c2. |
…eview
Blockers:
B1 §9.1 load_order wire shape: Vec<String> -> Vec<{kind,cid,source}>;
loaded also fixed from Vec<String> to Vec<{kind,cid}> per §9.1 CDDL.
Registry CIDs now byte-compatible with any spec-conformant loader.
Added LoadOrderEntry + LoadedEntry types in types.rs; updated
compute_registry_cid to hash the new object-array shapes.
B2 §9.1 loaded ordering: sorted by cid ascending per spec; same in
§9.3 CID input. PluginRegistry::emit_registry_memento now sorts
loaded entries. Added loaded_is_sorted_by_cid test.
B3 §3.1 --loss-function alias: added as spec-canonical name (primary),
--loss-fn retained as ergonomic short alias. Implemented via manual
clap::Args + FromArgMatches replacing the #[derive(Parser)] that
could not support B4.
B4 §3.2 flag-order preservation: manual FromArgMatches uses
ArgMatches::indices_of to recover true interleaved argv order for
--plugin / --sugar / --loss-function / --lifter. Eliminates the
two-pass ordering bug. PluginFlags.ordered field records the
correct insertion-order sequence for build_registry.
Nits:
N1 §6.2 delivery-independence test: stub RPC server now emits
byte-identical JCS payload to tests/fixtures/dummy-sugar.json.
rpc_stdio_registry_cid_matches_file_load_when_payload_identical
now asserts CID(file) == CID(rpc) == pinned value.
N2 --no-default-plugins / --no-default-plugin <kind>: both flags are
parsed and consulted in build_registry. v0 ships zero default
plugins so flags are no-ops; the wire surface is preserved for
when defaults arrive.
N3 sealed_at: use chrono::Utc::now() for real ISO8601 timestamp in
cmd_bind.rs. Added chrono 0.4 dep to provekit-cli Cargo.toml.
All 25/25 tests pass (17 unit + 8 integration). cargo check workspace
clean. catalog-verify exit 0 (spec files untouched).
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
|
fix: four spec-correctness blockers + three nits addressed at b673fac. Ready for re-review. B1 §9.1 load_order wire shape: B2 §9.1 loaded ordering: sorted by cid ascending; same in §9.3 CID input. New B3 §3.1 B4 §3.2 flag-order preservation: manual N1 §6.2 delivery-independence: stub RPC server now emits byte-identical payload to N2 N3 All 25/25 tests pass. |
Re-review verdict: MERGE-WITH-NITSRe-reviewed at b673fac. All four blockers and all three nits land correctly. The substance is right — wire shape matches §9.1, JCS ordering is right, B4's Verified
Nits (cleanup, NOT merge-blocking)N4: B4 has no automated regression test. The most substantive piece of this fix — interleaved argv-order recovery — ships with zero pinned coverage. The 25/25 includes nothing that asserts the interleaving invariant; only this re-review exercised it. Suggested paste-in for #[cfg(test)]
mod b4_tests {
use super::*;
use clap::{ArgMatches, Command, FromArgMatches};
fn parse(argv: &[&str]) -> PluginFlags {
let cmd = PluginFlags::augment_args(Command::new("probe"));
let m: ArgMatches = cmd.try_get_matches_from(argv).unwrap();
PluginFlags::from_arg_matches(&m).unwrap()
}
#[test]
fn b3_loss_function_primary_name_accepted() {
let f = parse(&["probe", "--loss-function", "foo.json"]);
assert_eq!(f.loss_fn, vec!["foo.json".to_string()]);
}
#[test]
fn b3_loss_fn_alias_still_accepted() {
let f = parse(&["probe", "--loss-fn", "foo.json"]);
assert_eq!(f.loss_fn, vec!["foo.json".to_string()]);
}
#[test]
fn b4_interleaved_order_preserved() {
let f = parse(&[
"probe",
"--plugin", "sugar:a.json",
"--sugar", "b.json",
"--plugin", "sugar:c.json",
"--loss-function", "d.json",
"--plugin", "loss-function:e.json",
]);
let kinds: Vec<&str> = f.ordered.iter().map(|(k,_,_)| k.as_str()).collect();
let sources: Vec<&str> = f.ordered.iter().map(|(_,s,_)| s.as_str()).collect();
assert_eq!(kinds, vec!["sugar","sugar","sugar","loss-function","loss-function"]);
assert_eq!(sources, vec!["a.json","b.json","c.json","d.json","e.json"]);
}
#[test]
fn b4_lifter_uses_wire_kind_lift() {
let f = parse(&["probe", "--lifter", "x.json"]);
assert_eq!(f.ordered.len(), 1);
assert_eq!(f.ordered[0].0, "lift");
}
}Requires making N5: dead-code branch. N6: stray hardcoded timestamp. VerdictShip it. The B4 mechanics are correct under empirical probing; the missing test is a real gap but a follow-up tightening, not a merge blocker — the spec compliance and wire shape are right. N5 and N6 are pre-existing scope items. Re-reviewed in worktree 🤖 Generated with Claude Code |
The blockers fix at b673fac changed the plugin loader wire shape per spec §9.1 (LoadOrderEntry/LoadedEntry). That CID shift propagated into the contract set referenced by ACCEPTED_LIFT_PLUGIN_PROTOCOL_CONTRACT_SET_CID in provekit-self-contracts, which cross-kit-conformance pins. Regenerated CID: updated to match the actual contract set bytes post-blockers-fix. Cargo.lock churn is mechanical (cargo rebuild). No semantic drift.
…e Maven probe provekit-ir is local-source-only and is not published to Maven Central. java_classpath() was running `mvn package` which writes to target/ only; the subsequent `dependency:build-classpath` on provekit-claim-envelope then tried to resolve provekit-ir as a declared dep and hit Maven Central, where it is not published, producing a cached-resolution failure on CI. Fix: change `package` to `install` so provekit-ir lands in ~/.m2 before the classpath probe runs. Matches the precedent at Makefile:216 (build-java uses `mvn install -am` for exactly this reason). Also refresh self-contract attestations for csharp/go/java/ruby/rust to reflect the kit contract-set CIDs that are live in this branch. Verified: make cross-kit-conformance exits 0 with a clean ~/.m2/com/provekit. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
|
fix(357cdc3): |
|
cross-kit-conformance fixtures are clean. Both blockers already landed on this branch:
Local verification: |
…dditions #758 added new self-contracts (c1_holds_on_legacy_protocol_version_in_ migration_window, etc.). The pinned contractSetCid values in mint_kit_integration.rs referenced the pre-additions contract set and diverged. Updated pinned values to match the actual current contract set bytes. No semantic content changes; only the test pins reflect the substrate's current state.
|
fix: regenerated pinned kit contractSetCids for the plugin-loader's new self-contracts. CCG should re-run green. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
implementations/rust/provekit-plugin-loader/src/loader.rs (1)
98-104: ⚖️ Poor tradeoffConsider documenting command-line parsing limitations.
The
stdio:command parsing at lines 98-104 usessplitn(2, ' ')followed bysplit_whitespace(). This approach won't handle quoted arguments, escaped spaces, or shell special characters. For example,stdio:my-plugin --arg "value with spaces"would split incorrectly.This is acceptable for the current test-focused implementation, but worth documenting as a known limitation if
stdio:sources are intended for production use.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@implementations/rust/provekit-plugin-loader/src/loader.rs` around lines 98 - 104, Document that the command-line parsing in the stdio loader is simplistic: the code that splits cmd_str using splitn(2, ' ') and then split_whitespace() (variables/expressions: cmd_str, parts, bin, rest_args, args, splitn, split_whitespace) does not support quoted arguments, escaped spaces, or shell metacharacters, and therefore will misparse inputs like `stdio:my-plugin --arg "value with spaces"`; add a concise note in the loader.rs comments (near the parsing block) describing this limitation and recommending a more robust parser (e.g., a shell-like tokenizer) if stdio sources are ever used in production.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@implementations/rust/provekit-cli/src/cmd_plugin.rs`:
- Around line 358-367: The block that serializes and writes the registry uses
serde_json::to_string_pretty(&memento).expect(...) and logs write failures
instead of returning them; change this to propagate errors back to the caller:
replace the .expect(...) with using the ? operator (or map_err/Context to attach
a descriptive message) when calling serde_json::to_string_pretty(&memento) and
likewise change the std::fs::write(out_path, json).unwrap_or_else(...) to a ?
(or map_err/Context) so the function returns an Err on failure (include
descriptive context like "failed to serialize plugin registry" and "failed to
write plugin registry to {out_path}"). Ensure the function signature returns a
Result so these ?-propagated errors compile and bubble up to the caller instead
of panicking or only logging.
In `@implementations/rust/provekit-plugin-loader/src/cid.rs`:
- Around line 71-76: The serde_json::Value::Number branch currently coerce
non-i64 numbers to strings (Value::string(n.to_string())), which silently
changes JSON types; instead make the conversion explicit-failing: in the
function that converts serde_json::Value to the canonicalizer Value (the branch
handling serde_json::Value::Number), keep Value::integer(i) for n.as_i64() but
remove the Value::string fallback and return a validation error for any number
that is not representable as i64 (e.g., Err(Error::new("unsupported JSON number:
not i64"))). Update the conversion function signature to return Result<...>
(e.g., Result<Value, ValidationError>) and propagate/handle that error at the
callers so unsupported numeric forms are rejected at validation time rather than
coerced into strings.
In `@implementations/rust/provekit-plugin-loader/src/loader.rs`:
- Around line 97-125: In load_plugin_from_stdio_rpc replace the unconditional
panic from serde_json::to_vec(...).expect("request serialization") with proper
error handling: call serde_json::to_vec(&request) and map any Err into a
LoadError::RpcError (include a descriptive detail string mentioning the
serialization error and the original error), returning that Err early; then
proceed using the produced request_bytes. This change should reference the
serde_json::to_vec call, the request_bytes variable, and return
LoadError::RpcError on failure.
---
Nitpick comments:
In `@implementations/rust/provekit-plugin-loader/src/loader.rs`:
- Around line 98-104: Document that the command-line parsing in the stdio loader
is simplistic: the code that splits cmd_str using splitn(2, ' ') and then
split_whitespace() (variables/expressions: cmd_str, parts, bin, rest_args, args,
splitn, split_whitespace) does not support quoted arguments, escaped spaces, or
shell metacharacters, and therefore will misparse inputs like `stdio:my-plugin
--arg "value with spaces"`; add a concise note in the loader.rs comments (near
the parsing block) describing this limitation and recommending a more robust
parser (e.g., a shell-like tokenizer) if stdio sources are ever used in
production.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 86a8179a-37b1-48f3-8e4c-90eeabc33b23
⛔ Files ignored due to path filters (2)
implementations/rust/Cargo.lockis excluded by!**/*.locktools/cross-kit-conformance/Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (24)
.provekit/self-contracts-attestations/csharp.json.provekit/self-contracts-attestations/go.json.provekit/self-contracts-attestations/java.json.provekit/self-contracts-attestations/ruby.json.provekit/self-contracts-attestations/rust.jsonimplementations/rust/Cargo.tomlimplementations/rust/provekit-cli/Cargo.tomlimplementations/rust/provekit-cli/src/cmd_bind.rsimplementations/rust/provekit-cli/src/cmd_plugin.rsimplementations/rust/provekit-cli/src/main.rsimplementations/rust/provekit-cli/tests/mint_kit_integration.rsimplementations/rust/provekit-plugin-loader/Cargo.tomlimplementations/rust/provekit-plugin-loader/src/bin/stub_rpc_server.rsimplementations/rust/provekit-plugin-loader/src/cid.rsimplementations/rust/provekit-plugin-loader/src/error.rsimplementations/rust/provekit-plugin-loader/src/lib.rsimplementations/rust/provekit-plugin-loader/src/loader.rsimplementations/rust/provekit-plugin-loader/src/registry.rsimplementations/rust/provekit-plugin-loader/src/types.rsimplementations/rust/provekit-plugin-loader/tests/fixtures/dummy-sugar.jsonimplementations/rust/provekit-plugin-loader/tests/integration.rsimplementations/rust/provekit-self-contracts/src/lib.rsprotocol/specs/2026-04-30-protocol-catalog.jsontools/cross-kit-conformance/src/lib.rs
| if let Some(ref out_path) = self.plugin_registry_out { | ||
| let json = | ||
| serde_json::to_string_pretty(&memento).expect("registry memento serialization"); | ||
| std::fs::write(out_path, json).unwrap_or_else(|e| { | ||
| eprintln!( | ||
| "plugin-loader: could not write registry to {}: {e}", | ||
| out_path.display() | ||
| ); | ||
| }); | ||
| } |
There was a problem hiding this comment.
Consider propagating registry write errors.
The serde_json::to_string_pretty call at line 360 uses .expect(), which will panic if serialization fails. While unlikely for well-formed data structures, defensive error handling would be safer.
Additionally, the file write failure at line 361 is logged but not propagated to the caller. If the user explicitly requested --plugin-registry-out, they likely expect either the file to be written or an error returned.
🛡️ Proposed fix to propagate write errors
// Write registry memento to disk if requested (§7).
if let Some(ref out_path) = self.plugin_registry_out {
- let json =
- serde_json::to_string_pretty(&memento).expect("registry memento serialization");
- std::fs::write(out_path, json).unwrap_or_else(|e| {
- eprintln!(
- "plugin-loader: could not write registry to {}: {e}",
- out_path.display()
- );
- });
+ let json = serde_json::to_string_pretty(&memento).map_err(|e| {
+ PluginLoadRefusal {
+ failure: mint_failure_memento(
+ "--plugin-registry-out",
+ "registry",
+ &LoadError::ValidationError {
+ detail: format!("registry serialization failed: {e}"),
+ },
+ sealed_at,
+ ),
+ source: format!("--plugin-registry-out={}", out_path.display()),
+ }
+ })?;
+ std::fs::write(out_path, json).map_err(|e| {
+ PluginLoadRefusal {
+ failure: mint_failure_memento(
+ "--plugin-registry-out",
+ "registry",
+ &LoadError::ValidationError {
+ detail: format!("failed to write {}: {e}", out_path.display()),
+ },
+ sealed_at,
+ ),
+ source: format!("--plugin-registry-out={}", out_path.display()),
+ }
+ })?;
}🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@implementations/rust/provekit-cli/src/cmd_plugin.rs` around lines 358 - 367,
The block that serializes and writes the registry uses
serde_json::to_string_pretty(&memento).expect(...) and logs write failures
instead of returning them; change this to propagate errors back to the caller:
replace the .expect(...) with using the ? operator (or map_err/Context to attach
a descriptive message) when calling serde_json::to_string_pretty(&memento) and
likewise change the std::fs::write(out_path, json).unwrap_or_else(...) to a ?
(or map_err/Context) so the function returns an Err on failure (include
descriptive context like "failed to serialize plugin registry" and "failed to
write plugin registry to {out_path}"). Ensure the function signature returns a
Result so these ?-propagated errors compile and bubble up to the caller instead
of panicking or only logging.
| serde_json::Value::Number(n) => { | ||
| if let Some(i) = n.as_i64() { | ||
| Value::integer(i) | ||
| } else { | ||
| Value::string(n.to_string()) | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== Locate canonicalizer Value definition and numeric constructors =="
rg -n --type=rust -C3 'enum\s+Value|fn\s+integer\s*\(|fn\s+number\s*\(|float|double|u64|i128' implementations/rust/provekit-canonicalizer
echo
echo "== Locate current number coercion in plugin loader CID code =="
rg -n --type=rust -C3 'serde_json::Value::Number|as_i64\(|to_string\(\)|Value::string' implementations/rust/provekit-plugin-loader/src/cid.rsRepository: TSavo/provekit
Length of output: 12098
Avoid coercing JSON numbers into strings during CID input construction.
Line 75 silently converts non-i64 numbers to strings, changing JSON value type and affecting CID computation. The canonicalizer's Value type supports only i64 integers, not arbitrary precision or floating-point numbers. Rather than type-coercing unsupported numeric forms to strings, reject them explicitly at validation time so mismatched CIDs surface as errors instead of propagating silently.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@implementations/rust/provekit-plugin-loader/src/cid.rs` around lines 71 - 76,
The serde_json::Value::Number branch currently coerce non-i64 numbers to strings
(Value::string(n.to_string())), which silently changes JSON types; instead make
the conversion explicit-failing: in the function that converts serde_json::Value
to the canonicalizer Value (the branch handling serde_json::Value::Number), keep
Value::integer(i) for n.as_i64() but remove the Value::string fallback and
return a validation error for any number that is not representable as i64 (e.g.,
Err(Error::new("unsupported JSON number: not i64"))). Update the conversion
function signature to return Result<...> (e.g., Result<Value, ValidationError>)
and propagate/handle that error at the callers so unsupported numeric forms are
rejected at validation time rather than coerced into strings.
| fn load_plugin_from_stdio_rpc(cmd_str: &str) -> Result<PluginMemento, LoadError> { | ||
| let parts: Vec<&str> = cmd_str.splitn(2, ' ').collect(); | ||
| let (bin, rest_args) = (parts[0], parts.get(1).copied().unwrap_or("")); | ||
| let args: Vec<&str> = if rest_args.is_empty() { | ||
| vec![] | ||
| } else { | ||
| rest_args.split_whitespace().collect() | ||
| }; | ||
|
|
||
| let mut child = Command::new(bin) | ||
| .args(&args) | ||
| .stdin(Stdio::piped()) | ||
| .stdout(Stdio::piped()) | ||
| .stderr(Stdio::null()) | ||
| .spawn() | ||
| .map_err(|e| LoadError::RpcError { | ||
| detail: format!("failed to spawn stdio plugin `{bin}`: {e}"), | ||
| })?; | ||
|
|
||
| // Build request (§4.2.1). | ||
| let request = serde_json::json!({ | ||
| "jsonrpc": "2.0", | ||
| "id": 1, | ||
| "method": "provekit.plugin.describe", | ||
| "params": { | ||
| "runtime_protocol_versions": RUNTIME_PROTOCOL_VERSIONS | ||
| } | ||
| }); | ||
| let request_bytes = serde_json::to_vec(&request).expect("request serialization"); |
There was a problem hiding this comment.
Replace .expect() with proper error handling.
Line 125 uses .expect("request serialization") which will panic if serde_json::to_vec fails. While serialization of a json! macro result should never fail, defensive error handling is safer.
🛡️ Proposed fix
let request = serde_json::json!({
"jsonrpc": "2.0",
"id": 1,
"method": "provekit.plugin.describe",
"params": {
"runtime_protocol_versions": RUNTIME_PROTOCOL_VERSIONS
}
});
- let request_bytes = serde_json::to_vec(&request).expect("request serialization");
+ let request_bytes = serde_json::to_vec(&request).map_err(|e| LoadError::RpcError {
+ detail: format!("failed to serialize JSON-RPC request: {e}"),
+ })?;🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@implementations/rust/provekit-plugin-loader/src/loader.rs` around lines 97 -
125, In load_plugin_from_stdio_rpc replace the unconditional panic from
serde_json::to_vec(...).expect("request serialization") with proper error
handling: call serde_json::to_vec(&request) and map any Err into a
LoadError::RpcError (include a descriptive detail string mentioning the
serialization error and the original error), returning that Err early; then
proceed using the produced request_bytes. This change should reference the
serde_json::to_vec call, the request_bytes variable, and return
LoadError::RpcError on failure.
…rent value The prior pin-fix updated GO_CONTRACT_SET_CID but skipped the rust pin because the rust mint-self-contracts binary hits a skip path on macOS, making the local test appear to pass. On Linux CI it actually runs and catches the drift. Updated to the CID emitted by Linux CI (from CCG run 25764503850 failure message). Both Linux and macOS pins updated to the same value per the existing 'macOS currently emits the same' comment. Old: blake3-512:eb9979cc... New: blake3-512:3b41145b... No semantic change; tracking actual current contract set bytes.
2 participants
Closes #734. Part of the #732 plugin-protocol umbrella.
Summary
Runtime engine for PEP 1.7.0. Implements the spec at
protocol/specs/2026-05-12-plugin-protocol.md(PR #740). No concrete plugin implementations ship in this PR -- only the loader infrastructure.New crate:
provekit-plugin-loaderSpec sections implemented:
PluginMemento,PluginEnvelope,PluginHeader,PluginMetadatawire types (CDDL-matching)load_plugin_from_file: parse JSON, shape-validate, protocol-version check, CID verifyload_plugin_from_rpc: stdio spawn +provekit.plugin.describedispatch; HTTP/TCP stubs withrpc-errorcompute_plugin_cid: JCS over header-without-cid, BLAKE3-512.protocol_versionssorted ascending before JCS. §6.2 delivery-independence invariant tested.PluginLoadFailureMemento,FailureReasonKind(all canonical labels),mint_failure_memento,compute_failure_cidPluginRegistry:BTreeMap<(kind, cid), PluginMemento>,register,lookup,by_kind,register_builtin,record_failure,emit_registry_memento.PluginRegistryMemento+compute_registry_cid. Built-ins appended AFTER user flags per §7.Fixture:
tests/fixtures/dummy-sugar.json--kind = "test:dummy"open-enum label, pinned CIDblake3-512:ad148c5f.... The CID is verified on everycargo testrun byfile_load_valid_fixture.CLI plumbing (
provekit-cli)New
cmd_plugin.rsmodule +PluginFlagsstruct:--plugin <kind>:<source>--sugar <source>--plugin sugar:<source>--loss-fn <source>--plugin loss-function:<source>--lifter <source>--plugin lift:<source>(wire kind ="lift", §2.1)--no-default-plugins--no-default-plugin <kind>--strict-plugins--plugin-registry-out <path>PluginRegistryMementoJSONBindArgsgains#[command(flatten)] pub plugins: PluginFlags. Therun()function seals the registry before pipeline work and logs the registry CID prefix. The_registry_cidvariable is plumbed for §9.4 provenance citation by downstream pipeline steps.Tests
16 unit (in
src/):cid::tests::cid_elides_cid_field-- CID input never includes thecidfieldcid::tests::protocol_versions_sort_is_applied-- unsorted vs sorted produce same CIDcid::tests::failure_cid_is_deterministic-- §8.3 stabilityregistry::tests::register_and_lookup-- (kind, cid) round-tripregistry::tests::lookup_miss_returns_noneregistry::tests::by_kind_returns_all_of_kindregistry::tests::duplicate_cid_deduplication-- §9.2registry::tests::emit_registry_memento_round_trip-- §9.1registry::tests::registry_cid_is_stable_across_calls-- §9.3 byte-stabilityregistry::tests::failure_memento_mintingregistry::tests::builtin_count_tracks_register_builtin-- §7 built-in orderingloader::tests::load_file_not_foundloader::tests::load_file_parse_error_on_bad_jsonloader::tests::load_file_missing_header_fieldloader::tests::rpc_http_stub_returns_rpc_errorloader::tests::rpc_unknown_scheme_returns_rpc_error8 integration (in
tests/integration.rs):file_load_valid_fixture-- pinned CID assertion (byte-stability)file_load_round_trip_cid-- §6.2 delivery-independence (file path)file_load_nonexistent_returns_file_not_found_error-- §8 error modelfailure_memento_for_nonexistent_file-- §8.1 + §8.3 deterministic CIDregistry_lookup_by_kind_cid-- §9 (kind, cid) lookupregistry_memento_includes_loaded_cid-- §9.1 + §9.4 registry CID presentrpc_stdio_load_valid-- §4 stdio RPC viastub_rpc_serverbinaryrpc_stdio_registry_cid_matches_file_load_when_payload_identical-- §6.2Out of scope (skeleton)
ProtocolBlessingMemento(§14) -- follow-onprovekit.plugin.invoke/.shutdown(§4.2.2/.3) -- onlydescribeneeded for loadrpc-error; follow-oncontentpayload -- consumer specs validate (§1.2)Follow-on sub-issues
Test plan
cargo check -p provekit-plugin-loader-- clean (0 warnings)cargo check -p provekit-cli-- clean (3 pre-existing dead-code warnings, not introduced here)cargo test -p provekit-plugin-loader-- 24/24 pass (16 unit + 8 integration)make test-rust(full workspace gate on spec/plugin-protocol base)🤖 Generated with Claude Code
Summary by CodeRabbit
Release Notes
New Features
--plugin,--strict-plugins, and--plugin-registry-out).Chores