diff --git a/src/agent/tools/practice.ts b/src/agent/tools/practice.ts index c35b8bc..dc0b8c0 100644 --- a/src/agent/tools/practice.ts +++ b/src/agent/tools/practice.ts @@ -111,6 +111,27 @@ export interface PracticeToolContext { chatStore: { getChat(id: string): { messages: LhMessage[]; kind?: string } | null }; } +/** TURN-SCOPED GATE STATE (the BKT incident, 2026-10-05): the gates used + * to read the STORED record — but the record freezes at the turn's + * write-ahead (persistTurn lands only at turn END), so a mid-turn + * conclude → pose pair could never see each other: every conclude + * succeeded against the stale record and every following pose died on + * "a problem is already active" until the model gave up ("the problem + * slot is stuck"). The gate seeds ONCE from the record at construction + * (one practiceTools per turn — chat-handler builds the agent per + * request, so there is no cross-turn leakage), and the executes flip + * this closure — the intra-turn truth the record cannot hold yet. */ +export interface PracticeGate { + problemActive: boolean; +} + +/** Seed the turn's gate from the record (result-aware activeProblemOf — + * only a cleanly-posed problem counts). */ +export function createPracticeGate(ctx: PracticeToolContext): PracticeGate { + const record = ctx.chatStore.getChat(ctx.chatId); + return { problemActive: record ? activeProblemOf(record.messages) !== null : false }; +} + // ─── problem: the pose ─────────────────────────────────────────────────────── export const problemInputObject = Type.Object({ @@ -144,7 +165,7 @@ export const problemInputObject = Type.Object({ export type ProblemInput = Static; -export function problemTool(ctx: PracticeToolContext): AgentTool { +export function problemTool(gate: PracticeGate): AgentTool { return { name: "problem", label: "problem", @@ -157,12 +178,12 @@ export function problemTool(ctx: PracticeToolContext): AgentTool healEncodedArguments(args, problemInputObject) as never, execute: async (_toolCallId, _input) => { - const record = ctx.chatStore.getChat(ctx.chatId); - if (record && activeProblemOf(record.messages) !== null) { + if (gate.problemActive) { return invalidInputResult("problem", [ "a problem is already active — call conclude on it before posing the next one", ]); } + gate.problemActive = true; return problemResult(); }, }; @@ -207,7 +228,7 @@ export const concludeInputObject = Type.Object({ export type ConcludeInput = Static; -export function concludeTool(ctx: PracticeToolContext): AgentTool { +export function concludeTool(gate: PracticeGate): AgentTool { return { name: "conclude", label: "conclude", @@ -219,10 +240,10 @@ export function concludeTool(ctx: PracticeToolContext): AgentTool healEncodedArguments(args, concludeInputObject) as never, executionMode: "sequential", execute: async (_toolCallId, _input) => { - const record = ctx.chatStore.getChat(ctx.chatId); - if (!record || activeProblemOf(record.messages) === null) { + if (!gate.problemActive) { return invalidInputResult("conclude", ["no active problem to conclude — pose one first"]); } + gate.problemActive = false; return concludeResult(); }, }; @@ -230,8 +251,10 @@ export function concludeTool(ctx: PracticeToolContext): AgentTool> { + // ONE gate per turn, shared by both tools — the intra-turn truth. + const gate = createPracticeGate(ctx); return { - problem: problemTool(ctx), - conclude: concludeTool(ctx), + problem: problemTool(gate), + conclude: concludeTool(gate), }; } diff --git a/src/core/native-messages.ts b/src/core/native-messages.ts index a0d0190..4895093 100644 --- a/src/core/native-messages.ts +++ b/src/core/native-messages.ts @@ -316,9 +316,26 @@ export function concludedByDismissalPair( * conclude call or a dismissal marker deactivates. The ack result at pose * time does NOT (the problem stays on screen through nudges; the dock's * subject is the call, not a dangling result). */ +/** The ids of tool calls whose results are CLEAN (not errored) — the only + * calls that ever happened, per the honest-activation ruling. */ +function cleanToolCallIds(messages: readonly LhMessage[]): Set { + const ids = new Set(); + for (const m of messages) { + if ((m as { role?: string }).role !== "toolResult") continue; + const result = m as ToolResultMessage; + if (!result.isError) ids.add(result.toolCallId); + } + return ids; +} + export function activeProblemOf( messages: readonly LhMessage[], ): { toolCallId: string; input: unknown } | null { + // HONEST ACTIVATION (owner ruling, 2026-10-05 — the BKT incident): + // calls count only through their RESULTS — a problem activates on its + // clean ack, a conclude frees on its clean ack; errored and dangling + // calls are skipped ("either it goes through or it doesn't"). + const cleanCallIds = cleanToolCallIds(messages); for (let mi = messages.length - 1; mi >= 0; mi--) { const m = messages[mi]!; const role = (m as { role?: string }).role; @@ -332,9 +349,13 @@ export function activeProblemOf( for (let bi = assistant.content.length - 1; bi >= 0; bi--) { const block = assistant.content[bi]!; if (block.type !== "toolCall") continue; - if (block.name === PRACTICE_CONCLUDE_TOOL) return null; + if (block.name === PRACTICE_CONCLUDE_TOOL) { + if (cleanCallIds.has(block.id)) return null; // a CLEAN conclude frees + continue; // errored/dangling conclude: the free never happened + } if (block.name !== PRACTICE_PROBLEM_TOOL) continue; - return { toolCallId: block.id, input: block.arguments }; + if (cleanCallIds.has(block.id)) return { toolCallId: block.id, input: block.arguments }; + // errored/dangling pose — it did not go through; keep scanning } continue; } @@ -347,6 +368,7 @@ export function activeProblemOf( * (input-events). Newest-marker-wins scan: the first dismissal marker or * conclude call from the end decides. */ export function pendingProblemDismissalOf(messages: readonly LhMessage[]): string | null { + const cleanCallIds = cleanToolCallIds(messages); for (let mi = messages.length - 1; mi >= 0; mi--) { const m = messages[mi]!; const role = (m as { role?: string }).role; @@ -362,8 +384,14 @@ export function pendingProblemDismissalOf(messages: readonly LhMessage[]): strin for (let bi = assistant.content.length - 1; bi >= 0; bi--) { const block = assistant.content[bi]!; if (block.type !== "toolCall" && block.type !== "text") continue; - if (block.type === "toolCall" && block.name === PRACTICE_CONCLUDE_TOOL) return null; - if (block.type === "toolCall" && block.name === PRACTICE_PROBLEM_TOOL) return null; + if (block.type === "toolCall" && block.name === PRACTICE_CONCLUDE_TOOL) { + if (cleanCallIds.has(block.id)) return null; // a CLEAN conclude settles it + continue; // errored/dangling: it never happened + } + if (block.type === "toolCall" && block.name === PRACTICE_PROBLEM_TOOL) { + if (cleanCallIds.has(block.id)) return null; // a CLEAN pose supersedes it + continue; + } } continue; } diff --git a/tests/practice-agents.test.ts b/tests/practice-agents.test.ts index 092d828..7677eee 100644 --- a/tests/practice-agents.test.ts +++ b/tests/practice-agents.test.ts @@ -17,7 +17,7 @@ import { PRACTICE_PROBLEM_TOOL, type LhMessage, } from "../src/core/native-messages.ts"; -import { concludeTool, problemTool, practiceTools } from "../src/agent/tools/practice.ts"; +import { concludeTool, createPracticeGate, problemTool, practiceTools } from "../src/agent/tools/practice.ts"; import { composeInstructions, createPracticeAgent } from "../src/agent/teacher-agent.ts"; import { allSegmentPaths } from "../src/agent/prompt-segments.ts"; @@ -95,14 +95,14 @@ const CONCLUDE_INPUT = { describe("the practice tool gating law", () => { it("problem acks on a quiet record and ends the turn (terminate)", async () => { - const result = await problemTool(ctxOf([])).execute("call_x", PROBLEM_INPUT as never); + const result = await problemTool(createPracticeGate(ctxOf([]))).execute("call_x", PROBLEM_INPUT as never); assert.equal(result.isError, undefined); assert.equal(result.terminate, true); assert.match((result.content[0] as { text: string }).text, /Problem posed/); }); it("problem is REJECTED while a problem is active (conclude first)", async () => { - const result = await problemTool(ctxOf(activeProblemRecord())).execute("call_x", PROBLEM_INPUT as never); + const result = await problemTool(createPracticeGate(ctxOf(activeProblemRecord()))).execute("call_x", PROBLEM_INPUT as never); assert.equal(result.isError, true); assert.deepEqual((result.details as { problems: string[] }).problems, [ "a problem is already active — call conclude on it before posing the next one", @@ -110,7 +110,7 @@ describe("the practice tool gating law", () => { }); it("conclude is REJECTED with nothing active", async () => { - const result = await concludeTool(ctxOf([])).execute("call_x", CONCLUDE_INPUT as never); + const result = await concludeTool(createPracticeGate(ctxOf([]))).execute("call_x", CONCLUDE_INPUT as never); assert.equal(result.isError, true); assert.deepEqual((result.details as { problems: string[] }).problems, [ "no active problem to conclude — pose one first", @@ -118,10 +118,26 @@ describe("the practice tool gating law", () => { }); it("conclude acks on the active problem and the turn CONTINUES (no terminate)", async () => { - const result = await concludeTool(ctxOf(activeProblemRecord())).execute("call_x", CONCLUDE_INPUT as never); + const result = await concludeTool(createPracticeGate(ctxOf(activeProblemRecord()))).execute("call_x", CONCLUDE_INPUT as never); assert.equal(result.isError, undefined); assert.equal(result.terminate, undefined); }); + + it("THE BKT INCIDENT — a mid-turn conclude → pose pair works (the gate is turn-scoped, not record-frozen)", async () => { + // One turn, one toolset: the record froze at the write-ahead with + // the old problem active. The gates used to re-read that frozen + // record per execute, so the pose after a successful conclude + // always died on "a problem is already active" until the model + // gave up ("the problem slot is stuck"). + const gate = createPracticeGate(ctxOf(activeProblemRecord())); + const tools = { problem: problemTool(gate), conclude: concludeTool(gate) }; + const conclude = await tools.conclude.execute("call_c1", CONCLUDE_INPUT as never); + assert.equal(conclude.isError, undefined, "the conclude acks against the seeded record"); + const pose = await tools.problem.execute("call_p2", PROBLEM_INPUT as never); + assert.equal(pose.isError, undefined, "the freed gate lets the next problem through — SAME turn"); + const greedy = await tools.problem.execute("call_p3", PROBLEM_INPUT as never); + assert.equal(greedy.isError, true, "the gate re-arms on the successful pose"); + }); }); // ── the registry + the agent ──────────────────────────────────────────────── diff --git a/tests/practice-record.test.ts b/tests/practice-record.test.ts index 49c8761..269d55a 100644 --- a/tests/practice-record.test.ts +++ b/tests/practice-record.test.ts @@ -68,21 +68,35 @@ function problemPose(callId = "call_p1", statement = "Show that…"): LhMessage[ return [assistant, ack]; } -function concludeCall(outcome: string): LhMessage & AssistantMessage { - return { - id: `msg_c_${outcome}`, - role: "assistant", - content: [ - { - type: "toolCall", - id: `call_conclude_${outcome}`, - name: PRACTICE_CONCLUDE_TOOL, - arguments: { outcome }, - }, - ], - timestamp: 3, - ...STAMPS, - }; +function concludeCall(outcome: string): LhMessage[] { + // Honest folds: the conclude call rides WITH its clean ack — a + // dangling or errored conclude never freed anything. + return [ + { + id: `msg_c_${outcome}`, + role: "assistant", + content: [ + { + type: "toolCall", + id: `call_conclude_${outcome}`, + name: PRACTICE_CONCLUDE_TOOL, + arguments: { outcome }, + }, + ], + timestamp: 3, + ...STAMPS, + }, + { + id: `msg_cr_${outcome}`, + role: "toolResult", + toolCallId: `call_conclude_${outcome}`, + toolName: PRACTICE_CONCLUDE_TOOL, + content: [{ type: "text", text: "Problem concluded — the composer is free; pose the next problem or wrap up." }], + details: null, + isError: false, + timestamp: 4, + }, + ]; } // ── the active-problem fold ────────────────────────────────────────────────── @@ -108,10 +122,39 @@ describe("activeProblemOf (the conclude-before-next-problem gate's fold)", () => }); it("a conclude call deactivates", () => { - const messages = [...problemPose(), concludeCall("solved")]; + const messages = [...problemPose(), ...concludeCall("solved")]; assert.equal(activeProblemOf(messages), null); }); + it("an ERRORED pose never activates (the BKT phantom — honest activation)", () => { + // The gate refused the pose ("a problem is already active" in the + // BKT incident); the composer must not mount from its arguments. + const rejected = problemPose("call_bad", "A different student…"); + (rejected[1]! as unknown as { isError: boolean }).isError = true; + assert.equal(activeProblemOf(rejected), null, "a lone refused pose is nothing"); + }); + + it("a clean pose UNDER an errored rejection stays active (the scan skips refusals)", () => { + const rejected = problemPose("call_bad", "A different student…"); + (rejected[1]! as unknown as { isError: boolean }).isError = true; + const active = activeProblemOf([...problemPose(), ...rejected]); + assert.ok(active, "the clean pose below is still the active one"); + assert.equal(active!.toolCallId, "call_p1"); + }); + + it("an ERRORED conclude does not free (only the clean ack frees)", () => { + const badConclude = concludeCall("solved"); + (badConclude[1]! as unknown as { isError: boolean }).isError = true; + const active = activeProblemOf([...problemPose(), ...badConclude]); + assert.ok(active, "the conclude never happened — the problem stays live"); + assert.equal(active!.toolCallId, "call_p1"); + }); + + it("a DANGLING pose (mid-turn, no result) does not activate — it has not gone through", () => { + const dangling = problemPose("call_new", "Fresh problem…")[0]!; + assert.equal(activeProblemOf([dangling]), null, "no ack, no problem"); + }); + it("a dismissal marker deactivates immediately (the card closes at Esc)", () => { const messages = [...problemPose(), newProblemDismissedMarker("call_p1")]; assert.equal(activeProblemOf(messages), null); @@ -120,7 +163,7 @@ describe("activeProblemOf (the conclude-before-next-problem gate's fold)", () => it("a NEWER problem after a conclude is the active one", () => { const messages = [ ...problemPose("call_p1"), - concludeCall("solved"), + ...concludeCall("solved"), ...problemPose("call_p2", "second"), ]; assert.equal(activeProblemOf(messages)?.toolCallId, "call_p2"); @@ -235,7 +278,7 @@ describe("the dismiss-problem input (ask-style discard)", () => { it("400s when nothing active or a stale id", () => { assert.throws(() => dismissAppends({ messages: [] }, { type: "dismiss-problem", problemCallId: "x" }), /no active problem/); - const concluded = [...problemPose(), concludeCall("solved")]; + const concluded = [...problemPose(), ...concludeCall("solved")]; assert.throws(() => dismissAppends({ messages: concluded }, { type: "dismiss-problem", problemCallId: "call_p1" }), /no active problem/); }); }); @@ -278,7 +321,7 @@ describe("needsContinuation (a persisted pose ack owes nothing)", () => { it("a mixed batch (conclude ack then pose ack) still owes nothing — the pose is terminal", () => { const messages = [ ...problemPose("call_p0", "the earlier one"), - concludeCall("solved"), + ...concludeCall("solved"), { id: "msg_r_conclude", role: "toolResult",