[AAASM-5663] 🐛 (node-sdk): Make both README quickstart entrypoints run and bind their policy claims - #369
Merged
Conversation
Both snippets failed on execution. The ESM one wrapped LangChain tools with no gatewayClient, no mode and a default (fail-closed) enforcementMode, which trips the AAASM-4735 guard in init-assembly.ts and throws ConfigurationError. The CJS twin has the same config, and additionally used top-level await in a CommonJS file — a SyntaxError that fails before initAssembly is ever called, so fixing the config alone would have left it broken. Both now supply the gatewayClient that decides the wrapped tools, and the CJS sequence runs inside an async function. Verified by extracting each snippet verbatim from this file and executing it: both exit 0. Refs AAASM-5663
"checked against gateway policy before it runs/before invocation" was false in the configuration this README documents: with no check-capable client the wrapper routes through the allow-all no-op client, so nothing is decided. The sweep found the same claim at three more sites the report did not name — the Quickstart lead-in, "auto-detects and governs", and "so every tool call is checked against policy before it runs" under How it works. All four now name the ADR 0033 §6 term the shown configuration earns, "denied before execution", and attribute the decision to the gatewayClient rather than to a gateway that is not in the picture. The fail-closed ConfigurationError is called a startup configuration check so a reader cannot read the refusal as a policy decision about a tool. Drops the banned absolutes "every"/"each". Refs AAASM-5663
…ides them init-assembly.ts names this page as the concrete documented vector for the AAASM-4735 fail-open, and it carried the README's defect verbatim: the same false "every invoke() is checked against gateway policy before it runs" sentence, plus two snippets that wrap LangChain tools with no gatewayClient. Executed as written, the first throws ConfigurationError before reaching the wrap guard because it also omits gatewayUrl. Both snippets now supply the deciding client and a gateway URL, and the prose names the ADR 0033 §6 term the configuration earns. "the gateway is consulted on every call" becomes what the wrapper actually does — ask the configured client — since no gateway is consulted on this path. Refs AAASM-5663
"every tool an agent calls is checked against policy before it runs" is the same claim the README made and is false on the default path. The paragraph after it already qualified the audit half honestly (AAASM-5681); this gives the enforcement half the same treatment and drops the banned absolute. Refs AAASM-5663
The prose added for this ticket reintroduced the fd-7 absolutes it was meant
to remove ("Decides every wrapped invoke()", "each tool is wrapped"). Reworded
to state what the wrapper does without quantifying over calls.
The 04-guides paragraph on the fail-closed refusal also restated the
":::note[In-process tool enforcement needs a check-capable mode]" admonition
sitting directly under the same snippet; replaced with a pointer to it.
Refs AAASM-5663
…dated The AAASM-5529 gate registered "The SDK enforces policy by talking to an Agent Assembly gateway" as unproven, justified by the README entrypoints not running as written. That premise no longer holds once this ticket fixes them, so the justification is restated on the ground that still stands: the controls decide tool calls through a caller-supplied gatewayClient answering in-process, which proves a wrapped call is decided before its body runs, not that a gateway is the deciding party. The quote is unchanged, so the gate's whole-sentence equality match is unaffected. Refs AAASM-5663
…es it The allow/deny/pending walkthrough still credited "the gateway" for the verdict and the reason string, one paragraph below the sentence corrected earlier in this ticket. On this path the decision comes from the configured gatewayClient, so the surrounding text now says so consistently. Refs AAASM-5663
…lkthrough "The SDK wraps each tool so the gateway sees the call first" is the same misattribution corrected elsewhere in this ticket, sitting three paragraphs below one of those corrections. On this path the deciding party is the configured gateway client, not a gateway. Also rewraps the Framework-compatibility line this ticket had pushed past the file's wrap width. Refs AAASM-5663
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
The reason on "The SDK enforces policy by talking to an Agent Assembly
gateway" is a FORWARD pointer — "no control covers a reader actually reaching
a gateway" — not a citation for where the claim was last examined. Naming
AAASM-5663 sent a reader to a ticket that fixes README snippets and never
intended to deliver gateway coverage, and that pointer goes stale the moment
this work merges.
AAASM-5758 ("CI job running each documented SDK quick-start from a clean
environment, using published artifacts only") owns the gap, is still open, and
carries the node-sdk, python-sdk and go-sdk components — so the three SDKs name
one referent rather than three. It lists AAASM-5663 among its blockers, which
is the right direction: this ticket unblocks that one.
The reason now says outright that it is a forward pointer, so the next reader
does not have to infer which kind of reference it is. The bound quote is
unchanged, so the gate's whole-sentence equality match is unaffected.
Refs AAASM-5663, AAASM-5758
…roduction A sentence THIS BRANCH added was false. "wrap tools without such a client and initAssembly refuses to start" is unscoped, but the refusal fires only for explicit langchain.tools: wrapsToolsThroughGatewayClient (init-assembly.ts :217-220) reads config.langchain?.tools and nothing else, and the guard at :755-762 is &&-gated on it. init-assembly.ts:538-540 states the opposite for everything else — auto-detected frameworks have no init-time throw. An in-repo control falsifies the sentence directly: tests/auto-detected-noop- enforcement-warning.test.ts:59-66 calls initAssembly with no gatewayClient under default fail-closed enforcement and asserts ctx IS defined. The README paragraph keeps the langchain.tools conjunct because it opens on it; dropping it here made a true sentence false. Now scoped, and says what the other path does instead: it warns and proceeds. Also corrects the plain-terms opener, which promised "an agent can only do what your rules allow" and "a record of everything it did" — both false on the default path, and both already contradicted by this file's own lines 34-38 and 65-67. The opener now defers to them instead of contradicting them. Refs AAASM-5663
"the tools you handed it are wrapped so the gateway evaluates each call before it executes" is the untouched twin of the introduction sentence already fixed under this ticket. No gateway evaluates anything on the default path — the fallback client answers in-process and allows everything. The mermaid flowchart directly above draws SDK to aa-ffi-node to gateway, so the sentence reads as a product claim rather than a description of one configuration; the replacement says which path the diagram depicts and what the fallback client does instead. Refs AAASM-5663
Byte-for-byte the sentence already corrected in the README, left behind on the page README.md:213-214 calls "the authoritative reference" — so the derived copy was fixed and the authoritative one was not. "Governs" is exactly the undifferentiated verb ADR 0033 §6 tells downstream material not to use. Refs AAASM-5663
…d it "the gateway denied the tool call ... the gateway's reason" is the same misattribution corrected in 04-guides, phrased differently enough that a phrase-pattern sweep did not reach it. Found by reading the file. Refs AAASM-5663
…not route "The governance decisions it enforces are made by the core Rust runtime" is false in the configuration this README now documents, where the decision comes from a caller-supplied client answering in-process. Narrowed to the authoritative decisions, which is the claim that holds. Refs AAASM-5663
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Target
Task summary:
The README's two quickstart entrypoints did not run, and the surrounding prose claimed a
policy check that does not happen in the configuration the README itself documents. Both
are fixed, along with every same-class claim found in the live docs outside the
gate-bound quick-start. What remains unfixed is enumerated under Found and not fixed
below — this PR does not claim the class is exhausted.
AC1 — both entrypoints run as written. Measured by extracting each snippet verbatim
from the rendered file and executing it.
ConfigurationError: in-process tool enforcement requires a check-capable client…results for agent assemblySyntaxError: await is only valid in async functions…results for agent assemblydocs/04-guidesLangChainConfigurationError: No gateway found at http://localhost:7391results for agent assemblyThe CJS snippet had two independent defects: the same missing-client config as ESM,
and top-level
awaitin a CommonJS file — a syntax error that fails beforeinitAssemblyis ever reached. Fixing only the config would have left it broken.AC2 — the policy-check claim.
initAssemblywas behaving correctly: it refuses toroute tool checks through the allow-all no-op client rather than pretend to enforce
(AAASM-3105 / AAASM-4735). The docs were wrong, not the SDK. The snippets now supply the
gatewayClientthat decides the wrapped tools, which makes the claim true rather thanmerely hedged, stated with the ADR 0033 §6 term the configuration earns:
denied before execution.
That term is earned, not asserted: §6 admits it for "an SDK shim that honoured a
Deny".A deny control against the documented config throws
PolicyViolationError, and aninstrumented tool body never printed — the body does not run.
The fail-closed
ConfigurationErroris explicitly described as a startup configurationcheck, not a policy decision about a tool, so a reader cannot read the refusal as
enforcement.
Task tickets:
Key point change (optional):
The sweep found more than was reported. The ticket named 2 sites (README ESM + its CJS
twin). A brace-balanced scan of every
initAssembly({…})block wrappinglangchain.toolsacross all live docs found 4 defective blocks, and the same false sentence in
docs/04-guides/index.md— the very pageinit-assembly.tsnames as "the concrete,documented (04-guides LangChain quick-start) vector" for the AAASM-4735 fail-open.
langchain.toolsin live docsThe scan is a positive control, not a claim: run against
remote/mainit reports the 4defective blocks by file and line; run against this branch it reports 0.
Effecting Scope
Action Types:
Scopes:
Additional description:
No
src/change — the SDK's behaviour was already correct. The only non-doc edit is thejustification string on an existing binding in
tests/quickstart-claim-bindings.test.ts;the bound
quoteis unchanged, so the gate's whole-sentence equality match is unaffected.Description
README.md— both quickstart snippets now supply agatewayClientand the CJS one runsits sequence inside an
async function. Five claim sentences reworded: the Quickstartlead-in, the post-snippet paragraph (the one the ticket named), "auto-detects and governs",
"so every tool call is checked against policy before it runs" under How it works, and
:405, which credited the core Rust runtime for decisions this SDK may never route to it.docs/03-core-concepts/index.md— "the gateway evaluates each call before it executes",the untouched twin of the introduction sentence.
docs/07-compatibility-versioning/compatibility.md— "auto-detects and governs",left behind on the page the README calls "the authoritative reference".
docs/08-troubleshooting/index.md— "the gateway denied the tool call … thegateway's reason".
docs/04-guides/index.md— the same false sentence and both broken snippets fixed; theallow/deny/pending walkthrough now attributes the verdict and reason to the client that
produces them rather than to a gateway that is not on this path.
docs/01-introduction/index.md— the "every tool an agent calls is checked againstpolicy before it runs" thesis given the same treatment its audit half already had. A
later revision of this PR then had to fix a sentence this PR itself introduced: the
replacement said "wrap tools without such a client and
initAssemblyrefuses to start",dropping the
langchain.toolsconjunct the README version keeps. The refusal fires onlyfor explicit
langchain.tools—tests/auto-detected-noop-enforcement-warning.test.ts:59-66callsinitAssemblywith no client under default fail-closed enforcement andasserts
ctxis defined. The plain-terms opener is corrected too; it promised "arecord of everything it did", which this same file already contradicts at
:65-67.tests/quickstart-claim-bindings.test.ts— an existing unproven-claim justificationcited "both README entrypoints … neither runs as written" as its ground. This PR
invalidates that premise, so the justification is restated on the ground that still stands.
The first sweep was run on the wrong predicate
The first pass operationalised "the claim class" as a phrase pattern (
checked against,before it runs,governs). A claim class is defined by what a sentenceasserts, not by the words it uses, so anything phrased differently was invisible to it.
docs/03-core-concepts/index.md:22says "the gateway evaluates each call before itexecutes" — same assertion, no matching phrase, missed. So did
docs/08-troubleshooting/index.md:66("the gateway denied the tool call").The sweep was redone by reading every live doc against the predicate "asserts that
something evaluated, decided, checked, governed or enforced a call". That found four
sites the phrase sweep could not reach, three of which are fixed here and one of which
(
README.md:405) nobody had flagged.Found and NOT fixed — enumerated, not waved past
docs/02-quick-start/index.md— carries genuine same-class defects, notably:297("the gateway's reason") and:309("violations recorded as shadow events",which the shipped clients discard). Not touched because this file is the one the
AAASM-5529 gate binds by whole-sentence equality; several of the flagged sentences are
bound claims. Editing them means rewriting that gate's bindings in lockstep, which is
AAASM-5529's artifact and a materially larger change than this ticket. Flagging for a
scope decision rather than unilaterally rewriting a gated file.
docs/03-core-concepts/architecture.md:64-70and03-core-concepts/index.md:19—mermaid edges (
LC->>GW: policy decision request,GW-->>LC: allow / deny / pending)put the gateway on the per-call decision path. The prose beside them is fixed here.
Whether the diagrams depict the intended architecture or must depict the default runtime
path is a judgement I did not want to make by redrawing them.
docs/05-configuration/index.md:128-129and03-core-concepts/index.md:133—"
enforceblocks denied actions" / "observerecords would-be violations". Thesedescribe the core runtime's
EnforcementModeposture, which is the semantics sent atregistration, not a claim about this SDK's in-process path.
Examined and correct as written
docs/09-examples/*— verified, not assumed: every page useswithAssemblywith asupplied client and none calls
initAssemblyor attributes a decision to "thegateway".
index.md:23-26states that scope explicitly.docs/06-api-reference/index.md:27— scoped to "a gateway client you supply".docs/04-guides/container-base-image.md— "governed out of the box" is qualified by anexplicit
:::noteon the same page disclaiming the SDK as an authoritative boundary.docs/03-core-concepts/index.md:57,:59— negative claims/limitations.docs/07-compatibility-versioning/releasing.md,SECURITY.md,09-examples/setup.md—no claims of this class.
website/versioned_docs/**— 36 frozen snapshots of released versions carry the oldpattern. Not edited: published records of what shipped. AAASM-5676 owns whether retracted
claims get corrected in frozen snapshots.
Verification
pnpm typecheckandpnpm lint("ESLint: No issues found") are green. Snippet executionused a consumer harness resolving
@agent-assembly/sdkthrough the package's realexportsfield, so both the ESM and CJS conditions are the ones a reader gets.
No Prettier reformatting is included.
prettier --checkflags these files, but it flagsremote/main'sREADME.mdtoo — the drift is pre-existing blank-line handling inside theGENERATED … DO NOT EDITblocks, no CI job runs a format check, and no hook is installed inthis repo. Reformatting generated blocks is not this ticket's business, so the only edits
here are content.
One limitation, stated plainly: the harness is a source checkout with no prebuilt native
binding, so every run also prints the "agent is NOT registered" warning. That is an artifact
of the checkout, not of the documented config — a reader installing from npm on Linux/macOS
receives a prebuilt binding. It does not affect the defect or the fix, because
mode: "auto"routes to the no-op client whether or not the binding loads.