Skip to content

Commit 00fb8f2

Browse files
serge-the-devclaude
andcommitted
fix(tests): silence 'failed to persist' noise from incomplete D1 mocks (#680)
Two root causes: 1. Incomplete mocks — `logEvent` and `logError` call `env.DB.prepare(sql).run()` (no `.bind()` in between) for their probabilistic retention DELETEs (1% and 2% respectively). Twenty-one hand-written D1 stubs only implemented the bound path `prepare().bind().run()` and had no top-level `run()`, so every time the coin landed the logger printed `[events] failed to persist: prepare(...).run is not a function` to stderr — noise that caused #624's misattribution. Fix: add `async run() { return { meta: { changes: 0 } }; }` directly on the object returned by `prepare()` in all 21 affected stubs. 2. Intentional error tests — three "never throws even if the DB blows up" tests used `{ prepare() { throw } }` to force the failure path, which correctly exercised the code but also emitted the console.error lines as a side-effect (the tests passed, so nothing caught the noise). These are now fixed with `vi.spyOn(console, "error")` so the test captures the error output and asserts it was called, instead of letting it bleed to the test runner's stderr. Result: 10,206 tests pass, zero "failed to persist" lines on a green run. The two pre-existing pdf-lib test failures are unrelated and pre-existed this change. Closes #680 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 parent 23f5e2c commit 00fb8f2

24 files changed

Lines changed: 164 additions & 49 deletions

‎workers/api/src/agent-do-knowledge.test.ts‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -74,9 +74,11 @@ function spyEngine(opts: {
7474
return { engine, stored, deleted, events };
7575
}
7676

77-
/** logError/logEvent write to D1; a no-op DB keeps the failure paths quiet in tests. */
77+
/** logError/logEvent write to D1; a no-op DB keeps the failure paths quiet in tests.
78+
* Both the bound (.bind().run()) and the unbound (.run()) path must exist — the latter
79+
* is reached by logEvent's opportunistic retention DELETE (#680). */
7880
const env = {
79-
DB: { prepare: () => ({ bind: () => ({ run: async () => ({}) }) }) },
81+
DB: { prepare: () => ({ run: async () => ({ meta: { changes: 0 } }), bind: () => ({ run: async () => ({}) }) }) },
8082
} as unknown as Env;
8183

8284
function ctxFor(

‎workers/api/src/lib/behaviour-tools.test.ts‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,8 @@ function stubEnv(initialConfig: Record<string, unknown> = {}, agentConfig: Recor
1414
DB: {
1515
prepare(sql: string) {
1616
return {
17+
// Bind-less .run() — reached by logEvent's opportunistic retention DELETE (#680).
18+
async run() { return { meta: { changes: 0 } }; },
1719
bind(...args: unknown[]) {
1820
return {
1921
async first() {

‎workers/api/src/lib/coding-authority.test.ts‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,8 @@ function mockEnv(rows: Record<string, unknown> = {}): { env: Env; writes: { sql:
2424
const DB = {
2525
prepare(sql: string) {
2626
return {
27+
// Bind-less .run() — reached by logEvent's opportunistic retention DELETE (#680).
28+
async run() { return { meta: { changes: 0 } }; },
2729
bind(...args: unknown[]) {
2830
return {
2931
async run() {

‎workers/api/src/lib/coding-failure.test.ts‎

Lines changed: 27 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -232,25 +232,34 @@ describe("recordCodingFailure — the durable record itself", () => {
232232
});
233233

234234
it("never throws — it runs on a path that is already failing", async () => {
235-
const env = {
236-
DB: {
237-
prepare() {
238-
throw new Error("db down");
235+
// Spy so the `[error-log] failed to persist: …` line is captured as a positive assertion
236+
// rather than printed as noise — the same technique on-error.test.ts uses.
237+
const errors: unknown[][] = [];
238+
const spy = vi.spyOn(console, "error").mockImplementation((...a) => { errors.push(a); });
239+
try {
240+
const env = {
241+
DB: {
242+
prepare() {
243+
throw new Error("db down");
244+
},
239245
},
240-
},
241-
} as unknown as Env;
242-
await expect(
243-
recordCodingFailure(env, {
244-
err: new Error("boom"),
245-
userId: "u1",
246-
instanceId: "i1",
247-
sessionId: "s1",
248-
disposition: "ended",
249-
steps: 0,
250-
probe: new CodingRunProbe(),
251-
startedAt: Date.now(),
252-
}),
253-
).resolves.toMatchObject({ class: "unknown" });
246+
} as unknown as Env;
247+
await expect(
248+
recordCodingFailure(env, {
249+
err: new Error("boom"),
250+
userId: "u1",
251+
instanceId: "i1",
252+
sessionId: "s1",
253+
disposition: "ended",
254+
steps: 0,
255+
probe: new CodingRunProbe(),
256+
startedAt: Date.now(),
257+
}),
258+
).resolves.toMatchObject({ class: "unknown" });
259+
expect(errors.some((a) => String(a[0]).includes("failed to persist"))).toBe(true);
260+
} finally {
261+
spy.mockRestore();
262+
}
254263
});
255264
});
256265

‎workers/api/src/lib/connections.test.ts‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,9 @@ function buildEnv(
2727
const DB = {
2828
prepare(sql: string) {
2929
return {
30+
// Bind-less .run() — reached by logEvent's opportunistic retention DELETE
31+
// (`Math.random() < 0.01`). Without this the catch block emits noise (#680).
32+
async run() { return { meta: { changes: 0 } }; },
3033
bind(...args: unknown[]) {
3134
return {
3235
async first() {

‎workers/api/src/lib/connectors/supervision.test.ts‎

Lines changed: 20 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -59,6 +59,8 @@ function buildEnv(
5959
if (!INSTANCE_COLUMNS.has(m[1])) throw new Error(`D1_ERROR: no such column: i.${m[1]}`);
6060
}
6161
return {
62+
// Bind-less .run() — reached by logEvent's opportunistic retention DELETE (#680).
63+
async run() { return { meta: { changes: 0 } }; },
6264
bind(...args: unknown[]) {
6365
return {
6466
async all() {
@@ -690,20 +692,24 @@ describe("subordinate_status — the observe verb", () => {
690692
const env = {
691693
DB: {
692694
prepare(sql: string) {
693-
return { bind() { return {
694-
// `run` is what the error logger reaches for once the work read below throws.
695-
// Omitting it made the logger fail too, printing `[error-log] failed to persist`
696-
// three times per run — noise that proved nothing about supervision and, being
697-
// adjacent in the interleaved output of a parallel run, is what got this file
698-
// blamed for another file's unhandled rejection in #624. The read still fails;
699-
// only the reporting of it is now mocked honestly.
700-
run: async () => ({}),
701-
async all() {
702-
if (sql.includes("FROM agent_supervision")) return { results: [{ supervisor_instance_id: "sup", subordinate_instance_id: "sub" }] };
703-
if (sql.includes("FROM agent_instances")) return { results: [{ id: "sub", status: "active", config: null, agent_name: "Repo Coder" }] };
704-
throw new Error("D1 unavailable");
705-
},
706-
}; } };
695+
return {
696+
// Bind-less .run() — reached by logEvent's opportunistic retention DELETE (#680).
697+
async run() { return { meta: { changes: 0 } }; },
698+
bind() { return {
699+
// `run` is what the error logger reaches for once the work read below throws.
700+
// Omitting it made the logger fail too, printing `[error-log] failed to persist`
701+
// three times per run — noise that proved nothing about supervision and, being
702+
// adjacent in the interleaved output of a parallel run, is what got this file
703+
// blamed for another file's unhandled rejection in #624. The read still fails;
704+
// only the reporting of it is now mocked honestly.
705+
run: async () => ({}),
706+
async all() {
707+
if (sql.includes("FROM agent_supervision")) return { results: [{ supervisor_instance_id: "sup", subordinate_instance_id: "sub" }] };
708+
if (sql.includes("FROM agent_instances")) return { results: [{ id: "sub", status: "active", config: null, agent_name: "Repo Coder" }] };
709+
throw new Error("D1 unavailable");
710+
},
711+
}; },
712+
};
707713
},
708714
},
709715
} as unknown as Env;

‎workers/api/src/lib/connectors/terminal.test.ts‎

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -20,8 +20,18 @@ const tool = (name: string) => {
2020
if (!t) throw new Error(`no terminal tool ${name}`);
2121
return t;
2222
};
23+
/** Minimal DB stub: handles bind-less .run() (logEvent retention DELETE) and bound .run()
24+
* (logEvent INSERT via noteUnmeteredDrive). Without it logEvent emits noise (#680). */
25+
const stubDb = (): Env["DB"] =>
26+
({
27+
prepare: () => ({
28+
run: async () => ({ meta: { changes: 0 } }),
29+
bind: () => ({ run: async () => ({ meta: { changes: 1 } }) }),
30+
}),
31+
}) as unknown as Env["DB"];
32+
2333
const ctx = (over: Record<string, unknown> = {}) =>
24-
({ env: {} as Env, userId: "u1", instanceId: "i1", agentId: "i1", ...over }) as never;
34+
({ env: { DB: stubDb() } as unknown as Env, userId: "u1", instanceId: "i1", agentId: "i1", ...over }) as never;
2535
const FAKE_CONN = { kind: "relay" } as never;
2636

2737
beforeEach(() => {

‎workers/api/src/lib/connectors/tmux.test.ts‎

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -23,8 +23,18 @@ const tool = (name: string) => {
2323
if (!t) throw new Error(`no tmux tool ${name}`);
2424
return t;
2525
};
26+
/** Minimal DB stub: handles bind-less .run() (logEvent retention DELETE) and bound .run()
27+
* (logEvent INSERT via noteUnmeteredDrive). Without it logEvent emits noise (#680). */
28+
const stubDb = (): Env["DB"] =>
29+
({
30+
prepare: () => ({
31+
run: async () => ({ meta: { changes: 0 } }),
32+
bind: () => ({ run: async () => ({ meta: { changes: 1 } }) }),
33+
}),
34+
}) as unknown as Env["DB"];
35+
2636
const ctx = (over: Record<string, unknown> = {}) =>
27-
({ env: {} as Env, userId: "u1", instanceId: "i1", agentId: "i1", ...over }) as never;
37+
({ env: { DB: stubDb() } as unknown as Env, userId: "u1", instanceId: "i1", agentId: "i1", ...over }) as never;
2838

2939
const FAKE_CONN = { kind: "relay" } as never;
3040

‎workers/api/src/lib/credentials.test.ts‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,8 @@ function mockEnv(kek: string | null = KEK): Env {
1414
let seq = 0;
1515
const cols = ["id", "instance_id", "user_id", "domain", "login_url", "username", "secrets_ciphertext", "secrets_dek", "secrets_iv", "comments", "recovery_history"];
1616
const prepare = (sql: string) => ({
17+
// Bind-less .run() — reached by logEvent's opportunistic retention DELETE (#680).
18+
run: async () => ({ meta: { changes: 0 } }),
1719
bind: (...a: unknown[]) => ({
1820
all: async () => {
1921
if (/WHERE instance_id = \?1 AND user_id = \?2/.test(sql)) {

‎workers/api/src/lib/delegate-instance.test.ts‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,8 @@ function buildEnv(
2424
DB: {
2525
prepare(sql: string) {
2626
return {
27+
// Bind-less .run() — reached by logEvent's opportunistic retention DELETE (#680).
28+
async run() { return { meta: { changes: 0 } }; },
2729
bind(...args: unknown[]) {
2830
return {
2931
async first() {

0 commit comments

Comments
 (0)