feat: make streaming reachable from every supported language - #39
feat: make streaming reachable from every supported language#39liamcrumm wants to merge 10 commits into
Conversation
Section 18.1 stream mediation shipped in 0.4.0-alpha.2 as Rust only. The `streaming` cargo feature is off by default and none of the three binding crates enabled it, so a Python, Node or .NET consumer installing a prebuilt binary had no way to turn it on and no API to call. Enable it in each binding and expose the session. Add thirteen stream session entry points to the C ABI, following the handle shape `AcsActivatedPolicy` already uses. Scalar queries return i64 so an absent value needs no allocation, distinguishing absent from failed, because a settled session legitimately has no safe offset. Structured queries return JSON, so the wire contract is owned at the boundary rather than derived from Rust layout. Bind that surface in .NET, and bind the engine directly through pyo3 and napi for Python and Node. All four spell an absent release point in their own vocabulary rather than as 0 or -1, both of which read as permission. Ship the engine binary in the NuGet package. The managed assembly reaches the engine through agent_control_spec_ffi, and the package carried no native library, so every call threw DllNotFoundException on a clean install. Nothing in the .NET SDK worked as published, streaming or otherwise. Build one library per runtime identifier in the release matrix and pack them under runtimes/, and refuse at pack time to build a managed-only package, which installs cleanly and fails in someone else's process. Verify the artifact rather than the checkout. The existing suite runs with the engine on the loader path, which a consumer does not have, so it passed throughout. scripts/verify-dotnet-package.sh builds a console app outside the repository, restores the packed artifact, and calls both a plain entry point and the session. Assert the four languages agree. They reach the engine through four mechanisms and each converts enums, offsets and the absent release point at its own boundary, so agreement is not structural. cross_language_parity.py runs one scenario in all four and diffs, using an astral-plane character whose rune count differs from its UTF-16 length. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Signed-off-by: Liam Crumm <liamcrumm@gmail.com>
actionlint's shellcheck pass flags SC2012 on the version extraction. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Signed-off-by: Liam Crumm <liamcrumm@gmail.com>
The parity harness covered streaming only, so a gap anywhere else stayed invisible. Auditing the four surfaces found one: the C ABI and .NET carry a payload-free interceptor name for the host's record, Node carries it on its wrapper, and Python had no name at all. Add it to Python, defaulting to "acs" as the others do. Widen the harness from 8 streaming assertions to 17 across manifest validation, the interceptor, the activated policy and streaming, and move it to tests/conformance/parity now that it is not streaming specific. A deliberately invalid manifest is included so every language has to fail rather than return. Build that manifest by concatenation rather than writing it out. A repo guard scans committed files for the version key and validates whatever follows, and it cannot strip the quotes of a Python string literal. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Signed-off-by: Liam Crumm <liamcrumm@gmail.com>
MohammadHaroonAbuomar
left a comment
MohammadHaroonAbuomar
left a comment
There was a problem hiding this comment.
Full pass over the surface at c199e97: read the FFI, all three bindings and their suites, packaging and workflows against section 18.1 as merged on main, plus the obligations pending in #30; rebuilt everything locally and probed the boundaries adversarially in each language.
Verified locally
cargo test --workspace --all-features --locked: 380 passed- .NET 44 / Node 43 / Python 52, all green
cross_language_parity.py: 4 of 4 languages agree across 17 assertions- C ABI probes: 19/19 held (double-settle idempotent, deny-after-partial-release stays terminal with the confirmed offset readable, uncleared residue settles
host_error:streaming_unsupported, offset ceiling at i32::MAX enforced, gap/empty/inverted spans refused, null handles and malformed config rejected) - CI: all 14 checks green, including the new
streaming-paritylane and the packed-artifact gate
What is genuinely strong here. The .NET packaging diagnosis and the two independent gates (pack-time guard plus consumer-side script) are exactly the right shape — a defect that only ever fails in a consumer's process now fails in CI. The parity harness is a real asset, and the i64 sentinel design (absent distinct from failed, each language spelling absent natively) is clean. One pleasant side effect worth naming: enabling streaming in the sdk crates means feature unification now runs the engine's streaming tests under a plain cargo test --workspace, so the old silent-skip trap is gone.
Why request-changes. Adversarial probes found four boundary holes, three in .NET and one in Node, detailed inline with reproductions: use-after-Dispose reaches freed native memory (the SafeHandle is present but its protection is bypassed), a two-thread lost-update race on the release accounting (3,098 of 40,000 observed runes silently dropped), interior-NUL truncation that diverges from Python and Node on identical input, and Node silently applying ToUint32 modular arithmetic to out-of-range offsets where the other languages throw. On a surface whose whole job is deciding which runes reach the caller, the boundary is the product; none of these is hard to fix and the rest of the work is solid.
Interaction with #30. #30 has not merged, so its obligations (durable-write watermark, expansive caller definition, L-1 retained tail on resume, mandatory settlement including abandonment, rune identity, output record-and-close) are not gates for this PR. I checked the binding surfaces against the pending text anyway: nothing contradicts it — resume offsets, post-settlement watermark readability, and idempotent finish all line up. Two non-blocking forward-compat notes inline (a Python context manager; documenting that Dispose is not settlement).
Minor, non-blocking. The verify script's NuGet.config lists nuget.org alongside the local feed; on a workflow re-run for a version that already published, restore may serve the published artifact rather than the just-packed one. Package source mapping (local feed for ResponsibleAI.AgentControlSpec, nuget.org for dependencies) would make the gate airtight.
The release note in the description is right: this needs a version bump, and the .NET package should be republished regardless.
The engine takes an annotator dispatcher, a policy dispatcher, a telemetry sink and a perf level. Every binding hardcoded the bundled defaults for all four, so a host that classifies through its own service, evaluates through its own engine, or records its own audit trail could reach none of them from any language but Rust. The 0.3 Python package exposed all four, and a consumer is blocked on exactly this: their classifier calls Azure Content Safety over HTTP and is bound as an annotator. Add the four to the C ABI as JSON callbacks, then bind them in .NET, Python and Node. Rust already had them. A string a host callback returns is freed by the host through a callback registered alongside, so nothing crosses allocators. A dispatcher that fails denies rather than returning nothing. A classifier that could not be reached must not read as a classifier that found nothing, which is the difference between an outage and an approval. A telemetry sink that fails is swallowed, because a sink records what happened and does not get a vote on it. Add manifest tooling alongside: parse without running, compose an overlay chain, and return validation findings as data. Validation that answers by raising cannot be rendered against a document, which is what an authoring tool or a CI linter needs. Widen the parity harness from 17 assertions to 26 so the four languages have to agree about the host hooks too, including that a failing classifier denies. Confirmed the gate fails when one binding is made to allow instead. Document the port from 0.3 in docs/MIGRATION.md, mapping all 67 exports of the old package. Two of them will look like ACS defects and are not: the distribution was renamed, and a startup check for the opa binary now rejects a working install, because Rego is evaluated in process. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Signed-off-by: Liam Crumm <liamcrumm@gmail.com>
The suites and the parity harness import from this checkout, where the engine sits on the loader path, the TypeScript is in dist, and the Python package resolves from source. A consumer has a crate, a wheel, a tarball and a nupkg, and no test that imports from the tree can tell the difference. That gap already shipped once. ResponsibleAI.AgentControlSpec 0.4.0-alpha.2 passed its whole suite and threw DllNotFoundException on the first call any consumer made, because the package carried no native library and CI supplied one through LD_LIBRARY_PATH. So build the four artifacts the release workflow builds, install each into a throwaway project outside the repository, and run the manifest tooling, the host extension points and streaming from there. Node is packed as two artifacts because napi splits it that way, and the main package gains its optionalDependencies at publish time, so both halves are installed explicitly. Confirmed the check fails on a package that ships without its native library rather than reporting green. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Signed-off-by: Liam Crumm <liamcrumm@gmail.com>
d2978d8 to
2f86730
Compare
Both carry a shebang, which ruff's EXE001 requires the mode to match. The parity harness has the same mismatch but sits outside the paths CI lints, so it would have surfaced later. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Signed-off-by: Liam Crumm <liamcrumm@gmail.com>
The manifest check answers only for the document. A manifest can name a bundle, satisfy the grammar, and still fail at activation because the Rego does not compile, so the failure surfaces on a host's first agent action rather than in its CI. 0.3 covered both halves through validate_acs_artifacts and 0.4 covered neither. Compilation happens at activation, so this activates against the supplied bundles in memory and reports what that surfaced. A broken module comes back as the Rego compiler's own complaint, with its line and column. The manifest is checked first and on its own. A document that does not parse would otherwise be reported as an activation failure, which names the wrong half. Validating a manifest that names Rego without supplying the Rego is not a pass: the bundle is still missing, and all four languages say so. Widen the parity harness to 30 assertions and add the case to the artifact verifier, so the four have to agree about this too. Stamp the artifact verifier's package version per run. NuGet caches by id and version, so a fixed version let a stale package from an earlier run satisfy a later one, which is a verifier that can report a pass for code the artifact does not contain. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Signed-off-by: Liam Crumm <liamcrumm@gmail.com>
…nically Found by auditing the direction I had never audited. Every earlier pass asked what 0.3 lost. None asked what the 0.4 engine holds that no binding exposes, which is the question that matters, and the parity harness cannot answer it: a capability absent from all four languages is consistent across all four. Asking it surfaced Limits. Runtime::with_limits has always been there and no binding in any language could reach it, so a host feeding large payloads could not raise max_snapshot_bytes and a host hardening against a hostile manifest could not lower max_extends_depth or manifest_url_timeout_ms. Each field is individually optional, so raising one cap does not restate the other nine. A field present but not a non negative integer is refused rather than silently kept at its default: a host that asked for a smaller bound and got the larger one would believe it was protected when it was not. Add check-language-coverage.py so the omission class fails the build rather than waiting to be noticed. It reads the engine's 85 public re exports and requires every one to be either a declared capability with the token each binding exposes, or a non capability with a written reason. Eighteen capabilities, sixty seven explained, none unaccounted for. Writing the reason down is the point, because an unexplained omission and a deliberate one look identical six months later. Parity is 33 assertions and the artifact check 13, both covering a cap smaller than the context, which must deny in every language or the limit was accepted and dropped. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Signed-off-by: Liam Crumm <liamcrumm@gmail.com>
…nformance The bindings had each decided separately what a watermark, an end reason, a completion and a set of resource caps look like on the wire, because none of those types derives Serialize and something had to choose. That put three copies of the same decision in three crates, and three copies of what "response" means are three chances to disagree. A disagreement there is a host releasing text no task evaluated, which is the drift this whole change exists to prevent. So the contract moves to the engine that defines the behaviour. StreamTrack and SegmentOutcome gain parse alongside the as_str they already had, matching SafetyLevel and StreamSourceType. The bindings keep only an adapter that reshapes a core error for their boundary, which translates calling conventions rather than meaning. Consolidating surfaced a defect all three copies shared. A misspelled resource cap was accepted and ignored, so a host that wrote max_snapshot_byte believed it had set a bound it had not set. That is the same defect as quietly widening the cap, and the core now refuses it. Move the binding checks under tests/conformance/bindings with one entry point. They test the layer above the case corpus, whether a host can reach a decision at all from the language it writes in and the artifact it installs, so they belong beside the corpus rather than in scripts. Extend the coverage check to require every C ABI entry point be declared by the .NET binding, which is the only binding that goes through the C ABI rather than linking the engine. Match whole words: acs_interceptor_new is a substring of acs_interceptor_new_ex, and a substring test called the shorter one declared on the strength of the longer one. Delete the two documents added earlier. They restated the code without adding to it. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Signed-off-by: Liam Crumm <liamcrumm@gmail.com>
…nding Every claim below was reproduced on this branch before it was fixed. A call after Dispose read through freed memory, returning 6 from a session whose native allocation was gone. The stream wrappers used DangerousGetHandle without the DangerousAddRef and DangerousRelease pair the policy wrappers use, so a free during an in-flight call ran under it. The engine's null check cannot catch that: the pointer is not null, only dead. The P/Invoke declarations now take the SafeHandle so the marshaller holds a reference for the call, and a closed handle surfaces as ObjectDisposedException. Two threads observing one session lost 1,109 of 40,000 runes, because the session's methods take &mut and nothing serialized them. A lost observe shortens the received offset, which releases text no task evaluated. The lock goes in the C ABI rather than in each binding, so every consumer of the ABI inherits it instead of rediscovering the need, and Python's and Node's own locks can retire. ObserveText truncated at an interior NUL, counting 1 rune where Python and Node counted 4. U+0000 is a scalar a model can emit, and the profile obliges a host to report counts matching the text it accumulated, so this refuses rather than quietly shifting every later offset. Node accepted rune offsets that N-API reshaped with ToUint32, so recordOutcome with an end of 2^32 + 5 recorded a cleared span of [0, 5). Python and .NET already refused the same input. Guard the offsets and pin the behaviour in both languages. Move the last hand-built wire shapes into the core. All three bindings now hold zero json! calls; every shape they emit is stated once. Cover settlement with uncleared residue in the cross-language check. Every prior scenario settled clean, so nothing pinned the bindings to the fail-closed core of the profile. Restore the ActivatedPolicyHandle remarks this branch had dropped, which document the very invariant the stream wrappers were missing. Add a Python context manager and a .NET note on Dispose, so settling an abandoned session is the path of least resistance. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Signed-off-by: Liam Crumm <liamcrumm@gmail.com>
I tried to address all your feedback. Thanks for the review. Let me know if you want to discuss anything else. |
2 participants
Summary
Ten engine capabilities were reachable only from Rust, or from nowhere. This binds all ten in Rust, Python, Node and .NET, closes a naming gap in Python, and fixes a packaging defect that made the published .NET package non-functional.
What is now reachable
LimitsRust needed no additions. It already had all of it, which was the defect.
Problem
Three defects. The existing tests could not see any of them.
Nine of the ten were locked behind a cargo feature or a hardcoded default.
streamingis off by default and no binding crate enabled it. The dispatchers, sink, perf level and limits were hardcoded to their bundled defaults at every entry point. A consumer installing a prebuilt wheel, tarball or nupkg cannot flip a cargo feature or edit a constructor.The published .NET package never worked. The release job packed only the managed project, so the nupkg carried no native library. Against the published artifact:
That affected every feature, not only streaming. It passed CI because the suite runs with the engine on
LD_LIBRARY_PATH.Two capabilities existed nowhere. Validation answered by raising, which a linter cannot render against a document, and nothing checked a manifest against the Rego it names. A manifest can satisfy the grammar and still fail at activation because its Rego does not compile.
Changes
sdk/ffi/sdk/dotnet/runtimes/{rid}/native/in the packagesdk/python/,sdk/node/.github/workflows/scripts/verify-artifacts.py,check-language-coverage.py,verify-dotnet-package.shdocs/STREAMING.md,MIGRATION.mdDesign notes
Scalar FFI queries return
i64. Zero or above is the value,-1is absent,-2is a failure witherr_outset. Absent and failed stay distinct because a settled session legitimately has no safe offset. Each language then spells absent natively, never0and never-1, since both read as permission.A host dispatcher that fails denies. A classifier that could not be reached must not read as one that found nothing. A telemetry sink that fails is swallowed, because a sink records what happened and does not get a vote on it. A resource cap that is not a non-negative integer is refused rather than silently kept at its default, because a host that asked for a smaller bound and got the larger one would believe it was protected.
Structured values cross as JSON. None of the types derive serde, so the boundary owns the wire contract rather than deriving it from Rust layout.
Verification
Three gates guard the failure classes that produced this PR.
verify-artifacts.pybuilds the crate, wheel, tarball and nupkg, installs each into a project outside the repository, and runs the surface there. No test that imports from the checkout can catch a package shipping less than the tree holds.check-language-coverage.pyreads the engine's 85 public re-exports and requires each one to be a declared capability with the token every binding exposes, or a non-capability with a written reason. It foundLimits.cross_language_parity.pyruns one scenario in all four and diffs them. Its rune count uses an astral-plane character, where a binding counting UTF-16 code units reports 4 instead of 3 and releases text no task evaluated.Each gate was confirmed to fail on the defect it guards. A native-less package exits non-zero. A binding made to allow where the others deny names the field and exits 1.
Note for the release
The version is unchanged at
0.4.0-alpha.2, which is already published, so releasing this needs a bump. The .NET package wants republishing regardless, since the current one cannot run.Open
The 18
guard_*framework adapters from 0.3 have no home in 0.4. They wrap nine frameworks, so a policy decision runtime is the wrong owner.docs/MIGRATION.mdmaps them. Where they should live is undecided.OtelMetricsTelemetrySinkhas no 0.4 equivalent, because OTel was a separate crate. A host can write one against the telemetry hook, but adding an OTel dependency to ACS is a separate decision.