Repository navigation
fix(policy-engine)!: remove AgentControl::new and make the host always carry its runtime parts - #4416
Open
MohammadHaroonAbuomar wants to merge 6 commits into
Open
MohammadHaroonAbuomar wants to merge 6 commits into
MohammadHaroonAbuomar wants to merge 6 commits into
Conversation
… fallible AgentControl::new(runtime) wrapped a caller-built Runtime that exposes no accessor for its annotator dispatcher or Limits, so the control could not keep them. Three paths then failed open: a transform was revalidated against Limits::default() instead of the caller's limits, with_telemetry could not rebuild the runtime and kept the old sink without saying so, and the system_prompt_file rejection in the manifest constructors was bypassed. Remove the constructor. RuntimeParts becomes a plain field that every manifest constructor fills, with_telemetry returns Result<Self, RuntimeError> so a failed rebuild surfaces, and evaluate_intervention_point always revalidates against the retained limits. Add with_perf_telemetry_level so the Rust surface keeps the knob the FFI, Python and Node hosts already have. Tests build through every public constructor and check that tightened limits block an oversized transform (enforce and evaluate-only, at the byte boundary, on a clone, and after a telemetry rebuild), that an installed sink receives the decision event, that the perf level and approval resolver survive the rebuild, that a second sink replaces the first, and that system_prompt_file is rejected everywhere. Refs #3941 Signed-off-by: MohammadHaroonAbuomar <40180927+MohammadHaroonAbuomar@users.noreply.github.com>
Every in-repo caller of AgentControl::new built Runtime::new(manifest, annotator, policy) with default limits, so the rewrite to AgentControl::from_manifest_with_dispatchers(manifest, Some(annotator), Some(policy)) keeps behaviour. Covers the SDK integration tests, the rig tests, the annotators example and the standalone coding_agent example, which CI does not compile and was built by hand. Refs #3941 Signed-off-by: MohammadHaroonAbuomar <40180927+MohammadHaroonAbuomar@users.noreply.github.com>
…change Move agent_control_specification and the five integration crates to 0.4.0-beta.1, update the agent-governance-rust path dependency requirement and the affected lockfiles. Record the removed constructor and the fallible with_telemetry in BREAKING_CHANGES.md and CHANGELOG.md, add the ? to the README telemetry table, and state in acs-retarget.md that construction stays manifest-only until the upstream accessor lands. Refs #3941 Signed-off-by: MohammadHaroonAbuomar <40180927+MohammadHaroonAbuomar@users.noreply.github.com>
cargo clippy --workspace --all-targets -- -D warnings failed on the six-field tuple every_constructor() returns (clippy::type_complexity). Give it a ConstructorCase alias so the lint passes and the shape is documented once. Refs #3941 Signed-off-by: MohammadHaroonAbuomar <40180927+MohammadHaroonAbuomar@users.noreply.github.com>
The harness depends on agentmesh and through it on the Rust SDK, so its lockfile must follow the 0.4.0-beta.1 bump. It was also still on agent-control-spec 0.4.0-alpha.4's predecessor from before #4224, so cargo metadata --locked refused it. Regenerated offline; the harness builds with --locked --offline. Refs #3941 Signed-off-by: MohammadHaroonAbuomar <40180927+MohammadHaroonAbuomar@users.noreply.github.com>
MohammadHaroonAbuomar
requested review from
liamcrumm and
Prayag (prayagupa)
as code owners
October 23, 2026 15:04
Dependency Review✅ No vulnerabilities or license issues or OpenSSF Scorecard issues found.OpenSSF Scorecard
Scanned Files
|
PR Review Summary
Verdict: AI review comments are untrusted advisory output. The summary reports workflow-generated completion status only, not model-authored pass/fail claims. |
📦 Dependency diff (SBOM)Comparing main → mhabuomar/issue-3941. Summary: ➕ 0 added · ➖ 2 removed · 🔄 6 bumped ➖ Removed
|
| Package | Version |
|---|---|
| serde_yaml | 0.9.34%2Bdeprecated |
| unsafe-libyaml | 0.2.11 |
🔄 Bumped
other (6)
| Package | From | To |
|---|---|---|
| agent_control_specification | 0.4.0-beta.0 | 0.4.0-beta.1 |
| agent_control_specification_annotators | 0.4.0-beta.0 | 0.4.0-beta.1 |
| agent_control_specification_mcp | 0.4.0-beta.0 | 0.4.0-beta.1 |
| agent_control_specification_openai | 0.4.0-beta.0 | 0.4.0-beta.1 |
| agent_control_specification_otel | 0.4.0-beta.0 | 0.4.0-beta.1 |
| agent_control_specification_rig | 0.4.0-beta.0 | 0.4.0-beta.1 |
|
|
||
| [workspace.dependencies] | ||
| agent_control_specification = { path = "../policy-engine/sdk/rust", version = "0.4.0-beta.0" } | ||
| agent_control_specification = { path = "../policy-engine/sdk/rust", version = "0.4.0-beta.1" } |
Collaborator
There was a problem hiding this comment.
CI failed because 4 Cargo lockfiles have changed without the required dated dependency-audit note.
Run chmod +x scripts/ci/vendored-patch-audit.sh
⚡ Lockfile changed: agent-governance-rust/Cargo.lock
benchmarks/prompt-injection/harness/agt-rules-baseline/Cargo.lock
policy-engine/Cargo.lock
policy-engine/examples/coding_agent/app/Cargo.lock
❌ vendored-patch-audit: lockfiles changed but no dependency audit doc found.
Rest looks good to me.
Collaborator
Author
|
Thanks Prayag (@prayagupa). Added the dated dependency-audit note for the four lockfiles in b6bf991; the vendored-patch audit passes locally against main now. |
…fresh The vendored-patch audit gate requires a dated note whenever a lockfile changes. Record the four Cargo.lock updates that come with the SDK version bump and the benchmark harness refresh. Refs #3941 Signed-off-by: MohammadHaroonAbuomar <40180927+MohammadHaroonAbuomar@users.noreply.github.com>
MohammadHaroonAbuomar
force-pushed
the
mhabuomar/issue-3941
branch
from
October 26, 2026 14:20
b6bf991 to
b337560
Compare
Ricky Gummadi (Ricky-G)
approved these changes
Oct 28, 2026
Ricky Gummadi (Ricky-G)
left a comment
Contributor
There was a problem hiding this comment.
looks good no issues
This branch has not been 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.
Related Issue
Fixes #3941
Problem & Solution
Problem.
agent_control_spec::Runtimeexposes no accessor for the annotator dispatcher or theLimitsit was built with. A RustAgentControlbuilt throughAgentControl::new(runtime)therefore could not recover them, and three paths failed open:Transformwas revalidated againstLimits::default()instead of the caller's limits. A tightenedmax_snapshot_bytesdid not apply to the transformed snapshot and the guarded body ran.with_telemetry(sink)could not rebuild the runtime, so it kept the old sink and said nothing. The installed sink received no events.system_prompt_filerejection in the manifest constructors was bypassed, sincenewtook any runtime.Reproduced on
origin/main(fb950bc) in a throwaway worktree at/tmp/verify-3941, with a repro module appended topolicy-engine/sdk/rust/src/host/tests.rsusing main's own helpers:AgentControl::new(Runtime::with_limits(run_manifest, NoopAnnotator, <transform of 100 bytes>, Limits { max_snapshot_bytes: 256 })), thenrun_with_optionswith a 180-byte ambient snapshot. Output:MAIN new()+with_limits(256): executed=true outcome_is_ok=false; the test panicked onassert!(!executed). The guarded body ran.AgentControl::new(runtime(run_manifest, <deny policy>)).with_telemetry(InMemoryTelemetrySink), thenevaluate_intervention_point(Input, Enforce). Output:MAIN new()+with_telemetry: decision=Deny events=0; panicked onassert_eq!(events, 1).test result: FAILED. 0 passed; 2 failed.tests.rsfails to compile with 5 errors: E0599 no methodwith_perf_telemetry_level; E0599 nounwrap_or_elseonAgentControlbecausewith_telemetryreturnsSelf, notResult; 3x E0425build_controlmissing (the branch renamed it fromruntime()). So the new tests cannot pass on main.git worktree remove --force,git worktree prune).Solution.
AgentControl::new(runtime). A control is built from a manifest only:from_path,from_manifest,from_manifest_chainand their_with_dispatchersand_and_limitsforms. Each keeps the dispatcher andLimitsit was built with, and each rejectssystem_prompt_file.AgentControlnow always carries its runtime parts (manifest, dispatchers, limits, perf level), so transforms are revalidated against the configured limits andHostEvaluationreceives them.with_telemetryreturnsResult<AgentControl, RuntimeError>. It rebuilds the runtime with the new sink; a second call replaces the first sink. The error is reachable only if the upstream rebuild fails on a manifest that already built once, and it is surfaced rather than swallowed.AgentControl::with_perf_telemetry_level(level)(Off,External,Full), the knob the FFI, Python and Node hosts already expose, so Rust keeps parity.integrations/rig,integrations/annotators,examples/coding_agent/app) to the manifest constructors.CHANGELOG.mdandBREAKING_CHANGES.md.Why removal rather than guarding or deprecation. The retargeted 0.4.0-beta.0 crate is not on crates.io, so there is no external user to protect with a deprecation window, and a window would keep the two remaining holes (telemetry and
system_prompt_file) open for its length.HostEvaluation::from_engineand the Python/Node bindings are a separate follow-up.Impact on Your Work
Breaking for Rust hosts that built
AgentControlfrom a prebuiltRuntime. The rewrites are inBREAKING_CHANGES.md(one line each forRuntime::new,Runtime::with_limits, andwith_telemetry). All 31 in-repo call sites are migrated in this PR.Version changes:
policy-engine/sdk/rust/Cargo.toml: 0.4.0-beta.0 -> 0.4.0-beta.1.policy-engine/integrations/{annotators,mcp,openai,otel,rig}/Cargo.toml: -> 0.4.0-beta.1. Their SDK dependencies are path-only, so no version requirement needed updating.agent-governance-rust/Cargo.toml: workspace dependencyversion = "0.4.0-beta.1".policy-engine/Cargo.lockupdated for the six crates;agent-governance-rust/Cargo.lockandpolicy-engine/examples/coding_agent/app/Cargo.lockupdated.benchmarks/prompt-injection/harness/agt-rules-baseline/Cargo.lockrefreshed (SDK -> beta.1;agent-control-specalpha.3 -> alpha.4 withserde-saphyr,serde_path_to_error,tempfile;thiserror2.0.19 -> 2.0.21;serde_yamlandunsafe-libyamlremoved). This lock was already behind main onagent-control-specsince feat(policy-engine): upgrade ACS and clarify upstream ownership #4224 andcargo metadata --lockedrefused it. The supply-chain-check workflow runs cargo-deny on that manifest without--locked, so CI would not have caught it, but the previous SDK bump (a6db33e) kept this lock in step and this PR does the same.Intentionally untouched and still 0.4.0-beta.0: the
sdk/pythonandsdk/nodebinding crates (agent_control_specification_py,agent-control-specification-node), thelib.rsdeprecated(since = "0.4.0-beta.0")attributes, and theQUICKSTART.mdanddocs/dependency-auditsreferences, which are historical kit and audit text.Timeline
None
Alternatives Considered
Transformonly. Fixes the limits hole but leaves the silent telemetry failure and thesystem_prompt_filebypass in place.newand remove later. No external user exists for the unpublished crate, and a window keeps two fail-open paths alive for its length.newwith caller-supplied limits. Still cannot recover the dispatcher or enforce thesystem_prompt_filecheck, and it asks the caller to restate limits the runtime already holds, which invites drift.Runtimeaccessors. Leaves the holes open on an unknown schedule; the manifest constructors already give the host everything it needs.Type of Change
Package(s) Affected
Core & runtime:
Governance & security:
Platform & tooling:
CLI plugins:
Shared / other:
Testing
Unit Testing
New tests in
policy-engine/sdk/rust/src/host/tests.rs(all pass on this branch; they do not compile on main, see above):max_snapshot_bytesis admitted (canonical length asserted == 256); one byte over is blocked withhost_error:transform_invalid.host_error:transform_invalidwithtransformed_policy_targetNonewhile the body runs. (EnforcementModehas onlyEnforceandEvaluateOnly; there is no observe variant, so this is the observe-style path.)with_telemetry(escalate then allow, 2 events).with_telemetryreplaces the first sink (0 vs 1 events).with_perf_telemetry_levelround-tripsOff -> External -> Fullviaruntime().perf_telemetry()and surviveswith_telemetry.system_prompt_fileis rejected by all seven public constructors.with_telemetrydelivers exactly one deny event for every constructor (from_path,from_path_with_dispatchers,from_manifest,from_manifest_with_dispatchers,from_manifest_with_dispatchers_and_limits,from_manifest_chain,from_manifest_chain_with_dispatchers).Extra boundary probes, added, run and reverted (tree clean):
Manifest::from_yaml_chainover two documents, thenfrom_manifest_with_dispatchers_and_limitswith a 256-byte limit: oversized transform blocked,executed=false, reasonhost_error:transform_invalid. There is nofrom_manifest_chain_with_limitsconstructor, so chain plus limits takes this two-step route.host_error:transform_invalid, 3 events reached the sink.with_telemetryon the clone: the clone blocks (executed=false) and emits 2 events; the original still blocks and its runs add 0 events to the clone's sink. (A first attempt failed only becauseQueuePolicyis a shared queue the clone consumed; re-run with two queued responses passed.)Debugimpl prints only<runtime>and the resolver presence; no limits or dispatcher leak.No defects found in the branch logic.
Manual Testing
All run in the foreground on this branch from
policy-engine/unless noted:cargo test --locked -p agent_control_specification --lib: 57 passed, 0 failed.cargo test --locked -p agent_control_specification --features bundled-dispatchers,opa --all-targets: lib 57, agent_loop_exercise 9, artifact_validation 1, conformance_corpus 3, fail_closed_error_parity 1, ffi_default_dispatchers 5, ffi_roundtrip 10, streaming_conformance 5, upstream_compatibility 6; examples basic_host and ifc_agent compiled (0 tests); all ok.AGT_EXPECT_UPSTREAM_REGO=1 cargo test --locked -p agent_control_specification --features bundled-dispatchers,opa,agent-control-spec/rego --test upstream_compatibility: 6 passed.cargo clippy --locked --workspace --all-targets -- -D warnings: failed on the first version of the branch (clippy::type_complexityon theevery_constructor()return type atsdk/rust/src/host/tests.rs:1510); fixed in 4fb4975 by naming the tuple; re-run exits 0.cargo fmt --all -- --check: exit 0 before and after that fix.policy-engine/integrations/rig:cargo test, guarded_tool 17 passed.policy-engine/integrations/annotators:cargo test, dispatcher 11 passed, moderation_flow 0, lib 0; ok.policy-engine/examples/coding_agent/app:cargo build --locked, Finished (exit 0).cargo build --locked -p agent_control_specification --example basic_host --example ifc_agent: Finished (exit 0).agent-governance-rust:cargo build --locked -p agentmesh, Finished, compiledagent_control_specification v0.4.0-beta.1(exit 0).benchmarks/prompt-injection/harness/agt-rules-baseline:cargo build --locked --offlineafter the lock refresh, Finished (exit 0).python3 -I scripts/ci/changed_lines.py --base origin/main --extensions ".md,.txt,.rst,.py,.ts,.js,.go,.rs,.cs,.yml,.yaml" --mode added-lines --output ci-diff/added.txt(701 lines), thennpx --yes cspell@8.17.3 --config .cspell.json --no-progress ci-diff/added.txt: Issues found 0, run before and after the follow-up commits;ci-diffremoved.python3 scripts/check_v4_ratchet.py:v4 removal ratchet OK (grand total 4).git merge-tree --write-tree origin/main HEAD: clean.Checklist
cargo fmt --checkandcargo clippy -D warnings; no Python changed, so ruff does not apply)cargo test; no Python changed, so pytest does not apply)Attribution & Prior Art
Prior art / related projects (if any):
None
AI Assistance
If AI tools materially shaped this change, briefly note what was used:
Claude Code drafted the change and tests under my direction following the direction I posted on the issue; an independent verification pass re-ran the CI commands, confirmed the new tests fail on main, and probed the boundaries, and I reviewed the result.
IP, Patents, and Licensing