Skip to content

Commit 1155f2a

Browse files
committed
refactor(hub): name the steps an approval opened from the start gates
1 parent cc8d07a commit 1155f2a

4 files changed

Lines changed: 124 additions & 137 deletions

File tree

‎apps/hub/src/projects/deploy-tools.test.ts‎

Lines changed: 13 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -219,7 +219,6 @@ function writerTools(preferences: Record<string, string> = {}) {
219219
const documents: Record<string, unknown>[] = [
220220
{ kind: "problem_brief", artifactId: "brief", title: "Brief", version: 2 },
221221
{ kind: "constraints", artifactId: "c", title: "C", version: 1 },
222-
{ kind: "material", artifactId: "m", title: "M", version: 1 },
223222
].map((item) => ({ ...item, sha256: "a".repeat(64) }));
224223
const deployed: string[] = [];
225224
const approvals: unknown[] = [];
@@ -298,22 +297,21 @@ const writer = (kind: string) => ({
298297
"Write the " + kind + " from the filed documents and the person's answers.",
299298
});
300299

301-
test("an approval names the step it unblocks once every earlier document is approved", async () => {
300+
test("an approval reports the steps whose gates it opened, unless one is under way", async () => {
302301
const { run, documents, inFlight } = writerTools();
303-
const unblocked = async (kinds: string[]) => {
302+
const canStart = async (kinds: string[]) => {
304303
const result = await run("request_approval", { kinds });
305-
return [JSON.parse(String(result.content)).unblocked, result.detail];
304+
return [JSON.parse(String(result.content)).canStart, result.detail];
306305
};
307306

308-
expect(await unblocked(["problem_brief"])).toEqual([undefined, undefined]);
307+
expect(await canStart(["problem_brief"])).toEqual([[], undefined]);
309308
inFlight.push({ runId: "w", role: "writer", status: "running" });
310-
expect(await unblocked(["constraints"])).toEqual([undefined, undefined]);
309+
expect(await canStart(["constraints"])).toEqual([[], undefined]);
311310
inFlight.pop();
312-
expect(await unblocked(["constraints"])).toEqual([
313-
"approaches",
314-
{ unblocked: "approaches" },
311+
expect(await canStart(["constraints"])).toEqual([
312+
["approaches"],
313+
{ canStart: ["approaches"] },
315314
]);
316-
expect(await unblocked(["material"])).toEqual([undefined, undefined]);
317315

318316
for (const kind of ["approaches", "product_requirements"])
319317
documents.push({
@@ -323,21 +321,12 @@ test("an approval names the step it unblocks once every earlier document is appr
323321
version: 1,
324322
sha256: "a".repeat(64),
325323
});
326-
expect(await unblocked(["approaches", "product_requirements"])).toEqual([
327-
"design",
328-
{ unblocked: "design" },
329-
]);
330-
documents.push({
331-
kind: "design_option",
332-
artifactId: "o",
333-
title: "Calm",
334-
version: 1,
335-
sha256: "a".repeat(64),
336-
});
337-
expect(await unblocked(["product_requirements"])).toEqual([
338-
undefined,
339-
undefined,
324+
expect(await canStart(["approaches", "product_requirements"])).toEqual([
325+
["design"],
326+
{ canStart: ["design"] },
340327
]);
328+
inFlight.push({ runId: "d", role: "designer", status: "running" });
329+
expect(await canStart(["product_requirements"])).toEqual([[], undefined]);
341330
});
342331

343332
test("one ask approves several documents, and a writer starts once every earlier one is approved", async () => {

‎apps/hub/src/projects/deploy-tools.ts‎

Lines changed: 91 additions & 97 deletions
Original file line numberDiff line numberDiff line change
@@ -158,35 +158,6 @@ const ArtifactList = type({
158158
});
159159
type Ref = typeof SourceRef.infer;
160160

161-
// A coordinator on 2026-11-05 often read a bare approval as still pending and started nothing, so
162-
// an approval of a document names the step it unblocks: the document after the last filed one, once
163-
// every filed document before it is approved and no writer is at work, or the design while none of
164-
// it is under way. A writer's run ends with its one task, so one in flight is at work.
165-
function unblockedStep(
166-
documents: readonly { kind: string }[],
167-
byKind: ReadonlyMap<string, Ref>,
168-
approved: ReadonlyMap<string, number>,
169-
inFlight: readonly { role: string }[],
170-
designOptions: boolean,
171-
): string | undefined {
172-
const approving = (kind: string) =>
173-
documents.some((doc) => doc.kind === kind);
174-
if (!DOCUMENT_ORDER.some(approving)) return undefined;
175-
const index = DOCUMENT_ORDER.findLastIndex((kind) => byKind.has(kind)) + 1;
176-
const next = DOCUMENT_ORDER[index];
177-
const atWork = (role: string) => inFlight.some((run) => run.role === role);
178-
if (next === undefined || atWork("writer")) return undefined;
179-
if (next === "design" && (designOptions || atWork("designer")))
180-
return undefined;
181-
const earlierApproved = DOCUMENT_ORDER.slice(0, index).every(
182-
(kind) =>
183-
!byKind.has(kind) ||
184-
approving(kind) ||
185-
approved.get(kind) === byKind.get(kind)?.version,
186-
);
187-
return earlierApproved ? next : undefined;
188-
}
189-
190161
interface DeployEnv extends BaseEnv {
191162
capabilities: RuntimeCapabilities;
192163
}
@@ -409,13 +380,9 @@ export function createDeployTools(options?: {
409380
const materials: ArtifactRef[] = [];
410381
const approved = new Map<string, number>();
411382
const openQuestions = new Map<string, number>();
412-
let designOptions = false;
413383
for (const item of list.artifacts) {
414384
const { kind, artifactId, version, sha256, approvedVersion } = item;
415-
if (kind === "design_option") {
416-
designOptions = true;
417-
continue;
418-
}
385+
if (kind === "design_option") continue;
419386
if (kind === MATERIAL_KIND)
420387
materials.push({ kind, artifactId, version, sha256 });
421388
byKind.set(kind, { artifactId, version, sha256 });
@@ -429,7 +396,6 @@ export function createDeployTools(options?: {
429396
materials,
430397
approved,
431398
openQuestions,
432-
designOptions,
433399
inFlight: list.inFlight,
434400
preferences: list.preferences,
435401
};
@@ -476,6 +442,81 @@ export function createDeployTools(options?: {
476442
);
477443
return ref;
478444
};
445+
type Listed = Awaited<ReturnType<typeof listed>>;
446+
/*
447+
* Each start tool refuses through its gate, and an approval reports the steps whose gates
448+
* then pass. The coordinator learns what it may start from the rule that enforces it, so
449+
* loosening a gate loosens what it is told.
450+
*/
451+
const writerGate = (
452+
{ byKind, approved, preferences }: Listed,
453+
kind: (typeof DOCUMENT_ORDER)[number],
454+
) => {
455+
const earlier = DOCUMENT_ORDER.slice(0, DOCUMENT_ORDER.indexOf(kind));
456+
// An imported project may lack an earlier document that its later ones were written without.
457+
const writtenWithout = (index: number) =>
458+
DOCUMENT_ORDER.slice(index + 1).some((later) => byKind.has(later));
459+
for (const [index, before] of earlier.entries()) {
460+
const ref = byKind.get(before);
461+
if (!ref) {
462+
if (writtenWithout(index)) continue;
463+
throw new Refusal(
464+
before +
465+
"_missing: " +
466+
(before === "design"
467+
? "have the designer make it with design_app"
468+
: "write and file the " + before) +
469+
" before the " +
470+
kind,
471+
);
472+
}
473+
if (
474+
!agentDecidesApprovals(preferences) &&
475+
approved.get(before) !== ref.version
476+
)
477+
throw notApproved(before, ref.version);
478+
}
479+
};
480+
const designGate = ({ byKind }: Listed) => {
481+
for (const kind of DESIGNER_INPUTS) required(byKind, kind);
482+
};
483+
const coderGate = ({ byKind, approved, preferences }: Listed) => {
484+
for (const kind of CODER_DOCUMENTS) required(byKind, kind);
485+
// The design is approved by choosing it, whoever decides the other documents.
486+
for (const kind of DOCUMENT_ORDER) {
487+
const { version } = required(byKind, kind);
488+
if (
489+
(kind === "design" || !agentDecidesApprovals(preferences)) &&
490+
approved.get(kind) !== version
491+
)
492+
throw notApproved(kind, version);
493+
}
494+
};
495+
// A step already made or under way is not offered again, whatever its gate says.
496+
const startable = (state: Listed) => {
497+
const busy = new Set(state.inFlight.map((run) => run.role));
498+
const open = (step: string, role: string, gate: () => void) => {
499+
if (state.byKind.has(step) || busy.has(role)) return false;
500+
try {
501+
gate();
502+
return true;
503+
} catch (error) {
504+
if (error instanceof Refusal) return false;
505+
throw error;
506+
}
507+
};
508+
return [
509+
...DOCUMENT_ORDER.filter(
510+
(kind) =>
511+
kind in WRITER_PACKAGES &&
512+
open(kind, "writer", () => writerGate(state, kind)),
513+
),
514+
...(open("design", "designer", () => designGate(state))
515+
? ["design"]
516+
: []),
517+
...(open("app", "coder", () => coderGate(state)) ? ["app"] : []),
518+
];
519+
};
479520
// A fresh deployment warms its workspace with the current versions, the ones named here.
480521
const inputs = (
481522
kinds: readonly InputKind[],
@@ -538,18 +579,11 @@ export function createDeployTools(options?: {
538579
try {
539580
signal.throwIfAborted();
540581
let result: unknown;
541-
let unblocked: string | undefined;
582+
let canStart: readonly string[] = [];
542583
if (call.name === "request_approval") {
543584
const { kinds } = validArguments(RequestApproval, call.arguments);
544-
const {
545-
byKind,
546-
materials,
547-
approved,
548-
openQuestions,
549-
designOptions,
550-
preferences,
551-
inFlight,
552-
} = await listed();
585+
const { byKind, materials, openQuestions, preferences } =
586+
await listed();
553587
const open = [...new Set(kinds)].flatMap((kind) => {
554588
const count = openQuestions.get(kind);
555589
return count
@@ -572,45 +606,14 @@ export function createDeployTools(options?: {
572606
return materials;
573607
});
574608
await recordApproval("request_approval", documents);
575-
unblocked = unblockedStep(
576-
documents,
577-
byKind,
578-
approved,
579-
inFlight,
580-
designOptions,
581-
);
582-
result = { approved: documents, unblocked };
609+
canStart = startable(await listed());
610+
result = { approved: documents, canStart };
583611
} else if (call.name === "deploy_writer") {
584612
const input = validArguments(DeployWriter, call.arguments);
585-
const { byKind, approved, preferences } = await listed();
613+
const state = await listed();
614+
const { byKind, preferences } = state;
586615
const briefRef = required(byKind, "problem_brief");
587-
const earlier = DOCUMENT_ORDER.slice(
588-
0,
589-
DOCUMENT_ORDER.indexOf(input.kind),
590-
);
591-
// An imported project may lack an earlier document that its later ones were written without.
592-
const writtenWithout = (index: number) =>
593-
DOCUMENT_ORDER.slice(index + 1).some((kind) => byKind.has(kind));
594-
for (const [index, kind] of earlier.entries()) {
595-
const ref = byKind.get(kind);
596-
if (!ref) {
597-
if (writtenWithout(index)) continue;
598-
throw new Refusal(
599-
kind +
600-
"_missing: " +
601-
(kind === "design"
602-
? "have the designer make it with design_app"
603-
: "write and file the " + kind) +
604-
" before the " +
605-
input.kind,
606-
);
607-
}
608-
if (
609-
!agentDecidesApprovals(preferences) &&
610-
approved.get(kind) !== ref.version
611-
)
612-
throw notApproved(kind, ref.version);
613-
}
616+
writerGate(state, input.kind);
614617
const target = byKind.get(input.kind);
615618
const sourceRefs = [...byKind]
616619
.filter(
@@ -672,26 +675,16 @@ export function createDeployTools(options?: {
672675
options.projectId + ":coder",
673676
"coder",
674677
async () => {
675-
const { byKind, approved, inFlight, preferences } =
676-
await listed();
677-
for (const kind of CODER_DOCUMENTS) required(byKind, kind);
678+
const state = await listed();
679+
const { byKind, inFlight, preferences } = state;
678680
const live = inFlight.find((run) => run.role === "coder");
679681
if (live)
680682
throw new Refusal(
681683
"coder_already_running: the coder is building in run " +
682684
live.runId +
683685
"; send it changes with instruct_coder",
684686
);
685-
// The design is approved by choosing it, whoever decides the other documents.
686-
for (const kind of DOCUMENT_ORDER) {
687-
const { version } = required(byKind, kind);
688-
if (
689-
(kind === "design" ||
690-
!agentDecidesApprovals(preferences)) &&
691-
approved.get(kind) !== version
692-
)
693-
throw notApproved(kind, version);
694-
}
687+
coderGate(state);
695688
const design = required(byKind, "design");
696689
return deploy(
697690
"coder",
@@ -745,8 +738,9 @@ export function createDeployTools(options?: {
745738
options.projectId + ":designer:" + concept.label,
746739
"designer",
747740
async () => {
748-
const { byKind, inFlight, preferences } = await listed();
749-
for (const kind of DESIGNER_INPUTS) required(byKind, kind);
741+
const state = await listed();
742+
const { byKind, inFlight, preferences } = state;
743+
designGate(state);
750744
const message =
751745
designLookIdeas(input.lookIdeas) +
752746
coderTaskMessage(input.instruction);
@@ -884,7 +878,7 @@ export function createDeployTools(options?: {
884878
return {
885879
callId: call.id,
886880
content: JSON.stringify(result),
887-
...(unblocked ? { detail: { unblocked } } : {}),
881+
...(canStart.length ? { detail: { canStart } } : {}),
888882
};
889883
} catch (error) {
890884
return {

‎apps/hub/src/workflows/director.test.ts‎

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -297,7 +297,7 @@ test("designer director holds replies until the latest edits pass check_design a
297297
expect(accepted.some((action) => action.type === "reply")).toBe(true);
298298
});
299299

300-
test("coordinator director reminds once after an approval that unblocked a step, unless the reply asks the person", async () => {
300+
test("coordinator director reminds once after an approval that opened a step, unless the reply asks the person", async () => {
301301
const subject = () =>
302302
compactingDirector("@corbits-builder/coordinator-test").factory(
303303
{ contextWindow: 200_000, compactAt: 0.8, startAfterApproval: true },
@@ -344,7 +344,7 @@ test("coordinator director reminds once after an approval that unblocked a step,
344344
timestamp: 2,
345345
},
346346
];
347-
const unblocked = { detail: { unblocked: "product_requirements" } };
347+
const opened = { detail: { canStart: ["product_requirements"] } };
348348
const read: ConversationTurn[] = [
349349
{
350350
role: "assistant",
@@ -380,18 +380,18 @@ test("coordinator director reminds once after an approval that unblocked a step,
380380
);
381381
};
382382

383-
expect(await holds([...approval(unblocked), ...read])).toBe(true);
383+
expect(await holds([...approval(opened), ...read])).toBe(true);
384384
expect(await holds(approval())).toBe(false);
385-
expect(await holds(approval({ ...unblocked, isError: true }))).toBe(false);
385+
expect(await holds(approval({ ...opened, isError: true }))).toBe(false);
386386
expect(
387387
await holds(
388-
approval(unblocked),
388+
approval(opened),
389389
"Do you want one design or several? Tell me any apps or colours you like.",
390390
),
391391
).toBe(false);
392392

393393
const director = subject();
394-
const turns = approval(unblocked);
394+
const turns = approval(opened);
395395
expect(await holds(turns, undefined, director)).toBe(true);
396396
const retried = await director.decide(
397397
{
@@ -406,7 +406,7 @@ test("coordinator director reminds once after an approval that unblocked a step,
406406
(action) => action.type === "infer",
407407
);
408408
expect(infer?.type === "infer" ? infer.options?.systemPrompt : "").toContain(
409-
"unblocked product_requirements",
409+
"these can start now: product_requirements",
410410
);
411411
expect(await holds(turns, "I will wait until you are ready.", director)).toBe(
412412
false,

0 commit comments

Comments
 (0)