Skip to content

[AAASM-5529] ✅ (tests): Add enforcement-truth negative controls to the quick-start - #352

Merged
Chisanan232 merged 9 commits into
mainfrom
v0.0.1-rc.7/AAASM-5529/enforcement_negative_controls
Aug 6, 2026
Merged

[AAASM-5529] ✅ (tests): Add enforcement-truth negative controls to the quick-start#352
Chisanan232 merged 9 commits into
mainfrom
v0.0.1-rc.7/AAASM-5529/enforcement_negative_controls

Conversation

@Chisanan232

@Chisanan232 Chisanan232 commented Aug 6, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Target

  • Task summary:

    Add enforcement-truth negative controls to the documented Node quick-start (AAASM-5529, Epic AAASM-5526, Goal CBLPCRLM-13).

    A negative control must prove a denial prevented a side effect, not that a PolicyViolationError was thrown. All 62 existing not.toHaveBeenCalled() assertions in tests/ prove the SDK did not call a reference it holds — not that the effect the tool exists to produce was prevented, and not that the tool was capable of producing one at all.

    Each control drives withAssembly — the enforcement point the quick-start documents (docs/02-quick-start/index.md:105-114) — over a tool with a real, externally-observable effect (a file on disk, an HTTP request delivered to a live loopback listener), and asserts the deny as the absence of that effect, paired with a positive control that observes it. The side-effect assertion runs before the error assertion so that removing enforcement fails the suite on the absence check, not on "no error was thrown".

  • Task tickets:

  • Key point change:

    Falsification evidence

    Correction (post-review). The falsification block previously published in this PR was wrong, and the mistake was visible inside it. It listed three failing controls but printed an AssertionError under only two of them; the third — records the same agent id, tool name and run id ... — had no failure message beneath it at all. That silence was the defect. That control failed on expected '/var/folders/…' to be an instance of PolicyViolationError — on the missing exception, aborting before its effect.occurred() check was ever reached. The sentence that followed the block, "Every negative control fails on the side-effect assertion", was therefore false when written: two of three did, and the third had never been shown to bite. The block was also stale — it reported (8) tests, from a revision predating the current 10. Both defects are fixed in 3b08bdf, and the evidence below is now reproduced in full rather than elided.

    Produced by disabling the deny in the SDK (not in the tests) and re-running. vitest resolves src/, not dist/, so the mutation bites without a rebuild:

    # src/wrappers/with-assembly.ts:164
-  if (decision.denied) {
+  if (false && decision.denied) {

    Before the fix — note the third control's failure line and location:

    $ pnpm exec vitest run tests/quickstart-negative-control.test.ts   # exit 1
FAIL > NEGATIVE CONTROL: a denied write_file leaves no file on disk
AssertionError: expected true to be false // Object.is equality
 ❯ tests/quickstart-negative-control.test.ts:109:31   expect(effect.occurred()).toBe(false)

FAIL > NEGATIVE CONTROL: a denied egress tool never reaches the listener
AssertionError: expected true to be false // Object.is equality
 ❯ tests/quickstart-negative-control.test.ts:161:31   expect(effect.occurred()).toBe(false)

FAIL > records the same agent id, tool name and run id the deny was decided against
AssertionError: expected '/var/folders/b7/6_9p89_119bcsnt8jqn98…' to be an instance of PolicyViolationError
 ❯ tests/quickstart-negative-control.test.ts:190:21   expect(outcome).toBeInstanceOf(PolicyViolationError)

Tests  3 failed | 7 passed (10)

    After the fix — the same mutation, all three now failing on the side-effect assertion, with a materially different message and location for the third:

    $ pnpm exec vitest run tests/quickstart-negative-control.test.ts   # exit 1
FAIL > NEGATIVE CONTROL: a denied write_file leaves no file on disk
AssertionError: expected true to be false // Object.is equality
 ❯ tests/quickstart-negative-control.test.ts:119:31   expect(effect.occurred()).toBe(false)

FAIL > NEGATIVE CONTROL: a denied egress tool never reaches the listener
AssertionError: expected true to be false // Object.is equality
 ❯ tests/quickstart-negative-control.test.ts:168:31   expect(effect.occurred()).toBe(false)

FAIL > records the tool name and run id the deny was decided against, and no agent id
AssertionError: expected true to be false // Object.is equality
 ❯ tests/quickstart-negative-control.test.ts:200:31   expect(effect.occurred()).toBe(false)

Tests  3 failed | 7 passed (10)

    All four controls that assert an absence now check it before any error assertion (:119, :168, :200, :277); a sweep of the file confirms no other site had the inverted order. Source restored inside the same uninterruptible apply→run→restore sequence, verified by a positive grep that the real guard body and its throw are back — not merely that the mutation marker is gone — with git status --porcelain empty.

    The agent-identity assertion was tautological — removed, not quietly repaired

    Review also found that expect(decision?.agentId).toBe(AGENT_ID) could not fail. The fixture populated RecordedCheck.agentId from its own constructor argument, so the assertion compared the constant the test passed with itself. An executable probe confirms it: with the fixture built as FIXTURE-AGENT and withAssembly handed agentId: "TOTALLY-DIFFERENT-AGENT", the recorded value was still "FIXTURE-AGENT", and the verbatim outbound request carried no agent field at all:

    PROBE fixture-recorded agentId = "FIXTURE-AGENT"
PROBE verbatim outbound GatewayCheckRequest keys = ["action","args","runId","toolName"]
PROBE outbound request has 'agentId'? false

    The SDK does not attribute a policy check to an agent. WithAssemblyOptions declares agentId (src/wrappers/with-assembly.ts:25) and no code path reads it — the only options. reads in that file are gatewayClient, approvalTimeoutMs and opControl — and GatewayCheckRequest (src/types/gateway-governance.ts:1-6) has no field to carry one. The documented quick-start passes agentId: "langchain-js-example-agent" (docs/02-quick-start/index.md:140) and the SDK discards it.

    Sending agentId in GatewayCheckRequest would change an outbound wire contract, which is an owner decision and not something to fold into a test-only PR. So the assertion is deleted, and the real behaviour pinned in its place: the fixture no longer accepts an agentId it could only echo back, it records the verbatim outbound GatewayCheckRequests, and the test asserts their exact key set. That pin fails the moment an agent identity is added, and its failure message says the gap is closed and the test must be rewritten, not deleted. Verified by mutating the SDK to send one:

    AssertionError: The outbound GatewayCheckRequest shape changed. If an agent identity is now
sent on the tool-call check path then this gap is CLOSED, and this test must be REWRITTEN
(not deleted) to assert the deny carries the correct agent id — ...
+ "agentId",

    The underlying gap — no agent attribution on the check path, and this ticket's audit-evidence AC being satisfied against the outbound CheckRequest rather than against audit evidence — is real and unfixed, and is being filed separately for the Epic owner. It is not addressed here.

    Findings — the AAASM-4991 defect is narrower than stated, and the README quick-start is broken

    Derived from source and confirmed by an executable probe on this branch. Each answer carries a known-present positive control.

    1. The documented README quick-start (README.md:89-105) does not silently allow — it does not start. With no mode and no enforcementMode, resolveFailClosed(undefined) === true (src/types/enforcement-mode.ts:36), the resolved mode is "auto" ≠ CHECK_CAPABLE_MODE (src/core/init-assembly.ts:144), no gatewayClient is supplied, and langchain.tools is non-empty — so the AAASM-4735 guard at src/core/init-assembly.ts:590-606 throws ConfigurationError. Probe, running the README snippet verbatim:

    A README default: INIT_THREW ConfigurationError | warnings=NONE
  "in-process tool enforcement requires a check-capable client, but mode "auto" routes
   tool policy checks through the allow-all no-op gateway client ..."
C enforce (explicit):  INIT_THREW ConfigurationError

    The README quick-start is therefore non-functional as written — a separate defect from AAASM-4991, and arguably more user-visible. Not fixed here (docs are another lane's scope); pinned by a test instead.

    2. Configurations that reach createNoopGatewayClient and get {denied:false} with no throw: explicit enforcementMode: "observe" or "disabled", in any non-napi-inprocess mode. Probe:

    B observe          : INIT_OK | call=TOOL_BODY_RAN (SILENT ALLOW) | no enforcement warning
D sdk-only+observe : INIT_OK | call=TOOL_BODY_RAN (SILENT ALLOW) | no enforcement warning
E disabled         : INIT_OK | call=TOOL_BODY_RAN (SILENT ALLOW) | no enforcement warning
G withAssembly + createNoopGatewayClient() : body_ran=true, warnings=NONE

    These are documented advisory postures, so passing through is correct — but nothing at init or call time says "no policy decision can block here". The autoDetectedToolsRouteThroughNoop warning is gated on resolveFailClosed (src/core/init-assembly.ts:427-434) and so is silent under exactly these postures.

    3. The two no-ops are genuinely distinct, and the distinction matters.

    • createNoopGatewayClient (src/gateway/client.ts:34-46 (allow-all check at :40)) — the default gateway client for every mode except napi-inprocess (src/core/init-assembly.ts:226). Its check() is the allow-all on the tool path.

    • buildStubClient (src/native/client.ts:286-296) — a binding-load-failure fallback, returned only when loadNativeBinding() throws and the mode is not napi-inprocess (src/native/client.ts:447-452). Under napi-inprocess a load failure throws NativeConnectError (:442). Probe: napi-inprocess -> THREW NativeConnectError ; grpc-sidecar -> canRegister=false queryPolicy={"denied":false,"pending":false}.

      Because createClient only builds a native-backed gateway client for napi-inprocess, and that mode never yields the stub, the stub's allow-all queryPolicy is not reachable from the tool-check path — it serves registration/events. Conflating the two overstates the defect.

    4. Auto-detected frameworks still warn without throwing — and this remains the dangerous path. Bare initAssembly() with ai / @openai/agents present:

    F auto-detect only: INIT_OK
  adapters=["langchain-js","vercel-ai-sdk","openai-agents","langgraph-js"] registered=false
  WARNING: the agent is NOT registered ...
  WARNING: the Vercel AI SDK ("ai") is installed as a frozen ES module namespace ...
  WARNING: auto-detected framework(s) openai-agents were patched for in-process tool
           governance, but mode "auto" routes tool policy checks through [the no-op] ...

    So: process.stderr.write, not console.warn; loud; and no throw by design (AAASM-1847 / AAASM-4769). A policy DENY cannot block an auto-detected framework's tool on the default path.

    Additional defect found while probing: activeAdapters reports ["langchain-js","vercel-ai-sdk","openai-agents","langgraph-js"] even though the Vercel patch demonstrably failed (frozen-ESM, warned in the same run). buildActiveAdapters (src/core/init-assembly.ts:540-555) unions adapters.map(a => a.id) — every detected framework — with the successful patch flags, so detection alone is enough to appear "active". That is a programmatic surface reporting protection that does not exist, i.e. exactly the AAASM-5526 attestation problem. Not in this ticket's scope; flagged for the Epic owner.

    Net for this ticket: the negative controls pass on the enforcement path the quick-start documents (withAssembly + a caller-supplied gatewayClient). No control was weakened, skipped, or marked expected-to-fail. The paths that cannot satisfy a negative control (observe/disabled, auto-detected frameworks) are named and pinned rather than tuned away.

Effecting Scope

  • Action Types:
    • ✨ Adding new something
      • 🟢 No breaking change
  • Scopes:
    • 🧪 Testing
      • 🧪 Unit testing
  • Additional description:
    Test-only. No src/ file is modified.

Description

  • tests/helpers/negative-control.ts — reusable fixture: a real filesystem effect, a real loopback HTTP listener that records deliveries, and a policy-driven GatewayClient that records both the verbatim outbound GatewayCheckRequests and the resulting decisions. It deliberately accepts no agentId: the SDK sends none, so a fixture that took one could only hand it straight back.
  • tests/quickstart-negative-control.test.ts — 10 tests (see the table below). Its afterEach also had a real leak, fixed in a1e9bc0: it drained the cleanup queue and awaited each entry in a bare loop, so the first throwing cleanup skipped every remaining one — leaking temp dirs and, worse, live loopback HTTP listeners whose open handles hang the vitest worker. Every cleanup is now settled before the first error is rethrown.
Group Asserts
filesystem allow → file exists with content; deny → nothing on disk; falsification → ungoverned write does create it
network allow → listener records the body; deny → listener records nothing; falsification → ungoverned POST does arrive
deny attribution the absent effect first, then the deny carries toolName, action and a run_-prefixed run id — and no agent id, pinned as the exact outbound GatewayCheckRequest key set so the gap surfaces the moment it is closed
ungoverned seam a tool with no execute/invoke is warned about (will NOT be governed) and its effect really occurs under a deny policy with no decision recorded — the warning is load-bearing, not cosmetic
zero-config boundary the README config refuses to init (ConfigurationError, message contains allow-all no-op); enforcementMode: "observe" inits and does run the tool body

Acceptance-criteria mapping: allow/deny side effects end to end (filesystem, network); deny asserts the body did not execute rather than that an error was logged (side-effect assertion, placed first); tool identity in the deny record (deny attribution group) — agent identity is NOT satisfied: the SDK attributes no agent to a check, so that AC is pinned as an open gap rather than claimed, and filed separately; no-op / failed-adapter paths cannot display a protected state (ungoverned seam + zero-config boundary); callback-only vs wrapper paths (the wrapper is the only enforcement point exercised — the callback layer stays audit-only per AAASM-4799). Not covered and reported rather than dropped: the clean-environment CI job running the quick-start, and drift-gating the doc snippets against these fixtures.

Validation

Gate Command Exit code
Target suite pnpm exec vitest run tests/quickstart-negative-control.test.ts 0 (10 passed)
Full suite pnpm test 0 — 69 files passed / 1 skipped, 641 tests passed / 2 skipped
Lint pnpm lint 0
Types pnpm typecheck (tsc --noEmit) 0
Format pnpm exec prettier --check (the two files this PR touches) 0

Lint, typecheck and the target suite were re-run at every commit, so the branch is bisectable.

One honesty note on the format gate: repo-wide pnpm exec prettier --check . exits 1 on this branch, but that is pre-existing and unrelated — it flags 95 files including src/index.ts, src/runtime.ts and ~48 test files this PR never touches, while both files it does touch pass individually. No format:check script exists and no workflow runs prettier --check (only format: prettier --write .), so reformatting 95 files was left out of scope rather than folded into a test-only PR.

@Chisanan232
✅ (tests): Add reusable enforcement-truth negative-control fixture
17cd061 
Real, externally-observable side effects (a file on disk, a live loopback
HTTP listener) plus a policy-driven GatewayClient that records the identity
triple each decision was made against. Existing deny tests assert over
vi.fn() spies, which prove the SDK did not call a reference it holds — not
that the effect the tool exists to produce was prevented.

Refs AAASM-5529, Epic AAASM-5526
@Chisanan232
✅ (tests): Prove a denied quick-start tool writes no file to disk
cfb991e 
Pairs a positive control (allow -> the file exists with the written
content) with the negative control (deny -> nothing on disk) and a
falsification case running the same write ungoverned. The side-effect
assertion runs before the error assertion so removing the deny fails the
suite on the absence check, not on "no error was thrown".

Refs AAASM-5529, Epic AAASM-5526
@Chisanan232
✅ (tests): Prove a denied quick-start tool sends no HTTP request
033e7fd 
A real loopback listener records every request it receives, so the deny is
asserted as zero deliveries rather than as a raised exception. The positive
control on the same fixture establishes the listener was reachable, which
is what makes the empty request log evidence of prevention.

Refs AAASM-5529, Epic AAASM-5526
@Chisanan232
✅ (tests): Assert a deny carries agent, tool and run identity
94e791e 
AAASM-5529 requires deny evidence to be attributable: the fixture gateway
records the identity triple it decided against, and the control checks the
recorded agent id, tool name, action and run id alongside the absent side
effect. An anonymous refusal is not usable audit evidence.

Refs AAASM-5529, Epic AAASM-5526
@Chisanan232
✅ (tests): Prove an unwrappable tool cannot present as protected
83005ac 
A tool exposing neither execute nor invoke has no seam for withAssembly to
wrap (AAASM-4847). The control checks both halves of that: the SDK warns on
stderr, and the tool's side effect really does occur under a deny policy
with no decision ever recorded — so the warning is load-bearing, not
cosmetic.

Refs AAASM-5529, Epic AAASM-5526
@Chisanan232
✅ (tests): Pin the zero-config initAssembly enforcement boundary
36a467c 
The README quickstart config (no mode, no enforcementMode, langchain.tools)
refuses to init rather than registering under an allow-all check — assert
that, plus the observe opt-out really passing the tool body through. Without
the second, the refusal is indistinguishable from "this path never works",
and a reader cannot tell an advisory posture from an enforcing one.

Refs AAASM-5529, Epic AAASM-5526, AAASM-4991
This was referenced Aug 6, 2026
Merged
Merged
@codecov

codecov Bot commented Aug 6, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

@Chisanan232
✅ (test): Assert the absent side effect before the policy error
3b08bdf 
The audit-evidence control asserted `toBeInstanceOf(PolicyViolationError)`
before its `effect.occurred()` check. A failed assertion aborts the test, so
under a mutation that neuters the deny this control failed on the missing
exception and its load-bearing absence assertion was never exercised — the
control had never been shown to bite. The two sibling controls in this file
already asserted absence first; this one was missed.

Refs AAASM-5529, Epic AAASM-5526
@Chisanan232
✅ (test): Replace the tautological agent-identity assertion with the …
7493c55 
…real gap

`expect(decision.agentId).toBe(AGENT_ID)` could not fail. The fixture set that
field from its own constructor argument, so it compared the test's constant
with itself — a probe passing withAssembly `agentId: "TOTALLY-DIFFERENT-AGENT"`
still observed `"FIXTURE-AGENT"`. The SDK supplies no agent identity at all on
the check path: `WithAssemblyOptions.agentId` is declared and never read (the
only `options.` reads in with-assembly.ts are gatewayClient, approvalTimeoutMs
and opControl), and `GatewayCheckRequest` has no field to carry one. The
outbound request keys are exactly action/args/runId/toolName.

Drop the fixture's `agentId` option so it can no longer echo back a value the
SDK never sent, record the verbatim outbound requests instead, and pin today's
real behaviour over them. The pin fails if an agent identity is ever added, and
says in its failure message that it must then be rewritten rather than deleted.

Refs AAASM-5529, Epic AAASM-5526
@Chisanan232
🐛 (test): Settle every cleanup so one failure cannot leak the rest
a1e9bc0 
The afterEach drained the queue and awaited each entry in a bare loop, so the
first throwing cleanup aborted the iteration and skipped every remaining one.
The leaked temp dirs are merely untidy, but the leaked loopback HTTP listeners
keep open handles that hang the vitest worker — turning one cleanup failure
into a stalled run. Settle them all, then rethrow the first error.

Refs AAASM-5529, Epic AAASM-5526
@sonarqubecloud

sonarqubecloud Bot commented Aug 6, 2026

Copy link
Copy Markdown

Quality Gate Passed Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

@Chisanan232
Chisanan232 merged commit 4655f5c into main Aug 6, 2026
25 checks passed
@Chisanan232
Chisanan232 deleted the v0.0.1-rc.7/AAASM-5529/enforcement_negative_controls branch August 6, 2026 12:34
@Chisanan232 Chisanan232 mentioned this pull request Aug 7, 2026
Merged
28 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

1 participant

@Chisanan232