From 2ad970de64f381f8edc04913483276a4c65006cc Mon Sep 17 00:00:00 2001 From: Suleiman Shahbari Date: Sat, 22 Aug 2026 20:25:00 +0300 Subject: [PATCH] Run now on the planning routine spends the concurrency, not one agent (#1204) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two routines can fan out — a drain takes one entry off the queue, a plan agent writes one ticket's own sibling files — and until now only one of them did so from the card. The drain's Run now fires the sweep; [Plan tickets] fell through to a plain single start, so on the one rotation job where the concurrency setting is safe to spend, the click was the thing that ignored it. The daemon running the same routine unattended started up to `Concurrent agents`. The sweep's narrowing stops being drain-shaped. `drainOnly: true` becomes `only: 'drain' | 'plan'`, and a plan-only tick reaches the same claim-then-start path the rotation takes: candidates enumerated, ticket locks pushed before any agent starts, one agent per locked ticket. The locks are the point — without them two clicks a second apart plan the same ticket twice, which is the exact double-work this issue is about. Three things a narrowed tick must not do, each its own test: - work the queue instead. The queue-picked mode would send a plan click to the drain whenever entries are waiting; the click named a routine, so it plans. - override a switched-off routine. An unticked box stands the click down, and says so, rather than firing a routine the card shows as off. - cost the rotation its turn. A click borrows the tick for a routine it named, so the cycle stays where it was — the rule a due maintenance sweep follows. `projectId` scopes a sweep to one project. The drain sends none and visits them all, which is what its tooltip promises; planning is the picked project's own work and carries one. Both surfaces that quote a number now read `fansOut` rather than only `drains`: the tooltip says "Starts up to 2 agents in gemstack, one per open ticket" instead of "Starts one agent", and the menu item's "one agent, not the fan-out" warning — which was true of the drain and became true here the moment the button stopped being one start — now shows on both. Every new test was checked by breaking the code it guards. The first version of the plans-instead-of-draining test passed with the fix reverted: it asserted the job's name, which stays `plan` either way, when the difference is the batch — entries off the queue instead of tickets. It asserts the prompts now. --- FEATURES-SPEC.md | 2 +- .../dashboard/components/RoutineWork.SPEC.md | 6 +- .../dashboard/components/RoutineWork.test.tsx | 45 +++++++- .../dashboard/components/RoutineWork.tsx | 35 ++++-- packages/the-framework/src/auto-pm.SPEC.md | 1 + packages/the-framework/src/auto-pm.test.ts | 107 +++++++++++++++++- packages/the-framework/src/auto-pm.ts | 64 +++++++++-- .../the-framework/src/daemon-services.test.ts | 12 +- packages/the-framework/src/daemon-services.ts | 4 +- .../src/dashboard-rpc/context.ts | 4 +- .../src/dashboard-rpc/quota.test.ts | 13 ++- .../the-framework/src/dashboard-rpc/quota.ts | 14 ++- .../the-framework/src/dashboard/rpc-serve.ts | 4 +- .../the-framework/src/dashboard/server.ts | 4 +- packages/the-framework/src/e2e/harness.ts | 6 +- .../e2e/story-projects-and-settings.test.ts | 4 +- packages/the-framework/src/index.ts | 2 +- 17 files changed, 265 insertions(+), 62 deletions(-) diff --git a/FEATURES-SPEC.md b/FEATURES-SPEC.md index 269d13dad..ae9705698 100644 --- a/FEATURES-SPEC.md +++ b/FEATURES-SPEC.md @@ -25,7 +25,7 @@ happens while nobody is at the keyboard. - Start an agent from a queue entry's play button - "Run now" on a routine - "Configure first, then run" on a routine — the launcher opens with its prompt, so the model and location can be set first -- What a routine's "Run now" is about to spend, on hover — what that routine does, which model it will use, and where it runs +- What a routine's "Run now" is about to spend, on hover — what that routine does, how many agents it costs, which model it will use, and where it runs - The whole CLI is one command: `the-framework` serves the dashboard — four options, no verbs - `--host` / `--port`, the two things a browser cannot be asked; `--help` / `--version` - Reach the dashboard from another machine — non-loopback bind behind a generated shared token diff --git a/packages/the-framework/dashboard/components/RoutineWork.SPEC.md b/packages/the-framework/dashboard/components/RoutineWork.SPEC.md index 2b295bbd8..25008363b 100644 --- a/packages/the-framework/dashboard/components/RoutineWork.SPEC.md +++ b/packages/the-framework/dashboard/components/RoutineWork.SPEC.md @@ -3,11 +3,11 @@ The Overview's Routine work card: the jobs fired by the scheduled sweep — the ## Flows - The list is read straight from the definition the daemon runs, so screen and schedule cannot drift; Run now starts the work at once rather than asking the sweep to come sooner. -- The queue-draining routine's Run now fires a drain-only sweep — the only path that can fan out several agents, up to the concurrency setting; card-fired routines run unattended, like the sweep's own. +- The two routines that fan out — queue-draining and ticket-planning — have their Run now ask the sweep for that routine's work only, which is the one path that can spin up several agents, up to the concurrency setting. Draining visits every project; planning stays in the project the card has picked. Every other routine's Run now is one agent, because concurrent copies of it would undo each other. Card-fired routines run unattended, like the sweep's own. - Two checkbox tiers: the master switch turns the schedule on or off, a row's box takes that one routine in or out of it — recorded as opt-outs, so a routine added by a later version runs by default. - "Trigger routine now" sweeps once even with auto-run off (the click is the consent), and the sweep answers on the card per project, so "ran and found nothing" never looks like "never ran". -- Hovering a Run now says what it is about to spend before it is spent: what that routine does, which model it will use, and where it runs — none of which the card can otherwise show, because all three come from the Global options on another page. The queue-draining routine answers differently, since its Run now is the sweep rather than one start: it visits every project the daemon watches, and each of those decides its own model and place. -- Beside each Run now sits "Configure first, then run": it opens the picked project's launcher with that routine's prompt already in the box, so the model and where it runs can be set before an agent is spent. For the queue-draining routine it says what it costs — the launcher sends one agent, not the fan-out. +- Hovering a Run now says what it is about to spend before it is spent: what that routine does, which model it will use, and where it runs — none of which the card can otherwise show, because all three come from the Global options on another page. It also says how many agents the click costs, which is one for most routines but the concurrency setting for the two that fan out. The queue-draining routine answers the model and place differently, since its Run now visits every project the daemon watches and each of those decides its own. +- Beside each Run now sits "Configure first, then run": it opens the picked project's launcher with that routine's prompt already in the box, so the model and where it runs can be set before an agent is spent. For either routine that fans out it says what it costs — the launcher sends one agent, not the fan-out. ## Before modifying/creating SPEC.md files diff --git a/packages/the-framework/dashboard/components/RoutineWork.test.tsx b/packages/the-framework/dashboard/components/RoutineWork.test.tsx index 41aaaf675..9c0b03fbd 100644 --- a/packages/the-framework/dashboard/components/RoutineWork.test.tsx +++ b/packages/the-framework/dashboard/components/RoutineWork.test.tsx @@ -153,6 +153,47 @@ describe('RoutineWork (#1159)', () => { expect(hint.textContent).not.toContain('Opus') }) + test("the planning routine's menu item says it is one agent too, not its fan-out (#1204/#1507)", async () => { + renderCard() + await openRunMenu(AUTO_PM_ROUTINES.find(job => job.fansOut)!) + // The launcher sends one agent whichever routine it is handed, so the moment a Run now stops + // being one start its menu item has to say what it gives up. + expect(await screen.findByText(/one agent, not the fan-out/)).toBeTruthy() + }) + + // #1204: the planning routine is the other one that fans out, so its Run now goes to the sweep + // too — a plain start could only ever be one agent, which is the one thing the concurrency + // setting could not reach. + const PLAN_JOB = AUTO_PM_ROUTINES.find(job => job.fansOut)! + + test("the planning routine's Run now fires the sweep for its project, not a single start (#1204)", async () => { + renderCard() + fireEvent.click(await runNowOf(PLAN_JOB)) + await waitFor(() => expect(sendAutoPmSweep).toHaveBeenCalled()) + // Narrowed to planning, and scoped to the project the picker shows: the drain's Run now + // deliberately sends no id because it sweeps every project, and this one is not that. + expect(sendAutoPmSweep).toHaveBeenCalledWith({ only: 'plan', projectId: 'p1' }) + // The fan-out is the sweep's whole point here: a plain start is the pre-#1204 behaviour and + // is exactly what must not happen. + expect(start).not.toHaveBeenCalled() + }) + + test("the planning routine's Run now says it spends the concurrency, one agent per ticket (#1204)", async () => { + prefs = { model: 'opus', autoPmConcurrency: 3 } + renderCard() + const hint = await hoverTooltip(await runNowOf(PLAN_JOB)) + expect(hint.textContent).toMatch(/Starts up to 3 agents in gemstack, one per open ticket, unattended/) + // Not the drain's line: this one stays in the picked project and does resolve these settings. + expect(hint.textContent).not.toContain('Sweeps every project') + expect(hint.textContent).toContain('Claude Code · Opus · This machine') + }) + + test('a concurrency of one is said as one agent, not "up to 1 agents" (#1204)', async () => { + prefs = { autoPmConcurrency: 1 } + renderCard() + expect((await hoverTooltip(await runNowOf(PLAN_JOB))).textContent).toMatch(/Starts up to 1 agent in gemstack/) + }) + /** Open one row's secondary half — the chevron beside its Run now. */ const openRunMenu = async (job: AutoPmJob) => { await waitFor(() => expect(screen.getAllByText('Run now').length).toBe(AUTO_PM_ROUTINES.length)) @@ -218,7 +259,9 @@ describe('RoutineWork (#1159)', () => { renderCard({ onAgentStarted: (...args) => started.push(args) }) await waitFor(() => expect(screen.getAllByText('Run now').length).toBeGreaterThan(0)) fireEvent.click(screen.getAllByText('Run now')[0]!) - await waitFor(() => expect(sendAutoPmSweep).toHaveBeenCalledWith({ drainOnly: true })) + // No project id on purpose: the drain sweeps every project the daemon watches, which is what + // its own tooltip promises — unlike the planning routine's click, which stays in the picked one. + await waitFor(() => expect(sendAutoPmSweep).toHaveBeenCalledWith({ only: 'drain' })) expect(start).not.toHaveBeenCalled() // No navigation: the batch lands in the Agents card, not one session's page. expect(started).toHaveLength(0) diff --git a/packages/the-framework/dashboard/components/RoutineWork.tsx b/packages/the-framework/dashboard/components/RoutineWork.tsx index 6a5a2c5f6..c57655f62 100644 --- a/packages/the-framework/dashboard/components/RoutineWork.tsx +++ b/packages/the-framework/dashboard/components/RoutineWork.tsx @@ -156,15 +156,20 @@ export function RoutineWork({ const runNow = async (job: AutoPmJob) => { if (!projectId || busy) return - // The drain's Run now means "spin agents up on the queue" (#1204), and only the sweep can fan - // out — one agent per entry, up to the concurrency. A plain start could only ever be one - // agent reading the first entry. Drain-only, so an empty queue is reported on the card - // rather than the click quietly borrowing a rotation job. No navigation on purpose: the - // agents land in the Agents card, which is where a batch is watchable. - if (job.drains) { + // The two routines that fan out go through the sweep (#1204), because only the sweep can: + // it claims the work before each agent starts — a queue entry for a drain, a ticket lock for + // planning — and a plain start could only ever be one agent, reading whatever is first. + // Narrowed to the one routine the click named, so having nothing to work is reported on the + // card rather than the click quietly borrowing a different rotation job. No navigation on + // purpose: the agents land in the Agents card, which is where a batch is watchable. + // + // The drain visits every project, which is what its tooltip says and why it sends no id. + // Planning is the picked project's own work, so it carries one. + if (job.drains || job.fansOut) { setStarting(job.name) setSweepNote(null) - const result = await sendAutoPmSweep({ drainOnly: true }).catch(() => ({ ok: false as const })) + const narrowed = job.drains ? { only: 'drain' as const } : { only: 'plan' as const, projectId } + const result = await sendAutoPmSweep(narrowed).catch(() => ({ ok: false as const })) setStarting(null) if (!result.ok) setSweepNote('This dashboard is not running the sweep, so there is nothing to trigger here.') else setSweepNote(describeOutcomes('outcomes' in result ? result.outcomes : undefined)) @@ -267,10 +272,16 @@ export function RoutineWork({ {job.drains ? "Each project's own settings decide the model and where it runs." : settings} + {/* Three cases, because the click does three different things (#1204). + A routine that fans out spends what the setting allows rather than one + agent, so a tooltip that exists to say what a click costs has to say + so — and "one per ticket" is why several here is not redundant work. */} {job.drains ? `Sweeps every project the daemon watches, up to ${concurrency} ${concurrency === 1 ? 'agent' : 'agents'} each, unattended.` - : `Starts one agent${projectName ? ` in ${projectName}` : ''}, unattended — nothing is asked mid-run.`} + : job.fansOut + ? `Starts up to ${concurrency} ${concurrency === 1 ? 'agent' : 'agents'}${projectName ? ` in ${projectName}` : ''}, one per open ticket, unattended.` + : `Starts one agent${projectName ? ` in ${projectName}` : ''}, unattended — nothing is asked mid-run.`} @@ -291,11 +302,13 @@ export function RoutineWork({ > ['entry a'], optedOut: async () => [AUTO_PM_DRAIN_JOB.name], }) - await loop.tick({ onDemand: true, drainOnly: true }) + await loop.tick({ onDemand: true, only: 'drain' }) loop.stop() assert.deepEqual(ran, []) assert.equal(loop.report().outcomes[0]?.message, 'the queue has work waiting and its routine is switched off') @@ -1142,6 +1142,107 @@ test('a fansOut job fans out to the concurrency, one locked ticket per agent (#1 assert.match(prompts[0]!, new RegExp(`CLAIMED: ${lockCalls[0]![0]!.agentId}`)) }) +// #1204: Run now on the planning routine reaches the same fan-out the daemon uses. It used to be +// a plain single start, so the concurrency setting was the one thing that click ignored. + +test("a plan-only sweep fans out the planning routine, one locked ticket per agent (#1204)", async () => { + const prompts: string[] = [] + const { loop } = harness({ + jobs: [PLAN_JOB], + cooldownMs: 0, + concurrency: async () => 3, + planCandidates: async () => ['a.md', 'b.md', 'c.md'], + lockPlans: async (_p, assignments) => assignments, + start: async (_p, job) => { + prompts.push(job.prompt) + return `run-${prompts.length}` + }, + }) + await loop.tick({ onDemand: true, only: 'plan', projectId: 'p1' }) + loop.stop() + assert.equal(prompts.length, 3, 'the click spends the concurrency, not one agent') + assert.match(prompts[0]!, /tickets\/a\.md/) + assert.match(prompts[2]!, /tickets\/c\.md/) +}) + +test("a plan-only sweep plans instead of draining, however full the queue is (#1204)", async () => { + // The queue-picked mode would send this tick to the drain. The click named the planning + // routine, so the queue is not its business. + // + // Asserted on the prompts rather than the job name: the name stays `plan` even when the tick + // falls through to the drain's fan-out, because it is the *batch* that differs — entries off + // the queue instead of tickets. The name alone passes either way, which is no guard at all. + const prompts: string[] = [] + const { loop, ran } = harness({ + jobs: [PLAN_JOB], + cooldownMs: 0, + concurrency: async () => 2, + queue: async () => ['work one', 'work two'], + drainJob: { name: 'drain', prompt: 'Work the queue.', drains: true }, + planCandidates: async () => ['a.md'], + lockPlans: async (_p, assignments) => assignments, + start: async (_p, job) => { + prompts.push(job.prompt) + return `run-${prompts.length}` + }, + }) + await loop.tick({ onDemand: true, only: 'plan', projectId: 'p1' }) + loop.stop() + assert.deepEqual(ran, []) + assert.equal(prompts.length, 1, 'one open ticket is one agent, not one per queue entry') + assert.match(prompts[0]!, /tickets\/a\.md/) + assert.ok( + !prompts.some(prompt => prompt.includes('work one')), + 'no agent was handed a queue entry: the click asked for planning, not draining', + ) +}) + +test("a plan-only sweep stands down when the planning routine is switched off (#1204)", async () => { + const { loop, ran, logs } = harness({ + jobs: [PLAN_JOB], + cooldownMs: 0, + optedOut: async () => ['plan'], + planCandidates: async () => ['a.md'], + lockPlans: async (_p, assignments) => assignments, + }) + await loop.tick({ onDemand: true, only: 'plan', projectId: 'p1' }) + loop.stop() + assert.deepEqual(ran, [], 'an unticked box is not overridden by the click') + assert.ok(logs.some(line => line.includes('the planning routine is switched off'))) +}) + +test("a plan-only sweep visits only the project the card picked (#1204)", async () => { + const { loop, started } = harness({ + projects: async () => [ + { id: 'p1', path: '/one' }, + { id: 'p2', path: '/two' }, + ], + jobs: [PLAN_JOB], + cooldownMs: 0, + planCandidates: async () => ['a.md'], + lockPlans: async (_p, assignments) => assignments, + }) + await loop.tick({ onDemand: true, only: 'plan', projectId: 'p2' }) + loop.stop() + assert.deepEqual(started, ['p2'], 'the other project is not swept by a click that named one') +}) + +test("a plan click does not cost the rotation its turn (#1204)", async () => { + // The rotation is mid-cycle; a click that borrows the tick for a routine it named must leave + // the cycle where it was, the same way a due maintenance sweep does. + const other: AutoPmJob = { name: 'triage', prompt: 'Triage.' } + const { loop, ran } = harness({ + jobs: [other, PLAN_JOB], + cooldownMs: 0, + planCandidates: async () => ['a.md'], + lockPlans: async (_p, assignments) => assignments, + }) + await loop.tick({ onDemand: true, only: 'plan', projectId: 'p1' }) + await loop.tick() + loop.stop() + assert.deepEqual(ran, ['plan', 'triage'], 'the scheduled tick still gets the rotation job it was owed') +}) + test('only the tickets the lock actually claimed go out (#1327)', async () => { // A lost race — b.md's sibling appeared between the enumeration and the lock — costs that one // agent, not the batch. @@ -1308,13 +1409,13 @@ test('a drain-only sweep works the queue and never borrows the tick for the rota return `run-${prompts.length}` }, }) - await loop.tick({ onDemand: true, drainOnly: true }) + await loop.tick({ onDemand: true, only: 'drain' }) loop.stop() assert.equal(prompts.length, 2) // ...and with an empty queue it says so instead of starting a rotation job. const { loop: empty, started } = harness({ cooldownMs: 0, queue: async () => [] }) - await empty.tick({ onDemand: true, drainOnly: true }) + await empty.tick({ onDemand: true, only: 'drain' }) empty.stop() assert.equal(started.length, 0) assert.equal(empty.report().outcomes[0]?.message, 'the queue is empty, so there is nothing to drain') diff --git a/packages/the-framework/src/auto-pm.ts b/packages/the-framework/src/auto-pm.ts index f100e4733..956f8d1ff 100644 --- a/packages/the-framework/src/auto-pm.ts +++ b/packages/the-framework/src/auto-pm.ts @@ -630,16 +630,34 @@ export interface AutoPmLoop { * runs with the preference off, and the master switch is the only gate it skips: every other * reason to stand down (live agents, cooldowns, the quota boundary, unticked routines) still holds. * - * `drainOnly` narrows the sweep to working the queue (#1204): the drain row's Run now means - * "spin agents up on the queue", so a tick that would fall through to a rotation job (the queue - * is empty) says so instead of borrowing the click for work nobody asked for. + * `only` narrows the sweep to one routine's work (#1204), for a Run now that fans out: + * + * - `'drain'` — the drain row's Run now means "spin agents up on the queue", so a tick that + * would fall through to a rotation job (the queue is empty) says so instead of borrowing the + * click for work nobody asked for. + * - `'plan'` — the same for the one rotation job that fans out ({@link AutoPmJob.fansOut}). + * Its Run now used to be a plain single start, so the concurrency setting was the one thing + * that click ignored; narrowing here reaches the same claim-then-start path the rotation + * takes, locks included. + * + * `projectId` scopes the sweep to one project, which is what a Run now fired from a card with a + * project picked means. Absent, every project the daemon watches is visited, which is what the + * drain row's Run now says it does. */ - tick(opts?: { onDemand?: boolean; drainOnly?: boolean }): Promise + tick(opts?: { onDemand?: boolean; only?: AutoPmOnly; projectId?: string }): Promise /** What the last sweep decided, for the dashboard to show (#1161). */ report(): AutoPmReport stop(): void } +/** + * Which routine's work a narrowed sweep is for (#1204). Both are the fan-out kinds: a drain takes + * one entry *off* the queue and a pinned plan agent writes one ticket's own sibling files, so + * several agents do disjoint work. The rotation jobs that rewrite the shared queue document have + * nothing to narrow to — one agent is all they can ever use. + */ +export type AutoPmOnly = 'drain' | 'plan' + /** * Start the auto-PM sweep (#685): every {@link DEFAULT_AUTO_PM_INTERVAL_MS}, ask * {@link autoPmDecision} for each project and start an agent for the ones that say yes. @@ -679,7 +697,7 @@ export function startAutoPm(deps: AutoPmDeps): AutoPmLoop { let sweeping = false let stopped = false - const tick = async (opts?: { onDemand?: boolean; drainOnly?: boolean }): Promise => { + const tick = async (opts?: { onDemand?: boolean; only?: AutoPmOnly; projectId?: string }): Promise => { if (stopped || sweeping) return sweeping = true let enabled = false @@ -694,7 +712,11 @@ export function startAutoPm(deps: AutoPmDeps): AutoPmLoop { // where the box stood. enabled = await deps.enabled().catch(() => false) if (!enabled && !opts?.onDemand) return - const projects = await deps.projects().catch(() => []) + const all = await deps.projects().catch(() => []) + // A Run now fired from a card names the project its picker shows; the scheduled sweep names + // none and visits them all. An id matching nothing leaves an empty list, which stands the + // sweep down rather than silently widening it to every project. + const projects = opts?.projectId ? all.filter(project => project.id === opts.projectId) : all if (!projects.length) return // Read beside the master switch and for the same reason (#1209): it is the same preference // file, and a routine switched off mid-sweep should not fire for the projects still to come. @@ -788,7 +810,7 @@ export function startAutoPm(deps: AutoPmDeps): AutoPmLoop { // A drain-only sweep (#1204) works the queue or says why not — it never borrows the // click for a rotation job the user did not ask for. Logged like every other stand-down // (#855/#1433): these two used to be the only silent ones. - if (opts?.drainOnly) { + if (opts?.only === 'drain') { if (decision.mode !== 'drain') { deps.log(`[framework] auto PM: standing down for ${project.path} — the queue is empty, so there is nothing to drain`) note(project, false, 'the queue is empty, so there is nothing to drain') @@ -800,6 +822,16 @@ export function startAutoPm(deps: AutoPmDeps): AutoPmLoop { continue } } + // The planning routine this click asked for, read off the *enabled* rotation so an + // unticked box stands the click down instead of firing a routine the card shows as off. + // `fansOut` rather than a name, for the same no-name-matching reason the job carries the + // property at all. + const planJob = opts?.only === 'plan' ? rotation.find(item => item.fansOut) : undefined + if (opts?.only === 'plan' && !planJob) { + deps.log(`[framework] auto PM: standing down for ${project.path} — the planning routine is switched off`) + note(project, false, 'the planning routine is switched off') + continue + } /** * What this tick does, which is the queue-picked mode (#855) unless the draining routine * is switched off — then the rotation gets the tick instead of the sweep standing down. @@ -813,7 +845,10 @@ export function startAutoPm(deps: AutoPmDeps): AutoPmLoop { * A drain-only sweep never gets here: it has already stood down above, because the click * that fires it asked for the queue specifically. */ - const mode: 'pm' | 'drain' = decision.mode === 'drain' && !drainJob ? 'pm' : decision.mode + // A plan-only click is never a drain, however full the queue is: the click named the + // planning routine, and the queue-picked mode would otherwise send it to work entries. + const mode: 'pm' | 'drain' = + opts?.only === 'plan' ? 'pm' : decision.mode === 'drain' && !drainJob ? 'pm' : decision.mode const index = nextJob.get(project.id) ?? 0 // A due codebase sweep (#882) outranks the rotation: the rotation invents work, and the // sweep is a standing instruction to go find some. Only ever while the queue is empty -- @@ -830,7 +865,9 @@ export function startAutoPm(deps: AutoPmDeps): AutoPmLoop { decision.mode === 'pm' && maintenanceJob !== undefined && (await deps.maintenanceDue?.(project).catch(() => false)) === true - const job = sweep ? maintenanceJob : mode === 'drain' ? drainJob : rotation[index % rotation.length] + // The plan click outranks both the calendar and the rotation index: it asked for one + // routine by name, so neither a due codebase sweep nor whose turn it is may take its tick. + const job = planJob ?? (sweep ? maintenanceJob : mode === 'drain' ? drainJob : rotation[index % rotation.length]) if (!job) { // Told apart on purpose: a rotation emptied by the checkboxes is a setting the user can // see and undo, and reads nothing like a daemon wired without jobs at all. @@ -931,8 +968,9 @@ export function startAutoPm(deps: AutoPmDeps): AutoPmLoop { ) if (!candidates.length) { // Nothing left to plan is this job's work being done, not a refusal: the rotation - // advances, so the next tick tries the next job instead of re-asking forever. - nextJob.set(project.id, index + 1) + // advances, so the next tick tries the next job instead of re-asking forever. A click + // that named this routine advances nothing — it did not come from the cycle. + if (!planJob) nextJob.set(project.id, index + 1) note(project, false, 'every open ticket already has a plan, or an agent on the way to one') continue } @@ -1002,8 +1040,10 @@ export function startAutoPm(deps: AutoPmDeps): AutoPmLoop { // by the calendar, not the cycle, so borrowing this tick must not cost the rotation its // turn. Stamped after the start took for the same reason the rotation is -- a sweep the // daemon refused should be retried next tick, not postponed a whole interval. + // A plan click does not advance it either, for the reason a sweep does not: it borrows + // the tick for a routine it named, so the rotation keeps the turn it was on. if (sweep) await deps.recordMaintenance?.(project).catch(() => {}) - else if (mode === 'pm') nextJob.set(project.id, index + 1) + else if (mode === 'pm' && !planJob) nextJob.set(project.id, index + 1) // One line per project however many agents went out, and a single start keeps the old // wording exactly. const described = started.map(item => doing(item)).join('; ') diff --git a/packages/the-framework/src/daemon-services.test.ts b/packages/the-framework/src/daemon-services.test.ts index 6d62d9af2..f93a0be95 100644 --- a/packages/the-framework/src/daemon-services.test.ts +++ b/packages/the-framework/src/daemon-services.test.ts @@ -171,7 +171,7 @@ test("the drain row's Run now fans out to the setting with auto-run off (#1204/# assert.equal(starts.length, 0, 'auto-run off means the schedule starts nothing by itself') // Exactly what the card's Run now sends for the draining routine, through the same // `wakeAutoPm` seam the RPC calls. - running.wakeAutoPm({ onDemand: true, drainOnly: true }) + running.wakeAutoPm({ onDemand: true, only: 'drain' }) await settle(() => starts.length >= 3) assert.equal(starts.length, 3, 'the click spins the setting up, not one agent') for (const [index, start] of starts.entries()) { @@ -195,12 +195,12 @@ test('a drained entry is checked off on the data branch once its run reports the // the check-off as a data-branch commit. The agent never touches the queue. const { starts, stop, services: running, projectDir } = await services({ autoPm: false, autoPmConcurrency: 1 }) try { - running.wakeAutoPm({ onDemand: true, drainOnly: true }) + running.wakeAutoPm({ onDemand: true, only: 'drain' }) await settle(() => starts.length >= 1) assert.ok(starts[0]!.prompt.includes(QUEUE_ENTRIES[0]!), 'the first entry is the pinned one') // The run settles `done` and its epilogue reported the push/PR: the one ending that retires. await archiveMeta(projectDir, { id: 'run-1', status: 'done', handoffReport: 'done' }) - running.wakeAutoPm({ onDemand: true, drainOnly: true }) + running.wakeAutoPm({ onDemand: true, only: 'drain' }) // Waited for on the branch, not the checkout file: the funnel writes the file first and // commits a beat later, and only the commit is the retirement every machine pulls. const deadline = Date.now() + 5000 @@ -227,10 +227,10 @@ test('a run whose hand-off failed leaves its entry open: unpublished work is not // the entry stays open for the next drain rather than vanishing with the work unpublished. const { starts, stop, services: running, projectDir } = await services({ autoPm: false, autoPmConcurrency: 1 }) try { - running.wakeAutoPm({ onDemand: true, drainOnly: true }) + running.wakeAutoPm({ onDemand: true, only: 'drain' }) await settle(() => starts.length >= 1) await archiveMeta(projectDir, { id: 'run-1', status: 'done', handoffReport: 'failed' }) - running.wakeAutoPm({ onDemand: true, drainOnly: true }) + running.wakeAutoPm({ onDemand: true, only: 'drain' }) // No positive signal marks "the promote settled without retiring", so give the tick a // moment and then require the queue untouched — the check-off in the sibling test lands // well inside this window when the gate passes. @@ -254,7 +254,7 @@ test('a drain claims an entry whose ticket file is gone, recreating tickets/ on await rm(join(dir, 'tickets'), { recursive: true, force: true }) }) assert.ok(cleared.ok, 'the fixture must drop tickets/ from the data branch') - running.wakeAutoPm({ onDemand: true, drainOnly: true }) + running.wakeAutoPm({ onDemand: true, only: 'drain' }) await settle(() => starts.length >= 1) assert.equal(starts.length, 1, 'the batch starts instead of standing down') assert.ok(starts[0]!.prompt.includes(QUEUE_ENTRIES[0]!), 'the first entry is the pinned one') diff --git a/packages/the-framework/src/daemon-services.ts b/packages/the-framework/src/daemon-services.ts index ed402b4dc..85c51ff9d 100644 --- a/packages/the-framework/src/daemon-services.ts +++ b/packages/the-framework/src/daemon-services.ts @@ -9,7 +9,7 @@ import { readLiveMetas, listAgents, type LiveAgent } from './store/index.js' import { startKeyedWatcher, type KeyedWatcher } from './dashboard/keyed-watcher.js' import { buildInterventions, interventionKey, postInterventionsDiscord } from './dashboard/interventions.js' import { buildActivity, activityKey, postActivityDiscord } from './dashboard/activity.js' -import { startAutoPm, AUTO_PM_JOBS, DEFAULT_AUTO_PM_INTERVAL_MS, quotaHeadroom, type AutoPmReport } from './auto-pm.js' +import { startAutoPm, AUTO_PM_JOBS, DEFAULT_AUTO_PM_INTERVAL_MS, quotaHeadroom, type AutoPmReport, type AutoPmOnly } from './auto-pm.js' import { startDaemonTick, DAEMON_TICK_MS } from './daemon-tick.js' import { ciFixPrompt, startCiWatch } from './ci-watch.js' import { releaseStalePinnedBranch } from './stale-branch.js' @@ -86,7 +86,7 @@ export interface BackgroundServices { * Resolves when the tick does (#1433), so the trigger button can await the sweep and say what * it decided; the switched-on-preference wake simply does not await it. */ - wakeAutoPm: (opts?: { onDemand?: boolean; drainOnly?: boolean }) => Promise + wakeAutoPm: (opts?: { onDemand?: boolean; only?: AutoPmOnly; projectId?: string }) => Promise /** What the last auto-PM sweep decided, for the usage panel to show (#1161). */ autoPmReport: () => AutoPmReport } diff --git a/packages/the-framework/src/dashboard-rpc/context.ts b/packages/the-framework/src/dashboard-rpc/context.ts index be938cbda..79b704f93 100644 --- a/packages/the-framework/src/dashboard-rpc/context.ts +++ b/packages/the-framework/src/dashboard-rpc/context.ts @@ -4,7 +4,7 @@ import type { DashboardContext, EventsSource, RemoteAgents } from '../dashboard/ import type { PreferencesStore } from '../registry.js' import type { DiscordCredentialsStore } from '../discord-credentials.js' import type { QuotaSource } from '../dashboard/quota.js' -import type { AutoPmReporter } from '../auto-pm.js' +import type { AutoPmReporter, AutoPmOnly } from '../auto-pm.js' import type { ProjectErrorsReader } from '../project-errors.js' /** @@ -109,7 +109,7 @@ export function contextAutoPm(): AutoPmReporter { } /** How a sweep is fired on demand (#1210). */ -export function contextAutoPmSweep(): (opts?: { drainOnly?: boolean }) => void | Promise { +export function contextAutoPmSweep(): (opts?: { only?: AutoPmOnly; projectId?: string }) => void | Promise { return fromContext('autoPmSweep') } diff --git a/packages/the-framework/src/dashboard-rpc/quota.test.ts b/packages/the-framework/src/dashboard-rpc/quota.test.ts index 559352305..8846f8912 100644 --- a/packages/the-framework/src/dashboard-rpc/quota.test.ts +++ b/packages/the-framework/src/dashboard-rpc/quota.test.ts @@ -2,7 +2,7 @@ import { strict as assert } from 'node:assert' import { test } from 'node:test' import { provideTestContext } from './test-context.js' import { sendAutoPmSweep } from './quota.js' -import type { AutoPmOutcome } from '../auto-pm.js' +import type { AutoPmOutcome, AutoPmOnly } from '../auto-pm.js' // #1433: "Trigger routine now" was fire-and-forget — the RPC returned before the sweep ran, so // the button flashed and the card showed nothing, with the stand-down reason recoverable only @@ -65,15 +65,18 @@ test('a sweep that throws is a failure; an unreadable report is not (#1433)', as assert.deepEqual(await sendAutoPmSweep(), { ok: true }) }) -test('drainOnly travels to the loop untouched (#1204)', async () => { +test('the narrowing travels to the loop untouched (#1204)', async () => { const seen: unknown[] = [] provideTestContext({ - autoPmSweep: (opts?: { drainOnly?: boolean }) => { + autoPmSweep: (opts?: { only?: AutoPmOnly; projectId?: string }) => { seen.push(opts) }, autoPm: () => ({ nextSweepAt: 0, outcomes: [] }), }) - await sendAutoPmSweep({ drainOnly: true }) + await sendAutoPmSweep({ only: 'drain' }) + await sendAutoPmSweep({ only: 'plan', projectId: 'p1' }) await sendAutoPmSweep() - assert.deepEqual(seen, [{ drainOnly: true }, undefined]) + // Passed through rather than rebuilt: a wrapper that reconstructed the object is how a new + // narrowing silently stops reaching the loop. + assert.deepEqual(seen, [{ only: 'drain' }, { only: 'plan', projectId: 'p1' }, undefined]) }) diff --git a/packages/the-framework/src/dashboard-rpc/quota.ts b/packages/the-framework/src/dashboard-rpc/quota.ts index 55853a2ec..78c03ffa4 100644 --- a/packages/the-framework/src/dashboard-rpc/quota.ts +++ b/packages/the-framework/src/dashboard-rpc/quota.ts @@ -1,5 +1,5 @@ import { contextAutoPm, contextAutoPmSweep, contextQuota } from './context.js' -import type { AutoPmOutcome, AutoPmReport } from '../auto-pm.js' +import type { AutoPmOutcome, AutoPmReport, AutoPmOnly } from '../auto-pm.js' import type { QuotaView } from '../dashboard/quota.js' // The usage panel's read surface (#533): where the account's subscription quota stands, and where @@ -49,11 +49,13 @@ export async function onAutoPm(): Promise { * off the loop's own report once the tick resolves, so the card can say them without a poll * having to race the sweep. `false` means the sweep itself failed. * - * `drainOnly` narrows the sweep to working the queue (#1204): the drain routine's Run now spins - * agents up on the queue's entries — the fan-out only the sweep can do — and an empty queue is - * reported rather than borrowed for a rotation job. + * `only` narrows the sweep to one routine's work (#1204), which is what a Run now that fans out + * means: `'drain'` spins agents up on the queue's entries, `'plan'` on the open tickets. Either + * way the fan-out is the sweep's — a plain start could only ever be one agent — and having + * nothing to work is reported rather than borrowed for a rotation job. `projectId` scopes it to + * the project the card has picked. */ -export async function sendAutoPmSweep(opts?: { drainOnly?: boolean }): Promise<{ ok: boolean; outcomes?: AutoPmOutcome[] }> { +export async function sendAutoPmSweep(opts?: { only?: AutoPmOnly; projectId?: string }): Promise<{ ok: boolean; outcomes?: AutoPmOutcome[] }> { const sweep = contextAutoPmSweep() // The reporter is captured BEFORE the sweep is awaited. It had to be: the context was // request-scoped and did not survive an await, so a post-await `contextAutoPm()` found nothing @@ -62,7 +64,7 @@ export async function sendAutoPmSweep(opts?: { drainOnly?: boolean }): Promise<{ // load-bearing; the captured closure needs no context to be called later either way. const reporter = contextAutoPm() try { - await sweep(opts?.drainOnly ? { drainOnly: true } : undefined) + await sweep(opts) } catch { return { ok: false } } diff --git a/packages/the-framework/src/dashboard/rpc-serve.ts b/packages/the-framework/src/dashboard/rpc-serve.ts index 000bd5d99..462125200 100644 --- a/packages/the-framework/src/dashboard/rpc-serve.ts +++ b/packages/the-framework/src/dashboard/rpc-serve.ts @@ -8,7 +8,7 @@ import type { FrameworkEvent } from '../events.js' import type { PreferencesStore } from '../registry.js' import type { DiscordCredentialsStore } from '../discord-credentials.js' import type { QuotaSource } from './quota.js' -import type { AutoPmReporter } from '../auto-pm.js' +import type { AutoPmReporter, AutoPmOnly } from '../auto-pm.js' import type { ProjectErrorsReader } from '../project-errors.js' import type { AddProjectResult, StartAgentKind, StartAgentOptions, StartAgentResult } from './types.js' import type { AgentMeta } from '../store/index.js' @@ -66,7 +66,7 @@ export interface DashboardContext { * Run an auto PM sweep now rather than at the next interval (#1210). Resolves when the sweep * does (#1433), so the trigger RPC can await it and return what it decided. */ - autoPmSweep: (opts?: { drainOnly?: boolean }) => void | Promise + autoPmSweep: (opts?: { only?: AutoPmOnly; projectId?: string }) => void | Promise /** What a project currently suffers from (#1500): the daemon's error state, read per project. */ projectErrors: ProjectErrorsReader } diff --git a/packages/the-framework/src/dashboard/server.ts b/packages/the-framework/src/dashboard/server.ts index a1bd8934d..2100d146e 100644 --- a/packages/the-framework/src/dashboard/server.ts +++ b/packages/the-framework/src/dashboard/server.ts @@ -6,7 +6,7 @@ import { registryPreferencesStore, type PreferencesStore } from '../registry.js' import { registryDiscordCredentialsStore } from '../discord-credentials-store.js' import type { DiscordCredentialsStore } from '../discord-credentials.js' import { defaultQuotaSource, type QuotaSource } from './quota.js' -import type { AutoPmReporter } from '../auto-pm.js' +import type { AutoPmReporter, AutoPmOnly } from '../auto-pm.js' import type { ProjectErrorsReader } from '../project-errors.js' import { serveClientBundle } from './static.js' import { BROWSER_PROXY_PREFIX, handleBrowserProxy } from './browser-proxy.js' @@ -60,7 +60,7 @@ export interface DashboardOptions { * Fire an auto PM sweep now instead of waiting out the interval (#1210). Resolves when the * sweep does (#1433), so the trigger RPC can await it. */ - autoPmSweep: (opts?: { drainOnly?: boolean }) => void | Promise + autoPmSweep: (opts?: { only?: AutoPmOnly; projectId?: string }) => void | Promise /** What a project currently suffers from (#1500), for the project list to carry. */ projectErrors: ProjectErrorsReader /** diff --git a/packages/the-framework/src/e2e/harness.ts b/packages/the-framework/src/e2e/harness.ts index 839ce9ff4..88735fbb6 100644 --- a/packages/the-framework/src/e2e/harness.ts +++ b/packages/the-framework/src/e2e/harness.ts @@ -22,7 +22,7 @@ import type { FrameworkEvent } from '../events.js' import type { StartAgentKind, StartAgentOptions } from '../dashboard/types.js' import type { AgentSpec } from '../agent-spec.js' import type { QuotaView } from '../dashboard/quota.js' -import type { AutoPmReport } from '../auto-pm.js' +import type { AutoPmReport, AutoPmOnly } from '../auto-pm.js' // Re-home the process-global config home FIRST: the registry, preferences, and daemon state all // resolve through $XDG_CONFIG_HOME at call time, and run-tests.mjs gives the whole suite ONE @@ -65,7 +65,7 @@ export interface StoryWorld { /** The usage panel's reading (mutable): what `onQuota` serves. */ quota: { view: QuotaView } /** The auto-PM panel's stubs (mutable): what `onAutoPm` reports and what a sweep records. */ - autoPm: { report?: AutoPmReport; sweeps: Array<{ drainOnly?: boolean }> } + autoPm: { report?: AutoPmReport; sweeps: Array<{ only?: AutoPmOnly; projectId?: string }> } /** * Bind one dashboard RPC to this world's context. The real mount wires the context once, at * start-up; a story stands several worlds up in one process, so re-providing before every call @@ -158,7 +158,7 @@ export async function makeWorld(): Promise { // the model's own week states that window in the view it sets (#1619). quota: { read: async () => quota.view, boundaryFor: async () => quota.view.boundary, stop: () => {} }, autoPm: () => autoPm.report, - autoPmSweep: async (opts?: { drainOnly?: boolean }) => { + autoPmSweep: async (opts?: { only?: AutoPmOnly; projectId?: string }) => { autoPm.sweeps.push(opts ?? {}) }, projectErrors: () => [], diff --git a/packages/the-framework/src/e2e/story-projects-and-settings.test.ts b/packages/the-framework/src/e2e/story-projects-and-settings.test.ts index 84303fba5..80d6714d3 100644 --- a/packages/the-framework/src/e2e/story-projects-and-settings.test.ts +++ b/packages/the-framework/src/e2e/story-projects-and-settings.test.ts @@ -130,8 +130,8 @@ test('the usage panel reads the daemon quota source, and the sweep button fires nextSweepAt: 789, outcomes: [{ projectId: 'p', path: '/p', started: false, message: 'the queue is empty' }], } - const swept = await rpc(sendAutoPmSweep)({ drainOnly: true }) - assert.deepEqual(world.autoPm.sweeps, [{ drainOnly: true }]) + const swept = await rpc(sendAutoPmSweep)({ only: 'drain' }) + assert.deepEqual(world.autoPm.sweeps, [{ only: 'drain' }]) assert.equal(swept.ok, true) assert.equal(swept.outcomes?.[0]?.message, 'the queue is empty') } finally { diff --git a/packages/the-framework/src/index.ts b/packages/the-framework/src/index.ts index 9e6dae5f2..7819d1116 100644 --- a/packages/the-framework/src/index.ts +++ b/packages/the-framework/src/index.ts @@ -14,7 +14,7 @@ */ export type { AgentError, HandoffState, SessionInfo } from './agent-view.js' -export type { AutoPmJob, AutoPmOutcome, AutoPmReport } from './auto-pm.js' +export type { AutoPmJob, AutoPmOutcome, AutoPmReport, AutoPmOnly } from './auto-pm.js' export type { FrameworkFileConfig } from './config.js' export type { ChoiceRequest, FrameworkEvent } from './events.js' export type { QuotaBoundaryStatus } from './quota-boundary.js'