feat: add external evaluator handoff example - #3419
Conversation
|
Azure Pipelines: There may be pipelines that require an authorized user to comment /azp run to run. |
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. |
github-actions
Bot
added
documentation
|
🔴 Contributor Check: HIGH
Automated check by AGT Contributor Check. |
github-actions
Bot
added
the
needs-review:HIGH
There was a problem hiding this comment.
Pull request overview
TL;DR: 2 blockers, 0 warnings. Fix #1 and #2 and this ships.
| # | Sev | Issue | Where |
|---|---|---|---|
| 1 | Block | assert used for runtime validation (can be stripped with -O) |
test_external_evaluator_handoff.py |
| 2 | Block | README claims hash covers emitted “request bytes”, but request_id is computed before it is added |
README.md |
Changes:
- Add a runnable, offline example that exports
DecisionBOMobservations into a deterministic strict-JSON external-evaluation request with an explicit allowlist boundary. - Add pytest coverage for determinism, immutability, allowlisting behavior, and strict-JSON/timezone validation.
- Add example documentation and a local requirements file for running/testing the example.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| examples/external-evaluator-handoff/external_evaluator_handoff.py | Implements the deterministic strict-JSON handoff request builder and a synthetic runnable demo. |
| examples/external-evaluator-handoff/test_external_evaluator_handoff.py | Adds regression tests covering determinism, allowlisting, immutability, and validation behavior. |
| examples/external-evaluator-handoff/README.md | Documents the boundary, usage, test commands, and security notes for the example. |
| examples/external-evaluator-handoff/requirements.txt | Defines minimal dependencies to run and test the example. |
|
Azure Pipelines: There may be pipelines that require an authorized user to comment /azp run to run. |
🤖 AI Agent: code-reviewer — View details
TL;DR: 0 blockers, 1 warning. Safe experimental feature addition with minor follow-up needed.
Action items:
Warnings:
|
🤖 AI Agent: breaking-change-detector — API Compatibility
API CompatibilityNo breaking changes detected. |
🤖 AI Agent: docs-sync-checker — Docs Sync
Docs SyncDocumentation is in sync. |
🤖 AI Agent: test-generator — `examples/external-evaluator-handoff/external_evaluator_handoff.py`
|
🤖 AI Agent: security-scanner — View details
No security issues found. |
|
The prior art link in the README, github.com/joy7758/SAEE, returns 404. The example itself is scoped correctly as a standalone with no runtime dependency, which is what the contribution guide asks for. Please point the reference at the right repository or drop it if the project is not public. |
|
liamcrumm Thanks for catching this. The SAEE repository was private at the time of submission, which caused the 404. It has now been made public, so the prior art reference at https://github.com/joy7758/SAEE resolves correctly (verified HTTP 200). No content changes were needed — the README link already points at this repository. |
Imran Siddique (imran-siddique)
left a comment
Imran Siddique (imran-siddique)
left a comment
There was a problem hiding this comment.
Diagnosed all three failing checks rather than the first one, because a partial diagnosis on a PR
with three reds is worse than none. All three are genuine and all three are yours, but they are all
small.
1. No Unauthorized Crypto. The gate reports:
❌ no-custom-crypto: direct crypto usage found outside designated modules:
+import hashlib
From external_evaluator_handoff.py:12, used at line 132:
request["request_id"] = f"eval_{hashlib.sha256(canonical).hexdigest()}"That is a deterministic content-addressed identifier, not a security primitive, so it is the same
category as the exemption scripts/ci/no-custom-crypto.sh already grants to
agt-policies/.../cli/_migrate_resolution/build.py ("writes a non-security SHA-256 content checksum
of a generated rego bundle").
Two ways out, and I would take the second. You can ask for a per-file exemption on the same grounds.
But this is a runnable example under examples/, and the gate's own comment says "Everything else
should use the SDK's public API, not raw primitives". An example that reaches for hashlib teaches
the pattern the gate exists to discourage, and the next contributor copies it. If a stable
request_id is what you need, deriving it through the SDK, or from the already-canonical JSON with a
non-crypto identifier, keeps the example exemplary and clears the gate with no exemption to maintain.
2. Spell-check changed files. Three unknown words, all in your own added lines, none in
.cspell-repo-terms.txt:
:385:40 - Unknown word (SAEE's)
:386:39 - Unknown word (SAEE)
:414:38 - Unknown word (utcoffset)
SAEE and SAEE's are from the README attribution at lines 105 to 106, utcoffset from
value.utcoffset() in the tz check. A <!-- cspell:ignore SAEE --> in the README and a
# cspell:ignore utcoffset in the module are the lightest fix and keep a project-specific proper
noun out of the repo-wide dictionary. This one is genuine, unlike several other PRs in the queue
right now where the same check is red from a stale-base over-scan.
3. Dependency Audit Trail. "lockfiles changed but no dependency audit doc found". Your new
examples/external-evaluator-handoff/requirements.txt (agent-governance-toolkit-core>=4.1.0,<6.0,
pytest>=8.0.0,<10.0) trips the gate. It wants a matching doc under docs/dependency-audits/; the
files added by #3843 and #3875 are good templates. Both pins look sensible and the doc should be
short.
One more thing, mechanical. This branch is 189 commits behind main. Worth rebasing along
with the three fixes, both so the checks re-run against a current base and because the
agentmesh.governance.decision_bom import surface has had time to move under you.
On the attribution, which I want to say something positive about: naming SAEE as the source of the
interface boundary, with the explicit "No SAEE source code, engine implementation..." disclaimer, is
exactly the right way to handle an externally-inspired design in a Microsoft repository. It is
volunteered rather than extracted, and it makes the provenance question answerable instead of latent.
Nothing blocking on the design.
Signed-off-by: BIN Zhang <joy7759@gmail.com>
Signed-off-by: BIN Zhang <joy7759@gmail.com>
Signed-off-by: BIN Zhang <joy7759@gmail.com>
Signed-off-by: BIN Zhang <joy7759@gmail.com>
5429e64 to
42f6c39
Compare
|
Imran Siddique (@imran-siddique) Thanks for the concrete diagnosis. I have addressed all three items and rebased the branch onto current
Because the public digest helper is a v5 API, the standalone example now requires Local verification on
GitHub currently marks the fork-origin |
Description
This PR adds an experimental, community-driven example that converts AGT
DecisionBOMobservations into a deterministic, strict-JSON request for anexternal post-execution evaluator.
The boundary remains intentionally narrow:
allowlist; the default exports none.
mutate source records, or turn an evaluator result into a governance
decision.
sha256_jcsSDK helper, not araw cryptographic primitive.
This provides an integration-first interoperability surface without adding an
external evaluator, a second audit model, or a new runtime dependency to AGT
core.
Maintainer-review update
This branch is rebased onto current
mainand addresses the three requestedrepository gates:
hashlibuse with the publicsha256_jcsSDK API and adds aregression test for the identifier binding;
SAEEandutcoffset;docs/dependency-audits/.The standalone requirements now use
agent-governance-toolkit-core>=5.0.0,<6.0, because the public digest helper ispart of the v5 supported surface.
Relationship to existing merged work
This PR continues an existing upstream interoperability path rather than
introducing a parallel evidence model:
documented the boundary between AGT runtime evidence and downstream external
operation-accountability profiles.
the merged AuditEntry / AuditService accountability export and EEOAP mapping
example.
generic post-execution evaluator request. It complements feat(agentmesh-integrations): add AuditEntry accountability export example #1370: feat(agentmesh-integrations): add AuditEntry accountability export example #1370 exports
operation-accountability records, while this example hands decision
observations to a separate longitudinal evaluator.
User and developer impact
The implementation is limited to
examples/external-evaluator-handoff/, withone repository-required record under
docs/dependency-audits/. It does notchange a published API, core runtime behavior, policy enforcement, or existing
package dependencies. Developers can run the example locally and adapt the
request at an explicit external-evaluator boundary.
Type of Change
Package(s) Affected
Checklist
The full monorepo test matrix was not run locally. The scoped checks below pass;
the repository CI remains authoritative for the full matrix.
Validation
agent-governance-toolkit-core>=5.0.0,<6.0: example tests — 7 passedagent-meshsource: example tests — 7 passedpython -m compileall -q examples/external-evaluator-handoff— passedgit diff --check— passedThe consolidated package emits existing deprecation warnings for legacy Python
import namespaces; this example does not introduce those warnings.
Attribution & Prior Art
Prior art / related projects:
runtime governance and longitudinal external evaluation. No SAEE source code,
engine implementation, or runtime dependency is included.
AI Assistance
Codex assisted with implementation, tests, documentation, validation, and the
PR update under the contributor's direction. The contributor reviewed the
original candidate and explicitly authorized this maintainer-requested update.
IP, Patents, and Licensing
Related Issues and Merged Work