diff --git a/apps/server/src/gits/Layers/AutomodeDriver.test.ts b/apps/server/src/gits/Layers/AutomodeDriver.test.ts index 2c164833df4f..3add86b7b87b 100644 --- a/apps/server/src/gits/Layers/AutomodeDriver.test.ts +++ b/apps/server/src/gits/Layers/AutomodeDriver.test.ts @@ -460,6 +460,31 @@ describe("AutomodeDriver", () => { }).pipe(Effect.provide(makeLayer(peerStatus))); }); + // Head-of-line blocking guard: notBefore parks ONE goal, not the whole queue. A goal + // scheduled for tonight sitting at the head must not stall work that is due now. + it.effect("dispatches a due goal queued behind a not-yet-due one", () => { + const peerStatus = { current: "absent" as PeerStatus | "absent" }; + return Effect.gen(function* () { + const supervisor = yield* AutomodeSupervisor; + const driver = yield* AutomodeDriver; + yield* armAutonomous(supervisor); + // Enqueued first, so it is the oldest and wins the FIFO pick — but it is not due. + yield* supervisor.enqueueGoal({ + title: "Tonight", + repo: "/tmp/source-repo", + prompt: "later", + notBefore: "2099-01-01T00:00:00.000Z", + }); + yield* supervisor.enqueueGoal({ title: "Now", repo: "/tmp/source-repo", prompt: "now" }); + + yield* driver.tickOnce(); + + const goals = (yield* supervisor.getSnapshot()).goals; + assert.equal(goals.find((goal) => goal.title === "Now")?.status, "running"); + assert.equal(goals.find((goal) => goal.title === "Tonight")?.status, "queued"); + }).pipe(Effect.provide(makeLayer(peerStatus))); + }); + it.effect("does not dispatch a second goal while one is running (sequential)", () => { const peerStatus = { current: "absent" as PeerStatus | "absent" }; return Effect.gen(function* () { diff --git a/apps/server/src/gits/Layers/AutomodeDriver.ts b/apps/server/src/gits/Layers/AutomodeDriver.ts index d4d2bdf336b6..f76071f5e09e 100644 --- a/apps/server/src/gits/Layers/AutomodeDriver.ts +++ b/apps/server/src/gits/Layers/AutomodeDriver.ts @@ -72,12 +72,28 @@ export function merge_verify_commands( const TERMINAL_FAIL_STATUSES = new Set(["failed", "frozen", "killed", "halted"]); const TERMINAL_DONE_STATUSES = new Set(["done", "completed"]); -function oldestQueued(goals: ReadonlyArray): AutomodeGoal | null { +/** + * A goal's operator-chosen not-before time. Unparseable input is treated as due now rather + * than parking the goal forever — the value is schema-validated at the RPC boundary, so a + * bad one here means corrupted state, and stalling silently is the worse failure. + */ +function isDue(goal: AutomodeGoal, nowMillis: number): boolean { + if (goal.notBefore === null) { + return true; + } + const at = Date.parse(goal.notBefore); + return Number.isNaN(at) || at <= nowMillis; +} + +function oldestQueued(goals: ReadonlyArray, nowMillis: number): AutomodeGoal | null { // The snapshot sorts goals newest-first; reverse before sorting so that // equal-timestamp goals remain in oldest-first (FIFO) order. + // + // notBefore is filtered HERE, not checked on the winner: parking the head of the queue + // would stall every due goal behind it until the scheduled one fired. const queued = goals .toReversed() - .filter((goal) => goal.status === "queued") + .filter((goal) => goal.status === "queued" && isDue(goal, nowMillis)) .sort((left, right) => left.createdAt.localeCompare(right.createdAt)); return queued[0] ?? null; } @@ -580,18 +596,11 @@ export const AutomodeDriverLive = Layer.effect( } // 3) Queue drained → held-PR lifecycle only. - const next = oldestQueued(snapshot.goals); + const next = oldestQueued(snapshot.goals, yield* Clock.currentTimeMillis); if (next === null) { yield* maintainHeldPr(snapshot); return; } - if ( - next.notBefore !== null && - Date.parse(next.notBefore) > (yield* Clock.currentTimeMillis) - ) { - yield* maintainHeldPr(snapshot); - return; - } // Scheduler start gate (decisions 6/7/11/20): a deny leaves the goal queued for a // later tick — quiet, NOT a halt. Manual RPC dispatch stays ungated (human-driven). const gate = yield* scheduler diff --git a/apps/server/src/gits/Layers/HermesCliAdapter.test.ts b/apps/server/src/gits/Layers/HermesCliAdapter.test.ts index 7e2db8e71dfc..99df6c9442e6 100644 --- a/apps/server/src/gits/Layers/HermesCliAdapter.test.ts +++ b/apps/server/src/gits/Layers/HermesCliAdapter.test.ts @@ -34,6 +34,7 @@ import { makeHermesEnv, makeProposal, normalizeProposal, + citedSourceThreadId, summarizeProposal, parseCodexChainHealth, parseHermesModelStatus, @@ -804,6 +805,27 @@ describe("HermesCliAdapter chat preflight", () => { }); }); +describe("citedSourceThreadId", () => { + it("claims the thread only when Hermes cited it", () => { + expect( + citedSourceThreadId("Continuing thread-9: finish the retry migration.", "thread-9"), + ).toBe("thread-9"); + }); + + // The sweep offers a continuation as a hint; Hermes may ignore it and propose something + // unrelated. Stamping the id anyway would persist a source thread the card never touched. + it("drops the claim when the proposal ignored the candidate", () => { + expect( + citedSourceThreadId("Add a missing index to the projections table.", "thread-9"), + ).toBeNull(); + }); + + it("is null when no candidate was offered", () => { + expect(citedSourceThreadId("Anything at all.", undefined)).toBeNull(); + expect(citedSourceThreadId("Anything at all.", " ")).toBeNull(); + }); +}); + describe("HermesCliAdapter proposal helpers", () => { it("summarizeProposal skips hermes chrome lines and uses the first real line as title", () => { const summarized = summarizeProposal( diff --git a/apps/server/src/gits/Layers/HermesCliAdapter.ts b/apps/server/src/gits/Layers/HermesCliAdapter.ts index a54f657dacb9..63700f3329c8 100644 --- a/apps/server/src/gits/Layers/HermesCliAdapter.ts +++ b/apps/server/src/gits/Layers/HermesCliAdapter.ts @@ -1086,6 +1086,24 @@ function nullableStringFromUnknown(value: unknown): string | null { return typeof value === "string" && value.trim().length > 0 ? value.trim() : null; } +/** + * Continuation provenance: a card may claim to continue a past thread ONLY when Hermes + * actually cited that thread in its output. The sweep offers a continuation candidate as a + * hint ("propose the smallest safe continuation if it is still relevant"), and Hermes is + * free to ignore it and propose something else entirely — stamping the id regardless would + * record a source thread the card has nothing to do with, and that lie is durable: it is + * persisted on the card and carried into the goal. + */ +export function citedSourceThreadId( + detail: string, + candidateThreadId: string | undefined, +): string | null { + if (candidateThreadId === undefined || candidateThreadId.trim().length === 0) { + return null; + } + return detail.includes(candidateThreadId) ? candidateThreadId : null; +} + export function normalizeProposal(value: unknown): HermesProposalCard | null { if (typeof value !== "object" || value === null) { return null; @@ -1961,7 +1979,8 @@ const makeInspectGitsAndPropose = : `Hermes Codex OAuth chain requires re-login. Run \`${preflight.command}\`.`, source: "hermes chat -q", projectDir: input.projectDir, - ...(input.sourceThreadId === undefined ? {} : { sourceThreadId: input.sourceThreadId }), + // No sourceThreadId: Hermes was never spawned, so this auth-failure card + // continues nothing — claiming a source thread here is pure mislabeling. now, evidence: ["GITS preflight blocked the Hermes spawn before execution.", preflight.reason], }); @@ -2001,6 +2020,7 @@ const makeInspectGitsAndPropose = const detail = nonEmpty(exec.stdout) ?? nonEmpty(exec.stderr) ?? "Hermes returned no proposal."; const summary = summarizeProposal(detail); + const citedThreadId = citedSourceThreadId(detail, input.sourceThreadId); const proposal = makeProposal({ title: summary.title, summary: summary.summary, @@ -2013,11 +2033,12 @@ const makeInspectGitsAndPropose = blockedReason: exec.exitCode === 0 ? null : "Hermes proposal command did not complete.", source: "hermes chat -q", projectDir: input.projectDir, - ...(input.sourceThreadId === undefined ? {} : { sourceThreadId: input.sourceThreadId }), + ...(citedThreadId === null ? {} : { sourceThreadId: citedThreadId }), now, evidence: [ "Hermes was invoked in read-only inspection mode.", `Hermes exit code: ${exec.exitCode === null ? "unknown" : String(exec.exitCode)}`, + ...(citedThreadId === null ? [] : [`Continues unfinished GITS thread ${citedThreadId}.`]), ], }); const proposals = yield* Effect.tryPromise({