Skip to content

Commit 5312683

Browse files
saltbosaltbo
committed
fix(daemon): send non-empty prompt on rate-limit resume
Empty string prompt caused Claude API 400 ("cache_control cannot be set for empty text blocks") on first resume after rate-limit recovery.
1 parent 07e98b3 commit 5312683

3 files changed

Lines changed: 266 additions & 4 deletions

File tree

‎packages/cli/src/daemon/loop.ts‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,8 @@ import type { RuntimePool } from "./runtimePool.js";
2727

2828
const logger = createLogger("loop");
2929

30+
const RATE_LIMIT_RESUME_PROMPT = "Rate limit window has reset. Continue working on the task where you left off.";
31+
3032
export interface LoopOpts {
3133
maxConcurrent: number;
3234
pollInterval: number;
@@ -76,7 +78,7 @@ export class DaemonLoop {
7678
if (s.runtime !== runtime) continue;
7779
if (!s.taskId || this.pool.hasTask(s.taskId)) continue;
7880
if (s.resumeAfter && s.resumeAfter > now) continue;
79-
await resumeOneSession(s, "", this.client, this.pool);
81+
await resumeOneSession(s, RATE_LIMIT_RESUME_PROMPT, this.client, this.pool);
8082
}
8183
this.schedulePoll(0);
8284
}
@@ -91,7 +93,7 @@ export class DaemonLoop {
9193
if (this.pool.activeCount >= this.opts.maxConcurrent) return;
9294
if (!s.taskId || this.pool.hasTask(s.taskId)) continue;
9395
if (!s.resumeAfter || s.resumeAfter > now) continue;
94-
await resumeOneSession(s, "", this.client, this.pool);
96+
await resumeOneSession(s, RATE_LIMIT_RESUME_PROMPT, this.client, this.pool);
9597
}
9698
}
9799

‎packages/cli/tests/scheduler.test.ts‎

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -231,7 +231,12 @@ describe("DaemonLoop.resumeRateLimitedSessions()", () => {
231231

232232
await new Promise((r) => setTimeout(r, 50));
233233

234-
expect(mockResumeOneSession).toHaveBeenCalledWith(expect.objectContaining({ sessionId: "sess-rl" }), "", expect.anything(), expect.anything());
234+
expect(mockResumeOneSession).toHaveBeenCalledWith(
235+
expect.objectContaining({ sessionId: "sess-rl" }),
236+
expect.stringContaining("Rate limit"),
237+
expect.anything(),
238+
expect.anything(),
239+
);
235240
loop.stop();
236241
rl.stop();
237242
});
@@ -448,7 +453,7 @@ describe("DaemonLoop tick — resumeBackoffSessions: expired backoff triggers re
448453

449454
expect(mockResumeOneSession).toHaveBeenCalledWith(
450455
expect.objectContaining({ sessionId: "sess-backoff-expired" }),
451-
"",
456+
expect.stringContaining("Rate limit"),
452457
expect.anything(),
453458
expect.anything(),
454459
);

‎tests/loop-rate-limit-prompt.test.ts‎

Lines changed: 255 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,255 @@
1+
// @vitest-environment node
2+
/**
3+
* Tests for the RATE_LIMIT_RESUME_PROMPT fix in loop.ts.
4+
*
5+
* Verifies that both resumeRateLimitedSessions and the private
6+
* resumeBackoffSessions pass a non-empty message string to resumeOneSession,
7+
* preventing the "cache_control cannot be set for empty text blocks" 400 error.
8+
*
9+
* RATE_LIMIT_RESUME_PROMPT is a module-private constant, so we verify it
10+
* indirectly by capturing the message argument received by resumeOneSession.
11+
*/
12+
13+
import { afterEach, beforeEach, describe, expect, it, vi } from "vitest";
14+
15+
// ---- Capture calls to resumeOneSession before the module is imported --------
16+
// vi.hoisted ensures the variable is available in the vi.mock factory (which
17+
// is hoisted to the top of the file by Vitest's transformer).
18+
19+
const { resumeOneSessionMock } = vi.hoisted(() => ({
20+
resumeOneSessionMock: vi.fn().mockResolvedValue(undefined),
21+
}));
22+
23+
vi.mock("../packages/cli/src/daemon/resumer.js", () => ({
24+
resumeOneSession: resumeOneSessionMock,
25+
}));
26+
27+
// ---- Suppress path-related fs side effects ----------------------------------
28+
29+
vi.mock("../packages/cli/src/paths.js", () => {
30+
const { join } = require("node:path");
31+
const { tmpdir } = require("node:os");
32+
const base = join(tmpdir(), `ak-test-loop-prompt-${process.pid}`);
33+
return {
34+
STATE_DIR: base,
35+
CONFIG_DIR: base,
36+
DATA_DIR: base,
37+
LOGS_DIR: join(base, "logs"),
38+
CONFIG_FILE: join(base, "config.json"),
39+
PID_FILE: join(base, "daemon.pid"),
40+
DAEMON_STATE_FILE: join(base, "daemon-state.json"),
41+
REPOS_DIR: join(base, "repos"),
42+
WORKTREES_DIR: join(base, "worktrees"),
43+
SESSIONS_DIR: join(base, "sessions"),
44+
TRACKED_TASKS_FILE: join(base, "tracked-tasks.json"),
45+
IDENTITIES_DIR: join(base, "identities"),
46+
LEGACY_SAVED_SESSIONS_FILE: join(base, "saved-sessions.json"),
47+
LEGACY_SESSION_PIDS_FILE: join(base, "session-pids.json"),
48+
};
49+
});
50+
51+
// ---- Suppress logger noise --------------------------------------------------
52+
53+
vi.mock("../packages/cli/src/logger.js", () => ({
54+
createLogger: () => ({
55+
info: vi.fn(),
56+
warn: vi.fn(),
57+
error: vi.fn(),
58+
debug: vi.fn(),
59+
fatal: vi.fn(),
60+
}),
61+
}));
62+
63+
// ---- Imports after mocks are set up -----------------------------------------
64+
65+
import { DaemonLoop } from "../packages/cli/src/daemon/loop.js";
66+
import type { SessionFile } from "../packages/cli/src/session/types.js";
67+
68+
// ---- Minimal fakes ----------------------------------------------------------
69+
70+
function makeRateLimitedSession(runtime: string, taskId = "task-1"): SessionFile {
71+
return {
72+
type: "worker",
73+
agentId: "agent-1",
74+
sessionId: "session-aaaaaaaa-1111-2222-3333-444444444444",
75+
runtime: runtime as any,
76+
startedAt: Date.now(),
77+
apiUrl: "http://localhost",
78+
privateKeyJwk: {} as any,
79+
taskId,
80+
status: "rate_limited",
81+
resumeAfter: undefined, // no future backoff — eligible for immediate resume
82+
};
83+
}
84+
85+
function makePool(overrides: Partial<{ activeCount: number; hasTask: (id: string) => boolean }> = {}) {
86+
return {
87+
activeCount: overrides.activeCount ?? 0,
88+
hasTask: overrides.hasTask ?? ((_id: string) => false),
89+
getActiveTaskIds: () => [],
90+
} as any;
91+
}
92+
93+
function makeRateLimiter() {
94+
return {} as any;
95+
}
96+
97+
function makePrMonitor() {
98+
return {} as any;
99+
}
100+
101+
function makeClient() {
102+
return {} as any;
103+
}
104+
105+
function makeLoop(sessions: SessionFile[], poolOverrides = {}) {
106+
const pool = makePool(poolOverrides);
107+
108+
// Wire a minimal session manager that returns our test sessions
109+
const sessionManager = {
110+
list: (filter: { type: string; status: string }) => {
111+
return sessions.filter((s) => s.type === filter.type && s.status === filter.status);
112+
},
113+
patch: vi.fn().mockResolvedValue(undefined),
114+
};
115+
116+
const loop = new DaemonLoop(makeClient(), pool, makeRateLimiter(), makePrMonitor(), {
117+
maxConcurrent: 4,
118+
pollInterval: 1000,
119+
});
120+
121+
// Replace the internal session manager with our fake
122+
(loop as any).sessions = sessionManager;
123+
124+
// Set running=true so resumeRateLimitedSessions does not short-circuit.
125+
// We avoid calling loop.start() to prevent scheduling a real setTimeout.
126+
(loop as any).running = true;
127+
128+
return { loop, pool };
129+
}
130+
131+
// ---- Tests ------------------------------------------------------------------
132+
133+
describe("DaemonLoop — RATE_LIMIT_RESUME_PROMPT is non-empty", () => {
134+
beforeEach(() => {
135+
resumeOneSessionMock.mockClear();
136+
});
137+
138+
afterEach(() => {
139+
vi.clearAllMocks();
140+
});
141+
142+
describe("resumeRateLimitedSessions", () => {
143+
it("calls resumeOneSession with a non-empty message when a rate-limited session is eligible", async () => {
144+
const session = makeRateLimitedSession("claude");
145+
const { loop } = makeLoop([session]);
146+
147+
await loop.resumeRateLimitedSessions("claude");
148+
149+
expect(resumeOneSessionMock).toHaveBeenCalledTimes(1);
150+
const messageArg: string = resumeOneSessionMock.mock.calls[0][1];
151+
expect(typeof messageArg).toBe("string");
152+
expect(messageArg.length).toBeGreaterThan(0);
153+
});
154+
155+
it("does not call resumeOneSession when the runtime does not match", async () => {
156+
const session = makeRateLimitedSession("other-runtime");
157+
const { loop } = makeLoop([session]);
158+
159+
await loop.resumeRateLimitedSessions("claude");
160+
161+
expect(resumeOneSessionMock).not.toHaveBeenCalled();
162+
});
163+
164+
it("does not call resumeOneSession when the pool is at max capacity", async () => {
165+
const session = makeRateLimitedSession("claude");
166+
const { loop } = makeLoop([session], { activeCount: 4 });
167+
168+
await loop.resumeRateLimitedSessions("claude");
169+
170+
expect(resumeOneSessionMock).not.toHaveBeenCalled();
171+
});
172+
173+
it("does not call resumeOneSession when the task is already in the pool", async () => {
174+
const session = makeRateLimitedSession("claude", "task-active");
175+
const { loop } = makeLoop([session], { hasTask: (id) => id === "task-active" });
176+
177+
await loop.resumeRateLimitedSessions("claude");
178+
179+
expect(resumeOneSessionMock).not.toHaveBeenCalled();
180+
});
181+
182+
it("does not call resumeOneSession when resumeAfter is in the future", async () => {
183+
const session = makeRateLimitedSession("claude");
184+
session.resumeAfter = Date.now() + 60_000;
185+
const { loop } = makeLoop([session]);
186+
187+
await loop.resumeRateLimitedSessions("claude");
188+
189+
expect(resumeOneSessionMock).not.toHaveBeenCalled();
190+
});
191+
});
192+
193+
describe("resumeBackoffSessions (via tick)", () => {
194+
it("calls resumeOneSession with a non-empty message when a session's backoff has expired", async () => {
195+
const session = makeRateLimitedSession("claude");
196+
// resumeAfter in the past → backoff expired → eligible
197+
session.resumeAfter = Date.now() - 1000;
198+
const { loop } = makeLoop([session]);
199+
200+
// resumeBackoffSessions is private; drive it through tick() via a
201+
// minimal tick invocation. We call the private method directly to
202+
// avoid wiring the full tick pipeline (dispatchTasks etc.)
203+
await (loop as any).resumeBackoffSessions();
204+
205+
expect(resumeOneSessionMock).toHaveBeenCalledTimes(1);
206+
const messageArg: string = resumeOneSessionMock.mock.calls[0][1];
207+
expect(typeof messageArg).toBe("string");
208+
expect(messageArg.length).toBeGreaterThan(0);
209+
});
210+
211+
it("does not call resumeOneSession when resumeAfter is absent (no backoff set)", async () => {
212+
const session = makeRateLimitedSession("claude");
213+
// resumeAfter is undefined → resumeBackoffSessions skips (no backoff to expire)
214+
session.resumeAfter = undefined;
215+
const { loop } = makeLoop([session]);
216+
217+
await (loop as any).resumeBackoffSessions();
218+
219+
expect(resumeOneSessionMock).not.toHaveBeenCalled();
220+
});
221+
222+
it("does not call resumeOneSession when the pool is at max capacity", async () => {
223+
const session = makeRateLimitedSession("claude");
224+
session.resumeAfter = Date.now() - 1000;
225+
const { loop } = makeLoop([session], { activeCount: 4 });
226+
227+
await (loop as any).resumeBackoffSessions();
228+
229+
expect(resumeOneSessionMock).not.toHaveBeenCalled();
230+
});
231+
});
232+
233+
describe("both callers pass the same non-empty prompt string", () => {
234+
it("resumeRateLimitedSessions and resumeBackoffSessions pass identical message text", async () => {
235+
// Call each function in isolation so call indices are unambiguous.
236+
const sessionA = makeRateLimitedSession("claude", "task-A");
237+
const { loop: loopA } = makeLoop([sessionA]);
238+
await loopA.resumeRateLimitedSessions("claude");
239+
const msgFromRateLimit: string = resumeOneSessionMock.mock.calls[0][1];
240+
241+
resumeOneSessionMock.mockClear();
242+
243+
const sessionB = makeRateLimitedSession("claude", "task-B");
244+
sessionB.resumeAfter = Date.now() - 1000;
245+
const { loop: loopB } = makeLoop([sessionB]);
246+
await (loopB as any).resumeBackoffSessions();
247+
const msgFromBackoff: string = resumeOneSessionMock.mock.calls[0][1];
248+
249+
// Both callers must use the same non-empty constant
250+
expect(typeof msgFromRateLimit).toBe("string");
251+
expect(msgFromRateLimit.length).toBeGreaterThan(0);
252+
expect(msgFromBackoff).toBe(msgFromRateLimit);
253+
});
254+
});
255+
});

0 commit comments

Comments
 (0)