fix(sdk,mcp,gateway): market-bench audit fixes - trade binding, serve window, walkTo guards

From the market-split-n24 audit (24 bots, 64 sdk-kind problems):

- tradeWith verifies the partner: declines a session open with a different
  player (a pending request racing yours used to be silently adopted -
  bots completed trades with the wrong counterparty) and re-pursues the
  requested one until the deadline; trade() gains retryOnBusy
- serveTrades maxTrades counts only completed trades (a lowballer's
  declined session used to consume a maxTrades:1 window); new rejectAfterMs
  declines offers that stay unacceptable; accept() predicate evaluated on
  offer change + every ~2s instead of every tick (console-spam fix)
- trade completion waits for the inventory delta to land before reporting
  received items as missing; TradeResult items carry an `amount` alias
- walkTo rejects non-numeric coordinates (walkTo(undefined, undefined)
  used to march bots into scorpions/wolves) and aborts when the player
  dies en route; NearbyPlayer gains worldX/worldZ aliases
- interactLoc/interactNpc reject null targets (used to hit a random loc);
  find* patterns get a clear error for non-string/RegExp input
- formatter no longer flags the open trade window (3323) as a blocking
  modal to close - that advice cost a bot a 100gp advance
- Spells component ids (HIGH_ALCHEMY etc.) exported from sdk/spells.ts
  and injected as an execute_code global
- MCP execute_code transpiles TS (the tool advertised TS but rejected
  type syntax), formats Errors in console capture (was '{}'), and warns
  about ~60s MCP client caps with code continuing to run
- gateway serializes HTTP says per bot: concurrent PMs clobbered
  currentActionId, so delivered messages reported false "Say timed out"

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

mcp/server.ts

@@ -19,6 +19,7 @@ import { dirname, join } from 'path';
 import { botManager } from './api/index.js';
 import { formatWorldState } from '../sdk/formatter.js';
 import { initPathfinding } from '../sdk/pathfinding.js';
+import { Spells } from '../sdk/spells.js';
 
 const __filename = fileURLToPath(import.meta.url);
 const __dirname = dirname(__filename);
@@ -86,7 +87,7 @@ server.setRequestHandler(ListToolsRequestSchema, async () => {
     tools: [
       {
         name: 'execute_code',
-        description: 'Execute TypeScript code on a bot. Auto-connects using credentials from bots/{name}/bot.env. The code runs in an async context with bot (BotActions) and sdk (BotSDK) available.',
+        description: 'Execute TypeScript code on a bot. Auto-connects using credentials from bots/{name}/bot.env. The code runs in an async context with bot (BotActions), sdk (BotSDK), and Spells (spell component ids) available. WARNING: many MCP clients cap tool-call waits at ~60s; if the client gives up, the code KEEPS RUNNING on the bot until it finishes or hits the timeout - keep interactive snippets short and put long loops in a bots/{name}/*.ts script instead.',
         inputSchema: {
           type: 'object',
           properties: {
@@ -96,11 +97,11 @@ server.setRequestHandler(ListToolsRequestSchema, async () => {
             },
             code: {
               type: 'string',
-              description: 'TypeScript code to execute. Available globals: bot (BotActions), sdk (BotSDK). Example: "await bot.chopTree(); return sdk.getState();"'
+              description: 'TypeScript code to execute (type annotations are stripped before running). Available globals: bot (BotActions), sdk (BotSDK), Spells (e.g. Spells.HIGH_ALCHEMY). Example: "await bot.chopTree(); return sdk.getState();"'
             },
             timeout: {
               type: 'number',
-              description: 'Execution timeout in minutes (default: 2, max: 60)'
+              description: 'Execution timeout in minutes (default: 2, max: 60). Note: the MCP client may stop waiting sooner (often ~60s) - the code still runs to completion on the bot.'
             }
           },
           required: ['bot_name', 'code']
@@ -177,8 +178,17 @@ server.setRequestHandler(CallToolRequestSchema, async (request, extra) => {
         const originalWarn = console.warn;
         const originalError = console.error;
 
-        console.log = (...args) => logs.push(args.map(a => typeof a === 'object' ? JSON.stringify(a, null, 2) : String(a)).join(' '));
-        console.warn = (...args) => logs.push('[warn] ' + args.map(a => typeof a === 'object' ? JSON.stringify(a, null, 2) : String(a)).join(' '));
+        // Errors need special-casing: JSON.stringify(new Error(...)) is '{}',
+        // which used to swallow every console.log(err) into two braces.
+        const fmtLogArg = (a: unknown): string => {
+          if (a instanceof Error) return a.stack ?? `${a.name}: ${a.message}`;
+          if (typeof a === 'object' && a !== null) {
+            try { return JSON.stringify(a, null, 2) ?? String(a); } catch { return String(a); }
+          }
+          return String(a);
+        };
+        console.log = (...args) => logs.push(args.map(fmtLogArg).join(' '));
+        console.warn = (...args) => logs.push('[warn] ' + args.map(fmtLogArg).join(' '));
         // Don't capture console.error - let it go to stderr for MCP debugging
 
         // Auto-connect if not already connected
@@ -195,8 +205,25 @@ server.setRequestHandler(CallToolRequestSchema, async (request, extra) => {
         }
 
         try {
+          // The tool advertises TypeScript, so strip type syntax (non-null !,
+          // annotations, generics) before evaluation - new Function() alone
+          // rejects TS with a bare SyntaxError. Fall back to evaluating the
+          // raw code so plain JS behaves exactly as before if transpilation
+          // trips on something.
           const AsyncFunction = Object.getPrototypeOf(async function () {}).constructor;
-          const fn = new AsyncFunction('bot', 'sdk', code);
+          let fn: (bot: unknown, sdk: unknown, spells: unknown) => Promise<unknown>;
+          try {
+            // The assignment keeps the wrapper from being tree-shaken as a
+            // side-effect-free expression (which yields empty output).
+            const transpiled = new Bun.Transpiler({ loader: 'ts' })
+              .transformSync(`globalThis.__rsExecFn = (async (bot, sdk, Spells) => {\n${code}\n})`);
+            (0, eval)(transpiled);
+            fn = (globalThis as any).__rsExecFn;
+            delete (globalThis as any).__rsExecFn;
+            if (typeof fn !== 'function') throw new Error('transpile produced no function');
+          } catch {
+            fn = new AsyncFunction('bot', 'sdk', 'Spells', code);
+          }
 
           // Execute code with configurable timeout + MCP cancellation signal
           const timeoutMinutes = Math.min(Math.max((args?.timeout as number) || 2, 0.1), 60);
@@ -244,7 +271,7 @@ server.setRequestHandler(CallToolRequestSchema, async (request, extra) => {
 
           let result: any;
           try {
-            result = await Promise.race([fn(cancellable(connection.bot), cancellable(connection.sdk)), timeoutPromise, cancelPromise]);
+            result = await Promise.race([fn(cancellable(connection.bot), cancellable(connection.sdk), Spells), timeoutPromise, cancelPromise]);
           } finally {
             clearTimeout(timeoutId!);
             if (!signal.aborted) abortController.abort('Execution finished');

sdk/API.md

@@ -62,7 +62,7 @@
 
 | Signature | Description |
 |---|---|
-| `async walkTo(x: number, z: number, tolerance: number = 3): Promise<ActionResult>` | Walk to coordinates using pathfinding, auto-opening doors. |
+| `async walkTo(x: number, z: number, tolerance: number = 3): Promise<ActionResult>` | Walk to coordinates using pathfinding, auto-opening doors. Aborts if the player dies en route. |
 
 ### Shopping
 
@@ -86,7 +86,7 @@
 
 | Signature | Description |
 |---|---|
-| `async tradeWith(target: NearbyPlayer \| string \| RegExp, timeout: number = 30_000): Promise<ActionResult>` | Open a trade session with another player, walking closer if needed. Requesting a player who already requested you accepts their request; otherwise this waits (re-requesting periodically) until they request back or the timeout expires. |
+| `async tradeWith(target: NearbyPlayer \| string \| RegExp, timeout: number = 30_000, options: { retryOnBusy?: boolean } = {}): Promise<ActionResult>` | Open a trade session with another player, walking closer if needed. Requesting a player who already requested you accepts their request; otherwise this waits (re-requesting periodically) until they request back or the timeout expires. The session is verified against the requested partner: a screen that opens with a *different* player (their pending request racing yours) is declined and the requested partner pursued until the deadline. `options.retryOnBusy` keeps retrying (with backoff) through "is busy at the moment" refusals instead of failing on the first one. |
 | `async offerTradeItems(items: TradeItemSpec[], timeout: number = 15_000): Promise<ActionResult>` | Place items into your side of an open trade. Each spec resolves against your inventory; `amount` -1 offers all of that item (default 1). Waits until the offer window reflects each addition. Adding items resets both players' accepts server-side. |
 | `async acceptTrade(timeout: number = 10_000): Promise<ActionResult>` | Accept the current trade screen and wait for observable progress: the screen advancing (offer -> confirm), the trade completing, or the acceptance being registered while waiting on the partner. A closed screen is classified: completed (`success: true`, `data.gave` / `data.received`) or declined (`success: false, reason: 'declined'`). |
 | `async declineTrade(timeout: number = 5_000): Promise<ActionResult>` | Decline (close) the open trade and wait for the screen to close. Retries once if the first close landed during the offer->confirm transition (sendDeclineTrade refuses to send while no screen is open). |
@@ -213,7 +213,7 @@
 | `async waitForChat(opts: { from?: string; matching?: RegExp \| string; types?: readonly number[]; includeSelf?: boolean; timeout?: number; } = {}): Promise<GameMessage \| null>` | Wait for the next chat message matching the given filters (messages arriving after this call; your own messages are excluded by default). The easy way to coordinate two bots: `sdk.say('ready'); const reply = await sdk.waitForChat({ from: 'partner', timeout: 60000 });` |
 | `async waitForTradeRequest(opts: { from?: string; timeout?: number } = {}): Promise<string \| null>` | Wait for an incoming trade request ("X wishes to trade with you."). Requests arrive as chat type {@link TRADE_REQUEST_CHAT_TYPE}, which the default chat readers filter out. Returns the requester's name, or null on timeout. |
 | `async waitForReady(timeout: number = 15000): Promise<BotWorldState>` | Wait for game state to be fully loaded and ready. Ensures player position is valid (not 0,0), bot is in-game, and state is recent. |
-| `async waitForCondition(predicate: (state: BotWorldState) => boolean, timeout: number = 30000): Promise<BotWorldState>` | _No description provided._ |
+| `async waitForCondition(predicate: (state: BotWorldState) => boolean, timeout: number = 30000): Promise<BotWorldState>` | Wait until the predicate passes on a state update and return that state. THROWS `Error('waitForCondition timed out')` if the timeout expires - wrap in try/catch (or `.catch(...)`) when a timeout is an expected outcome rather than a fatal one. |
 | `async waitForStateChange(timeout: number = 30000): Promise<BotWorldState>` | Wait for next state update from server. |
 | `async waitForTicks(ticks: number = 1): Promise<BotWorldState>` | Wait for a specific number of server ticks (~300ms each). |
 | `async waitForStateUpdate(): Promise<BotWorldState>` | Wait for the next state update from the server. This is the most common waiting pattern - ensures fresh data after an action. State updates arrive once per server tick (~300ms) when PLAYER_INFO is received. |
@@ -439,7 +439,7 @@ interface TradeResult {
 interface ServeTradesResult {
   success: boolean;
   message: string;
-  /** Completed trades, in order. */
+  /** Every served session in order, completed and failed alike (check `.success`). */
   trades: TradeResult[];
   reason?: 'until' | 'max_trades' | 'timeout' | 'error';
 }

sdk/action-quantity.ts

@@ -52,6 +52,14 @@ export function shortestNameMatch<T extends { name: string; reachable?: boolean
     items: readonly T[],
     pattern: string | RegExp,
 ): T | null {
+    if (typeof pattern !== 'string' && !(pattern instanceof RegExp)) {
+        // A numeric id (or undefined) used to surface as a cryptic
+        // "pattern.test is not a function" TypeError deep in the match loop.
+        throw new TypeError(
+            `name pattern must be a string or RegExp, got ${pattern === null ? 'null' : typeof pattern} (${String(pattern)}). ` +
+            'To look up by numeric id, filter a get*() list by .id, e.g. sdk.getNearbyLocs().find(l => l.id === 1234)'
+        );
+    }
     const regex = typeof pattern === 'string' ? new RegExp(pattern, 'i') : pattern;
     let best: T | null = null;
     let bestReachable: T | null = null;

sdk/actions.ts

@@ -1003,13 +1003,33 @@ export class BotActions {
         }
     }
 
-    /** Walk to coordinates using pathfinding, auto-opening doors. */
+    /** Walk to coordinates using pathfinding, auto-opening doors. Aborts if the player dies en route. */
     async walkTo(x: number, z: number, tolerance: number = 3): Promise<ActionResult> {
+        if (!Number.isFinite(x) || !Number.isFinite(z)) {
+            // walkTo(undefined, undefined) used to be accepted and marched the
+            // bot across the map (players expose coordinates as .x/.z).
+            return {
+                success: false,
+                message: `walkTo requires numeric coordinates - got (${x}, ${z}). Nearby entities (players included) expose them as .x/.z; your own position is state.player.worldX/.worldZ`,
+            };
+        }
         await this.dismissBlockingUI();
 
         const state = this.sdk.getState();
         if (!state?.player) return { success: false, message: 'No player state' };
 
+        // Death mid-walk: an orphaned walkTo that keeps marching after a
+        // respawn re-enters the danger zone that killed it. Abort instead.
+        const startLifeId = state.player.lifeId;
+        const diedEnRoute = (): ActionResult | null => {
+            const p = this.sdk.getState()?.player;
+            if (!p) return null;
+            if (p.isDead || (startLifeId !== undefined && p.lifeId !== undefined && p.lifeId !== startLifeId)) {
+                return { success: false, message: `Walk aborted: player died en route to (${x}, ${z})${p.isDead ? '' : ' (respawned)'}` };
+            }
+            return null;
+        };
+
         const distTo = (pos: { x: number; z: number }) => this.helpers.distance(pos.x, pos.z, x, z);
         let pos = { x: state.player.worldX, z: state.player.worldZ };
 
@@ -1060,6 +1080,9 @@ export class BotActions {
         };
 
         for (let i = 0; i < MAX_ITERATIONS; i++) {
+            const dead = diedEnRoute();
+            if (dead) return dead;
+
             // Try pathfinding (with one retry)
             let path = await this.sdk.sendFindPath(x, z, 500);
             if (!path.success || !path.waypoints?.length) {
@@ -1167,6 +1190,8 @@ export class BotActions {
                 }
 
                 const result = await this.helpers.walkStepToward(wp.x, wp.z, 2, pos);
+                const deadMidStep = diedEnRoute();
+                if (deadMidStep) return deadMidStep;
                 if (distTo(result.pos) <= tolerance) return { success: true, message: 'Arrived' };
 
                 if (result.status === 'stuck') {
@@ -1965,13 +1990,39 @@ export class BotActions {
      * Requesting a player who already requested you accepts their request;
      * otherwise this waits (re-requesting periodically) until they request
      * back or the timeout expires.
+     *
+     * The session is verified against the requested partner: a screen that
+     * opens with a *different* player (their pending request racing yours)
+     * is declined and the requested partner pursued until the deadline.
+     *
+     * `options.retryOnBusy` keeps retrying (with backoff) through "is busy
+     * at the moment" refusals instead of failing on the first one.
      */
-    async tradeWith(target: NearbyPlayer | string | RegExp, timeout: number = 30_000): Promise<ActionResult> {
+    async tradeWith(
+        target: NearbyPlayer | string | RegExp,
+        timeout: number = 30_000,
+        options: { retryOnBusy?: boolean } = {}
+    ): Promise<ActionResult> {
         await this.dismissBlockingUI();
 
+        // Same matching semantics as findNearbyPlayer: substring for strings,
+        // the pattern itself for RegExps, exact name for resolved entities.
+        const matchesTarget = (name: string | null | undefined): boolean => {
+            if (!name) return false;
+            if (typeof target === 'object' && 'index' in target) return exactNamePattern(target.name).test(name);
+            if (typeof target === 'string') return new RegExp(target, 'i').test(name);
+            return target.test(name);
+        };
+
         const open = this.sdk.getTradeState();
         if (open.isOpen) {
-            return { success: true, message: `Trade already open with ${open.partner ?? 'unknown partner'}` };
+            if (matchesTarget(open.partner)) {
+                return { success: true, message: `Trade already open with ${open.partner}` };
+            }
+            // A session with someone else (their request raced this call) must
+            // not be silently adopted - decline it and pursue the requested
+            // partner instead.
+            await this.declineTrade();
         }
 
         const resolvePlayer = (): NearbyPlayer | null =>
@@ -1993,17 +2044,19 @@ export class BotActions {
         }
 
         const deadline = Date.now() + timeout;
-        const msgBaseline = this.helpers.getMessageTick();
+        let msgBaseline = this.helpers.getMessageTick();
         const REREQUEST_INTERVAL = 8_000;
+        const BUSY_RETRY_DELAY = 3_000;
 
         let failReason: 'busy' | null = null;
         while (Date.now() < deadline) {
+            failReason = null;
             const current = resolvePlayer();
             if (current) player = current;
             await this.sdk.sendTradeRequest(player.index);
 
             try {
-                const finalState = await this.waitForActionCondition(state => {
+                await this.waitForActionCondition(state => {
                     if (this.helpers.findRefusal(msgBaseline, ['is busy at the moment'])) {
                         failReason = 'busy';
                         return true;
@@ -2012,10 +2065,33 @@ export class BotActions {
                 }, Math.min(REREQUEST_INTERVAL, Math.max(1, deadline - Date.now())));
 
                 if (failReason) {
+                    if (options.retryOnBusy && Date.now() + BUSY_RETRY_DELAY < deadline) {
+                        // Partner is mid-trade with someone else - back off and retry.
+                        msgBaseline = this.helpers.getMessageTick();
+                        await this.sdk.waitForCondition(
+                            () => false, BUSY_RETRY_DELAY
+                        ).catch(() => {});
+                        continue;
+                    }
                     return { success: false, message: `${player.name} is busy at the moment`, reason: failReason };
                 }
-                const partner = finalState.trade?.partner ?? player.name;
-                return { success: true, message: `Trade opened with ${partner}` };
+
+                // The screen is open - but possibly with a different player
+                // whose pending request the server matched first. The partner
+                // label can lag the open by a tick, so give it a moment.
+                let partner = this.sdk.getTradeState().partner;
+                if (!partner) {
+                    const settled = await this.sdk.waitForCondition(
+                        s => !!s.trade?.partner || s.trade?.isOpen !== true, 2_000
+                    ).catch(() => null);
+                    partner = settled?.trade?.partner ?? this.sdk.getTradeState().partner;
+                }
+                if (!this.sdk.getTradeState().isOpen) continue; // closed while settling
+                if (partner && !matchesTarget(partner)) {
+                    await this.declineTrade();
+                    continue; // wrong partner - keep pursuing the requested one
+                }
+                return { success: true, message: `Trade opened with ${partner ?? player.name}` };
             } catch {
                 // No response yet - re-request and keep waiting until deadline.
             }
@@ -2184,7 +2260,7 @@ export class BotActions {
         const timeout = options.timeout ?? 60_000;
         const requestTimeout = options.requestTimeout ?? 30_000;
 
-        const opened = await this.tradeWith(target, requestTimeout);
+        const opened = await this.tradeWith(target, requestTimeout, { retryOnBusy: options.retryOnBusy });
         const deadline = Date.now() + timeout;
         if (!opened.success) {
             return {
@@ -2223,6 +2299,10 @@ export class BotActions {
         } = options;
         const deadline = Date.now() + timeout;
         const trades: TradeResult[] = [];
+        // maxTrades counts COMPLETED trades: a declined/failed/empty session
+        // must not consume the serving window (a lowballer would end a
+        // maxTrades:1 window without anything changing hands).
+        const completed = () => trades.filter(t => t.success).length;
 
         const fromMatches = (name: string): boolean => {
             if (!from) return true;
@@ -2232,14 +2312,14 @@ export class BotActions {
 
         while (true) {
             if (until?.()) {
-                return { success: true, message: `Stop condition met after ${trades.length} trade(s)`, trades, reason: 'until' };
+                return { success: true, message: `Stop condition met after ${completed()} completed trade(s) (${trades.length} session(s))`, trades, reason: 'until' };
             }
-            if (maxTrades !== undefined && trades.length >= maxTrades) {
-                return { success: true, message: `Completed ${trades.length} trade(s)`, trades, reason: 'max_trades' };
+            if (maxTrades !== undefined && completed() >= maxTrades) {
+                return { success: true, message: `Completed ${completed()} trade(s) (${trades.length} session(s))`, trades, reason: 'max_trades' };
             }
             const remaining = deadline - Date.now();
             if (remaining <= 0) {
-                return { success: true, message: `Serving window ended after ${trades.length} trade(s)`, trades, reason: 'timeout' };
+                return { success: true, message: `Serving window ended after ${completed()} completed trade(s) (${trades.length} session(s))`, trades, reason: 'timeout' };
             }
 
             // A session can already be open (e.g. a request raced the loop).
@@ -2270,7 +2350,7 @@ export class BotActions {
             }
 
             const result = await this.runOpenTradeSession(
-                { give: options.give, want: options.want, accept: options.accept },
+                { give: options.give, want: options.want, accept: options.accept, rejectAfterMs: options.rejectAfterMs },
                 Date.now() + Math.min(tradeTimeout, Math.max(1, deadline - Date.now()))
             );
             trades.push(result);
@@ -2286,6 +2366,27 @@ export class BotActions {
     private async runOpenTradeSession(options: TradeOptions, deadline: number): Promise<TradeResult> {
         const want = options.want ?? [];
         const acceptOffer = options.accept ?? ((offer: TradeItem[]) => offerSatisfies(want, offer));
+        // Evaluate the accept policy when the partner's visible offer changes,
+        // plus a periodic re-check - re-running a user predicate every tick
+        // floods the console (a logging predicate produced hundreds of lines
+        // per session), but never re-running it would strand predicates that
+        // depend on external state (time, inventory) on an unchanged offer.
+        const REEVAL_INTERVAL_MS = 2_000;
+        let lastOfferKey: string | null = null;
+        let lastOfferDecision = false;
+        let lastEvalAt = 0;
+        const offerAcceptable = (theirOffer: TradeItem[]): boolean => {
+            const key = JSON.stringify(theirOffer.map(i => [i.id, i.count]));
+            if (key !== lastOfferKey || Date.now() - lastEvalAt >= REEVAL_INTERVAL_MS) {
+                lastOfferKey = key;
+                lastEvalAt = Date.now();
+                lastOfferDecision = acceptOffer(theirOffer);
+            }
+            return lastOfferDecision;
+        };
+        // rejectAfterMs bounds how long an unacceptable offer may occupy the
+        // screen; without it a non-buyer holds the session until the timeout.
+        const rejectDeadline = options.rejectAfterMs !== undefined ? Date.now() + options.rejectAfterMs : Infinity;
         const startState = this.sdk.getState();
         const partner = this.sdk.getTradeState().partner ?? undefined;
         const invBefore = countInventoryById(startState?.inventory ?? []);
@@ -2356,8 +2457,16 @@ export class BotActions {
             noteOffers(trade);
             if (trade.screen === 'confirm') break;
 
+            const acceptable = offerAcceptable(trade.theirOffer);
+            if (!acceptable && Date.now() >= rejectDeadline) {
+                return fail(
+                    `Partner's offer still unacceptable after ${options.rejectAfterMs}ms - declined`,
+                    'want_not_met',
+                    true
+                );
+            }
             const tick = this.sdk.getState()?.tick ?? 0;
-            if (acceptOffer(trade.theirOffer) && !trade.myAccepted && tick !== lastAcceptTick) {
+            if (acceptable && !trade.myAccepted && tick !== lastAcceptTick) {
                 lastAcceptTick = tick;
                 await this.sdk.sendAcceptTrade();
             }
@@ -2456,6 +2565,13 @@ export class BotActions {
         }
 
         if (accepted) {
+            // The post-trade inventory update can land a tick or two behind
+            // the close - don't report received items as missing (or the
+            // received list as empty) while the delta is still in flight.
+            for (let i = 0; i < 2 && shortfall(offers.theirs, gained).length > 0; i++) {
+                await this.sdk.waitForTicks(1).catch(() => {});
+                ({ gained, lost } = diff());
+            }
             const gaveText = lost.length ? lost.map(i => `${i.name} x${i.count}`).join(', ') : 'nothing';
             const receivedText = gained.length ? gained.map(i => `${i.name} x${i.count}`).join(', ') : 'nothing';
             let message = `Trade completed with ${partner ?? 'partner'}: gave ${gaveText}, received ${receivedText}`;
@@ -3752,6 +3868,11 @@ export class BotActions {
         target: NearbyLoc | string | RegExp,
         option: number | string | RegExp = 1,
     ): Promise<InteractLocResult> {
+        if (target == null) {
+            // An undefined target used to fall through to a match-anything
+            // pattern and silently interact with a random nearby loc.
+            return { success: false, message: `interactLoc requires a target loc, name, or pattern (got ${target})`, reason: 'loc_not_found' };
+        }
         const resolvedLoc = this.helpers.resolveLocation(target, /./);
         return this.helpers.withDoorRetry(
             () => this._interactLocOnce(target, option),
@@ -3902,6 +4023,9 @@ export class BotActions {
         target: NearbyNpc | string | RegExp,
         option: number | string | RegExp = 1,
     ): Promise<InteractNpcResult> {
+        if (target == null) {
+            return { success: false, message: `interactNpc requires a target npc, name, or pattern (got ${target})`, reason: 'npc_not_found' };
+        }
         return this.helpers.withDoorRetry(
             () => this._interactNpcOnce(target, option),
             (r) => r.reason === 'cant_reach'

sdk/formatter.ts

@@ -112,7 +112,10 @@ export function formatWorldStateSummary(
     }
     if (state.bank?.isOpen) blockers.push('bank open');
     if (state.shop?.isOpen) blockers.push(`shop open (${state.shop.title})`);
-    if (state.interface?.isOpen && !state.bank?.isOpen && !state.shop?.isOpen) {
+    if (state.trade?.isOpen) {
+        // A live two-party session, not a stray modal - closing it declines the trade.
+        blockers.push(`trade ${state.trade.screen} screen open with ${state.trade.partner ?? 'unknown'}`);
+    } else if (state.interface?.isOpen && !state.bank?.isOpen && !state.shop?.isOpen) {
         blockers.push(`interface ${state.interface.interfaceId} open`);
     } else if (state.modalOpen && !state.dialog?.isOpen) {
         blockers.push(`modal ${state.modalInterface} open`);
@@ -195,7 +198,7 @@ export function formatWorldState(
     }
 
     // Modal/dialog state (important for understanding game state)
-    if (state.modalOpen) {
+    if (state.modalOpen && !state.trade?.isOpen) {
         lines.push('');
         lines.push(`## Modal Open (interface: ${state.modalInterface})`);
         if (state.modalInterface === 3559) {
@@ -229,7 +232,9 @@ export function formatWorldState(
                 lines.push(`  ${opt.index}. ${opt.text}`);
             }
         }
-        if (!state.shop?.isOpen && !state.bank?.isOpen) {
+        if (state.trade?.isOpen) {
+            lines.push('(This is the player-trade window - see the Trade section below. Drive it with bot.trade()/offerTradeItems()/acceptTrade(); closing it would DECLINE the trade.)');
+        } else if (!state.shop?.isOpen && !state.bank?.isOpen) {
             lines.push('(This modal blocks most actions - close it with bot.closeInterface() or sdk.sendCloseModal())');
         }
     }

sdk/index.ts

@@ -118,6 +118,7 @@ interface PendingScreenshot {
 // importing the full SDK; re-exported here for existing consumers and tests.
 export { chunkMessage } from './chunking';
 import { chunkMessage } from './chunking';
+export { Spells, type SpellName } from './spells';
 
 export class BotSDK {
     readonly config: Required<SDKConfig>;
@@ -1734,6 +1735,12 @@ export class BotSDK {
         }
     }
 
+    /**
+     * Wait until the predicate passes on a state update and return that state.
+     * THROWS `Error('waitForCondition timed out')` if the timeout expires -
+     * wrap in try/catch (or `.catch(...)`) when a timeout is an expected
+     * outcome rather than a fatal one.
+     */
     async waitForCondition(
         predicate: (state: BotWorldState) => boolean,
         timeout: number = 30000
@@ -1869,6 +1876,26 @@ export class BotSDK {
                 this.chatHistory.record(message.state.gameMessages);
             }
 
+            // Players publish coordinates as x/z (NPCs/locs also carry
+            // worldX/worldZ) - fill the aliases in so walkTo(p.worldX, ...)
+            // can't silently become walkTo(undefined, undefined).
+            if (message.state.nearbyPlayers) {
+                for (const p of message.state.nearbyPlayers) {
+                    if (p.worldX === undefined) { p.worldX = p.x; p.worldZ = p.z; }
+                }
+            }
+
+            // Trade offers publish counts as `count`; mirror to `amount` so
+            // either spelling works (TradeResult item lists do the same).
+            if (message.state.trade) {
+                for (const list of [message.state.trade.myOffer, message.state.trade.theirOffer]) {
+                    if (!list) continue;
+                    for (const item of list) {
+                        if (item.amount === undefined) item.amount = item.count;
+                    }
+                }
+            }
+
             // Wrap skills array with a Proxy so both state.skills[0] and state.skills.Woodcutting work
             if (message.state.skills) {
                 const nameMap: Record<string, SkillState> = {};

sdk/spells.ts

@@ -0,0 +1,42 @@
+// Spell component ids (from server/content/pack/interface.pack).
+// These are what sdk.sendSpellOnTarget / sendSpellOnItem and bot.castSpell
+// take as the spell argument, e.g.:
+//   await sdk.sendSpellOnItem(item.slot, Spells.HIGH_ALCHEMY);
+//   await bot.castSpell('goblin', Spells.WIND_STRIKE);
+export const Spells = {
+    // Combat spells
+    WIND_STRIKE: 1152,
+    CONFUSE: 1153,
+    WATER_STRIKE: 1154,
+    ENCHANT_LVL1: 1155, // Sapphire
+    EARTH_STRIKE: 1156,
+    WEAKEN: 1157,
+    FIRE_STRIKE: 1158,
+    WIND_BOLT: 1160,
+    CURSE: 1161,
+    LOW_ALCHEMY: 1162,
+    WATER_BOLT: 1163,
+    VARROCK_TELEPORT: 1164,
+    ENCHANT_LVL2: 1165, // Emerald
+    EARTH_BOLT: 1166,
+    LUMBRIDGE_TELEPORT: 1167,
+    FIRE_BOLT: 1169,
+    FALADOR_TELEPORT: 1170,
+    WIND_BLAST: 1172,
+    SUPERHEAT: 1173, // Requires Magic 43 AND Smithing level for the bar
+    CAMELOT_TELEPORT: 1174,
+    WATER_BLAST: 1175,
+    ENCHANT_LVL3: 1176, // Ruby
+    EARTH_BLAST: 1177,
+    HIGH_ALCHEMY: 1178,
+    ENCHANT_LVL4: 1180, // Diamond
+    FIRE_BLAST: 1181,
+    WIND_WAVE: 1183,
+    WATER_WAVE: 1185,
+    ENCHANT_LVL5: 1187, // Dragonstone
+    EARTH_WAVE: 1188,
+    FIRE_WAVE: 1189,
+    BIND: 1572,
+} as const;
+
+export type SpellName = keyof typeof Spells;

sdk/test/trade-actions.test.ts

@@ -134,7 +134,7 @@ describe('bot.trade gift flow', () => {
         expect(result.success).toBe(true);
         expect(result.reason).toBeUndefined();
         expect(result.partner).toBe('MuleBot');
-        expect(result.gave).toEqual([{ slot: -1, id: 1511, name: 'Logs', count: 1 }]);
+        expect(result.gave).toEqual([{ slot: -1, id: 1511, name: 'Logs', count: 1, amount: 1 }]);
         expect(result.received).toEqual([]);
         expect(harness.dispatched.map(d => d.call)).toEqual([
             'sendTradeRequest',
@@ -211,6 +211,32 @@ describe('bot.tradeWith failure modes', () => {
         expect(result).toMatchObject({ success: false, reason: 'player_not_found' });
         expect(harness.dispatched).toEqual([]);
     });
+
+    test('declines a session that is already open with a DIFFERENT player', async () => {
+        // A pending request from Impostor raced this call; the open screen
+        // must not be silently adopted as a trade with MuleBot.
+        const harness = createHarness([
+            { trade: { isOpen: true, screen: 'offer', partner: 'Impostor' } },
+            { }, // closed after the decline; MuleBot never answers
+        ]);
+
+        const result = await harness.bot.tradeWith(partner, 600);
+        const calls = harness.dispatched.map(d => d.call);
+        expect(calls).toContain('sendDeclineTrade');
+        expect(calls).toContain('sendTradeRequest');
+        expect(result).toMatchObject({ success: false, reason: 'no_response' });
+    });
+
+    test('adopts an already-open session when the partner matches', async () => {
+        const harness = createHarness([
+            { trade: { isOpen: true, screen: 'offer', partner: 'MuleBot' } },
+        ]);
+
+        const result = await harness.bot.tradeWith(partner, 600);
+        expect(result.success).toBe(true);
+        expect(result.message).toContain('MuleBot');
+        expect(harness.dispatched).toEqual([]);
+    });
 });
 
 describe('bot.serveTrades', () => {
@@ -235,7 +261,7 @@ describe('bot.serveTrades', () => {
         expect(result.success).toBe(true);
         expect(result.reason).toBe('until');
         expect(result.trades).toHaveLength(1);
-        expect(result.trades[0]!.received).toEqual([{ slot: -1, id: 995, name: 'Coins', count: 250 }]);
+        expect(result.trades[0]!.received).toEqual([{ slot: -1, id: 995, name: 'Coins', count: 250, amount: 250 }]);
         expect(harness.dispatched.map(d => d.call)).toEqual([
             'sendTradeRequest',  // requesting back = accepting
             'sendAcceptTrade',
@@ -255,6 +281,58 @@ describe('bot.serveTrades', () => {
         expect(result.trades).toHaveLength(0);
         expect(harness.dispatched).toEqual([]);
     });
+
+    test('a declined session does not consume maxTrades', async () => {
+        const coins = (n: number) => item(0, 995, 'Coins', n);
+        const harness = createHarness([
+            { },
+            // Session 1: a lowballer offers nothing and then declines.
+            { trade: { isOpen: true, screen: 'offer', partner: 'MuleBot' } },
+            { message: 'Other player declined trade.' },
+            { },
+            { },
+            // Session 2: a real buyer's session (already open when the loop looks).
+            { trade: { isOpen: true, screen: 'offer', partner: 'MuleBot', theirOffer: [coins(250)] } },
+            { trade: { isOpen: true, screen: 'confirm', partner: 'MuleBot', theirOffer: [coins(250)] } },
+            { message: 'Accepted trade.', inventory: [{ slot: 0, id: 995, name: 'Coins', count: 250 }] },
+        ]);
+        harness.sdk.waitForTradeRequest = async () => 'MuleBot';
+
+        const result = await harness.bot.serveTrades({
+            from: /mule/i,
+            want: [{ item: 'coins', amount: 100 }],
+            maxTrades: 1,
+            timeout: 5_000,
+        });
+
+        // The declined session is recorded but only the completed one counts.
+        expect(result.reason).toBe('max_trades');
+        expect(result.trades).toHaveLength(2);
+        expect(result.trades[0]!.success).toBe(false);
+        expect(result.trades[1]!.success).toBe(true);
+        expect(result.trades[1]!.received).toEqual([{ slot: -1, id: 995, name: 'Coins', count: 250, amount: 250 }]);
+    });
+
+    test('rejectAfterMs declines an offer that stays unacceptable', async () => {
+        const harness = createHarness([
+            { },
+            { trade: { isOpen: true, screen: 'offer', partner: 'MuleBot' } },
+            { }, // decline lands here
+        ], { hold: [1] });
+        harness.sdk.waitForTradeRequest = async () => 'MuleBot';
+
+        const result = await harness.bot.serveTrades({
+            from: /mule/i,
+            want: [{ item: 'coins', amount: 100 }],
+            rejectAfterMs: 50,
+            timeout: 3_000,
+            until: () => harness.dispatched.some(d => d.call === 'sendDeclineTrade'),
+        });
+
+        expect(harness.dispatched.map(d => d.call)).toContain('sendDeclineTrade');
+        expect(result.trades[0]!.success).toBe(false);
+        expect(result.trades[0]!.reason).toBe('want_not_met');
+    });
 });
 
 describe('bot.trade completion detection', () => {
@@ -275,7 +353,7 @@ describe('bot.trade completion detection', () => {
         const result = await harness.bot.trade(partner, { give: [{ item: 'logs', amount: -1 }], timeout: 3_000 });
 
         expect(result.success).toBe(true);
-        expect(result.gave).toEqual([{ slot: -1, id: 1511, name: 'Logs', count: 1 }]);
+        expect(result.gave).toEqual([{ slot: -1, id: 1511, name: 'Logs', count: 1, amount: 1 }]);
         expect(result.message).toContain('inferred from inventory');
     });
 
@@ -319,8 +397,8 @@ describe('bot.trade completion detection', () => {
 
         expect(harness.dispatched.map(d => d.call)).toContain('sendDeclineTrade');
         expect(result.success).toBe(true);
-        expect(result.received).toEqual([{ slot: -1, id: 995, name: 'Coins', count: 50 }]);
-        expect(result.gave).toEqual([{ slot: -1, id: 1511, name: 'Logs', count: 1 }]);
+        expect(result.received).toEqual([{ slot: -1, id: 995, name: 'Coins', count: 50, amount: 50 }]);
+        expect(result.gave).toEqual([{ slot: -1, id: 1511, name: 'Logs', count: 1, amount: 1 }]);
         expect(result.message).toContain('completed as the decline landed');
     });
 
@@ -338,7 +416,7 @@ describe('bot.trade completion detection', () => {
         const result = await harness.bot.trade(partner, { timeout: 3_000 });
 
         expect(result.success).toBe(true);
-        expect(result.possiblyDropped).toEqual([{ slot: -1, id: 379, name: 'Lobster', count: 1 }]);
+        expect(result.possiblyDropped).toEqual([{ slot: -1, id: 379, name: 'Lobster', count: 1, amount: 1 }]);
         expect(result.message).toContain('check the ground');
     });
 });
@@ -369,7 +447,7 @@ describe('bot.acceptTrade', () => {
         const result = await harness.bot.acceptTrade(2_000);
         expect(result.success).toBe(true);
         expect(result.message).toContain('Trade completed');
-        expect(result.data.received).toEqual([{ slot: -1, id: 1285, name: 'Mithril sword', count: 1 }]);
+        expect(result.data.received).toEqual([{ slot: -1, id: 1285, name: 'Mithril sword', count: 1, amount: 1 }]);
     });
 
     test('treats the offer->confirm blip as progress, not a close', async () => {

sdk/test/trade-helpers.test.ts

@@ -68,15 +68,15 @@ describe('inventory diffing', () => {
         ]);
         const { gained, lost } = diffInventories(before, after);
         expect(gained).toEqual([
-            { slot: -1, id: 2, name: 'Coins', count: 250 },
-            { slot: -1, id: 3, name: 'Lobster', count: 2 },
+            { slot: -1, id: 2, name: 'Coins', count: 250, amount: 250 },
+            { slot: -1, id: 3, name: 'Lobster', count: 2, amount: 2 },
         ]);
-        expect(lost).toEqual([{ slot: -1, id: 1, name: 'Logs', count: 3 }]);
+        expect(lost).toEqual([{ slot: -1, id: 1, name: 'Logs', count: 3, amount: 3 }]);
     });
 });
 
 describe('shortfall', () => {
-    const it = (id: number, name: string, count: number): TradeItem => ({ slot: -1, id, name, count });
+    const it = (id: number, name: string, count: number): TradeItem => ({ slot: -1, id, name, count, amount: count });
 
     test('lists what the final offer promised but the inventory never gained', () => {
         expect(shortfall([it(995, 'Coins', 250), it(379, 'Lobster', 1)], [it(995, 'Coins', 250)]))

sdk/test/utils/save-generator.ts

@@ -108,42 +108,8 @@ export const Items = {
     BIG_BONES: 532,
 };
 
-// Spell component IDs (from content/pack/interface.pack)
-export const Spells = {
-    // Combat spells
-    WIND_STRIKE: 1152,
-    CONFUSE: 1153,
-    WATER_STRIKE: 1154,
-    ENCHANT_LVL1: 1155,  // Sapphire
-    EARTH_STRIKE: 1156,
-    WEAKEN: 1157,
-    FIRE_STRIKE: 1158,
-    WIND_BOLT: 1160,
-    CURSE: 1161,
-    LOW_ALCHEMY: 1162,
-    WATER_BOLT: 1163,
-    VARROCK_TELEPORT: 1164,
-    ENCHANT_LVL2: 1165,  // Emerald
-    EARTH_BOLT: 1166,
-    LUMBRIDGE_TELEPORT: 1167,
-    FIRE_BOLT: 1169,
-    FALADOR_TELEPORT: 1170,
-    WIND_BLAST: 1172,
-    SUPERHEAT: 1173,
-    CAMELOT_TELEPORT: 1174,
-    WATER_BLAST: 1175,
-    ENCHANT_LVL3: 1176,  // Ruby
-    EARTH_BLAST: 1177,
-    HIGH_ALCHEMY: 1178,
-    ENCHANT_LVL4: 1180,  // Diamond
-    FIRE_BLAST: 1181,
-    WIND_WAVE: 1183,
-    WATER_WAVE: 1185,
-    ENCHANT_LVL5: 1187,  // Dragonstone
-    EARTH_WAVE: 1188,
-    FIRE_WAVE: 1189,
-    BIND: 1572,
-};
+// Spell component IDs - canonical table lives in sdk/spells.ts
+export { Spells } from '../../spells';
 
 // Skill indices
 export const Skills = {

sdk/test/walkto-partial-path.test.ts

@@ -106,4 +106,20 @@ describe('walkTo() partial paths', () => {
 
         expect(result.success).toBe(true);
     });
+
+    test('rejects non-numeric coordinates instead of walking', async () => {
+        // walkTo(p.worldX, p.worldZ) with a NearbyPlayer (which only has .x/.z)
+        // used to become walkTo(undefined, undefined) and march across the map.
+        const { bot, queries } = makeBot(() => ({
+            success: true,
+            reachedDestination: true,
+            waypoints: [{ x: 3170, z: 3430, level: 0 }],
+        }));
+
+        const result = await bot.walkTo(undefined as any, undefined as any);
+
+        expect(result.success).toBe(false);
+        expect(result.message).toContain('numeric');
+        expect(queries()).toBe(0);
+    });
 });

sdk/trade-helpers.ts

@@ -48,8 +48,8 @@ export function diffInventories(
         const a = after.get(id);
         const delta = (a?.count ?? 0) - (b?.count ?? 0);
         const name = a?.name ?? b?.name ?? 'Unknown';
-        if (delta > 0) gained.push({ slot: -1, id, name, count: delta });
-        else if (delta < 0) lost.push({ slot: -1, id, name, count: -delta });
+        if (delta > 0) gained.push({ slot: -1, id, name, count: delta, amount: delta });
+        else if (delta < 0) lost.push({ slot: -1, id, name, count: -delta, amount: -delta });
     }
     return { gained, lost };
 }
@@ -67,7 +67,7 @@ export function shortfall(expected: TradeItem[], received: TradeItem[]): TradeIt
     }
     for (const item of seen.values()) {
         const missing = item.count - (got.get(item.id) ?? 0);
-        if (missing > 0) out.push({ ...item, count: missing });
+        if (missing > 0) out.push({ ...item, count: missing, amount: missing });
     }
     return out;
 }

sdk/types.ts

@@ -131,6 +131,10 @@ export interface NearbyPlayer {
     combatLevel: number;
     x: number;
     z: number;
+    /** Alias of `x` (filled in by the SDK; some callers expect worldX/worldZ). */
+    worldX?: number;
+    /** Alias of `z`. */
+    worldZ?: number;
     distance: number;
     /** See NearbyNpc.reachable. */
     reachable?: boolean;
@@ -322,6 +326,8 @@ export interface TradeItem {
     id: number;
     name: string;
     count: number;
+    /** Alias of `count` (filled in by the SDK; some callers expect `.amount`). */
+    amount?: number;
 }
 
 /**
@@ -368,13 +374,23 @@ export interface TradeOptions {
     want?: TradeItemSpec[];
     /**
      * Escape hatch replacing `want`: accept only when this predicate passes
-     * on the partner's visible offer. Also re-checked on the confirm screen.
+     * on the partner's visible offer. Called when the visible offer changes
+     * and re-checked every ~2s (not every tick - keep any logging light).
+     * Also re-checked on the confirm screen.
      */
     accept?: (theirOffer: TradeItem[]) => boolean;
     /** Deadline in ms for the open offer+confirm screens (default 60_000). On expiry the trade is declined. */
     timeout?: number;
     /** How long to wait for the partner to accept the trade request (default 30_000). */
     requestTimeout?: number;
+    /**
+     * Decline if the partner's offer still fails the accept policy after this
+     * many ms on the offer screen. Default: only `timeout` applies, so an
+     * unacceptable offer can occupy the screen for the whole window.
+     */
+    rejectAfterMs?: number;
+    /** Keep retrying (with backoff) through "is busy at the moment" refusals until `requestTimeout`. Default false. */
+    retryOnBusy?: boolean;
 }
 
 export interface TradeResult {
@@ -405,10 +421,16 @@ export interface ServeTradesOptions {
     onTrade?: (result: TradeResult) => void;
     /** Serving stops once this returns true (checked between trades). */
     until?: () => boolean;
-    /** Stop after this many completed trades. Default: unlimited. */
+    /**
+     * Stop after this many COMPLETED trades. Declined, failed, or empty
+     * sessions do not count toward this limit (they are still recorded in
+     * the result's `trades` list).
+     */
     maxTrades?: number;
     /** Per-trade deadline in ms once a session opens (default 60_000). */
     tradeTimeout?: number;
+    /** Decline a session whose offer still fails the accept policy after this many ms (see TradeOptions.rejectAfterMs). */
+    rejectAfterMs?: number;
     /** Overall serving deadline in ms (default 10 minutes). */
     timeout?: number;
 }
@@ -416,7 +438,7 @@ export interface ServeTradesOptions {
 export interface ServeTradesResult {
     success: boolean;
     message: string;
-    /** Completed trades, in order. */
+    /** Every served session in order, completed and failed alike (check `.success`). */
     trades: TradeResult[];
     reason?: 'until' | 'max_trades' | 'timeout' | 'error';
 }

server/gateway/gateway.ts

@@ -664,7 +664,7 @@ async function authenticateHttpRequest(req: Request, url: URL, username: string,
  * actionResult. Never rejects: a client that doesn't answer inside the budget
  * yields a failed result, so the HTTP handler always responds.
  */
-function dispatchHttpSay(botSession: BotSession, text: string, timeoutMs: number, to?: string): Promise<{ success: boolean; message: string; data?: any; reason?: string }> {
+function dispatchHttpSayNow(botSession: BotSession, text: string, timeoutMs: number, to?: string): Promise<{ success: boolean; message: string; data?: any; reason?: string }> {
     const actionId = `http-${++httpActionSeq}-${Math.random().toString(36).slice(2, 8)}`;
     return new Promise(resolve => {
         const timeout = setTimeout(() => {
@@ -685,6 +685,26 @@ function dispatchHttpSay(botSession: BotSession, text: string, timeoutMs: number
     });
 }
 
+/**
+ * Per-bot serialization for HTTP says. Concurrent requests used to each
+ * overwrite `botSession.currentActionId`; when a client doesn't echo the
+ * actionId, only the LAST dispatch got its actionResult and every earlier
+ * one reported a false "timed out" for a message that was delivered. One
+ * outstanding say per bot also matches the server's one-social-packet-per-tick
+ * budget.
+ */
+const httpSayQueues = new Map<string, Promise<unknown>>();
+function dispatchHttpSay(botSession: BotSession, text: string, timeoutMs: number, to?: string): Promise<{ success: boolean; message: string; data?: any; reason?: string }> {
+    const key = botSession.username;
+    const prev = httpSayQueues.get(key) ?? Promise.resolve();
+    const next = prev.catch(() => {}).then(() => dispatchHttpSayNow(botSession, text, timeoutMs, to));
+    httpSayQueues.set(key, next);
+    next.finally(() => {
+        if (httpSayQueues.get(key) === next) httpSayQueues.delete(key);
+    });
+    return next;
+}
+
 // ============ Message Router ============
 
 /**