diff --git a/FEATURES-SPEC.md b/FEATURES-SPEC.md index 87c20c698..6abfc01a5 100644 --- a/FEATURES-SPEC.md +++ b/FEATURES-SPEC.md @@ -105,7 +105,7 @@ happens while nobody is at the keyboard. - Commit what the agent left uncommitted - Push the branch (on by default) - Open a PR (on by default) -- The PR is described by the agent that did the work, when it wrote a description for it +- The PR is named and described by the agent that did the work, when it wrote them — never titled with the prompt it was given - Auto-merge — armed by config, authorized by the agent's ready signal - Empty agents publish nothing - Handoff panel: push / open PR / merge, as buttons @@ -133,6 +133,7 @@ happens while nobody is at the keyboard. - Usage panel: quota consumed, pace, projection - Unattended work stands down past the pro-rated share of the week — quota gates *starting* work, never an agent already running +- The chosen model's own weekly allowance gates unattended work too — a spent model week holds the work back while the account's week still has room - Work you asked for is never starved - Spend-offset slider — the one budget control for autonomous work diff --git a/packages/the-framework/prompts/protocols/signal.md b/packages/the-framework/prompts/protocols/signal.md index 9caa4c04a..339172980 100644 --- a/packages/the-framework/prompts/protocols/signal.md +++ b/packages/the-framework/prompts/protocols/signal.md @@ -11,8 +11,10 @@ When you call setReadyForMerge() — you believe the work is complete and ready ``` ## Opening a pull request -Whenever you emit `ready-for-merge`, emit an `open-pr` block too, describing the work. The Framework opens the pull request for you and this block is its body — you do not need to run `gh pr create` yourself: +Whenever you emit `ready-for-merge`, emit an `open-pr` block too, naming and describing the work. The Framework opens the pull request for you — you do not need to run `gh pr create` yourself. Write it like a commit message: the first line is the title, the rest is the body. ```open-pr + + ``` -Without it the pull request can only repeat the prompt you were given, which does not say what the work turned out to be. The Framework supplies everything else: the title from your session name, the ticket's issue reference where there is one, and recording the number so every surface shows the same pull request. You do not stop, and you can re-emit it as the work changes — the last one is used. Opening the pull request yourself instead still works; you then own all of the above. +Without it the pull request has no name for your work and can only repeat the prompt you were given, which does not say what the work turned out to be. The Framework supplies the rest: the ticket's issue reference where there is one, and recording the number so every surface shows the same pull request. You do not stop, and you can re-emit it as the work changes — the last one is used. Opening the pull request yourself instead still works; you then own all of the above. diff --git a/packages/the-framework/src/auto-pm.SPEC.md b/packages/the-framework/src/auto-pm.SPEC.md index b5ef77a62..4c0faa70a 100644 --- a/packages/the-framework/src/auto-pm.SPEC.md +++ b/packages/the-framework/src/auto-pm.SPEC.md @@ -21,6 +21,7 @@ Auto PM spends leftover subscription quota on the product's own roadmap: while t ## Rationales - An unreadable quota fails closed — the opposite of the per-agent guard: quietly burning quota on work nobody asked for is worse than skipping a pass. +- Where the account stands is asked per project rather than once per pass, because the model a project's work would run on is a project setting and each model's own weekly allowance binds alongside the account's. Two projects on two models can therefore stand at two different places against the same reading. - "Run now" skips only the master switch: the click is the consent the preference exists to record; every other stand-down holds. - A switched-off draining routine falls through to the rotation rather than standing the pass down, because a stand-down would make every inventing routine unreachable whenever the queue holds anything — and the queue is auto-populated, so it usually does. - The ticketless hand-off window is accepted rather than closed: closing it would take a durable per-entry claim — a second claim shape beside the pushed ticket lock that already covers the queue's normal case — for a race whose cost is a duplicated attempt, never lost work. diff --git a/packages/the-framework/src/auto-pm.ts b/packages/the-framework/src/auto-pm.ts index ffdd5b306..6881077cf 100644 --- a/packages/the-framework/src/auto-pm.ts +++ b/packages/the-framework/src/auto-pm.ts @@ -472,8 +472,15 @@ export interface AutoPmDeps { * setting has never meant "less". */ concurrency?(): Promise - /** Where the account stands against its boundary, or `undefined` when there is no reading. */ - quota(): Promise + /** + * Where the account stands against its boundary for the work *this project* would start, or + * `undefined` when there is no reading. + * + * Asked per project rather than once per sweep (#1619): the model is a project-resolvable + * setting, and the model's own weekly window binds alongside the account's (#879) — so two + * projects on two models can stand at two different places against the same reading. + */ + quota(project: AutoPmProject): Promise /** The jobs to rotate through, in cycle order. Used only while the queue is empty. */ jobs: readonly AutoPmJob[] /** The job for a queue with open entries (#855); {@link AUTO_PM_DRAIN_JOB} by default. */ @@ -684,9 +691,6 @@ export function startAutoPm(deps: AutoPmDeps): AutoPmLoop { // the cycle stays a cycle: with two of four off, the remaining two alternate instead of // every other tick landing on a job that cannot run. const rotation = deps.jobs.filter(job => !optedOut.has(job.name)) - // One reading for the whole sweep: it is an account-wide meter, and re-reading it per - // project would spend a rate-limited call to learn the same number. - const quota = await deps.quota().catch(() => undefined) // How many agents each project may keep going (#1204). Read beside the opt-outs and for the // same reason: it is the same preference file, re-read so the setting takes effect // mid-schedule. Floored at one, since zero agents is the master switch's job. @@ -747,6 +751,9 @@ export function startAutoPm(deps: AutoPmDeps): AutoPmLoop { } } const entries = await deps.queue(project).catch(() => undefined) + // Per project, because the model the work would run on is (#1619). It costs no reading: + // the meter is polled elsewhere and this only measures the last one against the boundary. + const quota = await deps.quota(project).catch(() => undefined) const activeAgents = deps.activeAgents(project) const since = lastStart.get(project.id) const decision = autoPmDecision({ diff --git a/packages/the-framework/src/cli.ts b/packages/the-framework/src/cli.ts index bc965b282..4c8327196 100644 --- a/packages/the-framework/src/cli.ts +++ b/packages/the-framework/src/cli.ts @@ -16,6 +16,7 @@ import { connectCdp, startBrowserStream, type BrowserStream } from './browser-st import { randomUUID } from 'node:crypto' import { formatFrameworkEvent, mergeWithheldWhy } from './terminal.js' import { defuseClosingKeywords } from './closing-keywords.js' +import type { ParsedPullRequest } from './turn-gate.js' import { CLAUDE_CODE_SESSION_LINK } from './session-link.js' import { type AutoHandoffSkip, type ChoicePick, type ChoiceRequest, type FrameworkEvent, type MergeWithheldReason, type OnBeforeMergeableSkip } from './events.js' import { agentAutoHandoff, withheldMerge } from './dashboard/agent-handoff.js' @@ -548,8 +549,8 @@ export interface AgentJournal { sessionName: () => string | undefined /** The agent signalled setReadyForMerge() this agent (#326). */ sawReadyForMerge: () => boolean - /** The pull-request description the agent wrote via an `open-pr` block (#1567), if any. */ - prDescription: () => string | undefined + /** The pull request the agent asked for via an `open-pr` block (#1567/#1618), if any. */ + pullRequest: () => ParsedPullRequest | undefined /** The agent stopped cleanly (user interrupt / budget cap #322) rather than failed. */ stoppedCleanly: () => boolean /** Hold the browser preview's port until the session opens (#829/#813). */ @@ -581,9 +582,9 @@ export function createAgentJournal(deps: { let stoppedCleanly = false let sawReadyForMerge = false let sessionName: string | undefined - // The agent's own pull-request description (#1567), latest wins: it may revise it as the work + // The pull request the agent asked for (#1567), latest wins: it may revise it as the work // changes, and the handoff wants what it said last. - let prDescription: string | undefined + let pullRequest: ParsedPullRequest | undefined // The browser preview's port, announced on the first `session` event rather than when the // bridge opens (#829): the dashboard renders only the tail from the last `session` event, so // anything emitted ahead of it is dropped from the agent's view. @@ -598,7 +599,7 @@ export function createAgentJournal(deps: { const onEvent = (event: FrameworkEvent) => { if (event.kind === 'ready-for-merge') sawReadyForMerge = true - if (event.kind === 'pull-request-description') prDescription = event.description + if (event.kind === 'open-pr') pullRequest = { ...(event.title ? { title: event.title } : {}), ...(event.description ? { description: event.description } : {}) } if (event.kind === 'session-name') { sessionName = event.name // The framework-owned checkout (#736) was branched as `tf-agent-` before a @@ -632,7 +633,7 @@ export function createAgentJournal(deps: { onEvent, sessionName: () => sessionName, sawReadyForMerge: () => sawReadyForMerge, - prDescription: () => prDescription, + pullRequest: () => pullRequest, stoppedCleanly: () => stoppedCleanly, announceBrowserPort: port => { pendingBrowserPort = port @@ -1044,14 +1045,19 @@ async function driveAgent(opts: AgentOptions, io: CliIO): Promise { // closing phrase in it would close the ticket's issue on merge — which is exactly what // happened on #1560. The same reasoning already keeps `(fix #N)` off a plan agent's title // just above; the description is the other half of the same rule. - const written = journal.prDescription() - const description = written && opts.planAgent ? defuseClosingKeywords(written) : written + const written = journal.pullRequest() + // Both halves are defused, not just the body: since #1618 the title is the agent's prose too, + // and a closing phrase there would ride the squash-merge subject straight into the issue. + const defuse = (text: string | undefined) => (text && opts.planAgent ? defuseClosingKeywords(text) : text) + const prTitle = defuse(written?.title) + const description = defuse(written?.description) const agent = { id: opts.agentId ?? '', branch, ...(sessionName ? { sessionName } : {}), ...(intent ? { intent } : {}), ...(fixes ? { fixes } : {}), + ...(prTitle ? { prTitle } : {}), ...(description ? { description } : {}), } const handedOff = await agentAutoHandoff(cwd, agent, armed) diff --git a/packages/the-framework/src/closing-keywords.SPEC.md b/packages/the-framework/src/closing-keywords.SPEC.md index 2004e203b..e3338ad40 100644 --- a/packages/the-framework/src/closing-keywords.SPEC.md +++ b/packages/the-framework/src/closing-keywords.SPEC.md @@ -2,7 +2,7 @@ Why a pull request that does not finish an issue must not carry a phrase GitHub ## User Stories -- The user merges a plan's pull request and the ticket it discusses stays open, even though the plan's own text says the work will close it. +- The user merges a plan's pull request and the ticket it discusses stays open, even though the plan's own text — its title as much as its body — says the work will close it. - The user follows the issue reference in that sentence, and finds it still links to the issue — and the issue still shows that the pull request mentioned it. ## Flows diff --git a/packages/the-framework/src/daemon-services.SPEC.md b/packages/the-framework/src/daemon-services.SPEC.md index eb44b4b69..495aef547 100644 --- a/packages/the-framework/src/daemon-services.SPEC.md +++ b/packages/the-framework/src/daemon-services.SPEC.md @@ -5,6 +5,7 @@ Everything the daemon runs in the background beside serving the dashboard: Disco - The user toggles a background service on the dashboard and it takes effect without restarting the daemon. - The user pastes a Discord webhook into the dashboard and notifications start immediately. - The user walks away and idle quota is spent on the roadmap: unattended agents drain the confirmed queue, CI-green pull requests are merged, and red ones get a fix agent. +- The user finds unattended work held back, and the reason said, when the model that work would run on has spent its own weekly allowance — rather than a stack of runs that died at their first request. - The user reads on this machine what other machines and cloud sessions pushed, without waiting. - The user sees a project flagged when its shared data cannot reach origin, and the flag clears with the first sync that converges. @@ -15,6 +16,7 @@ Everything the daemon runs in the background beside serving the dashboard: Disco - An agent the daemon starts resolves its options from the same two tiers a dashboard start uses — the user's settings, then the repo's committed file — so an agent nobody asked for and one someone clicked differ only in who asked. - Auto PM spends idle quota on the roadmap: it fans out up to the configured number of unattended agents, each pinned to one queue entry. An entry is retired on the data branch (the dedicated branch the framework's shared records live on) once its agent's ending reports the work published — and it is the daemon, never the agent, that writes queue check-offs and ticket locks. - The CI watch merges a watched PR once its checks pass, and puts a fix agent on one whose checks fail. +- Every job that starts work on its own measures the account against the model that start would use, resolved from the same two tiers the start itself resolves. A model whose own weekly allowance is spent therefore stands the work down, however much of the account's week is left. The fix half says its stand-down once per failing head commit rather than on every turn of the clock. - An hourly sweep deletes the dead refs Claude-web hand-offs leave on origin, once they are old enough and provably hold no work. - Settled web runs are matched to the `claude/*` branch that grew out of their hand-off, and the branch and its PR are adopted onto the run's record — with the armed draft PR opened when the session never opened one. - The Discord notification watchers are rebuilt when the webhook changes, so a value pasted into the dashboard works immediately. diff --git a/packages/the-framework/src/daemon-services.test.SPEC.md b/packages/the-framework/src/daemon-services.test.SPEC.md index 77165f8c1..b25a5e765 100644 --- a/packages/the-framework/src/daemon-services.test.SPEC.md +++ b/packages/the-framework/src/daemon-services.test.SPEC.md @@ -1,4 +1,4 @@ -Covers the concurrent-agents setting end to end over a real data branch: the number stored on disk is the number of unattended agents the auto-PM sweep fans out, one pinned queue entry each with its claim committed on the branch, whether the sweep fires on schedule or from the dashboard's run-now click with auto-run off. Also covers one project's data-sync turn: a repository whose data branch cannot reach a remote is recorded as a data-sync error, and the error is cleared by the first sync that converges. +Covers the concurrent-agents setting end to end over a real data branch: the number stored on disk is the number of unattended agents the auto-PM sweep fans out, one pinned queue entry each with its claim committed on the branch, whether the sweep fires on schedule or from the dashboard's run-now click with auto-run off. Also covers the quota gate every self-starting job passes through: with the account's own week under its line but the chosen model's week spent, the sweep starts nothing and says which window held it back — while the same reading starts work as usual for a project whose model still has its own allowance. Also covers one project's data-sync turn: a repository whose data branch cannot reach a remote is recorded as a data-sync error, and the error is cleared by the first sync that converges. ## Before modifying/creating SPEC.md files diff --git a/packages/the-framework/src/daemon-services.test.ts b/packages/the-framework/src/daemon-services.test.ts index 3c7f2f599..6d62d9af2 100644 --- a/packages/the-framework/src/daemon-services.test.ts +++ b/packages/the-framework/src/daemon-services.test.ts @@ -8,8 +8,9 @@ import { promisify } from 'node:util' import { startBackgroundServices, syncProjectData } from './daemon-services.js' import { projectErrorStore } from './project-errors.js' import { dataWorktreePath, withDataBranch } from './data-branch.js' -import { quotaBoundaryStatus } from './quota-boundary.js' -import type { QuotaSource, QuotaView } from './dashboard/quota.js' +import { pollerQuotaSource, type QuotaSource } from './dashboard/quota.js' +import { QuotaPoller } from './quota-poller.js' +import type { DriverQuotaWindow } from './driver/index.js' import type { StartAgentOptions, StartAgentResult } from './dashboard/types.js' /** @@ -24,26 +25,6 @@ import type { StartAgentOptions, StartAgentResult } from './dashboard/types.js' * is about how many agents are asked for and with what, not about the child processes. */ -/** A reading with room to spare, so the quota gate is never the reason a start did not happen. */ -function spareQuota(): QuotaSource { - // Placed against the real clock, since the sweep's own `now` is not injectable from here: the - // week resets four days out, so ~43% of it has elapsed and 1% used is nowhere near the line. - const resets = new Date(Date.now() + 4 * 24 * 60 * 60 * 1000) - const month = ['Jan', 'Feb', 'Mar', 'Apr', 'May', 'Jun', 'Jul', 'Aug', 'Sep', 'Oct', 'Nov', 'Dec'][resets.getUTCMonth()] - const windows = [ - { - label: 'Current week (all models)', - kind: 'week' as const, - percentUsed: 1, - resetsAtText: `${month} ${resets.getUTCDate()} at 7am (UTC)`, - }, - ] - const boundary = quotaBoundaryStatus({ windows, now: Date.now() }) - if (!boundary) throw new Error('the fixture week should be placeable') - const view: QuotaView = { windows, boundary, readAt: Date.now() } - return { read: async () => view, stop: () => {} } -} - /** Six open entries, distinguishable, in the format the sweep's reader actually parses. */ const QUEUE_ENTRIES = [ '[Entry one](tickets/2026-07-01_one.md) — the first thing', @@ -56,8 +37,27 @@ const QUEUE_ENTRIES = [ const git = promisify(execFile) +/** + * A quota source over a real reading, built the way the daemon builds its own (#1619): the poller + * and `pollerQuotaSource` are the production ones, and only the driver call is stubbed. A hand-made + * fake would answer whatever the test wanted here, which is exactly how a gate that never passed a + * model went a year without anyone noticing. + */ +async function accountQuota(windows: DriverQuotaWindow[]): Promise { + const poller = new QuotaPoller({ read: async () => ({ available: true, windows }) }) + await poller.poll() + return pollerQuotaSource(poller) +} + +/** A week that resets four days out, so ~43% of it has elapsed and the boundary sits there. */ +function weekResetText(): string { + const resets = new Date(Date.now() + 4 * 24 * 60 * 60 * 1000) + const month = ['Jan', 'Feb', 'Mar', 'Apr', 'May', 'Jun', 'Jul', 'Aug', 'Sep', 'Oct', 'Nov', 'Dec'][resets.getUTCMonth()] + return `${month} ${resets.getUTCDate()} at 7am (UTC)` +} + /** A registry + checkout wired to `startBackgroundServices`, with every start recorded. */ -async function services(preferences: Record) { +async function services(preferences: Record, quota?: QuotaSource) { const config = await mkdtemp(join(tmpdir(), 'framework-concurrency-cfg-')) const project = await mkdtemp(join(tmpdir(), 'framework-concurrency-proj-')) // A real git checkout whose tickets and queue live on the data branch (#1582), because the @@ -91,7 +91,9 @@ async function services(preferences: Record) { cwd: project, env: { XDG_CONFIG_HOME: config }, dashboardUrl: 'http://localhost:4000', - quota: spareQuota(), + // A reading with room to spare, unless the test is about the gate itself: 1% used against a + // week that is ~43% elapsed is nowhere near the line. + quota: quota ?? (await accountQuota([{ label: 'Current week (all models)', kind: 'week', percentUsed: 1, resetsAtText: weekResetText() }])), startAgent: async (prompt, options, projectId): Promise => { starts.push({ prompt, options, projectId }) return { ok: true, agentId: `run-${starts.length}` } @@ -296,3 +298,44 @@ test('a project whose data branch cannot reach a remote carries a data-sync erro await rm(remote, { recursive: true, force: true }) } }) + +test("unattended work stands down when the model it would run on has spent its own week (#1619)", async () => { + // The account has most of its week left; the model the runs would use has none of its own. + // Before this was wired, the sweep read the account's 30%, started its batch, and every agent in + // it died at its first API call with "limit reached". + const quota = await accountQuota([ + { label: 'Current week (all models)', kind: 'week', percentUsed: 30, resetsAtText: weekResetText() }, + { label: 'Current week (Fable)', kind: 'week-model', percentUsed: 100, resetsAtText: weekResetText() }, + ]) + const { starts, stop, services: running } = await services({ autoPm: true, autoPmConcurrency: 4, model: 'claude-fable-5' }, quota) + try { + // Waited on the sweep's own report rather than a bare delay: the assertion is that the pass ran + // and decided not to start, which a timeout could not tell from a pass that had not run yet. + await settle(() => running.autoPmReport().outcomes.length > 0) + assert.equal(starts.length, 0, 'a spent model week starts nothing, however much of the account week is left') + // And it says which window stopped it: "the quota" alone would send someone to a panel that + // is showing 30% and looking fine. + const outcome = running.autoPmReport().outcomes[0] + assert.equal(outcome?.started, false) + assert.match(outcome?.message ?? '', /Current week \(Fable\) is 100% used/) + } finally { + await stop() + } +}) + +test('the same spent model week does not stop work on a model that has its own allowance left (#1619)', async () => { + // The gate is the model's week, not any model's week: with the preference on Opus, Fable being + // spent is none of this run's business — and the account's own week is still under its line. + const quota = await accountQuota([ + { label: 'Current week (all models)', kind: 'week', percentUsed: 30, resetsAtText: weekResetText() }, + { label: 'Current week (Fable)', kind: 'week-model', percentUsed: 100, resetsAtText: weekResetText() }, + ]) + const { starts, stop } = await services({ autoPm: true, autoPmConcurrency: 2, model: 'claude-opus-5' }, quota) + try { + await settle(() => starts.length >= 2) + assert.equal(starts.length, 2, 'the batch goes out as usual') + assert.equal(starts[0]!.options.model, 'claude-opus-5', 'and on the model the gate was measured for') + } finally { + await stop() + } +}) diff --git a/packages/the-framework/src/daemon-services.ts b/packages/the-framework/src/daemon-services.ts index 2b33fbcea..a072485dd 100644 --- a/packages/the-framework/src/daemon-services.ts +++ b/packages/the-framework/src/daemon-services.ts @@ -169,6 +169,17 @@ export function startBackgroundServices(deps: BackgroundServiceDeps): Background return deps.startAgent(prompt, { ...options, ...extra, unattended: true }, projectId) } + /** + * Where the account stands against the boundary for the run {@link startUnattended} would make + * on this project (#1619) — the same resolve, asked for the model rather than for the options. + * + * Both self-starting gates go through here for the same reason both starts go through + * `startUnattended`: a gate that measured a different model than the one about to run would + * clear a window that is already spent, and the run would die at its first API call. + */ + const quotaFor = async (projectId: string) => + deps.quota.boundaryFor((await resolveProjectAgentOptions(projectId, env)).model) + // Auto PM (#685/#773): while the queue is dry and there is quota to spare, triage and // plan tickets rather than let the day's allowance expire unused. const autoPm = startAutoPm({ @@ -185,8 +196,10 @@ export function startBackgroundServices(deps: BackgroundServiceDeps): Background // How many agents the routine may keep going per project (#1204). Global like the opt-outs; // the sweep applies the default when it is unset. concurrency: async () => (await prefs()).autoPmConcurrency, - // The quota boundary is the gate (#879): auto PM has no budget notion of its own. - quota: async () => (await deps.quota.read()).boundary, + // The quota boundary is the gate (#879): auto PM has no budget notion of its own. Measured for + // the project's own model (#1619), so a spent model week stands the sweep down here instead of + // letting it start runs that die at their first API call. + quota: project => quotaFor(project.id), // The periodic codebase sweep (#882). The schedule is a file in the project checkout rather // than loop state, because unlike the rotation it has to survive a daemon restart: a machine // rebooted daily would otherwise sweep every morning and never reach its interval. @@ -294,6 +307,10 @@ export function startBackgroundServices(deps: BackgroundServiceDeps): Background log, }) + // The stand-downs the fix half has already said, keyed `\0\0` like the + // sweep's own attempted-merge set: in memory, so a restart says it once more. + const quotaSaid = new Set() + // Watch the PRs the framework is waiting to land (#1418): merge a `watched` PR once its checks // pass (the #1417/#1406 answer for repos without GitHub auto-merge), and put an agent on a // watched PR whose checks fail. The merge half runs ungated — it finishes a merge the agent was @@ -306,10 +323,22 @@ export function startBackgroundServices(deps: BackgroundServiceDeps): Background deps: { fix: async (cwd, request) => { if ((await prefs()).autoPm !== true) return undefined - const boundary = (await deps.quota.read().catch(() => undefined))?.boundary - if (!quotaHeadroom(boundary).start) return undefined const project = (await projects()).find(p => p.path === cwd) if (!project) return undefined + // Resolved before the meter is read, because the model the fix would run on is what the + // meter has to be measured against (#1619). + const headroom = quotaHeadroom(await quotaFor(project.id).catch(() => undefined)) + if (!headroom.start) { + // Said once per failing head, the same re-arm rule the fix half's own restraint uses + // (#1418): a stand-down that repeated every tick would drown the log for as long as the + // PR stayed red, and one that said nothing at all is what made this invisible (#1619). + const key = `${cwd}\0${request.number}\0${request.headSha}` + if (!quotaSaid.has(key)) { + quotaSaid.add(key) + log(`[framework] CI watch: not starting a fix for PR #${request.number} — ${headroom.reason}`) + } + return undefined + } // The fix lands on the red PR's own branch, so this agent's handoff must not push or open // anything of its own. const result = await startUnattended(project.id, ciFixPrompt(request), { handoff: 'local' }) diff --git a/packages/the-framework/src/dashboard/agent-handoff.SPEC.md b/packages/the-framework/src/dashboard/agent-handoff.SPEC.md index 898123b9b..515cab4b1 100644 --- a/packages/the-framework/src/dashboard/agent-handoff.SPEC.md +++ b/packages/the-framework/src/dashboard/agent-handoff.SPEC.md @@ -4,7 +4,7 @@ Hands a finished agent's work back to the user: measures what its branch holds, - The user finds a settled agent's branch pushed and a draft pull request opened for it, without pressing anything. - The user is told when an agent produced nothing; an empty agent is never published. -- The user reads a pull request written by the agent that did the work, instead of a restatement of the request the agent was given. +- The user reads a pull request written by the agent that did the work — titled with what the change does, not with the instruction the agent was given — instead of a restatement of the request. - The user pushes, opens the PR, or merges by hand from the handoff panel — the merge button lands the draft an automatic merge withheld. - The user arms an automatic merge in configuration, and it still waits for the agent's own ready signal. - The user sees a cloud agent's pull request even though its branch never existed on this machine. @@ -14,6 +14,7 @@ Hands a finished agent's work back to the user: measures what its branch holds, - The read is addressed by branch, not by checkout: an agent reads the same whether or not its checkout still exists. A branch gone locally still reports its pull request — a hands-off cloud agent only ever pushed to the remote. - An agent that produced nothing — no commits, or changes only to the framework's own records — is reported as empty and never published. - The pull request describes the work in the agent's own words, when the agent wrote a description for it; otherwise it repeats what the user asked for, which is all the framework knows by itself. +- The pull request is titled with the agent's own name for the work, else with the session's name, else with the session's id. The prompt the session was given is never a title: a squash merge turns the title into the permanent commit subject, and an instruction cut mid-sentence describes neither what changed nor a whole thought. - Push and a draft PR are armed by default; drafts keep the automatic path out of reviewers' inboxes. Whatever the agent left uncommitted is swept into a commit first, guarded so only the agent's own checkout and branch are ever committed. - The PR number is recorded on the agent the moment one is opened for it, so every surface reads the same integer instead of re-deriving it. Its *state* is still read live, because that changes without the agent doing anything. - A pull request opened after the agent's process is gone is recorded too, by patching the agent's archived record: it is the same fact, and a surface should not have to know which of the two paths produced it. diff --git a/packages/the-framework/src/dashboard/agent-handoff.test.SPEC.md b/packages/the-framework/src/dashboard/agent-handoff.test.SPEC.md index 9168f01d7..cdbd5020d 100644 --- a/packages/the-framework/src/dashboard/agent-handoff.test.SPEC.md +++ b/packages/the-framework/src/dashboard/agent-handoff.test.SPEC.md @@ -1,4 +1,4 @@ -The tests cover the whole handoff story: reading a branch's work (empty, bookkeeping-only, gone, unpushed, and no-remote cases, against fakes and real repos), push and PR-opening with git's own reason on failure, the push-free draft PR for a remote-only branch with gh's refusal reported rather than thrown, the armed push/draft-PR/merge combinations including never opening a second PR, the recorded branch and PR winning over re-derivation, the PR body carrying the agent's own description of the work and falling back to what was asked for when the agent wrote none, merge authorization, and the human Merge action with its refusals. +The tests cover the whole handoff story: reading a branch's work (empty, bookkeeping-only, gone, unpushed, and no-remote cases, against fakes and real repos), push and PR-opening with git's own reason on failure, the push-free draft PR for a remote-only branch with gh's refusal reported rather than thrown, the armed push/draft-PR/merge combinations including never opening a second PR, the recorded branch and PR winning over re-derivation, the PR body carrying the agent's own description of the work and falling back to what was asked for when the agent wrote none, the PR title taken from the agent's own name for the work and falling back to the session's id — never to the prompt it was given — with the ticket's issue reference riding along either way, merge authorization, and the human Merge action with its refusals. ## Before modifying/creating SPEC.md files diff --git a/packages/the-framework/src/dashboard/agent-handoff.test.ts b/packages/the-framework/src/dashboard/agent-handoff.test.ts index 3773e282f..e4e3caae2 100644 --- a/packages/the-framework/src/dashboard/agent-handoff.test.ts +++ b/packages/the-framework/src/dashboard/agent-handoff.test.ts @@ -889,6 +889,42 @@ test("a run implementing a ticket carries its issue as `(fix #42)` in the PR tit assert.equal(title, 'fix-login (fix #42)') }) +async function titleOf(agent: Parameters[1]): Promise { + const gh: string[][] = [] + const { git } = fakeGit({ ...READY, push: '' }) + await agentAutoHandoff('/repo', agent, { push: true, pr: true }, { + git, + pr: async () => undefined, + gh: async args => { + gh.push(args) + return 'https://github.com/o/r/pull/9\n' + }, + }) + return gh[0]?.[gh[0].indexOf('--title') + 1] +} + +test("the PR is titled with the agent's own name for the work (#1618)", async () => { + // The first line of its `open-pr` block: the one rung that says what the change turned out to + // be, in a whole sentence, rather than the slug the session happens to be called. + const title = await titleOf({ + id: 'r1', + branch: 'the-framework/x', + sessionName: 'queue-reader', + prTitle: 'Keep the queued state across a reload', + fixes: '#42', + }) + assert.equal(title, 'Keep the queued state across a reload (fix #42)') +}) + +test('a session that named nothing gets its id as the title, not the prompt it was given (#1618)', async () => { + // The prompt used to be the middle rung, cut to 72 characters. The squash merge made that a + // permanent commit subject: an instruction, truncated mid-sentence, standing in for a + // description of the change. The session id says less and misleads nobody. + const intent = 'Open TODO_AGENTS.md and work on the FIRST open entry only. When the work is done, close the ticket it links to.' + assert.equal(await titleOf({ id: 'r1', branch: 'the-framework/x', intent }), 'Session r1') + assert.equal(await titleOf({ id: 'r1', branch: 'the-framework/x', intent, fixes: '#1' }), 'Session r1 (fix #1)') +}) + test("the Merge action merges the session's open PR, marking a draft ready on the way (#1391)", async () => { const gh: string[][] = [] const result = await mergeAgentPr( diff --git a/packages/the-framework/src/dashboard/agent-handoff.ts b/packages/the-framework/src/dashboard/agent-handoff.ts index 4bd7cc356..5d369c13e 100644 --- a/packages/the-framework/src/dashboard/agent-handoff.ts +++ b/packages/the-framework/src/dashboard/agent-handoff.ts @@ -743,6 +743,11 @@ export type HandoffAgent = Pick): string { - const title = agent.sessionName ?? agent.intent?.split('\n')[0]?.slice(0, 72) ?? `Session ${agent.id}` +/** + * The PR title for a session (#1102), with the ticket's issue reference riding along (#1334). + * + * Three rungs, each a name for the work the session did: what the agent called it in its + * `open-pr` block (#1618), else the session's own name, else the session id — which says little, + * but says it honestly. + * + * The prompt the session was given is not among them. It used to be, cut to 72 characters, and a + * squash merge made that permanent: `main` ended up carrying instructions truncated mid-sentence + * as commit subjects, which describe neither what changed nor even a whole thought (#1618). + */ +function agentPrTitle(agent: Pick): string { + const title = agent.prTitle ?? agent.sessionName ?? `Session ${agent.id}` return agent.fixes ? `${title} (fix ${agent.fixes})` : title } diff --git a/packages/the-framework/src/dashboard/quota.SPEC.md b/packages/the-framework/src/dashboard/quota.SPEC.md index 4a471e490..8e4f90224 100644 --- a/packages/the-framework/src/dashboard/quota.SPEC.md +++ b/packages/the-framework/src/dashboard/quota.SPEC.md @@ -1,16 +1,18 @@ -Feeds the dashboard's usage panel: the account's quota windows and where they stand against the spending boundary (the line past which unattended work stands down). +Feeds the dashboard's usage panel: the account's quota windows and where they stand against the spending boundary (the line past which unattended work stands down). It also answers, for work about to start, where the account stands once the model that work will run on is named. ## User Stories - The user sees where the account's quota stands at any moment, agent running or not. - The user moves the spend-limit slider and both the panel and automation follow, with no restart. - The user can tell a stale or missing reading from real zero usage. +- The user's unattended work is held back when the model it would run on has spent that model's own weekly allowance, even while the account still has allowance left. ## Flows - The daemon polls for the dashboard's whole life, not just while an agent is up — the panel must answer while nothing is running. - A failed reading never blanks the panel: the last good reading is kept and marked stale, and "no reading at all" is reported as such rather than as zero usage. - The boundary is recomputed on every read, because it moves with the clock. The user's spend-limit slider is read fresh each time, so moving it needs no restart. One source feeds both, so the bar the user sees and the line automation obeys cannot disagree. +- Two questions are asked of that one reading. The panel asks where the *account* stands, and names no model: the bar is about the account. Work about to start asks where the account stands *for the model that work will run on*, so that model's own weekly window binds alongside the account's. Naming no model is the account's week alone — with no model chosen, the coding agent picks, and a window nobody can tie to the work must never stop it. ## Before modifying/creating SPEC.md files diff --git a/packages/the-framework/src/dashboard/quota.test.SPEC.md b/packages/the-framework/src/dashboard/quota.test.SPEC.md index 9d2fbadc6..a528d1f01 100644 --- a/packages/the-framework/src/dashboard/quota.test.SPEC.md +++ b/packages/the-framework/src/dashboard/quota.test.SPEC.md @@ -1,4 +1,4 @@ -The tests cover the usage view: windows plus boundary standing, keeping and marking a stale reading through a blip, reporting no reading rather than an empty one, a boundary that follows the clock, an unplaceable week left unguessed, the default spend cushion, and stopping the poll. +The tests cover the usage view: windows plus boundary standing, keeping and marking a stale reading through a blip, reporting no reading rather than an empty one, a boundary that follows the clock, an unplaceable week left unguessed, the default spend cushion, and stopping the poll. They also cover the two questions asked of one reading: the panel's bar stays about the account and a spent model week does not move it, naming the model work will run on brings that model's own week into the gate, and a model the account reported no window for is measured on the account alone. ## Before modifying/creating SPEC.md files diff --git a/packages/the-framework/src/dashboard/quota.test.ts b/packages/the-framework/src/dashboard/quota.test.ts index ac8473b0a..6efbc90ba 100644 --- a/packages/the-framework/src/dashboard/quota.test.ts +++ b/packages/the-framework/src/dashboard/quota.test.ts @@ -96,3 +96,57 @@ test('with no slider position given, the limit defaults to a half-day cushion ab assert.equal(view.boundary?.limit.offset, DEFAULT_SPEND_OFFSET) assert.equal(view.boundary?.limit.percent, view.boundary!.boundary.percent + DEFAULT_SPEND_OFFSET) }) + +/** + * The account with room to spare on the week overall, and one model's own week spent (#1619). + * + * The live shape this came from, with the account's number moved under the line: on day 3 of 7 the + * week's own 54% was already past its limit, which would have hidden the very thing under test. + */ +function spentModelWeek(): DriverQuota { + return { + available: true, + windows: [ + { label: 'Current session', kind: 'session', percentUsed: 4, resetsAtText: 'in 2h 53m' }, + { label: 'Current week (all models)', kind: 'week', percentUsed: 30, resetsAtText: 'Jul 25 at 7am (UTC)' }, + { label: 'Current week (Fable)', kind: 'week-model', percentUsed: 100, resetsAtText: 'Jul 25 at 7am (UTC)' }, + ], + } +} + +test('the panel stays about the account: a spent model week is not the bar the user reads (#533/#879)', async () => { + const { source, poller } = sourceOf([spentModelWeek()]) + await poller.poll() + const view = await source.read() + // All three windows are shown — the panel draws what the account reported. + assert.equal(view.windows.length, 3) + // But only the account's own week is measured, so a model nobody named cannot make the bar + // say the account is out of allowance when the week itself is only 30% spent. + assert.deepEqual(view.boundary?.windows.map(w => w.label), ['Current week (all models)']) + assert.equal(view.boundary?.reached, null) +}) + +test("naming the model the work will run on brings that model's own week into the gate (#1619)", async () => { + const { source, poller } = sourceOf([spentModelWeek()]) + await poller.poll() + // The question a start asks, as against the question the panel asks: same reading, same slider, + // one more window in force — the one the work would actually spend. + const boundary = await source.boundaryFor('claude-fable-5') + assert.deepEqual(boundary?.windows.map(w => w.label), ['Current week (all models)', 'Current week (Fable)']) + assert.equal(boundary?.reached?.label, 'Current week (Fable)') + // And the account-wide answer is still the account-wide answer: this is a second question, not + // a replacement for the first. + assert.equal((await source.read()).boundary?.reached, null) +}) + +test('a model whose week the account never reported is gated on the account alone (#1619)', async () => { + const { source, poller } = sourceOf([spentModelWeek()]) + await poller.poll() + // Opus has no window here, so there is nothing of its own to measure and the spent Fable week + // is none of its business: a window we cannot tie to the model must not stop work (#879). + const boundary = await source.boundaryFor('claude-opus-5') + assert.deepEqual(boundary?.windows.map(w => w.label), ['Current week (all models)']) + assert.equal(boundary?.reached, null) + // No model at all is the same answer as the panel's. + assert.deepEqual((await source.boundaryFor())?.windows, (await source.read()).boundary?.windows) +}) diff --git a/packages/the-framework/src/dashboard/quota.ts b/packages/the-framework/src/dashboard/quota.ts index b2f6c7471..34f5b7ece 100644 --- a/packages/the-framework/src/dashboard/quota.ts +++ b/packages/the-framework/src/dashboard/quota.ts @@ -34,6 +34,22 @@ export interface QuotaView { /** Where a dashboard reads the quota from. */ export interface QuotaSource { read(): Promise + /** + * Where the account stands once the model the work will run on is named (#1619). + * + * A different question from {@link QuotaView.boundary}, and deliberately a second method rather + * than an argument to {@link QuotaSource.read}: the panel's boundary is about the account, this + * one is about one impending run, and a shared entry point that answered both would leave the + * panel's call looking like it had simply forgotten to pass a model — which is exactly how the + * model's own week came to be filtered out of every gate for a year. + * + * Both are measured off the same reading and the same slider, so #960's one-source promise + * holds: they cannot disagree about the account, only about a question the other never asked. + * + * No model given is the account's week alone. That is not a shortcut: with no model preference + * set the driver picks, and a window we cannot name the model of must not stop work (#879). + */ + boundaryFor(model?: string): Promise /** Stop any polling behind it. */ stop(): void } @@ -41,10 +57,12 @@ export interface QuotaSource { /** * A {@link QuotaSource} backed by a live poller. * - * The boundary is computed per call rather than captured: it moves with the - * clock, so a cached one would be stale the moment the week's day rolls over. - * No model is passed — the panel is about the account, and a model's own week - * only narrows the gate for an agent that has chosen one (#879). + * Both questions are measured per call rather than captured: the boundary moves with the clock, + * so a cached one would be stale the moment the week's day rolls over. Neither costs a reading — + * the poller's last good windows are measured again, so asking per project is free. + * + * The panel's own boundary names no model on purpose: the bar is about the account, and a model's + * own week only narrows the gate for a run that has chosen one (#879/#1619). */ export function pollerQuotaSource( poller: QuotaPoller, @@ -52,12 +70,17 @@ export function pollerQuotaSource( /** The user's slider position, read per call so moving it takes effect without a restart (#960). */ limitOffset: () => number | Promise = () => DEFAULT_SPEND_OFFSET, ): QuotaSource { + const measure = async (model?: string) => { + const windows = poller.current().lastGood?.windows ?? [] + return quotaBoundaryStatus({ windows, now: now(), ...(model ? { model } : {}), limitOffset: await limitOffset() }) + } return { stop: () => poller.stop(), + boundaryFor: model => measure(model), read: async () => { const envelope = poller.current() const windows = envelope.lastGood?.windows ?? [] - const boundary = quotaBoundaryStatus({ windows, now: now(), limitOffset: await limitOffset() }) + const boundary = await measure() const view: QuotaView = { windows, ...(boundary ? { boundary } : {}), diff --git a/packages/the-framework/src/e2e/harness.ts b/packages/the-framework/src/e2e/harness.ts index 40cd9f94b..839ce9ff4 100644 --- a/packages/the-framework/src/e2e/harness.ts +++ b/packages/the-framework/src/e2e/harness.ts @@ -154,7 +154,9 @@ export async function makeWorld(): Promise { remote: runtime.remoteAgents, preferences: registryPreferencesStore(), discord: registryDiscordCredentialsStore(), - quota: { read: async () => quota.view, stop: () => {} }, + // The story sets one view; both questions are answered off it, since a story that cares about + // 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 }) => { autoPm.sweeps.push(opts ?? {}) diff --git a/packages/the-framework/src/events.ts b/packages/the-framework/src/events.ts index fd0b5610e..f3a874135 100644 --- a/packages/the-framework/src/events.ts +++ b/packages/the-framework/src/events.ts @@ -220,12 +220,13 @@ export type FrameworkEvent = */ | { kind: 'ready-for-merge' } /** - * The description the agent wrote for its pull request (#1567), via an `open-pr` block: how - * an agent opens a PR *through* the framework instead of running `gh pr create` itself, so - * the title convention, the ticket's issue reference and recording the number all still - * apply. Non-blocking; the end-of-agent handoff uses the latest one as the PR's body. + * The pull request the agent asked for (#1567/#1618), via an `open-pr` block: how an agent + * opens a PR *through* the framework instead of running `gh pr create` itself, so the ticket's + * issue reference and recording the number still apply. The title is the agent's name for the + * work and the description is what changed; either may be absent when the agent wrote only the + * other. Non-blocking; the end-of-agent handoff uses the latest one. */ - | { kind: 'pull-request-description'; description: string } + | { kind: 'open-pr'; title?: string; description?: string } /** * The #326 post-merge cleanup step settled (#835): it queued the quality follow-ups, * queued them but did not finish cleanly, or declined with a {@link OnBeforeMergeableSkip}. diff --git a/packages/the-framework/src/terminal.ts b/packages/the-framework/src/terminal.ts index 512be4350..9fa459af6 100644 --- a/packages/the-framework/src/terminal.ts +++ b/packages/the-framework/src/terminal.ts @@ -31,8 +31,8 @@ export function formatFrameworkEvent(event: FrameworkEvent): string { return ` session: ${event.name}` case 'ready-for-merge': return `✓ ready for merge` - case 'pull-request-description': - return ` pull request description written` + case 'open-pr': + return ` pull request written${event.title ? `: ${event.title}` : ''}` case 'settled': return `◆ done for now — waiting for your next message` case 'ticket': diff --git a/packages/the-framework/src/turn-gate.SPEC.md b/packages/the-framework/src/turn-gate.SPEC.md index 82f4788e3..6ec02c4f9 100644 --- a/packages/the-framework/src/turn-gate.SPEC.md +++ b/packages/the-framework/src/turn-gate.SPEC.md @@ -12,7 +12,7 @@ The turn-boundary contract with the wrapped agent: each turn runs as a black box - Parsing is tolerant on purpose: a malformed block is ignored rather than crashing an agent, the block appearing latest in the turn wins (falling back past a broken one), and missing ids and titles get sensible defaults. A block with nothing pickable in it is not a gate — the agent carries on rather than parking on an empty question. - One continuation wording resumes the agent after any answered gate, and a shared cap on ask-rounds stops an agent that keeps asking. - Signal emission is deduped across a span of turns: ready-for-merge fires once, and a session name or a pull-request description re-emits only on a real change. -- An agent describes its pull request in a block instead of opening one itself; the description is the body the end-of-agent handoff publishes, and the last one the agent wrote is the one used. +- An agent names and describes its pull request in a block instead of opening one itself, written like a commit message: the first line is the title the end-of-agent handoff publishes, the rest is the body, and the last block the agent wrote is the one used. A first line too long to be a name for the work is read as body text instead, so a paragraph never becomes a pull request title. ## Rationales diff --git a/packages/the-framework/src/turn-gate.test.SPEC.md b/packages/the-framework/src/turn-gate.test.SPEC.md index d85210eea..355ba1d94 100644 --- a/packages/the-framework/src/turn-gate.test.SPEC.md +++ b/packages/the-framework/src/turn-gate.test.SPEC.md @@ -1,4 +1,4 @@ -Covers the turn-signal parsing: the one ask-gate block with its tolerant defaults and latest-block-wins (falling back past a malformed block, and refusing a block with nothing pickable in it), the several-picks and plan-file gate variants, an option marked as ending the agent, markdown views, session-name slugging (a session legitimately named "view" is kept), ready-for-merge detection, the pull-request description an agent writes for the framework to publish (markdown kept whole, the last block winning, an empty one ignored), and the single continuation wording shared by every path. +Covers the turn-signal parsing: the one ask-gate block with its tolerant defaults and latest-block-wins (falling back past a malformed block, and refusing a block with nothing pickable in it), the several-picks and plan-file gate variants, an option marked as ending the agent, markdown views, session-name slugging (a session legitimately named "view" is kept), ready-for-merge detection, the pull request an agent writes for the framework to publish (its first line taken as the title and the rest as the body, a one-line block taken as a title alone, a first line too long to be a name for the work taken as body text instead, markdown kept whole, the last block winning, an empty one ignored), and the single continuation wording shared by every path. ## Before modifying/creating SPEC.md files diff --git a/packages/the-framework/src/turn-gate.test.ts b/packages/the-framework/src/turn-gate.test.ts index 8f8ba86a7..58a4ddb9e 100644 --- a/packages/the-framework/src/turn-gate.test.ts +++ b/packages/the-framework/src/turn-gate.test.ts @@ -1,6 +1,6 @@ import assert from 'node:assert/strict' import { test } from 'node:test' -import { continuationPrompt, parseAwaitGate, parseMarkdownViews, parseSessionName, parseReadyForMerge, parsePullRequestDescription } from './turn-gate.js' +import { continuationPrompt, parseAwaitGate, parseMarkdownViews, parseSessionName, parseReadyForMerge, parsePullRequest } from './turn-gate.js' const block = (json: string): string => 'Here are the options.\n```await-choices\n' + json + '\n```' @@ -161,29 +161,49 @@ test('parseReadyForMerge is true only when a ready-for-merge block is present (# -// #1567: the `open-pr` block is how an agent opens a pull request through the framework rather -// than by running `gh pr create` itself — the agent writes the description, the framework keeps -// the title, the ticket's issue reference, and the recorded number. -test('parsePullRequestDescription takes the block body as the PR description (#1567)', () => { - const text = 'Done.\n```open-pr\nRewrites the queue reader so a reload keeps the queued state.\n```\n' - assert.equal(parsePullRequestDescription(text), 'Rewrites the queue reader so a reload keeps the queued state.') +// #1567/#1618: the `open-pr` block is how an agent opens a pull request through the framework +// rather than by running `gh pr create` itself — the agent names and describes the work, the +// framework keeps the ticket's issue reference and the recorded number. Read like a commit +// message: first line the title, the rest the body. +test('parsePullRequest reads the block as a commit message: first line title, rest body (#1618)', () => { + const text = 'Done.\n```open-pr\nKeep the queued state across a reload\n\nThe reader re-read the file on mount.\n```\n' + assert.deepEqual(parsePullRequest(text), { + title: 'Keep the queued state across a reload', + description: 'The reader re-read the file on mount.', + }) }) -test('parsePullRequestDescription returns undefined when the agent wrote no block (#1567)', () => { - assert.equal(parsePullRequestDescription('just some output'), undefined) +test('parsePullRequest returns undefined when the agent wrote no block (#1567)', () => { + assert.equal(parsePullRequest('just some output'), undefined) }) -test('parsePullRequestDescription keeps markdown whole, since the block is the PR body (#1567)', () => { +test('parsePullRequest keeps the body markdown whole below the title (#1567)', () => { const body = '## What changed\n\n- one thing\n- another\n\nSee `src/thing.ts`.' - assert.equal(parsePullRequestDescription('```open-pr\n' + body + '\n```'), body) + assert.deepEqual(parsePullRequest('```open-pr\nRewrite the queue reader\n\n' + body + '\n```'), { + title: 'Rewrite the queue reader', + description: body, + }) }) -test('parsePullRequestDescription takes the last block, so the agent can revise it (#1567)', () => { +test('parsePullRequest takes a one-line block as a title with nothing below it (#1618)', () => { + // A name for the work and no more: the PR body then says what was asked for, as it does for + // any session that wrote no description. + assert.deepEqual(parsePullRequest('```open-pr\nRewrite the queue reader\n```'), { title: 'Rewrite the queue reader' }) +}) + +test('parsePullRequest refuses a first line too long to be a name for the work (#1618)', () => { + // A paragraph, not a title. Taken as body alone rather than cut to fit: a truncated sentence + // as a PR title becomes a truncated sentence in `main`'s history once the PR is squashed. + const paragraph = 'This change rewrites the queue reader so that a reload keeps the queued state, which it did not before because the reader re-read the file on mount.' + assert.deepEqual(parsePullRequest('```open-pr\n' + paragraph + '\n```'), { description: paragraph }) +}) + +test('parsePullRequest takes the last block, so the agent can revise it (#1567)', () => { const text = '```open-pr\nfirst\n```\nlater…\n```open-pr\nsecond\n```' - assert.equal(parsePullRequestDescription(text), 'second') + assert.deepEqual(parsePullRequest(text), { title: 'second' }) }) -test('parsePullRequestDescription ignores an empty block rather than blanking the body (#1567)', () => { - assert.equal(parsePullRequestDescription('```open-pr\n\n```'), undefined) - assert.equal(parsePullRequestDescription('```open-pr\nreal\n```\n```open-pr\n \n```'), 'real') +test('parsePullRequest ignores an empty block rather than blanking the body (#1567)', () => { + assert.equal(parsePullRequest('```open-pr\n\n```'), undefined) + assert.deepEqual(parsePullRequest('```open-pr\nreal\n```\n```open-pr\n \n```'), { title: 'real' }) }) diff --git a/packages/the-framework/src/turn-gate.ts b/packages/the-framework/src/turn-gate.ts index 434d370f5..f687ac8c7 100644 --- a/packages/the-framework/src/turn-gate.ts +++ b/packages/the-framework/src/turn-gate.ts @@ -182,23 +182,50 @@ export function parseSessionName(text: string): string | undefined { } /** - * Parse the pull-request description the agent wrote this turn (#1567), from the last - * non-empty `open-pr` block (per {@link SIGNAL_PROTOCOL}). Returns `undefined` when the agent - * wrote none, which is the common case and simply leaves the handoff describing the work - * itself. A later block in the same turn wins, so an agent may revise it as the work changes. + * The longest first line still readable as a title (#1618). Past this the agent wrote a + * paragraph, not a name for its work, and the block is taken as body alone — a pull request + * whose title runs to a paragraph is a squash-merge subject that runs to a paragraph. + */ +const MAX_PR_TITLE = 100 + +/** What the agent asked the framework to open a pull request with (#1567/#1618). */ +export interface ParsedPullRequest { + /** The agent's name for the work: the block's first line, when it reads as a title. */ + title?: string + /** What changed and why: everything after that line, or the whole block when there is no title. */ + description?: string +} + +/** + * Parse the pull request the agent asked for this turn (#1567), from the last non-empty + * `open-pr` block (per {@link SIGNAL_PROTOCOL}). Returns `undefined` when the agent wrote none, + * which simply leaves the handoff describing the work itself. A later block in the same turn + * wins, so an agent may revise it as the work changes. * * The block is how an agent opens a pull request *through the framework* rather than by - * reaching for `gh` itself: the description is the agent's, and the handoff keeps the parts - * that have to be consistent — the title, the ticket's issue reference, and recording the - * number on the agent. + * reaching for `gh` itself: the title and the description are the agent's, and the handoff keeps + * the parts that have to be consistent — the ticket's issue reference and recording the number + * on the agent. + * + * Shaped like a commit message, and read like one: the first line names the work, the rest + * describes it. A first line too long to be a name is not treated as one (#1618) — the block is + * all description then, and the title falls back to the session's name rather than being cut + * mid-sentence, which is how a raw prompt ended up as a permanent commit subject. */ -export function parsePullRequestDescription(text: string): string | undefined { - let description: string | undefined +export function parsePullRequest(text: string): ParsedPullRequest | undefined { + let parsed: ParsedPullRequest | undefined for (const body of blocks(text, 'open-pr')) { const trimmed = body.trim() - if (trimmed) description = trimmed + if (!trimmed) continue + const [first = '', ...rest] = trimmed.split('\n') + const title = first.trim() + const description = rest.join('\n').trim() + parsed = + title.length <= MAX_PR_TITLE + ? { title, ...(description ? { description } : {}) } + : { description: trimmed } } - return description + return parsed } /** @@ -311,10 +338,11 @@ export function createTurnSignalEmitter(emit: (event: FrameworkEvent) => void): ready = true emit({ kind: 'ready-for-merge' }) } - const description = parsePullRequestDescription(text) - if (description && description !== described) { - described = description - emit({ kind: 'pull-request-description', description }) + const pr = parsePullRequest(text) + const seen = pr && `${pr.title ?? ''}\n${pr.description ?? ''}` + if (pr && seen !== described) { + described = seen + emit({ kind: 'open-pr', ...pr }) } } }