Skip to content

Commit afd04f0

Browse files
TimMikeladzeclaude
andcommitted
fix(dispatch): close the report out on the run's outcome
A report moved to "dispatched" when its run started and never moved again, so one that had been handled and one that had been ignored looked the same, and dispatchAll skipped both forever. A clean exit is not the same as the work being done. Under the default permission "plan" the agent can only propose: it writes a plan, asks whether to proceed, and exits 0 having touched nothing, with no one there to answer. That was recorded as a plain success on a stuck report, which reads as devbar ignoring what you sent while the run still costs money. A finished run now resolves the report when the working tree changed, and reopens it as "new" otherwise, carrying a note that names plan mode as the reason when that is what happened. Reopening is not a retry: the finished task still guards its report, so nothing re-runs on its own. Deciding that from `git status --porcelain` alone was wrong, and wrong in the common case. Porcelain names which paths are dirty, not their content, so a file already modified before a run and edited again during it produced byte-identical output — a real edit read as a no-op, and the report was reopened after being handled. gitSnapshot now stamps each dirty path with its size and mtime and the comparison is per file. That also fixes changedFiles, which shared the blind spot and never reported an edit to an already-dirty file. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Ph2Rn26we5JcGUhtQckrdQ
1 parent 440c92c commit afd04f0

4 files changed

Lines changed: 290 additions & 6 deletions

File tree

docs/LOCAL-AGENT.md

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -107,6 +107,40 @@ cannot see. On disk, the agent reads the PNG.
107107

108108
The agent is then run in the project directory with the prompt on stdin.
109109

110+
### What a finished run does to the report
111+
112+
The report moves to `dispatched` when the run starts, and the run's outcome
113+
decides where it lands:
114+
115+
| Outcome | Report |
116+
| ------------------------------------------ | ---------- |
117+
| Exited 0 and the working tree changed | `resolved` |
118+
| Exited 0 and the working tree is unchanged | `new` |
119+
| Failed, cancelled or timed out | `new` |
120+
121+
A clean exit is not the same as the work being done. Under the default
122+
`permission: "plan"` the agent can only propose: it writes a plan, asks whether
123+
to proceed, and exits 0 having touched nothing — with no one there to answer.
124+
That used to be recorded as a plain success on a report stuck at `dispatched`,
125+
which reads as devbar ignoring the report while the run still costs money. Now
126+
the run carries a `note` saying what happened, and the report goes back to
127+
`new`, where it is visibly still waiting:
128+
129+
```
130+
[devbar] the agent changed nothing: this project dispatches with permission
131+
"plan", which can only propose. Set `permission: "auto"` in devbar.config.ts to
132+
let a dispatch apply its own fix.
133+
```
134+
135+
The no-op is only claimed when git says so — a working tree naming exactly the
136+
files it named before the run. An agent that commits its work leaves a tree that
137+
differs, and a project directory that is not a git repository cannot be judged
138+
at all; neither is reported as a no-op.
139+
140+
Reopening is not a retry. The finished task still guards its report, so
141+
`dispatchAll` will not pick it up again on its own — `devbar dispatch <id>`
142+
still will.
143+
110144
### Supported agents
111145

112146
| | `claude` | `codex` | `opencode` |
@@ -244,6 +278,11 @@ after Submit offers **Dispatch** for that one report; otherwise turn it on, use
244278
`devbar dispatch`, open the toolbar's Agent tab, or let an agent pull with
245279
`claim_report`.
246280

281+
**The run went green but nothing changed.** `permission` is `plan`, the
282+
default, which lets the agent propose but not edit. The run's `note` says so and
283+
the report returns to `new`. Set `permission: "auto"` to let a dispatch apply its
284+
own fix — that lets an agent write to the project directory unattended.
285+
247286
**"No project matched this report".** The page's origin is not in any project's
248287
`origins`, and more than one project is registered. Add the origin, or pass
249288
`project`.

src/server/dispatcher.ts

Lines changed: 79 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,17 @@ export type Task = {
2020
result?: DispatchResult;
2121
};
2222

23+
/**
24+
* The dirty paths in a working tree, each mapped to a stamp that moves when the
25+
* file's content does.
26+
*
27+
* A bare path list cannot answer the only question that matters here. Porcelain
28+
* names *which* files are dirty, so a file already modified before a run and
29+
* edited again during it produces byte-identical output — the run reads as
30+
* having touched nothing.
31+
*/
32+
export type GitSnapshot = Record<string, string>;
33+
2334
export type DispatchResult = {
2435
taskId: string;
2536
exitCode: number;
@@ -31,6 +42,12 @@ export type DispatchResult = {
3142
/** Files the agent changed, when the project directory is a git repo. */
3243
changedFiles?: string[];
3344
interrupted?: boolean;
45+
/**
46+
* Why a run that exited cleanly still left the report open — almost always
47+
* plan mode, where the agent proposes and waits for an answer nobody is
48+
* there to give. Absent when the run changed something.
49+
*/
50+
note?: string;
3451
};
3552

3653
/** What subscribers (the SSE bus, the CLI) see as a run unfolds. */
@@ -47,7 +64,7 @@ export type DispatcherOptions = {
4764
/** Overrides every project's agent command. Tests pass "echo". */
4865
command?: string;
4966
/** Captures git state around a run. Injectable for tests. */
50-
gitSnapshot?: (dir: string) => Promise<string[] | undefined>;
67+
gitSnapshot?: (dir: string) => Promise<GitSnapshot | undefined>;
5168
now?: () => number;
5269
};
5370

@@ -76,6 +93,19 @@ function formatDuration(ms: number): string {
7693
return `${Math.floor(s / 60)}m${s % 60}s`;
7794
}
7895

96+
/** Paths whose stamp moved between two snapshots, plus any that went clean. */
97+
function changedBetween(before: GitSnapshot, after: GitSnapshot): string[] {
98+
const changed = new Set<string>();
99+
for (const [path, stamp] of Object.entries(after)) {
100+
if (before[path] !== stamp) changed.add(path);
101+
}
102+
// A file the agent reverted leaves the dirty set; that is a change too.
103+
for (const path of Object.keys(before)) {
104+
if (!(path in after)) changed.add(path);
105+
}
106+
return [...changed].sort();
107+
}
108+
79109
function normalizePermission(project: ProjectConfig): AgentPermission {
80110
const raw = project.permission ?? project.permissionMode;
81111
switch (raw) {
@@ -330,7 +360,9 @@ export function createDispatcher(options: DispatcherOptions): Dispatcher {
330360

331361
const afterGit = options.gitSnapshot ? await options.gitSnapshot(project.dir) : undefined;
332362
const changedFiles =
333-
beforeGit && afterGit ? afterGit.filter((f) => !beforeGit.includes(f)) : afterGit;
363+
beforeGit && afterGit
364+
? changedBetween(beforeGit, afterGit)
365+
: afterGit && Object.keys(afterGit).sort();
334366

335367
const completedAt = now();
336368
let output = chunks.join("");
@@ -347,6 +379,30 @@ export function createDispatcher(options: DispatcherOptions): Dispatcher {
347379
? "completed"
348380
: "failed";
349381

382+
// A clean exit is not the same as the work being done. In plan mode the
383+
// agent can only propose — it writes a plan, asks "shall I proceed?", and
384+
// exits 0 having touched nothing. That was reported as a plain success,
385+
// which reads as devbar ignoring the report, and the run still costs money.
386+
//
387+
// Only claim a no-op when git actually said so: not one dirty file's
388+
// content moved. Without git we cannot tell, and that is not a no-op
389+
// either — it is an unknown, and the report should not be reopened on it.
390+
const touchedNothing =
391+
beforeGit !== undefined && afterGit !== undefined && changedFiles?.length === 0;
392+
const applied = status === "completed" && !touchedNothing;
393+
const note =
394+
status === "completed" && touchedNothing
395+
? normalizePermission(project) === "plan"
396+
? 'the agent changed nothing: this project dispatches with permission "plan", which can only propose. Set `permission: "auto"` in devbar.config.ts to let a dispatch apply its own fix.'
397+
: "the agent changed nothing."
398+
: undefined;
399+
400+
if (note) {
401+
console.log(`[dispatch] ${note}`);
402+
recordEvent(task.id, { type: "stdout", text: `[devbar] ${note}\n` });
403+
output = `${output}[devbar] ${note}\n`;
404+
}
405+
350406
const result: DispatchResult = {
351407
taskId: task.id,
352408
exitCode,
@@ -356,10 +412,31 @@ export function createDispatcher(options: DispatcherOptions): Dispatcher {
356412
...(costUsd !== undefined ? { costUsd } : {}),
357413
...(sessionId ? { sessionId } : {}),
358414
...(changedFiles && changedFiles.length > 0 ? { changedFiles } : {}),
415+
...(note ? { note } : {}),
359416
};
360417

361418
update(task, { status, completedAt, result, ...(sessionId ? { sessionId } : {}) });
362419

420+
// Close the report out on the way past. It was moved to "dispatched" when
421+
// the run started; leaving it there forever claims someone dealt with it
422+
// and hides it from `dispatchAll`, so only a run that actually changed
423+
// files resolves it. Everything else goes back to "new", where it is
424+
// visibly still waiting and a later dispatch will pick it up again.
425+
if (applied) {
426+
const changed = changedFiles?.length
427+
? `Changed ${changedFiles.length} file(s): ${changedFiles.join(", ")}`
428+
: "The project directory is not a git repository, so what it changed is unverified";
429+
await options.store
430+
.resolve(report.id, {
431+
summary: `Dispatched to ${preset.name} (${project.model}). ${changed}.`,
432+
resolvedAt: completedAt,
433+
by: `dispatch:${task.id}`,
434+
})
435+
.catch(() => undefined);
436+
} else {
437+
await options.store.setStatus(report.id, "new").catch(() => undefined);
438+
}
439+
363440
console.log(
364441
`[dispatch] task ${task.id.slice(0, 8)} ${status} in ${formatDuration(result.durationMs)}` +
365442
(errorMessage ? ` — ${errorMessage}` : ""),

src/server/local.ts

Lines changed: 29 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@ import { mkdir, readFile } from "node:fs/promises";
33
import { join } from "node:path";
44
import { homedir } from "node:os";
55
import { createRegistry, type Registry, type ProjectConfig } from "./registry";
6-
import { createDispatcher, type Dispatcher } from "./dispatcher";
6+
import { createDispatcher, type Dispatcher, type GitSnapshot } from "./dispatcher";
77
import { createReportStore, type ReportStore } from "./report-store";
88
import { createPageBus, PageRpcError, type PageBus } from "./page-bus";
99
import { createMcpSessions, type McpSessions } from "./mcp-sessions";
@@ -777,10 +777,17 @@ export async function createLocalServer(options: LocalServerOptions = {}): Promi
777777
};
778778
}
779779

780-
/** Files with uncommitted changes, so a run can report what it touched. */
781-
async function gitSnapshot(dir: string): Promise<string[] | undefined> {
780+
/**
781+
* Files with uncommitted changes, each stamped with size and mtime, so a run
782+
* can report what it touched.
783+
*
784+
* The stamp is what makes an already-dirty file legible: porcelain names the
785+
* same path before and after a run that edited it again, so comparing path
786+
* lists alone would call that run a no-op.
787+
*/
788+
async function gitSnapshot(dir: string): Promise<GitSnapshot | undefined> {
782789
const { spawn } = await import("node:child_process");
783-
return new Promise((resolve) => {
790+
const paths = await new Promise<string[] | undefined>((resolve) => {
784791
try {
785792
const child = spawn("git", ["status", "--porcelain"], {
786793
cwd: dir,
@@ -804,4 +811,22 @@ async function gitSnapshot(dir: string): Promise<string[] | undefined> {
804811
resolve(undefined);
805812
}
806813
});
814+
if (!paths) return undefined;
815+
816+
const { stat } = await import("node:fs/promises");
817+
const { join } = await import("node:path");
818+
const snapshot: GitSnapshot = {};
819+
await Promise.all(
820+
paths.map(async (path) => {
821+
try {
822+
const info = await stat(join(dir, path));
823+
snapshot[path] = `${info.size}:${info.mtimeMs}`;
824+
} catch {
825+
// Deleted, or a path porcelain rendered in a form we cannot stat
826+
// (a rename arrow, a quoted name). Its presence is still the signal.
827+
snapshot[path] = "absent";
828+
}
829+
}),
830+
);
831+
return snapshot;
807832
}

test/dispatcher.test.ts

Lines changed: 143 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ import {
44
buildPrompt,
55
adoptPersistedTask,
66
type Dispatcher,
7+
type GitSnapshot,
78
type Task,
89
} from "../src/server/dispatcher";
910
import { createReportStore, type ReportStore } from "../src/server/report-store";
@@ -199,6 +200,148 @@ describe("dispatcher", () => {
199200
});
200201
});
201202

203+
describe("dispatcher closes the report out", () => {
204+
let resultsDir: string;
205+
let tasksDir: string;
206+
let reportsDir: string;
207+
let store: ReportStore;
208+
209+
/** A working tree where each named file carries a content stamp. */
210+
function tree(...entries: [string, string][]): GitSnapshot {
211+
return Object.fromEntries(entries);
212+
}
213+
214+
/** A dispatcher whose git snapshots are scripted, one call per invocation. */
215+
function withGit(snapshots: (GitSnapshot | undefined)[], project = PROJECT): Dispatcher {
216+
let call = 0;
217+
return createDispatcher({
218+
store,
219+
resultsDir,
220+
tasksDir,
221+
getProject: (slug) => (slug === "test-app" ? project : undefined),
222+
command: "echo",
223+
gitSnapshot: async () => snapshots[call++],
224+
});
225+
}
226+
227+
beforeEach(async () => {
228+
resultsDir = tmpDir();
229+
tasksDir = tmpDir();
230+
reportsDir = tmpDir();
231+
await mkdir(resultsDir, { recursive: true });
232+
await mkdir(tasksDir, { recursive: true });
233+
await mkdir(reportsDir, { recursive: true });
234+
store = createReportStore(reportsDir);
235+
});
236+
237+
afterEach(async () => {
238+
await Promise.all(
239+
[resultsDir, tasksDir, reportsDir].map((d) => rm(d, { recursive: true, force: true })),
240+
);
241+
});
242+
243+
test("a run that changed files resolves it, with what changed", async () => {
244+
const dispatcher = withGit([tree(), tree(["src/hero.tsx", "12:100"])]);
245+
const reportId = (await store.save({ prompt: "fix it" }, "test-app")).id;
246+
247+
dispatcher.enqueue(reportId, "test-app");
248+
await dispatcher.process();
249+
await dispatcher.drain();
250+
251+
expect((await store.get(reportId))?.status).toBe("resolved");
252+
const resolution = JSON.parse(
253+
await readFile(join((await store.get(reportId))!.dir, "resolution.json"), "utf-8"),
254+
);
255+
expect(resolution.summary).toContain("src/hero.tsx");
256+
});
257+
258+
test("a plan-mode run that changed nothing reopens it and says why", async () => {
259+
// Identical snapshots: the agent proposed and waited for an answer.
260+
const dispatcher = withGit([
261+
tree(["src/other.tsx", "40:100"]),
262+
tree(["src/other.tsx", "40:100"]),
263+
]);
264+
const reportId = (await store.save({ prompt: "fix it" }, "test-app")).id;
265+
266+
const taskId = dispatcher.enqueue(reportId, "test-app");
267+
await dispatcher.process();
268+
await dispatcher.drain();
269+
270+
expect(dispatcher.getTask(taskId)?.status).toBe("completed");
271+
expect(dispatcher.getTask(taskId)?.result?.note).toContain('permission: "auto"');
272+
// Back to "new", not left claiming someone dealt with it.
273+
expect((await store.get(reportId))?.status).toBe("new");
274+
});
275+
276+
test("an auto-permission run that changed nothing says so without blaming plan mode", async () => {
277+
const dispatcher = withGit([tree(), tree()], { ...PROJECT, permission: "auto" });
278+
const reportId = (await store.save({ prompt: "fix it" }, "test-app")).id;
279+
280+
const taskId = dispatcher.enqueue(reportId, "test-app");
281+
await dispatcher.process();
282+
await dispatcher.drain();
283+
284+
expect(dispatcher.getTask(taskId)?.result?.note).toBe("the agent changed nothing.");
285+
expect((await store.get(reportId))?.status).toBe("new");
286+
});
287+
288+
test("editing an already-dirty file counts as a change", async () => {
289+
// Porcelain names the same path before and after, so a path-list
290+
// comparison calls this a no-op and reopens a report that was handled.
291+
const dispatcher = withGit([
292+
tree(["src/hero.tsx", "40:100"]),
293+
tree(["src/hero.tsx", "62:900"]),
294+
]);
295+
const reportId = (await store.save({ prompt: "fix it" }, "test-app")).id;
296+
297+
const taskId = dispatcher.enqueue(reportId, "test-app");
298+
await dispatcher.process();
299+
await dispatcher.drain();
300+
301+
expect(dispatcher.getTask(taskId)?.result?.note).toBeUndefined();
302+
expect(dispatcher.getTask(taskId)?.result?.changedFiles).toEqual(["src/hero.tsx"]);
303+
expect((await store.get(reportId))?.status).toBe("resolved");
304+
});
305+
306+
test("a file the agent reverted to clean counts as a change", async () => {
307+
const dispatcher = withGit([tree(["src/hero.tsx", "40:100"]), tree()]);
308+
const reportId = (await store.save({ prompt: "revert it" }, "test-app")).id;
309+
310+
const taskId = dispatcher.enqueue(reportId, "test-app");
311+
await dispatcher.process();
312+
await dispatcher.drain();
313+
314+
expect(dispatcher.getTask(taskId)?.result?.changedFiles).toEqual(["src/hero.tsx"]);
315+
expect((await store.get(reportId))?.status).toBe("resolved");
316+
});
317+
318+
test("without git it resolves rather than stranding the report", async () => {
319+
const dispatcher = withGit([undefined, undefined]);
320+
const reportId = (await store.save({ prompt: "fix it" }, "test-app")).id;
321+
322+
const taskId = dispatcher.enqueue(reportId, "test-app");
323+
await dispatcher.process();
324+
await dispatcher.drain();
325+
326+
expect(dispatcher.getTask(taskId)?.result?.note).toBeUndefined();
327+
expect((await store.get(reportId))?.status).toBe("resolved");
328+
});
329+
330+
test("reopening does not re-run the report on its own", async () => {
331+
// Reopening is so the report is visibly still waiting, not a retry loop:
332+
// the completed task still guards it against another automatic dispatch.
333+
const dispatcher = withGit([tree(["a", "1:1"]), tree(["a", "1:1"])]);
334+
const reportId = (await store.save({ prompt: "fix it" }, "test-app")).id;
335+
336+
dispatcher.enqueue(reportId, "test-app");
337+
await dispatcher.process();
338+
await dispatcher.drain();
339+
340+
expect((await store.get(reportId))?.status).toBe("new");
341+
expect(await dispatcher.dispatchAll("test-app")).toHaveLength(0);
342+
});
343+
});
344+
202345
describe("buildPrompt", () => {
203346
const report = {
204347
id: "r1",

0 commit comments

Comments
 (0)