fix(builder): close boundary-test gaps, drop dangling spend-limits copy, guard tool descriptions end-to-end
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>
This commit is contained in:
@@ -1,6 +1,9 @@
|
||||
// packages/builder/scripts/regen-tool-schemas.mjs
|
||||
// Boots the BUILT server, lists tools over SSE, rewrites the smoke snapshot
|
||||
// sorted by name. Run from the repo root: npm run build && node packages/builder/scripts/regen-tool-schemas.mjs
|
||||
// sorted by name. Run: npm run build && node packages/builder/scripts/regen-tool-schemas.mjs
|
||||
// (npm run build must still run from the repo root - or via `npm run build
|
||||
// --workspace=...` - since it's an npm script; but this script itself now
|
||||
// runs from any cwd, since its paths are resolved from its own location).
|
||||
//
|
||||
// Deviation from the original brief version: the brief used a flat 2000ms
|
||||
// sleep before connecting. On this machine that raced the server's actual
|
||||
@@ -18,11 +21,18 @@
|
||||
// packages/builder/test/smoke/tool-inventory.test.ts for the full rationale.
|
||||
import { spawn } from 'node:child_process';
|
||||
import { writeFileSync } from 'node:fs';
|
||||
import { resolve } from 'node:path';
|
||||
import { Client } from '@modelcontextprotocol/sdk/client/index.js';
|
||||
import { SSEClientTransport } from '@modelcontextprotocol/sdk/client/sse.js';
|
||||
|
||||
// Resolved from this file's own location, not from process.cwd(), so the
|
||||
// script behaves identically regardless of where it's invoked from (repo
|
||||
// root, packages/builder, or anywhere else).
|
||||
const SERVER_ENTRY = resolve(import.meta.dirname, '../build/index.js');
|
||||
const SNAPSHOT_PATH = resolve(import.meta.dirname, '../test/smoke/tool-schemas.snapshot.json');
|
||||
|
||||
const PORT = 3939;
|
||||
const server = spawn('node', ['packages/builder/build/index.js'], {
|
||||
const server = spawn('node', [SERVER_ENTRY], {
|
||||
env: { ...process.env, PORT: String(PORT) },
|
||||
stdio: ['ignore', 'ignore', 'pipe'],
|
||||
});
|
||||
@@ -54,7 +64,7 @@ try {
|
||||
.sort(([a], [b]) => a.localeCompare(b)),
|
||||
);
|
||||
writeFileSync(
|
||||
'packages/builder/test/smoke/tool-schemas.snapshot.json',
|
||||
SNAPSHOT_PATH,
|
||||
JSON.stringify(snapshot, null, 2) + '\n',
|
||||
);
|
||||
console.log(`wrote ${tools.length} tool schemas`);
|
||||
|
||||
@@ -357,7 +357,7 @@ Save your Agent DID - you'll need it to update, deregister, or let other agents
|
||||
|
||||
**Troubleshooting Tips:**
|
||||
1. Ensure wallet has at least 12 AP3X (10 deposit + fees)
|
||||
2. Check spend limits with vector_get_spend_limits
|
||||
2. Server spend limits for this tool are set via VECTOR_SPEND_LIMIT_PER_TX / VECTOR_SPEND_LIMIT_DAILY
|
||||
3. Each wallet can register multiple agents (different seed UTxOs)
|
||||
4. Verify Ogmios endpoint is reachable at ${VECTOR_OGMIOS_URL}`,
|
||||
}],
|
||||
@@ -451,7 +451,7 @@ The agent's profile has been updated on-chain. The identity NFT and deposit are
|
||||
1. Verify the agent DID is correct
|
||||
2. Your wallet must be the agent's current owner
|
||||
3. Ensure wallet has enough AP3X for transaction fees
|
||||
4. Check spend limits with vector_get_spend_limits`,
|
||||
4. Server spend limits for this tool are set via VECTOR_SPEND_LIMIT_PER_TX / VECTOR_SPEND_LIMIT_DAILY`,
|
||||
}],
|
||||
};
|
||||
}
|
||||
@@ -672,7 +672,7 @@ The message is recorded on-chain in TX metadata label 674.`,
|
||||
1. Verify the agent DID is correct: did:vector:agent:{policyId}:{nftAssetName}
|
||||
2. Ensure wallet has at least 3 AP3X (2 AP3X delivery + fees)
|
||||
3. Use vector_get_agent_profile to verify the agent exists first
|
||||
4. Check spend limits with vector_get_spend_limits`,
|
||||
4. Server spend limits for this tool are set via VECTOR_SPEND_LIMIT_PER_TX / VECTOR_SPEND_LIMIT_DAILY`,
|
||||
}],
|
||||
};
|
||||
}
|
||||
|
||||
@@ -465,7 +465,7 @@ Each batch UTxO holds ~30 AP3X for adoption rewards.`,
|
||||
const stakeLovelace = stakeApex * 1_000_000;
|
||||
const safetyCheck = safetyLayer.checkTransaction(stakeLovelace + 4_000_000);
|
||||
if (!safetyCheck.allowed) {
|
||||
return { content: [{ type: "text", text: `Safety limit exceeded: ${safetyCheck.reason}. Check limits with vector_get_spend_limits.` }] };
|
||||
return { content: [{ type: "text", text: `Safety limit exceeded: ${safetyCheck.reason}. Server spend limits for this tool are set via VECTOR_SPEND_LIMIT_PER_TX / VECTOR_SPEND_LIMIT_DAILY.` }] };
|
||||
}
|
||||
|
||||
try {
|
||||
@@ -695,7 +695,7 @@ activity tracking token (\`pact_\`) minted. Visible on the Foundation dashboard.
|
||||
1. Ensure wallet has at least ${stakeApex + 5} AP3X (stake + fees)
|
||||
2. For Emergency proposals, your agent needs Established reputation (100+ AP3X staked in Game 3)
|
||||
3. proposalHash must be 64 hex characters (blake2b_256 of your proposal document)
|
||||
4. Check spend limits with vector_get_spend_limits`,
|
||||
4. Server spend limits for this tool are set via VECTOR_SPEND_LIMIT_PER_TX / VECTOR_SPEND_LIMIT_DAILY`,
|
||||
}],
|
||||
};
|
||||
}
|
||||
|
||||
@@ -175,7 +175,7 @@ ${utxoList}`,
|
||||
// vector_build_send_apex - build an UNSIGNED AP3X payment (keyless)
|
||||
server.tool(
|
||||
"vector_build_send_apex",
|
||||
"Build an UNSIGNED transaction sending AP3X to a recipient. Takes no key material - sign the returned CBOR with your local signer (vector_signer_sign), then broadcast with vector_submit_transaction.",
|
||||
"Build an UNSIGNED transaction sending AP3X to a recipient. Takes no key material - sign the returned CBOR with your local signer (vector_signer_sign), then broadcast with vector_submit_transaction and confirm with vector_await_transaction.",
|
||||
{
|
||||
changeAddress: z.string().describe("Your wallet address (source of funds; receives change). Get it from your local signer's vector_signer_get_address."),
|
||||
recipientAddress: z.string().describe("Recipient Vector address (addr1...)"),
|
||||
@@ -228,7 +228,7 @@ Non-custodial flow: 1) sign locally with vector_signer_sign { txCbor } — your
|
||||
// vector_build_send_tokens - build an UNSIGNED native-token transfer (keyless)
|
||||
server.tool(
|
||||
"vector_build_send_tokens",
|
||||
"Build an UNSIGNED transaction sending Vector native tokens. Takes no key material - sign the returned CBOR with your local signer (vector_signer_sign), then broadcast with vector_submit_transaction.",
|
||||
"Build an UNSIGNED transaction sending Vector native tokens. Takes no key material - sign the returned CBOR with your local signer (vector_signer_sign), then broadcast with vector_submit_transaction and confirm with vector_await_transaction.",
|
||||
{
|
||||
changeAddress: z.string().describe("Your wallet address (source of funds; receives change). Get it from your local signer's vector_signer_get_address."),
|
||||
recipientAddress: z.string().describe("Recipient Vector address (addr1...)"),
|
||||
@@ -285,7 +285,7 @@ Non-custodial flow: 1) sign locally with vector_signer_sign { txCbor }; 2) broad
|
||||
// vector_build_transaction - build an UNSIGNED multi-output transaction (keyless)
|
||||
server.tool(
|
||||
"vector_build_transaction",
|
||||
"Build an UNSIGNED multi-output transaction with optional metadata. Takes no key material and never submits - sign the returned CBOR with your local signer (vector_signer_sign), then broadcast with vector_submit_transaction.",
|
||||
"Build an UNSIGNED multi-output transaction with optional metadata. Takes no key material and never submits - sign the returned CBOR with your local signer (vector_signer_sign), then broadcast with vector_submit_transaction and confirm with vector_await_transaction.",
|
||||
{
|
||||
changeAddress: z.string().describe("Your wallet address (source of funds; receives change). Get it from your local signer's vector_signer_get_address."),
|
||||
outputs: z.array(z.object({
|
||||
@@ -602,7 +602,7 @@ ${txList}
|
||||
// vector_build_deploy_contract - build an UNSIGNED contract deployment (keyless)
|
||||
server.tool(
|
||||
"vector_build_deploy_contract",
|
||||
"Build an UNSIGNED transaction deploying a Plutus/Aiken smart contract (locks funds at its script address). Takes no key material - sign with your local signer, then broadcast with vector_submit_transaction.",
|
||||
"Build an UNSIGNED transaction deploying a Plutus/Aiken smart contract (locks funds at its script address). Takes no key material - sign with your local signer, then broadcast with vector_submit_transaction and confirm with vector_await_transaction.",
|
||||
{
|
||||
changeAddress: z.string().describe("Your wallet address (source of funds; receives change). Get it from your local signer's vector_signer_get_address."),
|
||||
scriptCbor: z.string().describe("Compiled Plutus/Aiken script in CBOR hex format"),
|
||||
@@ -657,7 +657,7 @@ Sign locally with vector_signer_sign, broadcast with vector_submit_transaction,
|
||||
// vector_build_interact_contract - build an UNSIGNED contract interaction (keyless)
|
||||
server.tool(
|
||||
"vector_build_interact_contract",
|
||||
"Build an UNSIGNED transaction interacting with a deployed Plutus/Aiken contract - lock AP3X at it or spend from it. Takes no key material - sign with your local signer, then broadcast with vector_submit_transaction.",
|
||||
"Build an UNSIGNED transaction interacting with a deployed Plutus/Aiken contract - lock AP3X at it or spend from it. Takes no key material - sign with your local signer, then broadcast with vector_submit_transaction and confirm with vector_await_transaction.",
|
||||
{
|
||||
changeAddress: z.string().describe("Your wallet address (source of funds, change, and the required signer for spends). Get it from your local signer's vector_signer_get_address."),
|
||||
scriptCbor: z.string().describe("Compiled Plutus/Aiken script in CBOR hex"),
|
||||
|
||||
@@ -16,7 +16,14 @@ const CUSTODIAL_UNTIL_LATER_PR = new Set([
|
||||
'vector/agent-network.ts', // family 2 — goes keyless in spec PR 7
|
||||
'vector/self-improvement.ts', // family 3 — goes keyless in spec PR 8
|
||||
]);
|
||||
const FORBIDDEN = /\b(mnemonic|seed|bip39|fromSeed|walletFromSeed|selectWallet\.fromSeed)\b/i;
|
||||
// No \b boundaries: this is deliberately a substring match, not a whole-word
|
||||
// one. Word-bounding let `seedPhrase`, `WALLET_SEED` or `bip39x` slip past
|
||||
// undetected — those are real key-material vocabulary wearing a different
|
||||
// word shape, not false positives. `fromSeed`/`walletFromSeed` are already
|
||||
// covered as substrings of `seed`, but stay in the alternation anyway for
|
||||
// self-documentation (they cost nothing and name the exact API shape this
|
||||
// guard exists to catch).
|
||||
const FORBIDDEN = /(mnemonic|seed|bip39|fromSeed|walletFromSeed|selectWallet\.fromSeed)/i;
|
||||
|
||||
function sourceFiles(dir: string): string[] {
|
||||
const out: string[] = [];
|
||||
@@ -51,4 +58,32 @@ describe('builder custody boundary', () => {
|
||||
);
|
||||
}
|
||||
});
|
||||
|
||||
// The allowlist may only shrink (PR 7 drops agent-network.ts to reach 1, PR 8
|
||||
// drops self-improvement.ts to reach 0 - see the "goes keyless in spec PR N"
|
||||
// comments above). Pinning the size means any widening is a deliberate,
|
||||
// reviewable number change instead of a silent `.add(...)` slipping through.
|
||||
test('the allowlist may only shrink', () => {
|
||||
assert.equal(CUSTODIAL_UNTIL_LATER_PR.size, 2, 'the custodial allowlist may only shrink - PR 7 takes it to 1 (drop agent-network), PR 8 to 0');
|
||||
});
|
||||
|
||||
// Pins every FORBIDDEN alternative individually against a fixture corpus, so
|
||||
// deleting one alternative from the regex (e.g. "we don't need bip39
|
||||
// anymore") fails a test by name instead of silently narrowing what this
|
||||
// guard catches. Also pins representative keyless vocabulary that must NOT
|
||||
// trip the guard, so widening FORBIDDEN too aggressively is equally visible.
|
||||
test('every FORBIDDEN alternative is individually pinned against a fixture corpus', () => {
|
||||
const mustMatch = [
|
||||
'mnemonic', 'MNEMONIC', 'seedPhrase', 'WALLET_SEED', 'bip39',
|
||||
'fromSeed', 'walletFromSeed', 'selectWallet.fromSeed', 'seed',
|
||||
];
|
||||
for (const s of mustMatch) {
|
||||
assert.match(s, FORBIDDEN, `FORBIDDEN should match "${s}" - deleting the alternative that catches this would silently narrow the guard`);
|
||||
}
|
||||
|
||||
const mustNotMatch = ['address-only wallet', 'unsigned transaction', 'changeAddress'];
|
||||
for (const s of mustNotMatch) {
|
||||
assert.doesNotMatch(s, FORBIDDEN, `FORBIDDEN should NOT match "${s}" - this is legitimate keyless vocabulary`);
|
||||
}
|
||||
});
|
||||
});
|
||||
|
||||
@@ -19,8 +19,8 @@ import type { RawServerHandle } from '../setup.ts';
|
||||
// validates the address with getAddressDetails() and throws BEFORE any
|
||||
// provider call - proven by Task 2's booby-trapped-provider test. Handlers
|
||||
// return caught errors as normal content, so isError stays false and this
|
||||
// probe is just as hermetic as the vector_get_spend_limits tool it replaces
|
||||
// (which vector.ts no longer has - Task 5 deleted it, since SafetyLayer state
|
||||
// probe is just as hermetic as the spend-limit-status tool it replaces
|
||||
// (vector.ts no longer has one - Task 5 deleted it, since SafetyLayer state
|
||||
// is no longer exposed to keyless callers).
|
||||
// vector_get_agent_profile (agent-network.ts) and
|
||||
// vector_self_improvement_analyze_metrics (self-improvement.ts) DO reach
|
||||
|
||||
@@ -7,9 +7,10 @@ import { startServer, stopServer, ServerContext } from '../setup.ts';
|
||||
/**
|
||||
* Snapshot of the current tool surface (23 tools) for the non-custodial split
|
||||
* series (see docs/architecture/non-custodial-split.md). PR 6 (Task 5) is the
|
||||
* PR that took this list from 25 to 23: it deleted `vector_get_address` and
|
||||
* `vector_get_spend_limits` outright (the safety-layer/wallet-info tools have
|
||||
* no keyless equivalent — spend limits are gone because the builder no longer
|
||||
* PR that took this list from 25 to 23: it deleted `vector_get_address`
|
||||
* outright, along with the tool that reported per-transaction/daily
|
||||
* SafetyLayer spend status (the safety-layer/wallet-info tools have no
|
||||
* keyless equivalent — spend status is gone because the builder no longer
|
||||
* holds keys to limit) as part of making the remaining query tools
|
||||
* (`vector_get_utxos`, `vector_get_transaction_history`, `vector_dry_run`)
|
||||
* keyless. PRs 2, 6, 7 and 8 each deliberately restructure and rename tool
|
||||
@@ -141,6 +142,14 @@ describe('tool schema snapshot', () => {
|
||||
live,
|
||||
`Tool "${toolName}" is in the snapshot but was not returned by the live server's listTools().`
|
||||
);
|
||||
// Named separately from the deepStrictEqual below: a regen that
|
||||
// silently drops descriptions (e.g. JSON.stringify dropping an
|
||||
// `undefined` description key) should fail by naming exactly that,
|
||||
// not just show up as an opaque diff.
|
||||
assert.ok(
|
||||
'description' in (schemaSnapshot[toolName] as any),
|
||||
`Snapshot entry for "${toolName}" has no "description" key in test/smoke/tool-schemas.snapshot.json.`
|
||||
);
|
||||
assert.deepStrictEqual(
|
||||
{ description: live.description, inputSchema: live.inputSchema },
|
||||
schemaSnapshot[toolName],
|
||||
|
||||
@@ -22,7 +22,7 @@
|
||||
}
|
||||
},
|
||||
"vector_build_deploy_contract": {
|
||||
"description": "Build an UNSIGNED transaction deploying a Plutus/Aiken smart contract (locks funds at its script address). Takes no key material - sign with your local signer, then broadcast with vector_submit_transaction.",
|
||||
"description": "Build an UNSIGNED transaction deploying a Plutus/Aiken smart contract (locks funds at its script address). Takes no key material - sign with your local signer, then broadcast with vector_submit_transaction and confirm with vector_await_transaction.",
|
||||
"inputSchema": {
|
||||
"type": "object",
|
||||
"properties": {
|
||||
@@ -61,7 +61,7 @@
|
||||
}
|
||||
},
|
||||
"vector_build_interact_contract": {
|
||||
"description": "Build an UNSIGNED transaction interacting with a deployed Plutus/Aiken contract - lock AP3X at it or spend from it. Takes no key material - sign with your local signer, then broadcast with vector_submit_transaction.",
|
||||
"description": "Build an UNSIGNED transaction interacting with a deployed Plutus/Aiken contract - lock AP3X at it or spend from it. Takes no key material - sign with your local signer, then broadcast with vector_submit_transaction and confirm with vector_await_transaction.",
|
||||
"inputSchema": {
|
||||
"type": "object",
|
||||
"properties": {
|
||||
@@ -139,7 +139,7 @@
|
||||
}
|
||||
},
|
||||
"vector_build_send_apex": {
|
||||
"description": "Build an UNSIGNED transaction sending AP3X to a recipient. Takes no key material - sign the returned CBOR with your local signer (vector_signer_sign), then broadcast with vector_submit_transaction.",
|
||||
"description": "Build an UNSIGNED transaction sending AP3X to a recipient. Takes no key material - sign the returned CBOR with your local signer (vector_signer_sign), then broadcast with vector_submit_transaction and confirm with vector_await_transaction.",
|
||||
"inputSchema": {
|
||||
"type": "object",
|
||||
"properties": {
|
||||
@@ -170,7 +170,7 @@
|
||||
}
|
||||
},
|
||||
"vector_build_send_tokens": {
|
||||
"description": "Build an UNSIGNED transaction sending Vector native tokens. Takes no key material - sign the returned CBOR with your local signer (vector_signer_sign), then broadcast with vector_submit_transaction.",
|
||||
"description": "Build an UNSIGNED transaction sending Vector native tokens. Takes no key material - sign the returned CBOR with your local signer (vector_signer_sign), then broadcast with vector_submit_transaction and confirm with vector_await_transaction.",
|
||||
"inputSchema": {
|
||||
"type": "object",
|
||||
"properties": {
|
||||
@@ -210,7 +210,7 @@
|
||||
}
|
||||
},
|
||||
"vector_build_transaction": {
|
||||
"description": "Build an UNSIGNED multi-output transaction with optional metadata. Takes no key material and never submits - sign the returned CBOR with your local signer (vector_signer_sign), then broadcast with vector_submit_transaction.",
|
||||
"description": "Build an UNSIGNED multi-output transaction with optional metadata. Takes no key material and never submits - sign the returned CBOR with your local signer (vector_signer_sign), then broadcast with vector_submit_transaction and confirm with vector_await_transaction.",
|
||||
"inputSchema": {
|
||||
"type": "object",
|
||||
"properties": {
|
||||
|
||||
Reference in New Issue
Block a user