From 2593d29e7debbd0a68457b991445ddeb2dd60e39 Mon Sep 17 00:00:00 2001 From: suzuke Date: Mon, 27 Apr 2026 11:04:46 +0800 Subject: [PATCH] feat(#24): cost-guard pre-check at outbound dispatch MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Before this change, when a target instance hit its daily cost limit the limit handler paused it but the rest of the fleet still dispatched new messages to it via send_to_instance / delegate_task. The sender saw no signal until the lack of reply timed them out, and the paused target accumulated a backlog it could not act on. Add a fleet-wide pre-check at the outbound dispatch handler: - Expose `costGuard: CostGuard | null` on `OutboundContext`. FleetManager already owns the field; the interface declaration just makes it available to handler code. - At the top of `sendToInstance`, when `costGuard?.isLimited(target)` is true, short-circuit with an error response that names the target and its daily limit (in dollars). Caller's MCP tool surfaces this as an immediate error rather than waiting for a silent timeout. - `delegate_task` funnels through `sendToInstance` via wrapAsSend with request_kind="task", so it inherits the gate without duplication. - `report_result` (request_kind="report") bypasses the gate. Terminal status updates must reach the orchestrator even when the target is paused, otherwise the merge gate stalls. - Null-safe: when fleet config has no cost guard, `ctx.costGuard` is null and the optional chain skips the check entirely. Tests cover the operator-mandated three states for both handlers, plus the report-kind bypass: - (a) limited → immediate warning, no dispatch - (b) not limited → normal dispatch - (c) ctx.costGuard null → graceful fallback, no error - report_kind → bypasses gate even when limited `npx tsc --noEmit` clean. `npx vitest run` 63 files / 524 tests pass (+1 file, +7 tests over baseline; no regressions). Closes #24. Co-Authored-By: Claude Opus 4.7 (1M context) --- src/outbound-handlers.ts | 23 +++ tests/outbound-cost-guard-precheck.test.ts | 184 +++++++++++++++++++++ 2 files changed, 207 insertions(+) create mode 100644 tests/outbound-cost-guard-precheck.test.ts diff --git a/src/outbound-handlers.ts b/src/outbound-handlers.ts index e374ae22..57d89d3c 100644 --- a/src/outbound-handlers.ts +++ b/src/outbound-handlers.ts @@ -7,6 +7,7 @@ import type { Logger } from "./logger.js"; import type { RoutingEngine } from "./routing-engine.js"; import type { InstanceLifecycle, LifecycleCreateArgs } from "./instance-lifecycle.js"; import type { EventLog } from "./event-log.js"; +import type { CostGuard } from "./cost-guard.js"; import type { z } from "zod"; import { BroadcastArgs, @@ -40,6 +41,14 @@ export interface OutboundContext { readonly lifecycle: InstanceLifecycle; readonly sessionRegistry: Map; readonly eventLog: EventLog | null; + /** + * Optional fleet-wide cost guard. When set, outbound dispatch handlers + * short-circuit and surface a warning to the sender if the target is over + * its daily limit (Feature #24). FleetManager already wires this field; the + * interface declaration just exposes it to handler code. Null when cost + * guard is disabled in fleet config. + */ + readonly costGuard: CostGuard | null; lastActivityMs(name: string): number; startInstance(name: string, config: InstanceConfig, topicMode: boolean): Promise; connectIpcToInstance(name: string): Promise; @@ -81,6 +90,20 @@ const sendToInstance: Handler = (ctx, rawArgs, respond, meta) => { const v = validateArgs(SendToInstanceArgs, rawArgs, "send_to_instance"); if (!v.ok) { respond(null, v.error); return; } const { instance_name: targetName, message, request_kind: reqKind, requires_reply, task_summary, working_directory, branch, correlation_id: parsedCorrelationId } = v.data; + + // Feature #24: cost-guard pre-check. Surface the limit to the sender + // immediately rather than dispatch a message that the target instance + // cannot act on (the limit handler pauses the target). `report_result` + // funnels through this handler with `request_kind: "report"` and is + // exempt — terminal status updates must reach the orchestrator even when + // the target is paused, otherwise the merge gate stalls. Null-safe: if + // the fleet has no cost guard configured, isLimited is never called. + if (reqKind !== "report" && ctx.costGuard?.isLimited(targetName)) { + const limitUsd = (ctx.costGuard.getLimitCents() / 100).toFixed(2); + respond(null, `cost-guard: instance '${targetName}' has reached its daily cost limit ($${limitUsd}). Message not delivered — target is paused. Wait for daily reset or escalate to operator.`); + return; + } + const senderLabel = meta.senderSessionName ?? meta.instanceName; const isExternalSender = meta.senderSessionName != null && meta.senderSessionName !== meta.instanceName; diff --git a/tests/outbound-cost-guard-precheck.test.ts b/tests/outbound-cost-guard-precheck.test.ts new file mode 100644 index 00000000..05cda866 --- /dev/null +++ b/tests/outbound-cost-guard-precheck.test.ts @@ -0,0 +1,184 @@ +/** + * Feature #24 regression: cost-guard pre-check at outbound dispatch. + * + * Operator-mandated coverage of three states: + * (a) costGuard.isLimited(target) === true → immediate warning, no dispatch + * (b) costGuard.isLimited(target) === false → normal dispatch + * (c) ctx.costGuard === null → graceful fallback, no error + * + * Verified for both `send_to_instance` (direct) and `delegate_task` (which + * funnels through `sendToInstance` via wrapAsSend with request_kind="task"). + */ +import { describe, it, expect, vi } from "vitest"; +import { outboundHandlers } from "../src/outbound-handlers.js"; + +interface MockIpcChannel { + messages: unknown[]; + ipc: { send: (msg: unknown) => void }; +} + +function mockIpc(): MockIpcChannel { + const messages: unknown[] = []; + return { messages, ipc: { send: (msg) => { messages.push(msg); } } }; +} + +interface CostGuardMock { + isLimited: ReturnType; + getLimitCents: ReturnType; + getDailyCostCents: ReturnType; +} + +function mockCostGuard(opts: { limited: boolean; limitUsd?: number }): CostGuardMock { + const limitCents = (opts.limitUsd ?? 5) * 100; + return { + isLimited: vi.fn(() => opts.limited), + getLimitCents: vi.fn(() => limitCents), + getDailyCostCents: vi.fn(() => opts.limited ? limitCents + 100 : 0), + }; +} + +function makeCtx(costGuard: CostGuardMock | null) { + const targetIpc = mockIpc(); + const senderIpc = mockIpc(); + return { + target: targetIpc, + sender: senderIpc, + ctx: { + fleetConfig: { + instances: { + sender: { working_directory: "/tmp/s" }, + target: { working_directory: "/tmp/t" }, + }, + }, + adapter: null, + logger: { info: vi.fn(), warn: vi.fn(), debug: vi.fn(), error: vi.fn() }, + routing: { resolve: () => undefined }, + instanceIpcClients: new Map([ + ["sender", senderIpc.ipc], + ["target", targetIpc.ipc], + ]), + lifecycle: { daemons: new Map() }, + sessionRegistry: new Map(), + eventLog: null, + costGuard, + lastActivityMs: () => 0, + startInstance: vi.fn(), + connectIpcToInstance: vi.fn(), + } as any, + }; +} + +const meta = { + instanceName: "sender", + requestId: 1, + fleetRequestId: undefined, + senderSessionName: undefined, +}; + +describe("Feature #24 — cost-guard pre-check at outbound dispatch", () => { + describe("send_to_instance", () => { + it("(a) target is limited → immediate warning result, no dispatch", async () => { + const cg = mockCostGuard({ limited: true, limitUsd: 5 }); + const { ctx, target } = makeCtx(cg); + const handler = outboundHandlers.get("send_to_instance")!; + const respond = vi.fn(); + + await handler(ctx, { instance_name: "target", message: "hi" }, respond, meta); + + expect(cg.isLimited).toHaveBeenCalledWith("target"); + expect(target.messages).toHaveLength(0); + expect(respond).toHaveBeenCalledTimes(1); + const [result, error] = respond.mock.calls[0]; + expect(result).toBeNull(); + expect(error).toContain("cost-guard"); + expect(error).toContain("'target'"); + expect(error).toContain("$5.00"); + }); + + it("(b) target is not limited → normal dispatch", async () => { + const cg = mockCostGuard({ limited: false, limitUsd: 5 }); + const { ctx, target } = makeCtx(cg); + const handler = outboundHandlers.get("send_to_instance")!; + const respond = vi.fn(); + + await handler(ctx, { instance_name: "target", message: "hi" }, respond, meta); + + expect(cg.isLimited).toHaveBeenCalledWith("target"); + expect(target.messages).toHaveLength(1); + expect(respond).toHaveBeenCalledWith(expect.objectContaining({ sent: true, target: "target" })); + }); + + it("(c) ctx.costGuard is null → graceful dispatch, no error, no isLimited call", async () => { + const { ctx, target } = makeCtx(null); + const handler = outboundHandlers.get("send_to_instance")!; + const respond = vi.fn(); + + await handler(ctx, { instance_name: "target", message: "hi" }, respond, meta); + + expect(target.messages).toHaveLength(1); + expect(respond).toHaveBeenCalledWith(expect.objectContaining({ sent: true, target: "target" })); + }); + + it("report-kind messages bypass the cost-guard gate (terminal status)", async () => { + // Reports must reach the orchestrator even when target is limited; + // otherwise the merge gate stalls and impl can't escalate. + const cg = mockCostGuard({ limited: true, limitUsd: 5 }); + const { ctx, target } = makeCtx(cg); + const handler = outboundHandlers.get("send_to_instance")!; + const respond = vi.fn(); + + await handler(ctx, { + instance_name: "target", + message: "task done", + request_kind: "report", + }, respond, meta); + + expect(target.messages).toHaveLength(1); + expect(respond).toHaveBeenCalledWith(expect.objectContaining({ sent: true, target: "target" })); + }); + }); + + describe("delegate_task", () => { + it("(a) target is limited → immediate warning result, no dispatch", async () => { + const cg = mockCostGuard({ limited: true, limitUsd: 5 }); + const { ctx, target } = makeCtx(cg); + const handler = outboundHandlers.get("delegate_task")!; + const respond = vi.fn(); + + await handler(ctx, { target_instance: "target", task: "do thing" }, respond, meta); + + expect(cg.isLimited).toHaveBeenCalledWith("target"); + expect(target.messages).toHaveLength(0); + expect(respond).toHaveBeenCalledTimes(1); + const [result, error] = respond.mock.calls[0]; + expect(result).toBeNull(); + expect(error).toContain("cost-guard"); + expect(error).toContain("'target'"); + expect(error).toContain("$5.00"); + }); + + it("(b) target is not limited → normal dispatch", async () => { + const cg = mockCostGuard({ limited: false, limitUsd: 5 }); + const { ctx, target } = makeCtx(cg); + const handler = outboundHandlers.get("delegate_task")!; + const respond = vi.fn(); + + await handler(ctx, { target_instance: "target", task: "do thing" }, respond, meta); + + expect(cg.isLimited).toHaveBeenCalledWith("target"); + expect(target.messages).toHaveLength(1); + expect(respond).toHaveBeenCalledWith(expect.objectContaining({ sent: true, target: "target" })); + }); + + it("(c) ctx.costGuard is null → graceful dispatch, no error, no isLimited call", async () => { + const { ctx, target } = makeCtx(null); + const handler = outboundHandlers.get("delegate_task")!; + const respond = vi.fn(); + + await handler(ctx, { target_instance: "target", task: "do thing" }, respond, meta); + + expect(target.messages).toHaveLength(1); + expect(respond).toHaveBeenCalledWith(expect.objectContaining({ sent: true, target: "target" })); + }); + }); +});