From 017a217959878042f316105394700da98539cc2d Mon Sep 17 00:00:00 2001 From: Suleiman Shahbari Date: Fri, 21 Aug 2026 02:33:23 +0300 Subject: [PATCH 1/3] Agents open a pull request through the framework by describing it, rather than running gh themselves MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit #1567 found that a plan agent's PR body ended "…then comment on and close #1164", which GitHub read as a closing keyword: the plan merged, the issue closed, and the next tickets sync deleted the ticket and its fresh plan. The first fix directions were both wrong. Linting closing keywords out of every PR body breaks the correct case — 8 of the last 40 PRs legitimately close their issue that way. And the body that caused it was the agent's own: it ran `gh pr create` itself, outside the handoff path, where the framework never sees the text. brillout's answer on the issue: "As always, I'd frame it as a capability… We avoid limiting agents — we *enable* agents instead." So there is no prohibition here. An `open-pr` block is a new non-blocking signal, alongside set-session-name and ready-for-merge: the agent writes the description, the framework opens the PR. The agent gets to say what the change turned out to be instead of the PR body reciting the opening prompt; the framework keeps the parts that have to stay consistent — the title from the session name, the ticket's `(fix #N)`, and recording the number on the agent. An agent that prefers to open its own still can. The closing-keyword bug is then fixed where it belongs: a plan agent's description is defused before it is published, because a plan agent's PR lands the plan, not the work. That is the same rule that already keeps `(fix #N)` off a plan agent's title (#1327) — this is its other half. Defusing wraps the issue reference in backticks: the sentence a human reads is unchanged, and GitHub stops acting on it. Suite green: 1502 framework tests, 776 dashboard, typecheck clean. --- FEATURES-SPEC.md | 1 + packages/the-framework/prompts/SPEC.md | 2 +- .../the-framework/prompts/protocols/signal.md | 7 +++ packages/the-framework/src/cli.ts | 20 +++++++++ .../src/closing-keywords.SPEC.md | 16 +++++++ .../src/closing-keywords.test.SPEC.md | 5 +++ .../src/closing-keywords.test.ts | 43 +++++++++++++++++++ .../the-framework/src/closing-keywords.ts | 43 +++++++++++++++++++ .../src/dashboard/agent-handoff.SPEC.md | 1 + .../src/dashboard/agent-handoff.test.ts | 34 +++++++++++++++ .../src/dashboard/agent-handoff.ts | 18 +++++++- packages/the-framework/src/events.ts | 7 +++ packages/the-framework/src/terminal.ts | 2 + packages/the-framework/src/turn-gate.SPEC.md | 8 ++-- .../the-framework/src/turn-gate.test.SPEC.md | 2 +- packages/the-framework/src/turn-gate.test.ts | 29 ++++++++++++- packages/the-framework/src/turn-gate.ts | 31 ++++++++++++- 17 files changed, 259 insertions(+), 10 deletions(-) create mode 100644 packages/the-framework/src/closing-keywords.SPEC.md create mode 100644 packages/the-framework/src/closing-keywords.test.SPEC.md create mode 100644 packages/the-framework/src/closing-keywords.test.ts create mode 100644 packages/the-framework/src/closing-keywords.ts diff --git a/FEATURES-SPEC.md b/FEATURES-SPEC.md index c5dca7568..93c23dc14 100644 --- a/FEATURES-SPEC.md +++ b/FEATURES-SPEC.md @@ -105,6 +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 - Auto-merge — armed by config, authorized by the agent's ready signal - Empty agents publish nothing - Handoff panel: push / open PR / merge, as buttons diff --git a/packages/the-framework/prompts/SPEC.md b/packages/the-framework/prompts/SPEC.md index cd23a6081..d6a9c34f0 100644 --- a/packages/the-framework/prompts/SPEC.md +++ b/packages/the-framework/prompts/SPEC.md @@ -3,7 +3,7 @@ Every word the framework says to a coding agent, authored as markdown: the built ## Flows - The system prompt wraps the user's prompt in a working discipline: analyze it and gate on ambiguity or large scope, keep every read and write under the working directory, name the session and do all work on a branch of that name, offer alternatives wherever the best solution is unclear, and signal ready-for-merge only when nothing is left — without that signal the work is never merged. -- The protocols define the agent's side of the conversation: how to park on a gate (a choice, a multi-select, a document approval, handing the browser to a human at a login wall) — including marking the answers that end the agent rather than resume it, so a rejection is not something it is asked to build on — and how to emit the non-blocking signals (show a document, name the session, ready-for-merge); per-capability protocols adapt it — an agent with a real browser is told when to use it, a hands-off agent is told gates can never be answered, so assume the recommended option and carry on. +- The protocols define the agent's side of the conversation: how to park on a gate (a choice, a multi-select, a document approval, handing the browser to a human at a login wall) — including marking the answers that end the agent rather than resume it, so a rejection is not something it is asked to build on — and how to emit the non-blocking signals (show a document, name the session, ready-for-merge, describe the pull request the framework will open); per-capability protocols adapt it — an agent with a real browser is told when to use it, a hands-off agent is told gates can never be answered, so assume the recommended option and carry on. - The presets are the one-click task prompts behind the dashboard's buttons: research, the quality reviews (readability, maintainability, security, UX), ticket triage and planning, and draining the queue. - The format docs teach the repo conventions: tickets as dated proposal files with plan and claim siblings, and the priority-ordered queue file of confirmed work. - The before-mergeable prompt is the final quality turn: queue follow-up refactor and security passes when the changes warrant them, and fold what the agent learned into the project's knowledge base. diff --git a/packages/the-framework/prompts/protocols/signal.md b/packages/the-framework/prompts/protocols/signal.md index e0fe77d34..beb3cd544 100644 --- a/packages/the-framework/prompts/protocols/signal.md +++ b/packages/the-framework/prompts/protocols/signal.md @@ -9,3 +9,10 @@ You do not stop; re-emit it if you rename the session. When you call setReadyForMerge() — you believe the work is complete and ready for human review — emit an empty `ready-for-merge` block. This flips the dashboard status from building to ready; it does not stop your turn. ```ready-for-merge ``` + +## Opening a pull request +When your work is ready to be published, describe it in an `open-pr` block and The Framework opens the pull request for you — you do not need to run `gh pr create` yourself: +```open-pr + +``` +The block is the body of the pull request. The Framework supplies the rest: 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 still works if you prefer; you then own all of the above. diff --git a/packages/the-framework/src/cli.ts b/packages/the-framework/src/cli.ts index f416f75b6..bc965b282 100644 --- a/packages/the-framework/src/cli.ts +++ b/packages/the-framework/src/cli.ts @@ -15,6 +15,7 @@ import { launchSharedBrowser, withBrowser, type SharedBrowser } from './browser. import { connectCdp, startBrowserStream, type BrowserStream } from './browser-stream.js' import { randomUUID } from 'node:crypto' import { formatFrameworkEvent, mergeWithheldWhy } from './terminal.js' +import { defuseClosingKeywords } from './closing-keywords.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' @@ -547,6 +548,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 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). */ @@ -578,6 +581,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 + // changes, and the handoff wants what it said last. + let prDescription: string | 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. @@ -592,6 +598,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 === 'session-name') { sessionName = event.name // The framework-owned checkout (#736) was branched as `tf-agent-` before a @@ -625,6 +632,7 @@ export function createAgentJournal(deps: { onEvent, sessionName: () => sessionName, sawReadyForMerge: () => sawReadyForMerge, + prDescription: () => prDescription, stoppedCleanly: () => stoppedCleanly, announceBrowserPort: port => { pendingBrowserPort = port @@ -1027,12 +1035,24 @@ async function driveAgent(opts: AgentOptions, io: CliIO): Promise { const fixes = opts.ticket && isTicketPath(opts.ticket) && !opts.planAgent ? ticketIssueRef((await readDataFile(cwd, opts.ticket).catch(() => undefined)) ?? '') : undefined + // The agent's own description of the work (#1567), when it wrote one: this is what an + // `open-pr` block is for — the agent describes the change and the framework opens the PR, + // so it has no reason to run `gh pr create` itself and lose the title convention, the + // ticket's issue reference, and the recorded number along the way. + // + // A plan agent's description is defused first: its PR lands the plan, not the work, so a + // 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 agent = { id: opts.agentId ?? '', branch, ...(sessionName ? { sessionName } : {}), ...(intent ? { intent } : {}), ...(fixes ? { fixes } : {}), + ...(description ? { description } : {}), } const handedOff = await agentAutoHandoff(cwd, agent, armed) const outcome = diff --git a/packages/the-framework/src/closing-keywords.SPEC.md b/packages/the-framework/src/closing-keywords.SPEC.md new file mode 100644 index 000000000..c3ac4fed9 --- /dev/null +++ b/packages/the-framework/src/closing-keywords.SPEC.md @@ -0,0 +1,16 @@ +Why a pull request that does not finish an issue must not carry a phrase GitHub reads as closing it, and how such a phrase is defused without changing what the sentence says. + +## Flows + +- A closing phrase is one of GitHub's keywords — close, fix, resolve, and their plural and past forms — followed by an issue reference, in this repository or another. +- Defusing wraps the reference in backticks and leaves every word alone, so the sentence a human reads is unchanged and the pull request stops closing the issue. +- A reference already wrapped is left as it is, so text that has been through this once can go through it again unchanged. + +## Rationales + +- The phrase is defused rather than forbidden: an agent writing "…then close #1164" as the last step of a plan is describing its plan accurately, and the sentence is worth keeping — what is wrong is only that GitHub acts on it. +- Backticks are the chosen form because they defeat the parser while rendering the same words, where rewording would put the framework in the business of editing an agent's prose. + +## 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/src/closing-keywords.test.SPEC.md b/packages/the-framework/src/closing-keywords.test.SPEC.md new file mode 100644 index 000000000..d0df73d42 --- /dev/null +++ b/packages/the-framework/src/closing-keywords.test.SPEC.md @@ -0,0 +1,5 @@ +Covers defusing GitHub's closing phrases: the exact sentence that closed #1164 keeps its words, every keyword form and letter case is caught, the cross-repo reference is caught too, an issue mentioned without a keyword is untouched, a word merely ending in a keyword is not one, and defusing twice changes nothing. + +## 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/src/closing-keywords.test.ts b/packages/the-framework/src/closing-keywords.test.ts new file mode 100644 index 000000000..93fe8e8ee --- /dev/null +++ b/packages/the-framework/src/closing-keywords.test.ts @@ -0,0 +1,43 @@ +import { test, describe } from 'node:test' +import assert from 'node:assert/strict' +import { defuseClosingKeywords } from './closing-keywords.js' + +// #1567: a plan PR whose body ended "…then comment on and close #1164" closed #1164 on merge, +// and the next tickets sync then removed the ticket and its fresh plan. The words stay; only +// GitHub's reading of them changes. +describe('defuseClosingKeywords', () => { + test('the phrase that closed #1164 keeps its words and loses its effect', () => { + const before = 'The plan proposes: expose `queued` on the single-ticket read — then comment on and close #1164.' + assert.equal( + defuseClosingKeywords(before), + 'The plan proposes: expose `queued` on the single-ticket read — then comment on and close `#1164`.', + ) + }) + + test('every keyword form GitHub accepts is defused, whatever the case', () => { + for (const keyword of ['close', 'closes', 'closed', 'fix', 'fixes', 'fixed', 'resolve', 'resolves', 'resolved']) { + assert.equal(defuseClosingKeywords(`this ${keyword} #42`), `this ${keyword} \`#42\``, keyword) + assert.equal(defuseClosingKeywords(`this ${keyword.toUpperCase()} #42`), `this ${keyword.toUpperCase()} \`#42\``, keyword) + } + }) + + test('the cross-repo form closes just as well, so it is defused too', () => { + assert.equal(defuseClosingKeywords('fixes gemstack-land/the-framework#7'), 'fixes `gemstack-land/the-framework#7`') + }) + + test('an issue mentioned without a keyword is left alone — a reference is not a command', () => { + const text = 'See #1164 for the symptom, and the evidence table on #1334.' + assert.equal(defuseClosingKeywords(text), text) + }) + + test('a word that merely ends in a keyword is not one', () => { + const text = 'the enclose #42 case' + assert.equal(defuseClosingKeywords(text), text) + }) + + test('running it twice changes nothing the second time', () => { + const once = defuseClosingKeywords('fixes #9 and closes #10') + assert.equal(defuseClosingKeywords(once), once) + assert.equal(once, 'fixes `#9` and closes `#10`') + }) +}) diff --git a/packages/the-framework/src/closing-keywords.ts b/packages/the-framework/src/closing-keywords.ts new file mode 100644 index 000000000..1deca396f --- /dev/null +++ b/packages/the-framework/src/closing-keywords.ts @@ -0,0 +1,43 @@ +/** + * GitHub's issue-closing grammar, and how to defuse it (#1567). + * + * A pull request whose body says `close #1164` closes that issue the moment the PR merges. + * That is the wanted behaviour when the PR completes the issue — the handoff puts `(fix #42)` + * on the title of an implementing agent's PR for exactly that reason (#1334). It is the wrong + * behaviour when the PR delivers something short of the work: a plan agent's PR lands a plan + * whose own text says the implementation is still to come, and a sentence like "…then close + * #1164" reads perfectly sensibly to the human reviewing it while quietly closing the ticket. + * + * That happened (#1560 closed #1164, and the next tickets sync then deleted the ticket and its + * fresh plan). The cure is not to forbid the phrase — the agent is describing its plan, and the + * sentence is true — but to write the issue reference in a form GitHub's parser does not read as + * a command. Backticks do it: `close `#1164`` renders as the same words and links nowhere. + */ + +/** + * The keywords GitHub accepts before an issue reference, per its "linking a pull request to an + * issue" documentation. Matched case-insensitively; every listed form (and its plural/past + * tense) is a live trigger. + */ +const CLOSING_KEYWORDS = ['close', 'closes', 'closed', 'fix', 'fixes', 'fixed', 'resolve', 'resolves', 'resolved'] + +/** + * A closing keyword, whitespace, then an issue reference — `#123`, or the cross-repo + * `owner/repo#123` form, which closes just as well. Only a reference already inside backticks + * is left alone, since it is already defused; that is what makes this safe to run twice. + */ +const CLOSING_PHRASE = new RegExp( + String.raw`(^|[^\`\w])(${CLOSING_KEYWORDS.join('|')})(\s+)((?:[\w.-]+\/[\w.-]+)?#\d+)(?!\`)`, + 'gi', +) + +/** + * Rewrite every closing phrase in `text` so GitHub stops reading it as a command, leaving the + * words as the agent wrote them: `close #1164` becomes ``close `#1164``. + * + * Prose only — the reference keeps its own text, so a human reads the same sentence and the + * PR simply stops closing an issue it did not finish. + */ +export function defuseClosingKeywords(text: string): string { + return text.replace(CLOSING_PHRASE, (_all, before: string, keyword: string, gap: string, ref: string) => `${before}${keyword}${gap}\`${ref}\``) +} diff --git a/packages/the-framework/src/dashboard/agent-handoff.SPEC.md b/packages/the-framework/src/dashboard/agent-handoff.SPEC.md index 6aa90fbc8..46d4560a0 100644 --- a/packages/the-framework/src/dashboard/agent-handoff.SPEC.md +++ b/packages/the-framework/src/dashboard/agent-handoff.SPEC.md @@ -4,6 +4,7 @@ How a finished agent's work is handed back to the human: measure what its branch - Branch-addressed: an agent reads the same whether or not its checkout still exists, and a locally-gone branch still reports its PR — a hands-off cloud agent only ever pushed remotely. - An agent that produced nothing — no commits, or only the framework's own bookkeeping — is said so and never published. +- The pull request says what the agent said about the work, when the agent wrote a description for it; otherwise it says what was asked for, which is all the framework knows by itself. - Push and a draft PR are armed by default; drafts keep the automatic path out of reviewers' inboxes, and uncommitted leftovers are 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 its archive: 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.ts b/packages/the-framework/src/dashboard/agent-handoff.test.ts index b8b2cfe10..3773e282f 100644 --- a/packages/the-framework/src/dashboard/agent-handoff.test.ts +++ b/packages/the-framework/src/dashboard/agent-handoff.test.ts @@ -223,6 +223,40 @@ test('a remote-only branch gets its draft PR without any push (#1601)', async () assert.ok(!args.includes('--base'), "gh's default base is the repo's default branch") }) +test("the agent's own description becomes the PR body, in place of the intent (#1567)", async () => { + // The `open-pr` capability: the agent describes what the change turned out to be, and the + // framework opens the PR — so the agent has no reason to run `gh pr create` itself and lose + // the title, the ticket's issue reference and the recorded number along with it. + const ghCalls: string[][] = [] + await openRemoteBranchPullRequest( + '/repo', + { id: 'r1', sessionName: 'fix-the-thing', intent: 'fix it', description: '## What changed\n\nThe queue reader keeps its state across a reload.' }, + 'claude/fix-the-thing', + { + gh: async args => { + ghCalls.push(args) + return 'https://github.com/o/r/pull/14\n' + }, + }, + ) + const body = (ghCalls[0] ?? [])[(ghCalls[0] ?? []).indexOf('--body') + 1] ?? '' + assert.match(body, /The queue reader keeps its state across a reload\./) + assert.doesNotMatch(body, /fix it/, "the agent's description replaces the opening intent rather than joining it") + assert.match(body, /Opened from The Framework session/, 'the session line still rides along') +}) + +test('without a description the PR body still says what was asked for (#1567)', async () => { + const ghCalls: string[][] = [] + await openRemoteBranchPullRequest('/repo', { id: 'r1', sessionName: 'x', intent: 'fix it' }, 'claude/x', { + gh: async args => { + ghCalls.push(args) + return 'https://github.com/o/r/pull/15\n' + }, + }) + const body = (ghCalls[0] ?? [])[(ghCalls[0] ?? []).indexOf('--body') + 1] ?? '' + assert.match(body, /fix it/) +}) + test('a remote-only PR that gh refuses is a reported failure, never a throw (#1601)', async () => { const result = await openRemoteBranchPullRequest('/repo', { id: 'r1' }, 'claude/x', { gh: async () => { diff --git a/packages/the-framework/src/dashboard/agent-handoff.ts b/packages/the-framework/src/dashboard/agent-handoff.ts index 796d5b437..4bd7cc356 100644 --- a/packages/the-framework/src/dashboard/agent-handoff.ts +++ b/packages/the-framework/src/dashboard/agent-handoff.ts @@ -743,6 +743,12 @@ export type HandoffAgent = Pick 'Here are the options.\n```await-choices\n' + json + '\n```' @@ -160,3 +160,30 @@ 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.') +}) + +test('parsePullRequestDescription returns undefined when the agent wrote no block (#1567)', () => { + assert.equal(parsePullRequestDescription('just some output'), undefined) +}) + +test('parsePullRequestDescription keeps markdown whole, since the block is the PR body (#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) +}) + +test('parsePullRequestDescription 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') +}) + +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') +}) diff --git a/packages/the-framework/src/turn-gate.ts b/packages/the-framework/src/turn-gate.ts index c2730462f..434d370f5 100644 --- a/packages/the-framework/src/turn-gate.ts +++ b/packages/the-framework/src/turn-gate.ts @@ -181,6 +181,26 @@ export function parseSessionName(text: string): string | undefined { return name } +/** + * 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 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. + */ +export function parsePullRequestDescription(text: string): string | undefined { + let description: string | undefined + for (const body of blocks(text, 'open-pr')) { + const trimmed = body.trim() + if (trimmed) description = trimmed + } + return description +} + /** * 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 @@ -266,18 +286,20 @@ function parseGateBody(body: string): ParsedAwaitGate | undefined { /** * Emit the {@link PROTOCOLS_SIGNAL} signals an agent turn carries: markdown views, the - * session name, and `setReadyForMerge()`. Every turn the framework prompts goes through + * 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 only re-emits on an actual rename. Each caller makes one + * fires once, and a session name and a pull-request description only re-emit on an actual + * change. 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. */ export function createTurnSignalEmitter(emit: (event: FrameworkEvent) => void): (text: string) => void { let named: string | undefined let ready = false + let described: string | undefined return (text: string): void => { for (const view of parseMarkdownViews(text)) emit({ kind: 'view', ...view }) const name = parseSessionName(text) @@ -289,5 +311,10 @@ 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 }) + } } } From d373e1d45a82afce09776ef7941b71e79d36f8bb Mon Sep 17 00:00:00 2001 From: Suleiman Shahbari Date: Fri, 21 Aug 2026 03:10:55 +0300 Subject: [PATCH 2/3] Tie the open-pr block to the ready-for-merge moment, after a dogfood run skipped it MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Live evidence: a Sonnet run on a scratch project got the protocol verbatim in its system prompt (12,686 chars, `open-pr` present), emitted set-session-name and ready-for-merge from that same file, and did not emit open-pr. The other two signals name a concrete moment — the setSessionName() and setReadyForMerge() calls. This one said "when your work is ready to be published", which is not a moment the agent can recognize, and the agent had already emitted ready-for-merge by then. It now hangs off exactly that signal, and says what is lost without it. --- packages/the-framework/prompts/protocols/signal.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/the-framework/prompts/protocols/signal.md b/packages/the-framework/prompts/protocols/signal.md index beb3cd544..9caa4c04a 100644 --- a/packages/the-framework/prompts/protocols/signal.md +++ b/packages/the-framework/prompts/protocols/signal.md @@ -11,8 +11,8 @@ When you call setReadyForMerge() — you believe the work is complete and ready ``` ## Opening a pull request -When your work is ready to be published, describe it in an `open-pr` block and The Framework opens the pull request for you — you do not need to run `gh pr create` yourself: +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: ```open-pr ``` -The block is the body of the pull request. The Framework supplies the rest: 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 still works if you prefer; you then own all of the above. +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. From 4c11d09e1b2b25de23ba285b5b39c71fc881c183 Mon Sep 17 00:00:00 2001 From: Suleiman Shahbari Date: Fri, 21 Aug 2026 12:56:37 +0300 Subject: [PATCH 3/3] Defuse closing keywords with a filler, so the issue reference stays live MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Rom on #1612: wrapping the reference in backticks stopped the closing, but it also stopped the reference being a link, and stopped the issue's own timeline recording that a pull request had mentioned it. The ticket lost the backlink that says someone is discussing it. GitHub only obeys a closing keyword when the reference follows it directly, so two words in between are enough: `close #1164` becomes `close the ticket #1164`. The sentence still reads as the agent wrote it, the reference is untouched — clickable, and still cross-referenced onto the issue. Idempotence now falls out of the rewrite instead of needing a guard: after the first pass the keyword is followed by the filler, not by a reference. A reference already inside backticks is still skipped — it is a code sample, GitHub does not act on it, and the filler would corrupt it. Also lands the agent-handoff.test.SPEC.md clause deferred while #1611 was rewriting that sentence. --- .../src/closing-keywords.SPEC.md | 17 +++++++---- .../src/closing-keywords.test.SPEC.md | 2 +- .../src/closing-keywords.test.ts | 30 ++++++++++++++----- .../the-framework/src/closing-keywords.ts | 29 +++++++++++++----- .../src/dashboard/agent-handoff.test.SPEC.md | 2 +- 5 files changed, 58 insertions(+), 22 deletions(-) diff --git a/packages/the-framework/src/closing-keywords.SPEC.md b/packages/the-framework/src/closing-keywords.SPEC.md index c3ac4fed9..2004e203b 100644 --- a/packages/the-framework/src/closing-keywords.SPEC.md +++ b/packages/the-framework/src/closing-keywords.SPEC.md @@ -1,15 +1,22 @@ -Why a pull request that does not finish an issue must not carry a phrase GitHub reads as closing it, and how such a phrase is defused without changing what the sentence says. +Why a pull request that does not finish an issue must not carry a phrase GitHub reads as closing it, and how such a phrase is defused without changing what the sentence says or what the reader can click. + +## 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 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 -- A closing phrase is one of GitHub's keywords — close, fix, resolve, and their plural and past forms — followed by an issue reference, in this repository or another. -- Defusing wraps the reference in backticks and leaves every word alone, so the sentence a human reads is unchanged and the pull request stops closing the issue. -- A reference already wrapped is left as it is, so text that has been through this once can go through it again unchanged. +- A closing phrase is one of GitHub's keywords — close, fix, resolve, and their plural and past forms — followed directly by an issue reference, in this repository or another. +- Defusing puts the words "the ticket" between the keyword and the reference. GitHub only obeys the keyword when the reference follows it directly, so the phrase loses its authority while the sentence still reads as the agent wrote it. +- The reference itself is never touched, so it stays a live link and the issue still records that the pull request mentioned it. +- A reference already inside backticks is left alone: it is a code sample, GitHub does not act on it, and rewriting it would corrupt the sample. +- Text that has been through this once can go through it again unchanged, because the keyword is no longer followed by a reference. ## Rationales - The phrase is defused rather than forbidden: an agent writing "…then close #1164" as the last step of a plan is describing its plan accurately, and the sentence is worth keeping — what is wrong is only that GitHub acts on it. -- Backticks are the chosen form because they defeat the parser while rendering the same words, where rewording would put the framework in the business of editing an agent's prose. +- A filler is chosen over wrapping the reference in backticks, which was the first form of this: backticks stopped the closing but also took away the link and the mention on the issue's timeline, so the ticket was no longer told that a pull request had discussed it. Breaking only the adjacency keeps everything a reader gains from the reference and removes just the part a machine acts on. ## Before modifying/creating SPEC.md files diff --git a/packages/the-framework/src/closing-keywords.test.SPEC.md b/packages/the-framework/src/closing-keywords.test.SPEC.md index d0df73d42..b893469cd 100644 --- a/packages/the-framework/src/closing-keywords.test.SPEC.md +++ b/packages/the-framework/src/closing-keywords.test.SPEC.md @@ -1,4 +1,4 @@ -Covers defusing GitHub's closing phrases: the exact sentence that closed #1164 keeps its words, every keyword form and letter case is caught, the cross-repo reference is caught too, an issue mentioned without a keyword is untouched, a word merely ending in a keyword is not one, and defusing twice changes nothing. +Covers defusing GitHub's closing phrases: the exact sentence that closed #1164 keeps its words, the issue reference is never rewritten so it stays clickable and still cross-references, every keyword form and letter case is caught, the cross-repo reference is caught too, an issue mentioned without a keyword is untouched, a word merely ending in a keyword is not one, a reference inside backticks is a code sample rather than a command, and defusing twice changes nothing. ## Before modifying/creating SPEC.md files diff --git a/packages/the-framework/src/closing-keywords.test.ts b/packages/the-framework/src/closing-keywords.test.ts index 93fe8e8ee..fe5a301a5 100644 --- a/packages/the-framework/src/closing-keywords.test.ts +++ b/packages/the-framework/src/closing-keywords.test.ts @@ -3,26 +3,37 @@ import assert from 'node:assert/strict' import { defuseClosingKeywords } from './closing-keywords.js' // #1567: a plan PR whose body ended "…then comment on and close #1164" closed #1164 on merge, -// and the next tickets sync then removed the ticket and its fresh plan. The words stay; only -// GitHub's reading of them changes. +// and the next tickets sync then removed the ticket and its fresh plan. The words stay and the +// reference stays live; only GitHub's reading of them changes. describe('defuseClosingKeywords', () => { test('the phrase that closed #1164 keeps its words and loses its effect', () => { const before = 'The plan proposes: expose `queued` on the single-ticket read — then comment on and close #1164.' assert.equal( defuseClosingKeywords(before), - 'The plan proposes: expose `queued` on the single-ticket read — then comment on and close `#1164`.', + 'The plan proposes: expose `queued` on the single-ticket read — then comment on and close the ticket #1164.', ) }) test('every keyword form GitHub accepts is defused, whatever the case', () => { for (const keyword of ['close', 'closes', 'closed', 'fix', 'fixes', 'fixed', 'resolve', 'resolves', 'resolved']) { - assert.equal(defuseClosingKeywords(`this ${keyword} #42`), `this ${keyword} \`#42\``, keyword) - assert.equal(defuseClosingKeywords(`this ${keyword.toUpperCase()} #42`), `this ${keyword.toUpperCase()} \`#42\``, keyword) + assert.equal(defuseClosingKeywords(`this ${keyword} #42`), `this ${keyword} the ticket #42`, keyword) + const upper = keyword.toUpperCase() + assert.equal(defuseClosingKeywords(`this ${upper} #42`), `this ${upper} the ticket #42`, keyword) } }) + test('the reference itself is never rewritten, so it stays clickable and cross-references', () => { + // Rom's point on #1612: backticking the reference defused the phrase but also killed the + // link and the mention on the issue's own timeline. Only the adjacency may be broken. + assert.equal(defuseClosingKeywords('close #1164').includes('`'), false) + assert.match(defuseClosingKeywords('close #1164'), /(^|\s)#1164\b/) + }) + test('the cross-repo form closes just as well, so it is defused too', () => { - assert.equal(defuseClosingKeywords('fixes gemstack-land/the-framework#7'), 'fixes `gemstack-land/the-framework#7`') + assert.equal( + defuseClosingKeywords('fixes gemstack-land/the-framework#7'), + 'fixes the ticket gemstack-land/the-framework#7', + ) }) test('an issue mentioned without a keyword is left alone — a reference is not a command', () => { @@ -35,9 +46,14 @@ describe('defuseClosingKeywords', () => { assert.equal(defuseClosingKeywords(text), text) }) + test('a reference already inside backticks is a code sample, not a command', () => { + const text = 'write `close #42` to close it' + assert.equal(defuseClosingKeywords(text), text) + }) + test('running it twice changes nothing the second time', () => { const once = defuseClosingKeywords('fixes #9 and closes #10') assert.equal(defuseClosingKeywords(once), once) - assert.equal(once, 'fixes `#9` and closes `#10`') + assert.equal(once, 'fixes the ticket #9 and closes the ticket #10') }) }) diff --git a/packages/the-framework/src/closing-keywords.ts b/packages/the-framework/src/closing-keywords.ts index 1deca396f..59985c0eb 100644 --- a/packages/the-framework/src/closing-keywords.ts +++ b/packages/the-framework/src/closing-keywords.ts @@ -10,8 +10,9 @@ * * That happened (#1560 closed #1164, and the next tickets sync then deleted the ticket and its * fresh plan). The cure is not to forbid the phrase — the agent is describing its plan, and the - * sentence is true — but to write the issue reference in a form GitHub's parser does not read as - * a command. Backticks do it: `close `#1164`` renders as the same words and links nowhere. + * sentence is true — but to break the adjacency GitHub's parser needs. The keyword only counts + * when the reference follows it directly, so two words in between end its authority while the + * sentence keeps saying what it said. */ /** @@ -21,10 +22,16 @@ */ const CLOSING_KEYWORDS = ['close', 'closes', 'closed', 'fix', 'fixes', 'fixed', 'resolve', 'resolves', 'resolved'] +/** + * What goes between the keyword and the reference. Chosen to read as the sentence's own words + * rather than as an escape: "…then close the ticket #1164" is what the agent meant anyway. + */ +const FILLER = 'the ticket' + /** * A closing keyword, whitespace, then an issue reference — `#123`, or the cross-repo - * `owner/repo#123` form, which closes just as well. Only a reference already inside backticks - * is left alone, since it is already defused; that is what makes this safe to run twice. + * `owner/repo#123` form, which closes just as well. A reference already inside backticks is + * left alone: GitHub does not act on one, and rewriting it would corrupt a code sample. */ const CLOSING_PHRASE = new RegExp( String.raw`(^|[^\`\w])(${CLOSING_KEYWORDS.join('|')})(\s+)((?:[\w.-]+\/[\w.-]+)?#\d+)(?!\`)`, @@ -33,11 +40,17 @@ const CLOSING_PHRASE = new RegExp( /** * Rewrite every closing phrase in `text` so GitHub stops reading it as a command, leaving the - * words as the agent wrote them: `close #1164` becomes ``close `#1164``. + * issue reference itself untouched: `close #1164` becomes `close the ticket #1164`. + * + * The reference stays live — clickable, and still cross-referenced onto the issue's own timeline, + * so the ticket is told a pull request mentioned it. Only the closing authority is removed. * - * Prose only — the reference keeps its own text, so a human reads the same sentence and the - * PR simply stops closing an issue it did not finish. + * Idempotent by construction: after the rewrite the keyword is followed by the filler rather than + * by a reference, so a second pass finds nothing to change. */ export function defuseClosingKeywords(text: string): string { - return text.replace(CLOSING_PHRASE, (_all, before: string, keyword: string, gap: string, ref: string) => `${before}${keyword}${gap}\`${ref}\``) + return text.replace( + CLOSING_PHRASE, + (_all, before: string, keyword: string, gap: string, ref: string) => `${before}${keyword}${gap}${FILLER} ${ref}`, + ) } 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 9dd324ddc..9168f01d7 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, 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, merge authorization, and the human Merge action with its refusals. ## Before modifying/creating SPEC.md files