Skip to content

Commit 4baf097

Browse files
mchwangclaude
andauthored
F2d: already-fixed check and opening the task's PR (#84)
* F2d: check whether the issue is already fixed, then open the task's PR Before opening a PR, check the issue's close, links from other open or merged PRs, and new base commits that mention it. Exclude the task's own PRs by repository and number and its own commits by SHA. Fail closed past every bound. A match or an incomplete check opens nothing and moves a running task to possibly already fixed. Otherwise push the task head, re-read the task right before the GitHub call, and open the PR (a draft with open problems for needs human). Record each opening first, with a marker in the description, so a lost outcome is recovered instead of opening twice. A later run reuses the task's still-open PR. Plan text and problems are fenced so closing keywords and mentions in them do nothing. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Address review round 1 on F2d: ownership, lost openings, stale responses - Branch names end in a hash of the exact task identity, so tasks whose IDs normalize alike never share a branch or PR. - gh runs with an allowlisted environment only. - The PR adapter refuses answers for a PR that is not open. - Publishing checks the task status before the no-changes shortcut. - An opening GitHub does not show yet stays owned until its settle time has passed; publish reports OpeningUnsettled instead of posting again. - Each opening or refresh owns the task state version it was guarded at; a response for an older version is recorded but never moves the task. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Keep a needs-human task in needs human when it reuses a ready PR; avoid spreading backtick runs A refresh for a needs-human task turns the earlier PR back into a draft, and the task status changes only from running. fenced() finds the longest backtick run in a loop, so unbounded plan text cannot overflow the stack. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Pass the D-Bus session bus to gh, so Linux keyring sign-in works Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * AGENTS.md: rules from the F2d review rounds Guarded status transitions, preconditions before shortcuts, platform credential channels in subprocess allowlists, and no spreading of unbounded collections. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Address Copilot round 2 on F2d: pre-push re-read, disconnects, deadline, refresh record - Re-read the task right before the push, after the last await. - Replay connected and disconnected timeline events; a disconnected PR no longer counts as linked. Cross-references stay. - One deadline for the whole already-fixed check; it aborts the running gh call and makes the check unknown. - Record an update of the open PR before it starts; if its confirmation is lost, the next publish drops the record and repeats the update. - AGENTS.md: replay add and remove events for removable relations. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Address Copilot round 3 on F2d: review version, one publish per task, abort, deadline - Bind each check to the plan's review_version and refuse every push, open and refresh if approvals, choices or notes changed since. - Run one publish per task at a time, so an in-flight update is never cleared or overtaken. - Check the signal before any durable write. - Default the check deadline to 12 s, below the 15 s request budget. - AGENTS.md: honour an aborted signal before the first durable write. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Share the one-publish-per-task guard across publishers over the same Store Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Close handed-off review findings on F2d: references, adoption, ProjectV2, order - Neutralise issue references in the PR title, plan and problems, since GitHub closes issues from keywords in default-branch commit messages, which fences do not protect. - Look up the branch PR with the markers of every earlier opening, abandoned ones included, before the check; adopt an abandoned opening's PR (and count it as own) instead of getting stuck behind GitHub's one-open-PR-per-branch refusal. - Treat a ProjectV2 closer as a closing by someone else, not unknown. - Refuse a PR-number mismatch before the push. - Run the timeline and base-commit reads together; the first failure aborts the other. - Cut titles and problems by UTF-16 unit without splitting a surrogate pair; an empty summary becomes "codeboost plan". - AGENTS.md: three rules from these findings. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Address Copilot round 4 on F2d: stdin transport, exact problem cut, settle both reads - Send the PR title and description as a JSON body on stdin (gh api --input -), so a large multibyte description or a NUL never hits the per-argument limit. The runner settles after gh exits. - Cut each shown problem to exactly 2,000 characters. - The already-fixed check aborts the other read on the first failure but returns only after both have settled. - AGENTS.md: pass input-sized text on stdin, not as an argument. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Escalate to SIGKILL when gh ignores SIGTERM, so a timeout or abort always settles Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Close high-effort review findings on F2d: settle, head read-back, drafts - The already-fixed check's default runner reuses runWithInput, so an aborted stage escalates to SIGKILL and the 12 s deadline holds. - runWithInput settles after exit even if a process gh started keeps the pipes open, closing them after a grace period. - A refresh read-back polls briefly until GitHub shows the pushed head, so a correct publish is not sent to needs human by a stale head. - When the check matches, the task's earlier ready PR becomes a draft, so it is never left ready for review. - #recover reads the task's PR records once. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Draft the earlier PR before recording a matching check, so a failure stays retryable Also refuse a PR-number mismatch before any GitHub change, and remove each head-poll abort listener when its wait ends. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Address Copilot round 5 on F2d: verified draft state, re-read before drafting, title mentions, deadline budget - markDraft and refresh poll the read-back for the requested draft or ready state and fail if GitHub never shows it, so a publish stays retryable instead of recording a change that did not apply. - Re-read the task, its review and its head right before drafting the earlier PR; a stale publish no longer touches the current PR. - Neutralise @-mentions in the unfenced PR title. - The check's runner waits 1 s before SIGKILL and 0.5 s for pipes, so the whole check settles in 13.5 s, below the 15 s request budget. - AGENTS.md: grace periods count inside deadlines; read back external state changes; neutralise mentions in unfenced titles. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Close the xhigh review findings on F2d: own merged PR, drafts unsupported, refresh ordering - A close by the task's own PR or commit, and the task's own merged PR, now count as already fixed; only its own open PR is excluded. - A repository without draft PRs: GitHub's refusal becomes DraftsUnsupported, a definite outcome. Opening drops the opening, refresh drops the pending update, and a matching check reports the ready PR it could not draft (leftReady). No loops, no ready PR for a needs-human task. - The refresh is recorded before the push that moves the open PR's head, so a task change during the push cannot strand a half-updated PR; the new-PR path keeps the guard after the push. - gh gets the Windows variables it needs; GH_PAGER is empty there. - Commit subjects are cut without splitting a surrogate pair, through a shared core/text.ts; the GitHub adapters share github/validate.ts. - markDraft changes the PR straight away and reads it back once. - recordPullRequestOpened is split from recordRefreshConfirmed. - Test Stores are closed after each test. - AGENTS.md: exclusions only in the states they are for; record in-flight ownership before an operation's first external write. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Address Copilot round 6 on F2d: branch lookup always, draft first, docs, v6→v7 test - Look up the branch's open PR on every publish, even with no known markers, so a PR codeboost did not open is refused before the push can move it. - For a needs-human task with a ready PR, make it a draft before the push or any description change; a "drafts unsupported" refusal now leaves the PR exactly as it was. A failure later in the refresh stays recorded in flight and is redone by the next publish. - Correct the already-fixed module summary to the implemented policy. - Add a v6-to-v7 upgrade test on a populated file database. - AGENTS.md: refusable steps come before writes that cannot be undone. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Address Copilot round 7 on F2d: marker line only, draft flag repair, recovery draft reconcile - A PR's marker is read only from its description's first line; a marker-shaped string in plan text or problems no longer identifies, or blocks, anything. - When publish sees the task's PR, it corrects the recorded draft flag from GitHub under the state-version guard, so a draft change whose record was lost is repaired. - Recovering a lost draft opening turns a PR that was made ready meanwhile back into a draft before confirming it; a failure keeps the opening owned. - AGENTS.md: markers only from their reserved position; correct owned fields from the observed external record. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Address Copilot round 8 on F2d: duplicate commits, stale recovery, verified draft opening - The base comparison fails closed when a commit appears twice, since another commit must then be missing. - A recovered opening ends the publish only when it is this publish's own work (same task version and draft mode); otherwise the PR is recorded and publishing continues with the current head and mode. Draft reconciliation in recovery runs only for draft publishes. - A draft opening that GitHub creates as ready fails, so the opening stays owned and recovery turns the PR into a draft. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Address Copilot round 9 on F2d: one deadline per PR operation, adopt on sight - Each PR operation (open, lookup, refresh, draft change) runs under one deadline for all of its commands and poll waits (60 s by default), combined with the caller's signal, instead of 30 s per command. - An abandoned opening's PR is adopted as soon as publish sees it, under the state-version guard and without changing the task status, so a non-clear check no longer leaves the PR unrecorded. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Refresh only an opened PR: drop beginRefresh's unused adopt path Adoption of an abandoned opening's PR happens in adoptOpening, under the state-version guard, when publish first sees the PR. beginRefresh now requires an opened row, leaving one adoption path. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Address Copilot round 10 on F2d: own the review version too - recordAlreadyFixed takes the review version the publish read and refuses a result obtained before a review change. - Each opening and refresh stores the plan's review version it owns; its confirmation still records the PR but moves the task only when both the state version and the review version are unchanged. Recovery treats a lost opening as current only under both. - Design doc: adoption happens in step 3, not in the update; the owned versions are the state and review versions. - AGENTS.md: an in-flight action owns every version counter. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Close the independent review's findings on F2d: guard after push, head mismatch, no-changes, settle - Refresh path: re-read the task, its review and its status after the push's await, before the PATCH or ready/draft change. - A PR that GitHub shows at another head than the one pushed no longer moves the task to needs human with a ready PR: the task stays running, the PR is made a draft, and the record keeps the reported head. - The "no changes" path looks up the branch first and turns an earlier ready PR into a draft before moving the task to needs human. - An update whose confirmation was lost is settled from what GitHub shows (draft flag, head) before its record is dropped, for every pending row. - adoptOpening and the draft-flag repair also require the review version to be unchanged. - Publishing refuses a task with interrupted work waiting to be requeued or a rebase in progress. - Tests: the two tests that asserted the unguarded behaviour now assert the guarded one; added tests for every guard the review listed as untested (review during the check, active attempt, recovery's drafts-unsupported and other-repository branches, paging changes). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Close independent review round 2 on F2d: no-changes guards, leftReady, mismatch order - The no-changes path runs the full publish guard (including requeue and rebase) after the lookup, handles drafts being unsupported (leftReady), and moves the task to needs human only then. - The PR-number mismatch is refused before any GitHub change on every path, including no-changes. - After an open or refresh, a ready PR is made a draft unless its task is now in review at the pushed head; a failure there is reported as leftReady instead of failing the publish that succeeded. - Tests: the two review-change tests now assert the PR ends as a draft; new tests for each fix and for settleUnconfirmedRefresh's number match. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Close independent review round 3 on F2d: recovery drafts, no-changes re-read, settle errors - Recovery makes a recovered ready PR (lost opening or lost update) a draft when its task can no longer be published as ready, since the main path will refuse on status. - The no-changes path re-reads the task after a refused draft change, as it does after a successful one. - #settleHead reports only the GitHub call's failure as leftReady; a Store failure after a draft change that landed propagates. - Tests for every guard the review found untested: the lost opening's mode check, a recovered opening that is now a draft, one opening in flight, the refresh confirmation filter, an abort during the refresh push, issue/PR state and closer validation, the read-back identity check and markDraft's abort rethrow. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Close independent review round 4 on F2d: busy tasks, early guard, wrong-repo updates - Recovery makes a recovered ready PR a draft whenever the task cannot be published as ready right now, including a running task with an attempt active, requeue pending, rebase or merge in progress. - The full publish guard runs before any GitHub call, so a busy task is refused before the check. - A pending update recorded for another repository is refused loudly instead of being cleared after a lookup in the wrong repository. - #settleHead drafts a draft publish's PR whatever GitHub returned, and uses the recovered opening's base; the test that locked in a ready PR for a needs-human task now asserts the draft. - Tests for the guards the review found untested: own-PR repository filter, branch-record base filter, abort during the lookup, the clear check before an opening, PR-number matches, commit message, base head and commit-count validation. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Close independent review round 5 on F2d: re-check before ready, adopt in recovery - refresh() calls a beforeReady check after the PATCH's await and before any ready or draft change; the publisher passes assertRefreshCurrent, so a cancel or review during the description update never marks the PR ready (and never sends review requests). - Recovery adopts an abandoned opening's PR when the branch PR carries its marker, and makes it a draft if the task cannot be published as ready; a task that left running no longer keeps an unrecorded ready PR. - Tests: the no-changes re-read, the in-review and publishable exemptions, beforeReady at the gateway and in the publisher, and the recovery adoption. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Close independent review round 6 on F2d: shutdown guard, owed drafts - The publisher reads the coordinator's closing flag and its own right before each push, opening, description update and ready change, and close() aborts and awaits every publish in progress (runner-lifecycle.md, "Irreversible actions"; AGENTS.md in-flight work). - A task that cannot be published in the requested mode has every PR its record shows as ready looked up and made a draft before the status refusal. The "ready" record is the durable signal, so a failed draft change (leftReady) is repeated by the next publish, and recovery no longer drafts in three places of its own. Drafts being unsupported is named in the refusal instead of dropped. Tasks in review, approved or merged are left alone (GitHub does not merge a draft), and a GitHub failure in this step is noted in the status refusal, not raised instead. - The no-changes path checks the abort signal before moving the task. - Gateways configured for another repository are refused; the gh environment allows TEMP and TMP. - Tests: main-path adoption and its own-PR exclusion, draft-mode no changes, the shutdown checks, owed drafts, abort on no changes, the repository check. Doc: the new step, shutdown, the 66 s bound. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Close independent review round 7 on F2d: adopt late PRs after a stop - The draft step looks up every branch with a PR recorded as ready or an abandoned opening, so an abandoned opening's PR that appears after its task stopped is adopted and drafted; before, no path reached it. - It re-reads the task after the lookup, so a task approved meanwhile keeps its PR ready, and corrects the draft flag only where it differs. - A failed lookup gets its own note; an abort in the step, or during the head-mismatch draft change, rejects instead of becoming a note or leftReady. - Doc: the results table is whole again; the draft step and shutdown wording match the code. - Tests: late adoption, approval during the lookup, the three abort paths, the repository filter, abort during the head-mismatch draft. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Test the draft step's late-adoption record and abort after a lookup - Tests: a late draft PR is recorded by adoption alone (one state version bump); one a person made ready is adopted as ready, drafted and recorded as a draft; an abort during a lookup that still answers adopts nothing. - Doc and comment: the main path may refuse before it adopts; tasks in review, approved or merged have no draft step; the PR-number wording names the draft step. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Close independent review round 8 on F2d: tests for untested guards - Tests: adoption's version guard (a task change during the lookup that adopts refuses the main path), recovery drafting a lost draft opening only for a draft publish, a non-current lost draft opening continuing to the main path when drafts are unsupported, and a URL fragment (`/#12`) not counting as an issue mention. - Doc: a draft owed is fulfilled only by a later publish; nothing calls publish for a cancelled task yet. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Close independent review round 9 on F2d: both sides of a manual link - The already-fixed check reads both the source and the subject of a connect or disconnect event and takes the side that is a PR, so a link made from the PR's side is not missed (fail-open). Two PRs, or a side of an unknown type, make the check unknown. - Index the new check and PR tables by task; publish reads both by task on every call. - Tests: links from either side, two-PR and unknown-type links, an unknown closer, a process that exits without reading a large body (no crash on the broken pipe), and output past the limit. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Close independent review round 10 on F2d: no read:project field - The timeline query asked for `number` on a ProjectV2 closer. That field needs the read:project scope, which a default `gh auth login` token lacks, and GitHub then refuses the whole query: every check was `unknown` and no PR was ever opened. The closer is now read by its type name alone, which is all the code uses. Checked against GitHub with a default token: closed issues, a PR-side link, and an open issue with no links (clear). - Test: the query text reads both sides of manual links and selects no field on ProjectV2, since the fake answers any query. - Doc: the scopes the check and the PR calls need. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Close independent review round 11 on F2d: definite 422, retargeted PRs - A validation refusal of the POST (HTTP 422: no commits between base and head, a PR already open, and so on) is definite: the adapter throws PullRequestRefused and the publisher abandons the opening at once, instead of leaving it to settle for 10 minutes and repeating the same refusal forever. - The failure of a gh run keeps the response body gh prints on stdout, so GitHub's reason is no longer lost. - The branch lookup is no longer filtered by base: a PR a person retargeted to another base is refused, instead of being missed and a second PR opened from the same branch. More than one open PR from the branch is refused by name. - Tests follow: the refused POST now leaves no opening in flight, so the late-PR adoption tests go through the main path (two duplicates removed); new adapter and runner tests. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Count only closing cross-references in the already-fixed check A cross-reference is a mention: a PR anywhere on GitHub whose text names the issue. Run against cli/cli, the check flagged a third of the 100 newest open issues, mostly for merged PRs in unrelated repositories, so those tasks would never get a PR. Decided with the user: a cross-reference counts only when it would close the issue (willCloseTarget, a closing keyword). Manual links, the closed state and base-branch commits are unchanged. Checked live (read-only): the mention-only issue is now clear, a closed issue is still found. - Tests: mention-only PRs here and in another repository are clear; a non-boolean willCloseTarget makes the check unknown; the query asks for willCloseTarget. - Doc: the rule, the decision, and why a merged closing PR shows up as the closed state. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Close independent review round 12 on F2d: branch lookup across bases - The branch lookup lists every open PR from the branch and uses the one into the configured base. A PR into another base is ignored (a backport someone opened), unless it carries one of the task's markers: then the task's own PR was retargeted or the base setting changed, and publish refuses with a message that says what to do. Round 11's version refused any other-base PR and any second PR, which could block recovery and the draft step for good, and sent markers for the configured base only, so a retargeted own PR read as foreign. - Every lookup sends the markers of all the branch's records, whatever their base; an update uses the configured base the PR was found in. - Refusals (HTTP 422, drafts unsupported) are matched against gh's stderr only (CommandFailed.stderr): the stdout now kept in the message can echo the PR body. - Tests: the harness models PR bases; a retargeted own PR, a backport from the branch, stderr-only matching. Doc: lookup rule, repeated refusals, test list. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Count same-repo PRs into a non-default base in the already-fixed check GitHub closes issues only from PRs into the default branch, so it reports willCloseTarget false for every PR into another branch, even with "Fixes #12" (seen live on PRs into staging). With the closing-only rule, a task whose base is develop or staging would miss a competing fix into that branch and could open a duplicate PR. Decided with the user: when the task's base is not the repository's default branch, a PR in this repository into that same base that references the issue counts too. A missing default branch or PR base fails closed. - Query: the repository's default branch and each cross-referencing PR's base (repo scope only; the query still runs with a default token, checked live read-only). - Tests: the rule for a develop base, not for another base, another repository or a default-branch base; fail closed on a missing default branch or base; the query text. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Close independent review round 13 on F2d: configured base everywhere - Recovery (lost openings and lost updates), the draft step and the head settle now look PRs up and change them in the configured base, like the main path. Before, they used the base stored on each record: after a base change, a retargeted PR was asked to go back to the old base by recovery and to the new one by the main path, so only closing it ended the loop, and a stopped task's PR could stay ready. - Refusals are matched against gh's stderr and GitHub's own error fields in the JSON body on stdout (message, errors[].message). Round 12 matched stderr only, but gh api prints only "Validation Failed (HTTP 422)" there, so a drafts-unsupported refusal of the POST would have become PullRequestRefused. Only a finished gh run (CommandFailed, now carrying stdout too) is read. - Tests: adapter tests use failures shaped as gh reports them; the harness checks PR bases like the adapter; base change + retarget converges through a lost update, a lost opening, and the draft step. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Close independent review round 14 on F2d: draft a stopped task's PR in any base - The draft step finds the task's own PR in whatever base it is (findOpened anyBase: only PRs carrying the task's markers, with their base) and makes it a draft there. Round 13's configured-base lookup got the retarget refusal for a PR left in an old base, so a stopped task's PR stayed ready. Recovery and the main path keep the configured base. - Refusals: the top-level message is already on stderr, so only errors[].message is read from the JSON body. - Tests: a stopped task's PR in its old base is drafted; recovery's draft change and the head settle use the configured base after a base change; the any-base lookup finds own PRs only and refuses two. Doc: the base rules, the refusal sources, the test list. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Close independent review round 15 on F2d: observe in any base, write in one Rounds 12 to 15 each found a case where two lookups disagreed about which PR is the task's after a base change or a retarget, or where one lookup's refusal stopped another step's safe action. One rule now: - Observing and drafting (recovery of lost updates and openings, the draft step) use findOwned: every open PR from the branch that carries one of the task's markers, in any base, with its base. Records and draft changes happen where the PR is. A stopped task's PRs are all drafted, and a lost update settles, whatever the base setting says. - Changing content (push, update, ready change, opening) stays on the main path in the configured base (findOpened), which now refuses two of the task's PRs, and its own PR in another base even when the configured base has one too. - A lost opening is dropped only when another of the task's PRs is open into the base that opening asked for. - Tests: the harness mirrors both lookups and one PR per branch and base; a lost update + base change on a cancelled task, two own PRs (drafted when stopped, refused when running), lost draft opening and head settle in their own base, a young lost opening not dropped for an own PR in another base. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Close independent review round 16 on F2d: in review only from the configured base - Recovery confirms a lost opening's PR found in any base, and that confirmation could move the task to in review for a PR the main path would refuse: one retargeted or left in an old base, or one with another of the task's PRs open. Once in review nothing reconciled it, and a merge would target the wrong base. recordPullRequestOpened now takes mayReview; recovery passes it only for a PR in the configured base that is the task's only open PR. Any other is recorded and the head settle makes it a draft. - Recovery also adopts the task's other open PRs (abandoned openings) on that path, which ends the publish before the main path or the draft step could. The adoption on the "created nothing" path is removed: the main path or the draft step adopts next. - Tests: retarget and base change after a lost opening, two own PRs. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Close independent review round 17 on F2d: draft misplaced PRs before refusing - The main path's refusal for the task's PRs being where it cannot publish (its own PR in another base, or two of its PRs open) is now PullRequestMisplaced. Before refusing, the publisher runs the draft step with the task treated as not keeping ready, so none of its PRs stays ready while a person retargets or closes them. Before, a running task counted as "will be published as ready", so a PR that recovery recorded (not current, or its draft change failed), or an in-review PR sent back and then retargeted, stayed ready for good. - The drafts-unsupported branch of recovery leaves adopting the task's other PRs to the next publish's draft step (documented). - Tests: the three routes to a misplaced ready PR; both refusals are PullRequestMisplaced; two own PRs are drafted before the refusal. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Close independent review round 18 on F2d: look up every misplaced PR - Before refusing misplaced PRs, the draft step now looks up every branch with an opened record, not only ones recorded as ready: a PR recorded as a draft may have been made ready by the person who moved it. Test: a needs-human draft PR made ready and retargeted is drafted. - The notes of both draft-step runs are kept in the refusal, instead of the second shadowing the first. - The abort check in the misplaced catch is removed: the draft step rethrows on an aborted signal itself. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Close independent review round 19 on F2d: one path for misplaced PRs - Recovery ends a publish only for a PR the main path would accept (in the configured base, the task's only open PR). Any other recovered PR is recorded and publishing continues to the main path, which drafts all the task's PRs and refuses with what a person has to do. Before, a current recovered opening ended as "opened" and drafted only its own PR, leaving another of the task's PRs ready. The head settle is therefore always in the configured base again. - The misplaced refusal carries only the second draft-step run's notes: it looks every PR up again, so a failure it fixed is not reported. - Doc: step 1 rewritten against the code (the settled update, the review version in "own work", drafts unsupported, "created nothing" only for the same base, adoption left to the main path or draft step). - Tests: recovered misplaced PRs now end in the refusal, drafted. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Close independent review round 20 on F2d: no early ends in recovery - Recovery's drafts-unsupported branch no longer returns early: the PR is recorded as it is and publishing continues, so the main path returns `draft unsupported` in the configured base, or the misplaced refusal for a PR moved elsewhere (before, a moved lost draft opening was reported as `draft unsupported` with no word about its base). - Recovery's adoption of the task's other PRs is removed: since round 19 any such case continues to the main path, whose misplaced draft step adopts them. - Tests: a moved lost draft opening without drafts support is refused as misplaced; the misplaced refusal leaves out a draft failure that the second run fixed. Doc: step 1's drafts-unsupported sentence. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Close independent review round 21 on F2d: doc follows round 20 Round 21 found no code defects; three doc statements lagged behind round 20's removal of recovery's early return and adoption: - step 3: the main path's misplaced draft step adopts the task's other PRs, not recovery; - step 1: a task in review, approved or merged is refused on status, so a late PR of another abandoned opening is not adopted for it; - the drafts table: the draft change comes after a clear check, and a match or no changes report the ready PR as leftReady. A test title no longer names recovery as the one reporting it. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Close independent review round 22 on F2d: tests for untested guards Round 22 found no code defects. It found guards no test would miss: - runWithInput starts nothing for an already-aborted signal (a listener added to an aborted signal never fires); - every gh environment allowlist variable is passed on, and the proxy, CA, host and enterprise-token names are pinned; - an empty comparison page that should hold commits fails closed at once, with one compare call; - GH-N needs a boundary before it (XGH-12, foo-GH-12 are not mentions). Doc: titles and problems are cut by UTF-16 length, not by code point. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Close independent review round 23 on F2d: tests for the remaining guards Round 23 found no code defects. Tests now fail when these break: - the abort checks after the check (no result recorded), after the new branch's push (no opening left to settle) and after the needs-human draft change (no update recorded or pushed); - the PATCH answer's own check in refresh, before any ready change; - PR validation: an https URL, the base repository, and a marked PR without a readable base in findOwned; - neutralising lowercase gh-N and /pull/N, and ##N not being a mention. A stale comment on recordPullRequestDraft is removed. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Close independent review round 24 on F2d: control characters, blocking test Round 24 found no code defects. - Fenced plan text and problems replace control characters other than newline and tab with U+FFFD: a NUL from agent output could make GitHub refuse the description, and that refusal would repeat after every push. - Test: a lost opening is dropped at once when another of the task's PRs is open into the same base (only the other-base case was tested). - The PR operation deadline comment states the 66-second settle bound the doc gives. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Close independent review round 25 on F2d: non-JSON bodies are not read Round 25 found no code defects. Test: a gh failure whose stdout is not JSON (a proxy's HTML error page) is not matched for refusals, even when it contains the drafts-unsupported or HTTP 422 text. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Close independent review round 26 on F2d: list lag, CRLF markers, surrogates Round 26 found no high or medium defects. - A PR recorded as opened that GitHub's list does not show is read directly (isOpen) before the main path acts as if it were gone; if it is still open, publish stops with OpeningUnsettled and pushes nothing, so a lagging list never lets a push move an open PR's head with no update recorded as in flight. - Test: a marker in a description with CRLF line endings (GitHub returns these after an edit on github.com, seen live) is recognised. - Lone UTF-16 surrogates in the title and fenced text become U+FFFD. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Close independent review round 27 on F2d: tell list lag from a moved PR Round 27 found no high or medium defects. The direct read added in round 26 treated every open recorded PR missing from the branch's list as list lag ("try again later"). A PR whose branch a person renamed, or whose marker was removed and base changed, never comes back, so that retry looped forever. readPull now returns the PR's branch, base and marker: only a PR still on the task branch, into the configured base, with its marker is lag (OpeningUnsettled); any other open PR is refused as PullRequestMisplaced with what a person has to do. Tests: the moved-branch refusal; the adapter's readPull validation. Doc: the two outcomes and one request per recorded PR. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Close independent review round 28 on F2d: draft a moved PR before refusing Round 28 found no high or medium defects. - A PR the direct read finds moved (branch renamed, retargeted while the list lags) is made a draft where it is before the misplaced refusal, while its first line still identifies it; a PR whose marker was removed is reported as possibly still ready. Before, this refusal skipped the drafting every other misplaced refusal does. - Tests: retargeted and unmarked PRs during list lag; the base and marker conditions of the lag test now fail when removed. - PullRequestMisplaced's comment covers moved PRs and the drafting. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Close independent review round 29 on F2d: moved-PR draft record Round 29 found no high or medium defects; all items were in round 28's drafting of a moved PR: - The record write is outside the draft call's catch, so a Store failure after a draft that landed propagates instead of being told as "may still be ready". - The draft flag is recorded only when it differs, so retries of a refused publish do not move the task's version. - Tests: the fake draft change checks the head branch, a repeat does not move the version, an abort during the change rejects with the abort, and a retargeted PR is drafted in its new base. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Close independent review round 30 on F2d: own PR without its marker Round 30 found no high or medium defects. A recorded PR whose first-line marker a person removed was refused as "not opened by codeboost" when the list showed it, but as misplaced (with what to do) when the list lagged. The main path now passes its recorded PR numbers to findOpened, which refuses such a PR as PullRequestMisplaced: restore its first line or close it. Tests at the gateway and the publisher; doc step 3. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Close independent review round 31 on F2d: unmarked PRs, faithfully Round 31 found no high or medium defects. - The test harness let the draft step find and draft a PR whose marker a person removed, which the real gateway cannot: findOwned matches by first-line marker and markDraft's read-back needs it. The fake now does the same, and the test asserts what really happens: no draft, the record still ready. - The refusal for such a PR says it may still be ready for review, as the list-lag path already did. - findOpened's interface comment describes `numbers`. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Fix the unmarked-PR publisher test pushed failing in e22ec2a e22ec2a asserted the gateway's new message text through the test harness, whose fake lookup does not produce it, so the test failed. The publisher test now checks the error class and what the publisher does (no push, no draft, the record still ready); the message is tested at the gateway. The fake mirrors the message anyway. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Close independent review round 32 on F2d: a more faithful fake GitHub Round 32 found no code defects; it found places where the test harness was more forgiving than GitHub: - The fake refresh now fails, like the adapter's read-back, when GitHub does not apply the requested draft or ready change, and refuses a closed PR. The test that relied on a refresh returning a ready PR for a draft request now asserts what really happens: the publish fails, the update stays in flight, and the next publish settles it and makes the PR a draft. - A PR whose branch a person renamed is no longer listed under the task's branch by the fake lookups. Doc: the check counting the task's own abandoned PR when GitHub's list lags its timeline past the settle time (fails closed). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Close independent review round 33 on F2d: abort tests for two lookups Round 33 found no defects. Tests now fail if the abort checks after recovery's lookup or after the direct read of a PR the list does not show are removed: an abort there leaves the lost opening owned with the task's version unchanged, and draws no draft change or push. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Close independent review round 34 on F2d: three tests, one scope note Round 34 found no defects. Tests now fail if these break: the abort check after the draft change a match makes (no match recorded after a cancel), a non-array GraphQL `errors` field failing closed, and lowercase gh-N mentions in base commits. Doc: repository renames and transfers are out of this slice's scope (publish fails closed). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Address Copilot round 11 on F2d: full pages, malformed entries, approvals - The branch PR list fails closed on a full page of 100: a later page could hide the task's PR, and this lookup decides whether the branch is clear to push to. - Every list entry is validated (number, body null or string, base ref) before any is set aside as someone else's, so a partial answer cannot look like a clear branch. - The draft change for a moved PR found by the direct read, and recovery's draft change for a lost draft opening, re-read the task right before the call: a task approved meanwhile keeps its PR ready, since GitHub does not merge a draft. The draft step uses the same check (#mayKeepReady). - Tests for each; doc step 3. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Follow the code review of 7c5ca43 on F2d - The list entry check requires a PR number of at least 1, as #pull does, and a test covers a missing or zero number (the check was untested). - A merged task keeps its PR ready in the draft step; the test that covered in review and approved covers merged too. - The full-page error says to close the PRs that are not needed. - Doc: the re-read before draft changes applies to recovery, the draft step and moved PRs; the head settle is the stated exception (it drafts a PR whose head on GitHub is not the one reviewed). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Address Copilot round 12 on F2d: fail closed on ambiguous markers Recovery picked the first PR GitHub listed among those carrying a marker. A marker is editable text, so a copied description could make recovery record the wrong PR, or silently clear an unconfirmed update whose only matching PR had another number. Recovery now refuses with PullRequestMisplaced, keeping the update or opening in flight, when more than one open PR carries the update's or the lost opening's marker, or when the one carrying an update's marker is not the recorded PR. Tests for each; the earlier test that expected the mismatched number to be ignored now expects the refusal. Doc step 1. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Follow the code review of 683f538 on F2d: settle updates by number The review of 683f538 found that refusing a lost update when a copy carries its marker was a regression: the refusal ran before the draft step, so a stopped task's ready PR stayed ready for as long as anyone kept a copy open. An update's PR has a recorded number, which cannot be edited, so recovery now settles it from the PR with that number, whatever order GitHub lists them in, and leaves copies to the main path (two own PRs: drafted, refused) and the draft step. The Store's own number check already kept a copy from being recorded; this makes the publisher's choice explicit too. A lost opening has no recorded number, so two PRs carrying its marker still make recovery refuse; the message now says which to close and that the task's PR may still be ready. Tests: copies are into another base and listed first by the fake (one open PR per branch and base; GitHub's order is undefined); the combined test is split; a stopped task's PR is drafted with a copy present; the update's own PR's draft flag is recorded with a copy listed first; the older test models a closed PR replaced by a copy. Doc step 1. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Address Copilot round 13 on F2d: no issue tokens inside URL paths The GH-N, owner/repo#N and issue-URL forms in mentionsIssue could start inside a URL path or a longer path-like token, so a base commit with `https://example.com/GH-12` or `example.com/github.com/owner/repo/ issues/12` counted as mentioning issue 12 and moved the task to possibly already fixed. Like `#N` already did, the three forms now need a start that is not a word character, `/`, `.` or `-`. Tests: four embedded forms are not mentions; a bracketed URL and `(GH-12)` still are. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Follow the code review of 2aaa4fd on F2d - Test: `example.com.GH-12` is not a mention (the `.` in the GH-N rule was untested). - The boundary rule moves into mentionsIssue's JSDoc, which covers all four forms, with the rare real mentions it drops. - Doc: the check table lists path-embedded tokens as not a match. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
1 parent 872b0d9 commit 4baf097

14 files changed

Lines changed: 4099 additions & 3 deletions

‎AGENTS.md‎

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,8 @@ For features with background jobs, polling, retries, cancellation, or shutdown:
99
- Define the lifecycle states and ownership before implementation: pending, running, completed, failed, cancelled, stale, and closing.
1010
- Treat persisted state, in-memory jobs, subprocesses, HTTP requests, and rendered UI as separate state holders. Define how each transitions and settles.
1111
- Never apply a background response without proving it is still current. Use a generation, attempt ID, version, or guarded merge so older polling responses cannot overwrite newer actions.
12+
- When the local state has more than one version counter (for example a task's state version and its plan's review version), an in-flight action owns all of them, and applying its response requires every one to be unchanged. One counter does not cover writes that advance only another.
13+
- A current response may still move a record only along a transition allowed from the state the action was guarded for. Derive the new status from that state as well as the response; a response alone must never move a record out of a state that waits for a person.
1214
- Do not release a concurrency slot when cancellation is requested. Keep the job tracked until its underlying invocation or subprocess has terminated.
1315
- Do not let a retry replace a locally active job, even when its persisted lease has expired or wall-clock time changes.
1416
- Validate retry context against the current snapshot, plan revision, assignment, and referenced code. If any context is stale, disable retry and require a new request.
@@ -64,13 +66,21 @@ Every reproduced race requires a failing-before and passing-after regression. As
6466

6567
- A bounded safety scan must fail closed when its limit is exceeded. Never truncate evidence and report the result as clear.
6668
- Align subprocess output limits with every payload the schema accepts, or tighten the upstream page and field bounds; valid bounded input must not fail only because the transport budget is smaller.
69+
- Pass text whose size follows user or agent input to a subprocess on stdin, never as an argument: the OS limits one argument's size and cannot pass a NUL, so valid bounded input can fail to spawn.
6770
- Exclude the subject of a duplicate or supersession check by stable identity only. A shared branch name or other mutable attribute does not prove two records are the same subject.
71+
- Exclude the subject's own records only in the states the exclusion is for. A check that skips its own open PR must still count its own merged PR, or its own close of the issue, as done.
6872
- Preserve repository identity with pull request numbers in cross-reference scans. Never resolve or exclude a repository-qualified reference by number alone.
73+
- When a relation can be added and removed (a manually linked PR, a label, an assignment), replay its add and remove events in order and count only its latest state. An add event alone does not prove the relation still holds.
6974
- After the final asynchronous external validation, re-read the local generation immediately before an irreversible action. A generation check performed before that await is insufficient.
75+
- Check an operation's source-state preconditions before any shortcut or early return that writes state or reports success, not only on the main path.
76+
- Honour a cancellation signal that is already aborted before the first durable write, not only after awaits: a path with no await otherwise writes after the caller cancelled.
7077
- Batch and briefly cache read-only status probes, and give the combined operation an overall deadline below the serving request timeout.
78+
- Count the subprocess shutdown grace periods (SIGTERM-to-SIGKILL wait, pipe drain) inside that overall deadline: the operation ends when its processes have settled, not when the abort fires.
79+
- After an external state change (ready, draft, close), read the record back and require the new state before recording success; a command that exits 0 does not prove the change applied.
7180
- Budget a multi-stage validation across all sequential stages; giving each stage the full request allowance does not create an overall deadline.
7281
- Preserve the distinction between an explicit unbound identity and missing or malformed authorization metadata. Missing or malformed identities must fail closed.
7382
- Validate every field used to classify an external record as clear, including enum values and required nullable fields. Partial records and malformed policy objects must fail closed.
83+
- Select every member the API documents for a union or enum you classify (for example every GraphQL `Closer` type). Fail closed only on types outside that list; a missing known member turns every result into a false unknown.
7484
- Validate coupled lifecycle fields as allowed combinations. A terminal-looking conclusion must not override an active or unknown status.
7585
- Treat a successful external command as the transition it actually performed. If it can enqueue or schedule work, model and verify that lifecycle before reporting the final action as complete.
7686
- For safety-critical API responses, require and validate every requested field before any early return, including terminal-success paths. Treat an omitted field differently from an explicit `null` allowed by the API contract.
@@ -86,21 +96,29 @@ Every reproduced race requires a failing-before and passing-after regression. As
8696
- When a durable external-action attempt is bound to an older snapshot, require approvals or evidence recorded against the replacement generation before another action, whether or not the prior outcome explicitly requested fresh review. A mismatch with the old context is not itself fresh review.
8797
- If the external lifecycle mechanism or mode changes between validation passes, abort before the irreversible command. Create durable lifecycle ownership from the final stable mode, never from an earlier observation.
8898
- When an irreversible command has an ambiguous timeout, cancellation, transport, or unknown outcome, retain durable in-flight ownership and reconcile external state before enabling retry. Only a confirmed refusal may become retryable failure.
99+
- Record in-flight ownership before the first external write of an operation, not before a later step: when a push moves an open PR's head, the refresh it belongs to is already in flight.
100+
- Order an operation's external writes so that any step the remote can definitely refuse comes before the writes that cannot be taken back. A refusal after a push or a description change leaves a half-updated record.
89101
- Correlate retry observations to the current attempt with an immutable external identity or event boundary, and fail closed when multiple post-boundary action sequences appear. Matching only the resource or commit identity can replay another attempt's terminal event.
102+
- When recovery looks for the result of an earlier attempt, recognise the identities of all earlier attempts, abandoned ones included. A result that appears after its attempt was abandoned must be adopted, not treated as foreign, or recovery can never settle.
103+
- Read an identity marker only from its reserved position (for example the first line of a description). Text around it may be agent-controlled, so a marker-shaped string elsewhere must never identify or disqualify a record.
104+
- When the external record is observed, correct the local copy of any field the operation owns (such as a draft flag) from it. A change that landed but whose confirmation was lost is otherwise never repaired, because the next run sees nothing left to change.
90105
- Make an idempotency key required at the API boundary for every replayable action, and look up its saved outcome before any other guard, including in-flight, validation and coordinator shutdown guards. The one exception is the server's HTTP 503 during shutdown, which applies nothing; the client must keep the key and resend it. Save every definite outcome under the key (refusals before admission too), keep the saved response current with the durable outcome it reports, and replay failures as failures with complete result fields. Only a passing, nothing-applied outcome (shutdown, abort, deadline, storage error) stays resendable.
91106
- When a refusal must also change durable state (for example, handing an expired task to a person), commit that change outside the refused transaction, together with the saved refusal. Never write it inside the transaction the refusal rolls back.
92107

93108
## Owned host and Docker resources
94109

95110
- Treat the cleanup handle of an external resource (container, volume, network, temporary directory) as owned state. If removal fails, keep the handle, record it durably before its in-memory owner can be dropped (shutdown, crash, abandon, restart), and fail closed until removal is confirmed. Never delete the durable evidence before the final release report has been saved.
96111
- Give every subprocess an explicit allowlisted environment. Pass credentials only to the component that needs them, through a separate channel. Name-based scrubbing of an inherited environment is not isolation. Run Git with the repository's hardened invocation: no user or system config, no hooks, no lazy fetch, no network protocols.
112+
- Build that allowlist from each tool's documented credential and configuration channels on every supported platform (for example, the D-Bus session bus that a Linux keyring uses), and test that each is passed.
97113
- Treat paths read from a durable record or discovered on disk as untrusted. Before deleting, opening or probing one, validate its exact location and name, not only its basename, and never follow a link to it. Keep files that other local users must not plant or swap, such as lock files, in a directory only the current user can write. Write durable records through a unique temporary file opened exclusively, and delete it if the write fails.
98114
- Exclude other processes with an OS-level lock held for the owner's lifetime, keyed by the resource's stable identity rather than a path spelling. A PID liveness check never authorizes taking over a lock. Run shared one-time startup work single-flight under that lock, and keep the lock until the work has finished.
99115

100116
## Agent-controlled content
101117

102118
- When a subprocess reports a problem only as a warning and carries on, decide pass or fail by what each message means for the result, not by whether anything was printed: fail on messages that mean it did less than it should (for example could not read a path), and let through messages about harmless input the agent controls. Test both a benign case and a failing case, and filter the output as it arrives so that no volume of benign messages can push a failure out of a bounded buffer.
103119
- Quote or escape agent-controlled text (file names, paths, branch names) wherever it lands in output that people or tools parse, such as diffs, notices, logs or reports, so it cannot forge that output's structure.
120+
- Neutralise issue references (`#N`, `GH-N`, `owner/repo#N`, issue URLs) in any text codeboost writes that can become a commit message, such as a PR title or description, and neutralise @-mentions in any of that text that is not fenced (a title). Code fences do not protect commit messages, and GitHub closes issues from closing keywords in default-branch commits.
121+
- Never spread a collection whose size follows unbounded input into function arguments (`Math.max(...runs)`); engines limit the argument count, so use a loop.
104122
- A hardened Git invocation must also keep Git out of nested repositories and populated submodules, whose own config and hooks are the agent's: pass `--ignore-submodules` on the command line (the config default does not bind plumbing or override `.gitmodules`), and never run Git with a nested repository as its working directory.
105123
- Decide a filesystem fact (whether a name exists, where a link resolves, what a path's entries are) on the file system that will serve it, not on a copy. A host that folds case or Unicode normalization, or decodes names as text, answers differently from the container's; check links and names inside the container, as its kernel resolves them, by raw bytes.
106124
- Never infer that agent-written content is unchanged from timestamps, sizes or an index's stat cache: not every write moves them (tmpfs, for one). Compare the content itself.

‎core/pull-request-body.ts‎

Lines changed: 85 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,85 @@
1+
import type { Plan } from './plan.ts';
2+
import { cutText as cut } from './text.ts';
3+
4+
/** GitHub refuses a PR description longer than 65,536 characters; stay below it with room for the frame. */
5+
export const MAX_BODY = 60_000;
6+
export const MAX_TITLE = 200;
7+
const MAX_PROBLEMS = 20, MAX_PROBLEM = 2000;
8+
9+
/**
10+
* Plan text and open problems go inside fenced code blocks. GitHub does not act on closing keywords ("Fixes #12") or
11+
* @-mentions inside code, so text from the plan or from agent output cannot close other issues or notify people.
12+
* The fence is longer than any backtick run in the text, so the text cannot end the block. Control characters other
13+
* than newline and tab (a NUL from agent output, say), and lone UTF-16 surrogates, become U+FFFD: GitHub may refuse
14+
* them, and a refusal would repeat on every publish.
15+
*/
16+
export function fenced(text: string): string {
17+
// A loop, not Math.max(...runs): plan text is not length-bounded, and spreading every run can overflow the stack.
18+
let longest = 0;
19+
for (const match of text.matchAll(/`+/g)) longest = Math.max(longest, match[0].length);
20+
const fence = '`'.repeat(Math.max(3, longest + 1));
21+
return `${fence}text\n${text.toWellFormed().replace(/\r\n?/g, '\n').replace(/[\u0000-\u0008\u000b-\u001f\u007f]/g, '\ufffd')}\n${fence}`;
22+
}
23+
24+
/**
25+
* GitHub also reads closing keywords ("Fixes #7") in commit messages on the default branch, fenced or not, and a squash
26+
* or merge commit can carry the PR title and description. So every issue reference in plan text and problems is
27+
* neutralised: `#7` becomes `#7`, `GH-7` becomes `GH‑7` (non-breaking hyphen), and `/issues/7` or `/pull/7` in a URL gets a
28+
* division slash. The text stays readable, and no merge method can close another issue through it.
29+
*/
30+
export function neutralizeReferences(text: string): string {
31+
return text.replace(/#(?=\d)/g, '#').replace(/\b(GH)-(?=\d)/gi, '$1‑').replace(/\/(issues|pull)\/(?=\d)/gi, '/$1∕');
32+
}
33+
34+
function planText(plan: Plan, full: boolean): string {
35+
return plan.items.map(item => {
36+
const lines = [`${item.id}: ${item.title}`];
37+
if (full) {
38+
lines.push(` Intent: ${item.intent}`);
39+
for (const file of item.files) lines.push(` ${file.kind} ${file.renamed_from ? `${file.renamed_from} -> ` : ''}${file.path}: ${file.change}`);
40+
for (const check of item.acceptance) lines.push(` ${check.type}: ${check.text}`);
41+
if (item.depends_on.length) lines.push(` After: ${item.depends_on.join(', ')}`);
42+
}
43+
return lines.join('\n');
44+
}).join('\n\n');
45+
}
46+
const planTextSafe = (plan: Plan, full: boolean) => neutralizeReferences(planText(plan, full));
47+
48+
/** A title is not fenced, so an @-mention in it would notify: `@name` becomes `@name`. */
49+
export function neutralizeMentions(text: string): string {
50+
return text.replace(/@(?=[A-Za-z0-9])/g, '@');
51+
}
52+
53+
/** Single line, no control characters, no issue references except its own, no mentions, bounded in UTF-16 units. */
54+
export function pullRequestTitle(plan: Plan): string {
55+
const summary = neutralizeMentions(neutralizeReferences(plan.summary.toWellFormed().replace(/[\u0000-\u001f\u007f]+/g, ' ').replace(/\s+/g, ' ').trim())) || 'codeboost plan';
56+
const suffix = ` (#${plan.issue})`;
57+
return cut(summary, MAX_TITLE - suffix.length) + suffix;
58+
}
59+
60+
/**
61+
* The PR description: the marker that lets codeboost find the PR again, the issue link, and the plan. A needs-human
62+
* draft also lists its open problems. When the full plan is too long, only item IDs and titles are listed.
63+
*/
64+
export function pullRequestBody(input: { plan: Plan; marker: string; problems?: readonly string[] }): string {
65+
const { plan, marker } = input;
66+
// The full problems stay in codeboost; the description shows a bounded summary of them.
67+
const all = input.problems ?? [], shown = all.slice(0, MAX_PROBLEMS).map(problem => neutralizeReferences(cut(problem, MAX_PROBLEM)));
68+
const problems = all.length > shown.length ? [...shown, `(${all.length - shown.length} more in codeboost)`] : shown;
69+
const build = (full: boolean) => [
70+
marker,
71+
`Fixes #${plan.issue}`,
72+
'',
73+
`Opened by codeboost from plan revision r${plan.revision}. Review it one plan item at a time in codeboost.`,
74+
...(problems.length ? ['', '**Needs human.** These problems are still open:', '', fenced(problems.join('\n\n'))] : []),
75+
'',
76+
full ? '**Plan**' : '**Plan** (items only; the full plan is too long for this description)',
77+
'',
78+
fenced(planTextSafe(plan, full)),
79+
].join('\n');
80+
const body = build(true);
81+
if (body.length <= MAX_BODY) return body;
82+
const short = build(false);
83+
if (short.length <= MAX_BODY) return short;
84+
throw new Error('The plan and its open problems are too long for a pull request description.');
85+
}

‎core/text.ts‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
/** Cut to at most `max` UTF-16 units (the unit every length bound here counts), never inside a surrogate pair. */
2+
export function cutText(text: string, max: number): string {
3+
if (text.length <= max) return text;
4+
let end = max - 1;
5+
if (/[\ud800-\udbff]/.test(text[end - 1] ?? '')) end--;
6+
return `${text.slice(0, end)}…`;
7+
}

0 commit comments

Comments
 (0)