Skip to content

Commit 1bd9bcc

Browse files
acartineclaude
andcommitted
fix(leases): make knotsLeaseId required throughout lease lifecycle
Harden the Knots lease contract: ensureKnotsLease now throws on failure instead of returning undefined, all downstream signatures require a non-optional knotsLeaseId, and prompt_delivered audit events are emitted after take/poll prompt construction. Passes lease ID to poll --claim. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
1 parent 7905e53 commit 1bd9bcc

15 files changed

+201
-81
lines changed

src/lib/__tests__/execution-backend-rollback.test.ts

Lines changed: 3 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -311,18 +311,14 @@ describe("rollbackIteration: metadata and edge cases", () => {
311311
expect(mockTerminateLease).toHaveBeenCalledWith("lease-k1", "/repo");
312312
});
313313

314-
it("warn-and-continues when createLease fails during prepareTake", async () => {
314+
it("returns failure when createLease fails during prepareTake", async () => {
315315
mockResolveMemoryManagerType.mockReturnValue("knots");
316316
mockCreateLease.mockResolvedValue({ ok: false, error: "lease create failed" });
317-
mockClaimKnot.mockResolvedValue({
318-
ok: true,
319-
data: { id: "beat-1", title: "Test", state: "in_progress", profile_id: "default", prompt: "do work" },
320-
});
321317

322318
const result = await seb.prepareTake({ beatId: "beat-1", repoPath: "/repo", mode: "take" });
323319

324-
expect(result.ok).toBe(true);
325-
expect(result.data?.knotsLeaseId).toBeUndefined();
320+
expect(result.ok).toBe(false);
321+
expect(result.error?.message).toContain("lease create failed");
326322
});
327323

328324
it("releases the knots lease when preparePoll fails after lease creation", async () => {

src/lib/__tests__/knots-backend-contract.test.ts

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -349,13 +349,18 @@ describe("KnotsBackend: buildTakePrompt", () => {
349349
expect(created.ok).toBe(true);
350350
const id = created.data!.id;
351351

352-
const result = await backend.buildTakePrompt(id);
352+
const result = await backend.buildTakePrompt(id, {
353+
knotsLeaseId: "test-lease",
354+
});
353355
expect(result.ok).toBe(true);
354356
expect(result.data?.prompt).toContain(id);
355357
expect(result.data?.prompt).toContain(
356358
"KNOTS CLAIM MODE",
357359
);
358360
expect(result.data?.prompt).toContain("kno claim");
361+
expect(result.data?.prompt).toContain(
362+
'--lease "test-lease"',
363+
);
359364
expect(result.data?.prompt).toContain(
360365
"single-step authorization",
361366
);

src/lib/__tests__/knots-backend-coverage-deps.test.ts

Lines changed: 11 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -234,6 +234,7 @@ describe("KnotsBackend coverage: buildTakePrompt single-beat mode", () => {
234234
const result = await backend.buildTakePrompt("SOLO-1", {
235235
isParent: true,
236236
childBeatIds: [],
237+
knotsLeaseId: "test-lease",
237238
});
238239

239240
expect(result.ok).toBe(true);
@@ -253,6 +254,7 @@ describe("KnotsBackend coverage: buildTakePrompt single-beat mode", () => {
253254
const result = await backend.buildTakePrompt("SOLO-2", {
254255
isParent: false,
255256
childBeatIds: ["unused"],
257+
knotsLeaseId: "test-lease",
256258
});
257259

258260
expect(result.ok).toBe(true);
@@ -263,7 +265,9 @@ describe("KnotsBackend coverage: buildTakePrompt single-beat mode", () => {
263265

264266
it("returns error when showKnot fails for single-beat mode", async () => {
265267
const backend = new KnotsBackend("/repo");
266-
const result = await backend.buildTakePrompt("MISSING-SINGLE");
268+
const result = await backend.buildTakePrompt("MISSING-SINGLE", {
269+
knotsLeaseId: "test-lease",
270+
});
267271

268272
expect(result.ok).toBe(false);
269273
expect(mockClaimKnot).not.toHaveBeenCalled();
@@ -277,7 +281,9 @@ describe("KnotsBackend coverage: buildTakePrompt single-beat mode", () => {
277281
description: "Do the thing",
278282
});
279283

280-
const result = await backend.buildTakePrompt("DETAIL-1");
284+
const result = await backend.buildTakePrompt("DETAIL-1", {
285+
knotsLeaseId: "test-lease",
286+
});
281287

282288
expect(result.ok).toBe(true);
283289
expect(result.data?.prompt).toContain("Beat ID: DETAIL-1");
@@ -303,7 +309,9 @@ describe("KnotsBackend coverage: buildTakePrompt single-beat mode", () => {
303309
body: "Body text here",
304310
});
305311

306-
const result = await backend.buildTakePrompt("BODY-1");
312+
const result = await backend.buildTakePrompt("BODY-1", {
313+
knotsLeaseId: "test-lease",
314+
});
307315

308316
expect(result.ok).toBe(true);
309317
expect(result.data?.prompt).toContain(

src/lib/__tests__/terminal-manager-abort.test.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -82,7 +82,7 @@ vi.mock("@/lib/knots", () => ({
8282

8383
vi.mock("@/lib/lease-audit", () => ({
8484
appendLeaseAuditEvent: vi.fn(async () => undefined),
85-
logLeaseAudit: vi.fn(),
85+
logLeaseAudit: vi.fn(async () => undefined),
8686
}));
8787

8888
vi.mock("@/lib/beads-state-machine", () => ({

src/lib/__tests__/terminal-manager-error-retry-retry.test.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -68,6 +68,7 @@ vi.mock("@/lib/backend-instance", () => ({
6868
}));
6969

7070
vi.mock("@/lib/interaction-logger", () => ({
71+
resolveInteractionLogRoot: vi.fn(() => "/tmp/foolery-logs"),
7172
startInteractionLog: vi.fn(async () => interactionLog),
7273
noopInteractionLog: vi.fn(() => interactionLog),
7374
}));
@@ -81,7 +82,7 @@ vi.mock("@/lib/knots", () => ({
8182

8283
vi.mock("@/lib/lease-audit", () => ({
8384
appendLeaseAuditEvent: vi.fn(async () => undefined),
84-
logLeaseAudit: vi.fn(),
85+
logLeaseAudit: vi.fn(async () => undefined),
8586
}));
8687

8788
vi.mock("@/lib/beads-state-machine", () => ({

src/lib/__tests__/terminal-manager-knots-next-guard-take-loop.test.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -89,7 +89,7 @@ vi.mock("@/lib/knots", () => ({
8989

9090
vi.mock("@/lib/lease-audit", () => ({
9191
appendLeaseAuditEvent: vi.fn(async () => undefined),
92-
logLeaseAudit: vi.fn(),
92+
logLeaseAudit: vi.fn(async () => undefined),
9393
}));
9494

9595
vi.mock("@/lib/beads-state-machine", () => ({

src/lib/__tests__/terminal-manager-queue-claims.test.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -134,7 +134,7 @@ vi.mock("@/lib/agent-outcome-stats", () => ({
134134

135135
vi.mock("@/lib/lease-audit", () => ({
136136
appendLeaseAuditEvent: vi.fn(async () => undefined),
137-
logLeaseAudit: vi.fn(),
137+
logLeaseAudit: vi.fn(async () => undefined),
138138
}));
139139

140140
import { createSession } from "@/lib/terminal-manager";

src/lib/__tests__/terminal-manager-step-failure-rollback.test.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -82,7 +82,7 @@ vi.mock("@/lib/knots", () => ({
8282

8383
vi.mock("@/lib/lease-audit", () => ({
8484
appendLeaseAuditEvent: vi.fn(async () => undefined),
85-
logLeaseAudit: vi.fn(),
85+
logLeaseAudit: vi.fn(async () => undefined),
8686
}));
8787

8888
vi.mock("@/lib/beads-state-machine", () => ({

src/lib/backends/knots-backend-prompts.ts

Lines changed: 20 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -300,10 +300,26 @@ export async function buildSingleTakePrompt(
300300
}
301301
const knot = showResult.data!;
302302

303-
const claimCmd = options?.knotsLeaseId
304-
? `kno claim "${beatId}" --json ` +
305-
`--lease "${options.knotsLeaseId}"`
306-
: `kno claim "${beatId}" --json`;
303+
if (!options?.knotsLeaseId) {
304+
return {
305+
ok: false,
306+
error: {
307+
code: "INTERNAL",
308+
message: `knotsLeaseId is required to build ` +
309+
`a take prompt for ${beatId}`,
310+
retryable: false,
311+
},
312+
};
313+
}
314+
const claimCmd =
315+
`kno claim "${beatId}" --json ` +
316+
`--lease "${options.knotsLeaseId}"`;
317+
console.log(
318+
`[knots-backend-prompts] buildSingleTakePrompt: ` +
319+
`beatId=${beatId} ` +
320+
`leaseId=${options.knotsLeaseId} ` +
321+
`claimCmd=${claimCmd}`,
322+
);
307323

308324
const prompt = [
309325
`Beat ID: ${beatId}`,

src/lib/execution-backend-helpers.ts

Lines changed: 83 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@ import {
2323
logAttachedKnotsLease,
2424
terminateKnotsRuntimeLease,
2525
} from "@/lib/knots-lease-runtime";
26+
import { logLeaseAudit } from "@/lib/lease-audit";
2627
import {
2728
forwardTransitionTarget,
2829
resolveStep,
@@ -69,14 +70,23 @@ export async function prepareTakeKnots(
6970
leaseId: string,
7071
loadSnapshot: LoadSnapshotFn,
7172
): Promise<BackendResult<ExecutionLease>> {
72-
const knotsLeaseId = await ensureKnotsLease({
73-
repoPath: input.repoPath,
74-
source: "structured_prepare_take",
75-
executionLeaseId: leaseId,
76-
beatId: input.beatId,
77-
interactionType: input.mode,
78-
agentInfo: input.agentInfo,
79-
});
73+
let knotsLeaseId: string;
74+
try {
75+
knotsLeaseId = await ensureKnotsLease({
76+
repoPath: input.repoPath,
77+
source: "structured_prepare_take",
78+
executionLeaseId: leaseId,
79+
beatId: input.beatId,
80+
interactionType: input.mode,
81+
agentInfo: input.agentInfo,
82+
});
83+
} catch (err) {
84+
return fail(
85+
err instanceof Error
86+
? err.message
87+
: "Failed to create Knots lease",
88+
);
89+
}
8090
const claimResult = await claimKnot(
8191
input.beatId, input.repoPath, {
8292
agentName: input.agentInfo?.agentName,
@@ -115,7 +125,7 @@ async function finalizeTakeKnots(
115125
backend: BackendPort,
116126
input: PrepareTakeInput,
117127
leaseId: string,
118-
knotsLeaseId: string | undefined,
128+
knotsLeaseId: string,
119129
claimData: { prompt: string; state: string },
120130
loadSnapshot: LoadSnapshotFn,
121131
): Promise<BackendResult<ExecutionLease>> {
@@ -172,6 +182,27 @@ async function finalizeTakeKnots(
172182
agentInfo: input.agentInfo,
173183
knotsLeaseId,
174184
});
185+
void logLeaseAudit({
186+
event: "prompt_delivered",
187+
repoPath: input.repoPath,
188+
executionLeaseId: leaseId,
189+
knotsLeaseId,
190+
beatId: input.beatId,
191+
interactionType: input.mode,
192+
agentName: input.agentInfo?.agentName,
193+
agentModel: input.agentInfo?.agentModel,
194+
agentVersion: input.agentInfo?.agentVersion,
195+
outcome: "success",
196+
message:
197+
`Execution prompt includes lease ` +
198+
`${knotsLeaseId} for ${input.beatId}.`,
199+
data: {
200+
source: "structured_prepare_take",
201+
promptLength: claimData.prompt.length,
202+
hasLeaseInPrompt:
203+
claimData.prompt.includes("--lease"),
204+
},
205+
});
175206
return ok(lease);
176207
}
177208

@@ -297,15 +328,27 @@ export async function preparePollKnots(
297328
loadSnapshot: LoadSnapshotFn,
298329
): Promise<BackendResult<PollLeaseResult>> {
299330
const executionLeaseId = generateLeaseId();
300-
const knotsLeaseId = await ensureKnotsLease({
301-
repoPath: input.repoPath,
302-
source: "structured_prepare_poll",
303-
executionLeaseId,
304-
interactionType: "poll",
305-
agentInfo: input.agentInfo,
306-
});
331+
let knotsLeaseId: string;
332+
try {
333+
knotsLeaseId = await ensureKnotsLease({
334+
repoPath: input.repoPath,
335+
source: "structured_prepare_poll",
336+
executionLeaseId,
337+
interactionType: "poll",
338+
agentInfo: input.agentInfo,
339+
});
340+
} catch (err) {
341+
return fail(
342+
err instanceof Error
343+
? err.message
344+
: "Failed to create Knots lease",
345+
);
346+
}
307347
const pollResult = await pollKnot(
308-
input.repoPath, input.agentInfo,
348+
input.repoPath, {
349+
...input.agentInfo,
350+
leaseId: knotsLeaseId,
351+
},
309352
);
310353
if (!pollResult.ok || !pollResult.data) {
311354
await terminateKnotsRuntimeLease({
@@ -333,7 +376,7 @@ async function finalizePollKnots(
333376
backend: BackendPort,
334377
input: PreparePollInput,
335378
executionLeaseId: string,
336-
knotsLeaseId: string | undefined,
379+
knotsLeaseId: string,
337380
pollData: { id: string; prompt: string; state: string },
338381
loadSnapshot: LoadSnapshotFn,
339382
): Promise<BackendResult<PollLeaseResult>> {
@@ -391,6 +434,28 @@ async function finalizePollKnots(
391434
agentInfo: input.agentInfo,
392435
knotsLeaseId,
393436
});
437+
void logLeaseAudit({
438+
event: "prompt_delivered",
439+
repoPath: input.repoPath,
440+
executionLeaseId,
441+
knotsLeaseId,
442+
beatId: pollData.id,
443+
claimedId: pollData.id,
444+
interactionType: "poll",
445+
agentName: input.agentInfo?.agentName,
446+
agentModel: input.agentInfo?.agentModel,
447+
agentVersion: input.agentInfo?.agentVersion,
448+
outcome: "success",
449+
message:
450+
`Poll prompt includes lease ` +
451+
`${knotsLeaseId} for ${pollData.id}.`,
452+
data: {
453+
source: "structured_prepare_poll",
454+
promptLength: pollData.prompt.length,
455+
hasLeaseInPrompt:
456+
pollData.prompt.includes("--lease"),
457+
},
458+
});
394459
return ok({ lease, claimedId: pollData.id });
395460
}
396461

0 commit comments

Comments
 (0)