README, .env.example, packages/signer/README.md, and docs/architecture/non-custodial-split.md
updated for the family-2 keyless migration (agent-registry build_* tools, no mnemonic
parameters): security notice narrowed to name the Self-Improvement Module as the only
remaining custodial family; agent-registry tool table renamed to the five build_* rows;
features bullets, self-hosting copy, Configuration/spend-limit scope notes, and the
architecture diagram's Safety Layer footnote all updated to "self-improvement only"; signer
README's pairing-status bullet and closing line extended to cover wallet/tx/contract AND
agent-registry; architecture doc's PR-sequence table marks family 2 as landed.
docs/VECTOR.md gains five new tool sections (register/update/transfer/deregister/message) -
this doc never documented the registry family under any name before, so these are additions
rather than renames; keyless framing (changeAddress, no mnemonic, unsigned CBOR + 4-call flow)
matches the style of the existing wallet/tx/contract sections. Safety Controls section's
custodial-family list also corrected to self-improvement only.
Closes an Opus review round on Task 5's registry integration work.
Adds packages/builder/test/sign-helper.test.ts (top-level test/, picked up
by the unit glob) as the first hermetic gate that would fail if
signWithMnemonic's witness-preservation fix regressed. Before this, the
fix had zero coverage under any required gate: sign-helper.ts lives under
test/integration/, outside the unit glob; run.test.ts's
assertSuccessOrKnownError swallows the resulting "Failed to submit
transaction" as an indistinguishable known error; and the E2E's
hash-equality check can't catch it either, since dropping witnesses never
changes the transaction body hash - only the ledger tells the two apart,
and this test never talks to it. The new test builds a synthetic,
fully-offline script-bearing transaction directly via CML (one input/
output, a witness set carrying one PlutusV3 script, one map-form redeemer,
one datum), signs it with a public, well-known, never-funded BIP39 test
vector (computed from the installed bip39 package rather than transcribed
from memory - the first guess had the wrong checksum word and failed
loudly), and asserts every witness category plus the vkey witness survive
signing with the body hash unchanged. Proved it's load-bearing: reverted
the fix to the buggy from-scratch witness set, watched the test fail
naming the exact lost witness, reverted back (byte-identical, zero diff),
re-ran green.
registry-keyless.test.ts: the non-owner-update assertion drops its second
regex alternative (`Failed to build agent update`), which was a superset
match able to hide a real, unrelated build failure behind the same
"ownership was correctly refused" label. Now asserts the specific
`Ownership check failed` message only.
run.test.ts: buildSignSubmit now hard-asserts `Transaction Submitted` on
the submit leg when walletHasAda is true, instead of routing through the
tolerant assertSuccessOrKnownError - on a funded wallet a submit failure
is a real regression, not indistinguishable from an unfunded wallet, and
should fail the test rather than pass silently as a "known error". The
graceful path is kept for the unfunded case. Also documents, in a comment,
the known/accepted risk that a register-succeeds/deregister-fails run in
this file leaves the 10 AP3X test deposit locked on-chain (same shape as
the pre-keyless suite).
Hermetic only this round, per instruction: no live network calls, no
transaction submissions, no real secrets. Full account in
.superpowers/sdd/task-5-report.md's Addendum section.
Gates: typecheck 0, unit 351/351 (350 + 1 new), smoke 26/26, auth 12/12,
boundary 4/4 (confirms the new test's mnemonic constant sits outside the
custody scan by design, same as the rest of test/, and is labeled
TEST_ONLY_PUBLIC_MNEMONIC regardless per public-repo hygiene).
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Adds the three remaining integration layers for spec PR 7 (agent-registry
keyless): a tier-1 suite that exercises the five build_* tools against the
live testnet registry with zero secrets (build-only, nothing signed or
submitted), a gated full lifecycle E2E (register -> update -> deregister)
that actually submits, and a shared signWithMnemonic() helper extracted
from keyless-e2e.test.ts so registry-e2e.test.ts and run.test.ts's
registry section don't each re-inline the same CML signing sequence.
run.test.ts's "Agent Network Tools" section is rewritten: each of the five
old custodial calls is now build_* -> signWithMnemonic -> submit, via a
new buildSignSubmit() helper that reuses the file's existing
assertSuccessOrKnownError convention at both the build and submit legs.
Found and fixed one load-bearing bug while proving this live, not just in
review: the inline signing block moved verbatim per the brief's Step 1
built the signed tx's witness set from scratch, which is lossless only for
a script-less payment (the one case it had been proven against). The
registry family's build_* tools attach a minting policy and/or spending
validator, so their unsigned CBOR already carries scripts/redeemers that
the from-scratch witness set silently dropped - the ledger rejected the
first live E2E attempt with MissingScriptWitnessesUTXOW /
PPViewHashesDontMatch. Verified zero funds were lost (balance identical
before/after the rejected submission - a mempool-level rejection never
reaches a block) before fixing it by merging into the tx's existing
witness set via CML's add_all_witnesses instead of discarding it. Re-run
clean: all 3 E2E transactions confirmed on-chain, plus all 5 of the legacy
suite's registry operations submitted successfully too (independent
confirmation across both the mint and spend redeemer paths). Also widened
one tier-1 regex that didn't tolerate the live tool's Markdown-bold label
formatting.
Live evidence (testnet, funded wallet, 8 real transactions total across
the gated E2E and the legacy suite): balance 986.689510 -> 982.280960
AP3X, fully reconciled against transaction fees plus one message delivery
to an owner-derived enterprise address. All tx hashes, explorer links, and
the full reconciliation are in task-5-report.md. mnemonic.txt and the two
temporary evidence-capture scripts were removed and confirmed absent
before this commit.
Gates: typecheck 0, unit 350/350, smoke 26/26, auth 12/12, boundary 4/4 -
all re-verified after cleanup. E2E skip-path (VECTOR_E2E_SUBMIT unset)
exits 0 with tests 0 for both keyless-e2e.test.ts and registry-e2e.test.ts,
confirmed with mnemonic.txt genuinely absent.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Rewrites agent-network.ts as a typed, keyless module: the five custodial
signing tools (vector_register_agent, vector_update_agent,
vector_transfer_agent, vector_deregister_agent, vector_message_agent) are
replaced by unsigned-build equivalents (vector_build_register_agent,
vector_build_update_agent, vector_build_transfer_agent,
vector_build_deregister_agent, vector_build_message_agent) that take no
mnemonic - the 4-call flow is get_address -> build_* -> signer sign ->
submit -> confirm. The two read-only tools (vector_get_agent_profile,
vector_discover_agents) keep identical behavior, description, and schema,
now backed by registry-build.ts's typed helpers (Tasks 2-3) instead of
local duplicated logic.
@ts-nocheck is dropped - the file now typechecks clean under strict mode
(server.tool()'s generic inference did not explode; the TS2589 concern
that originally motivated @ts-nocheck is confirmed dead under zod/v4).
boundary.test.ts: CUSTODIAL_UNTIL_LATER_PR shrinks from
{agent-network.ts, self-improvement.ts} to {self-improvement.ts} - size
pin 2 -> 1, message updated for spec PR 8 (the last remaining family).
tool-inventory.test.ts: EXPECTED_TOOLS updated for the 5 renames,
alphabetical order preserved, count stays 23.
tool-schemas.snapshot.json regenerated; delta verified surgical - exactly
the 5 old names removed and 5 new names added, all other 18 entries
(11 vector.ts + 5 self-improvement + the 2 unchanged read-only agent
tools) byte-identical.
Also fixes a stale cross-reference this rename exposed:
self-improvement.ts's registry-NFT-not-found error told the caller to
"re-register with vector_register_agent", a tool name that no longer
exists post-rename. Updated to vector_build_register_agent. This is the
only reference to the old names left in packages/builder/src outside the
deferred integration test (test/integration/run.test.ts, explicitly
scoped to a later task per progress.md).
One addition beyond the task brief (Task 3 review, M-c context): the
build_deregister_agent troubleshooting copy gets a 4th tip pointing at
registry-build.ts's I-1 error (deregistration needs a plain AP3X-only
UTxO to clear the validator's returned-change floor).
Gates: typecheck 0, unit 350/350, smoke 26/26 (23 tools), auth+rate-limit
12/12, boundary 4/4.
Four items from the final whole-branch review, all docs-only:
1. Architecture diagram: the ASCII diagram routed every tool call through
Rate Limiter -> Safety Layer -> Lucid/Ogmios, which is false for the
keyless build_* path (safety.ts is never consulted there). Annotated the
Safety Layer box with a footnote marker instead of redrawing the diagram -
box alignment verified programmatically before and after (same column
positions for every vertical bar).
2. Configuration table: added one sentence stating VECTOR_SPEND_LIMIT_PER_TX/
DAILY govern only the still-custodial families, matching .env.example's
existing scope note.
3. Testing section: "builds every build_* tool's unsigned transaction"
overclaimed - keyless-build.test.ts covers 4 of 5 (verified directly
against the test source: vector_build_send_tokens is absent from it,
covered instead by unit tests and the legacy integration suite). Reworded
to name the gap explicitly.
4. packages/builder/test/integration/keyless-e2e.test.ts:9 (comment only):
"which lives on PR #7's branch" was stale now that #7 is merged and
packages/signer is in-tree. Reworded to name the in-tree package and be
precise about what the stand-in does and doesn't exercise (same CML
primitives, without decode/policy/audit).
#7 (signer) and #8 (builder hardening) merged into main this morning; this
branch is rebased onto the result and packages/signer is now in-tree. Three
sites still said pairing the signer with the hosted builder "does not yet
remove the custody exposure" (true when #7 was written, before this PR's
keyless build_* tools existed - now false for the wallet/tx/contract family):
- README.md's "Local signer" section: now states the 4-call non-custodial
flow (signer_get_address -> build_* -> signer_sign -> submit -> await) is
live today for that family, names the signer's four tools, and keeps the
agent-registry/self-improvement families explicitly scoped out.
- packages/signer/README.md's "Known limitations" bullet and "Running it"
closing line: same correction, with the specific now-keyless build_* tool
names and an accurate (not overclaiming) description of what the E2E test
exercises - it signs with the same CML primitives sign.ts uses, as a
stand-in for calling vector_signer_sign directly, not a call into the
signer package's own code.
Grepped case-insensitively across the repo for "does not yet remove" /
"custody exposure" / "signer half is ready" / "builder half is not yet"
before and after: exactly these 3 sites found, all corrected, zero stale
instances remain.
README, .env.example, and docs/architecture/non-custodial-split.md updated for
the family-1 keyless migration (build_* tools, no mnemonic parameters): custody
notice rewritten to describe the keyless boundary precisely, tool tables synced
to the 23-tool surface (4 renames, 4 changed in place, 2 removed), self-hosting
and testing copy updated, .env.example spend-limit scope annotated, and the
architecture doc's PR-sequence table marks family 1 as landed.
docs/VECTOR.md (flagged by Task 4 as referencing stale tool names) is also
updated: renamed/removed tool sections corrected, keyless changeAddress params
added, and the Safety Controls / Error Handling sections stop claiming
server-side spend limits and mnemonic requirements that no longer apply to the
tools this page documents. Edits confined to tool names and custody claims -
no restructuring, no wholesale ADA to AP3X pass beyond the sections touched.
Opus review round 2 on Task 6:
- agent-network.ts: seedUtxo -> oneShotUtxo (5 occurrences). This is
one-shot-minting-policy vocabulary that survives PR 7's keyless rewrite,
but the boundary test's FORBIDDEN regex is an unanchored case-insensitive
substring match on "seed" - left as-is it would have false-red PR 7's
guard the moment agent-network.ts drops off the custodial allowlist. Only
selectWallet.fromSeed (genuine key-material vocabulary, deleted by PR 7
anyway) still matches `seed` in this file. File is @ts-nocheck, so this
was verified mechanically, not just visually: line-match counts before
(seedUtxo=5) and after (seedUtxo=0, oneShotUtxo=5) line up exactly, and a
fresh `import()` of the module still loads error-free.
- keyless-build.test.ts: the first tier-1 test summed every output
(`paid >= 3_000_000n`), but a self-send's change output lands at the same
address as the payment, so the sum floor could never actually detect a
dropped or wrong payment amount - change alone clears it. Added an
exact-count check (outputs at exactly 3_000_000n lovelace, >=1) alongside
the existing sum/fee/unsigned assertions, matching the pattern already
used by the build_transaction test in the same file. Also added the
missing assertBuildNotSubmitted check on the dry-run test's own build
response.
- run.test.ts: deleted walletBalanceAda - write-only since the UTxO
Consolidation block (its only reader) was removed earlier in this task.
Re-verified full gate plus one live re-run of the tightened tier-1 suite
(build-only, no funds spent, no E2E resubmission).
"different seed UTxOs" is one-shot minting-policy vocabulary, not key
material, but the boundary test's FORBIDDEN regex matches "seed" as a
substring anywhere in scanned source. agent-network.ts is allowlisted as
custodial today so this line is invisible to the guard, but PR 7 takes it
keyless and drops the allowlist entry - at that point this comment would
false-red the guard for no reason. Reworded to "one-shot UTxOs", meaning
unchanged.
Addresses opus reviewer findings on the Task 5 commit (33c4b52).
Important:
- agent-network.ts (3 sites) and self-improvement.ts (2 sites): response
copy no longer tells a caller to run vector_get_spend_limits (deleted in
Task 5) after a safety-limit error. Replaced with
"Server spend limits for this tool are set via VECTOR_SPEND_LIMIT_PER_TX /
VECTOR_SPEND_LIMIT_DAILY" (matching each site's existing phrasing).
Nothing else in either file touched - both stay custodial until PRs 7/8.
Also reworded the two comments (of my own, from Task 5) that named the
deleted tool - tool-inventory.test.ts's header docblock and
rate-limit-gating.test.ts's probe-choice comment - so a final
`grep -rn vector_get_spend_limits packages/` is clean except
run.test.ts (Task 6's, left untouched as instructed).
- boundary.test.ts: added a third test pinning
CUSTODIAL_UNTIL_LATER_PR.size === 2, so PR 7/8 shrinking it is a visible,
reviewable number change rather than a silent list edit.
- boundary.test.ts: FORBIDDEN is no longer word-bounded
(`/(mnemonic|seed|bip39|fromSeed|walletFromSeed|selectWallet\.fromSeed)/i`,
\b dropped) so it catches `seedPhrase`, `WALLET_SEED`, `bip39x`-shaped
vocabulary, not just whole-word matches. Verified empirically (not just by
inspection) that this introduces zero new hits outside the two allowlisted
files - grepped `seed`/`bip39`/`mnemonic` case-insensitively across all of
packages/builder/src first, confirmed every hit was already confined to
agent-network.ts/self-improvement.ts, then reran the full boundary suite
to prove it: no benign collisions, nothing to report back before
committing. Added a fourth test pinning every FORBIDDEN alternative
against a fixture corpus (mnemonic/MNEMONIC/seedPhrase/WALLET_SEED/bip39/
fromSeed/walletFromSeed/selectWallet.fromSeed/seed all match;
"address-only wallet"/"unsigned transaction"/"changeAddress" don't) so
deleting an alternative from the regex fails a named test instead of
silently narrowing the guard.
Cheap folds:
- vector.ts: the five build_* tool DESCRIPTIONS (not response copy - that
already taught the confirm step as of Task 5) now end with "...then
broadcast with vector_submit_transaction and confirm with
vector_await_transaction." Regenerated the snapshot; verified structurally
that the delta is description-field-only on exactly those five entries
(inputSchema untouched, all 23 keys unchanged) - this is also what makes
the Task 5 addition-A docblock's "descriptions teach the full flow" claim
actually true end-to-end.
- tool-inventory.test.ts: each per-tool test now also asserts
'description' in schemaSnapshot[toolName] before the deepStrictEqual, so
a regen that silently drops a description (e.g. JSON.stringify eating an
undefined value) fails by naming the tool, not as an opaque diff.
- regen-tool-schemas.mjs: server entry point and snapshot output path are
now resolved from import.meta.dirname instead of hardcoded
repo-root-relative strings, so the script runs from any cwd. Verified by
actually invoking it from packages/builder (not the repo root) and
confirming it wrote to the one correct
packages/builder/test/smoke/tool-schemas.snapshot.json.
Full gate re-run after all of the above: typecheck 0 errors, test:unit
144/144 (140 baseline + 4 boundary tests, up from 2), test:smoke 26/26,
test:integration:auth 12/12.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Finishes the builder-side wallet/tx family's non-custodial split (spec PR 6).
Task 4 registered the keyless build_* tools; this task deletes the last
custodial surface in vector.ts and makes every remaining query tool keyless:
- Deleted initLucid, getWalletInfo, the vector_get_address and
vector_get_spend_limits tool registrations, and the now-unused safetyLayer
and Lucid imports. SafetyLayer/safety.ts itself stays (still used by
agent-network.ts and self-improvement.ts until PRs 7-8).
- vector_get_utxos, vector_get_transaction_history: address is now required,
mnemonic param and branch deleted, provider construction routed through
makeProvider().
- vector_dry_run: mnemonic param removed; the outputs-branch now builds via
lucidForAddress + buildMultiOutput (keyless, UNSIGNED) instead of signing
with a mnemonic-derived wallet.
- packages/shared/src/types.ts: removed the now-dead VectorToken,
VectorWalletInfo, VectorAdaTransactionResult, VectorTokenTransactionResult,
VectorBuildTransactionResult, VectorDeployContractResult and
VectorInteractContractResult interfaces (and their re-exports from
vector/index.ts). SpendLimits/SpendStatus/AuditEntry stay - safety.ts is
still live for the two custodial families.
- vector_build_transaction / vector_build_deploy_contract /
vector_build_interact_contract response copy now teaches the full
build -> sign -> submit -> confirm flow (they previously stopped at
submit), matching vector_build_send_apex/send_tokens.
- rate-limit-gating.test.ts: repointed its hermetic probe from the
now-deleted vector_get_spend_limits to vector_build_send_apex with an
invalid changeAddress - lucidForAddress rejects it in-process before any
provider call, so the suite stays hermetic.
- Added packages/builder/test/boundary.test.ts: a mechanical grep-based
custody boundary (design-spec success criterion 1). Verified failing before
the rewrite (vector.ts's mnemonic vocabulary) and passing after; also
verified the guard actually fails by injecting `// mnemonic` into build.ts
and watching it name the exact file:line before reverting.
- Tool surface is now 23 (25 - 2 deletions). tool-inventory.test.ts updated
accordingly.
Snapshot shape widened (this once, deliberately): tool-schemas.snapshot.json
entries are now { description, inputSchema } instead of a bare inputSchema,
so the regression guard also covers the security-teaching copy in tool
descriptions (the sign/submit/confirm flow callers learn from them), not just
JSON Schema shape. tool-inventory.test.ts's per-tool test now deep-compares
the whole entry and is relabeled "schema+description unchanged". This is why
the snapshot diff touches every entry, not just the 10 that actually changed.
Committed the regen script at packages/builder/scripts/regen-tool-schemas.mjs
(previously scratchpad-only) with this shape change applied.
Verified against the feat/builder-auth-and-submit baseline: the snapshot key
delta is exactly 4 renames (send_apex/send_tokens/deploy_contract/
interact_contract -> build_*), 4 changed-in-place (build_transaction,
dry_run, get_utxos, get_transaction_history), and 2 removals (get_address,
get_spend_limits) - confirmed with a structural JSON diff, since the uniform
shape-widening makes raw line-based git diff noisy (e.g. it shows
vector_message_agent's key line as touched even though its inputSchema is
byte-identical old vs new - a Myers-diff line-matching artifact, not real
agent-network.ts drift).
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
- Move fixture-provider.test.ts up to test/ (test:unit's glob is
packages/*/test/*.test.ts, deliberately shallow, so a nested
test/fixtures/ file was silently never run). The fixture module itself
(fixture-provider.ts, the capture script, the JSON fixture) stays in
test/fixtures/; only the *.test.ts file moved, importing it via
./fixtures/fixture-provider.ts.
- Harden capture-protocol-params.mjs: refuse to write a fixture whose
costModels/coinsPerUtxoByte/keyDeposit could have come from
OgmiosProvider's own `??` fallback defaults instead of a genuine
response. The brief's sanity numbers (minFeeA 45, minFeeB 156253, ...)
are literally those same fallback literals, so matching them proves
nothing; costModels has no populated fallback, so a well-formed,
appropriately-sized cost model per Plutus version is the one signal a
schema-mismatched response cannot fake.
- packages/builder/tsconfig.json: include test/**/* so `npm run
typecheck` actually checks builder's test files (it previously covered
src/**/* only, unlike packages/shared). Needed
allowImportingTsExtensions for the now-in-scope relative .ts-suffixed
imports across every existing test file. Surfaced one real pre-existing
gap: auth.test.ts calls loadAuthConfig with deliberately minimal env
objects, but the exported signature demanded the full
env.d.ts-augmented NodeJS.ProcessEnv (7 required custom fields)
even though the function only ever reads MCP_AUTH_TOKENS. Narrowed the
parameter to Record<string, string | undefined> - the function's real
contract - which is a type-only change with a single production call
site (index.ts's parameterless loadAuthConfig()) that already relies on
the default.
Verified: test:unit 117/117 (was 113 - the 4 fixture tests were never
counted before), typecheck 0 errors, test:smoke 28/28, build unaffected.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The "unusual identity strings" fixture embedded a literal 0x00 byte in a
string literal instead of the \x00 escape sequence. Consequence: the file
had no valid text encoding as far as git's binary-diff heuristic is
concerned, so the whole 5278-byte file rendered as "Binary files differ"
with zero textual diff - invisible to review, and would have been
invisible to any GitHub PR reviewer too. It also made the raw NUL
character indistinguishable from the adjacent ASCII-space case when
viewed directly.
Same runtime string (a genuine one-character NUL string, confirmed via
'\x00'.charCodeAt(0) === 0), no behavior change - the file is just
diffable text again.
The auth guidance was accurate but lived only in the Configuration
reference table, after both quickstart paths (npm start / docker run)
that a self-hoster would follow top to bottom. Following either to
completion on a public host reproduces the unauthenticated-server problem
this PR exists to fix, without ever seeing a warning at the point of
action.
Add one line after each quickstart's run command, pointing to
Configuration. The Docker line also notes that docker-compose.yml in this
repo already binds 127.0.0.1 by default, unlike the inline `docker run`
example.
vector_await_transaction's confirmation check only ever looked for an
unspent output at index 0 - a heuristic that can read a landed transaction
as unconfirmed, since these tools broadcast CBOR signed elsewhere with no
guarantee of that output existing or staying unspent. The check also wrapped
the lookup in .catch(() => []), collapsing genuine infrastructure failures
(Ogmios down, malformed response) into the same "not confirmed yet" result,
defeating pollUntilConfirmed's documented contract that a check rejection
should propagate.
OgmiosProvider.getKoiosTxStatus + koiosIndicatesConfirmed (shared/provider.ts)
now provide a direct, non-swallowing Koios tx_status check; poll.ts's new
buildConfirmationCheck picks it when VECTOR_KOIOS_URL is configured and
falls back to the outRef heuristic (also no longer swallowing errors)
otherwise - the same preference order OgmiosProvider.awaitTx already used,
without reusing awaitTx itself (still not caller-configurable, still
swallows every failure). awaitTx stays in place only because the Provider
interface requires it; nothing else calls it.
buildConfirmationCheck and koiosIndicatesConfirmed are unit tested via
dependency injection, the same pattern poll.test.ts already uses for
now/sleep, with no network access required.
Covers what Task 1 actually enforces: auth is only on when MCP_AUTH_TOKENS
is set (otherwise every caller is one anonymous, shared-bucket identity),
rate limits are per identity, and the token parser rejects empty tokens,
whitespace, duplicates, and all-blank values at startup rather than
degrading silently. Notes that vector_submit_transaction and
vector_await_transaction take no mnemonic and sign nothing. Updates the
tool count from 23 to 25 everywhere it appears in the README.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The snapshot guard flagged both additions, which is what it is for. Delta
verified as exactly two ADDED entries with no existing schema changed.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Completes the keyless half of the non-custodial flow: the signer returns
signed CBOR, and these broadcast it and wait for confirmation.
submitTx previously returned the first 64 hex characters of the submitted
CBOR when the response failed to parse - a string indistinguishable from a
real transaction hash. It now refuses via a new exported
parseSubmitResponse, since these tools are the first callers to actually
surface that value to an agent. Extracted to module scope and tested
directly (18 cases) rather than through submitTx itself, since provider.ts
imports fetch from cross-fetch rather than reading the global - stubbing it
would leave the real implementation live and hit a genuine DNS lookup.
vector_await_transaction's polling loop is extracted into vector/poll.ts
(pollUntilConfirmed) rather than left inline in the tool. Two reasons:
- The original loop slept a fixed 3s every iteration regardless of budget
remaining, so a short timeoutSeconds could overshoot its deadline by
close to 3x (e.g. timeoutSeconds=1 could block ~3s). The sleep is now
clamped to the time left before the deadline.
- Extracting it makes the loop unit-testable (11 cases) with an injected
fake clock, instead of only being reachable through a live network call.
Does not reuse the provider's own awaitTx: it hard-codes a 60-attempt
budget with no caller control and returns a bare boolean, collapsing
"still pending" and "gave up" into one signal - the tool needs a
configurable timeout and distinct wording for the two, since a pending tx
may still land and must not be reported as failed.
Judgement call on the brief's fixture: its final test asserted
parseSubmitResponse(CBOR.slice(0, 64)) throws. With the simple 64-hex-char
isTxHash check specified in the brief, 'ab'.repeat(32) legitimately
qualifies as a hash - it IS 64 valid hex characters - so that assertion was
changed to assert.equal instead of contorting the implementation to add
stricter validation the brief didn't ask for. The invariant that matters
still holds: parseSubmitResponse only ever receives the HTTP response body,
never the submitted tx, so this value can no longer be invented by our own
code from data it already held - only echoed back by a server, in which
case treating it like any other bare-text hash is correct.
Beyond the brief: added coverage for JSON responses in shapes the parser
doesn't recognize (array/number/null/boolean/nested object/unhandled field
name), the "id" field alias, a zero-budget/never-yielding poll, and that a
rejection from the confirmation check propagates instead of being read as
"not yet confirmed".
test:unit 96/96, typecheck clean. test:smoke fails as expected (23 -> 25,
naming vector_await_transaction and vector_submit_transaction as added) -
regenerating that snapshot is Task 5's job. test:integration:auth 12/12
unaffected.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Tasks 2 and 3 verified their work with throwaway scripts against a real
spawned server that were never committed, so npm test protected none of it -
index.ts's auth gate had zero automated coverage, and the @ts-nocheck
agent-network.ts/self-improvement.ts rate-limit wiring had none beyond
listTools().
Adds two hermetic integration suites (real spawned server, localhost only,
no wallet/chain calls):
- auth-gating.test.ts: both HTTP routes' 401/403 gating, the auth-before-
session-lookup ordering, cross-identity rejection, token-leak scanning,
and that the server survives all of it (the reviewer's severe finding -
a missing early `return` after a 401 falls through into a second
writeHead() and crashes the whole process via an unhandled rejection).
- rate-limit-gating.test.ts: per-identity isolation and that exhaustion on
a vector.ts tool also rate-limits agent-network.ts and self-improvement.ts
tools for that identity, through the real registered tools rather than
limiterFor() in isolation.
Extends test/setup.ts additively (spawnServer/stopRawServer/RawServerHandle)
so these tests can set MCP_AUTH_TOKENS and drive per-request Authorization
headers by hand; startServer()/stopServer()/callTool() are unchanged and the
existing run.test.ts still fails for exactly its pre-existing reason.
Wires a new test:integration:auth script into CI's build-and-test job.
All five mutations from the review were applied to a working-tree copy,
built, and reverted per-mutation to prove the new tests actually fail:
- removing either route's early `return` after a 401: caught (cascading
ECONNREFUSED once the process crashes; a dedicated liveness test targets
this directly)
- the cross-identity check's undefined-lookup branch: not independently
catchable - transports/sessionIdentities are always populated and
cleared together, synchronously, so that state is unreachable via HTTP;
documented in the report, verified a same-region operator-flip mutation
IS caught cleanly
- module-scope `identity` reference in self-improvement.ts: crashes the
server at startup (ReferenceError at import time), caught via the
before() hook
- checkRateLimit restored to module scope in agent-network.ts: caught
cleanly, isError:true / "rateLimiter is not defined" instead of the
well-formed rate-limit text
No file under any src/ changed. test:unit 67/67, test:smoke 26/26 (23
tools, 0 drift), typecheck clean, new suite 12/12.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The limiter was a module-level singleton, so one busy caller throttled every
other caller on the box and a shared daily budget was trivially exhausted.
Buckets are now keyed by authenticated identity, and keyed centrally so
reconnecting does not hand back a clean bucket.
POST /messages is gated too, and cross-checked against the session's own
identity - otherwise a valid token for one caller could drive another
caller's session by guessing its id.
All four loadAuthConfig throw sites could be made to interpolate the secret
that triggered them with the suite staying fully green - the one constraint
this module states twice in its own header had no regression protection.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Hosted deployments set MCP_AUTH_TOKENS; localhost self-hosters are unaffected.
The resolved identity doubles as the rate-limit bucket key. Rejection reasons
never echo the supplied token, and a bare token's log label is a hash prefix.
Hardened beyond the brief after an adversarial pass (20 additional tests):
- A malformed MCP_AUTH_TOKENS that parses to zero usable identities (e.g.
",,," from a broken template) now throws instead of silently resolving to
`enabled: false` - the exact "operator believes auth is on, server is
actually open" failure shape this module exists to prevent. An explicitly
empty/unset value still disables cleanly, unchanged.
- A token containing embedded whitespace is now rejected at config-load
time: resolveIdentity's \S+ match can never span it, so without this check
the token would be configured yet permanently unpresentable.
- Documented and pinned two non-fixable format limitations: only the first
colon in an entry separates label from token (a label can't itself contain
one), and a literal comma inside a label/token silently fragments the
entry since the delimiter has no escape mechanism.
- Pinned correct existing behaviour: exact case-sensitive token match, no
prefix/superstring matching, Map-backed storage is immune to
__proto__/constructor prototype pollution, one label may legitimately
cover multiple tokens (rotation), empty-string header treated as missing.
- Map.get token lookup is not constant-time; judged acceptable and
documented inline rather than swapped for crypto.timingSafeEqual - see
the comment above the lookup in auth.ts for the reasoning.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The review verified six obfuscations that bypass boundary.test.ts's ambient-
global and forbidden-import checks: globalThis['fetch'](...), an alias
(const f = fetch; f(...)), an aliased bracket reference, the comma-operator
trick ((0, globalThis.fetch)(...)), and two dynamic import()s with a
concatenated specifier (import('node:' + 'dns'), import('child_' +
'process')). README.md called the check "the package's central security
guarantee" and boundary.test.ts's header claimed the signer "CANNOT reach
the network -- not that it chooses not to." Neither line was true of what a
text scan can actually promise.
Hardened cheaply, within reach of a text scan:
- FORBIDDEN_IMPORTS gains node:dns, node:dns/promises, node:http2,
node:worker_threads, node:vm, each with a one-line rationale.
- AMBIENT_NETWORK_GLOBALS gains globalThisQualifiedAccess(name), matching a
global reached via globalThis in either dot or bracket ("computed member
access") notation, plus navigator['sendBeacon'] bracket notation.
Verified empirically against the real hardened check (temporary probe
files under src/, one per evasion, deleted immediately after -- see the
task report for the full per-file breakdown): globalThis['fetch'](...),
the aliased-bracket-reference variant, and the comma-operator
globalThis.fetch trick are now all caught. The plain-identifier alias and
both concatenated dynamic imports still pass -- those need real data-flow
tracking or evaluating an arbitrary runtime expression, neither of which a
text scan can do, and no attempt was made to chase them.
Reworded both the README section and boundary.test.ts's header comment to
say what the check actually verifies: today's shipped src/ has no
accidental or careless network capability. Deliberate obfuscation is named
as a code-review responsibility, not something this file promises to catch.
Final whole-branch review, Fix 4 of 4.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
decode.ts's trailing-bytes consumption check re-serialises a parsed
transaction and compares it against the input, refusing on any mismatch.
That check relies on CML's parse-then-reserialise round trip preserving the
agent registry's indefinite-length CBOR constructor encoding (D8 79 9F ...
FF, required by the Conway-era Aiken validators; Lucid's Data.to emits the
definite-length form instead, which those validators reject) rather than
silently canonicalising it -- if it did not, the consumption check would
misread that harmless re-encoding as unrecognised trailing data and refuse
every transaction using this ecosystem's own real encoding.
sign.test.ts already pins this exact claim for sign.ts's own, independent
from_cbor_hex call, and its own header comment flagged decode.ts as having
"the exact same untested gap" -- out of that task's file scope, left for a
later task. This is that closure: the same fixture (built with the same
cborBytes/cborUint helpers, embedded both as a body datum and a witness-set
datum) ported to exercise decodeTransaction directly.
Verified by mutation (swapping the comparison to tx.to_canonical_cbor_hex()):
probed directly first and confirmed canonical re-serialisation really does
rewrite this fixture's bytes, then confirmed exactly the 4 tests that call
decodeTransaction() on it fail, while the sanity test and the byte-survival
test (neither of which call decodeTransaction) are unaffected -- see the
task report.
Final whole-branch review, Fix 3b of 4.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
decode.ts's readAssets() builds each asset's `unit` as
`policy.to_hex() + name.to_hex()`, deliberately not
`name.to_cbor_hex().slice(2)` -- the CBOR byte-string header is 1 byte for
names up to 23 bytes but grows to 2 bytes at 24 (Cardano asset names run up
to 32 bytes), so a fixed slice(2) leaves the extra header byte glued onto
the front of the unit for every 24-32 byte name. That fix shipped with this
module's first commit but was only ever verified by a throwaway probe --
nothing in the suite constructed a real CML multi-asset output, across 8
tasks and 22 commits.
Builds a real TransactionOutput carrying a MultiAsset value with three
deterministic asset names (23 bytes, exactly 24, and the 32-byte max) under
one policy, re-serialised through Transaction.new(...) so the fixture
exercises decodeTransaction's full parse path, not just readAssets() in
isolation. Asserts the decoded unit is exactly policyHex + assetNameHex.
Verified by mutation (reverting to name.to_cbor_hex().slice(2)): the 24-byte
and 32-byte tests fail with the corrupted unit carrying a stray leading
header byte (18.../20... prefix); the 23-byte test is correctly unaffected,
since both forms agree below the boundary -- see the task report.
Final whole-branch review, Fix 3a of 4.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Number(env.VECTOR_SIGNER_ACCOUNT_INDEX ?? '0') was never validated. Verified
against the real walletFromSeed: a NaN-producing value, a negative integer,
and a fractional value each derived a DIFFERENT, syntactically valid
addr1... wallet with no error anywhere in the chain, and describe() would
echo the nonsense value back as if nothing were wrong ("mnemonic (account
index NaN)"). Spend limits three lines away already fail loud on the same
shape of mistake (BigInt(...) throws on garbage) -- this applies that same
instinct here: parseAccountIndex() rejects anything that is not a
non-negative integer, naming the variable and the offending value.
Only called from the two MnemonicKeySource-constructing branches, not
unconditionally -- PrivateKeyKeySource never reads an account index, so a
leftover garbage VECTOR_SIGNER_ACCOUNT_INDEX must not block a
private-key-configured signer that never uses it. Pinned by its own test.
config.test.ts covers: NaN-producing input, a negative integer, a fractional
value, a valid value (cross-checked against a directly-constructed
MnemonicKeySource for the same wallet), the unset default, and the
PrivateKeyKeySource non-interference case. Verified by mutation (temporarily
disabling the guard) that exactly the three invalid-input tests fail and the
other three are unaffected -- see the task report.
Final whole-branch review, Fix 2 of 4.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
vector-signer-audit-log.json resolves inside the repo checkout by default,
and the README self-disclosed that it was NOT in .gitignore (unlike the
builder's vector-audit-log.json). A documented footgun is worse than no
footgun when the fix is one line. Adds the entry and updates the README
paragraph that described the gap to describe the fix instead.
Final whole-branch review, Fix 1 of 4.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Includes an opt-in testnet round-trip test (keyless build -> decode -> policy
-> sign, never submits), excluded from CI because it needs a funded wallet.
Run against the real funded testnet wallet: pass 3, fail 0, nothing submitted.
Verified every README claim against the code as implemented, not just the
brief text, and corrected several:
- VECTOR_SIGNER_AUDIT_LOG_PATH's default resolves against the package's own
install location (repo root), not the process's cwd as originally stated --
and that default filename is not yet covered by .gitignore.
- An audit-log write failure blocks ALL signing, not just accounting:
audit.append() throws by design and vector_signer_sign withholds an
already-produced signature rather than release one with no durable record.
- Documented the #private-field defense against accidental key leakage via
JSON.stringify/console.log, and the read-only/sign tool split that keeps
the signing key out of scope for get_address/decode/get_spend_limits.
- Documented that pairing this signer with the CURRENT hosted builder does
not yet remove the custody exposure it's designed to fix: no builder tool
is keyless yet, and there is no submit_transaction tool to hand a produced
signature to. That's the next stage of the migration, not shipped here.
Env-var cross-check (brief's Step 6, both directions) passes clean.
A decode_transaction that signs only when policy-allowed, worded to avoid
"sign", passed all 21 tests - a preview tool returning real signed bytes with
no audit entry and no budget consumed. And the stdout-purity constraint was
only enforced incidentally by an unrelated stderr sanity check, while the MCP
client silently swallows unparseable stdout lines.
Gap 1 (decode-can-sign) is closed structurally, not just by test: tools.ts
splits into registerReadOnlyTools() (get_address, decode_transaction,
get_spend_limits - receives only signerAddress/limits/auditLogPath/audit,
never config or keySource) and registerSignTool() (the only function in the
module whose closure ever holds a loaded SigningIdentity). Reproducing the
review's exact mutation against the new code is now a compile error
(TS2304: Cannot find name 'identity') rather than a passing test suite.
Defense-in-depth on top of that: a new content-based test independently
computes what signing the previewed transaction would actually produce and
asserts decode's response never contains those bytes, under any wording -
this is what catches a more determined mutation that deliberately re-widens
the read-only context to reach signing capability again.
Gap 2 (stdout purity) is closed with a purpose-built minimal Transport
(RawCapturingStdioTransport) that captures every raw stdout byte independently
of the SDK's own lenient line parsing, while still driving the real Client
class for a correct initialize handshake. Asserts every line written to
stdout, across a healthy startup and a normal tool call, parses as JSON.
Both fixes reproduced-then-verified: applying the review's mutations first
confirmed each gap was real (all tests green under the vulnerable code),
then confirmed each new check catches it, then confirmed restoring the fix
returns to 23/23 with no collateral. Also re-ran the original hard-requirement
mutations (signed- and refused-path unguarded audit.append, the exact
catch{warn();return signature} anti-pattern, and a missing return after the
refused branch that lets a refused transaction fall through to real signing)
against the restructured tools.ts to confirm the refactor weakened nothing -
none did; the missing-return mutation alone is caught by 4 tests, including
3 of the brief's own original 9.
typecheck 0, test:unit 180/180, builder test:smoke 26/23/0, boundary 4/4,
signer test:smoke:signer 23/23 (9 brief + 12 original edge-case pass + 2 new).
Hard requirement for this task: audit.ts's append() throws by design on a
write failure (see its own CONTRACT note). The trap is catching that throw
and still returning the signature already produced - that reproduces exactly
the failure the throw exists to prevent, one call site later: a real
signature reaches the caller with no durable record of it, so a restarted
process's committedTodayLovelace() silently under-counts and the daily limit
stops binding.
vector_signer_sign now wraps both audit.append() calls explicitly:
- Signed path: if the append throws, the signature is withheld. The
function returns before ever reading signed.signedCborHex - the only
`return` that reaches it is the one after a successful append. The
response tells the caller a valid signature WAS produced but could not be
durably recorded, so it is being withheld rather than released.
- Refused path (lower stakes - no funds move either way): if the append
throws, the refusal itself is still delivered with its concrete policy
reason (never silently replaced by a generic disk-error message), plus a
note that this specific refusal could not be recorded, so the caller does
not mistake an incomplete audit trail for a normal one.
Mutation-tested against the brief's own unguarded version (see the task
report for the full transcript): reverting to unwrapped `audit.append()`
calls does NOT leak signed CBOR either way (the throw prevents the function
from ever reaching its return statement) - but it does replace "REFUSED.
<reason>" with the SDK's generic isError conversion of audit.ts's own
message on BOTH paths, so a refusal's own policy reason becomes invisible
to the caller exactly when the audit log is unhealthy. Exactly 2 of 21
tests failed under that mutation, precisely the two targeting this gap, no
collateral.
Edge-case pass added to the smoke test (12 new tests, each on its own
dedicated server instance):
- loadConfig() / the built server's behaviour with no key material
configured at all (clear throw; process exits non-zero and promptly,
never hangs, never writes anything to stdout).
- No mnemonic word or the derived private key appears in the startup
banner or in any tool response across a sweep that includes a real
successful sign.
- decode_transaction never writes an audit entry, even for an in-policy
transaction.
- An undecodable transaction passed to sign() never reaches evaluate(), so
there is nothing to audit yet - by design, pinned rather than assumed.
- get_spend_limits surfaces the concrete refusal reason, not just the
word REFUSED.
- The hard requirement itself: a control test proving the in-policy
fixture genuinely signs under a healthy audit log, then the same
transaction against an unwritable audit log withholding the signature,
then a pin of the (intentional, conservative) consequence that the
withheld signature still counts toward this process's own committed-
today total even though nothing was ever released.
- The refused-path decision under the same unwritable-audit-log condition.
Also reworded decode_transaction's response text ("(read-only)" /
"would be ALLOWED" instead of "(not signed)" / "would be signed") so it
never contains the substring "signed" even when a decoded transaction
happens to be in-policy - matches this same smoke test's own
`!/signed/i.test(text)` check on the brief's decode test, which otherwise
fails against the brief's own unmodified wording on every call.
typecheck 0, test:unit 180/180, builder test:smoke 26/23/0, boundary 4/4,
signer test:smoke:signer 21/21 (9 brief + 12 edge-case pass).
stdio transport only - no HTTP listener, no Provider, no outbound network.
Wires together decode.ts/policy.ts/sign.ts/audit.ts/keysource.ts (Tasks 2-6)
behind four tools: vector_signer_get_address, vector_signer_decode_transaction,
vector_signer_sign, vector_signer_get_spend_limits.
Two corrections needed beyond the brief's given code to reach its own stated
"pass 9, fail 0" outcome, both verified necessary by reproducing the failure
first:
- packages/signer/tsup.config.ts: target bumped es2020 -> es2022. index.ts's
top-level `await server.connect(...)` is ES2022 syntax; esbuild hard-errors
("Top-level await is not available in the configured target environment")
under the package's original es2020 target rather than attempting an unsafe
downlevel transform. Scoped to the signer package only - the builder never
uses top-level await and its own tsup.config.ts is untouched.
- tools.ts's decode_transaction response text: "(not signed)" and "would be
signed" both contain the substring "signed", which collides with this same
smoke test's own `!/signed/i.test(text)` check on every call, including the
refused case. Reworded to "(read-only)" / "would be ALLOWED" - same meaning,
no collision. Fixed the response text rather than loosening the test, which
stays exactly as given.
Brief's own smoke test (9 tests) passes verbatim. Workspace-wide: typecheck 0,
test:unit 180/180 (no regressions), builder test:smoke still 26/23/0.
The audit-append-failure handling required by the task (a produced signature
must never be released if its audit record fails to write) is not yet in this
commit - see the follow-up commit.
is_valid lives on Transaction rather than TransactionBody, so hardcoding it
was invisible to every hash-equality test - flipping it passed all 21. For a
deliberately-invalid script transaction that silently rewrites the claim a
node re-checks in phase 2.
Also pins the indefinite-length CBOR round trip that the trailing-bytes guard
depends on, which until now was asserted only in a comment.
from_cbor_hex only requires a valid prefix, so appending bytes to a real
transaction let sign.ts silently sign a shorter transaction than the caller
supplied and return a confident result — a signature over different bytes
than what was asked for, with nothing to reveal the discrepancy. sign.ts
calls from_cbor_hex independently of decode.ts and does not inherit that
module's consumption check (commit 5da4447) automatically; this closes the
same gap here, with the identical case-insensitive comparison and reasoning.
Also closes an untested claim: decode.ts's own suite documents but never
pins that the case-insensitive comparison tolerates legitimate uppercase-hex
input without a false positive. Pinned here for sign.ts.
Edge-case pass beyond the brief's 7 tests, all reproduced against the
unmodified implementation before any fix and mutation-verified after:
- trailing bytes silently accepted (BUG, fixed above)
- existing vkey witnesses are preserved and appended to, not replaced
(already correct; mutation-tested by dropping the copy loop)
- two different keys produce two independently-verifying witnesses,
chained or independent (already correct)
- determinism strengthened to compare raw signature bytes across a
freshly re-derived key instance, not just the memoized one (already
correct)
- auxiliary data / metadata survives signing byte-for-byte, body hash
unchanged (already correct)
- no thrown error, on any path, ever echoes the private key value
(already correct; mutation-tested by deliberately leaking it)
Lucid's constructor requires a Provider, which would put a network object in
the signer. The CML path was verified to produce an identical tx hash and
byte-identical signed output. Preserves any pre-existing witnesses.
