Skip to content

fix(hub): move on after an approval unless the person asks to wait #199

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
TheGreatAxios merged 1 commit into from
Nov 6, 2026
Merged

fix(hub): move on after an approval unless the person asks to wait #199

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
47 changes: 45 additions & 2 deletions apps/hub/src/projects/deploy-tools.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -222,6 +222,7 @@ function writerTools(preferences: Record<string, string> = {}) {
].map((item) => ({ ...item, sha256: "a".repeat(64) }));
const deployed: string[] = [];
const approvals: unknown[] = [];
const inFlight: Record<string, unknown>[] = [];
const fetch = async (path: string, init: RequestInit) => {
const body = init.body ? JSON.parse(String(init.body)) : undefined;
if (path.endsWith("/workflows/deployments")) {
Expand All @@ -235,7 +236,7 @@ function writerTools(preferences: Record<string, string> = {}) {
});
}
if (path.endsWith("/artifacts") && init.method === "GET")
return Response.json({ artifacts: documents, inFlight: [], preferences });
return Response.json({ artifacts: documents, inFlight, preferences });
if (path.includes("/artifacts/") && init.method === "GET") {
const source = documents.find((item) =>
path.includes("/artifacts/" + item["artifactId"]),
Expand Down Expand Up @@ -287,7 +288,7 @@ function writerTools(preferences: Record<string, string> = {}) {
{ id: "call", name, arguments: args },
new AbortController().signal,
);
return { run, deployed, approvals, documents };
return { run, deployed, approvals, documents, inFlight };
}

const writer = (kind: string) => ({
Expand All @@ -296,6 +297,48 @@ const writer = (kind: string) => ({
"Write the " + kind + " from the filed documents and the person's answers.",
});

test("an approval reports the steps whose gates it opened, unless one is under way", async () => {
const { run, documents, inFlight } = writerTools();
const canStart = async (kinds: string[]) => {
const result = await run("request_approval", { kinds });
return [JSON.parse(String(result.content)).canStart, result.detail];
};

// The filed constraints are not approved yet, so approving the brief opens nothing.
expect(await canStart(["problem_brief"])).toEqual([[], undefined]);
inFlight.push({ runId: "w", role: "writer", status: "running" });
expect(await canStart(["constraints"])).toEqual([[], undefined]);
inFlight.pop();
expect(await canStart(["constraints"])).toEqual([
["approaches"],
{ canStart: ["approaches"] },
]);

for (const kind of ["approaches", "product_requirements"])
documents.push({
kind,
artifactId: kind,
title: kind,
version: 1,
sha256: "a".repeat(64),
});
expect(await canStart(["approaches", "product_requirements"])).toEqual([
["design"],
{ canStart: ["design"] },
]);
inFlight.push({ runId: "d", role: "designer", status: "running" });
expect(await canStart(["product_requirements"])).toEqual([[], undefined]);
inFlight.pop();
documents.push({
kind: "design_option",
artifactId: "o",
title: "Calm",
version: 1,
sha256: "a".repeat(64),
});
expect(await canStart(["product_requirements"])).toEqual([[], undefined]);
});

test("one ask approves several documents, and a writer starts once every earlier one is approved", async () => {
const { run, deployed, approvals } = writerTools();

Expand Down
173 changes: 117 additions & 56 deletions apps/hub/src/projects/deploy-tools.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -380,9 +380,14 @@ export function createDeployTools(options?: {
const materials: ArtifactRef[] = [];
const approved = new Map<string, number>();
const openQuestions = new Map<string, number>();
// Options a person has yet to choose from mean the design is already under way.
let designOptions = false;
for (const item of list.artifacts) {
const { kind, artifactId, version, sha256, approvedVersion } = item;
if (kind === "design_option") continue;
if (kind === "design_option") {
designOptions = true;
continue;
}
if (kind === MATERIAL_KIND)
materials.push({ kind, artifactId, version, sha256 });
byKind.set(kind, { artifactId, version, sha256 });
Expand All @@ -396,6 +401,7 @@ export function createDeployTools(options?: {
materials,
approved,
openQuestions,
designOptions,
inFlight: list.inFlight,
preferences: list.preferences,
};
Expand Down Expand Up @@ -430,18 +436,96 @@ export function createDeployTools(options?: {
) => hub(projectPath + "/artifact-approvals", { tool, artifacts });
const refsOf = (byKind: Map<string, Ref>) =>
[...byKind].map(([kind, ref]) => ({ kind, ...ref }));
const notFiled = (kind: string) =>
new Refusal(
kind +
"_not_filed: " +
(kind === "design"
? "have the designer make it with design_app first"
: "write and file the " + kind + " first"),
);
const required = (byKind: Map<string, Ref>, kind: string) => {
const ref = byKind.get(kind);
if (!ref)
throw new Refusal(
kind +
"_not_filed: " +
(kind === "design"
? "have the designer make it with design_app first"
: "write and file the " + kind + " first"),
);
if (!ref) throw notFiled(kind);
return ref;
};
type Listed = Awaited<ReturnType<typeof listed>>;
/*
* Each start tool refuses with its gate's answer, and an approval reports the steps whose
* gates then pass. The coordinator learns what it may start from the rule that enforces it,
* so loosening a gate loosens what it is told.
*/
const writerGate = (
{ byKind, approved, preferences }: Listed,
kind: (typeof DOCUMENT_ORDER)[number],
): Refusal | undefined => {
if (!byKind.has("problem_brief")) return notFiled("problem_brief");
const earlier = DOCUMENT_ORDER.slice(0, DOCUMENT_ORDER.indexOf(kind));
// An imported project may lack an earlier document that its later ones were written without.
const writtenWithout = (index: number) =>
DOCUMENT_ORDER.slice(index + 1).some((later) => byKind.has(later));
for (const [index, before] of earlier.entries()) {
const ref = byKind.get(before);
if (!ref) {
if (writtenWithout(index)) continue;
return new Refusal(
before +
"_missing: " +
(before === "design"
? "have the designer make it with design_app"
: "write and file the " + before) +
" before the " +
kind,
);
}
if (
!agentDecidesApprovals(preferences) &&
approved.get(before) !== ref.version
)
return notApproved(before, ref.version);
}
return undefined;
};
const designGate = ({ byKind }: Listed) => {
const missing = DESIGNER_INPUTS.find((kind) => !byKind.has(kind));
return missing === undefined ? undefined : notFiled(missing);
};
const coderGate = ({ byKind, approved, preferences }: Listed) => {
const missing = CODER_DOCUMENTS.find((kind) => !byKind.has(kind));
if (missing !== undefined) return notFiled(missing);
for (const kind of DOCUMENT_ORDER) {
const ref = byKind.get(kind);
if (!ref) return notFiled(kind);
// The design is approved by choosing it, whoever decides the other documents.
if (
(kind === "design" || !agentDecidesApprovals(preferences)) &&
approved.get(kind) !== ref.version
)
return notApproved(kind, ref.version);
}
return undefined;
};
// A step already made or under way is not offered again, whatever its gate says.
const startable = (state: Listed) => {
const busy = new Set(state.inFlight.map((run) => run.role));
const open = (
step: string,
role: string,
gate: Refusal | undefined,
) => !state.byKind.has(step) && !busy.has(role) && gate === undefined;
Comment thread
TheGreatAxios marked this conversation as resolved.
Resolved
return [
...DOCUMENT_ORDER.filter(
(kind) =>
kind in WRITER_PACKAGES &&
open(kind, "writer", writerGate(state, kind)),
),
...(!state.designOptions &&
open("design", "designer", designGate(state))
? ["design"]
: []),
...(open("app", "coder", coderGate(state)) ? ["app"] : []),
];
};
// A fresh deployment warms its workspace with the current versions, the ones named here.
const inputs = (
kinds: readonly InputKind[],
Expand Down Expand Up @@ -504,10 +588,11 @@ export function createDeployTools(options?: {
try {
signal.throwIfAborted();
let result: unknown;
let canStart: readonly string[] = [];
if (call.name === "request_approval") {
const { kinds } = validArguments(RequestApproval, call.arguments);
const { byKind, materials, openQuestions, preferences } =
await listed();
const state = await listed();
const { byKind, materials, openQuestions, preferences } = state;
const open = [...new Set(kinds)].flatMap((kind) => {
const count = openQuestions.get(kind);
return count
Expand All @@ -530,38 +615,17 @@ export function createDeployTools(options?: {
return materials;
});
await recordApproval("request_approval", documents);
result = { approved: documents };
for (const { kind, version } of documents)
state.approved.set(kind, version);
canStart = startable(state);
result = { approved: documents, canStart };
} else if (call.name === "deploy_writer") {
const input = validArguments(DeployWriter, call.arguments);
const { byKind, approved, preferences } = await listed();
const state = await listed();
const { byKind, preferences } = state;
const refusal = writerGate(state, input.kind);
if (refusal) throw refusal;
const briefRef = required(byKind, "problem_brief");
const earlier = DOCUMENT_ORDER.slice(
0,
DOCUMENT_ORDER.indexOf(input.kind),
);
// An imported project may lack an earlier document that its later ones were written without.
const writtenWithout = (index: number) =>
DOCUMENT_ORDER.slice(index + 1).some((kind) => byKind.has(kind));
for (const [index, kind] of earlier.entries()) {
const ref = byKind.get(kind);
if (!ref) {
if (writtenWithout(index)) continue;
throw new Refusal(
kind +
"_missing: " +
(kind === "design"
? "have the designer make it with design_app"
: "write and file the " + kind) +
" before the " +
input.kind,
);
}
if (
!agentDecidesApprovals(preferences) &&
approved.get(kind) !== ref.version
)
throw notApproved(kind, ref.version);
}
const target = byKind.get(input.kind);
const sourceRefs = [...byKind]
.filter(
Expand Down Expand Up @@ -623,26 +687,17 @@ export function createDeployTools(options?: {
options.projectId + ":coder",
"coder",
async () => {
const { byKind, approved, inFlight, preferences } =
await listed();
for (const kind of CODER_DOCUMENTS) required(byKind, kind);
const state = await listed();
const { byKind, inFlight, preferences } = state;
const live = inFlight.find((run) => run.role === "coder");
if (live)
throw new Refusal(
"coder_already_running: the coder is building in run " +
live.runId +
"; send it changes with instruct_coder",
);
// The design is approved by choosing it, whoever decides the other documents.
for (const kind of DOCUMENT_ORDER) {
const { version } = required(byKind, kind);
if (
(kind === "design" ||
!agentDecidesApprovals(preferences)) &&
approved.get(kind) !== version
)
throw notApproved(kind, version);
}
const refusal = coderGate(state);
if (refusal) throw refusal;
const design = required(byKind, "design");
return deploy(
"coder",
Expand Down Expand Up @@ -696,8 +751,10 @@ export function createDeployTools(options?: {
options.projectId + ":designer:" + concept.label,
"designer",
async () => {
const { byKind, inFlight, preferences } = await listed();
for (const kind of DESIGNER_INPUTS) required(byKind, kind);
const state = await listed();
const { byKind, inFlight, preferences } = state;
const refusal = designGate(state);
if (refusal) throw refusal;
const message =
designLookIdeas(input.lookIdeas) +
coderTaskMessage(input.instruction);
Expand Down Expand Up @@ -832,7 +889,11 @@ export function createDeployTools(options?: {
content: "unknown_deploy_tool",
isError: true,
};
return { callId: call.id, content: JSON.stringify(result) };
return {
callId: call.id,
content: JSON.stringify(result),
...(canStart.length ? { detail: { canStart } } : {}),
};
} catch (error) {
return {
callId: call.id,
Expand Down
Loading