Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
16 commits
Select commit Hold shift + click to select a range
ce44ba0
Keep merge stop reasons and GitHub answers through every layer (#48)
mchwang Oct 13, 2026
8e49642
Cover parallel inspection stops; handle the already-fixed check early…
mchwang Oct 14, 2026
e3e9ea1
Merge origin/main (#170) into feat/48-merge-lifecycle-concerns
mchwang Oct 14, 2026
a0b2f0d
Restate the merge click's settle bound after #170; lighter test inspe…
mchwang Oct 14, 2026
2790321
Align the drain comment and replay signal docs with #48 after #170
mchwang Oct 14, 2026
6d1ddde
Read the merge answer's queue snapshot after the review load
mchwang Oct 14, 2026
6a6da0f
Replay a click's saved outcome even after its own abort (#48)
mchwang Oct 14, 2026
200012c
Assert the timeout stopped gh before shutdown in the saved-meanwhile …
mchwang Oct 14, 2026
c3a5b3d
Keep a runner trust timeout through shutdown; cover two traced paths …
mchwang Oct 14, 2026
ebf520e
Test trust-read stop reasons in merge preparation and publishing (#48)
mchwang Oct 14, 2026
21578cc
Fix the #48 regression list punctuation and name what each test covers
mchwang Oct 14, 2026
ec135ba
Add two review lessons from #48 to AGENTS.md; link follow-up #174
mchwang Oct 14, 2026
126b07f
Merge remote-tracking branch 'origin/main' into feat/48-merge-lifecyc…
mchwang Oct 14, 2026
0056e48
Never offer an action from a merge click's pre-command readiness (#48)
mchwang Oct 14, 2026
e967af3
Word the merge answer's refresh blocker for every path
mchwang Oct 14, 2026
91da232
Label which half of the merge answer claim is tested
mchwang Oct 14, 2026
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions AGENTS.md
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ For features with background jobs, polling, retries, cancellation, or shutdown:
- Back off recurring external-status polling to a bounded cap. Reset the interval only after a meaningful lifecycle change or explicit user action.
- When synchronous work moves behind an await (a worker thread, a subprocess, an async API), list every await it adds and apply the local-generation rule under "Guarded external actions" to each one. A local await must not follow the final remote validation: the last await before an irreversible external action is that validation.
- Work that leaves a serializing path (the event loop, a lock, a single queue) needs its own concurrency bound. Queued work must honour cancellation and deadlines.
- Attach a rejection handler to a promise when it is created whenever anything is awaited before the promise is consumed. A rejection that settles in between reaches Node's process-wide unhandled-rejection handler, which by default exits the process.
- Never compare clock readings across threads or processes. Pass a relative budget and let each side count it on its own monotonic clock.
- A test that runs the server in the test's own process must wait for observable state before acting. Do not rely on the server blocking the test's event loop to order steps.

Expand Down Expand Up @@ -50,6 +51,8 @@ Before opening or updating a PR for asynchronous behavior, test every applicable

Every reproduced race requires a failing-before and passing-after regression. Assert both the visible result and the durable state when they can diverge.

A race regression must assert that the interleaving it names actually happened (for example which abort stopped which stage, or that a deadline fired before a release) before it uses the outcome as evidence. Otherwise machine load can turn it into a test of a different order that still passes.

## Review readiness

- Before requesting or re-requesting an automated Copilot review, self-review the full current diff, fix every issue found, and repeat the self-review and fix cycle until a complete pass finds no new issues. Re-run the relevant validation after fixes, then complete the mandatory independent-agent review loop below before requesting Copilot review.
Expand Down
4 changes: 2 additions & 2 deletions docs/implementation/guarded-merge.md
Original file line number Diff line number Diff line change
Expand Up @@ -29,9 +29,9 @@ Merging blocks when any plan item is unreviewed or stale, an attributed file is

The GitHub adapter reads the current PR base/head, effective branch rulesets, classic branch protection, required check contexts and app identities, and issue cross-references. It ignores review records that are not required checks. The already-fixed check is the pre-PR check, `GhAlreadyFixedGateway` in `github/already-fixed.ts`, with the same rules (see “The check” in `pull-request-opening.md`): a closed issue, another open or merged PR that would close the issue or is linked manually, or a new commit on the base branch that mentions the issue. A PR that only mentions the issue does not count, in this repository or another; when the PR's base is not the default branch, a PR in this repository into that same base that references the issue counts too (decided 2026-09-30). The merge passes this PR, and every other PR the task records in this repository (#121), as the task's own PRs, matched by repository and number: each is excluded while open and counts once merged, as in the pre-PR check. Without a runner block only this PR is passed. The commit scan starts at the PR's base commit on its base branch. Merging requires that commit to equal the reviewed snapshot's base, which is the base the pre-PR check scanned from. The merge passes no own commits, because the task's commits reach the base branch only by this merge. Any result the check cannot complete is unknown and blocks merging, including a base branch name outside the pattern the check accepts. The check starts as soon as the PR is read, alongside the rule reads, and stops 0.65 seconds before the inspection's deadline (`MERGE_CHECK_SETTLE_MS` in `github/merge.ts`: both grace periods of the merge's `gh` runner, which the check also uses, plus a margin), so its processes settle before the inspection ends; a check stopped that way is unknown. If the rule reads fail, the check is stopped and awaited before the inspection rejects. When the PR read ends too late to leave that much time, the check is not started and is unknown. A check answer that arrives after the check was stopped is unknown, and an inspection whose caller aborted or whose deadline passed rejects even when its reads answer late. An injected check must name the merge repository. The blocker names what the check found (at most five closes, PRs and commits, by repository, number, state and short SHA, with a closing commit's SHA shortened too; never a commit message) or why it could not finish. Once this PR has merged, the check counts it as the fix, so the coordinator shows only the `pr-state` blocker, not an already-fixed one. Malformed pagination, rules without a type, malformed protection objects, missing or malformed check-app identities, and inconsistent check status/conclusion pairs fail closed, while an explicit `null` remains an unbound check. Display status is cached for five seconds and the combined inspection has a 12-second deadline. Each of the two sequential merge-validation inspections has a six-second deadline, keeping their combined validation budget below the 15-second serving request budget. A deadline aborts the underlying GitHub CLI process before releasing the shared in-flight request. A caller's own abort during the already-fixed check waits for its processes like any other `gh` call of the gateway, up to 0.4 seconds. An unreadable or incomplete rule source, a pending/missing/failing check, an already-fixed match or unknown result, a conflict, or a closed PR blocks merging.

Automatic merging also requires strict required status checks as a server-enforced current-base policy. A client-side fetch cannot supply that guarantee. Merge-queue branches were blocked in this increment because `gh pr merge` can report a successful enqueue before the pull request is merged. Queue support has since landed (#24; see `merge-queue.md`): a queue-capable adapter tracks each attempt until GitHub confirms the merge, removal or failure, and queue rules count as the server-side current-base guard. An adapter without queue inspection still fails closed with a `merge-queue` blocker. The coordinator performs the complete gate twice without using the display cache, verifies the same base/head pair, then re-reads the local review generation immediately before invoking `gh pr merge --match-head-commit` with literal argv. Each `gh` process gets only the allowlisted variables in `github/gh-env.ts`, plus fixed settings that turn off prompts, the pager, colour and update checks. No other server variables are passed on. A timeout or abort sends the `gh` process SIGTERM, then SIGKILL after a quarter of a second, and the call returns only after that process has exited (or 0.15 s after it exits, if a process it started keeps its output open). So a merge click aborted at its 14-second deadline settles by 14.4 s, inside the 14.5-second shutdown drain and below the 15-second request budget. Stopping `gh` cannot recall a merge request it had already sent: GitHub can still apply it, so a cancelled merge is reported as unknown and its attempt stays in flight, with the action disabled. A direct attempt is settled only when GitHub shows the pull request merged; a queued attempt also settles when GitHub reports it removed or failed (`merge-queue.md`). The adapter invalidates its pre-action cache before the command and after either success or refusal. A generation guard prevents inspections started before or during the command from restoring stale cache entries afterward. After command success, the server returns “merge submitted” even if its best-effort status reload fails, and the browser keeps the action disabled until Refresh confirms GitHub state. GitHub's command refusal text is returned to the local UI, and a failed attempt also remains disabled until Refresh loads fresh state. There is no “merge anyway” path around missing atomic protection. Demo mode never constructs the coordinator, including when a gateway is injected.
Automatic merging also requires strict required status checks as a server-enforced current-base policy. A client-side fetch cannot supply that guarantee. Merge-queue branches were blocked in this increment because `gh pr merge` can report a successful enqueue before the pull request is merged. Queue support has since landed (#24; see `merge-queue.md`): a queue-capable adapter tracks each attempt until GitHub confirms the merge, removal or failure, and queue rules count as the server-side current-base guard. An adapter without queue inspection still fails closed with a `merge-queue` blocker. The coordinator performs the complete gate twice without using the display cache, verifies the same base/head pair, then re-reads the local review generation immediately before invoking `gh pr merge --match-head-commit` with literal argv. Each `gh` process gets only the allowlisted variables in `github/gh-env.ts`, plus fixed settings that turn off prompts, the pager, colour and update checks. No other server variables are passed on. A timeout or abort sends the `gh` process SIGTERM, then SIGKILL after a quarter of a second, and the call returns only after that process has exited (or 0.15 s after it exits, if a process it started keeps its output open). So a merge call aborted at its 14-second deadline settles by 14.4 s. *Later (#48, #170):* a click can also be waiting on its issue-trust read, whose stop wait is 0.75 s, or on a review load, whose history worker gets 1 s after the abort before it is terminated; so it settles about 0.75 s after its deadline, or about 1 s or a little more after it when a review load was running, past the 14.5-second shutdown drain (traced, not tested); see [merge-queue.md](merge-queue.md). Stopping `gh` cannot recall a merge request it had already sent: GitHub can still apply it, so a cancelled merge is reported as unknown and its attempt stays in flight, with the action disabled. A direct attempt is settled only when GitHub shows the pull request merged; a queued attempt also settles when GitHub reports it removed or failed (`merge-queue.md`). The adapter invalidates its pre-action cache before the command and after either success or refusal. A generation guard prevents inspections started before or during the command from restoring stale cache entries afterward. After command success, the server returns “merge submitted” even if its best-effort status reload fails, and the browser keeps the action disabled until Refresh confirms GitHub state. GitHub's command refusal text is returned to the local UI, and a failed attempt also remains disabled until Refresh loads fresh state. There is no “merge anyway” path around missing atomic protection. Demo mode never constructs the coordinator, including when a gateway is injected.

Shutdown sets a terminal admission flag before inspecting active work and checks it again after partial request bodies are received. It then stops HTTP admission, aborts and awaits an active merge CLI process, drains requests, and closes the review service. A request admitted before shutdown cannot start a new merge afterward, and a cancelled command cannot outlive the local state that authorized it.
Shutdown sets a terminal admission flag before inspecting active work and checks it again after partial request bodies are received. It then stops HTTP admission, drains admitted requests, closes the pre-merge lifecycle and the Store write gate, and only then aborts and awaits an active merge CLI process (`web/server.ts`), so a refusal gh gave before the click was admitted, if it is handled only after the write gate closed, cannot be saved and answers a resendable 503 with the refusal's text; after admission, settlement keeps its shutdown capability and saves GitHub's refusal (#48). A request admitted before shutdown cannot start a new merge afterward, and a cancelled command cannot outlive the local state that authorized it.

## Deferred runner work

Expand Down
Loading
Loading