Add per-hook conditional attributes to the OTel mapping by rocklambros · Pull Request #261 · GenAI-Security-Project/agent-control-standard

rocklambros · 2026-10-12T12:16:49Z

What changed

otel-mapping.json required attributes whose source fields are optional on the wire, so a conformant exporter could not emit them. Each step_to_span entry can now carry a conditional_attributes list, as Bar decided on #227 with Maurits's proposal. An exporter MUST emit each conditional attribute when the payload carries the source field and MUST omit it when the field is absent. Two attributes that name no wire field, acs.provenance.lineage_depth and acs.provenance.origins_compacted, are gone from both mappings and the docs.

Reviewers are @bar-capsule, @afogel and @artmaro, and one approval is enough. This PR also changes .github/workflows/ and tests/, which CODEOWNERS gives to project leads and not to @artmaro. An approval from @bar-capsule or @afogel lands it alone. An approval from @artmaro needs one of theirs beside it. The proposals below need an answer from a Spec lead (@bar-capsule or @afogel) before any approval.

Which issue does this implement

Closes #227

This is an editorial correction (typo, grammar, link, formatting) with no change in meaning, so it needs no issue

Base branch

integration, because this touches the specification, schemas, a reference implementation, an adapter, tests, or CI
main, because every changed path is on the documentation lane allowlist in CONTRIBUTING.md

Type of change

Specification change (schema, hooks, events, AgBOM)
Reference implementation or adapter
Documentation
Tooling or CI
Governance (licensing, security policy, contributor docs)

Specification changes

I opened a Discussion before this PR
Schema changes validate against the JSON Schema spec
I described the impact on downstream implementers below

No Discussion was opened. The decisions are on #227, cited below.

Breaking for implementers? Partly.

An exporter that emitted acs.provenance.lineage_depth or acs.provenance.origins_compacted must stop, because both are removed.
An exporter that reads only required_attributes and optional_attributes stops emitting the moved attributes, so it must read the new list too. That includes Export Guardian traces as OTLP to a file and over HTTP #244 (see Merge order).
Nothing on the wire changes.

Decided by a Spec lead

Bar's decision (6061539514) with Maurits's table (6056794313):

Hook	Moved to `conditional_attributes`	Source field in the hook schema
`steps/toolCallRequest`	`acs.capability`	`capability`, optional
`steps/memoryStore`	`acs.memory_store.name`, `acs.operation`	`memory_store`, `operation`, both optional (only `content` is required)
`steps/memoryContextRetrieval`	`acs.memory_store.name`	`memory_store`, optional
`steps/knowledgeRetrieval`	`acs.source.type`	`source`, optional
`steps/subagentStop`	`acs.subagent.final_chain_hash`	`final_chain_hash`, optional
`agbom/changed`	`acs.agbom.change_reason`	`reason`, optional (the schema requires nothing)
decision event	`acs.evaluator`	`AcsResult.metadata`, optional

Bar also noted that lineage_depth and origins_compacted name no wire field, and that the mapping must say where acs.agbom.format comes from. I checked every row against its schema, and every one agrees with the table.

Proposed by me, for a Spec lead to confirm

These go a step past the decision. Each one is small, but none was decided on #227, so please say if any should change.

"MUST omit when absent", and an empty array counts as absent. Bar's decision says "filled whenever the payload has the field". I made the other half explicit so a reader can trust an absent attribute to mean an absent field. This also rules out filling in a default value.
acs.request_id_ref moves from optional to conditional on steps/toolCallResult. Its source is optional on the wire, which is this issue's definition of conditional. As an optional attribute it carried no obligation, yet it is the key that joins a result span to its request.
acs.agbom.format stays required, with a note on its source. No AgBOM payload has a format field, because the Observed Agent always sends the canonical form. An exporter recording the payload sets canonical. A Guardian recording a serialization it rendered sets its agbom_serializations_supported value. Moving it to the conditional list would make it an attribute nobody ever emits. events.md now uses the handshake's names (cyclonedx-1.6, spdx-3.0).
Drop lineage_depth and origins_compacted rather than point them at something else. Bar noted they name no wire field. Removing them is my choice of remedy, and it deletes two published optional attributes.
The ACS-Trace bar in events.md gains the conditional rule in requirement 1. Requirement 2 reads "evaluator identity and reasoning when the response carries them", because evaluator is optional on the wire.

conformance.md summarizes the bar in three lines and links to events.md. Open PR #24 rewrites that page, so this PR leaves it alone. Nothing it says becomes false.

Reference implementation and tooling

trace-pillar.ts walked only required_attributes for step spans, so moving acs.capability would have dropped its row. It now reads both lists, and every row records which list it came from. The table marks a conditional row with ○, and ✖ means only one thing: a required attribute the wire need not carry. A test fails if any required row is not emittable. I checked it bites: moving acs.evaluator back to required fails two tests. Before this change, that move passed all of them.
tests/test_trace_mapping_defaults.py is new. The mappings keep their tables in default keys, and nothing validated them, so a misspelled conditonal_attributes passed every check. The test validates every default against its own schema and holds each step span to one list per attribute. It also pins every conditional attribute to its wire field across all 21 step spans. The checker's own test covers only the six the reference Guardian evaluates, so moving acs.subagent.final_chain_hash back to required used to pass every suite. Now it fails. The step_to_span entry schema now declares _note, and a test rejects keys an entry doesn't declare. The published schema itself stays open. I checked it bites by planting that misspelling in the real file.
reference-implementation.yml now also runs on specification/**. This tree reads the schemas and the Trace mappings at runtime, so a spec-only change could break it with CI green.

This PR does not touch otel-trace-exporter.ts, which belongs to #244.

Merge order with #244

#244's exporter reads only required_attributes and optional_attributes today (head 34e1ee7). Whichever of the two lands second must leave the exporter reading conditional_attributes too. Otherwise the moved attributes stop appearing on spans and every test stays green. I've asked on #244 for that change, plus a test against the real mapping. Before merging the second one, check that this prints 1 or more. It prints 0 on #244's head 34e1ee7. A plain conditional_attributes grep finds the decision-event code and passes falsely.

git show origin/integration:reference-implementations/agt/packages/guardian/src/otel-trace-exporter.ts | grep -c 'entry.conditional_attributes'

Also check that the real-mapping coverage test requested on #244 exists and passes.

Merging

Merge with an explicit squash message: the title, the closing line from the issue section above, a blank line, and the Signed-off-by lines. The repository squashes with the PR body by default, which drops the sign-off.
Confirm the agt check passed (gh pr checks 261 | grep -E '^agt\s+pass'). It runs here, and it isn't a required check.
If Add a Go Guardian reference implementation #169 (the Go Guardian) has merged first, run cd reference-implementations/agt-go && go generate ./internal/schema and commit the result to this branch before merging.

I tested this

I synced my branch with the base branch before opening this
uv run pytest -v passes on my machine
uv run mkdocs build --strict passes on my machine

pytest: 912 passed, 1 skipped. From reference-implementations/agt/: bun install --frozen-lockfile clean, bun run typecheck clean, bun test 1115 pass, 1 skip, 0 fail. The hook taxonomy guard passes unchanged. The branch merges cleanly with #258.

Checklist

Commits are signed off with git commit -s (required by the DCO)
Prose follows STYLE.md
No secrets, tokens, or internal URLs in the diff

Security

This change has no security impact

otel-mapping.json required attributes whose source fields are optional on the wire, so a conformant exporter could not emit them and a verifier could not tell a missing value from a broken exporter. Bar decided on #227 to relax the mapping for v0.1 with Maurits's per-hook conditional_attributes list: an exporter MUST emit each one when the payload carries the source field and MUST omit it otherwise. The list holds acs.capability on steps/toolCallRequest, Maurits's four rows (memoryStore, memoryContextRetrieval, knowledgeRetrieval, subagentStop) and acs.agbom.change_reason on agbom/changed, each checked against its hook schema. acs.evaluator moves to the decision event's existing conditional list, because metadata is optional on AcsResult. acs.agbom.format stays required. No AgBOM payload carries a format, so the mapping now says where the value comes from: canonical for the wire payload, or the agbom_serializations_supported value of a serialization the Guardian rendered. events.md's value list now uses the handshake's enum names. acs.provenance.lineage_depth and acs.provenance.origins_compacted name fields no schema defines, so both go from the two mappings and the docs. The conformance checker in trace-pillar.ts now reads the conditional list for step spans as well as the required list, so moving an attribute between them keeps its row. A new test ties the rows to both lists and fails if the checker reads only one. Signed-off-by: Rock Lambros <rock@rockcyber.com>

A review of this change found four gaps. The conformance checker printed the same mark for a conditional attribute, where an optional wire source is the definition, and for a required one, where it is the defect #227 fixed. Moving acs.evaluator back into the required list kept every test green. Each row now carries the list it came from, the table marks conditional rows with their own symbol, and a test fails when a required row is not emittable. acs.request_id_ref sat in optional_attributes, which carries no obligation, although its source is optional on the wire and it is the join key from a result span to its request. It moves to the conditional list, where an exporter must emit it when the payload carries it. Nothing checked the data each mapping keeps in its default keys, so a misspelled list name passed every check while both readers skipped it. A new pytest validates every default against its own schema and holds each step span to one list per attribute. The step_to_span entry schema now declares _note and rejects unknown keys. An empty source array now counts as absent, so two exporters cannot disagree about one. The reference implementation job now also runs on specification/**, because this tree reads the schemas and the Trace mappings at runtime and a spec-only change could break it with CI green. Signed-off-by: Rock Lambros <rock@rockcyber.com>

github-actions · 2026-10-12T12:17:07Z

This pull request's description says it closes #227, which is on the project roadmap. When it merges, GitHub closes that issue and the roadmap counts it as delivered. Please confirm the change delivers the whole issue before merging.

…uates The checker's required-row test covers the six methods the reference Guardian evaluates, so moving acs.subagent.final_chain_hash or acs.agbom.change_reason back to required still passed every suite. A pytest now pins every conditional attribute to its wire field and fails if the attribute returns to a required list or the schema starts requiring its source. The table no longer marks a conditional row with no wire source at all as the list working as defined, and two comments that still said a row reads red now say what the table prints. Signed-off-by: Rock Lambros <rock@rockcyber.com>

…test Closing the entry schema with additionalProperties: false changed what a deployment may publish, which no one decided. The typo it was meant to catch, a misspelled list name, now fails a test that compares each entry's keys against the ones the entry schema declares. Signed-off-by: Rock Lambros <rock@rockcyber.com>

The table now marks a conditional row with ○, distinct from ✖, and the README described the rows without saying what either mark means. Signed-off-by: Rock Lambros <rock@rockcyber.com>

bar-capsule

Covers everything from my #227 comment, plus running the reference tests on specification/** changes, which closes the gap where a spec-only PR could break them. pytest, mkdocs --strict and the bun suite pass, also with #258 merged in. One thing for this PR: conformance.md:51 still says "with the required attributes populated"; it needs the conditional clause events.md now has.

It now matches events.md: each conditional attribute is populated whenever its source field is present. Signed-off-by: rocklambros <rock@rockcyber.com>

rocklambros · 2026-10-13T13:42:16Z

@bar-capsule, fixed in 6921ac4. conformance.md:51 now carries the same conditional clause as events.md: "with the required attributes populated and each conditional attribute populated whenever its source field is present." pytest and the strict docs build pass. Could you take another look when you get a minute?

rocklambros · 2026-10-13T20:57:04Z

@bar-capsule @afogel, this one's ready for an approval. Bar's only requested change is in 6921ac4: conformance.md:51 now carries the same conditional clause as events.md. CI is green, and I dry-ran it squashed on top of PR #258 with pytest, the strict docs build and the bun suite all passing. It's on the critical path for the trace work landing this week, so either of your approvals unblocks PR #244 and the issue #228 PR behind it.

Bar's one requested change is in 6921ac4: conformance.md:51 now carries the same conditional clause as events.md. Ariel approved 6921ac4.

The embedded schemas follow integration after GenAI-Security-Project#63, GenAI-Security-Project#258 and GenAI-Security-Project#261. The parity recording stores the git object IDs of the deciding TypeScript paths instead of a commit, so a squash merge no longer makes TestRecordingIsCurrent fail. The paths include agt.lock and the agt-bridge package.json, which pin the AGT commit and SDK. The recorder and the test read them from one list; the test fails when the recording names other paths or was made from another corpus, and the recorder refuses uncommitted changes to them. A test checks that the Go workflow runs on every deciding path. agt/README.md tells a TypeScript contributor when to re-record. A test requires agt-native.lock and agt.lock to pin the same AGT repo and commit. The native build fails when AGT at that commit changes ffi.rs, which agteval/acs.h is written from, or fetches another opa than agt-native.lock pins. Under the agt evaluator, policy.opa_timeout must be shorter than protocol.decision_timeout; guardian.yaml ships 4s. Otherwise a slow opa lets the agent stop waiting before AGT's deny arrives (GenAI-Security-Project#269). make load measures either evaluator under concurrent clients, selected by EVALUATOR like the other targets. Under agt each decision runs one opa process; on a 20-core host decisions stop rising near 16 concurrent ones, and at 256 they reach the 5s timeout. guardian.yaml now ships limits.max_engine_calls 32, and docs/load.md records the results. The load run found that an engine call gave its slot back only after its answer was sent, so a client's next step could be refused engine_saturated with no call running; callEngine now releases the slot first, also after a panic. When the host adapter's audit log cannot be written, the record goes to stderr as one JSON line with the final disposition, and the refusal's reasoning says whether the record went to stderr or nowhere. The audit file's close error now counts. The Codex and OpenCode hooks pass their own stderr. The §6.3 row says that ALLOW at a hook that permits no DENY is an interpretation (GenAI-Security-Project#270), and the docs say only the adapter library reaches those hooks. Dependabot proposes Go updates, one pull request per dependency across both modules. The Go workflow runs weekly, so govulncheck sees advisories for the toolchain that Dependabot does not move, triggers on the agt-bridge package.json, and no longer fetches full history. A test checks that the method table permits DENY exactly where hooks.md does, and fails on a Decision line it cannot classify. The coverage floor counts every package except commands, examples and test helpers. The Guardian meets the expectations GenAI-Security-Project#256 sets for reference implementations, mapped to tests in docs/conformance.md. audit.max_file_bytes and audit.max_files rotate each audit file; guardian.yaml keeps at most 1 GiB per log; max_file_bytes needs max_files of at least 1, so a rotation never deletes the live log. make load can set the rotation and fails if the log drops, splits or reorders a record or exceeds its bound; docs/load.md records a run and the cost of long sessions. storage.provider names the session store. The HMAC secret file holds base64 of at least 32 bytes. key_id is a configured label with no default, an unknown key_id is refused (GenAI-Security-Project#118, GenAI-Security-Project#272), and the hooks and the conformance harness take it as a flag. A toolCallResult whose request_id_ref matches no request is recorded (GenAI-Security-Project#272). Decisions from other pull requests that the Go code contradicted: - A shell command's URL ends only at ASCII whitespace; U+FEFF or U+00A0 in the userinfo no longer hides the host curl reaches (GenAI-Security-Project#268). - Versions are compatible when they share X.Y under 0.x; the handshake negotiates only such a version, and a step must match it (GenAI-Security-Project#196, GenAI-Security-Project#255). - A policy target that is not a string is denied evaluation_failed; the TypeScript fix is in GenAI-Security-Project#229, so two recorded cases are listed as ahead of the recording until it is re-recorded. - The host adapter records every decision failure under either posture, with its class, handshake failures included (GenAI-Security-Project#37). - The host adapter denies under any posture a signed error other than -32603, an unverified answer and a malformed answer; the docs mark this as an interpretation of §6.4 ahead of GenAI-Security-Project#32, which is open. The version rule is marked the same way against §3. - The audit directory is 0700; a symlinked audit path or an audit file open to group or others is refused (GenAI-Security-Project#71). Claude-Session: https://claude.ai/code/session_01DFdYnoBitHhPcUSq7wMWs1 Signed-off-by: Konstantin Trunin <konstantin@trunin.com>

rocklambros added 2 commits October 11, 2026 23:37

rocklambros requested review from GangGreenTemperTatum, Lrd0036, afogel, almogbhl, artmaro, bar-capsule, fewdisc, mamicidal, sclintonowasp and stefanoamorelli as code owners October 12, 2026 12:16

github-project-automation Bot added this to ACS Project Tracker Oct 12, 2026

rocklambros added 2 commits October 12, 2026 06:48

rocklambros self-assigned this Oct 12, 2026

rocklambros moved this from Needs triage to In progress in ACS Project Tracker Oct 12, 2026

rocklambros mentioned this pull request Oct 12, 2026

[Bug] OCSF provenance has two homes, and the mapping's enrichment shape fails OCSF validation #260

Open

Explain the trace table's three marks in the conformance README

3d0fb0b

The table now marks a conditional row with ○, distinct from ✖, and the README described the rows without saying what either mark means. Signed-off-by: Rock Lambros <rock@rockcyber.com>

rocklambros mentioned this pull request Oct 12, 2026

[RefImpl] Commit a sample OTLP trace from the reference Guardian, and test the exporter against it #266

Closed

MKaptein mentioned this pull request Oct 13, 2026

[Bug]: otel-mapping.json requires acs.capability on gen_ai.tool.call, but capability is optional in tool-call-request.json #227

Closed

bar-capsule previously requested changes Oct 13, 2026

View reviewed changes

Add the conditional clause to the ACS-Trace bar in conformance.md

6921ac4

It now matches events.md: each conditional attribute is populated whenever its source field is present. Signed-off-by: rocklambros <rock@rockcyber.com>

rocklambros requested a review from bar-capsule October 13, 2026 13:42

rocklambros mentioned this pull request Oct 13, 2026

Fix the HMAC key derivation, define key_id, and record an unresolved request_id_ref #272

Open

12 of 18 tasks

afogel approved these changes Oct 13, 2026

View reviewed changes

rocklambros merged commit e5d4664 into integration Oct 13, 2026
11 checks passed

github-project-automation Bot moved this from In progress to Done in ACS Project Tracker Oct 13, 2026

This was referenced Oct 13, 2026

Carry the decision's replay facts and decision failures on Trace #275

Merged

Add the skill lifecycle hooks to the Trace mappings #63

Merged

This was referenced Oct 15, 2026

Release 0.1.4 #281

Merged

Promote integration to main #286

Merged

Navigation Menu

Repository navigation

Add per-hook conditional attributes to the OTel mapping - #261

Add per-hook conditional attributes to the OTel mapping#261
rocklambros merged 6 commits into
integrationfrom
spec/227-conditional-attributes

rocklambros commented Oct 12, 2026 •

edited

Loading

Uh oh!

github-actions Bot commented Oct 12, 2026

Uh oh!

bar-capsule left a comment

Uh oh!

rocklambros commented Oct 13, 2026

Uh oh!

rocklambros commented Oct 13, 2026 •

edited

Loading

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

rocklambros commented Oct 12, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What changed

Which issue does this implement

Base branch

Type of change

Specification changes

Decided by a Spec lead

Proposed by me, for a Spec lead to confirm

Reference implementation and tooling

Merge order with #244

Merging

I tested this

Checklist

Security

Uh oh!

github-actions Bot commented Oct 12, 2026

Uh oh!

bar-capsule left a comment

Choose a reason for hiding this comment

Uh oh!

rocklambros commented Oct 13, 2026

Uh oh!

rocklambros commented Oct 13, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

rocklambros commented Oct 12, 2026 •

edited

Loading

rocklambros commented Oct 13, 2026 •

edited

Loading