Skip to content

Commit 801344a

Browse files
author
root
committed
Add configurable supervisor evaluation timeout
1 parent 1872e48 commit 801344a

14 files changed

Lines changed: 603 additions & 3 deletions

File tree

packages/core/src/domain/supervisor.ts

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -48,6 +48,21 @@ export interface SupervisorConfig {
4848
guidanceDedupeWindow: number;
4949
}
5050

51+
export const DEFAULT_SUPERVISOR_EVALUATION_TIMEOUT_SEC = 600;
52+
export const MAX_SUPERVISOR_EVALUATION_TIMEOUT_SEC = 86_400;
53+
54+
export function resolveSupervisorEvaluationTimeoutSec(value: unknown): number {
55+
if (typeof value !== "number" || !Number.isFinite(value) || !Number.isSafeInteger(value)) {
56+
return DEFAULT_SUPERVISOR_EVALUATION_TIMEOUT_SEC;
57+
}
58+
59+
if (value < 1 || value > MAX_SUPERVISOR_EVALUATION_TIMEOUT_SEC) {
60+
return DEFAULT_SUPERVISOR_EVALUATION_TIMEOUT_SEC;
61+
}
62+
63+
return value;
64+
}
65+
5166
export const DEFAULT_SUPERVISOR_CONFIG: SupervisorConfig = {
5267
maxCyclesPerSession: 100,
5368
terminalLinesForEvaluation: 500,

packages/core/src/domain/types.ts

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -138,6 +138,9 @@ export interface Settings {
138138
enabled: boolean;
139139
soundEnabled: boolean;
140140
};
141+
supervisor: {
142+
evaluationTimeoutSec: number;
143+
};
141144
appearance: {
142145
theme: "dark";
143146
terminalRenderer: "standard" | "compatibility";

packages/server/src/__tests__/supervisor-manager.test.ts

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -137,6 +137,9 @@ function createManagerDeps() {
137137
providerId === "codex" ? { additionalArgs: [], envVars: {} } : undefined
138138
),
139139
};
140+
const settingsRepo = {
141+
get: vi.fn(() => undefined),
142+
};
140143

141144
const supervisorRepo = {
142145
create: vi.fn((value: NewSupervisor) => {
@@ -222,6 +225,7 @@ function createManagerDeps() {
222225
}),
223226
],
224227
providerConfigRepo,
228+
settingsRepo,
225229
logger,
226230
supervisorRepo,
227231
cycleRepo,

packages/server/src/commands/settings.test.ts

Lines changed: 105 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,9 @@ describe("settings commands", () => {
4545
enabled: true,
4646
soundEnabled: false,
4747
},
48+
supervisor: {
49+
evaluationTimeoutSec: 600,
50+
},
4851
},
4952
},
5053
},
@@ -61,6 +64,63 @@ describe("settings commands", () => {
6164
expect(
6265
db.prepare("SELECT value FROM user_settings WHERE key = ?").get("notifications.soundEnabled")
6366
).toEqual({ value: "false" });
67+
expect(
68+
db
69+
.prepare("SELECT value FROM user_settings WHERE key = ?")
70+
.get("supervisor.evaluationTimeoutSec")
71+
).toEqual({ value: "600" });
72+
});
73+
74+
it("settings.update rejects fractional supervisor timeout values", async () => {
75+
const result = await dispatch(
76+
{
77+
kind: "command",
78+
id: "settings-update-supervisor-timeout-fractional",
79+
op: "settings.update",
80+
args: {
81+
settings: {
82+
supervisor: {
83+
evaluationTimeoutSec: 1.9,
84+
},
85+
},
86+
},
87+
},
88+
ctx
89+
);
90+
91+
expect(result.ok).toBe(false);
92+
expect(result.error?.code).toBe("validation_error");
93+
expect(
94+
db
95+
.prepare("SELECT value FROM user_settings WHERE key = ?")
96+
.get("supervisor.evaluationTimeoutSec")
97+
).toBeUndefined();
98+
});
99+
100+
it("settings.update rejects supervisor timeout values above the supported maximum", async () => {
101+
const result = await dispatch(
102+
{
103+
kind: "command",
104+
id: "settings-update-supervisor-timeout-too-large",
105+
op: "settings.update",
106+
args: {
107+
settings: {
108+
supervisor: {
109+
evaluationTimeoutSec: 86_401,
110+
},
111+
},
112+
},
113+
},
114+
ctx
115+
);
116+
117+
expect(result.ok).toBe(false);
118+
expect(result.error?.code).toBe("validation_error");
119+
expect(
120+
db
121+
.prepare("SELECT value FROM user_settings WHERE key = ?")
122+
.get("supervisor.evaluationTimeoutSec")
123+
).toBeUndefined();
64124
});
65125

66126
it("settings.update persists provider startup command arguments per provider config", async () => {
@@ -191,6 +251,10 @@ describe("settings commands", () => {
191251
"notifications.enabled",
192252
"true"
193253
);
254+
db.prepare("INSERT INTO user_settings (key, value) VALUES (?, ?)").run(
255+
"supervisor.evaluationTimeoutSec",
256+
"900"
257+
);
194258

195259
const result = await dispatch(
196260
{
@@ -206,6 +270,7 @@ describe("settings commands", () => {
206270
expect(result.data).toMatchObject({
207271
defaultProviderId: "codex",
208272
"notifications.enabled": true,
273+
"supervisor.evaluationTimeoutSec": 900,
209274
externalConfigAudit: {
210275
codex: {
211276
configPath: "/tmp/config.toml",
@@ -215,4 +280,44 @@ describe("settings commands", () => {
215280
},
216281
});
217282
});
283+
284+
it("settings.get normalizes invalid persisted supervisor timeout values", async () => {
285+
db.prepare("INSERT INTO user_settings (key, value) VALUES (?, ?)").run(
286+
"supervisor.evaluationTimeoutSec",
287+
"999999"
288+
);
289+
290+
const result = await dispatch(
291+
{
292+
kind: "command",
293+
id: "settings-get-supervisor-timeout-invalid",
294+
op: "settings.get",
295+
args: {},
296+
},
297+
ctx
298+
);
299+
300+
expect(result.ok).toBe(true);
301+
expect(result.data?.["supervisor.evaluationTimeoutSec"]).toBe(600);
302+
});
303+
304+
it("settings.get falls back when the persisted supervisor timeout is fractional", async () => {
305+
db.prepare("INSERT INTO user_settings (key, value) VALUES (?, ?)").run(
306+
"supervisor.evaluationTimeoutSec",
307+
"1.9"
308+
);
309+
310+
const result = await dispatch(
311+
{
312+
kind: "command",
313+
id: "settings-get-supervisor-timeout-fractional",
314+
op: "settings.get",
315+
args: {},
316+
},
317+
ctx
318+
);
319+
320+
expect(result.ok).toBe(true);
321+
expect(result.data?.["supervisor.evaluationTimeoutSec"]).toBe(600);
322+
});
218323
});

packages/server/src/commands/settings.ts

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,11 @@
22
* Settings Commands
33
*/
44

5+
import {
6+
DEFAULT_SUPERVISOR_EVALUATION_TIMEOUT_SEC,
7+
MAX_SUPERVISOR_EVALUATION_TIMEOUT_SEC,
8+
resolveSupervisorEvaluationTimeoutSec,
9+
} from "@coder-studio/core";
510
import { z } from "zod";
611
import { type ConfigType, readConfigFile, writeConfigFile } from "../config/config-io.js";
712
import {
@@ -12,6 +17,7 @@ import {
1217
sanitizeProviderLaunchConfig,
1318
} from "../provider-config.js";
1419
import { ProviderConfigRepo } from "../storage/repositories/provider-config-repo.js";
20+
import { SUPERVISOR_EVALUATION_TIMEOUT_SETTING_KEY } from "../supervisor/settings.js";
1521
import { registerCommand } from "../ws/dispatch.js";
1622

1723
const EMPTY_CODEX_AUDIT = {
@@ -35,6 +41,17 @@ const SettingsSchema = z.object({
3541
onlyWhenBackgrounded: z.boolean().optional(),
3642
})
3743
.optional(),
44+
supervisor: z
45+
.object({
46+
evaluationTimeoutSec: z
47+
.number()
48+
.int()
49+
.min(1)
50+
.max(MAX_SUPERVISOR_EVALUATION_TIMEOUT_SEC)
51+
.default(DEFAULT_SUPERVISOR_EVALUATION_TIMEOUT_SEC)
52+
.optional(),
53+
})
54+
.optional(),
3855
appearance: z
3956
.object({
4057
theme: z.enum(["dark"]).optional(),
@@ -88,6 +105,12 @@ registerCommand("settings.get", z.object({}), async (_args, ctx) => {
88105
settings.externalConfigAudit = null;
89106
}
90107

108+
if (Object.prototype.hasOwnProperty.call(settings, SUPERVISOR_EVALUATION_TIMEOUT_SETTING_KEY)) {
109+
settings[SUPERVISOR_EVALUATION_TIMEOUT_SETTING_KEY] = resolveSupervisorEvaluationTimeoutSec(
110+
settings[SUPERVISOR_EVALUATION_TIMEOUT_SETTING_KEY]
111+
);
112+
}
113+
91114
return settings;
92115
});
93116

packages/server/src/server.ts

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,7 @@ import { AuthLoginBlockRepo } from "./storage/repositories/auth-login-block-repo
3333
import { AuthSessionRepo } from "./storage/repositories/auth-session-repo.js";
3434
import { ProviderConfigRepo } from "./storage/repositories/provider-config-repo.js";
3535
import { rowToSession, type SessionRow } from "./storage/repositories/session-repo.js";
36+
import { SettingsRepo } from "./storage/repositories/settings-repo.js";
3637
import { SupervisorCycleRepo } from "./storage/repositories/supervisor-cycle-repo.js";
3738
import { SupervisorRepo } from "./storage/repositories/supervisor-repo.js";
3839
import { SupervisorManager } from "./supervisor/manager.js";
@@ -119,6 +120,7 @@ export async function createServer(
119120

120121
const sessionDb = createSessionDatabase(db);
121122
const providerConfigRepo = new ProviderConfigRepo(db);
123+
const settingsRepo = new SettingsRepo(db);
122124
const sessionMgr = new SessionManager({
123125
terminalMgr,
124126
eventBus,
@@ -183,6 +185,7 @@ export async function createServer(
183185
sessionMgr,
184186
providerRegistry,
185187
providerConfigRepo,
188+
settingsRepo,
186189
supervisorRepo,
187190
cycleRepo,
188191
logger: app.log,

packages/server/src/supervisor/evaluator.test.ts

Lines changed: 92 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,19 @@
11
import { existsSync, mkdtempSync, readFileSync, rmSync } from "node:fs";
22
import { tmpdir } from "node:os";
33
import path from "node:path";
4-
import type { ProviderDefinition, Supervisor } from "@coder-studio/core";
4+
import {
5+
DEFAULT_SUPERVISOR_EVALUATION_TIMEOUT_SEC,
6+
type ProviderDefinition,
7+
type Supervisor,
8+
} from "@coder-studio/core";
59
import type { FastifyBaseLogger } from "fastify";
610
import { beforeEach, describe, expect, it, vi } from "vitest";
11+
import { closeDatabase, openDatabase } from "../storage/db.js";
712
import type { ProviderConfigRepo } from "../storage/repositories/provider-config-repo.js";
13+
import { SettingsRepo } from "../storage/repositories/settings-repo.js";
814
import type { SupervisorEvaluationContext } from "./context-builder.js";
915
import { SupervisorEvaluator } from "./evaluator.js";
16+
import { getSupervisorEvaluationTimeoutMs } from "./settings.js";
1017

1118
function nodeEchoCommand(stdout: string) {
1219
return {
@@ -190,6 +197,90 @@ describe("SupervisorEvaluator", () => {
190197
expect(result.message).toBe("proceed with review");
191198
});
192199

200+
it("uses the shared 600-second default timeout when the setting is missing", () => {
201+
expect(getSupervisorEvaluationTimeoutMs()).toBe(
202+
DEFAULT_SUPERVISOR_EVALUATION_TIMEOUT_SEC * 1000
203+
);
204+
expect(
205+
getSupervisorEvaluationTimeoutMs({
206+
get: vi.fn(() => undefined),
207+
} as never)
208+
).toBe(DEFAULT_SUPERVISOR_EVALUATION_TIMEOUT_SEC * 1000);
209+
});
210+
211+
it("uses the stored supervisor timeout setting when available", () => {
212+
expect(
213+
getSupervisorEvaluationTimeoutMs({
214+
get: vi.fn(() => 900),
215+
} as never)
216+
).toBe(900_000);
217+
});
218+
219+
it("falls back to the default timeout when the stored setting exceeds the supported maximum", () => {
220+
expect(
221+
getSupervisorEvaluationTimeoutMs({
222+
get: vi.fn(() => 999_999_999),
223+
} as never)
224+
).toBe(DEFAULT_SUPERVISOR_EVALUATION_TIMEOUT_SEC * 1000);
225+
});
226+
227+
it("falls back to the default timeout when the stored setting is fractional", () => {
228+
expect(
229+
getSupervisorEvaluationTimeoutMs({
230+
get: vi.fn(() => 1.9),
231+
} as never)
232+
).toBe(DEFAULT_SUPERVISOR_EVALUATION_TIMEOUT_SEC * 1000);
233+
});
234+
235+
it("falls back to the default timeout when reading the stored setting throws", () => {
236+
expect(
237+
getSupervisorEvaluationTimeoutMs({
238+
get: vi.fn(() => {
239+
throw new SyntaxError("Unexpected token");
240+
}),
241+
} as never)
242+
).toBe(DEFAULT_SUPERVISOR_EVALUATION_TIMEOUT_SEC * 1000);
243+
});
244+
245+
it("reads the evaluator timeout from settingsRepo when timeoutMs is not provided", async () => {
246+
const settingsRepo = {
247+
get: vi.fn(() => 900),
248+
};
249+
const evaluator = new SupervisorEvaluator({
250+
providerRegistry: [createProvider("codex", "next step: run tests")],
251+
providerConfigRepo: createProviderConfigRepo(),
252+
settingsRepo: settingsRepo as never,
253+
});
254+
255+
const result = await evaluator.evaluate(makeSupervisor("codex"), makeContext());
256+
257+
expect(result.message).toBe("next step: run tests");
258+
expect(settingsRepo.get).toHaveBeenCalledWith("supervisor.evaluationTimeoutSec");
259+
});
260+
261+
it("falls back to the default timeout when the stored row is malformed JSON", async () => {
262+
const db = openDatabase(":memory:");
263+
264+
try {
265+
db.prepare("INSERT INTO user_settings (key, value) VALUES (?, ?)").run(
266+
"supervisor.evaluationTimeoutSec",
267+
"not-json"
268+
);
269+
270+
const evaluator = new SupervisorEvaluator({
271+
providerRegistry: [createProvider("codex", "next step: run tests")],
272+
providerConfigRepo: createProviderConfigRepo(),
273+
settingsRepo: new SettingsRepo(db),
274+
});
275+
276+
const result = await evaluator.evaluate(makeSupervisor("codex"), makeContext());
277+
278+
expect(result.message).toBe("next step: run tests");
279+
} finally {
280+
closeDatabase(db);
281+
}
282+
});
283+
193284
it("builds a natural language prompt matching the develop supervisor pattern", async () => {
194285
const logger = createLogger();
195286
const evaluator = new SupervisorEvaluator({

0 commit comments

Comments
 (0)