Skip to content

Commit 5ed9204

Browse files
committed
dreamer + git-commits: three UX fixes from live triage
1. Git-commit sweeps park structurally non-indexable directories (not a git repo, or a repo with no commits) on a 24h re-probe cooldown instead of retrying and logging the full git error every 15-minute tick. Two such projects were flooding a user's recent-errors log, drowning real errors in doctor reports. A plain directory that gets git-init'ed (or an empty repo that gains its first commit) is picked up at the next re-probe. 2. Manual /ctx-dream runs wait up to 60s for a busy domain lease. The lease holder is usually a scheduled catch-up task on the same domain finishing within seconds; giving up instantly turned an explicit user command into a confusing 'busy, try again'. Scheduled ticks still never wait. 3. The busy message no longer claims the requested task is 'already running': the domain lease is usually held by a SIBLING task (a scheduled verify blocking a manual curate). Both harnesses now say another dream task holds the domain's lease and suggest retrying.
1 parent 7ca26e1 commit 5ed9204

12 files changed

Lines changed: 269 additions & 23 deletions

File tree

packages/pi-plugin/src/commands/ctx-dream.ts

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -103,7 +103,10 @@ export function registerCtxDreamCommand(
103103
lines.push(`Skipped (no work): ${result.skippedNoWork.join(", ")}`);
104104
if (result.deferredBusy.length > 0)
105105
lines.push(
106-
`Busy (already running): ${result.deferredBusy.join(", ")}`,
106+
// "Busy" means the task's DOMAIN lease is held — usually
107+
// a sibling task (e.g. a scheduled verify blocking a
108+
// manual curate), not this task itself.
109+
`Busy: ${result.deferredBusy.join(", ")} — another dream task holds this domain's lease; retry in a minute`,
107110
);
108111
if (lines.length === 0) lines.push("No enabled dream tasks to run.");
109112

packages/pi-plugin/src/pi-todo-inject.test.ts

Lines changed: 3 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -174,10 +174,7 @@ describe("injectSyntheticTodowriteForPi", () => {
174174

175175
// Defer pass: the pair must NOT attach to the aborted anchor (skip
176176
// silently, same as an anchor outside the visible window).
177-
const deferMessages = [
178-
aborted,
179-
good,
180-
] as unknown as Parameters<
177+
const deferMessages = [aborted, good] as unknown as Parameters<
181178
typeof injectSyntheticTodowriteForPi
182179
>[0]["messages"];
183180
injectSyntheticTodowriteForPi({
@@ -200,11 +197,7 @@ describe("injectSyntheticTodowriteForPi", () => {
200197
stopReason: "error",
201198
timestamp: 3,
202199
};
203-
const bustMessages = [
204-
aborted,
205-
good,
206-
errored,
207-
] as unknown as Parameters<
200+
const bustMessages = [aborted, good, errored] as unknown as Parameters<
208201
typeof injectSyntheticTodowriteForPi
209202
>[0]["messages"];
210203
injectSyntheticTodowriteForPi({
@@ -218,8 +211,7 @@ describe("injectSyntheticTodowriteForPi", () => {
218211
expect(findOrphanedFunctionCallOutputs(bustMessages)).toEqual([]);
219212
const anchor = getPersistedTodoSyntheticAnchor(db, sessionId);
220213
expect(anchor?.messageId).toBe("resp_good");
221-
const goodContent = (good as { content: Array<{ id?: string }> })
222-
.content;
214+
const goodContent = (good as { content: Array<{ id?: string }> }).content;
223215
expect(goodContent.some((b) => b.id === callId)).toBe(true);
224216
} finally {
225217
closeQuietly(db);

packages/plugin/src/features/magic-context/dreamer/task-scheduler.test.ts

Lines changed: 63 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -56,6 +56,69 @@ function seedActiveMemory(d: Database, project = PROJECT): void {
5656
});
5757
}
5858

59+
describe("task-scheduler — manual lease wait", () => {
60+
it("manual run waits for a briefly-held domain lease instead of reporting busy", async () => {
61+
db = freshDb();
62+
seedActiveMemory(db);
63+
const leaseKey = leaseKeyFor("curate", PROJECT);
64+
const otherHolder = "other-process";
65+
expect(acquireLease(db, otherHolder, leaseKey)).toBe(true);
66+
// Free the lease shortly after the manual run starts waiting.
67+
setTimeout(() => releaseLease(db as Database, otherHolder, leaseKey), 150);
68+
69+
let executed = 0;
70+
const executor = async (): Promise<TaskExecOutcome> => {
71+
executed += 1;
72+
return { status: "completed" };
73+
};
74+
const result = await runManualDream({
75+
db,
76+
projectIdentity: PROJECT,
77+
tasks: [cfg("curate", "0 4 * * 0")],
78+
executor,
79+
task: "curate",
80+
});
81+
expect(executed).toBe(1);
82+
expect(result.ran).toEqual(["curate"]);
83+
expect(result.deferredBusy).toEqual([]);
84+
});
85+
86+
it("scheduled ticks do not wait on a busy lease", async () => {
87+
db = freshDb();
88+
seedActiveMemory(db);
89+
const leaseKey = leaseKeyFor("curate", PROJECT);
90+
expect(acquireLease(db, "other-process", leaseKey)).toBe(true);
91+
92+
const now = Date.now();
93+
writeTaskScheduleState(db, {
94+
projectPath: PROJECT,
95+
task: "curate",
96+
lastRunAt: null,
97+
nextDueAt: now - 1000,
98+
schedule: "0 4 * * 0",
99+
lastStatus: null,
100+
lastError: null,
101+
retryCount: 0,
102+
});
103+
let executed = 0;
104+
const executor = async (): Promise<TaskExecOutcome> => {
105+
executed += 1;
106+
return { status: "completed" };
107+
};
108+
const started = Date.now();
109+
await runDueTasksForProject({
110+
db,
111+
projectIdentity: PROJECT,
112+
tasks: [cfg("curate", "0 4 * * 0")],
113+
executor,
114+
now,
115+
});
116+
expect(executed).toBe(0);
117+
// No lease-wait loop on the scheduled path: returns immediately.
118+
expect(Date.now() - started).toBeLessThan(1500);
119+
});
120+
});
121+
59122
describe("task-scheduler — planDueTasks", () => {
60123
it("first-seed does NOT fire immediately (next_due in the future)", () => {
61124
db = freshDb();

packages/plugin/src/features/magic-context/dreamer/task-scheduler.ts

Lines changed: 25 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -256,11 +256,21 @@ function recordTransientFailure(
256256
interface DomainGroupCallbacks {
257257
/** Manual single-task run ignores the post-lease activity gate re-check. */
258258
forceGate?: boolean;
259+
/**
260+
* How long a manual run may wait for a busy domain lease before giving
261+
* up. Scheduled ticks leave this unset (the next tick retries anyway).
262+
*/
263+
leaseWaitMs?: number;
259264
onRan?: (task: DreamTaskName) => void;
260265
onFailed?: (task: DreamTaskName) => void;
261266
onBusy?: (task: DreamTaskName) => void;
262267
}
263268

269+
/** Poll cadence while a manual run waits for a busy domain lease. */
270+
const LEASE_WAIT_POLL_MS = 2_000;
271+
/** Lease wait budget for manual /ctx-dream runs. */
272+
export const MANUAL_RUN_LEASE_WAIT_MS = 60_000;
273+
264274
async function runDomainGroup(
265275
deps: RunDueTasksDeps,
266276
group: DueTask[],
@@ -271,7 +281,20 @@ async function runDomainGroup(
271281
const leaseKey = leaseKeyFor(group[0].config.task, projectIdentity);
272282
const holderId = crypto.randomUUID();
273283

274-
if (!acquireLease(db, holderId, leaseKey)) {
284+
let acquired = acquireLease(db, holderId, leaseKey);
285+
if (!acquired && cb?.leaseWaitMs) {
286+
// Explicit manual run: the lease holder is usually a scheduled
287+
// catch-up task on the same domain finishing within seconds. Waiting
288+
// briefly turns a confusing "busy, try again" into the run the user
289+
// asked for. Scheduled ticks never wait (leaseWaitMs unset) — the next
290+
// tick retries anyway.
291+
const deadline = Date.now() + cb.leaseWaitMs;
292+
while (!acquired && Date.now() < deadline) {
293+
await new Promise((resolve) => setTimeout(resolve, LEASE_WAIT_POLL_MS));
294+
acquired = acquireLease(db, holderId, leaseKey);
295+
}
296+
}
297+
if (!acquired) {
275298
// Busy (a long sibling run or another process holds it). Leave next_due_at
276299
// unchanged so these tasks re-attempt next tick — they run the instant the
277300
// lease frees. No state write.
@@ -430,6 +453,7 @@ export async function runManualDream(
430453
[...groups.values()].map((group) =>
431454
runDomainGroup({ ...deps, executor: deps.executor }, group, {
432455
forceGate,
456+
leaseWaitMs: MANUAL_RUN_LEASE_WAIT_MS,
433457
onRan: (t) => result.ran.push(t),
434458
onFailed: (t) => result.failed.push(t),
435459
onBusy: (t) => result.deferredBusy.push(t),

packages/plugin/src/features/magic-context/git-commits/git-log-reader.test.ts

Lines changed: 26 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
import { describe, expect, it } from "bun:test";
2-
import { parseGitLogOutput, readGitCommits } from "./git-log-reader";
2+
import { classifyGitLogFailure, parseGitLogOutput, readGitCommits } from "./git-log-reader";
33

44
// Field separator is US (0x1f, ASCII Unit Separator). We deliberately moved
55
// off NUL (0x00) because Node's child_process.execFile rejects argv elements
@@ -75,3 +75,28 @@ describe("readGitCommits (smoke)", () => {
7575
expect(Array.isArray(commits)).toBe(true);
7676
});
7777
});
78+
79+
describe("classifyGitLogFailure", () => {
80+
it("classifies structural failures that cannot succeed on retry", () => {
81+
expect(
82+
classifyGitLogFailure(
83+
"Command failed: git log HEAD\nfatal: not a git repository (or any of the parent directories): .git",
84+
),
85+
).toBe("not_a_repo");
86+
expect(
87+
classifyGitLogFailure(
88+
"Command failed: git log HEAD\nfatal: ambiguous argument 'HEAD': unknown revision or path not in the working tree.",
89+
),
90+
).toBe("no_head");
91+
expect(
92+
classifyGitLogFailure(
93+
"fatal: your current branch 'main' does not have any commits yet",
94+
),
95+
).toBe("no_head");
96+
});
97+
98+
it("keeps everything else transient so normal retries continue", () => {
99+
expect(classifyGitLogFailure("spawn git ENOENT")).toBe("transient");
100+
expect(classifyGitLogFailure("Command failed: git log HEAD (timeout)")).toBe("transient");
101+
});
102+
});

packages/plugin/src/features/magic-context/git-commits/git-log-reader.ts

Lines changed: 52 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,28 @@ const DEFAULT_MAX_COMMITS = 5000;
3939
const RECORD_SEPARATOR = "\x1e";
4040
const FIELD_SEPARATOR = "\x1f";
4141

42+
/**
43+
* Why `git log` produced no commits, when it failed. `not_a_repo` and
44+
* `no_head` (a repo with zero commits) are structural: retrying every sweep
45+
* tick cannot succeed and only floods the log, so callers put the project on
46+
* a long re-probe cooldown instead. `transient` covers everything else
47+
* (timeouts, missing git binary, permission errors) and keeps the normal
48+
* retry cadence.
49+
*/
50+
export type GitLogFailureKind = "not_a_repo" | "no_head" | "transient";
51+
52+
export function classifyGitLogFailure(message: string): GitLogFailureKind {
53+
if (message.includes("not a git repository")) return "not_a_repo";
54+
if (
55+
message.includes("unknown revision or path not in the working tree") ||
56+
message.includes("does not have any commits yet") ||
57+
message.includes("bad revision")
58+
) {
59+
return "no_head";
60+
}
61+
return "transient";
62+
}
63+
4264
export interface GitCommit {
4365
/** Full 40-char SHA. */
4466
sha: string;
@@ -80,6 +102,19 @@ export async function readGitCommits(
80102
directory: string,
81103
options: ReadGitCommitsOptions = {},
82104
): Promise<GitCommit[]> {
105+
return (await readGitCommitsResult(directory, options)).commits;
106+
}
107+
108+
/**
109+
* Like {@link readGitCommits}, but also reports WHY the read failed so the
110+
* sweep can distinguish structurally non-indexable directories (not a repo,
111+
* repo with no commits) from transient errors. `failure` is null on success —
112+
* including a successful read that matched zero commits.
113+
*/
114+
export async function readGitCommitsResult(
115+
directory: string,
116+
options: ReadGitCommitsOptions = {},
117+
): Promise<{ commits: GitCommit[]; failure: GitLogFailureKind | null }> {
83118
// Guard against argument injection: a `branch` value beginning with `-`
84119
// would be parsed as a git OPTION (not a revision) since it sits ahead of
85120
// the format/since flags below. No shell is involved (execFile), so this
@@ -124,12 +159,23 @@ export async function readGitCommits(
124159
} catch (error) {
125160
// Intentional: git may not be installed, directory may not be a repo,
126161
// or the invocation may time out. All are "skip indexing this cycle"
127-
// conditions, not crashes. We return empty and the next sweep will
128-
// retry. We DO log the reason though — a silent empty-result masked a
129-
// real cwd / PATH / timeout bug during the v0.14 git-commits rollout.
162+
// conditions, not crashes. We return empty; transient failures retry
163+
// next sweep, structural ones (classified below) go on a long cooldown.
164+
// We DO log the reason though — a silent empty-result masked a real
165+
// cwd / PATH / timeout bug during the v0.14 git-commits rollout.
130166
const message = error instanceof Error ? error.message : String(error);
131-
log(`[git-commits] readGitCommits failed for ${projectLabel}: ${message.slice(0, 500)}`);
132-
return [];
167+
const failure = classifyGitLogFailure(message);
168+
if (failure === "transient") {
169+
log(
170+
`[git-commits] readGitCommits failed for ${projectLabel}: ${message.slice(0, 500)}`,
171+
);
172+
} else {
173+
// One quiet line instead of the full multi-line git error: these
174+
// directories fail the same way every sweep and were flooding the
175+
// recent-errors section of doctor reports.
176+
log(`[git-commits] ${projectLabel} is not indexable (${failure})`);
177+
}
178+
return { commits: [], failure };
133179
}
134180

135181
if (stdout.trim().length === 0) {
@@ -138,7 +184,7 @@ export async function readGitCommits(
138184
);
139185
}
140186

141-
return parseGitLogOutput(stdout);
187+
return { commits: parseGitLogOutput(stdout), failure: null };
142188
}
143189

144190
export function parseGitLogOutput(stdout: string): GitCommit[] {

packages/plugin/src/features/magic-context/git-commits/index.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,7 @@ export {
3838
type GitSweepLeaseResult,
3939
getGitSweepCoordinatorState,
4040
markGitSweepSuccessAndRelease,
41+
parkGitSweepNonIndexable,
4142
releaseGitSweepLease,
4243
renewGitSweepLease,
4344
} from "./sweep-coordinator";

packages/plugin/src/features/magic-context/git-commits/indexer.ts

Lines changed: 15 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@
1313
import { log } from "../../../shared/logger";
1414
import type { Database } from "../../../shared/sqlite";
1515
import { embedBatchForProject, getProjectEmbeddingSnapshot } from "../memory/embedding";
16-
import { readGitCommits } from "./git-log-reader";
16+
import { readGitCommits, readGitCommitsResult } from "./git-log-reader";
1717
import {
1818
countEmbeddedCommits,
1919
loadUnembeddedCommits,
@@ -49,6 +49,12 @@ export interface IndexCommitsResult {
4949
updated: number;
5050
evicted: number;
5151
embedded: number;
52+
/**
53+
* Set when `git log` failed structurally (directory is not a repo, or the
54+
* repo has no commits yet). The sweep uses this to park the project on a
55+
* long re-probe cooldown instead of retrying every tick.
56+
*/
57+
nonIndexable: boolean;
5258
}
5359

5460
/**
@@ -70,6 +76,7 @@ export async function indexCommitsForProject(
7076
updated: 0,
7177
evicted: 0,
7278
embedded: 0,
79+
nonIndexable: false,
7380
};
7481

7582
if (indexInProgress.has(projectPath)) {
@@ -88,13 +95,19 @@ export async function indexCommitsForProject(
8895
Math.max(latestIndexed - 60_000, Date.now() - options.sinceDays * MS_PER_DAY)
8996
: Date.now() - options.sinceDays * MS_PER_DAY;
9097

91-
const commits = await readGitCommits(directory, {
98+
const read = await readGitCommitsResult(directory, {
9299
sinceMs,
93100
maxCommits: options.maxCommits,
94101
projectIdentity: projectPath,
95102
});
103+
const commits = read.commits;
96104
result.scanned = commits.length;
97105

106+
if (read.failure === "not_a_repo" || read.failure === "no_head") {
107+
result.nonIndexable = true;
108+
return result;
109+
}
110+
98111
if (commits.length === 0) {
99112
// No new commits. Still enforce the cap in case prior runs overflowed.
100113
result.evicted = enforceProjectCap(db, projectPath, options.maxCommits);

packages/plugin/src/features/magic-context/git-commits/sweep-coordinator.test.ts

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ import {
99
GIT_SWEEP_COOLDOWN_MS,
1010
getGitSweepCoordinatorState,
1111
markGitSweepSuccessAndRelease,
12+
parkGitSweepNonIndexable,
1213
releaseGitSweepLease,
1314
} from "./sweep-coordinator";
1415

@@ -155,4 +156,28 @@ describe("git sweep coordinator", () => {
155156
expect(retry.acquired).toBe(true);
156157
expect(getGitSweepCoordinatorState(db, projectPath)?.leaseHolder).toBe("holder-b");
157158
});
159+
160+
it("parks a non-indexable project on the long re-probe cooldown", () => {
161+
const PROJECT = "dir:non-indexable";
162+
const holder = "holder-park";
163+
const first = acquireGitSweepLease(db, PROJECT, holder);
164+
expect(first.acquired).toBe(true);
165+
166+
expect(parkGitSweepNonIndexable(db, PROJECT, holder)).toBe(true);
167+
168+
// Immediately after parking: cooldown blocks, far in the future.
169+
const blocked = acquireGitSweepLease(db, PROJECT, "holder-2");
170+
expect(blocked.acquired).toBe(false);
171+
if (!blocked.acquired) {
172+
expect(blocked.reason).toBe("cooldown_active");
173+
// Re-probe horizon is ~24h out, well past the ordinary 10m cooldown.
174+
expect((blocked.nextAllowedAt ?? 0) - Date.now()).toBeGreaterThan(60 * 60 * 1000);
175+
}
176+
177+
// A short custom horizon expires and allows the re-probe.
178+
const holder3 = "holder-3";
179+
const again = acquireGitSweepLease(db, PROJECT, holder3);
180+
expect(again.acquired).toBe(false);
181+
expect(parkGitSweepNonIndexable(db, PROJECT, holder3, 1)).toBe(false);
182+
});
158183
});

0 commit comments

Comments
 (0)