fix(sdk,webclient): classify every trade close as completed vs declined

- acceptTrade/trade/serveTrades: a closed screen is now settled from the
  'Accepted trade.' message or the inventory delta, including closes that
  race our own timeout decline; report possiblyDropped overflow.
- trade(): separate requestTimeout (30s) from the open-session timeout.
- declineTrade retries once across the offer->confirm blip.
- closeBotModal always sends CLOSE_MODAL (local modal ids can lag the server).

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

sdk/API.md

@@ -88,9 +88,9 @@
 |---|---|
 | `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 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. |
-| `async declineTrade(timeout: number = 5_000): Promise<ActionResult>` | Decline (close) the open trade and wait for the screen to close. |
-| `async trade(target: NearbyPlayer \| string \| RegExp, options: TradeOptions = {}): Promise<TradeResult>` | Full trade happy path with one player: open the session, offer `give`, accept once the partner's offer satisfies `want` (or the `accept` predicate), re-verify on the confirm screen, and report the actual inventory delta. Any offer change resets both accepts server-side, so the offer seen at accept time is the offer that reaches the confirm screen; the confirm re-verification makes offer-switching structurally impossible to slip past this method. |
+| `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). |
+| `async trade(target: NearbyPlayer \| string \| RegExp, options: TradeOptions = {}): Promise<TradeResult>` | Full trade happy path with one player: open the session, offer `give`, accept once the partner's offer satisfies `want` (or the `accept` predicate), re-verify on the confirm screen, and report the actual inventory delta. Any offer change resets both accepts server-side, so the offer seen at accept time is the offer that reaches the confirm screen; the confirm re-verification makes offer-switching structurally impossible to slip past this method. `options.requestTimeout` (default 30 s) bounds the trade request; `options.timeout` (default 60 s) bounds the open offer+confirm screens. Every close is classified as completed or declined, so `success: false` means nothing changed hands. |
 | `async serveTrades(options: ServeTradesOptions = {}): Promise<ServeTradesResult>` | Serve incoming trades: wait for "wishes to trade with you." requests, accept ones matching the `from` filter, and run each session with the shared give/want/accept policy. The receiving half of a muling setup: ```ts // Worker: bot.trade('mule01', { give: [{ item: /ore/i, amount: -1 }] }) // Mule: bot.serveTrades({ from: /^fleet_/i, until: () => sdk.getInventory().length >= 26 }) ``` This owns the bot while running - it is an explicit serving loop, not a background hook, so it never fights another controller for the session. |
 
 ### Combat & Equipment
@@ -426,6 +426,8 @@ interface TradeResult {
   /** Items added to your inventory (by the completed trade). */
   received: TradeItem[];
   reason?: 'player_not_found' | 'no_response' | 'declined' | 'busy' | 'offer_failed' | 'want_not_met' | 'no_space' | 'not_open' | 'timeout' | 'error';
+  /** Items from the partner's final offer that did not reach the inventory (dropped on the ground on overflow). */
+  possiblyDropped?: TradeItem[];
 }
 ```
 

sdk/actions.ts

@@ -7,6 +7,7 @@ import { ActionHelpers, ALREADY_FIGHTING_REFUSALS, NO_RUNES_REFUSALS } from './a
 import { findDoorsAlongPath, isTileWalkable } from './pathfinding';
 import type {
     ActionResult,
+    BotWorldState,
     SkillState,
     InventoryItem,
     BankItem,
@@ -52,6 +53,7 @@ import type {
     TradeItemSpec,
     TradeOptions,
     TradeResult,
+    TradeState,
     ServeTradesOptions,
     ServeTradesResult,
 } from './types';
@@ -62,6 +64,7 @@ import {
     diffInventories,
     missingFromOffer,
     offerSatisfies,
+    shortfall,
 } from './trade-helpers';
 import {
     classifyQuantity,
@@ -2071,7 +2074,9 @@ export class BotActions {
     /**
      * 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.
+     * 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 acceptTrade(timeout: number = 10_000): Promise<ActionResult> {
         const before = this.sdk.getTradeState();
@@ -2079,43 +2084,72 @@ export class BotActions {
             return { success: false, message: 'No trade screen is open', reason: 'not_open' };
         }
         const screenBefore = before.screen;
+        const partner = before.partner ?? undefined;
+        const invBefore = countInventoryById(this.sdk.getState()?.inventory ?? []);
+        const msgBaseline = this.helpers.getMessageTick();
+        const offers = { mine: before.myOffer, theirs: before.theirOffer };
+        const deadline = Date.now() + timeout;
 
         await this.sdk.sendAcceptTrade();
 
-        try {
-            const finalState = await this.waitForActionCondition(s => {
-                const t = s.trade;
-                if (!t || !t.isOpen) return true;               // completed (or declined)
-                if (t.screen !== screenBefore) return true;      // advanced to confirm
-                return t.myAccepted;                             // registered, waiting on partner
-            }, timeout);
+        while (true) {
+            let finalState: BotWorldState;
+            try {
+                finalState = await this.waitForActionCondition(s => {
+                    const t = s.trade;
+                    if (!t || !t.isOpen) return true;               // completed, declined, or the offer->confirm blip
+                    if (t.screen !== screenBefore) return true;      // advanced to confirm
+                    return t.myAccepted;                             // registered, waiting on partner
+                }, Math.max(1, deadline - Date.now()));
+            } catch {
+                return { success: false, message: 'No visible response to accept', reason: 'timeout' };
+            }
 
             const after = finalState.trade;
-            if (!after || !after.isOpen) {
-                return { success: true, message: 'Trade screen closed' };
+            if (after?.isOpen) {
+                offers.mine = after.myOffer;
+                offers.theirs = after.theirOffer;
+                if (after.screen !== screenBefore) {
+                    return { success: true, message: 'Advanced to confirm screen' };
+                }
+                return { success: true, message: 'Accepted - waiting for other player' };
             }
-            if (after.screen !== screenBefore) {
-                return { success: true, message: 'Advanced to confirm screen' };
-            }
-            return { success: true, message: 'Accepted - waiting for other player' };
-        } catch {
-            return { success: false, message: 'No visible response to accept', reason: 'timeout' };
+
+            // Only a persistent close is final (offer->confirm blip).
+            if (!(await this.tradeClosedForGood())) continue;
+            const settled = await this.classifyClosedSession(msgBaseline, partner, invBefore, offers);
+            return {
+                success: settled.success,
+                message: settled.message,
+                reason: settled.reason,
+                data: { gave: settled.gave, received: settled.received, possiblyDropped: settled.possiblyDropped },
+            };
         }
     }
 
-    /** Decline (close) the open trade and wait for the screen to close. */
+    /**
+     * 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).
+     */
     async declineTrade(timeout: number = 5_000): Promise<ActionResult> {
         const trade = this.sdk.getTradeState();
         if (!trade.isOpen) {
             return { success: true, message: 'No trade was open' };
         }
-        await this.sdk.sendDeclineTrade();
-        try {
-            await this.waitForActionCondition(s => s.trade?.isOpen !== true, timeout);
-            return { success: true, message: 'Trade declined' };
-        } catch {
-            return { success: false, message: 'Trade screen did not close', reason: 'timeout' };
+        const deadline = Date.now() + timeout;
+        for (let attempt = 0; attempt < 2; attempt++) {
+            await this.sdk.sendDeclineTrade();
+            try {
+                await this.waitForActionCondition(s => s.trade?.isOpen !== true, Math.max(1, deadline - Date.now()));
+            } catch {
+                return { success: false, message: 'Trade screen did not close', reason: 'timeout' };
+            }
+            if (await this.tradeClosedForGood()) {
+                return { success: true, message: 'Trade declined' };
+            }
         }
+        return { success: false, message: 'Trade screen did not close', reason: 'timeout' };
     }
 
     /**
@@ -2129,6 +2163,11 @@ export class BotActions {
      * re-verification makes offer-switching structurally impossible to slip
      * past this method.
      *
+     * `options.requestTimeout` (default 30 s) bounds the trade request;
+     * `options.timeout` (default 60 s) bounds the open offer+confirm screens.
+     * Every close is classified as completed or declined, so `success: false`
+     * means nothing changed hands.
+     *
      * @example
      * ```ts
      * // Pure gift (muling): give all herbs, expect nothing back
@@ -2143,9 +2182,10 @@ export class BotActions {
      */
     async trade(target: NearbyPlayer | string | RegExp, options: TradeOptions = {}): Promise<TradeResult> {
         const timeout = options.timeout ?? 60_000;
-        const deadline = Date.now() + timeout;
+        const requestTimeout = options.requestTimeout ?? 30_000;
 
-        const opened = await this.tradeWith(target, timeout);
+        const opened = await this.tradeWith(target, requestTimeout);
+        const deadline = Date.now() + timeout;
         if (!opened.success) {
             return {
                 success: false,
@@ -2211,6 +2251,8 @@ export class BotActions {
                     continue;
                 }
             } else {
+                // A blocking modal makes the server answer requests with "is busy".
+                await this.dismissBlockingUI();
                 const requester = await this.sdk.waitForTradeRequest({ timeout: Math.min(remaining, 15_000) });
                 if (!requester) continue;
                 if (!fromMatches(requester)) continue;
@@ -2248,18 +2290,35 @@ export class BotActions {
         const partner = this.sdk.getTradeState().partner ?? undefined;
         const invBefore = countInventoryById(startState?.inventory ?? []);
         const sessionBaseline = this.helpers.getMessageTick();
+        // Last offers seen while the screen was open (see classifyClosedSession).
+        const offers = { mine: [] as TradeItem[], theirs: [] as TradeItem[] };
+        const noteOffers = (t: TradeState) => {
+            if (t.isOpen) { offers.mine = t.myOffer; offers.theirs = t.theirOffer; }
+        };
+        noteOffers(this.sdk.getTradeState());
 
+        // Our decline can race the partner's accept, so re-classify after it.
         const fail = async (message: string, reason: TradeResult['reason'], decline: boolean): Promise<TradeResult> => {
-            if (decline) await this.declineTrade();
+            if (decline) {
+                const declined = await this.declineTrade();
+                const settled = await this.classifyClosedSession(sessionBaseline, partner, invBefore, offers);
+                if (settled.success) {
+                    return { ...settled, message: `${settled.message} (completed as the decline landed: ${message})` };
+                }
+                if (!declined.success) {
+                    message = `${message}; ${declined.message}`;
+                }
+            }
             return { success: false, message, partner, gave: [], received: [], reason };
         };
 
         // 1. Put our items in.
         if (options.give?.length) {
             const offered = await this.offerTradeItems(options.give, Math.max(1, deadline - Date.now()));
+            noteOffers(this.sdk.getTradeState());
             if (!offered.success) {
                 if (!this.sdk.getTradeState().isOpen) {
-                    return this.classifyClosedSession(sessionBaseline, partner, invBefore);
+                    return this.classifyClosedSession(sessionBaseline, partner, invBefore, offers);
                 }
                 return fail(offered.message, 'offer_failed', true);
             }
@@ -2290,10 +2349,11 @@ export class BotActions {
             const trade = this.sdk.getTradeState();
             if (!trade.isOpen) {
                 if (await this.tradeClosedForGood()) {
-                    return this.classifyClosedSession(sessionBaseline, partner, invBefore);
+                    return this.classifyClosedSession(sessionBaseline, partner, invBefore, offers);
                 }
                 continue; // transition blip - the confirm screen is opening
             }
+            noteOffers(trade);
             if (trade.screen === 'confirm') break;
 
             const tick = this.sdk.getState()?.tick ?? 0;
@@ -2328,10 +2388,11 @@ export class BotActions {
             const trade = this.sdk.getTradeState();
             if (!trade.isOpen) {
                 if (await this.tradeClosedForGood()) {
-                    return this.classifyClosedSession(sessionBaseline, partner, invBefore);
+                    return this.classifyClosedSession(sessionBaseline, partner, invBefore, offers);
                 }
                 continue;
             }
+            noteOffers(trade);
             if (trade.screen !== 'confirm') {
                 // Back on the offer screen: only possible if this session was
                 // re-entered mid-transition; let the offer-screen rules apply.
@@ -2361,34 +2422,52 @@ export class BotActions {
     }
 
     /**
-     * Once the trade screen has closed, decide from game messages whether the
-     * trade completed ("Accepted trade.") or was declined, and report the
-     * inventory delta for a completion.
+     * Once the trade screen has closed, decide whether the trade completed
+     * or was declined, and report the inventory delta for a completion.
+     *
+     * Primary signal is the "Accepted trade." message; fallback is our
+     * offered items still being gone from the inventory two ticks after close.
      */
     private async classifyClosedSession(
         sessionBaseline: number,
         partner: string | undefined,
-        invBefore: Map<number, { name: string; count: number }>
+        invBefore: Map<number, { name: string; count: number }>,
+        offers: { mine: TradeItem[]; theirs: TradeItem[] } = { mine: [], theirs: [] }
     ): Promise<TradeResult> {
         // Let the post-trade inventory update land before diffing.
         await this.sdk.waitForTicks(1).catch(() => {});
 
         const declined = this.helpers.findRefusal(sessionBaseline, ['declined trade']);
         const noSpace = this.helpers.findRefusal(sessionBaseline, ['enough inventory space']);
-        const accepted = this.helpers.findRefusal(sessionBaseline, ['accepted trade']);
+        let accepted = this.helpers.findRefusal(sessionBaseline, ['accepted trade']);
+        let inferred = false;
+
+        const diff = () => diffInventories(invBefore, countInventoryById(this.sdk.getState()?.inventory ?? []));
+        let { gained, lost } = diff();
+
+        const offerGone = () => offers.mine.length > 0 && shortfall(offers.mine, lost).length === 0;
+        if (!accepted && !declined && !noSpace && offerGone()) {
+            await this.sdk.waitForTicks(1).catch(() => {});
+            ({ gained, lost } = diff());
+            if (offerGone()) {
+                accepted = 'inferred';
+                inferred = true;
+            }
+        }
 
         if (accepted) {
-            const invAfter = countInventoryById(this.sdk.getState()?.inventory ?? []);
-            const { gained, lost } = diffInventories(invBefore, invAfter);
             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';
-            return {
-                success: true,
-                message: `Trade completed with ${partner ?? 'partner'}: gave ${gaveText}, received ${receivedText}`,
-                partner,
-                gave: lost,
-                received: gained,
-            };
+            let message = `Trade completed with ${partner ?? 'partner'}: gave ${gaveText}, received ${receivedText}`;
+            if (inferred) message += ' (inferred from inventory: "Accepted trade." message not seen)';
+
+            // Overflow at completion is dropped on the ground, not refused.
+            const possiblyDropped = shortfall(offers.theirs, gained);
+            if (possiblyDropped.length) {
+                message += `; ${possiblyDropped.map(i => `${i.name} x${i.count}`).join(', ')} shown in their final offer did not reach your inventory — check the ground under you (inventory full?)`;
+                return { success: true, message, partner, gave: lost, received: gained, possiblyDropped };
+            }
+            return { success: true, message, partner, gave: lost, received: gained };
         }
         if (noSpace) {
             return { success: false, message: noSpace, partner, gave: [], received: [], reason: 'no_space' };

sdk/test/trade-actions.test.ts

@@ -62,12 +62,15 @@ function state(frame: Frame, tick: number) {
  * fast-forwards to the last frame (the trade being closed), mirroring the
  * server closing both screens.
  */
-function createHarness(frames: Frame[]) {
+function createHarness(frames: Frame[], opts: { hold?: number[] } = {}) {
     const sequence = frames.map((frame, i) => state(frame, i + 1));
+    // Frames the harness refuses to auto-advance past: waits time out there
+    // until a decline fast-forwards. Models "nothing happens until we act".
+    const hold = new Set(opts.hold ?? []);
     let idx = 0;
     const dispatched: Array<{ call: string; args: unknown[] }> = [];
     const advance = () => {
-        if (idx < sequence.length - 1) idx++;
+        if (idx < sequence.length - 1 && !hold.has(idx)) idx++;
         return sequence[idx];
     };
     const record = (call: string, effect?: () => void) => async (...args: unknown[]) => {
@@ -83,7 +86,7 @@ function createHarness(frames: Frame[]) {
         waitForCondition: async (pred: (s: any) => boolean) => {
             for (let guard = 0; guard < sequence.length + 1; guard++) {
                 if (pred(sequence[idx])) return sequence[idx];
-                if (idx >= sequence.length - 1) break;
+                if (idx >= sequence.length - 1 || hold.has(idx)) break;
                 idx++;
             }
             throw new Error('waitForCondition timed out');
@@ -253,3 +256,131 @@ describe('bot.serveTrades', () => {
         expect(harness.dispatched).toEqual([]);
     });
 });
+
+describe('bot.trade completion detection', () => {
+    const logsInv = [{ slot: 0, id: 1511, name: 'Logs', count: 1 }];
+
+    test('infers completion from the inventory when "Accepted trade." was never seen', async () => {
+        const harness = createHarness([
+            { inventory: logsInv },
+            { trade: { isOpen: true, screen: 'offer', partner: 'MuleBot' }, inventory: logsInv },
+            { trade: { isOpen: true, screen: 'offer', partner: 'MuleBot', myOffer: [logs(1)] }, inventory: [] },
+            { trade: { isOpen: true, screen: 'confirm', partner: 'MuleBot', myOffer: [logs(1)] }, inventory: [] },
+            // Closed, no message (scrolled out of the client window), logs still gone.
+            { inventory: [] },
+            { inventory: [] },
+            { inventory: [] },
+        ]);
+
+        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.message).toContain('inferred from inventory');
+    });
+
+    test('a decline whose item return lands a tick late is still a decline', async () => {
+        const harness = createHarness([
+            { inventory: logsInv },
+            { trade: { isOpen: true, screen: 'offer', partner: 'MuleBot' }, inventory: logsInv },
+            { trade: { isOpen: true, screen: 'offer', partner: 'MuleBot', myOffer: [logs(1)] }, inventory: [] },
+            // Closed with no message; inventory update lags one tick.
+            { inventory: [] },
+            { inventory: [] },
+            { inventory: logsInv },
+            { inventory: logsInv },
+        ]);
+
+        const result = await harness.bot.trade(partner, { give: [{ item: 'logs', amount: -1 }], timeout: 3_000 });
+
+        expect(result.success).toBe(false);
+        expect(result.reason).toBe('declined');
+        expect(result.gave).toEqual([]);
+    });
+
+    test('a timeout whose decline races the partner accept reports the completed trade', async () => {
+        const coins = (n: number) => item(0, 995, 'Coins', n);
+        const harness = createHarness([
+            { inventory: logsInv },
+            { trade: { isOpen: true, screen: 'offer', partner: 'MuleBot', theirOffer: [coins(50)] }, inventory: logsInv },
+            { trade: { isOpen: true, screen: 'offer', partner: 'MuleBot', myOffer: [logs(1)], theirOffer: [coins(50)] }, inventory: [] },
+            // Confirm screen: we accept, partner never does (held) -> deadline expires.
+            { trade: { isOpen: true, screen: 'confirm', partner: 'MuleBot', myOffer: [logs(1)], theirOffer: [coins(50)], myAccepted: true }, inventory: [] },
+            // The decline arrives after the partner's accept: trade completed.
+            { message: 'Accepted trade.', inventory: [{ slot: 0, id: 995, name: 'Coins', count: 50 }] },
+            { message: 'Accepted trade.', inventory: [{ slot: 0, id: 995, name: 'Coins', count: 50 }] },
+        ], { hold: [3] });
+
+        const result = await harness.bot.trade(partner, {
+            give: [{ item: 'logs', amount: -1 }],
+            want: [{ item: 'coins', amount: 50 }],
+            timeout: 300,
+        });
+
+        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.message).toContain('completed as the decline landed');
+    });
+
+    test('flags items from the final offer that never reached the inventory', async () => {
+        const coins = (n: number) => item(0, 995, 'Coins', n);
+        const lobster = item(1, 379, 'Lobster', 1);
+        const harness = createHarness([
+            { },
+            { trade: { isOpen: true, screen: 'offer', partner: 'MuleBot', theirOffer: [coins(250), lobster] } },
+            { trade: { isOpen: true, screen: 'confirm', partner: 'MuleBot', theirOffer: [coins(250), lobster] } },
+            { message: 'Accepted trade.', inventory: [{ slot: 0, id: 995, name: 'Coins', count: 250 }] },
+            { message: 'Accepted trade.', inventory: [{ slot: 0, id: 995, name: 'Coins', count: 250 }] },
+        ]);
+
+        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.message).toContain('check the ground');
+    });
+});
+
+describe('bot.acceptTrade', () => {
+    test('reports a decline as failure, not "Trade screen closed"', async () => {
+        const harness = createHarness([
+            { trade: { isOpen: true, screen: 'offer', partner: 'MuleBot' } },
+            { message: 'Other player declined trade.' },
+            { message: 'Other player declined trade.' },
+            { message: 'Other player declined trade.' },
+        ]);
+
+        const result = await harness.bot.acceptTrade(2_000);
+        expect(result.success).toBe(false);
+        expect(result.reason).toBe('declined');
+    });
+
+    test('reports the delta when the confirm accept completes the trade', async () => {
+        const sword = item(0, 1285, 'Mithril sword', 1);
+        const harness = createHarness([
+            { trade: { isOpen: true, screen: 'confirm', partner: 'MuleBot', myOffer: [item(0, 995, 'Coins', 229)], theirOffer: [sword] }, inventory: [] },
+            { message: 'Accepted trade.', inventory: [{ slot: 0, id: 1285, name: 'Mithril sword', count: 1 }] },
+            { message: 'Accepted trade.', inventory: [{ slot: 0, id: 1285, name: 'Mithril sword', count: 1 }] },
+            { message: 'Accepted trade.', inventory: [{ slot: 0, id: 1285, name: 'Mithril sword', count: 1 }] },
+        ]);
+
+        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 }]);
+    });
+
+    test('treats the offer->confirm blip as progress, not a close', async () => {
+        const harness = createHarness([
+            { trade: { isOpen: true, screen: 'offer', partner: 'MuleBot' } },
+            { }, // side modal closed, confirm not yet open
+            { trade: { isOpen: true, screen: 'confirm', partner: 'MuleBot' } },
+            { trade: { isOpen: true, screen: 'confirm', partner: 'MuleBot' } },
+        ]);
+
+        const result = await harness.bot.acceptTrade(2_000);
+        expect(result).toMatchObject({ success: true, message: 'Advanced to confirm screen' });
+    });
+});

sdk/test/trade-helpers.test.ts

@@ -1,5 +1,5 @@
 import { describe, expect, test } from 'bun:test';
-import { countInventoryById, countMatching, diffInventories, missingFromOffer, offerSatisfies } from '../trade-helpers';
+import { countInventoryById, countMatching, diffInventories, missingFromOffer, offerSatisfies, shortfall } from '../trade-helpers';
 import type { TradeItem } from '../types';
 
 const offer = (...items: Array<[string, number]>): TradeItem[] =>
@@ -74,3 +74,18 @@ describe('inventory diffing', () => {
         expect(lost).toEqual([{ slot: -1, id: 1, name: 'Logs', count: 3 }]);
     });
 });
+
+describe('shortfall', () => {
+    const it = (id: number, name: string, count: number): TradeItem => ({ slot: -1, id, name, 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)]))
+            .toEqual([it(379, 'Lobster', 1)]);
+        expect(shortfall([it(995, 'Coins', 250)], [it(995, 'Coins', 100)])).toEqual([it(995, 'Coins', 150)]);
+    });
+
+    test('empty when everything (or more) arrived', () => {
+        expect(shortfall([it(995, 'Coins', 250)], [it(995, 'Coins', 250)])).toEqual([]);
+        expect(shortfall([], [it(995, 'Coins', 5)])).toEqual([]);
+    });
+});

sdk/trade-helpers.ts

@@ -54,6 +54,24 @@ export function diffInventories(
     return { gained, lost };
 }
 
+/** Items (by id) in `expected` that `received` falls short of. */
+export function shortfall(expected: TradeItem[], received: TradeItem[]): TradeItem[] {
+    const got = new Map<number, number>();
+    for (const item of received) got.set(item.id, (got.get(item.id) ?? 0) + item.count);
+    const out: TradeItem[] = [];
+    const seen = new Map<number, TradeItem>();
+    for (const item of expected) {
+        const existing = seen.get(item.id);
+        if (existing) existing.count += item.count;
+        else seen.set(item.id, { slot: -1, id: item.id, name: item.name, count: item.count });
+    }
+    for (const item of seen.values()) {
+        const missing = item.count - (got.get(item.id) ?? 0);
+        if (missing > 0) out.push({ ...item, count: missing });
+    }
+    return out;
+}
+
 /** Aggregate an inventory-like item list into an (id -> {name, count}) map. */
 export function countInventoryById(items: Array<{ id: number; name: string; count: number }>): Map<number, { name: string; count: number }> {
     const map = new Map<number, { name: string; count: number }>();

sdk/types.ts

@@ -371,8 +371,10 @@ export interface TradeOptions {
      * on the partner's visible offer. Also re-checked on the confirm screen.
      */
     accept?: (theirOffer: TradeItem[]) => boolean;
-    /** Overall deadline in ms (default 60_000). On expiry the trade is declined. */
+    /** 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;
 }
 
 export interface TradeResult {
@@ -386,6 +388,8 @@ export interface TradeResult {
     received: TradeItem[];
     reason?: 'player_not_found' | 'no_response' | 'declined' | 'busy' | 'offer_failed'
         | 'want_not_met' | 'no_space' | 'not_open' | 'timeout' | 'error';
+    /** Items from the partner's final offer that did not reach the inventory (dropped on the ground on overflow). */
+    possiblyDropped?: TradeItem[];
 }
 
 export interface ServeTradesOptions {

server/webclient/src/client/Client.ts

@@ -1721,12 +1721,9 @@ export class Client extends GameShell {
             return false;
         }
 
-        // Check if any modal is open
-        if (this.mainModalId === -1 && this.sideModalId === -1) {
-            return true; // Already closed
-        }
-
-        // Use the existing private closeModal() which handles all cleanup
+        // Always send CLOSE_MODAL: local modal ids can lag the server (trade
+        // offer->confirm transition). Note the engine still clears the weak
+        // queue on a stray close.
         this.closeModal();
 
         return true;

server/webclient/src/lite/actions.ts

@@ -415,9 +415,8 @@ export function closeBotModal(c: LiteClient): boolean {
     if (!c.isInGame()) {
         return false;
     }
-    if (c.mainModalId === -1 && c.sideModalId === -1) {
-        return false;
-    }
+    // Always send CLOSE_MODAL: local modal ids can lag the server (mirrors
+    // Client.closeBotModal).
     c.writeOpcode(ClientProt.CLOSE_MODAL);
     return true;
 }