diff --git a/.github/workflows/reference-implementation.yml b/.github/workflows/reference-implementation.yml index 424d2a97..6222485a 100644 --- a/.github/workflows/reference-implementation.yml +++ b/.github/workflows/reference-implementation.yml @@ -10,18 +10,25 @@ # concurrency group are an integer pull request number and a ref name GitHub validates, # and a concurrency group is not a shell context. # -# Scoped by path. A documentation or schema change should not pay for a Bun install. +# Scoped by path. A documentation change should not pay for a Bun install. A schema change +# does, because this tree reads the schemas at runtime. name: Reference implementation on: pull_request: paths: - "reference-implementations/**" + # The conformance checker and the trace exporter read the schemas and + # the Trace mappings at runtime, so a spec-only change can break them. + - "specification/**" - ".github/workflows/reference-implementation.yml" push: branches: ["main", "integration"] paths: - "reference-implementations/**" + # The conformance checker and the trace exporter read the schemas and + # the Trace mappings at runtime, so a spec-only change can break them. + - "specification/**" - ".github/workflows/reference-implementation.yml" workflow_dispatch: diff --git a/docs/spec/conformance.md b/docs/spec/conformance.md index 17c56335..dd937e95 100644 --- a/docs/spec/conformance.md +++ b/docs/spec/conformance.md @@ -48,7 +48,7 @@ Who verifies a conformance claim: nobody, in v0.1.0. `profiles_supported` and `p Adds deterministic Trace event emission per [Trace Events](./trace/events.md). A deployment claiming ACS-Trace MUST: -1. Emit at least one of {OTel, OCSF} for every supported ACS step, with the required attributes populated. +1. Emit at least one of {OTel, OCSF} for every supported ACS step, with the required attributes populated and each conditional attribute populated whenever its source field is present. 2. Record decisions as Trace events. 3. Carry provenance facts forward onto Trace events. diff --git a/docs/spec/trace/events.md b/docs/spec/trace/events.md index 0894b20b..740a37e6 100644 --- a/docs/spec/trace/events.md +++ b/docs/spec/trace/events.md @@ -16,6 +16,8 @@ flowchart LR Each ACS step produces a span whose `name` and required attributes are fixed by the table below. Decisions are recorded as span events on the parent step span, not as separate spans, so the enforcement verdict and the action it gates share a parent. +An attribute marked *conditional* comes from a field the hook payload or response may omit. An exporter MUST emit it whenever the source field is present and MUST omit it when the field is absent. An empty source array counts as absent. + The full mapping lives in [`trace/otel-mapping.json`](https://genai-security-project.github.io/agent-control-standard/schema/v0.1.0/trace/otel-mapping.json). See [Extending OpenTelemetry](./extend_opentelemetry.md) for a deeper integration guide. | ACS step | OTel span name | Required attributes | @@ -24,24 +26,26 @@ The full mapping lives in [`trace/otel-mapping.json`](https://genai-security-pro | `steps/agentTrigger` | `acs.agent.trigger` | `acs.agent.id`, `acs.trigger.type` | | `steps/userMessage` | `acs.message.user` | `acs.session.id`, `acs.content.types[]` | | `steps/agentResponse` | `acs.message.agent` | `acs.session.id`, `acs.agent.id` | -| `steps/toolCallRequest` | `gen_ai.tool.call` | `gen_ai.tool.name`, `acs.capability`; `acs.tool.provider` (optional) | -| `steps/toolCallResult` | `gen_ai.tool.result` | `gen_ai.tool.name`, `acs.exit_status`; `acs.duration_ms` (optional) | -| `steps/knowledgeRetrieval` | `acs.knowledge.retrieval` | `acs.source.type`, `acs.results.count` | -| `steps/memoryStore` | `acs.memory.store` | `acs.memory_store.name`, `acs.operation` | -| `steps/memoryContextRetrieval` | `acs.memory.retrieval` | `acs.memory_store.name`, `acs.results.count` | +| `steps/toolCallRequest` | `gen_ai.tool.call` | `gen_ai.tool.name`; `acs.capability` (conditional); `acs.tool.provider` (optional) | +| `steps/toolCallResult` | `gen_ai.tool.result` | `gen_ai.tool.name`, `acs.exit_status`; `acs.request_id_ref` (conditional); `acs.duration_ms` (optional) | +| `steps/knowledgeRetrieval` | `acs.knowledge.retrieval` | `acs.results.count`; `acs.source.type` (conditional) | +| `steps/memoryStore` | `acs.memory.store` | `acs.memory_store.name`, `acs.operation` (both conditional) | +| `steps/memoryContextRetrieval` | `acs.memory.retrieval` | `acs.results.count`; `acs.memory_store.name` (conditional) | | `steps/sessionEnd` | `acs.session.end` | `acs.session.reason` | | `steps/turnStart`, `steps/turnEnd` | `acs.turn`, `acs.turn.end` | `acs.turn.id`, `acs.turn.triggered_by` / `acs.turn.outcome` | | `steps/preCompact` | `acs.compact` | `acs.compact.entry_count`, `acs.compact.triggered_by` | | `steps/postCompact` | `acs.compact.complete` | `acs.compact.entry_count`; `acs.compact.lineage_depth_after` (optional) | -| `steps/subagentStart`, `steps/subagentStop` | `acs.subagent`, `acs.subagent.end` | `acs.subagent.session_id`, `acs.subagent.parent_session_id`, `acs.subagent.intent_derivation` / `acs.subagent.outcome`, `acs.subagent.final_chain_hash` | +| `steps/subagentStart`, `steps/subagentStop` | `acs.subagent`, `acs.subagent.end` | `acs.subagent.session_id`, `acs.subagent.parent_session_id`, `acs.subagent.intent_derivation` / `acs.subagent.outcome`; `acs.subagent.final_chain_hash` (conditional) | | `steps/skillRegister` | `acs.skill.register` | `acs.skill.id`, `acs.skill.digest.algorithm`, `acs.skill.digest.value`, `acs.skill.declared_capabilities` | | `steps/skillLoad` | `acs.skill.load` | `acs.skill.id`, `acs.skill.digest.algorithm`, `acs.skill.digest.value`, `acs.skill.load_trigger`, `acs.skill.load_path` (skill ids, root first); `acs.skill.digest_verified`, `acs.skill.registration_ref` (optional) | | `steps/skillUnload` | `acs.skill.unload` | `acs.skill.id`, `acs.skill.reason`; `acs.skill.replaced_by` (optional) | -| Decision (allow/deny/modify/ask/defer) | `acs.decision` (span event) | `acs.decision`, `acs.evaluator`, `acs.reasoning` (when present), `acs.confidence` (when present) | -| `agbom/snapshot` | `acs.agbom` | `acs.agbom.format` (`canonical`/`cyclonedx`/`spdx`/`swid`), `acs.agbom.component_count` | -| `agbom/changed` | `acs.agbom` | `acs.agbom.format` (`canonical`/`cyclonedx`/`spdx`/`swid`), `acs.agbom.change_reason` | +| Decision (allow/deny/modify/ask/defer) | `acs.decision` (span event) | `acs.decision`; `acs.evaluator`, `acs.reasoning`, `acs.confidence` (conditional) | +| `agbom/snapshot` | `acs.agbom` | `acs.agbom.format`, `acs.agbom.component_count` | +| `agbom/changed` | `acs.agbom` | `acs.agbom.format`; `acs.agbom.change_reason` (conditional) | + +No AgBOM payload carries a format field, because the Observed Agent always sends the canonical form. An exporter recording the payload sets `acs.agbom.format` to `canonical`. A Guardian recording a serialization it rendered sets the value it advertises in the handshake's `agbom_serializations_supported` (`cyclonedx-1.6`, `spdx-3.0`, or `swid`). -When Provenance is attached to a hook payload, the resulting span MUST carry `acs.provenance.origin` as an attribute, and SHOULD carry `acs.provenance.source_id` and `acs.provenance.lineage_depth` when populated. v0.1 emits factual provenance attributes; trust classification is computed by the Guardian against local policy and is not a v0.1 span attribute (see [Specification §7](../instrument/specification.md#7-provenance)). Provenance lineage edges MAY be linked via OTel span links keyed by `provenance_id`. +When Provenance is attached to a hook payload, the resulting span MUST carry `acs.provenance.origin` as an attribute, and SHOULD carry `acs.provenance.source_id` when populated. v0.1 emits factual provenance attributes; trust classification is computed by the Guardian against local policy and is not a v0.1 span attribute (see [Specification §7](../instrument/specification.md#7-provenance)). Provenance lineage edges MAY be linked via OTel span links keyed by `provenance_id`. ## OCSF event classes @@ -78,8 +82,8 @@ OCSF `severity_id` for decision events is set from the disposition: A deployment claiming **ACS-Trace** MUST: -1. Emit at least one of {OTel, OCSF} for every ACS step the deployment supports, with the required attributes populated. -2. Record every decision as a Trace event carrying the disposition, evaluator identity, and reasoning when present. +1. Emit at least one of {OTel, OCSF} for every ACS step the deployment supports, with the required attributes populated and each conditional attribute populated whenever its source field is present. +2. Record every decision as a Trace event carrying the disposition, plus the evaluator identity and reasoning when the response carries them. 3. Carry provenance facts forward from the hook payload onto the Trace event so audit replay reconstructs the policy decision without consulting Guardian state separately. Trace events MUST NOT block enforcement — failure of the Trace sink MUST NOT change the disposition returned to the Observed Agent. Deployments that do not claim ACS-Trace SHOULD still emit Trace events where feasible; the vocabulary is normative regardless of profile claim. diff --git a/docs/spec/trace/extend_opentelemetry.md b/docs/spec/trace/extend_opentelemetry.md index 90c14463..f84425c7 100644 --- a/docs/spec/trace/extend_opentelemetry.md +++ b/docs/spec/trace/extend_opentelemetry.md @@ -11,13 +11,13 @@ A typical agent session produces a tree of spans: - A root **session** span (`acs.session`) opened by `steps/sessionStart` and closed by `steps/sessionEnd`. Carries `acs.session.id` and (when set) `acs.tenant_id`. - One **turn** span (`acs.turn`) per agent turn, opened by `steps/turnStart` and closed by `steps/turnEnd`. Carries `acs.turn.id`. Nested turns (e.g. subagent turns inside a parent turn awaiting `subagentStop`) record `acs.turn.parent_id`. - One **step** span per `steps/*` hook, parented to the enclosing turn (or session when the step is session-level: `agentTrigger`, `agbom/*` outside a turn). -- Each **decision** is a span event on the step span carrying `acs.decision`, `acs.evaluator` (`deterministic` / `agent` / `composite`), `acs.reasoning` (when present), and `acs.confidence` (when present). Decisions are not separate spans because the verdict and the action it gates share a parent. +- Each **decision** is a span event on the step span carrying `acs.decision`, plus `acs.evaluator` (`deterministic` / `agent` / `composite`), `acs.reasoning`, and `acs.confidence` when the response carries them. Decisions are not separate spans because the verdict and the action it gates share a parent. This hierarchy is deterministic — implementations that follow the mapping produce comparable traces across vendors and Guardians. ## Provenance attributes -When a hook payload carries Provenance, the resulting span MUST carry `acs.provenance.origin` and SHOULD carry `acs.provenance.source_id` and `acs.provenance.lineage_depth`. The set of provenance ids cited by a Guardian decision (the response envelope's `cited_provenance_ids`) MAY be expressed as OTel span links keyed by `provenance_id`, letting tools that index span links reconstruct the lineage graph without parsing payloads. +When a hook payload carries Provenance, the resulting span MUST carry `acs.provenance.origin` and SHOULD carry `acs.provenance.source_id` when populated. The set of provenance ids cited by a Guardian decision (the response envelope's `cited_provenance_ids`) MAY be expressed as OTel span links keyed by `provenance_id`, letting tools that index span links reconstruct the lineage graph without parsing payloads. v0.1 emits factual provenance attributes only. Trust classification is computed by the Guardian against local policy and is not a v0.1 span attribute. diff --git a/reference-implementations/agt/packages/conformance/README.md b/reference-implementations/agt/packages/conformance/README.md index 62087828..e6b93f3f 100644 --- a/reference-implementations/agt/packages/conformance/README.md +++ b/reference-implementations/agt/packages/conformance/README.md @@ -27,6 +27,8 @@ The runner starts its own Guardian on an OS-assigned port for the one check that **The trace-pillar rows** list, for each of the six methods in scope, which OpenTelemetry attributes a consumer could emit from the envelopes alone. This tree does not claim the Trace profile, and the rows measure that non-claim. +Each row comes from the mapping's required or conditional list for that span. ✔ means a consumer can always emit the attribute. ○ means the attribute is conditional, so the envelope may omit its source and the exporter emits it only when present. ✖ means the mapping requires an attribute the wire doesn't guarantee, which is a mapping defect. + ## The checks | Check | What it measures | diff --git a/reference-implementations/agt/packages/conformance/src/render.ts b/reference-implementations/agt/packages/conformance/src/render.ts index ccbeeeca..b8819b67 100644 --- a/reference-implementations/agt/packages/conformance/src/render.ts +++ b/reference-implementations/agt/packages/conformance/src/render.ts @@ -278,8 +278,8 @@ export function renderCoverageMatrix(cells: CoverageMatrix, options: RenderOptio } /** - * Renders `checkTracePillar`'s `TraceRow[]` -- one line per required OTel - * attribute, against its v0.1.0 wire source and whether a downstream + * Renders `checkTracePillar`'s `TraceRow[]` -- one line per required or + * conditional OTel attribute, against its v0.1.0 wire source and whether a downstream * consumer of the ACS wire (not this Guardian) could emit it from that * source alone. A * trace row pairs an attribute with a wire source; a coverage cell pairs an @@ -303,10 +303,10 @@ export function renderCoverageMatrix(cells: CoverageMatrix, options: RenderOptio * row (`acs.capability`) is a smaller gap than an attribute with no wire * source at all, and the plain/coloured split is what keeps that * distinction visible at a glance rather than collapsing both into one - * undifferentiated ✖. None of the checker's own 17 rows takes this branch + * undifferentiated ✖. None of the checker's own rows takes this branch * today (`trace-pillar.ts`'s own test suite asserts and explains why -- every - * attribute currently in scope turned out to be a declared, merely optional - * property, not an absent one), so this split is exercised by a synthetic + * attribute currently in scope is a declared property, required or + * optional, never an absent one), so this split is exercised by a synthetic * fixture rather than by `checkTracePillar()`'s real output; it stays * general-purpose code because `resolveField` can still resolve a future * attribute to genuinely absent, and this renderer should not stop telling @@ -323,7 +323,12 @@ export function renderTraceRows(rows: TraceRow[], options: RenderOptions = {}): const spanWidth = Math.max(0, ...rows.map((row) => row.span.length)); const lines = rows.map((row) => { - const symbol = row.emittableByWireConsumer ? "✔" : "✖"; + // A conditional row that is not emittable is the list working as + // defined, so it gets its own symbol. ✖ stays reserved for a required + // attribute the wire need not carry, which is a mapping defect. + // A conditional row with no wire source at all is a defect, not the + // list working as defined, so only a present-but-optional one gets ○. + const symbol = row.emittableByWireConsumer ? "✔" : row.obligation === "conditional" && row.wireSource !== null ? "○" : "✖"; const painted = paint(symbol, row.wireSource === null ? YELLOW : null, color); const source = row.wireSource ?? "(no wire source)"; const suffix = row.reason !== undefined ? ` ${row.reason}` : ""; @@ -331,13 +336,14 @@ export function renderTraceRows(rows: TraceRow[], options: RenderOptions = {}): }); return [ - "Trace pillar (trace/otel-mapping.json): each required OTel attribute against its v0.1.0 wire source, and " + + "Trace pillar (trace/otel-mapping.json): each required or conditional OTel attribute against its v0.1.0 wire source, and " + "whether a downstream consumer of the ACS wire -- not this Guardian -- could emit it from that source " + "alone. This implementation does NOT claim the Trace pillar; this table is what that declaration is " + "measured against.", "", ...lines, "", - "Legend: ✔ emittable by a wire consumer alone ✖ not emittable (reason inline)", + "Legend: ✔ emittable by a wire consumer alone ○ conditional: emitted when the envelope carries it " + + "✖ required but not emittable (reason inline)", ].join("\n"); } diff --git a/reference-implementations/agt/packages/conformance/src/trace-pillar.ts b/reference-implementations/agt/packages/conformance/src/trace-pillar.ts index 4b16deeb..61262ed5 100644 --- a/reference-implementations/agt/packages/conformance/src/trace-pillar.ts +++ b/reference-implementations/agt/packages/conformance/src/trace-pillar.ts @@ -1,7 +1,7 @@ /** * Whether a downstream consumer of the ACS v0.1.0 wire -- reading nothing - * this Guardian keeps process-local -- could emit each required Trace-pillar - * attribute `trace/otel-mapping.json` names. This is a check, not an + * this Guardian keeps process-local -- could emit each required or + * conditional Trace-pillar attribute `trace/otel-mapping.json` names. This is a check, not an * exporter: nothing here is named `exportTrace`, `traceExporter` or * `emitSpan`. Its render half, `renderTraceRows`, lives in `render.ts` -- * that file's own module header is why: `render.ts`'s only imports are @@ -11,7 +11,8 @@ * `otel-mapping.json` is a JSON **Schema**, not a data file -- its data * lives inside `default` keys, three places: * - `properties.step_to_span.default` -- an object keyed by ACS method, - * 18 entries, each `{span_name, required_attributes, optional_attributes?}`. + * 21 entries, each `{span_name, required_attributes, + * conditional_attributes?, optional_attributes?}`. * - `properties.decision_event.properties.{required_attributes, * conditional_attributes}.default` -- two string arrays. * - `properties.provenance_attributes.properties.required.default` -- one @@ -26,11 +27,12 @@ * `steps/sessionStart`, `steps/sessionEnd`, `steps/userMessage`, * `steps/agentResponse`). This implementation can only claim or decline the * Trace pillar for the methods it evaluates. `otel-mapping.json` declares - * eighteen; the other twelve (`steps/agentTrigger`, + * twenty-one; the other fifteen (`steps/agentTrigger`, * `steps/knowledgeRetrieval`, `steps/memoryStore`, * `steps/memoryContextRetrieval`, `steps/turnStart`, `steps/turnEnd`, * `steps/preCompact`, `steps/postCompact`, `steps/subagentStart`, - * `steps/subagentStop`, `agbom/snapshot`, `agbom/changed`) are out of scope + * `steps/subagentStop`, `steps/skillRegister`, `steps/skillLoad`, + * `steps/skillUnload`, `agbom/snapshot`, `agbom/changed`) are out of scope * for the same reason `mapping.yaml` never reaches them. The six are frozen * here rather than re-read from `mapping.yaml`: this module's interface * consumes nothing else, so the scope is a stated design fact (which methods @@ -49,9 +51,24 @@ * `otel-mapping.json` names `acs.capability`, not * `hooks/tool-call-request.json`'s `capability` field. * - * Decision facts (`acs.decision`, `acs.evaluator`, and the four conditional - * attributes) are recorded as a span EVENT named `acs.decision` on the - * parent step span -- not a span of their own -- so every row built from + * A step span's attributes come from two lists, read together: + * `required_attributes` (source field required on the wire) and + * `conditional_attributes` (#227: source field optional, so an exporter + * emits the attribute exactly when the payload carries it). Both are rows + * here, because both are attributes the mapping obliges an exporter to emit + * from the wire. A conditional row is expected to read "exists but is + * optional" -- that is the definition of the list -- so for it the red + * verdict is the mapping and the wire agreeing. A REQUIRED row that reads + * red is the finding: the mapping demands what the wire need not carry, + * which is how `acs.capability` read before #227 moved it. Each row + * carries the list it came from as `obligation`, so the renderer and the + * tests can tell the two cases apart rather than trusting this paragraph: + * every `required` row must read emittable, and the suite asserts it. + * `optional_attributes` names no obligation and is not read. + * + * Decision facts (`acs.decision` and the five conditional attributes) are + * recorded as a span EVENT named `acs.decision` on the parent step span -- + * not a span of their own -- so every row built from * `decision_event` carries that literal string as its `span`, matching * `otel-mapping.json`'s own `event_name` const. Provenance attributes are * plainer still: `provenance_attributes`'s own description says they land @@ -84,7 +101,7 @@ * event) is this field present and unconditionally required", walking the * FULL path including whatever optional container sits in between -- * `acs.evaluator`'s path walks through `AcsResult.metadata`, itself optional - * on `AcsResult`, and that optionality is exactly what makes the row red. + * on `AcsResult`, and that optionality is exactly what makes the row read not emittable. * Provenance has no such always-present parent in scope: nothing in the six * in-scope spans' `required_attributes` names a `provenance` field, and * whether a `Provenance` object is attached at all is a per-argument, @@ -113,9 +130,15 @@ export type TraceRow = { span: string; wireSource: string | null; emittableByWireConsumer: boolean; + /** Which mapping list named the attribute. A `required` row that is not + * emittable is a defect in the mapping. A `conditional` row that is not + * emittable is the list working as defined (module header). */ + obligation: Obligation; reason?: string; }; +export type Obligation = "required" | "conditional"; + // The repository's own schemas, five directories up from this file; see // packages/guardian/src/validate-envelope.ts for the same relationship. const SPEC_ROOT = fileURLToPath(new URL("../../../../../specification/v0.1.0/", import.meta.url)); @@ -163,7 +186,12 @@ type SchemaNode = { $defs?: Record; }; -type StepSpanEntry = { span_name: string; required_attributes: string[]; optional_attributes?: string[] }; +type StepSpanEntry = { + span_name: string; + required_attributes: string[]; + conditional_attributes?: string[]; + optional_attributes?: string[]; +}; type OtelMappingSchema = { properties: { @@ -269,7 +297,7 @@ type FieldSite = { label: string; }; -function buildRow(attribute: string, span: string, site: FieldSite): TraceRow { +function buildRow(attribute: string, span: string, obligation: Obligation, site: FieldSite): TraceRow { const { present, required } = resolveField(site.fileRoot, site.startNode, site.path); if (!present) { @@ -278,6 +306,7 @@ function buildRow(attribute: string, span: string, site: FieldSite): TraceRow { span, wireSource: null, emittableByWireConsumer: false, + obligation, reason: `${site.file} has no "${site.label}" property -- no wire source for ${attribute}`, }; } @@ -289,13 +318,14 @@ function buildRow(attribute: string, span: string, site: FieldSite): TraceRow { span, wireSource, emittableByWireConsumer: false, + obligation, reason: `${site.file}'s "${site.label}" exists but is optional there, so a conformant envelope may omit it -- ` + `a wire consumer cannot rely on ${attribute} being emitted`, }; } - return { attribute, span, wireSource, emittableByWireConsumer: true }; + return { attribute, span, wireSource, emittableByWireConsumer: true, obligation }; } export function checkTracePillar(): TraceRow[] { @@ -321,7 +351,8 @@ export function checkTracePillar(): TraceRow[] { const acsResult = responseEnvelope.$defs?.AcsResult; /** Resolves the wire site for each of the ten (span, attribute) pairs the - * six in-scope spans' `required_attributes` name. Domain knowledge -- + * six in-scope spans' `required_attributes` and `conditional_attributes` + * name. Domain knowledge -- * which file and field an OTel attribute NAME resolves to -- not * something any schema states directly, so it is written here once, by * hand, and checked by `resolveField` above rather than trusted. */ @@ -333,6 +364,8 @@ export function checkTracePillar(): TraceRow[] { return { file: "hooks/tool-call-request.json", fileRoot: toolCallRequest, startNode: toolCallRequest, path: ["capability"], label: "capability" }; case "gen_ai.tool.result::gen_ai.tool.name": return { file: "hooks/tool-call-result.json", fileRoot: toolCallResult, startNode: toolCallResult, path: ["tool", "name"], label: "tool.name" }; + case "gen_ai.tool.result::acs.request_id_ref": + return { file: "hooks/tool-call-result.json", fileRoot: toolCallResult, startNode: toolCallResult, path: ["request_id_ref"], label: "request_id_ref" }; case "gen_ai.tool.result::acs.exit_status": return { file: "hooks/tool-call-result.json", fileRoot: toolCallResult, startNode: toolCallResult, path: ["exit_status"], label: "exit_status" }; case "acs.session::acs.session.id": @@ -379,7 +412,10 @@ export function checkTracePillar(): TraceRow[] { throw new Error(`checkTracePillar: trace/otel-mapping.json's step_to_span.default has no entry for "${method}"`); } for (const attribute of entry.required_attributes) { - rows.push(buildRow(attribute, entry.span_name, stepAttributeSite(entry.span_name, attribute))); + rows.push(buildRow(attribute, entry.span_name, "required", stepAttributeSite(entry.span_name, attribute))); + } + for (const attribute of entry.conditional_attributes ?? []) { + rows.push(buildRow(attribute, entry.span_name, "conditional", stepAttributeSite(entry.span_name, attribute))); } } @@ -395,7 +431,7 @@ export function checkTracePillar(): TraceRow[] { * `AcsResult.metadata.` (verified by reading * `response-envelope.json`: `AcsResult.properties.metadata.properties` * lists all four; `AcsResult.required` and `metadata`'s own `required` - * are both silent on them, which is exactly what makes the row red under + * are both silent on them, which is exactly what makes the row read not emittable under * this module's rule -- metadata itself is optional on `AcsResult`, and * none of its own fields is required either). `acs.decision` and * `acs.reasoning` remain direct `AcsResult` siblings. A `decision_event` @@ -447,9 +483,11 @@ export function checkTracePillar(): TraceRow[] { } const decisionEvent = otelMapping.properties.decision_event.properties; - const decisionAttributes = [...decisionEvent.required_attributes.default, ...decisionEvent.conditional_attributes.default]; - for (const attribute of decisionAttributes) { - rows.push(buildRow(attribute, DECISION_SPAN, decisionAttributeSite(attribute))); + for (const attribute of decisionEvent.required_attributes.default) { + rows.push(buildRow(attribute, DECISION_SPAN, "required", decisionAttributeSite(attribute))); + } + for (const attribute of decisionEvent.conditional_attributes.default) { + rows.push(buildRow(attribute, DECISION_SPAN, "conditional", decisionAttributeSite(attribute))); } // Provenance attributes land as ordinary attributes on whichever step @@ -462,7 +500,7 @@ export function checkTracePillar(): TraceRow[] { for (const attribute of otelMapping.properties.provenance_attributes.properties.required.default) { const field = attribute.slice("acs.provenance.".length); rows.push( - buildRow(attribute, PROVENANCE_SPAN, { + buildRow(attribute, PROVENANCE_SPAN, "required", { file: "provenance.json", fileRoot: provenance, startNode: provenance, diff --git a/reference-implementations/agt/packages/conformance/test/trace-pillar.test.ts b/reference-implementations/agt/packages/conformance/test/trace-pillar.test.ts index c856d7b8..25590651 100644 --- a/reference-implementations/agt/packages/conformance/test/trace-pillar.test.ts +++ b/reference-implementations/agt/packages/conformance/test/trace-pillar.test.ts @@ -1,4 +1,6 @@ import { describe, expect, it } from "bun:test"; +import { readFileSync } from "node:fs"; +import { fileURLToPath } from "node:url"; import { loadMapping } from "guardian"; import { renderTraceRows } from "../src/render.ts"; import { checkTracePillar, METHODS_IN_SCOPE, type TraceRow } from "../src/trace-pillar.ts"; @@ -55,7 +57,7 @@ describe("checkTracePillar resolves every row against the v0.1.0 schema files at // "present but optional" case, the same as acs.capability above, not // "no such property at all". `metadata` itself is absent from // `AcsResult.required`, and `metadata`'s own schema declares no - // `required` list either, which is what makes the row red: a + // `required` list either, which is what makes the row read not emittable: a // conformant envelope may omit `metadata` entirely, or include it // without `evaluator`. const row = only("acs.evaluator"); @@ -95,11 +97,11 @@ describe("checkTracePillar resolves every row against the v0.1.0 schema files at }); it("emittableByWireConsumer is false wherever wireSource is null -- an invariant of buildRow, currently unexercised by real data", () => { - // Every one of these 17 rows resolves against a declared + // Every one of these rows resolves against a declared // response-envelope.json or provenance.json property -- v0.1.0 declares // every field otel-mapping.json names in scope here, it just does not // make most of the decision-event ones required. So `wireSource === null` - // currently holds for zero of the 17 rows -- asserted explicitly (not + // currently holds for zero of the rows -- asserted explicitly (not // merely assumed) so this test cannot pass by accident, and the // invariant stays checked because buildRow's "no such property" branch // is still real, reachable code (the renderer has its own synthetic-row @@ -124,15 +126,61 @@ describe("checkTracePillar resolves every row against the v0.1.0 schema files at } expect(spanCounts).toEqual({ "gen_ai.tool.call": 2, // steps/toolCallRequest: gen_ai.tool.name, acs.capability - "gen_ai.tool.result": 2, // steps/toolCallResult: gen_ai.tool.name, acs.exit_status + "gen_ai.tool.result": 3, // steps/toolCallResult: gen_ai.tool.name, acs.exit_status, acs.request_id_ref "acs.session": 1, // steps/sessionStart: acs.session.id "acs.session.end": 1, // steps/sessionEnd: acs.session.reason "acs.message.user": 2, // steps/userMessage: acs.session.id, acs.content.types "acs.message.agent": 2, // steps/agentResponse: acs.session.id, acs.agent.id - "acs.decision": 6, // decision_event: 2 required + 4 conditional + "acs.decision": 6, // decision_event: 1 required + 5 conditional "(every step span, when Provenance is attached)": 1, // provenance_attributes.required }); - expect(rows).toHaveLength(17); + expect(rows).toHaveLength(18); + }); + + it("reads both obligation lists for each in-scope step span -- the rows are exactly required_attributes plus conditional_attributes, and never optional_attributes", () => { + // Read straight from the mapping, so moving an attribute between lists + // (as #227 moved acs.capability) cannot silently drop its row. + type Entry = { span_name: string; required_attributes: string[]; conditional_attributes?: string[]; optional_attributes?: string[] }; + const mappingPath = fileURLToPath(new URL("../../../../../specification/v0.1.0/trace/otel-mapping.json", import.meta.url)); + const stepToSpan = (JSON.parse(readFileSync(mappingPath, "utf8")) as { properties: { step_to_span: { default: Record } } }) + .properties.step_to_span.default; + + const expected = METHODS_IN_SCOPE.flatMap((method) => { + const entry = stepToSpan[method]!; + return [...entry.required_attributes, ...(entry.conditional_attributes ?? [])].map((attribute) => `${entry.span_name}::${attribute}`); + }).sort(); + const stepSpans = new Set(METHODS_IN_SCOPE.map((method) => stepToSpan[method]!.span_name)); + const actual = rows.filter((r) => stepSpans.has(r.span)).map((r) => `${r.span}::${r.attribute}`).sort(); + expect(actual).toEqual(expected); + + // acs.capability is the case #227 is about: it now sits in the + // conditional list, and still produces its row. + expect(stepToSpan["steps/toolCallRequest"]!.conditional_attributes).toContain("acs.capability"); + expect(stepToSpan["steps/toolCallRequest"]!.required_attributes).not.toContain("acs.capability"); + for (const method of METHODS_IN_SCOPE) { + for (const attribute of stepToSpan[method]!.optional_attributes ?? []) { + expect(rows.some((r) => r.attribute === attribute && r.span === stepToSpan[method]!.span_name)).toBe(false); + } + } + }); + + it("every REQUIRED row is emittable -- a required attribute whose wire source is optional is the mapping defect #227 fixed, and must not come back", () => { + // Moving acs.evaluator or acs.capability back into a required list + // fails here, where before the obligation field it read the same as a + // correctly conditional row. + const broken = rows.filter((r) => r.obligation === "required" && !r.emittableByWireConsumer).map((r) => `${r.span}::${r.attribute}`); + expect(broken).toEqual([]); + }); + + it("every conditional row records its list, so the renderer can tell it from a required gap", () => { + expect(only("acs.capability").obligation).toBe("conditional"); + expect(only("acs.evaluator").obligation).toBe("conditional"); + expect(only("acs.request_id_ref").obligation).toBe("conditional"); + expect(only("acs.decision").obligation).toBe("required"); + const table = renderTraceRows(rows); + expect(table).toContain("○ acs.capability"); + // The legend names ✖ too, so only the row lines are checked. + expect(table.split("\n").filter((line) => line.startsWith(" ✖"))).toEqual([]); }); it("every row names a real span, never blank", () => { @@ -165,7 +213,7 @@ describe("renderTraceRows", () => { }); it("renders '(no wire source)' for a synthetic row whose wireSource is genuinely null", () => { - // None of today's real 17 rows has a null wireSource (see the check + // None of today's real rows has a null wireSource (see the check // suite's own test above), so this exercises render.ts's fallback // branch directly rather than leaving it untested by accident of // today's schema facts. @@ -174,6 +222,7 @@ describe("renderTraceRows", () => { span: "acs.decision", wireSource: null, emittableByWireConsumer: false, + obligation: "required", reason: "no wire source for acs.made_up", }; expect(renderTraceRows([synthetic])).toContain("(no wire source)"); @@ -186,14 +235,14 @@ describe("renderTraceRows", () => { }); it("RenderOptions.color is opt-in, and stripping the ANSI codes back out recovers exactly the plain render", () => { - // A fixture, not `rows`: none of today's real 17 has wireSource === + // A fixture, not `rows`: none of today's real rows has wireSource === // null (the check suite's own test above), which is the only case this // renderer colours -- against the real data, `color: true` currently // produces byte-identical output to the plain render, and this test // would pass without exercising the colour path at all. const fixture: TraceRow[] = [ - { attribute: "gen_ai.tool.name", span: "gen_ai.tool.call", wireSource: "hooks/tool-call-request.json#tool.name", emittableByWireConsumer: true }, - { attribute: "acs.made_up", span: "acs.decision", wireSource: null, emittableByWireConsumer: false, reason: "no wire source for acs.made_up" }, + { attribute: "gen_ai.tool.name", span: "gen_ai.tool.call", wireSource: "hooks/tool-call-request.json#tool.name", emittableByWireConsumer: true, obligation: "required" }, + { attribute: "acs.made_up", span: "acs.decision", wireSource: null, emittableByWireConsumer: false, obligation: "required", reason: "no wire source for acs.made_up" }, ]; const plain = renderTraceRows(fixture); @@ -206,7 +255,7 @@ describe("renderTraceRows", () => { expect(stripped).toBe(plain); }); - it("is still a pure function of the rows it is handed when colour is on -- rendering the real 17 rows twice with color:true produces the same string", () => { + it("is still a pure function of the rows it is handed when colour is on -- rendering the real rows twice with color:true produces the same string", () => { // Separate from the fixture test above: even though `color: true` // happens to be a no-op against today's real data (nothing here is // painted), purity still holds and is worth asserting against the diff --git a/specification/v0.1.0/trace/ocsf-mapping.json b/specification/v0.1.0/trace/ocsf-mapping.json index 47e3957e..eab34789 100644 --- a/specification/v0.1.0/trace/ocsf-mapping.json +++ b/specification/v0.1.0/trace/ocsf-mapping.json @@ -78,7 +78,7 @@ "optional_enrichments": { "type": "array", "items": { "type": "string" }, - "default": ["acs_provenance_source_id", "acs_provenance_lineage_depth"] + "default": ["acs_provenance_source_id"] } } } diff --git a/specification/v0.1.0/trace/otel-mapping.json b/specification/v0.1.0/trace/otel-mapping.json index d8cbfded..7bf9b0fb 100644 --- a/specification/v0.1.0/trace/otel-mapping.json +++ b/specification/v0.1.0/trace/otel-mapping.json @@ -7,14 +7,24 @@ "properties": { "step_to_span": { "type": "object", - "description": "Map ACS step methods to OTel span names + required attributes.", + "description": "Map ACS step methods to OTel span names and attributes. A required attribute's source field is required on the wire. A conditional attribute's source field is optional on the wire.", "additionalProperties": { "type": "object", "required": ["span_name", "required_attributes"], "properties": { "span_name": { "type": "string" }, "required_attributes": { "type": "array", "items": { "type": "string" } }, - "optional_attributes": { "type": "array", "items": { "type": "string" } } + "conditional_attributes": { + "type": "array", + "items": { "type": "string" }, + "description": "Attributes populated whenever the hook payload carries the source field. An exporter MUST emit each one when its source field is present and MUST omit it when the source field is absent. An empty source array counts as absent." + }, + "optional_attributes": { + "type": "array", + "items": { "type": "string" }, + "description": "Attributes an exporter MAY emit when its source field is present." + }, + "_note": { "type": "string", "description": "Explanation for readers. The normative rules are in the lists and descriptions." } } }, "default": { @@ -37,25 +47,30 @@ }, "steps/toolCallRequest": { "span_name": "gen_ai.tool.call", - "required_attributes": ["gen_ai.tool.name", "acs.capability"], + "required_attributes": ["gen_ai.tool.name"], + "conditional_attributes": ["acs.capability"], "optional_attributes": ["acs.tool.provider", "acs.tool.version", "acs.operation"] }, "steps/toolCallResult": { "span_name": "gen_ai.tool.result", "required_attributes": ["gen_ai.tool.name", "acs.exit_status"], - "optional_attributes": ["acs.duration_ms", "acs.request_id_ref"] + "conditional_attributes": ["acs.request_id_ref"], + "optional_attributes": ["acs.duration_ms"] }, "steps/knowledgeRetrieval": { "span_name": "acs.knowledge.retrieval", - "required_attributes": ["acs.source.type", "acs.results.count"] + "required_attributes": ["acs.results.count"], + "conditional_attributes": ["acs.source.type"] }, "steps/memoryStore": { "span_name": "acs.memory.store", - "required_attributes": ["acs.memory_store.name", "acs.operation"] + "required_attributes": [], + "conditional_attributes": ["acs.memory_store.name", "acs.operation"] }, "steps/memoryContextRetrieval": { "span_name": "acs.memory.retrieval", - "required_attributes": ["acs.memory_store.name", "acs.results.count"] + "required_attributes": ["acs.results.count"], + "conditional_attributes": ["acs.memory_store.name"] }, "steps/sessionEnd": { "span_name": "acs.session.end", @@ -79,7 +94,7 @@ "steps/postCompact": { "span_name": "acs.compact.complete", "required_attributes": ["acs.compact.entry_count"], - "optional_attributes": ["acs.compact.lineage_depth_after", "acs.provenance.origins_compacted"] + "optional_attributes": ["acs.compact.lineage_depth_after"] }, "steps/subagentStart": { "span_name": "acs.subagent", @@ -87,7 +102,8 @@ }, "steps/subagentStop": { "span_name": "acs.subagent.end", - "required_attributes": ["acs.subagent.session_id", "acs.subagent.outcome", "acs.subagent.final_chain_hash"] + "required_attributes": ["acs.subagent.session_id", "acs.subagent.outcome"], + "conditional_attributes": ["acs.subagent.final_chain_hash"] }, "steps/skillRegister": { "span_name": "acs.skill.register", @@ -105,34 +121,37 @@ }, "agbom/snapshot": { "span_name": "acs.agbom", - "required_attributes": ["acs.agbom.format", "acs.agbom.component_count"] + "required_attributes": ["acs.agbom.format", "acs.agbom.component_count"], + "_note": "acs.agbom.format names the AgBOM form the span records. No AgBOM payload carries a format field, because the wire form is always canonical. An exporter recording the payload sets canonical. A Guardian recording a serialization it rendered sets that serialization's agbom_serializations_supported value from the handshake." }, "agbom/changed": { "span_name": "acs.agbom", - "required_attributes": ["acs.agbom.format", "acs.agbom.change_reason"] + "required_attributes": ["acs.agbom.format"], + "conditional_attributes": ["acs.agbom.change_reason"], + "_note": "acs.agbom.format is sourced as on agbom/snapshot." } } }, "decision_event": { "type": "object", - "description": "Decisions are recorded as a span event named 'acs.decision' on the parent step span. The event MUST carry the disposition and evaluator; reasoning and confidence are required when present in the decision envelope.", + "description": "Decisions are recorded as a span event named 'acs.decision' on the parent step span. The event MUST carry the disposition. It MUST carry each conditional attribute whenever the response carries the source field, and MUST omit it when the source field is absent. An empty source array counts as absent.", "properties": { "event_name": { "type": "string", "const": "acs.decision" }, "required_attributes": { "type": "array", "items": { "type": "string" }, - "default": ["acs.decision", "acs.evaluator"] + "default": ["acs.decision"] }, "conditional_attributes": { "type": "array", "items": { "type": "string" }, - "default": ["acs.reasoning", "acs.confidence", "acs.evaluator_version", "acs.model_id"] + "default": ["acs.evaluator", "acs.reasoning", "acs.confidence", "acs.evaluator_version", "acs.model_id"] } } }, "provenance_attributes": { "type": "object", - "description": "When Provenance is attached to a hook payload, the resulting span MUST carry provenance facts as attributes. Lineage edges MAY be linked via OTel span links keyed by provenance_id. v0.1 emits the factual provenance fields (origin, source_id, lineage_depth) on the wire and on the span; trust classification is computed by the Guardian against local policy and is not a v0.1 span attribute (see §7.1).", + "description": "When Provenance is attached to a hook payload, the resulting span MUST carry provenance facts as attributes. Lineage edges MAY be linked via OTel span links keyed by provenance_id. v0.1 emits the factual provenance fields (origin, source_id) on the wire and on the span; trust classification is computed by the Guardian against local policy and is not a v0.1 span attribute (see §7.1).", "properties": { "required": { "type": "array", @@ -142,7 +161,7 @@ "optional": { "type": "array", "items": { "type": "string" }, - "default": ["acs.provenance.lineage_depth", "acs.provenance.source_id"] + "default": ["acs.provenance.source_id"] } } } diff --git a/tests/test_trace_mapping_defaults.py b/tests/test_trace_mapping_defaults.py new file mode 100644 index 00000000..a21deecc --- /dev/null +++ b/tests/test_trace_mapping_defaults.py @@ -0,0 +1,103 @@ +"""The Trace mappings keep their data in `default` keys, and nothing checked it. + +`otel-mapping.json` and `ocsf-mapping.json` are JSON Schemas whose normative +tables sit in `default`. `test_publish_schemas.py` checks each file against the +JSON Schema metaschema, which never looks inside `default`. A misspelled list +name such as `conditonal_attributes` therefore passed every check, and both +readers (the conformance checker and the trace exporter) skip keys they do not +know, so the attribute it held stopped being emitted with nothing turning red. + +These tests validate every `default` against the schema it sits in, reject +keys an entry schema does not declare, and hold each `step_to_span` entry to +one list per attribute. +""" +import json +from pathlib import Path + +import pytest +from jsonschema import Draft202012Validator + +TRACE = Path(__file__).resolve().parents[1] / "specification" / "v0.1.0" / "trace" +MAPPINGS = sorted(TRACE.glob("*.json")) +ATTRIBUTE_LISTS = ("required_attributes", "conditional_attributes", "optional_attributes") + + +def nodes_with_defaults(node, path="#"): + """Every schema node under `node` that carries a `default`, with its path.""" + if isinstance(node, dict): + if "default" in node: + yield path, node + for key, child in node.items(): + if key != "default": + yield from nodes_with_defaults(child, f"{path}/{key}") + + +@pytest.mark.parametrize("path", MAPPINGS, ids=lambda p: p.name) +def test_every_default_validates_against_its_own_schema(path): + mapping = json.loads(path.read_text(encoding="utf-8")) + checked = 0 + for pointer, node in nodes_with_defaults(mapping): + schema = {key: value for key, value in node.items() if key != "default"} + errors = sorted(Draft202012Validator(schema).iter_errors(node["default"]), key=str) + assert not errors, f"{path.name} {pointer}: " + "; ".join(error.message for error in errors) + checked += 1 + # A refactor that moves the tables out of `default` would otherwise turn + # this into a test of nothing. + assert checked >= 4, f"{path.name}: found only {checked} defaults to check" + + +def test_every_step_span_entry_uses_only_known_keys(): + # Checked here, not by closing the published schema with + # additionalProperties: false, which would change what a deployment may + # publish. A misspelled list name fails this instead. + mapping = json.loads((TRACE / "otel-mapping.json").read_text(encoding="utf-8")) + entry_schema = mapping["properties"]["step_to_span"]["additionalProperties"] + known = set(entry_schema["properties"]) + unknown = { + method: sorted(set(entry) - known) + for method, entry in mapping["properties"]["step_to_span"]["default"].items() + if set(entry) - known + } + assert not unknown, f"step_to_span entries with keys the entry schema does not declare: {unknown}" + + +def test_no_attribute_sits_in_two_lists_of_one_step_span(): + mapping = json.loads((TRACE / "otel-mapping.json").read_text(encoding="utf-8")) + overlaps = {} + for method, entry in mapping["properties"]["step_to_span"]["default"].items(): + seen = {} + for name in ATTRIBUTE_LISTS: + for attribute in entry.get(name, []): + if attribute in seen: + overlaps.setdefault(method, []).append(f"{attribute} in {seen[attribute]} and {name}") + seen[attribute] = name + assert not overlaps, f"attributes listed twice: {overlaps}" + + +# Every conditional attribute and the wire field it comes from. The +# conformance checker's "every required row is emittable" test covers only the +# six methods the reference Guardian evaluates, so this one holds the other +# fifteen: moving an attribute back to required, or a schema making its +# source required, fails here. +CONDITIONAL_SOURCES = { + ("steps/toolCallRequest", "acs.capability"): ("tool-call-request.json", "capability"), + ("steps/toolCallResult", "acs.request_id_ref"): ("tool-call-result.json", "request_id_ref"), + ("steps/knowledgeRetrieval", "acs.source.type"): ("knowledge-retrieval.json", "source"), + ("steps/memoryStore", "acs.memory_store.name"): ("memory-store.json", "memory_store"), + ("steps/memoryStore", "acs.operation"): ("memory-store.json", "operation"), + ("steps/memoryContextRetrieval", "acs.memory_store.name"): ("memory-context-retrieval.json", "memory_store"), + ("steps/subagentStop", "acs.subagent.final_chain_hash"): ("subagent-stop.json", "final_chain_hash"), + ("agbom/changed", "acs.agbom.change_reason"): ("agbom-changed.json", "reason"), +} + + +def test_every_conditional_attribute_stays_conditional_and_its_source_stays_optional(): + table = json.loads((TRACE / "otel-mapping.json").read_text(encoding="utf-8"))["properties"]["step_to_span"]["default"] + listed = {(method, attribute) for method, entry in table.items() for attribute in entry.get("conditional_attributes", [])} + assert listed == set(CONDITIONAL_SOURCES), f"conditional lists drifted: {sorted(listed ^ set(CONDITIONAL_SOURCES))}" + for (method, attribute), (hook, field) in CONDITIONAL_SOURCES.items(): + payload = json.loads((TRACE.parent / "hooks" / hook).read_text(encoding="utf-8")) + assert field in payload["properties"], f"{hook} has no {field}, so {attribute} on {method} has no source" + assert field not in payload.get("required", []), ( + f"{hook} now requires {field}, so {attribute} on {method} belongs in required_attributes" + )