Merge pull request #6 from devweekends/feat/approval-requested

runner: send approval.requested after task.finished (contracts v0.2.0)

Author: imtahirabatool

runner/src/fake-runner.ts

@@ -12,6 +12,9 @@ export interface FakeRunnerOptions {
 
 type Step = [RunnerEvent["type"], Record<string, unknown>];
 
+const FAKE_STAT = " src/date.js | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n";
+
+
 function script(prompt: string): Step[] {
   return [
     ["task.started", { agent: "claude-code", model: "fake", prompt }],
@@ -46,7 +49,12 @@ export function createFakeRunner({ delayMs = 300 }: FakeRunnerOptions = {}): Run
       store.get(sessionId).lastSeq = seq.last;
       yield event;
     }
-    if (!stopped.has(sessionId)) store.transition(sessionId, "awaiting_approval");
+
+    if (stopped.has(sessionId)) return;
+    store.transition(sessionId, "awaiting_approval");
+    const event = seq.next("approval.requested", { diffStat: FAKE_STAT });
+    store.get(sessionId).lastSeq = seq.last;
+    yield event;
   }
 
   return {
@@ -72,7 +80,7 @@ export function createFakeRunner({ delayMs = 300 }: FakeRunnerOptions = {}): Run
       store.get(sessionId);
       return {
         patch: "--- a/src/date.js\n+++ b/src/date.js\n@@ -1 +1 @@\n-  return d.getDate();\n+  return d.getUTCDate();\n",
-        stat: " src/date.js | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n",
+        stat: FAKE_STAT
       };
     },
 

runner/src/runner.ts

@@ -125,6 +125,7 @@ export function createRunner({
     }
 
     // 5. Store the change and put its summary on task.finished, so the phone shows it right away.
+    //    Then ask for approval: the daemon shows Approve / Reject on approval.requested.
     try {
       record.diff = await diffSince(worktreePath, baseCommit);
       finished.payload.diffStat = record.diff.stat;
@@ -133,6 +134,7 @@ export function createRunner({
     }
     store.transition(sessionId, "awaiting_approval");
     yield emit(finished);
+    yield emit(seq.next("approval.requested", { diffStat: record.diff?.stat ?? "" }));
   }
 
   // Everything below needs the worktree that start() made.

runner/test/fake-runner.test.ts

@@ -15,7 +15,7 @@ describe("fake runner", () => {
     const runner = createFakeRunner({ delayMs: 0 });
     const events = await collect(runner.start(opts));
     expect(events[0]!.type).toBe("task.started");
-    expect(events.at(-1)!.type).toBe("task.finished");
+    expect(events.map((e) => e.type).slice(-2)).toEqual(["task.finished", "approval.requested"]);
     expect(events.map((e) => e.seq)).toEqual(events.map((_, i) => i + 1));
   });
 
@@ -38,6 +38,16 @@ describe("fake runner", () => {
     expect(last.type).toBe("task.stopped");
     const resumed = await collect(runner.resume(opts.sessionId));
     expect(resumed[0]!.seq).toBe(last.seq + 1);
-    expect(resumed.at(-1)!.type).toBe("task.finished");
+    expect(resumed.map((e) => e.type).slice(-2)).toEqual(["task.finished", "approval.requested"]);
+  });
+
+  it("does not ask for approval when stopped", async () => {
+    const runner = createFakeRunner({ delayMs: 0 });
+    const it1 = runner.start(opts)[Symbol.asyncIterator]();
+    await it1.next();
+    await runner.stop(opts.sessionId);
+    const rest: RunnerEvent[] = [];
+    for (let r = await it1.next(); !r.done; r = await it1.next()) rest.push(r.value);
+    expect(rest.map((e) => e.type)).toEqual(["task.stopped"]);
   });
 });

runner/test/runner.test.ts

@@ -59,8 +59,8 @@ describe("runner.start", () => {
     const events = await collect(o, { agentCommand: replayAgent(spike) });
 
     expect(events[0]!.type).toBe("task.started");
-    expect(events.at(-1)!.type).toBe("task.finished");
-    expect(events).toHaveLength(17);
+    expect(events.map((e) => e.type).slice(-2)).toEqual(["task.finished", "approval.requested"]);
+    expect(events).toHaveLength(18);
     expect(events.map((e) => e.seq)).toEqual(events.map((_, i) => i + 1));
     expect(events.every((e) => e.sessionId === o.sessionId)).toBe(true);
 
@@ -116,8 +116,10 @@ describe("runner diff, approve and reject", () => {
 
   it("puts the diff summary on task.finished and returns the full patch, new files included", async () => {
     const { runner, o, events } = await finishedRun();
-    expect(events.at(-1)!.type).toBe("task.finished");
-    expect(events.at(-1)!.payload.diffStat).toMatch(/2 files changed/);
+    const finished = events.at(-2)!;
+    expect(finished.type).toBe("task.finished");
+    expect(finished.payload.diffStat).toMatch(/2 files changed/);
+    expect(events.at(-1)).toMatchObject({ type: "approval.requested", payload: { diffStat: finished.payload.diffStat } });
     const { patch, stat } = await runner.diff(o.sessionId);
     expect(patch).toContain("+world");
     expect(patch).toContain("new.txt");
@@ -229,8 +231,8 @@ describe("runner.resume", () => {
     const { repo, o, runner, events } = await stoppedRun({ resumeCommand: resumeWith("edit") });
     const resumed = await drain(runner.resume(o.sessionId));
 
-    expect(resumed.at(-1)!.type).toBe("task.finished");
-    expect(resumed.at(-1)!.payload.diffStat).toMatch(/2 files changed/);
+    expect(resumed.map((e) => e.type).slice(-2)).toEqual(["task.finished", "approval.requested"]);
+    expect(resumed.at(-2)!.payload.diffStat).toMatch(/2 files changed/);
     expect([...events, ...resumed].map((e) => e.seq)).toEqual([...events, ...resumed].map((_, i) => i + 1));
 
     const { branch } = await runner.approve(o.sessionId, "Fix the failing test");