Skip to content

Add the skill lifecycle hooks to the Trace mappings - #63

Open
sabasiddique1 wants to merge 3 commits into
GenAI-Security-Project:mainfrom
sabasiddique1:fix/skill-hooks-trace-mappings
Open

Add the skill lifecycle hooks to the Trace mappings#63
sabasiddique1 wants to merge 3 commits into
GenAI-Security-Project:mainfrom
sabasiddique1:fix/skill-hooks-trace-mappings

Conversation

@sabasiddique1

@sabasiddique1 sabasiddique1 commented Sep 9, 2026

Copy link
Copy Markdown

What changed

skillRegister, skillLoad, and skillUnload had no entry in otel-mapping.json or ocsf-mapping.json, so a deployment claiming ACS-Trace had no normative span name or class_uid for a skill event. This adds both mappings, renders them on the Trace Events and Extending OCSF pages, corrects docs/acs.md from 16 to 19 hooks, and widens tests/test_hook_taxonomy.py to cover every surface that restates the hook set. Closes #57, closes #58.

Type of change

  • Specification change (schema, hooks, events, AgBOM)
  • 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

Breaking for implementers? No. Nothing on the wire changes. Three new keys appear in each mapping's default; existing keys, span names, and class assignments are untouched.

OTel: acs.skill.register, acs.skill.load, acs.skill.unload. Required attributes come from each schema's required list. acs.skill.load_path stays required because cascade containment is the reason the hook carries it.

OCSF, reviewer decision: skillRegister and skillUnload are Inventory Info (5001), the class agbom/* uses, since they admit a skill to or remove it from the component set and the unload schema already allows folding into agbom/changed. skillLoad is Process Activity (1007), sibling of toolCallRequest. A denied skillRegister still surfaces as Detection Finding 2004 through decision_class. The alternative is 1005 Module Activity (Load=1, Unload=2), which fits load and unload directly and reuses the activity_id pattern used for 3002.

Guard

The first commit widens the test and fails on exactly the surfaces #58 lists, plus one it did not: the class table in extend_ocsf.md never listed steps/agentTrigger. That row is fixed here too.

Not changed

The AgBOM trigger list in docs/spec/inspect/README.md omits skill; separate issue. The "Sixteen" in specification/proposals/skill-lifecycle/README.md describes the pre-skill state and is correct in context. Legacy example content in docs/acs.md and OCSF/implementation_examples.md is out of scope.

Checklist

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

Security

  • This change has no security impact

Full suite: 203 passed, 1 skipped.

Signed-off-by: Saba Siddique <sabasiddiqdev@gmail.com>
Signed-off-by: Saba Siddique <sabasiddiqdev@gmail.com>
@sabasiddique1

sabasiddique1 commented Sep 9, 2026

Copy link
Copy Markdown
Author

@rocklambros and @afogel

Please guide on the following

  1. OCSF class for the skill hooks. I went with skillRegister/skillUnload → 5001 Inventory Info (the skill enters or leaves the agent's component set, and the register schema says definition.ref/digest persist to the AgBOM — same class as agbom/*) and skillLoad → 1007 Process Activity (digest-verified load into execution, sibling of toolCallRequest). The closer semantic match for load/unload is 1005 Module Activity (Load=1 / Unload=2), but that introduces a class the mapping doesn't use today. Which do you want? A denied skillRegister surfaces as Detection Finding 2004 via decision_class either way.

  2. RESTATING_THRESHOLD = 8. A file counts as "restating the hook set" when it references 8+ distinct hooks via steps/. Today the tables reference 15–16 and nothing else exceeds 4, so 8 sits in the middle. Happy to change it or make it explicit-list-based if you'd rather.

  3. agentTrigger in extend_ocsf.md. The widened guard caught this as a seventh gap; I added it to the Application Activity row. Shout if you'd prefer it split out.

  4. OCSF 1.5 naming. 5001 is "Device Inventory Info" in OCSF 1.5 (5020 is "Software Inventory Info"). The repo's "Inventory Info" label predates this PR and I left it alone. Separate PR, or leave it?

  5. Follow-up: AgBOM update triggers. While sweeping for surfaces that omit the skill hooks, I found the Inspect overview's AgBOM update-trigger list covers seven component types and skips skill, even though the same page defines it. Out of scope here. Issue creation is restricted in this repo — if you open one I'll take it, or I can send the PR directly if you'd rather skip the issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: In progress

2 participants