Skip to content

[AAASM-5750] ✨ (gateway): Forward the hook-layer audit event to the runtime - #370

Merged
Chisanan232 merged 15 commits into
mainfrom
v0.0.1/AAASM-5750/feat/interceptor_audit_sink
Aug 14, 2026
Merged

[AAASM-5750] ✨ (gateway): Forward the hook-layer audit event to the runtime#370
Chisanan232 merged 15 commits into
mainfrom
v0.0.1/AAASM-5750/feat/interceptor_audit_sink

Conversation

@Chisanan232

@Chisanan232 Chisanan232 commented Aug 14, 2026

Copy link
Copy Markdown
Contributor

Target

  • Task summary:

    createNativeGatewayClient's record / recordResult / scanPrompts constructed the audit event, handed it over, and dropped it — firing only a one-time AA_DEBUG note. They now send it on nativeClient.sendEvent, the same primitive and the same connected session that already carries the boot registration event (init-assembly.ts:831), so a governed tool call — allowed or denied — reaches the runtime's audit pipeline.

  • Task tickets:

  • Key point change (optional):

    Design, and the code that decided it. The AAASM-4847 comment said hook-layer events "have nowhere to go". That was true of the gateway wire — there is still no HTTP audit route for the SDK to POST to — and false of the runtime channel, which was already open and already carrying an event. native/aa-ffi-node/src/lib.rs:152 (send_eventAssemblyClient::report_event) reaches the runtime pipeline, which enriches, re-scans and admits the frame to its audit pipeline.

    auditSink is computed, not fixed. createNativeGatewayClient is also handed the fallback stub createNativeClient returns when the native binding cannot be loaded, whose sendEvent is a no-op. Declaring "forwarded" there would be a claim about a channel the client does not hold, so the value keys off canRegister — which is exactly "the binding loaded", the same fact the record path depends on.

    §6 term earned. napi-inprocess over a loaded binding → Observed (forwarded). The no-op client (auto / sdk-only / grpc-sidecar) holds no transport at all → discardedUnsupported; that gap is structural, not unwired. A native client over the fallback stub → discardedDegraded.

    Not changed: no redaction is attempted on this side — the runtime's scanner is the unconditional gate on every inbound frame, and a weaker copy here would only invite the two to disagree.

Effecting Scope

  • Action Types:
    • ✨ Adding new something
      • 🟢 No breaking change
    • ✏️ Modifying existing something
      • 🟢 No breaking change
  • Scopes:
    • 🧩 SDK public API
    • 🪡 API client and transport
    • 🫀 Data model and types
    • 🧪 Testing
      • 🧪 Unit testing
    • 📚 Documentation
  • Additional description:
    AuditSinkDisposition gains "forwarded". The union is a public export, so a consumer exhaustively switching on it must add a branch; the two existing values keep their meanings.

Description

  • src/types/gateway-governance.ts — add "forwarded"; map each value onto the §6 term it earns.
  • src/gateway/client.ts — send the three audit methods over sendEvent; compute auditSink from canRegister; add stringifyForAudit so a circular or BigInt tool result degrades to a lossy record rather than throwing out of the sink into the governed call.
  • src/core/init-assembly.ts — the warning fires for "discarded" only, and says what to do about it.
  • Tests, docs, and every remaining prose claim that both shipped clients drop the event.

Validation

pnpm typecheck, pnpm lint (0 issues), pnpm test (730 passed, 2 skipped, 73 files), pnpm build — all green.

Downstream-boundary stub with a reachability control (AC2). tests/audit-sink-disposition.test.ts's boundary() is the NativeClient seam, not a stand-in for the code under test. The positive control asserts a queryPolicy crossing; the finding asserts three distinct sendEvent crossings carrying three distinct payloads. A new case drives a real deny through withAssembly — the shipped interceptor, no injected sink — and asserts the deny reason crosses the same boundary.

Proven able to fail (AC3). Reverting the three sinks to async () => undefined:

1. a client declaring "forwarded" reaches the boundary with every audit method
   AssertionError: expected [] to have a length of 3 but got +0
2. a governed tool call through withAssembly forwards the deny to the boundary
   AssertionError: expected false to be true
3. sends all three audit methods to the native event channel
   AssertionError: expected [] to deeply equal [ 'tool_call_audit', …(2) ]
4. does not let a non-serializable tool result fail the audit call
   AssertionError: expected "vi.fn()" to be called 1 times, but got 0 times

Quote lockstep is load-bearing. Rewording docs/02-quick-start/index.md alone, leaving quote= untouched, reddens no sentence is unbound and unallowed and … matches exactly one whole sentence.

tests/napi-audit-noop.test.ts is kept rather than deleted: it now asserts the inversion of what it used to demand, so a revert to the no-op reddens the exact file that once required it and the AA_DEBUG note cannot quietly return.

Known behaviour worth a reviewer's eye

nativeClient.sendEvent never throws — the transport stashes a failure and surfaces it on the next queryPolicy. That is what makes these safe to call from the hook layer, but it means a broken IPC channel can deny a later call under enforce. That is the correct fail-closed reading of an unreachable runtime rather than an audit-driven enforcement change, and it is pre-existing behaviour that registration already had.

🤖 Generated with Claude Code

https://claude.ai/code/session_01XWLmA8FgULT9e6ntdCo1H2

The union had no value for a client that carries the event onward, so wiring
one would have had to declare a retention the SDK cannot observe or a drop
that is no longer true. Adds it and maps each value onto the ADR 0033
section 6 term it earns.

Refs AAASM-5750
record / recordResult / scanPrompts constructed the event and dropped it,
firing only an AA_DEBUG note. They now send on nativeClient.sendEvent — the
same primitive and connected session that already carries the boot
registration event — so a governed call, allowed or denied, reaches the
runtime's audit pipeline. auditSink is computed from canRegister so a client
built over the fallback stub still declares honestly.

Refs AAASM-5750
The init warning, the context field doc and withAssembly's deny note all
said both shipped clients drop the event. State what the event's fate now
depends on, and name the runtime-absent case rather than dropping it.

Refs AAASM-5750
Inverts both suites over the same boundary stub and the same positive
control: the shipped native client must now cross it with each audit method,
and a governed deny through withAssembly must too. Adds the fallback-stub
input that shows the computed disposition moving, and keeps the superseded
AAASM-4847 file so a revert to the no-op reddens the tests that demanded it.

Refs AAASM-5750
5750 built the sink, so a claim still calling recording Planned under it
describes shipped behaviour as unbuilt. Collapses the guarded-site tier into
one repo-wide rule and adds a positive control over synthetic input, since a
rule expecting no findings is green when the scan is broken.

Refs AAASM-5750
Updates the bound sentence and its quote= in lockstep, and repoints the
controls at the boundary suite plus the branch that produces no evidence, so
both halves of the sentence are decided by something.

Refs AAASM-5750
Four sites across the introduction and the README asserted that governed
actions leave nothing behind on every path. Each now states which client
forwards and which drops, and names the disposition to read.

Refs AAASM-5750
…walk

readdirSync followed by a separate statSync leaves a window in which an entry
can be removed between the two, and the throw takes the whole scan down —
turning a gate whose verdict is "no findings" into one that produced no
verdict. CI hit it on pnpm's transient .tmp-postinstall directory. Reads the
type from the directory entry instead, and skips a file removed before it
could be read.

Refs AAASM-5750
@codecov

codecov Bot commented Aug 14, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

The payload's shape is what a reader downstream distinguishes on, and only one
side of each optional spread was exercised — a spread that always fired and
one that never fired look identical under that. Also covers the case where
JSON.stringify returns undefined rather than throwing, which would otherwise
put a recorded outcome on the wire as a missing field.

Refs AAASM-5750
The first sweep enumerated by phrase and missed sites whose false clause never
used the word "audit": the context field's per-framework Observed note, the
guides capability table and its footnote, a negative control's scope comment,
and a claim binding whose id asserted the opposite of the quote it guards.

Refs AAASM-5750
@Chisanan232

Copy link
Copy Markdown
Contributor Author

Second sweep — the first one's method was wrong, and it missed five sites.

The first pass enumerated by phrase. An independent audit against what each sentence asserts found five more, committed in 1cf65202b:

  • src/types/assembly-context.ts:21-25 — the per-framework §6 note still said the callback handler reaches Observed "only when the gateway client actually persists what it records, which the default no-op client and the napi-inprocess client both do not". The napi client now does.
  • docs/04-guides/index.md:164 and its ** footnote at :175-177 — the capability table told the reader a sink is caller-supplied-only. The two had to move together; correcting the footnote alone would have left the row pointing at it.
  • tests/quickstart-negative-control.test.ts:265-269 — a scope comment asserting a deny is Unmeasured on the shipped path.
  • tests/quickstart-claim-bindings.test.ts:154 — the binding id was allow-event-is-discarded-not-retained, i.e. the gate reported under a label asserting the opposite of the quote two lines below it, which had already been corrected.

Two residuals I looked at and deliberately did not change:

  1. tests/napi-audit-noop.test.ts — the filename still asserts the sinks are no-ops. The header supersedes it explicitly, every assertion in the file is the inversion, and tests/audit-sink-disposition.test.ts:53 refers to it by that name. Renaming is cosmetic and would touch a cross-reference; happy to do it if a reviewer prefers.
  2. dist/** carries compiled copies of the src/ docstrings and is untracked build output — it corrects itself on the next build.

Also in 1cf65202b: codecov/patch was red at 76.92% against a 93.5% target. The uncovered lines were both sides of each optional field in the audit payloads and the JSON.stringify(...) ?? String(...) branch. Those are real wire-shape decisions rather than coverage padding — a spread that always fires and one that never fires are indistinguishable if only one side is driven, and JSON.stringify returning undefined (a function, a symbol, undefined itself) would otherwise put a recorded outcome on the wire as a missing field. src/gateway/client.ts is now 100% statements / 97.5% branches; the one remaining uncovered branch (:79) predates this PR.

stringifyForAudit fell back to String(value) when JSON.stringify threw, but
String() throws too — an object from Object.create(null) has no primitive
conversion, and querystring.parse() returns exactly those. The throw escapes:
AssemblyCallbackHandler.handleToolEnd awaits the sink with no try/catch and
@langchain/core awaits that inside the tool invocation, so the user's tool
call rejects. Guards the second conversion and falls back to a constant.

Refs AAASM-5750
sendEvent returns void, is unacknowledged, and never throws, so a client
whose connect never succeeds still reports "forwarded". The SDK cannot see
arrival, let alone retention, and ADR 0033 section 6 Observed needs a durable
event attributed to the action. States the handoff and nothing beyond it, and
records that the discarded arm is unreachable through initAssembly.

Refs AAASM-5750
…the regex

The three positive controls drove deferralsInLines on synthetic strings, so
mutating scan() to return immediately left all five tests green — detector
reachability and walk reachability are different claims, and only the second
makes the repo-wide "no findings" mean anything. Threads a file count out of
the walk and pins a file that only exists here.

Refs AAASM-5750
…true

Five user-facing sites promised the record arrived. It is written to the
runtime's event channel and unacknowledged from there, so the docs now say
handoff and say plainly it is not an audit guarantee.

Refs AAASM-5750
@Chisanan232

Copy link
Copy Markdown
Contributor Author

Review round 2 — regression fixed, Observed withdrawn. Head 7b4d68cf0.

1. The regression, fixed first. stringifyForAudit fell back to String(value), and String() throws too — an Object.create(null) object has no Symbol.toPrimitive / valueOf / toString, and querystring.parse() returns exactly those. Confirmed both conversions throw, then reproduced the review's matched control against built output:

BEFORE   native client, circular null-prototype:  REJECTED -> TypeError: Cannot convert object to primitive value
         no-op  client, same input:               ok            <- control

AFTER    native client, circular null-prototype:  ok
         no-op  client, same input:               ok            <- control

The second conversion now has its own guard and falls back to "[unserializable]". The new case asserts as a precondition that both JSON.stringify and String throw on the input, so it cannot pass over a value that was never dangerous.

2. Observed withdrawn — seven sites. You were right that gateway-governance.ts:32-33 was the sharpest: "This SDK observes that the event crossed the boundary" is false as written. sendEvent returns void, is fire-and-forget, and never throws — the transport stashes failures for a later queryPolicy — so a client whose connect always rejects still reports forwarded. Every site now claims the handoff and nothing past it, and the type doc says up front that no value here earns §6 Observed.

I did not substitute a different §6 term. The honest position is that this layer makes no §6 recording claim at all; the enforcement claims (Evaluated / Denied before execution) are untouched. I have not referenced the durability defect by ticket since you are filing it — say the word and I will add the reference.

3. The Degraded row is gone. Verified your call graph: napi-inprocess throws on a binding load failure (createNativeClientloadNativeBinding), and every other mode returns the no-op client without reaching createNativeGatewayClient. So the stub can never arrive there via initAssembly. Reduced to Observed-free forwarded / discarded(Unsupported), and the discarded arm is now documented as reachable only by a caller constructing the exported factory directly — which is true, and is why the guard stays.

4. The referent gate discriminated nothing — fixed and proven. Your mutation reproduced exactly: scan()return; left all 5 green. Threaded a file count out of the walk and pinned a file that only exists in this repo. Re-running the same mutation now:

MUTATION scan() returns immediately:  PASS (5)  FAIL (1)
  1. the WALK reaches this repository's files — not just the detector

5. Count corrected. The suite is 740 (738 passed, 2 skipped) at this head — the 730 in the body was stale before I added tests, and 732 was your reading of the same stale moment. src/gateway/client.ts holds 100% statements / 97.5% branches; the one uncovered branch (:79) predates this PR.

Gates: typecheck, lint, test, build, test:coverage all green.

The gap was described in seven places with no referent, which reads as an
explained limitation rather than a tracked one. Points at what 5783 will
change — report_event payloads reaching the live stream and the durable entry
— and says the terms here are revisited when it lands, not before.

The quick-start sentence gets its own binding with an unprovenReason rather
than a control: the behaviour it points at lives in aa-sdk-client and
aa-runtime, outside this package's test boundary.

Refs AAASM-5750, AAASM-5783
@sonarqubecloud

Copy link
Copy Markdown

@Chisanan232
Chisanan232 merged commit 21fe924 into main Aug 14, 2026
27 checks passed
@Chisanan232
Chisanan232 deleted the v0.0.1/AAASM-5750/feat/interceptor_audit_sink branch August 14, 2026 06:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant