Skip to content

Commit afaf851

Browse files
chrisleekrclaude
andcommitted
feat(worker): alert when a held position has no protective stop resting
The existing protective-stop alert is shaped around one cause: the exchange's price band refuses the stop. Every other way a stop fails to reach the exchange produced a condition row the strategy re-reported on every tick and nothing ever escalated, so a position could sit unguarded indefinitely without the operator being told. Add an outcome-shaped alert beside it: whatever the cause, this position is held and nothing is resting that would sell it. It fires off the exit-blocked row carrying the unplaced code, gated on that row's age, so it cannot page about a position that opened moments ago. It fails closed on an undated span, unlike the band alert. That one has an independent span-free signal for a permanent fault; this one does not, and without the age it cannot be told apart from a fresh entry. The row is re-read next tick, so a lost write costs a tick of delay, not the alert. It is suppressed on any tick the band alert already fired, since both describe the same unguarded coin and the band one carries the more specific instruction, and it takes its own throttle key so the two alerts cannot mute each other on exactly the coin where both matter. The band explanation is now parsed before its throttle window opens; a detail it could not parse previously threw after the key was set, losing that alert and muting the next hour of them. Also closes a detached-fill leak: the reconciler's own terminal-status set omitted the self-trade-prevention status, so such a row was never closed and held its live slot and open exposure forever. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01YRceiDYdzzHo6aLFr4sZPj
1 parent 1c6eb54 commit afaf851

14 files changed

Lines changed: 907 additions & 39 deletions

apps/worker/__tests__/boot/builders/notifiers.test.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@ describe('buildNotifiers', () => {
2525
'orderFailedThrottle',
2626
'orderRefusalLoopThrottle',
2727
'protectiveStopBlockedThrottle',
28+
'protectiveStopUnplacedThrottle',
2829
]);
2930
expect(typeof n.accountNotify).toBe('function');
3031
expect(typeof n.notifyEvent).toBe('function');

apps/worker/__tests__/boot/builders/tick-handler-override-deps.test.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -101,6 +101,7 @@ const buildDeps = (): TickHandlerDeps => {
101101
orderFailedThrottle: { allow: async () => true } as never,
102102
orderRefusalLoopThrottle: { allow: async () => true } as never,
103103
protectiveStopBlockedThrottle: { allow: async () => true } as never,
104+
protectiveStopUnplacedThrottle: { allow: async () => true } as never,
104105
auditShipper: anyProxy(),
105106
});
106107
const deps = captured.deps;

apps/worker/__tests__/boot/builders/tick-handler.test.ts

Lines changed: 180 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -34,12 +34,14 @@ interface Built {
3434
readonly deps: TickHandlerDeps;
3535
readonly events: Record<string, unknown>[];
3636
readonly stopKeys: string[];
37+
readonly unplacedKeys: string[];
3738
readonly refusalKeys: string[];
3839
}
3940

4041
const build = (allowStop = true, allowRefusal = true): Built => {
4142
const events: Record<string, unknown>[] = [];
4243
const stopKeys: string[] = [];
44+
const unplacedKeys: string[] = [];
4345
const refusalKeys: string[] = [];
4446
const slice = buildTickHandler({
4547
env: ENV,
@@ -70,11 +72,17 @@ const build = (allowStop = true, allowRefusal = true): Built => {
7072
return allowStop;
7173
},
7274
} as never,
75+
protectiveStopUnplacedThrottle: {
76+
allow: async (key: string) => {
77+
unplacedKeys.push(key);
78+
return allowStop;
79+
},
80+
} as never,
7381
auditShipper: anyProxy(),
7482
});
7583
const deps = captured.deps;
7684
if (!deps) throw new Error('buildTickHandler did not construct a tick handler');
77-
return { slice, deps, events, stopKeys, refusalKeys };
85+
return { slice, deps, events, stopKeys, unplacedKeys, refusalKeys };
7886
};
7987

8088
const BAND = {
@@ -208,6 +216,36 @@ describe('buildTickHandler — the order-refusal-loop notifier', () => {
208216
});
209217
});
210218

219+
describe('buildTickHandler — the order-failed notifier', () => {
220+
it('reports a failed replacement as an unguarded position with the replace action label', async () => {
221+
const { deps, events } = build();
222+
const notify = deps.notifyOrderFailed;
223+
if (!notify) throw new Error('the builder did not wire notifyOrderFailed');
224+
225+
await notify({
226+
operatorId: OPERATOR,
227+
accountId: ACCOUNT,
228+
profileId: PROFILE,
229+
symbol: SYMBOL,
230+
decisionType: 'replace-order',
231+
result: {
232+
ok: false,
233+
retryable: false,
234+
phase: 'rejected',
235+
reason: 'cancelReplace: naked after -2021, re-place failed',
236+
},
237+
willRetry: false,
238+
});
239+
240+
expect(events).toHaveLength(1);
241+
expect(events[0]).toMatchObject({
242+
category: 'order-failed',
243+
body: 'The bot could not re-arm a protective stop, and the position is currently unguarded. Check it on Binance.',
244+
fields: expect.arrayContaining([{ label: 'Action', value: 'Replace order' }]),
245+
});
246+
});
247+
});
248+
211249
describe('buildTickHandler — the protective-stop-blocked notifier', () => {
212250
it('keys the throttle on the escalation level so the two never mute each other', async () => {
213251
// "Wait for the price to come back" and "no price ever arms this stop" are
@@ -313,3 +351,144 @@ describe('buildTickHandler — the protective-stop-blocked notifier', () => {
313351
expect(event.fields.map((f) => f.label)).not.toContain('Blocked for');
314352
});
315353
});
354+
355+
describe('buildTickHandler — the protective-stop-unplaced notifier', () => {
356+
const unplaced = (over: Record<string, unknown> = {}) => ({
357+
operatorId: OPERATOR,
358+
accountId: ACCOUNT,
359+
profileId: PROFILE,
360+
symbol: SYMBOL,
361+
sinceMs: Date.now() - 2 * 3_600_000,
362+
detail: { stop: '11.5511' },
363+
...over,
364+
});
365+
366+
it('keys the throttle on the coin alone, with no escalation level to split', async () => {
367+
// Unlike the band refusal there is one thing to say here — nothing is guarding this position — so a second key segment would only split one message into two alerts an hour apart.
368+
const { deps, unplacedKeys } = build();
369+
const notify = deps.notifyProtectiveStopUnplaced;
370+
if (!notify) throw new Error('the builder did not wire notifyProtectiveStopUnplaced');
371+
372+
await notify(unplaced());
373+
374+
expect(unplacedKeys).toEqual([`${PROFILE}:${SYMBOL}`]);
375+
});
376+
377+
it('sends nothing when the throttle window is already open', async () => {
378+
const { deps, events } = build(false);
379+
const notify = deps.notifyProtectiveStopUnplaced;
380+
if (!notify) throw new Error('the builder did not wire notifyProtectiveStopUnplaced');
381+
382+
await notify(unplaced());
383+
384+
expect(events).toEqual([]);
385+
});
386+
387+
it('files under order-failed, which already covers a stop that never reached the exchange', async () => {
388+
// That category is severity error and defaults ON, so the alert about an unguarded position is not behind a switch the operator has to find first.
389+
const { deps, events } = build();
390+
const notify = deps.notifyProtectiveStopUnplaced;
391+
if (!notify) throw new Error('the builder did not wire notifyProtectiveStopUnplaced');
392+
393+
await notify(unplaced());
394+
395+
expect(events).toHaveLength(1);
396+
expect(events[0]).toMatchObject({ category: 'order-failed', symbol: SYMBOL });
397+
});
398+
399+
it('leads with the exposure and how long it has run, then what to check', async () => {
400+
const { deps, events } = build();
401+
const notify = deps.notifyProtectiveStopUnplaced;
402+
if (!notify) throw new Error('the builder did not wire notifyProtectiveStopUnplaced');
403+
404+
await notify(unplaced());
405+
406+
const event = events[0] as { body: string; fields: { label: string; value: string }[] };
407+
expect(event.body).toContain('no protective stop on Binance for 2 hours');
408+
expect(event.body).toContain('Nothing on the exchange will sell it if the price falls');
409+
expect(event.body).toContain('another order is holding the coins');
410+
expect(event.fields).toContainEqual({ label: 'Unprotected for', value: '2 hours' });
411+
expect(event.fields).toContainEqual({ label: 'Wanted stop', value: '11.5511' });
412+
});
413+
414+
it('omits the wanted stop when the strategy could not price one', async () => {
415+
// The strategy publishes `stop: null` when it could not derive a level at all, and a field reading "null" tells the operator nothing while implying a number exists.
416+
const { deps, events } = build();
417+
const notify = deps.notifyProtectiveStopUnplaced;
418+
if (!notify) throw new Error('the builder did not wire notifyProtectiveStopUnplaced');
419+
420+
await notify(unplaced({ detail: { stop: null } }));
421+
422+
const event = events[0] as { fields: { label: string }[] };
423+
expect(event.fields.map((f) => f.label)).not.toContain('Wanted stop');
424+
});
425+
426+
it('names the trailing-stop refusal and the distance Binance would not take', async () => {
427+
// The cause this alert exists for: a native-trail profile on a symbol whose trailingDelta filter has no step for the wanted distance never places anything, and no price move clears that. The distance is quoted in the percent the settings screen shows, not the fraction the strategy records.
428+
const { deps, events } = build();
429+
const notify = deps.notifyProtectiveStopUnplaced;
430+
if (!notify) throw new Error('the builder did not wire notifyProtectiveStopUnplaced');
431+
432+
await notify(
433+
unplaced({ detail: { stop: '11.5511', nativeUnavailable: true, distancePct: '0.037' } }),
434+
);
435+
436+
const event = events[0] as { fields: { label: string; value: string }[] };
437+
const why = event.fields.find((f) => f.label === 'Why');
438+
expect(why?.value).toContain('3.7% below the high');
439+
expect(why?.value).toContain('follows the price up');
440+
expect(why?.value).toContain("Switch this profile's protective stop mode to priced");
441+
});
442+
443+
it('drops the distance from the refusal rather than quote one it cannot read', async () => {
444+
// `distancePct` is null whenever the strategy could not derive the distance at all, and the bag has crossed a JSON round-trip besides. The lever still has to be named, so the clause loses the number and keeps the advice.
445+
const { deps, events } = build();
446+
const notify = deps.notifyProtectiveStopUnplaced;
447+
if (!notify) throw new Error('the builder did not wire notifyProtectiveStopUnplaced');
448+
449+
await notify(unplaced({ detail: { stop: '11.5511', nativeUnavailable: true } }));
450+
await notify(
451+
unplaced({ detail: { stop: '11.5511', nativeUnavailable: true, distancePct: 0.037 } }),
452+
);
453+
await notify(
454+
unplaced({ detail: { stop: '11.5511', nativeUnavailable: true, distancePct: 'soon' } }),
455+
);
456+
457+
for (const raw of events) {
458+
const event = raw as { fields: { label: string; value: string }[] };
459+
const why = event.fields.find((f) => f.label === 'Why');
460+
expect(why?.value).toContain('at the distance your settings ask for');
461+
expect(JSON.stringify(event)).not.toContain('undefined');
462+
expect(JSON.stringify(event)).not.toContain('%');
463+
}
464+
});
465+
466+
it('still reports the exposure when the detail bag did not survive the round-trip', async () => {
467+
// The duration is the whole signal and it comes off the input, not the bag, so a bag that arrived as null must cost the operator its optional fields and nothing else.
468+
const { deps, events } = build();
469+
const notify = deps.notifyProtectiveStopUnplaced;
470+
if (!notify) throw new Error('the builder did not wire notifyProtectiveStopUnplaced');
471+
472+
await notify(unplaced({ detail: null }) as never);
473+
474+
const event = events[0] as { body: string; fields: { label: string }[] };
475+
expect(event.body).toContain('no protective stop on Binance for 2 hours');
476+
expect(event.fields).toEqual([{ label: 'Unprotected for', value: '2 hours' }]);
477+
});
478+
479+
it('says nothing about trailing stops when that is not what blocked this one', async () => {
480+
// The ordinary unplaced case has no named cause, and inventing one would send an operator to a setting that is not theirs: a priced-mode profile has no trail distance to change.
481+
const { deps, events } = build();
482+
const notify = deps.notifyProtectiveStopUnplaced;
483+
if (!notify) throw new Error('the builder did not wire notifyProtectiveStopUnplaced');
484+
485+
await notify(unplaced());
486+
487+
const event = events[0] as { fields: { label: string; value: string }[] };
488+
expect(event.fields.map((f) => f.label)).not.toContain('Why');
489+
expect(event.fields).toEqual([
490+
{ label: 'Unprotected for', value: '2 hours' },
491+
{ label: 'Wanted stop', value: '11.5511' },
492+
]);
493+
});
494+
});
Lines changed: 160 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,160 @@
1+
// Which statuses close a DETACHED order's ledger row.
2+
//
3+
// `reconcileDetachedFill` is the only thing that ever stamps `closed_at` on an order whose profile was deleted. A status it does not recognise as terminal leaves the row open forever: it keeps its `orders_one_live_per_intent` live slot and keeps counting toward the account's open exposure, which backs the delete-account guard. So the set of statuses it accepts is a contract, and it must be the SHARED one — the same predicate the open-orders cache eviction and the boot reaper read. `EXPIRED_IN_MATCH` is the status that proves it: Binance stamps it when self-trade prevention kills an order, which on a shared account wallet is exactly what a sibling profile's BUY crossing our resting SELL produces.
4+
5+
import { describe, expect, it, vi, beforeEach } from 'vitest';
6+
import type { Logger } from 'pino';
7+
import type { Queue } from 'bullmq';
8+
9+
import { asAccountId, asUserId } from '@app/contracts';
10+
11+
import { createFillAdopter, type DetachedOrderEvent } from '../../src/executor/fill-adopter.js';
12+
import { createChainByKey } from '../../src/lib/chain-by-key.js';
13+
import type { StatePort } from '../../src/state/state-port.js';
14+
import type { SymbolInfoCache } from '../../src/tick/symbol-info-cache.js';
15+
16+
const OPERATOR_ID = asUserId('00000000-0000-0000-0000-0000000705a1');
17+
const ACCOUNT_ID = asAccountId('00000000-0000-0000-0000-0000000705c1');
18+
const SYMBOL = 'XPLUSDT';
19+
// Above 2^32 so a lossy number/bigint hop surfaces as a miss rather than a pass.
20+
const ORDER_ID = 8_700_000_705;
21+
22+
const repoMocks = vi.hoisted(() => ({
23+
ordersFindByBinanceOrderId: vi.fn(),
24+
ordersCloseByBinanceOrderId: vi.fn(async () => 1),
25+
ordersMarkFilledByBinanceOrderId: vi.fn(async () => 1),
26+
}));
27+
28+
const accountScope = { db: undefined as unknown, operatorId: OPERATOR_ID, accountId: ACCOUNT_ID };
29+
30+
vi.mock('@app/db', async (importOriginal) => {
31+
const orig = await importOriginal<typeof import('@app/db')>();
32+
return {
33+
...orig,
34+
scopeAccount: vi.fn(async () => accountScope),
35+
accountRepoFromScope: vi.fn(() => ({
36+
orders: {
37+
findByBinanceOrderId: repoMocks.ordersFindByBinanceOrderId,
38+
closeByBinanceOrderId: repoMocks.ordersCloseByBinanceOrderId,
39+
markFilledByBinanceOrderId: repoMocks.ordersMarkFilledByBinanceOrderId,
40+
},
41+
})),
42+
};
43+
});
44+
45+
const noopLogger = {
46+
info: () => undefined,
47+
warn: () => undefined,
48+
error: () => undefined,
49+
debug: () => undefined,
50+
} as unknown as Logger;
51+
52+
/**
53+
* The production adopter with its position-side deps rigged to THROW. The detached path is the ledger half of adoption with the strategy half deliberately absent, so a regression that starts seeding cost basis or strategy state here fails loudly instead of quietly handing a deleted profile's position to a stranger.
54+
*/
55+
const makeAdopter = () =>
56+
createFillAdopter({
57+
db: {} as never,
58+
chain: createChainByKey(),
59+
logger: noopLogger,
60+
statePort: {
61+
mutate: () => {
62+
throw new Error('detached reconcile must not touch strategy state');
63+
},
64+
} as unknown as StatePort,
65+
registry: {
66+
get: () => {
67+
throw new Error('detached reconcile must not resolve a strategy');
68+
},
69+
},
70+
pipelineQueue: {
71+
add: () => {
72+
throw new Error('detached reconcile must not enqueue an archive');
73+
},
74+
} as unknown as Queue,
75+
symbolInfo: {
76+
get: () => {
77+
throw new Error('detached reconcile must not read symbol info');
78+
},
79+
} as unknown as SymbolInfoCache,
80+
});
81+
82+
const eventWith = (orderStatus: string): DetachedOrderEvent => ({
83+
operatorId: OPERATOR_ID,
84+
accountId: ACCOUNT_ID,
85+
symbol: SYMBOL,
86+
orderId: ORDER_ID,
87+
orderStatus,
88+
cumQty: '0',
89+
cumQuoteQty: '0',
90+
eventTimeMs: 1_735_000_000_000,
91+
});
92+
93+
describe('reconcileDetachedFill — terminal-status vocabulary', () => {
94+
beforeEach(() => {
95+
vi.clearAllMocks();
96+
// A detached row: present, and `profileId` null is what makes it ours to close.
97+
repoMocks.ordersFindByBinanceOrderId.mockResolvedValue({ profileId: null });
98+
repoMocks.ordersCloseByBinanceOrderId.mockResolvedValue(1);
99+
repoMocks.ordersMarkFilledByBinanceOrderId.mockResolvedValue(1);
100+
});
101+
102+
it('closes the row on EXPIRED_IN_MATCH, the self-trade-prevention terminator', async () => {
103+
await makeAdopter().reconcileDetachedFill(eventWith('EXPIRED_IN_MATCH'));
104+
105+
expect(repoMocks.ordersCloseByBinanceOrderId).toHaveBeenCalledTimes(1);
106+
expect(repoMocks.ordersCloseByBinanceOrderId).toHaveBeenCalledWith(
107+
BigInt(ORDER_ID),
108+
'EXPIRED_IN_MATCH',
109+
1_735_000_000_000,
110+
);
111+
// The exchange's own status is what lands on the row, so the ledger records WHY the order left the book.
112+
expect(repoMocks.ordersMarkFilledByBinanceOrderId).not.toHaveBeenCalled();
113+
});
114+
115+
it.each(['CANCELED', 'EXPIRED', 'REJECTED'])('closes the row on %s', async (status) => {
116+
await makeAdopter().reconcileDetachedFill(eventWith(status));
117+
118+
expect(repoMocks.ordersCloseByBinanceOrderId).toHaveBeenCalledTimes(1);
119+
expect(repoMocks.ordersCloseByBinanceOrderId).toHaveBeenCalledWith(
120+
BigInt(ORDER_ID),
121+
status,
122+
1_735_000_000_000,
123+
);
124+
});
125+
126+
it('routes FILLED to the totals-merging close, not the plain one', async () => {
127+
await makeAdopter().reconcileDetachedFill({
128+
...eventWith('FILLED'),
129+
cumQty: '12',
130+
cumQuoteQty: '340',
131+
});
132+
133+
expect(repoMocks.ordersMarkFilledByBinanceOrderId).toHaveBeenCalledTimes(1);
134+
expect(repoMocks.ordersMarkFilledByBinanceOrderId).toHaveBeenCalledWith(
135+
BigInt(ORDER_ID),
136+
{ executedQty: '12', cummulativeQuoteQty: '340' },
137+
1_735_000_000_000,
138+
);
139+
expect(repoMocks.ordersCloseByBinanceOrderId).not.toHaveBeenCalled();
140+
});
141+
142+
it.each(['NEW', 'PARTIALLY_FILLED', 'PENDING_CANCEL', 'SOME_STATUS_BINANCE_ADDS_TOMORROW'])(
143+
'leaves the row open on %s — a still-live commitment SHOULD keep counting toward exposure',
144+
async (status) => {
145+
await makeAdopter().reconcileDetachedFill(eventWith(status));
146+
147+
expect(repoMocks.ordersFindByBinanceOrderId).not.toHaveBeenCalled();
148+
expect(repoMocks.ordersCloseByBinanceOrderId).not.toHaveBeenCalled();
149+
expect(repoMocks.ordersMarkFilledByBinanceOrderId).not.toHaveBeenCalled();
150+
},
151+
);
152+
153+
it('refuses to close a row that still has a profile, whatever the status says', async () => {
154+
repoMocks.ordersFindByBinanceOrderId.mockResolvedValue({ profileId: 'still-owned' });
155+
156+
await makeAdopter().reconcileDetachedFill(eventWith('EXPIRED_IN_MATCH'));
157+
158+
expect(repoMocks.ordersCloseByBinanceOrderId).not.toHaveBeenCalled();
159+
});
160+
});

0 commit comments

Comments
 (0)