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.

@rocklambros rocklambros moved this to In progress in ACS Project Tracker Sep 10, 2026
rocklambros added a commit that referenced this pull request Sep 10, 2026
Operationalizes the contribution governance the core team agreed on
September 8, filtered through the Strategic Adoption Plan v3 committed
outcome.

Design: `design/2026-09-09-contribution-governance-design.md` (v1.1)
Plan: `design/plans/2026-09-09-contribution-governance.md`

## What this installs

**Current Priority Scope.** `CONTRIBUTING.md` gains one section that
states what the project is driving at for the next ninety days, what is
deferred to v0.2.0, and what is out of scope by design. It is stated
once. Every other surface links to it rather than restating it, and the
landing page now renders it straight out of `CONTRIBUTING.md` so the two
cannot drift.

**Branching.** `main` publishes the site and all 44 schema `$id` URIs on
merge, so specification and code land on `integration` and publish on a
deliberate promotion. A guard (`tools/base_branch_guard.py`, 16 tests)
enforces a positive path allowlist and fails closed on anything nobody
anticipated. The promotion exemption checks the head *repository*, not
just the ref, so a fork branch named `integration` cannot walk a schema
change onto the publishing branch.

**Intake.** Blank issues are off. Six forms route by type, and no form
can stamp a `scope:`, `priority:`, `workstream:`, or `status:accepted`
label. That prohibition is the whole structural guarantee behind
maintainer-only triage. The sixth form asks for nothing but a
description, because the strongest outside contribution this project has
received would have fit none of the other five.

**The gate.** A change to behavior, normative text, or code references
an issue carrying `status:accepted`. An editorial correction does not,
wherever it lands. A pull request whose issue is not accepted yet is
neither closed nor reviewed. It waits, and a bot says why.

**Automation.** Four workflows: the base-branch guard, an integration
sync, a weekly promotion pull request, and the intake comment. Plus a
reminder that opens an issue when the priority scope passes its review
date.

**Phase 2 executor.** `tools/apply_governance.py` performs the live
migration steps idempotently, `--dry-run` by default. It is structurally
incapable of merging, closing, or retargeting a pull request, and a test
asserts no code path can emit those commands.

## Authorship policy change

The rule that a maintainer strips a `Co-Authored-By` trailer naming a
model is removed. The project now mandates neither direction. The human
`Signed-off-by` stays required, because only a person can make the DCO
certification. A standard about agent provenance should not erase the
provenance of its own commits.

## Reviewer notes

- Nothing on the published site changes except the landing page's
Contribute section. I built the full pipeline from `main` and from this
branch and diffed: the only files that differ are `index.html` and
`acs.css`. All 44 schemas are byte identical.
- 253 tests pass. `mkdocs build --strict` passes.
- This targets `integration` rather than `main` because it touches
`.github/`, `tools/`, and `tests/`, which is exactly what the guard it
installs requires.

## Still to do after this merges

Retarget #63, #24, #60, and #22 to `integration`. Merge #21, #20, and
#22 when the team is ready. Then run the executor for rulesets, default
branch, required check, and the seeded issues. The order that matters is
in the plan.

---------

Signed-off-by: rocklambros <rock@rockcyber.com>
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