Add Audit Event

Add a semantic audit event (agent-manager-service)

Read first: agent-manager-service/AGENTS.md → "Audit logging", and agent-manager-service/docs/audit-logging.md for the full design.

First: do you need this at all?

Every route registered through RouteRegistrar, and every MCP tool registered through addTool, is already audited. That record names the actor, org, action, resource, outcome and source. Most changes need nothing.

You need this skill only for one of these:

The test is whether the envelope answers the forensic question. POST .../permissions/add → 200 does not say which permission was granted, so it needs a semantic event. PUT .../agents/{name} returning 200 is self-describing, so it does not.

Step 1 — name the action

Actions read <resource>:<verb>. Reuse an existing constant if one fits; add to audit/actions_domain.go if not.

The same action must be used by every surface that performs the operation — REST, MCP, internal, background. TestDomainActionsMatchRouteDerivedActions fails the build if a semantic emit and its route disagree, because a split action silently halves every query.

If the route derives the wrong label, add one line to actionOverrides in audit/policy.go, keyed by the exact registrar pattern:

"POST /orgs/{orgName}/identities/roles/{roleID}/permissions/add": "role:grant-permission",

Step 2 — register class, severity and detail schema together

All three go in audit/actions_domain.go, in the same place, so they cannot drift apart:

const ActionThingRotate Action = "thing:rotate"

// inside init()
registerCredential(ActionThingRotate, map[string]FieldKind{
    "ownerName": KindName,
    "keyName":   KindName,
})

Helpers: registerCredential (credential class, always critical) and registerIdentity (identity class, always critical). Otherwise call Register(action, class, severity) and RegisterDetailSchema(action, fields) directly.

A registered action with no detail schema fails TestRegisteredActionsHaveDetailSchemas. An empty map is a valid answer; skipping the decision is not.

Step 3 — choose fail-open or fail-closed

This is the decision that matters. Get it from the consequence, not the convenience.

Fail-closed — audit.Begin / attempt.Complete. Use when a live credential or a privilege change would otherwise exist with no trace of who caused it. The intent record is written before the change; if that write fails, the operation is refused.

attempt, err := audit.Begin(ctx, audit.ActionThingRotate,
    audit.Org(ouID),
    audit.ResourceNamed(audit.ResourceAPIKey, ownerID, keyName),
    audit.Project(projName),
    audit.Environment(envID),
    audit.Detail("ownerType", audit.APIKeyOwnerAgent),
    audit.Detail("keyName", keyName),
)
if err != nil {
    return nil, err // do NOT perform the operation
}

resp, err := s.doTheThing(ctx, ...)
attempt.Complete(ctx, err)
return resp, err

A record left at outcome: "unknown" means the process died mid-operation. That orphan is deliberate forensic signal, not a defect.

Fail-open — audit.Record. Use for frequent operations that issue no credential: builds, console test keys, gateway configuration, monitor lifecycle. Blocking CI on the audit path costs more than it protects.

err := s.doTheThing(ctx, ...)
audit.Record(ctx, audit.ActionThingUpdate,
    audit.Org(ouID),
    audit.ResourceNamed("thing", id, name),
    audit.Result(err),
)

audit.RecordAncillary — for a fact about how a request was handled (an authorization bypass, a rate-limited rejection) rather than what it did. Unlike Record, it does not suppress the coverage-tier record, so you keep both.

Emit from the service, not the controller. Controllers handle HTTP; a fail-closed emit is domain behaviour. The one exception is the identity surface, which has no service layer at all (see the documented exception in agent-manager-service/AGENTS.md → Layering) — do not treat it as precedent for new code.

In a controller, use beginAuditOrFail(w, r, operation, failureMessage, action, opts...): it writes the 503 and logs for you, so the refusal cannot be forgotten or answered with the wrong status.

Step 4 — what may go in the record

• Never a request or response body. Bodies are structurally out of reach; that is what makes auditing every route safe. Name the fields you want instead.
• audit.Detail records string, bool, int, int32, int64, float64, []string and fmt.Stringer. Anything else becomes a [unsupported:<type>] marker instead of being serialised. Pass the field you mean rather than relying on that.
• Never a secret value. Use audit.SecretRef(key, value) (SHA-256 prefix + last four) or record the key name.
• Free-form maps: keys only. Use audit.AttributeKeySummary(attrs) — it returns sorted key names, a count, and a flag when a key looks credential-shaped. Never the values.
• URLs: declare them KindURL. Pass the URL as-is; the kind is what strips userinfo, query and fragment at redaction, so a token in ?access_token= or an https://user:pass@host/ cannot reach a record. A test fails if a detail whose name ends in uri, url or endpoint is declared as anything else.
• Identifiers and scope strings are fine, and usually the point. role:grant-permission records the granted scopes in full; they are identifiers, not credentials.
• Keys not in the action's schema are dropped and reported under _droppedKeys.

Step 5 — test it

Services that fail closed refuse to run without a recorder, so a bare context.Background() makes them fail by design (audit: recorder unavailable).

// Exercising the operation: install a discarding recorder.
resp, err := svc.RotateThing(auditableCtx(t), ...)

// Asserting the refusal itself: bare context.
_, err := svc.RotateThing(context.Background(), ...)
require.ErrorIs(t, err, audit.ErrRecorderUnavailable)

auditableCtx(t) lives in services/audit_testing_test.go. Do not redeclare it.

To assert on the records themselves, write the test inside the audit package — audit.NewMemorySink() is a test double declared in audit/sink_doubles_test.go and is not importable from services/ or controllers/. See audit/actions_domain_test.go. From another package, assert the behaviour (did the operation proceed or refuse) rather than the record.

Done checklist

• Action reuses the constant its route derives (or actionOverrides makes them agree).
• Class, severity and detail schema registered together in audit/actions_domain.go.
• Fail-open vs fail-closed chosen from the consequence, and a fail-closed site aborts on error.
• No body, no secret value, no struct or map in audit.Detail.
• Tests exercising the path use auditableCtx(t).
• make test-unit passes — including TestDomainActionsMatchRouteDerivedActions, TestRegisteredActionsHaveDetailSchemas and TestEveryMutatingRouteIsAudited.
• docs/audit-logging.md semantic-event table updated if you added an action.
• CI lint clean: golangci-lint run --config .github/linters/.golangci.yaml ./...