diff --git a/FEATURES-SPEC.md b/FEATURES-SPEC.md index 6abfc01a5..1833364f0 100644 --- a/FEATURES-SPEC.md +++ b/FEATURES-SPEC.md @@ -72,6 +72,7 @@ happens while nobody is at the keyboard. - An agent is a URL you can paste, reload and bookmark - The agent names itself; the branch is renamed to match - Ready-for-merge flips the agent's badge +- The agent reports what it could not get past: a red line in the log where it hit it, and a running error count on the session - Live spend readout per agent - See the exact system prompt the agent ran under diff --git a/packages/the-framework/dashboard/components/AgentActionBar.tsx b/packages/the-framework/dashboard/components/AgentActionBar.tsx index 009e1cf30..1538e1af9 100644 --- a/packages/the-framework/dashboard/components/AgentActionBar.tsx +++ b/packages/the-framework/dashboard/components/AgentActionBar.tsx @@ -2,6 +2,7 @@ import type { ReactNode } from 'react' import type { FrameworkEvent } from '../../src/index.js' import { GitStatusBar } from './GitStatusBar.js' import { AgentActionsMenu } from './AgentActionsMenu.js' +import { AgentErrorCount } from './AgentErrorCount.js' import { agentStatusPill } from '../lib/agent-status.js' import { cn } from '../lib/utils.js' @@ -84,6 +85,10 @@ export function AgentActionBar({ grows but never shrinks (#1030), so a tight row takes its width from the label. */}
+ {/* What the agent could not get past (#1500): the log scrolls, this row does not. It sits + with the controls rather than among the branch facts, which give up width as the row + fills — a count is only useful if it is whole. */} + {/* The handoff's next step stays visible — the one thing here that moves the session forward rather than just opening it somewhere. Everything else is in the ⋮ menu. */} {actions} diff --git a/packages/the-framework/dashboard/components/AgentErrorCount.SPEC.md b/packages/the-framework/dashboard/components/AgentErrorCount.SPEC.md new file mode 100644 index 000000000..cc7210b6a --- /dev/null +++ b/packages/the-framework/dashboard/components/AgentErrorCount.SPEC.md @@ -0,0 +1,10 @@ +How many errors the agent reported, shown in the session's header — with the latest headline beside it, and every headline behind a hover. + +## Flows + +- Rendered where the header stays put while the log scrolls: the run's action bar and the project page's agent overview. A session that reported none shows nothing. +- The count only ever grows within a session, because each error is something that happened rather than a state that can be fixed; the errors themselves stay in the log, at the point in the run where the agent hit them. + +## Before modifying/creating SPEC.md files + +You must always read and respect https://raw.githubusercontent.com/brillout/sdd/refs/heads/main/sdd.md diff --git a/packages/the-framework/dashboard/components/AgentErrorCount.test.SPEC.md b/packages/the-framework/dashboard/components/AgentErrorCount.test.SPEC.md new file mode 100644 index 000000000..c0e2fb369 --- /dev/null +++ b/packages/the-framework/dashboard/components/AgentErrorCount.test.SPEC.md @@ -0,0 +1,5 @@ +Covers the header's error count: nothing for a session that reported none, the count and the latest headline once it has, and singular/plural wording. + +## Before modifying/creating SPEC.md files + +You must always read and respect https://raw.githubusercontent.com/brillout/sdd/refs/heads/main/sdd.md diff --git a/packages/the-framework/dashboard/components/AgentErrorCount.test.tsx b/packages/the-framework/dashboard/components/AgentErrorCount.test.tsx new file mode 100644 index 000000000..40f458678 --- /dev/null +++ b/packages/the-framework/dashboard/components/AgentErrorCount.test.tsx @@ -0,0 +1,38 @@ +import type { FrameworkEvent } from '../../src/index.js' +import { afterEach, describe, expect, test } from 'vitest' +import { cleanup, render, screen } from '@testing-library/react' +import { AgentErrorCount } from './AgentErrorCount.js' + +afterEach(cleanup) + +// The count is the session header's half of the error capability (#1500): the log carries the +// errors themselves, this says how many there were without the reader scrolling for them. +describe('AgentErrorCount', () => { + test('a session that reported no errors shows nothing', () => { + const { container } = render() + expect(container.textContent).toBe('') + }) + + test('one error reads singular, with its headline where the row has room', () => { + render() + expect(screen.getByText('1 error')).toBeTruthy() + expect(screen.getByText(/gh is not logged in/)).toBeTruthy() + }) + + test('a tight row shows the count alone — a clipped headline is worse than none', () => { + render() + expect(screen.getByText('1 error')).toBeTruthy() + expect(screen.queryByText(/gh is not logged in/)).toBeNull() + }) + + test('several errors read plural, and the LATEST headline is the one shown', () => { + const events: FrameworkEvent[] = [ + { kind: 'error', headline: 'first thing broke' }, + { kind: 'error', headline: 'second thing broke' }, + ] + render() + expect(screen.getByText('2 errors')).toBeTruthy() + expect(screen.getByText(/second thing broke/)).toBeTruthy() + expect(screen.queryByText(/first thing broke/)).toBeNull() + }) +}) diff --git a/packages/the-framework/dashboard/components/AgentErrorCount.tsx b/packages/the-framework/dashboard/components/AgentErrorCount.tsx new file mode 100644 index 000000000..464d8ba19 --- /dev/null +++ b/packages/the-framework/dashboard/components/AgentErrorCount.tsx @@ -0,0 +1,34 @@ +import { TriangleAlert } from 'lucide-react' +import type { FrameworkEvent } from '../../src/index.js' +import { agentErrors } from '../../src/client.js' +import { Tooltip, TooltipTrigger, TooltipContent } from './ui/tooltip.js' + +// How many errors the agent reported, said in the session's header (#1500) — on the run page's +// action bar and on the project page's overview, the two places that stay put while the log +// scrolls. The errors themselves stay in the log, at the point in the run where they happened; +// this is only the count, plus the last headline so it says *what* without being opened. +export function AgentErrorCount({ events, headline = false }: { events: FrameworkEvent[]; headline?: boolean }) { + const errors = agentErrors(events) + if (errors.length === 0) return null + const latest = errors[errors.length - 1]! + return ( + + + + + {errors.length} {errors.length === 1 ? 'error' : 'errors'} + + {/* Only where the row has room for it: in a tight bar the headline is the hover. */} + {headline && · {latest.headline}} + + } + /> + {/* Every headline, so a run with several says which — the detail stays in the log. */} + {errors.map(e => e.headline).join('\n')} + + ) +} diff --git a/packages/the-framework/dashboard/components/AgentOverview.SPEC.md b/packages/the-framework/dashboard/components/AgentOverview.SPEC.md index 43fcd7102..9c8a79294 100644 --- a/packages/the-framework/dashboard/components/AgentOverview.SPEC.md +++ b/packages/the-framework/dashboard/components/AgentOverview.SPEC.md @@ -1,4 +1,8 @@ -The agent-overview cards projected from the event stream — the status line and an honestly-labelled link to the live session — each rendered only once its data has arrived. +The agent-overview cards projected from the event stream — the status line, what the agent reported as broken, and an honestly-labelled link to the live session — each rendered only once its data has arrived. + +## Flows + +- The errors the agent reported are counted here, next to the latest of them, so a failure stays in sight after the log has scrolled past it. The errors themselves stay in the log, in their place in the story. ## Before modifying/creating SPEC.md files diff --git a/packages/the-framework/dashboard/components/AgentOverview.tsx b/packages/the-framework/dashboard/components/AgentOverview.tsx index dff76e62e..0d94a0928 100644 --- a/packages/the-framework/dashboard/components/AgentOverview.tsx +++ b/packages/the-framework/dashboard/components/AgentOverview.tsx @@ -1,5 +1,6 @@ import type { FrameworkEvent } from '../../src/index.js' -import { sessionInfo, agentProgress } from '../../src/client.js' +import { sessionInfo, agentProgress, agentErrors } from '../../src/client.js' +import { AgentErrorCount } from './AgentErrorCount.js' import { agentStatusPill } from '../lib/agent-status.js' import { describeSessionLink } from '../lib/session-link.js' import { cn } from '../lib/utils.js' @@ -13,13 +14,14 @@ export function AgentOverview({ events }: { events: FrameworkEvent[] }) { const session = sessionInfo(events) const progress = agentProgress(events) const status = agentStatusPill(events) + const errors = agentErrors(events) // The "Open session" link, labeled honestly: a headless Claude Code run has no per-session // URL, so the generic app entry (claude.ai/code) is shown as "Open Claude Code" with the id // surfaced separately, not as a deep link to that id. See {@link describeSessionLink}. const sessionLink = describeSessionLink(session) - if (!sessionLink && !status) return null + if (!sessionLink && !status && errors.length === 0) return null return (
@@ -30,6 +32,13 @@ export function AgentOverview({ events }: { events: FrameworkEvent[] }) { {status.label}
)} + {/* What went wrong, kept where the log cannot scroll it away (#1500). The rows themselves + stay in the log, at the point in the run where the agent hit them. */} + {errors.length > 0 && ( +
+ +
+ )} {sessionLink && ( { expect(row.className).toContain('text-danger') }) + test('an error the agent reported itself renders in red (#1500)', () => { + render() + expect(screen.getByText(/gh is not logged in/).className).toContain('text-danger') + }) + test('a failed run renders in red (#1199)', () => { render() expect(screen.getByText(/failed: exited 1/).className).toContain('text-danger') @@ -281,6 +286,11 @@ describe('EventList row wash (#1508)', () => { expect(container.querySelector('[class*="bg-info/10"]')).toBeTruthy() }) + test('an agent-reported error gets the red wash too (#1500)', () => { + const { container } = render() + expect(container.querySelector('[class*="bg-danger/10"]')).toBeTruthy() + }) + test('a failure gets the red wash', () => { const { container } = render() expect(container.querySelector('[class*="bg-danger/10"]')).toBeTruthy() diff --git a/packages/the-framework/dashboard/components/EventList.tsx b/packages/the-framework/dashboard/components/EventList.tsx index a9dfdb779..de6aca3ba 100644 --- a/packages/the-framework/dashboard/components/EventList.tsx +++ b/packages/the-framework/dashboard/components/EventList.tsx @@ -84,6 +84,9 @@ function isTurnBoundary(e: FrameworkEvent): boolean { */ function isFailure(e: FrameworkEvent): boolean { if (e.kind === 'driver') return e.event.type === 'error' + // An error the agent reported itself (#1500) is a failure like any other: the log already has + // one red lane, and a second vocabulary for the same thing would only make both quieter. + if (e.kind === 'error') return true return e.kind === 'end' && !e.ok && !e.stopped } diff --git a/packages/the-framework/prompts/presets/update_tickets.md b/packages/the-framework/prompts/presets/update_tickets.md index 8c8ba3b9b..1f9014fcd 100644 --- a/packages/the-framework/prompts/presets/update_tickets.md +++ b/packages/the-framework/prompts/presets/update_tickets.md @@ -5,7 +5,7 @@ Note the current UTC time before you fetch anything, in ISO 8601. That is the ti Read `tickets/meta.json` for `lastImportedAt`. Do one of the following: -- [Error] If there are existing `tickets/*.md` but `lastImportedAt` is missing, or `gh` is missing or logged out, show an error to the user and abort +- [Error] If there are existing `tickets/*.md` but `lastImportedAt` is missing, or `gh` is missing or logged out, report the error — say which of those it is — and abort - [Empty] If `tickets/` is empty (or doesn't exist), treat it as a first import and bring every open issue across - [Update] Fetch only what changed: `gh issue list --state all --limit 500 --json number,title,body,state,labels,updatedAt --search "updated:>="`, and the discussion with `gh api --paginate "repos/{owner}/{repo}/issues/comments?since="` diff --git a/packages/the-framework/prompts/protocols/signal.md b/packages/the-framework/prompts/protocols/signal.md index 339172980..f47f6ab18 100644 --- a/packages/the-framework/prompts/protocols/signal.md +++ b/packages/the-framework/prompts/protocols/signal.md @@ -18,3 +18,12 @@ Whenever you emit `ready-for-merge`, emit an `open-pr` block too, naming and des ``` 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. + +## Reporting an error +When you hit something only the user can fix — a missing file you were told to read, a command that will not run, a login you do not have — emit an `error` block saying what is wrong, then carry on or stop as the task requires. The first line is the headline; anything below it is the detail. +```error + + + +``` +The Framework marks it in the session log and counts it on the session, so the user sees it without reading the whole log. It does not stop your turn and it does not ask the user anything — use `AWAIT` for a question. Report the same thing once: a re-emitted identical block is ignored. diff --git a/packages/the-framework/src/agent-view.SPEC.md b/packages/the-framework/src/agent-view.SPEC.md index 345d6b87b..773d42cf6 100644 --- a/packages/the-framework/src/agent-view.SPEC.md +++ b/packages/the-framework/src/agent-view.SPEC.md @@ -1,15 +1,16 @@ -Distills an agent's event stream into the dashboard's summary cards: the agent's lifecycle progress, what it will do with its work when it ends, and the agent session behind it. +Distills an agent's event stream into the dashboard's summary cards: the agent's lifecycle progress, what went wrong along the way, what it will do with its work when it ends, and the agent session behind it. ## User Stories - The user reads an agent's card at a glance: the name it chose, and a badge that flips when the agent signals ready for merge. - The user sees what an agent will do with its work when it ends — push, open a pull request, merge — exactly as armed. +- The user sees how many errors an agent hit and the latest of them, without reading its log. - The user opens a past agent's record and sees the identical summary a live viewer saw. ## Flows - The cards are pure folds over the same events the log renders, so a live dashboard and a replay of a past agent always show the identical summary. -- Latest wins throughout: the agent may rename its session or re-arm its handoff at any point. +- Latest wins throughout: the agent may rename its session or re-arm its handoff at any point. Errors are the exception — they only accumulate, because an error is something that happened and nothing can un-happen it. - The publish state reads as armed (push and pull request) even for a stream that never says so, because that is what such an agent will actually do; merging is the opposite — opt-in, so silence reads as off. - A stored snapshot can seed the publish state for a viewer who attached after the agent's opening events — so a session the launcher armed push-only never shows as one that will open a pull request; an event in the stream always wins over the snapshot. - Resuming a finished agent needs to know where the conversation lived, and its working copy is deleted — so the workspace comes from the events, the surviving record, never from the disk. diff --git a/packages/the-framework/src/agent-view.test.SPEC.md b/packages/the-framework/src/agent-view.test.SPEC.md index 2d5367b8d..823bff161 100644 --- a/packages/the-framework/src/agent-view.test.SPEC.md +++ b/packages/the-framework/src/agent-view.test.SPEC.md @@ -1,4 +1,4 @@ -Tests the event-stream projections behind the dashboard cards: latest-wins folding of review status, names, deploy plans, session and model; the armed-by-default publish state with opt-in merge; snapshot seeding losing to real events; and handoff outcomes surviving into the summary. +Tests the event-stream projections behind the dashboard cards: latest-wins folding of review status, names, deploy plans, session and model; the armed-by-default publish state with opt-in merge; snapshot seeding losing to real events; the accumulating list of errors the agent reported; and handoff outcomes surviving into the summary. ## Before modifying/creating SPEC.md files diff --git a/packages/the-framework/src/agent-view.test.ts b/packages/the-framework/src/agent-view.test.ts index 2584bfa11..e48467f9b 100644 --- a/packages/the-framework/src/agent-view.test.ts +++ b/packages/the-framework/src/agent-view.test.ts @@ -1,6 +1,6 @@ import { strict as assert } from 'node:assert' import { test } from 'node:test' -import { sessionInfo, agentProgress, handoffState } from './agent-view.js' +import { sessionInfo, agentProgress, agentErrors, handoffState } from './agent-view.js' import type { FrameworkEvent } from './events.js' test('agentProgress starts building with no name and flips to ready on setReadyForMerge (#326)', () => { @@ -102,3 +102,16 @@ test('handoffState carries the outcome once the handoff has run (#1102)', () => const skipped: FrameworkEvent[] = [{ kind: 'handoff', outcome: 'skipped', reason: 'no-remote' }] assert.deepEqual(handoffState(skipped).result, { outcome: 'skipped', reason: 'no-remote' }) }) + +test('agentErrors folds the errors the agent reported, oldest first (#1500)', () => { + assert.deepEqual(agentErrors([]), []) + const events: FrameworkEvent[] = [ + { kind: 'error', headline: 'gh is not logged in', detail: 'ran `gh auth status`' }, + { kind: 'session-name', name: 'update-tickets' }, + { kind: 'error', headline: 'tickets/meta.json has no lastImportedAt' }, + ] + assert.deepEqual(agentErrors(events), [ + { headline: 'gh is not logged in', detail: 'ran `gh auth status`' }, + { headline: 'tickets/meta.json has no lastImportedAt' }, + ]) +}) diff --git a/packages/the-framework/src/agent-view.ts b/packages/the-framework/src/agent-view.ts index 1155aab05..349240a5c 100644 --- a/packages/the-framework/src/agent-view.ts +++ b/packages/the-framework/src/agent-view.ts @@ -29,6 +29,30 @@ export function agentProgress(events: readonly FrameworkEvent[]): AgentProgress return progress } +/** One error the agent reported through an `error` block (#1500). */ +export interface AgentError { + /** What is wrong, in one line. */ + headline: string + /** What it ran and what that said, when the agent wrote any. */ + detail?: string +} + +/** + * Every error the agent reported (#1500), oldest first — the count the dashboard shows on the + * session, and the latest headline it shows beside it. + * + * A fold over the log rather than state of its own: an error is an event that happened, so the + * list only ever grows, and reopening a finished agent shows exactly what it showed while it ran. + */ +export function agentErrors(events: readonly FrameworkEvent[]): AgentError[] { + const errors: AgentError[] = [] + for (const event of events) { + if (event.kind !== 'error') continue + errors.push({ headline: event.headline, ...(event.detail ? { detail: event.detail } : {}) }) + } + return errors +} + /** What a session will do with its work when it ends (#1102), and what it did. */ export interface HandoffState { /** Push the branch to `origin` on finish. */ diff --git a/packages/the-framework/src/client.ts b/packages/the-framework/src/client.ts index f1a46493e..bc0ff8995 100644 --- a/packages/the-framework/src/client.ts +++ b/packages/the-framework/src/client.ts @@ -10,9 +10,11 @@ export { pickedIds } from './events.js' export { sessionInfo, agentProgress, + agentErrors, handoffState, type SessionInfo, type AgentProgress, + type AgentError, type HandoffState, } from './agent-view.js' // The Start-an-agent presets (#433): pure prompt builders (no Node imports) the dashboard diff --git a/packages/the-framework/src/events.SPEC.md b/packages/the-framework/src/events.SPEC.md index 8555b5c30..407bd7af8 100644 --- a/packages/the-framework/src/events.SPEC.md +++ b/packages/the-framework/src/events.SPEC.md @@ -11,6 +11,7 @@ The single event stream an agent narrates itself over: one timeline uniting the - The framework owns the stream rather than exposing the driver's transport, so every surface — terminal, dashboard, chat — renders the same story. - Events are the agent's durable record: only events reach its stored history, so anything a dashboard tab opened later must know travels as an event — the ticket being implemented, the branch, the pull request opened for the work, the marker commit (hand-off anchor) a cloud run's branch is later recognized by, and what the end-of-work handoff is armed to do. - Interactive gates are events too: a choice pauses the agent until a pick is posted back, and both the question and who answered it are on the record. +- An error the agent hit is an event on that timeline: it happened at a point in the run and stays there as history, which is why nothing ever clears it. The errors a background job finds between runs are a different thing entirely — a condition that is true of the project right now, held elsewhere and cleared the moment it is fixed. - Every skipped or withheld outcome carries its reason, so "it was on and nothing happened" always has an answer in the log. ## Before modifying/creating SPEC.md files diff --git a/packages/the-framework/src/events.ts b/packages/the-framework/src/events.ts index f3a874135..5294155ea 100644 --- a/packages/the-framework/src/events.ts +++ b/packages/the-framework/src/events.ts @@ -200,6 +200,17 @@ export type FrameworkEvent = | { kind: 'browser'; url: string } /** A framework-level log line. */ | { kind: 'log'; message: string } + /** + * Something went wrong that only the user can fix (#1500), reported by the agent itself + * through an `error` block rather than left in prose the reader has to notice. The headline + * is the first line, the detail is the rest. + * + * An event, not a status: it says what happened at this point in the run and stays in the log + * as history — nothing clears it, because nothing can un-happen it. The project-level errors a + * background job finds between runs are the other half (project-errors.ts): those are + * conditions that are true *now*, and clear themselves when the condition is gone. + */ + | { kind: 'error'; headline: string; detail?: string } /** * An ad-hoc markdown view the agent pushed to show the user (#441), e.g. a plan, * a summary, or a diff writeup. Non-blocking (unlike a `choice`): the dashboard diff --git a/packages/the-framework/src/index.ts b/packages/the-framework/src/index.ts index 2aaf2b0d5..4f64096c8 100644 --- a/packages/the-framework/src/index.ts +++ b/packages/the-framework/src/index.ts @@ -13,7 +13,7 @@ * ends where its consumers end, so a name nothing renders cannot quietly live on in it. */ -export type { HandoffState, SessionInfo } from './agent-view.js' +export type { AgentError, HandoffState, SessionInfo } from './agent-view.js' export type { AutoPmJob, AutoPmOutcome, AutoPmReport } from './auto-pm.js' export type { FrameworkFileConfig } from './config.js' export type { ChoiceRequest, FrameworkEvent } from './events.js' diff --git a/packages/the-framework/src/project-errors.SPEC.md b/packages/the-framework/src/project-errors.SPEC.md index dd7b35b9d..b814b55af 100644 --- a/packages/the-framework/src/project-errors.SPEC.md +++ b/packages/the-framework/src/project-errors.SPEC.md @@ -14,6 +14,7 @@ The daemon's per-project error state: when a background job finds a project in a ## Rationales - Errors surface in the dashboard rather than as console lines on the daemon's stdout, which nobody reads — swallowing an error is the worst way to handle it. +- This holds only what a background job finds while nothing is running; what an agent hits during a run is reported by the agent itself and lives in that run's log. The split is what each thing is: a condition that is true now and clears itself, against something that happened and cannot un-happen. - A repository with no remote is not a supported mode but an error, since every other machine and cloud session converges through that remote. ## Before modifying/creating SPEC.md files diff --git a/packages/the-framework/src/terminal.ts b/packages/the-framework/src/terminal.ts index 9fa459af6..204a2df99 100644 --- a/packages/the-framework/src/terminal.ts +++ b/packages/the-framework/src/terminal.ts @@ -25,6 +25,8 @@ export function formatFrameworkEvent(event: FrameworkEvent): string { return `◆ browser: ${event.url}` case 'log': return ` ${event.message}` + case 'error': + return `✗ ${event.headline}${event.detail ? `\n ${event.detail.replace(/\n/g, '\n ')}` : ''}` case 'view': return `▶ view: ${event.title}` case 'session-name': diff --git a/packages/the-framework/src/todo-loop.test.SPEC.md b/packages/the-framework/src/todo-loop.test.SPEC.md index a69a9e506..0f196efcb 100644 --- a/packages/the-framework/src/todo-loop.test.SPEC.md +++ b/packages/the-framework/src/todo-loop.test.SPEC.md @@ -1,4 +1,4 @@ -Covers the backlog loop and queue plumbing over the data branch: draining one entry per turn to empty with the framework's own check-off landing as data-branch commits, the per-item gate and stopping, the failed-check-off stall and item-cap limits, gates and signals honored mid-backlog with ready-for-merge deduped to once, priority-section placement of queued entries, the resume note resolving the project root from an agent worktree, naming the ticket a drain agent will pick up, and the agent-TODO merge safety belt. +Covers the backlog loop and queue plumbing over the data branch: draining one entry per turn to empty with the framework's own check-off landing as data-branch commits, the per-item gate and stopping, the failed-check-off stall and item-cap limits, gates and signals honored mid-backlog — views, reported errors and the session name reaching the stream, with ready-for-merge deduped to once — priority-section placement of queued entries, the resume note resolving the project root from an agent worktree, naming the ticket a drain agent will pick up, and the agent-TODO merge safety belt. ## Before modifying/creating SPEC.md files diff --git a/packages/the-framework/src/todo-loop.test.ts b/packages/the-framework/src/todo-loop.test.ts index 38f023fa2..d30bf3bce 100644 --- a/packages/the-framework/src/todo-loop.test.ts +++ b/packages/the-framework/src/todo-loop.test.ts @@ -314,7 +314,7 @@ test('an aborted signal ends the loop before starting another entry', async () = } }) -test('a backlog turn emits its signals: views, session name, ready-for-merge', async () => { +test('a backlog turn emits its signals: views, errors, session name, ready-for-merge', async () => { const repo = await repoWorkspace() await seedQueue(repo, '- [ ] tidy the login redirect\n') try { @@ -329,6 +329,11 @@ test('a backlog turn emits its signals: views, session name, ready-for-merge', a '# What I changed', 'Rewrote the redirect guard.', '```', + '```error', + 'the redirect test fixture is missing', + '', + 'ran `ls test/fixtures`: No such file or directory', + '```', '```set-session-name', 'login-redirect-fix', '```', @@ -341,6 +346,16 @@ test('a backlog turn emits its signals: views, session name, ready-for-merge', a const view = events.find(e => e.kind === 'view') assert.equal(view?.title, 'What I changed') + assert.deepEqual( + events.filter(e => e.kind === 'error'), + [ + { + kind: 'error', + headline: 'the redirect test fixture is missing', + detail: 'ran `ls test/fixtures`: No such file or directory', + }, + ], + ) assert.equal(events.find(e => e.kind === 'session-name')?.name, 'login-redirect-fix') assert.equal(events.filter(e => e.kind === 'ready-for-merge').length, 1) } finally { diff --git a/packages/the-framework/src/turn-gate.SPEC.md b/packages/the-framework/src/turn-gate.SPEC.md index 6ec02c4f9..90c6aa044 100644 --- a/packages/the-framework/src/turn-gate.SPEC.md +++ b/packages/the-framework/src/turn-gate.SPEC.md @@ -1,17 +1,19 @@ -The turn-boundary contract with the wrapped agent: each turn runs as a black box, so everything the framework learns — the agent stopping to ask, views to show, its chosen session name, ready-for-merge, a pull-request description — is a tagged block parsed out of the turn's final message. +The turn-boundary contract with the wrapped agent: each turn runs as a black box, so everything the framework learns — the agent stopping to ask, views to show, errors it hit, its chosen session name, ready-for-merge, a pull-request description — is a tagged block parsed out of the turn's final message. ## User Stories - The user answers the agent's question, and the agent resumes the same conversation. +- The user learns what the agent could not get past, from the agent itself, instead of finding it buried in a paragraph of its reply. - The user's answer can end the agent instead of resuming it — declining a plan stops the run. ## Flows -- The protocol texts appended to the system channel pin how to emit, not when: one blocking ask-gate and the non-blocking signals (markdown views, session name, ready-for-merge, a pull-request description). +- The protocol texts appended to the system channel pin how to emit, not when: one blocking ask-gate and the non-blocking signals (markdown views, reported errors, session name, ready-for-merge, a pull-request description). - There is one gate block, not four: every gate is a question with options, and what distinguishes the kinds is what the agent writes in one — two options for an approval, a file for a plan, a flag for several picks, a mark on the options that end the agent rather than resuming it. - 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. +- Signal emission is deduped across a span of turns: ready-for-merge fires once, a session name or a pull-request description re-emits only on a real change, and an error is recorded once however often the agent restates it — an agent repeating its block every turn must not read as the same failure happening over and over. +- An error the agent reports is written as a headline and the detail below it, and every error in a turn is kept rather than only the last: two different things going wrong are two errors, and collapsing them would lose one. - 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 355ba1d94..ddff731b6 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 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. +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), the errors an agent reports (a headline and the detail below it, one-line blocks, every block in a turn kept, empty ones ignored) and their once-only recording however often the agent restates them, 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 58a4ddb9e..6be838419 100644 --- a/packages/the-framework/src/turn-gate.test.ts +++ b/packages/the-framework/src/turn-gate.test.ts @@ -1,6 +1,7 @@ import assert from 'node:assert/strict' import { test } from 'node:test' -import { continuationPrompt, parseAwaitGate, parseMarkdownViews, parseSessionName, parseReadyForMerge, parsePullRequest } from './turn-gate.js' +import { continuationPrompt, createTurnSignalEmitter, parseAwaitGate, parseErrors, parseMarkdownViews, parseSessionName, parseReadyForMerge, parsePullRequest } from './turn-gate.js' +import type { FrameworkEvent } from './events.js' const block = (json: string): string => 'Here are the options.\n```await-choices\n' + json + '\n```' @@ -207,3 +208,45 @@ test('parsePullRequest ignores an empty block rather than blanking the body (#15 assert.equal(parsePullRequest('```open-pr\n\n```'), undefined) assert.deepEqual(parsePullRequest('```open-pr\nreal\n```\n```open-pr\n \n```'), { title: 'real' }) }) + +test('parseErrors returns nothing when the turn reported none (#1500)', () => { + assert.deepEqual(parseErrors('All good, the import finished.'), []) +}) + +test('parseErrors splits the block into a headline and the detail below it (#1500)', () => { + assert.deepEqual(parseErrors('```error\ngh is not logged in\n\nran `gh auth status`: You are not logged into any hosts\n```'), [ + { headline: 'gh is not logged in', detail: 'ran `gh auth status`: You are not logged into any hosts' }, + ]) +}) + +test('parseErrors takes a one-line block as a headline with no detail (#1500)', () => { + assert.deepEqual(parseErrors('```error\ntickets/meta.json has no lastImportedAt\n```'), [ + { headline: 'tickets/meta.json has no lastImportedAt' }, + ]) +}) + +test('parseErrors keeps every block, in order: two things going wrong is two errors (#1500)', () => { + const text = '```error\nfirst\n```\nand then\n```error\nsecond\n```' + assert.deepEqual(parseErrors(text), [{ headline: 'first' }, { headline: 'second' }]) +}) + +test('parseErrors ignores an empty block — an error with nothing to say is not one (#1500)', () => { + assert.deepEqual(parseErrors('```error\n \n```'), []) + assert.deepEqual(parseErrors('```error\n \n```\n```error\nreal\n```'), [{ headline: 'real' }]) +}) + +test('the turn emitter logs an error once however often the agent restates it (#1500)', () => { + const events: FrameworkEvent[] = [] + const emit = createTurnSignalEmitter(e => events.push(e)) + emit('```error\npush rejected\n```') + emit('still stuck.\n```error\npush rejected\n```') + assert.deepEqual(events, [{ kind: 'error', headline: 'push rejected' }]) +}) + +test('the turn emitter treats a second, different failure as its own error (#1500)', () => { + const events: FrameworkEvent[] = [] + const emit = createTurnSignalEmitter(e => events.push(e)) + emit('```error\npush rejected\n\nfirst attempt\n```') + emit('```error\npush rejected\n\nsecond attempt, different remote\n```') + assert.equal(events.length, 2) +}) diff --git a/packages/the-framework/src/turn-gate.ts b/packages/the-framework/src/turn-gate.ts index f687ac8c7..7d6672bd5 100644 --- a/packages/the-framework/src/turn-gate.ts +++ b/packages/the-framework/src/turn-gate.ts @@ -228,6 +228,34 @@ export function parsePullRequest(text: string): ParsedPullRequest | undefined { return parsed } +/** An error the agent reported this turn (#1500), split the way the block is written. */ +export interface ParsedError { + /** What is wrong, in one line: the block's first line. */ + headline: string + /** What it ran and what that said: everything below the headline, when the agent wrote any. */ + detail?: string +} + +/** + * Parse the errors the agent reported this turn (#1500), from every non-empty `error` block + * (per {@link SIGNAL_PROTOCOL}), in the order they were written. + * + * Unlike the other signals, every block is kept rather than only the last: two different things + * going wrong in one turn are two errors, and collapsing them would lose one. Blocks that are + * empty are skipped — an error with nothing to say is not an error. + */ +export function parseErrors(text: string): ParsedError[] { + const errors: ParsedError[] = [] + for (const body of blocks(text, 'error')) { + const trimmed = body.trim() + if (!trimmed) continue + const [first = '', ...rest] = trimmed.split('\n') + const detail = rest.join('\n').trim() + errors.push({ headline: first.trim(), ...(detail ? { detail } : {}) }) + } + return errors +} + /** * Whether the agent signalled `setReadyForMerge()` this turn (#326): the presence of a * `ready-for-merge` block (per {@link SIGNAL_PROTOCOL}) anywhere in the text. Non-blocking @@ -312,14 +340,14 @@ function parseGateBody(body: string): ParsedAwaitGate | undefined { } /** - * Emit the {@link PROTOCOLS_SIGNAL} signals an agent turn carries: markdown views, the - * session name, `setReadyForMerge()`, and a pull-request description. Every turn the framework prompts goes through + * Emit the {@link PROTOCOLS_SIGNAL} signals an agent turn carries: markdown views, the errors it + * reported, the session name, `setReadyForMerge()`, and a pull-request description. Every turn the framework prompts goes through * one of these, because the protocols are unconditional (see `composeAgentSystem`) — the * agent is told it can signal on any turn, so any turn we don't parse drops the signal. * * The returned function holds the dedupe state for the turns it covers: `ready-for-merge` - * fires once, and a session name and a pull-request description only re-emit on an actual - * change. Each caller makes one + * fires once, a session name and a pull-request description only re-emit on an actual + * change, and an error is logged once however often the agent restates it. Each caller makes one * for its own span of turns (a build's await rounds, the whole backlog), so keep it for as * many turns as should share that dedupe rather than making one per turn. */ @@ -327,8 +355,18 @@ export function createTurnSignalEmitter(emit: (event: FrameworkEvent) => void): let named: string | undefined let ready = false let described: string | undefined + const reported = new Set() return (text: string): void => { for (const view of parseMarkdownViews(text)) emit({ kind: 'view', ...view }) + for (const error of parseErrors(text)) { + // Reported once per span (#1500): agents restate their blocks turn after turn, and the + // same failure logged ten times reads as ten failures. The whole block keys it, so a + // second attempt that fails differently is still its own error. + const key = `${error.headline}\n${error.detail ?? ''}` + if (reported.has(key)) continue + reported.add(key) + emit({ kind: 'error', ...error }) + } const name = parseSessionName(text) if (name && name !== named) { named = name