Skip to content

Agents open a pull request by describing it, rather than running gh themselves (#1567) - #1612

Open
suleimansh wants to merge 2 commits into
mainfrom
open-pr-capability
Open

Agents open a pull request by describing it, rather than running gh themselves (#1567)#1612
suleimansh wants to merge 2 commits into
mainfrom
open-pr-capability

Conversation

@suleimansh

Copy link
Copy Markdown
Member

🤖 agent PR

Fix #1567, built to @brillout's answer on the issue:

As always, I'd frame it as a capability: an agent can call a TF function (e.g. tf_open_pr) to trigger the PR creation… But the agent can decide to not call the TF function and do it itself instead. We avoid limiting agents — we enable agents instead.

So there is no prohibition in this PR.

The problem. A plan agent's PR body ended "…then comment on and close #1164." GitHub read that as a closing keyword: the plan merged, #1164 closed even though only the plan had landed, and the next tickets sync saw a closed issue and deleted the ticket and its fresh plan. Both had to be restored by hand.

Why the obvious fixes don't work. Linting closing keywords out of PR bodies breaks the correct case — 8 of the last 40 PRs legitimately close their issue that way (#1604, #1603, #1595). And the body that caused the incident was the agent's own: it ran gh pr create itself, outside the handoff, where the framework never sees the text at all.

What the user sees. Pull requests the framework opens are described by the agent that did the work, instead of repeating the prompt it was given. Today's body is the opening intent plus a session line; now it is what the change turned out to be.

The capability. A new non-blocking block, alongside set-session-name and ready-for-merge:

```open-pr
## What changed

The queue reader keeps its state across a reload.
```

The agent writes the description; the framework opens the PR and keeps the parts that have to stay consistent — the title from the session name, the ticket's (fix #N) (#1334), and recording the number on the agent so every surface reads the same one. The block is re-emittable, last one wins, and an agent that prefers to open its own PR still can — it then owns all of the above, which is the trade the issue was really about.

Where the original bug is actually fixed. A plan agent's description is defused before publishing, because a plan agent's PR lands the plan, not the work. That rule already exists for the title — maybeAutoHandoff deliberately withholds (fix #N) from a plan agent (#1327, "its PR lands the plan, not the work, so the merge must not close the issue") — and this is its other half. Defusing wraps the reference in backticks: close `#1164 reads identically to a human and links nowhere.

How it is wired, following ready-for-merge exactly: parsePullRequestDescription in turn-gate.ts → a pull-request-description event through createTurnSignalEmitter → the agent journal's prDescription()maybeAutoHandoff, which defuses it for a plan agent and hands it to agentPrBody. One new module, src/closing-keywords.ts, owns GitHub's closing grammar.

Tests. 13 new: the defuser (the exact #1164 sentence, every keyword form and case, the cross-repo owner/repo#123 form, a keyword-less mention left alone, a word merely ending in a keyword, and idempotence), the block parser (markdown kept whole, last block wins, empty block ignored), and the PR body itself (the description replaces the intent; without one the intent still shows). Suite green: 1502 framework, 776 dashboard, typecheck clean.

Not in this PR.

  • src/dashboard/agent-handoff.test.SPEC.md should gain a clause for the two new body tests, but Fix everything the SPEC.md migration's grounding pass flagged #1611 is rewriting that exact sentence right now — it is a one-line follow-up once that lands, rather than a conflict today.
  • Not yet dogfooded against a live agent: the protocol text is compiled into the prompts and the parser is tested, but no real turn has emitted the block yet.
  • The dashboard shows the event as a log row; rendering the description itself is a later slice if it is wanted.
  • The two remaining questions from the issue investigation stay open: whether the prompts should encourage the capability more strongly than the one paragraph added here, and Error capability #1500's error capability, which is the same mechanism again.

…ther than running gh themselves

#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.
@suleimansh

Copy link
Copy Markdown
Member Author

🤖 agent comment

SPEC walkthrough — five files, each as intro + Flows, then the diff, then what it means against the rest of the file.


1. src/closing-keywords.SPEC.md (new). Whole file:

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.

Meaning: this is the one place that knows GitHub's issue-closing grammar. It is a new file rather than a branch inside the handoff because the grammar is GitHub's, not the handoff's — and because the second emitter that will want it (#1500's error capability, or any later PR-writing path) should not have to reach into the handoff to find it. The idempotence flow is what makes it safe for a caller that cannot be sure whether the text has already been through it.


2. src/turn-gate.SPEC.md — the turn-boundary contract. Intro and the Flows as they now read:

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.

## 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).
- There is one gate block, not four …
- Parsing is tolerant on purpose …
- One continuation wording resumes the agent after any answered gate …
- 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.

Diff:

-… its chosen session name, ready-for-merge — is a tagged block parsed out of the turn's final message.
+… its chosen session name, ready-for-merge, a pull-request description — is a tagged block parsed out of the turn's final message.

-- … the non-blocking signals (markdown views, session name, ready-for-merge).
+- … the non-blocking signals (markdown views, session name, ready-for-merge, a pull-request description).

-- Signal emission is deduped across a span of turns: ready-for-merge fires once, and a session name re-emits only on a real rename.
+- 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.

And one added rationale:

+- The pull-request description is offered as a capability rather than the agent being told not to open its own: an agent that describes the work and lets the framework publish it keeps the title, the ticket's issue reference and the recorded pull-request number, and an agent that prefers its own way is not stopped.

Meaning: the file's first Flows line is that the protocols pin how to emit, not when — and the new block joins that list unchanged in kind. It is a non-blocking signal, so it belongs beside session-name and ready-for-merge rather than beside the gate: the agent does not stop for it. The dedupe line already covered "re-emits only on a real rename" for the session name; a description behaves the same way for the same reason, so the two share one sentence rather than getting one each. The rationale is the only new argument in the file, and it is the issue's whole answer: enabling rather than forbidding.


3. src/dashboard/agent-handoff.SPEC.md — how a finished agent's work is handed back. The Flows around the change:

- Branch-addressed: an agent reads the same whether or not its checkout still exists …
- 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 …
- The PR number is recorded on the agent the moment one is opened for it …

Diff: the third bullet is new.

Meaning: it sits directly above the arming bullet because it describes the PR's content, where the next lines describe whether it is opened at all and what is recorded about it. The "otherwise" half is the honest statement of today's behavior — the intent is what the framework knows on its own, and it says what was asked at the start rather than what the change became.


4. prompts/SPEC.md — what the prompts are.

-… and how to emit the non-blocking signals (show a document, name the session, ready-for-merge);
+… 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);

Meaning: the sentence lists the agent's side of the conversation. The wording is deliberately "the pull request the framework will open" rather than "open a pull request" — the agent describes, the framework publishes, and the spec should not imply the block itself is the publishing act.


5. src/turn-gate.test.SPEC.md — what the turn-gate tests cover.

-… ready-for-merge detection, and the single continuation wording shared by every path.
+… 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.

Meaning: the three parenthesised cases are the three that could silently corrupt a PR body — markdown mangled, a stale first draft published over a revision, or an empty block blanking the body that would otherwise have been written.

Plus src/closing-keywords.test.SPEC.md, new, describing the six defuser cases.


FEATURES-SPEC.md gains one line under Handoff and what lands in git, after "Open a PR (on by default)":

+- The PR is described by the agent that did the work, when it wrote a description for it

One spec is deliberately not updated here: src/dashboard/agent-handoff.test.SPEC.md should gain a clause for this PR's two new body tests, but #1611 is rewriting that same sentence right now. It is a one-line follow-up once #1611 lands, rather than a conflict today.

…run skipped it

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.
@suleimansh

suleimansh commented Aug 21, 2026

Copy link
Copy Markdown
Member Author

🤖 agent comment

Dogfooded on the real dashboard. The first run failed — that's why there's a second commit.

Run 1: the agent ignored the block

A local Sonnet agent, a plain task ("Add a README.md…"), no mention of the capability anywhere in the prompt. Its system prompt did contain the open-pr instructions. It ended its turn with two blocks and nothing else:

```set-session-name
add-readme
```

```ready-for-merge
```

Both of those come from the same protocol file as open-pr. So the agent was reading the file. It just skipped my part.

Why it skipped it. The other two blocks say exactly when to emit them — "when you call setSessionName()", "when you call setReadyForMerge()". Mine said "when your work is ready to be published". That isn't a moment an agent can point at. And by the time it got there, it had already sent ready-for-merge.

The fix (d373e1d4): give it the same moment. "Whenever you emit ready-for-merge, emit an open-pr block too."

Run 2: it works

Rebuilt, restarted the daemon, new task. The agent emitted the block on its own, and the framework recorded this as the PR description:

Adds a root-level CONTRIBUTING.md with three short bullets explaining how to run this static demo locally (open index.html directly, or serve it with npx serve . / python3 -m http.server, then edit and refresh). The repo has no build step — it's just a single HTML file — so the instructions are minimal to match.

Here is the same PR's body as it would have read without the block — it is the prompt, repeated back:

Add a CONTRIBUTING.md at the repo root: three short bullets on how someone would run this demo locally. Keep it under 15 lines.

One says what the change is. The other says what was asked for. That difference is the whole feature.

The dashboard shows it as a new row, PULL REQUEST DESCRIPTION, directly under READY FOR MERGE.

What is still untested

The last step: handing that description to gh pr create --body. The scratch repo has no remote, so the handoff correctly skipped opening a PR. That step has a unit test but has never run against GitHub. Happy to do a real run if you want a PR opened somewhere.

(Also confirmed in passing: #1604's project-error banner is live and correct on the same page — "The data branch is not syncing · since 11m ago — the repository has no remote".)

@suleimansh

suleimansh commented Aug 21, 2026

Copy link
Copy Markdown
Member Author

🤖 agent comment

Ran the capability across model tiers, since signal emission is known to vary by tier (#1334's evidence table: Haiku never emits setReadyForMerge() at all). Same scratch project, same shape of plain task, no mention of the capability in any prompt.

Model open-pr emitted What the run did
Opus Multi-paragraph description; volunteers what was not touched
Sonnet After the wording fix (d373e1d4)
Sonnet Before the fix — the original "when your work is ready to be published" wording
Haiku Emitted no signal blocks at all — no set-session-name, no ready-for-merge, no open-pr
Fable not tested Monthly spend limit exhausted, resets 2026-08-22

Opus produced this, unprompted:

Adds a LICENSE-NOTE.md at the repository root.

The file is 4 lines: a short heading plus the two requested bullets stating that this is a demo repository published for demonstration/experimentation only, and that it is not intended for production use (no support, warranty, or fitness for a particular purpose implied).

No other files were touched — the repo's only source file, index.html, is unchanged.

That last line is the argument for the capability in one sentence: it volunteers the negative, which is what a reviewer wants and which no framework-composed body could ever produce, since the framework only knows the prompt it sent.

Haiku confirms the prediction, and confirms it is a tier property rather than anything about this block. Its system prompt was 12,801 chars and contained the open-pr protocol; it emitted no blocks of any kind and simply ended with prose:

Done. I've added a CHANGELOG.md at the repo root with an "Unreleased" section containing two bullets describing the demo's current functionality…

So the tier that already fails ready-for-merge fails this too, for the same reason and at the same point. Since the open-pr block now hangs off that signal, the two travel together: a model that never declares itself ready never describes its pull request either. That is coherent rather than broken — a run that does not consider itself finished has nothing to describe yet — and it means #1334's merge-armed model floor would carry this capability along with it, at no extra cost.

Worth noting the dashboard already warns about exactly this when Haiku is selected: "Haiku consistently skips the session-finish protocol, so a publishing run ends as an unmerged draft PR and needs hand-holding." This capability lands on the correct side of a line the product had already drawn.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Plan PRs can auto-close their ticket's issue via closing keywords in PR body prose Queue button doesn't seem to work?

1 participant