Repository navigation
[Epic] PR status at a glance - #3538
Merged
Merged
Conversation
![propr-dev[bot]](https://github.githubassets.com/images/gravatars/gravatar-user-420.png){kind=avatar}
…ypecheck is clean in every workspace and lint is clean on the changed files. One status computation (`packages/core/src/services/pullRequestStatus/`) - `computePullRequestStatus(facts)` is a pure function that returns `state` (`working`, `needs_attention`, `ready`, `none`) and `detail`: activity, Ultrafix cycle and maximum, review score, checks passed/failed/pending, the reason a PR needs you, and epic issues merged/total. - A loader gathers the facts: ProPR tasks and their latest states, queued jobs, Ultrafix loop and CI-wait state from Redis, blocking checks on the head (respecting `nonBlockingChecks`), mergeability, the latest review score, the instance rating goal and epic progress. - `ready` is never reported from the review score alone. Goal reached with checks pending is `working`; with a failed check it is `needs_attention`. Checks that can't be read never count as ready. - The PR body block and the three MCP tools both use this function. The `prStatusDisplay` setting (off by default, applies to every branch of a repository) - It is wired through the config type, API validation, MCP `update_repository_configuration` / `get_repository_configuration`, the settings catalog, the CLI (`--pr-status on|off` on `repo add` and `repo toggle`), a UI toggle under Automation, and the regenerated settings docs. The live block in the PR body - The block sits between `<!-- propr:pr-status -->` markers; only text between them is ever rewritten. New issue PRs are created with it already in the body (room is reserved in the body length budget). It is removed when the PR is merged or closed, or on the next change after the setting is turned off. - Updates are triggered by task state changes (follow-up, review, conflict resolution, implementation), Ultrafix state and CI-wait writes, recorded review scores, and PR and check-run webhooks. - Bursts of changes are merged into one update, and updates for the same PR are serialized with an in-process chain plus a Redis lock. - No PATCH is sent when only the "updated HH:MM" time would change. - A failed update is logged and never fails the task, review or loop that triggered it. - The other body writers (visual preview publication, upload-failure rewrite, goal PR completion, goal previews, epic body update) now all go through one helper that re-reads the current body and keeps the block. - ProPR's own body edits come back as `edited` webhooks; these are recognised and ignored before any handler runs. MCP: `get_pull_request`, `list_pull_requests` and `get_work_overview` now return `proprStatus`. The two list tools compute it for at most 20 open PRs per call to bound GitHub traffic: the rest read `null` and the response sets `statusTruncated`. Behaviour to know before merging - Status updates only run in processes that turn them on: the worker, the daemon's webhook handler, and the API when it receives webhooks directly. That keeps tests and library use free of side effects. - If the setting is turned off, the block is only removed from PRs where ProPR recorded writing it, via a Redis marker with a 180-day TTL. A block written before a Redis reset would stay until edited by hand. - A PR whose checks are pending reads `working` even when a person, not ProPR, made the push. I simplified "after ProPR's last push" to "pending on the current head". Tests added: - Every row of the state table, including the four required cases. - A full transition sequence: created → follow-up → review → score → Ultrafix cycle → CI wait → loop end. A human edit, preview section and continuation marker stay byte-for-byte unchanged throughout. - No PATCH when only the time changes; removal on merge/close and when the setting is turned off. - Visual preview publication keeps the block; the status-only `edited` webhook is ignored. - A new issue PR opens with the block when the setting is on. - API, MCP, CLI and UI tests, plus a Playwright e2e test for the toggle. Test environment: several suites failed only because `/tmp/git-processor` is read-only in this container; they pass with writable clone paths set. Docs: a new `docs/docs/features/pr-status.md` page, plus rows in `mcp.md` and `propr-cli.md`, and a CHANGELOG entry. I captured a preview of the new toggle in Repositories → Automation.
…st for each. I ran every new test against the original code: each one fails there and passes with the fix. The status suites, the other webhook/PR-creation suites I re-ran, typecheck and lint are all clean. I fixed all four findings (F1–F4) and added a regression test for each. I ran every new test against the original code: each one fails there and passes with the fix. The status suites, the other webhook/PR-creation suites I re-ran, typecheck and lint are all clean. **F1 – Too many checks could still report ready.** `loadCheckRuns` now also says whether it read every check run. If the three-page limit (300 runs) leaves some unread, the checks count as incomplete, so the PR shows "needs attention · checks could not be read" instead of `ready`. Legacy commit statuses didn't have this problem: when that response is cut short, the existing code already falls back to the overall commit state. The test puts a failing run on page 4. A second check confirms that exactly 300 runs still counts as complete. **F2 – Reviewing a human's PR made it look like ProPR's.** Having tasks on a PR no longer proves ProPR created it. Creation is now proven only by one of these: - an implementation task (type `issue`, `implementation` or empty) whose `pr_number` is that PR. - a plan issue, a goal, or an epic branch. Review, follow-up and conflict tasks still feed the activity shown in the status. They just don't mark the PR as ProPR's. The test runs review, follow-up and merge-conflict tasks on a human PR: it stays `none` and nothing is written to its body. **F3 – Commit status changes didn't refresh the body.** A `status` webhook now looks up the open PRs for that commit and refreshes their status. Repositories with the setting off cost no GitHub lookup. A burst of status events for the same commit triggers only one lookup, and the per-PR refreshes are still merged and serialized as before. The test goes from "Waiting for CI" to "Ready" through this path. **F4 – The block a PR was created with could survive turning the setting off.** ProPR now records the block in two places: - right after a new issue PR is created with the block. - whenever a sync finds the block already up to date and skips the PATCH. Turning the setting off then removes that block on the PR's next change. Tests cover both, plus the worker side: PR creation records the marker, and doesn't when the setting is off. I also added three lines to `docs/docs/features/pr-status.md`: someone else's PR stays `none`, the 300-check limit, and commit statuses as a trigger. There are no visual previews: the change only affects PR body text and server behaviour. **Separate issues I found but didn't fix (outside the four findings):** - A `check_run` event that lists no PRs (typical for fork PRs) triggers no refresh. ProPR's own PRs come from branches in the repository, so this shouldn't affect them. - A PR becoming unmergeable because its base branch moved sends no event for that PR, so the body can lag until the PR's next change. PR: #3535 Comment by: @propr-ultrafix (ID: 0) Model: Claude Opus 5.5
…etting: ProPR now writes it on every open pull request it created, in every repository. The setting is now `prTitleStatus`, on by default, and nothing reads it yet. The status computation and the MCP `proprStatus` fields are unchanged. The status line at the top of the PR body no longer has a setting: ProPR now writes it on every open pull request it created, in every repository. The setting is now `prTitleStatus`, on by default, and nothing reads it yet. The status computation and the MCP `proprStatus` fields are unchanged. **Body block, no gate** - The body sync and the block a new pull request is created with no longer read any repository setting. In a process that runs status updates (the worker, the daemon, and the API when it receives webhooks), every new issue PR opens with the block. Library use and tests still write nothing. - I removed the code that only existed for "turning the setting off removes the block": the 180-day Redis marker, `recordPullRequestStatusBlock` and its call when a PR is created, and the check that skipped GitHub reads for repositories with the setting off. - I removed the tests for that path too. In their place, tests check that the block is written with no setting involved and that a PR created with the block is confirmed without a PATCH, then kept up to date. **`prTitleStatus` setting** - It's on every surface repository settings have: - config type and a loader, `loadRepositoryPrTitleStatus`, ready for #3530 - API validation and preserve logic - the MCP get/update tools - the settings catalog (`repository.pr_title_status`) - the CLI flag `--pr-title-status <on|off>` on `repo add` and `repo toggle` - a UI toggle labelled "Show the PR state as a leading emoji in the pull request title" - the regenerated settings docs - It's repository-wide and only off when an entry of the repository sets it to `false`; clients that leave the field out keep the stored value. - The catalog description, the MCP field description, the CLI help, the UI text and the docs all say it controls the title only. **Docs:** the PR status page now says the body line has no setting and can't be turned off. It has a new "The title setting" section saying the setting changes nothing until the title emoji arrives. The rows in `mcp.md`, `propr-cli.md` and the CHANGELOG entry match this. **Tests:** the status, PR-creation, visual-preview, CLI, UI and API settings suites pass, as does the Playwright test for the toggle. Typecheck and lint are clean on the changed files. Three API tests fail, but they fail the same way on the unchanged branch head, so they aren't caused by this change: - `repository read failures retain tolerant defaults…`: "Missing expected rejection". - `settings catalog documentation paths exist in the bundled docs index`: the docs index isn't built in this container. - `observable MCP surface…`: also needs the docs index. I captured a new preview of the renamed toggle in Repositories → Automation, shown on by default. PR: #3535 Comment by: @integry (ID: 6852147972) Model: Claude Opus 5.5
…ach. I ran each new test against the original code: 8 fail there and pass now. The one exception is the block-size test, which also passes on the old code because the old block already stayed under the limit. Typecheck is clean in core, api, shared and the root, and lint is clean on the changed source files. The related suites pass: `prTemplateDescription`, `visualPreviewAttachments`, `checkRunHandler`, `mcpPullRequests`, `mcpWorkOverview`, and the two status suites (48 tests).
I've fixed F5, F6 and F7, and added a regression test for each. I ran each new test against the original code: 8 fail there and pass now. The one exception is the block-size test, which also passes on the old code because the old block already stayed under the limit. Typecheck is clean in core, api, shared and the root, and lint is clean on the changed source files. The related suites pass: `prTemplateDescription`, `visualPreviewAttachments`, `checkRunHandler`, `mcpPullRequests`, `mcpWorkOverview`, and the two status suites (48 tests).
**F5: facts that can't be read no longer produce "Ready"**
- In `pullRequestStatusFacts.ts`, failed reads of the review score, the Ultrafix state and the queued-job count now raise an error. Before, they were silently treated as "never reviewed", "no loop" and "0 queued". Epic progress still falls back to "none" (now with a warning), because it never decides the state.
- When a read fails, the body sync sends no update, so the PR keeps its previous status line. MCP returns `proprStatus: null`.
- Each of the three failures has a test that checks the loader fails and the body is left unchanged.
**F6: unknown mergeability is not "Ready"**
- `computePullRequestStatus` now returns `working` with a new activity, `checking_mergeability` ("🔨 Checking for conflicts"), when everything else would be ready but GitHub hasn't decided mergeability yet. Known problems such as a failed check or a low review score still show "Needs you".
- GitHub sends no event when it finishes working out mergeability, so ProPR checks again after about 3, 10, 30 and 60 seconds, then stops. A new change during that wait triggers an update within the usual 1-second delay instead of waiting for the next check.
- The test covers a PR that turns out to conflict, and checks that the re-checks stop after the schedule.
**F7: the body budget covers the block's maximum size**
- `PR_STATUS_BLOCK_MAX_LENGTH` (512 characters) is now in `@propr/shared`. The renderer keeps the block within it: it drops failed-check names first, then cuts the line, always keeping the markers and the update time.
- When a new PR is created, its description now leaves room for the largest block, not just the short initial one (`pullRequestStatusBlockReserve` in `src/github/pullRequestBodyWriter.ts`).
- If a body is close to GitHub's 65,536-character limit, the sync uses a shorter line with just the state and time. If even that doesn't fit, it leaves the line out, or removes an existing one rather than leave it out of date. Text outside the markers is never shortened.
- The same check applies when other ProPR features rewrite the body: if keeping the block would go over the limit, their text is written and the block is dropped.
I updated `docs/docs/features/pr-status.md` and the CHANGELOG entry. I didn't make a visual preview, because the change only affects text in the PR body on GitHub, not ProPR's UI.
**Separate problem I noticed but didn't fix:** the size budget only applies to descriptions built from a repository's PR template. The default description has no length limit: a very long agent summary produced a 130,647-character body in a test. That was already the case before this PR, so I left it alone.
PR: #3535
Comment by: @propr-ultrafix (ID: 0)
Model: Claude Opus 5.5
…ath. Each new test fails on the old code and passes now. Typecheck and lint are clean in core, and the related suites pass (130 tests across the status, sync, MCP pull request, work overview and visual preview attachment suites). I've fixed F8 and F9 and added a regression test for each path. Each new test fails on the old code and passes now. Typecheck and lint are clean in core, and the related suites pass (130 tests across the status, sync, MCP pull request, work overview and visual preview attachment suites). **F8: active work hidden by the 20-task limit** (`pullRequestStatusFacts.ts`) - `loadPullRequestStatusTasks` still reads the 20 newest task records, which give the latest outcome. It now also reads every task that is still active, with no limit, in a separate query. A task with no history counts as pending, as it did before. - The two results are de-duplicated and sorted newest first, so `tasks[0]` is still the newest record. - To keep the definition of "active" in one place, `pullRequestStatus.ts` now exports `TERMINAL_TASK_STATES` and the query uses it. - New test: one running follow-up task followed by 20 newer cancelled tasks. The status now reads "Follow-up in progress" instead of Ready. - I checked the other inputs to the status: queued jobs, Ultrafix state and checks have no history limit, so they don't have the same problem. **F9: body lock expiring during protected work** (new `pullRequestBodyLock.ts`) - The lock is renewed while the protected work runs. It counts as lost when a renewal finds another owner, or when the lease runs out because renewals keep failing. - The protected work now receives a lease and must call `lease.assertHeld()` right before it writes. That call checks ownership in Redis, extends the lease, and throws `PullRequestBodyLockLostError` if the lock was lost. - Both writers that change PR bodies, the status sync and `updatePullRequestBody`, now make that check before their PATCH. No other code PATCHes a PR body, so nothing else needed the change. - If a scheduled status sync loses the lock, it runs again: it re-reads the newer body and fits the status back in. - **Behaviour change:** other body writers, such as visual-preview publication, now get an error when they lose the lock, instead of silently writing over a newer body. - The lock code moved into its own module because the sync file was close to the 400-line limit. `withPullRequestBodyLock` is still exported from the package index. - **What is still unguarded:** the GitHub PATCH can't be protected by Redis. If renewals fail for a whole 30-second lease while that one request is in flight, two writers could still overlap. That is a race between two external services, so I bounded it rather than tried to make it impossible. **Tests for F9** (`packages/core/test/pullRequestStatusSync.test.ts`) - The fake Redis now models lease expiry, the renew script and failing renewals. The other process is a second, separate copy of the lock module, so it doesn't share the first one's in-process queue. - There are four tests: - **Renewal holds the lock:** a slow status read holds it, and the other process waits. - **Status writer taken over:** the lock is lost and taken by the other process. The status write throws, the newer preview body stays, and the retry keeps the preview. - **Renewals fail:** they fail for longer than the lease, and nothing is written. - **Other writer taken over:** the same takeover case for `updatePullRequestBody`. There's no visual change in this fix, so I didn't capture a preview. PR: #3535 Comment by: @propr-ultrafix (ID: 0) Model: Claude Opus 5.5
… described: all 5 tests passed, none failed, and the process hung at exit until the timeout ended it. Nothing changes in the earlier report. The background `mcpWorkOverview` run has finished the way I described: all 5 tests passed, none failed, and the process hung at exit until the timeout ended it. Nothing changes in the earlier report. PR: #3535 Comment by: @propr-ultrafix (ID: 0) Model: Claude Opus 5.5
…ed because the macOS runner couldn't reach nodejs.org. While rebuilding the `better-sqlite3` native module, node-gyp tried to download the Node headers and got `ConnectTimeoutError: Connect Timeout Error (attempted address: nodejs.org:443, timeout: 10000ms)`. This failure didn't come from this PR's code. `npm ci` failed because the macOS runner couldn't reach nodejs.org. While rebuilding the `better-sqlite3` native module, node-gyp tried to download the Node headers and got `ConnectTimeoutError: Connect Timeout Error (attempted address: nodejs.org:443, timeout: 10000ms)`. The step wraps `npm ci` in `apps/desktop/scripts/retry-transient-download.mjs`, which retries only on known network-error patterns. It didn't recognise this one: node-gyp prints Undici's error class and message but not the `UND_ERR_CONNECT_TIMEOUT` code that the script looks for. So it logged "Command failed without a transient download signature; not retrying." and failed on the first attempt. That script is the same on `main`. **Fix** - `apps/desktop/scripts/retry-transient-download.mjs`: added `ConnectTimeoutError` / `Connect Timeout Error` to the patterns that trigger a retry. - `apps/desktop/scripts/retry-transient-download.test.mjs`: added the exact line from the job log to the cases that must retry. I also added a test that runs `npm ci` with the full node-gyp failure output and checks that it is retried after the backoff and then succeeds. **Checks** - All 14 tests in the retry suite pass. - Run against the old script, the 2 new tests fail and the other 12 still pass. - Lint printed only an npm update notice and no lint output, so I can't say lint actually ran on these two files. Nothing visible changed, so there's no visual preview. PR: #3535 Comment by: @propr-dev[bot] (ID: 6856400934) Model: Claude Opus 5.5
… doesn't need restarting: it was waiting on an earlier test run that used the wrong test command. I re-ran those suites with the right command afterwards and they all passed, so nothing in my previous report changes. A background watcher hit its time limit and was stopped. It doesn't need restarting: it was waiting on an earlier test run that used the wrong test command. I re-ran those suites with the right command afterwards and they all passed, so nothing in my previous report changes. PR: #3535 Comment by: @propr-ultrafix (ID: 0) Model: Claude Opus 5.5
…reshes the open ProPR PRs into it, and the worker brings every existing open ProPR PR up to date when it starts. Nine new tests cover this. The six that check behaviour fail on the old code and pass now, and the related suites (154 tests) all pass. F14 and F15 are both fixed. A push to a base branch now refreshes the open ProPR PRs into it, and the worker brings every existing open ProPR PR up to date when it starts. Nine new tests cover this. The six that check behaviour fail on the old code and pass now, and the related suites (154 tests) all pass. **F14: base-branch pushes** - Push events to a branch now schedule a refresh. Tags, deleted branches and push-salvage rescue refs are skipped. - Several pushes to one branch within a second cost one listing of the open PRs targeting it. That listing reads at most 3 pages of 100 and logs a warning if there are more. - Only PRs ProPR created get refreshed, using the same ownership rule as the status itself. Other people's PRs on that branch are never read. - Updates are started 250 ms apart so a fan-out doesn't hit GitHub all at once. - **Stale read after the push:** GitHub may still report the old mergeability for a moment after the webhook arrives. A single read could therefore confirm `ready` and drop the refresh, which is exactly this finding. So the PR is read again after about 3, 10, 30 and 60 seconds until GitHub reports having compared it with the pushed tip (its `base.sha`). It stops early once that happens. The race with GitHub's background comparison can't be closed, but this way a read from before the comparison no longer ends the refresh. **F15: existing open PRs at startup** - When the worker starts, it lists the open PRs in each enabled monitored repository (at most 100 repositories, 3 pages each). It then schedules an update for each one ProPR created, 250 ms apart. - It runs once per process and only in the worker. The daemon and the API's webhook intake don't run it, so one deployment doesn't list everything three times. **Failures are retried** A failed listing, or a failed update started by a push or by the backfill, is retried on the same 3/10/30/60-second schedule, unless a newer push has already taken over. Before, a failure just waited for the next event, and an idle PR may never get one. **Tests** - Each fix has its own regression test. - I removed each refinement in turn (the stale-read re-check, the update retry, the listing retry). Each time only the test written for it failed. - Typecheck and lint are clean in core and the root. - The 154 tests come from the status, PR-template, visual-preview, merge-conflict and delivery-interest suites. - The docs page and the changelog entry now describe both triggers. **Behaviour to know** - **`base.sha` assumption:** the re-check assumes GitHub updates a PR's `base.sha` once it has compared the PR with the new tip. If it doesn't, each base push costs 4 extra reads per PR and then stops. It never causes extra writes. - **Polling intake:** GitHub sends no push events in polling mode, so after a base push the block only catches up on the PR's next change. - **Disabled repositories:** the backfill skips them. Webhooks from those repositories are filtered out, so a block written there wouldn't be kept current. **Found but not fixed (outside F14/F15)** - If the lookup of a commit's PRs fails after a commit status change, it isn't retried. - A failed update started by an ordinary change is dropped until the next change. That includes timing out while waiting for the body lock. - A PR closed while no ProPR process was running keeps its block, because the backfill only reads open PRs. There's no visual preview: nothing visible in the UI changed. PR: #3535 Comment by: @propr-ultrafix (ID: 0) Model: Claude Opus 5.5
…one of the shard's 254 test files failed: `test/taskIntentWithdrawal.test.ts`, which crashed on import with `SyntaxError: The requested module '../auth/githubAuth.js' does not provide an export named 'getGitHubInstallationToken'`. I fixed the Shard 1/4 failure, and this PR caused it. Only one of the shard's 254 test files failed: `test/taskIntentWithdrawal.test.ts`, which crashed on import with `SyntaxError: The requested module '../auth/githubAuth.js' does not provide an export named 'getGitHubInstallationToken'`. **Cause:** the last commit (the F14 base-push fix) made `pullRequestStatusWebhook.ts`, which `webhookHandler.ts` loads, import `isRescueRef` from `git/rescueRefs.ts`. That file also loads `repoBranching.ts`, which needs `getGitHubInstallationToken` from `githubAuth.js`. The test replaces `githubAuth.js` with a mock that only provides `getAuthenticatedOctokit`, so loading the webhook handler crashed. **Fix:** - `RESCUE_REF_PREFIX` and `isRescueRef` now live in a new file, `packages/core/src/git/rescueRefPrefix.ts`, which imports nothing. - `rescueRefs.ts` re-exports both from there, so its other importers (`index.ts`, `mergeConflictDetector.ts`, `worktreeCreation.ts`) don't change. - `pullRequestStatusWebhook.ts` now imports from the new file, so the webhook handler no longer loads the git and GitHub auth code. The behaviour of `isRescueRef` is unchanged. **Verification:** - `taskIntentWithdrawal.test.ts` now passes (169 tests). With the old import put back, it fails with the same `SyntaxError` CI hit. - `rescueRefHygiene` (12), `mergeConflictHelpers` (10) and `mergeConflictDetector` (37) pass. - The two PR status suites in core pass (78 tests). - Typecheck in core and lint on the three files are clean. The Node 20 deprecation line in the evidence is only a warning and wasn't the cause. There's no visual change, so I didn't create a preview. PR: #3535 Comment by: @propr-dev[bot] (ID: 6858782214) Model: Claude Opus 5.5
…ows as needs attention even while other checks are still running, and the startup backfill and base-push refresh now continue past their batch limits until every pull request has been read. The three status suites pass (93 tests), as do the two root suites that use the status block. I fixed all three findings. A finished check failure now shows as needs attention even while other checks are still running, and the startup backfill and base-push refresh now continue past their batch limits until every pull request has been read. The three status suites pass (93 tests), as do the two root suites that use the status block. **F16: pending checks no longer hide a confirmed failure** (`pullRequestStatus.ts`) - The order is now: 1. ProPR work still running (a task, an active Ultrafix loop, or a queued job) → `working`. 2. A confirmed problem → `needs_attention`. The problems are a failed last run, a failed conflict resolution, a stopped Ultrafix loop, a failed check, a merge conflict, or a review score below the goal. 3. Otherwise, checks still pending → `working`. - The finding's exact case (goal reached, score 8, one check failed and one pending) now returns `needs_attention` / `check_failed`. - **Behaviour change:** a review score below the goal (and the other confirmed problems) now also wins over pending checks. Before, they read `working` until CI finished. - "Checks could not be read" still comes after pending checks, because it isn't a confirmed problem. - New tests cover each confirmed problem with checks pending, show running ProPR work still winning, and show the "could not be read" boundary. The behaviour tests fail on the old code. **F17/F18: no pull request or repository is left out** (`pullRequestStatusReconcile.ts`, new `pullRequestStatusReconcileRuns.ts`) - Batches stay the same size: 3 pages of 100 pull requests, and 100 repositories per backfill batch. - A batch that hits the limit now records where it stopped: the last page it read and the highest PR number covered. The next batch starts after this one's updates have started (at least 2 seconds later). - **Backfill:** repositories past the first 100, and listings cut short, run in the next batch. They are deferred, not retried, so they run regardless of how many retries came before. - **Base push:** each branch is read batch by batch. If the branch is pushed again mid-way, ProPR finishes the remaining pages for the new tip, then reads the pages it had already covered again. If a new push restarted the read from page 1 instead, frequent pushes would mean later pages are never reached; a test fails if that happens. - PRs that close between batches move later ones to earlier pages. A resumed batch steps back to the page holding its last position, so none are skipped. - One related change: retries of a failed base-push batch now resume from where the batch stopped. - The old test that confirmed PR 320 was excluded now checks it is reached. Two new end-to-end tests (backfill, and a push to `main`) show PR 320 getting its status; both fail on the old code. - I moved the batching logic into the new file so `pullRequestStatusSync.ts` stays under the 400-line lint limit. `docs/docs/features/pr-status.md` now describes the new order of states and the batches that continue until everything is read. **Found along the way, not fixed:** - **Gap within one batch:** if a PR closes in the moment between two page reads of the same batch, one PR can slip past unread. GitHub's REST pull request list has no stable cursor, so preventing this would mean moving to the GraphQL API. - **Test typechecking:** `pullRequestStatusSync.test.ts` already has three type errors, at lines 20 and 427–428. They don't show up because core tests aren't typechecked. I fixed the typing in the helper I added. There's no visual preview, since nothing in the UI changed. PR: #3535 Comment by: @propr-ultrafix (ID: 0) Model: Claude Opus 5.5
The fix for F19 is in place and verified. **What was wrong.** The status loader in `packages/core/src/services/pullRequestStatus/pullRequestStatusFacts.ts` handed legacy commit statuses to the shared check summary and derived `passed` by subtraction. The shared summary only classifies legacy contexts individually when a non-blocking pattern matched. Otherwise it reads the combined state, so a head with one failed and one pending context came back as one failed and nothing pending, and the loader reported the unfinished context as passed. **The correction.** The loader now classifies legacy contexts itself whenever the status response listed every context. Each blocking context is counted as failed, pending, or passed on its own, and non-blocking contexts count as none of those. When contexts went unlisted, the loader still passes the combined state to the shared summary so its conservative reading is kept. Check-run handling is unchanged. The code lives in a new `classifyLegacyStatuses` helper so the loader stays under the lint complexity limit. **Sibling check.** The PR body block and all three MCP tools read checks through this one loader, so the fix covers every PR-changed consumer. The Ultrafix CI-wait path uses the shared helper directly, but that code and the helper predate this PR, so I left them alone. **Regression tests** were added to the sync suite, since the facts module opens a database connection that only that suite closes at teardown: - Failed plus pending legacy contexts report one failed, one pending, zero passed. - Listed legacy contexts and check runs are counted together, context by context. - A non-blocking legacy context neither fails nor holds the status and is not counted as passed. - Unlisted contexts fall back to the combined state. The two mixed-context tests fail against the previous loader and pass with the fix. | Check | Result | |---|---| | Sync suite, 56 tests | pass | | Negative run on old loader | 2 of 4 new tests fail, as expected | | Core typecheck | clean | | Core lint on the changed source file, zero warnings | clean | **Independent discovery, not changed.** The pre-existing shared helper has the same blind spot for its own callers: with a failed and a pending legacy context and no non-blocking patterns, the Ultrafix CI-wait description lists only the failed context and omits the pending one. That is outside this PR's changed behaviour. No visual preview was produced. The change alters the counts in a text status line, which is not a perceptible UI change to capture. PR: #3535 Comment by: @propr-ultrafix (ID: 0) Model: Claude Fable 5.1
…es pass (103 tests, up from 97), the two root suites that render the block pass (34 tests), core typechecks, and lint is clean on the changed source files. No visual change, so no preview was captured. Both findings are fixed and verified. The three status suites pass (103 tests, up from 97), the two root suites that render the block pass (34 tests), core typechecks, and lint is clean on the changed source files. No visual change, so no preview was captured. **F20: legacy contexts are paginated and counted per context** (`packages/core/src/services/pullRequestStatus/pullRequestStatusFacts.ts`) - Legacy commit statuses are now read page by page under the same three-page budget as check runs, through one shared pager. The 101-context head from the finding takes two requests and reports 99 passed, 1 failed, 1 pending. - Passed is no longer derived by subtracting from GitHub's total. Only a listed, blocking, successful context counts as passed. A context left unread by the budget is neither passed nor pending. - When contexts stay unread, every listed failed or pending context keeps its classification. A combined failure or pending state that no listed context accounts for is attributed to an unread context, so it still blocks readiness. - The check-run pager had the same assumption in one corner: a short page with runs still unlisted used to be marked complete. It now reads as incomplete, the same as for legacy contexts. **F21: rejected check reads keep their reconciliation retries** (`pullRequestStatusSync.ts`) - The checks fact carries a new optional `readFailed` flag, set only when a GitHub check request rejected. The page budget leaves it unset, so a commit with too many checks is never retried. - The sync outcome carries that flag, and a scheduled run treats it as unsettled when the run came from the startup backfill or a base push. The existing bounded refresh schedule then re-reads, and a complete read ends the retries. Ordinary transition updates still rely on the next transition, as before. **Tests** in `packages/core/test/pullRequestStatusSync.test.ts`. The fake GitHub now pages legacy statuses and can reject check reads. I replaced the test that accepted the subtraction arithmetic with four loader tests, including the exact scenario from the finding, and added three scheduling tests: backfill retry, base-push retry, and the page budget not retrying. Six of the seven fail on the previous code; the page-budget test is the control and passes on both. **Docs:** two sentences in `docs/docs/features/pr-status.md` now state the commit-status read limit and which failures are retried. **Independent discovery, not changed:** the pre-existing shared check reader in `packages/core/src/webhook/checkRunHelpers.ts` still reads a single page of legacy contexts. Its summarizer falls back to the combined state, so it does not miscount, but it carries the same single-page assumption. It is outside this PR's changed behaviour. PR: #3535 Comment by: @propr-ultrafix (ID: 0) Model: Claude Fable 5.1
…tus edits by the block it recorded writing, not by the shape of the edit, and a person's block-only edit gets the status reconciled instead of standing. F22 is fixed and verified. ProPR now recognises its own status edits by the block it recorded writing, not by the shape of the edit, and a person's block-only edit gets the status reconciled instead of standing. **What changed** - **Write provenance** in a new module, `pullRequestStatusProvenance.ts`. Right before each PATCH, under the body lock, the sync records the block it is about to leave in the body, or its removal. Records are kept in-process and shared through Redis with a 6-hour expiry, keeping the last 8 writes per PR so a burst of updates with several webhooks in flight still matches. Recording happens before the PATCH, so the webhook it causes can never arrive ahead of its record. A failed record is logged and only costs one extra status read. - **Webhook classification** replaces the old own-edit predicate. A block-only `edited` event is "own" only when the resulting block, or its absence, is a recorded write. Comparison is exact, update time included, with CRLF normalised for browser saves. Anything else is "foreign": the handler schedules a status update for that PR and returns an accepted, seat-free disposition with reason `pull_request_status_block_edited`, without running any other handler. Own edits are still ignored as before. - **Redis access for the webhook process** is registered with the signal module when sync is enabled, so the webhook module stays off the heavy sync import graph. A process that does not maintain the status has no record to consult and treats block-only edits as foreign, where the reconcile request is a no-op. - **Docs**: the pr-status page now says how own edits are recognised and what happens when a person deletes or rewrites the line. **Sibling check.** The other body writers keep the block, so their edits are never block-only and were never affected. The block a new PR is created with is written by an `opened` event, not an edit, and is not recorded, so a person deleting it is correctly reconciled. A removal ProPR makes on close or merge is recorded and recognised as its own. **Tests.** Five new tests cover: a cross-process own edit matched only through Redis, human deletion, human replacement of the block text, deletion of the creation block, and the removal case on a closed PR, plus the record-before-PATCH ordering and CRLF matching. All four human-edit tests fail on the old classification and pass now. Results: | Suites | Result | |---|---| | Core status suites + routing payload | 111 pass | | Root webhook suites (intent withdrawal, delivery interest, repository filter) | 213 pass | | Core typecheck, lint on changed files, core dist build | clean | No visual change, so no preview was captured. PR: #3535 Comment by: @propr-ultrafix (ID: 0) Model: Claude Fable 5.1
…k only when the block it leaves is the latest one ProPR meant to write. Matching any block it wrote at some point is no longer enough. I fixed F23. ProPR now ignores a block-only `edited` webhook only when the block it leaves is the latest one ProPR meant to write. Matching any block it wrote at some point is no longer enough. **The fix** (`packages/core/src/services/pullRequestStatus/pullRequestStatusProvenance.ts`) - The list of eight past writes is replaced by one record per pull request: the latest block ProPR is about to write (empty for a removal), with the time it was recorded. Each write replaces the previous record. The record is still made before the PATCH, kept in this process, and shared through Redis. - `isRecordedPullRequestStatusWrite` takes the newer of the in-process and Redis records and compares its block exactly with the delivered block. - **Close, reopen, then a person deletes the block:** the old removal record has been replaced, so the event counts as someone else's edit and the status is reconciled. - **A person restores an older block:** it doesn't match the latest record either, so the current status is written back. - In-process records now expire after the same six hours as the Redis ones. - **Trade-off:** past writes are no longer kept to recognise late deliveries. If ProPR writes twice in quick succession, the webhook from the first write now triggers a reconcile. That costs one status read and no PATCH, because the body already holds the latest block. Keeping past writes would bring the defect back. I checked the other places that use provenance. `classifyPullRequestStatusEdit` is the only reader and status sync is the only writer; the other body writers keep the block as it is, so their edits are never block-only. One race remains and can't be closed: a person editing the body between ProPR's record and its PATCH. GitHub has no conditional body update. **New tests** (`packages/core/test/pullRequestStatusSync.test.ts`), each checked with and without the Redis record: - Close, reopen, then a person deletes the restored block: the edit is treated as someone else's, the status update is scheduled, and the block comes back. - A person puts back an earlier block after the status changed: it is reconciled to the current "Ready". - A recorded write in this process stops counting after six hours. All three fail against the previous code. The three status suites pass (70, 33 and 8 tests), core typechecks, and lint is clean on the changed source file. The existing tests still pass, including those for ProPR's own write, its removal on a closed pull request, and browser line endings. There is no visual change, so I didn't capture a preview. PR: #3535 Comment by: @propr-ultrafix (ID: 0) Model: Claude Opus 5.5
…e-pr-status-and-20261113-1020-mgy [3529 by Claude Opus 5.5] Compute one PR status and show it live at the top of the PR body
Contributor
Author
… neither needs restarting. They never fired because the process check in each one matched its own command line, so it always looked like the tests were still running. They were only waiting on test runs I had already re-run and read directly, and those all passed. Nothing in my earlier report changes.
…nger treats that as a person removing the status emoji, so the emoji is written again on the next update. I fixed F1: when GitHub rejects a title update, ProPR no longer treats that as a person removing the status emoji, so the emoji is written again on the next update. **What was wrong:** ProPR saved the emoji it was about to write before sending the update, so the webhook GitHub sends back is recognised as ProPR's own edit however fast it arrives. If GitHub rejected the update, that saved emoji stayed. On the next update the title had no emoji but the saved one matched what was wanted, so ProPR read it as "a person removed it" and skipped the retry. That lasted until the status changed or the saved record expired after 30 days. **The fix:** - **`pullRequestStatusTitle.ts`:** - The saved emoji now has a pending flag. A new `recordPendingPullRequestTitleSlot` saves it as pending before the update. The existing `recordPullRequestTitleSlot` saves it as confirmed. - Only a confirmed emoji counts as one a person removed. A pending one is written again. - Webhook recognition still accepts pending records, so a webhook that arrives before GitHub answers is still treated as ProPR's own. - When the title already shows the wanted emoji and the saved one is still pending, it is now marked confirmed. - Records saved before this change have no flag and are treated as confirmed, which is what they meant before. - **`pullRequestStatusSync.ts`:** saves the emoji as pending before the update and confirms it only after GitHub accepts. If the update fails, the record stays pending and the next sync retries. If the update actually went through but its response was lost, the next sync sees the emoji in the title and confirms it. If a person removed it in that short gap, it comes back once. That's the trade-off I chose for "unconfirmed". **Same problem elsewhere:** I checked the status block in the description, which uses the same save-before-update pattern. It isn't affected: that saved record is only used to recognise webhooks, and the sync always compares the description itself with what it should be, so a rejected update is retried. **Tests:** in `packages/core/test/pullRequestStatusTitle.test.ts`, the fake GitHub can now reject an update, and there are two new tests: - A rejected `✅` followed by a successful sync wanting the same `✅`. It also checks that the webhook arriving mid-update is still treated as ProPR's own, that a fresh process reading only from Redis retries too, and that removing the emoji by hand is respected again once GitHub has accepted it. - A rejected `🔨` followed by a retry that adds it. Both new tests fail on the previous code and pass with the fix. The four status suites pass (125 tests), core typechecks, and lint passes on the changed source files. There's no visible change, so I made no preview. PR: #3539 Comment by: @propr-ultrafix (ID: 0) Model: Claude Opus 5.5
…r-state-as-one--20261113-2040-5cl [3530 by Claude Opus 5.5] Show the PR state as one leading emoji in the PR title
…es are merged into the epic branch, in the title and the status block. The progress comment now also lists what is in progress and what is next. All the tests I ran pass. I didn't run the full CI suite. What the epic shows - Title (with `prTitleStatus` on): the status emoji is followed by the count, e.g. `🔨 3/5 [Epic] Scheduler primitives`. The shared title helpers recognise and strip the count everywhere, including `[Epic]` detection and the web UI. With the setting off, epic titles are left alone and the status block is still written. - Status block: the first line reads e.g. `ProPR 🔨 3 of 5 issues merged · #3475 in progress (Ultrafix cycle 4 of 10, last review 6/10) · updated 09:41 UTC`. - A sub-PR that is `ready` but unmerged gives `⚠️ … · #3523 (issue #3475) waits to be merged into the epic branch`. - With every issue merged it reads `✅ 5 of 5 issues merged · checks: 12 passed`, or `⚠️ ` with the failed check or the conflict. - States: they follow your table. An epic shows no review score, and a low score never holds it back. When issues remain but nothing is running and none of your listed causes applies, the epic shows `⚠️ ` with "nothing in progress". I added that as a fallback. - New epic PRs: created with `🔨 0/N` and the status block already in the body. An epic PR created on the first sub-PR merge (the usual case) starts at `🔨 1/N` instead, since one issue is already merged. The epic body builder keeps the status block when it rewrites the body. - Counts: these come from the existing `computeEpicMergeProgress`. That includes excluding sub-PRs that already shipped through an earlier epic PR. I removed the separate count the status facts used to do. Renames - The title changes only when the count or the state changes. - While the queue is about to start the next issue, the epic stays `🔨`, so a merge costs one rename. The test for this confirms exactly one. Progress comment - Below "Merged child PRs" it now adds `In progress:` and `Up next:` lines, whatever `prTitleStatus` says. - "In progress" is left out when empty. "Up next" reads `none` once the last issue is in progress, and both lines go when there is neither. That is how I reconciled "omit empty lines" with your `Up next: none` example. - It is updated in the same pass as the status block, with no GitHub write when nothing changed. When the epic gets updated - A sub-PR into the epic branch opens, closes or is retargeted. - A sub-PR's own status block or title changes. - A plan issue changes status, such as starting. - A sub-PR merges. Limits to know about - Reading an epic's status now reads the status of up to 10 of its open sub-PRs. That is extra GitHub calls per epic update, and also for MCP status reads. - Issue titles in the comment come from the epic body's issue list, so there is no extra GitHub read per issue. - The status pass only updates an existing progress comment. Creating it is still the job of the first merge, so the two writers can't post it twice. - The merge path keeps the existing "In progress"/"Up next" lines and then triggers the status pass to refresh them. For a moment after a merge, the merged sub-PR can still be listed as in progress. Tests and checks - New suites cover: - the epic state table, block wording and title slot (`pullRequestStatusEpic.test.ts`). - a fake-GitHub sync run (`pullRequestStatusEpicSync.test.ts`): title, block, comment, no-op writes, one rename per merge, setting off, and the signals. - creation with `🔨 0/N` (`epicPRCreationStatus.test.ts`). - Existing suites gained cases for the slot with a count, `[Epic]` detection behind it, and the comment lines. - I updated two existing expectations to the new wording (`issues: 1 of 2 merged` became `1 of 2 issues merged`). - I broke the title change on purpose and confirmed 5 tests fail, then restored it. - Core and API typecheck clean. Core source lint has zero warnings; four files went over the 400-line limit, so I moved the new code into new modules rather than leave warnings. Docs are updated in `docs/docs/features/pr-status.md` (new "Epic pull requests" section) and `work-splitting.md`, plus a `CHANGELOG.md` entry. There is no UI change, so I made no visual preview.
…un the full CI suite. I fixed F1, F2 and F3. The tests I ran pass, but I didn't run the full CI suite. **F1: the status pass could overwrite a finalized progress comment** - **Shared lock:** before writing the comment, the status pass now takes the same per-epic lock the merge-progress writer uses. If the merge writer holds it, the status pass waits up to 5 seconds and then skips; the merge writer requests a status sync when it finishes, so the comment still gets refreshed. - **Check again after locking:** once it holds the lock, the status pass reads the epic PR and its comments again. It leaves the comment alone if the epic is no longer open or already merged, if the final-merge confirmation exists, or if the comment already says "This epic PR has been merged into …". - **Only the activity lines change:** the status pass now replaces just the "In progress" and "Up next" lines in the current comment. The counts and "Merged child PRs" stay as the merge writer wrote them, because it has evidence a status read lacks, such as a just-merged child that GitHub's list doesn't show yet. - **Trade-off:** while the status pass holds the lock, a merge update that arrives is pushed to its retry sweep. That is how it already handles a busy lock; it keeps its retry and runs about 60 seconds later. The status pass also stops updating the comment's counts; between merges they stay at what the last merge wrote, as before this PR. **F2: an issue stopped on a blocker showed as in progress** - The issue named in the queue's agent-blocked reason is no longer counted as running, even though its status stays `processing`. - If nothing else is running, the epic shows `⚠️ … · #802 stopped on a blocker`, under a new reason `epic_issue_blocked`. - Work that is really running, on a sub-PR or another issue, still makes the epic `🔨`. - The progress comment lists the issue as "stopped on a blocker". **F3: the 10-sub-PR read limit could change the reported state** - Sub-PRs past the 10-status limit are now marked as unread instead of looking like sub-PRs without a ProPR status. - If a sub-PR it read is working, the epic still shows working. - If nothing it read is working but some were not read, the result is a new reason `epic_children_unread`, and the status pass leaves the existing body line and title as they are. MCP reads return that reason with a count of the unread sub-PRs. - **Limitation:** for an epic with more than 10 open sub-PRs where none of the first 10 is working, the line and title stay frozen until the number of open sub-PRs drops to 10 or fewer. That is the "wait until enough statuses are read" option from the finding. **Tests** - New cases in `pullRequestStatusEpic.test.ts` and `pullRequestStatusEpicSync.test.ts` cover: - the epic merging while the status pass runs - an existing final-merge comment - newer counts from the merge writer being kept - waiting for the lock, and the merge writer deferring while the status pass holds it - the blocked queue head, with and without running sibling work - an 11th unread sub-PR - I switched off each fix in turn and confirmed its tests fail, then restored the code. - The sync test fixture now starts from a comment as the merge writer writes it, instead of the placeholder `old`. The merge helper in that test also updates the counts the way the merge writer does. - Core and API typecheck clean, core source lint has no warnings, and the related core and root epic suites pass (225 and 136 tests). I updated the epic section of `docs/docs/features/pr-status.md` for the new reasons and how the two writers share the comment. Nothing in the UI changed, so there is no visual preview. PR: #3540 Comment by: @propr-ultrafix (ID: 0) Model: Claude Opus 5.5
… up with the right state and merged count, without needing sub-PRs to close first. The tests below pass; I didn't run the full CI suite. I fixed F4: an epic with more than 10 open sub-PRs now ends up with the right state and merged count, without needing sub-PRs to close first. The tests below pass; I didn't run the full CI suite. **What was wrong:** every update read the same first 10 sub-PRs and threw the results away, so the ones past 10 were never read. The epic stayed "undetermined" forever, and that path wrote nothing, not even a changed merged count. **What changed** - **Statuses are kept between updates** (new `pullRequestStatusEpicChildReads.ts`). Each sub-PR status ProPR reads is kept, tied to that sub-PR's head commit and GitHub's last-updated time. - A kept status is used only while both still match, and for at most 10 minutes. - A sub-PR's own status update rewrites its body, which changes its updated time and also triggers an epic update, so an out-of-date kept status is never reused after that. - Each update reads first the sub-PRs with no current status, then those read longest ago, up to 10. Epics with 10 or fewer sub-PRs still have all of them read every time. - To support this, the sub-PR list now also records each open sub-PR's head commit and updated time (`epicMergeProgress.ts`). - **Follow-up updates are scheduled.** If sub-PRs are still unread and the state can't be determined, ProPR schedules another update 5 seconds later. That stops after 20 in a row; reaching it means sub-PRs keep changing faster than they're read, and each change triggers an epic update anyway. - **The count updates while the state is unknown.** The state already shown stays (no new state is claimed), but the count is updated: - in the status block's first line (`🔨 1 of 14 issues merged`), along with its "updated" time. - in the title slot, keeping its emoji and going through the usual title rules: the `prTitleStatus` setting, a slot someone removed is not put back, and the delay before showing⚠️ . The kept statuses live in memory, so after a restart the first updates read the sub-PRs again. One window remains by design: if a sub-PR's status changes but its own update hasn't written the change to GitHub yet, the epic can briefly use the older status. That sub-PR's update then triggers an epic update, which reads it again. **Tests** (`pullRequestStatusEpicSync.test.ts`) - The finding's scenario: 12 open sub-PRs, all work finished. The first update keeps `🔨` with the count updated (`1/14`). The next one reads the two left over and switches to⚠️ "#911 waits to be merged". After a sub-PR merges with more than 10 still open, the count moves to `2/14` and the state stays⚠️ . - A sub-PR that changes after its status was kept is read again (#930 starts working → `🔨 … #930`). - With only one signal sent, the scheduled follow-up alone settles the state. - I changed the existing unread-sub-PR test: it now expects the state to stay and the count to update (`1 of 13`), not "nothing written". - Turning off each part on its own (the kept-status check, the follow-up, using kept statuses) makes the matching new tests fail. - These suites all pass: `pullRequestStatus*`, `epic*` (core) and the root `epicMergeProgress*`. Core typecheck is clean and core `src` lint shows zero warnings. I updated the read-limit wording in `docs/docs/features/pr-status.md`. Nothing visible in the app changed, so there's no preview. PR: #3540 Comment by: @propr-ultrafix (ID: 0) Model: Claude Opus 5.5
…Facts.ts` and added regression tests. The full epic sync suite (22 tests) and the related epic suites pass. Core typecheck is clean and the file lints with zero warnings. I put the old source back and confirmed the three new tests fail on it. I didn't run the full CI suite. I fixed both findings, F5 and F6, in `pullRequestStatusEpicFacts.ts` and added regression tests. The full epic sync suite (22 tests) and the related epic suites pass. Core typecheck is clean and the file lints with zero warnings. I put the old source back and confirmed the three new tests fail on it. I didn't run the full CI suite. **F5: blocked issue still marked pending** - An issue named by the queue's blocker reason is now reported as blocked even if its saved status is still `pending`. Before, only issues that had started were checked. - So the status line reads `⚠️ … #802 stopped on a blocker` instead of "nothing in progress". The progress comment lists it as `In progress: #802 (…) - stopped on a blocker`, and it no longer appears under "Up next". - An issue whose status is `merged` or `closed` is not marked blocked, even if an old blocker reason still names it. - I checked the related code: the queue already stops treating a blocked queue as runnable, so nothing else in this path made the same mistake. **F6: lists cut off after ten pages** - The page reader used to stop after 10 pages (1,000 items) and return what it had as if that were the whole list. If the 10th page is full, it now reads an 11th. If that page has anything on it, the read fails. If it is empty, the list is complete, so a list of exactly 1,000 still works. It never stops at a partial list. - A failed read leaves the epic's existing status block, title and progress comment as they are. - This reader handles every epic list: pull requests into the epic branch, earlier epic PRs to the base branch, and the epic PR's comments. - The comment list had a real failure of the same kind. If the "epic merged" confirmation comment is past the 10th page, the old code missed it and rewrote a progress comment that should have been left alone. Now that update is skipped and a warning is logged; the next epic update tries again. - I also capped the page size at 100, since GitHub never returns more than that per page. **Behaviour to be aware of:** an epic with more than 1,000 pull requests into its branch, or more than 1,000 comments, now keeps its last published status indefinitely instead of showing a possibly wrong one. Each update logs the failed read. I chose this over reading without a limit, which the finding allowed, because it bounds the GitHub calls per update. **Tests**, all in `packages/core/test/pullRequestStatusEpicSync.test.ts`: - The fake GitHub there now returns real pages of `per_page` items instead of everything on page 1. - A pending queue head stopped on a blocker shows `⚠️ … stopped on a blocker` and isn't listed as up next. - With 1,001 pull requests into the epic branch, and the open sub-PR last, the update fails without writing anything. The published status, title and comment are unchanged. At exactly 1,000 the status is read normally. - With 1,000 comments before the "epic merged" confirmation, the progress comment is not rewritten. There's no visual change, so I made no preview. PR: #3540 Comment by: @propr-ultrafix (ID: 0) Model: Claude Opus 5.5
…w schedules a follow-up update to read them, even when it finds the epic working. The new regression test passes, and it fails when I put the old source back. I've fixed F7. An epic update that leaves sub-PRs unread now schedules a follow-up update to read them, even when it finds the epic working. The new regression test passes, and it fails when I put the old source back. **What changed** - **The bug:** only the undetermined-state path in `undeterminedEpicUpdate` scheduled the follow-up read. When a sub-PR among the first ten was working, a plan issue was running or the next issue was queued, the epic went down the ordinary path. Nothing else changes while it keeps working, so the sub-PRs past the ten-read limit never had their status shown in the progress comment. - **The fix:** a new helper, `withEpicChildReadContinuation` in `pullRequestStatusEpicSync.ts`, decides when the update runs again. `syncUnderBodyLock` (`pullRequestStatusSync.ts:305`) now uses it for every outcome. Whenever the epic facts ask for more reads, the update runs again after the follow-up delay, or sooner if the title is already due to update earlier. Closed pull requests (no status) schedule nothing. - **Cleanup:** I removed the old scheduling from `undeterminedEpicUpdate`, along with the input field it no longer needs. The undetermined path now gets the same scheduling as every other outcome. The existing 20-in-a-row cap on follow-up reads still applies. **Regression test** I added one test to `pullRequestStatusEpicSync.test.ts`, "a working epic that left sub-PRs unread schedules the update that reads them into the progress comment". It sets up 12 open sub-PRs: #911 is working and read first, and #930 is ready to merge but past the ten-read limit. After one request and no further signal, the follow-up update reads #930 and the progress comment shows `#930 (PR 930) - ready to merge`, while the epic stays `🔨`. On the old source this test fails. **Checks run** - These suites pass: `pullRequestStatusEpicSync` (23), `pullRequestStatusEpic` (20), `pullRequestStatusSync` (70), `pullRequestStatusTitle` (14), `pullRequestStatus` (33), `pullRequestStatusReconcileRuns` (8) and `epicPRCreationStatus` (2). That last one needs `--experimental-test-module-mocks`. - Core typecheck is clean, and lint on `src/services/pullRequestStatus/` reports zero warnings, including the 400-line and complexity limits. - I didn't run the full CI suite. - This change has no visible UI, so there is no preview. **Separate discovery, not changed:** MCP status reads also go through the same sub-PR reading code, so each one counts toward the 20-in-a-row cap on follow-up reads. Frequent MCP reads of a large epic could use up that budget early. Those reads also store what they read, so later updates still converge. I haven't checked whether this matters in practice. PR: #3540 Comment by: @propr-ultrafix (ID: 0) Model: Claude Opus 5.5
…vious code and pass now. The three epic status suites pass (49 tests), core typecheck is clean, and the two changed source files lint with zero warnings. I didn't run the full CI suite. I fixed F8 and F9. The new regression tests fail on the previous code and pass now. The three epic status suites pass (49 tests), core typecheck is clean, and the two changed source files lint with zero warnings. I didn't run the full CI suite. **F8: an epic whose queue isn't ready no longer shows queued work** - `nextIssueQueued` in `pullRequestStatusEpicFacts.ts` now also requires `queue.ready`, the same check `startEpicQueueHead` and `advanceEpicQueue` make. The check comes before the sequential/parallel split, so it covers: - a pending head. - a merged head with an issue still to start. - the parallel branch. - Such an epic now shows `⚠️ … nothing in progress` instead of `🔨 … #N starts next`. - I checked what happens when the queue becomes ready. Setup and recovery call `startEpicQueueHead` right away. When the head starts, its status change triggers an epic update, so the `⚠️ ` doesn't linger. - Two new tests in `pullRequestStatusEpicSync.test.ts` cover an unready queue with nothing running, one sequential and one parallel. Each checks the epic goes back to `🔨 … starts next` once the queue is ready. **F9: large stable epics are now read to the end** - The limit of 20 continuations used to count every pass. Now an update reads 10 sub-PRs and counts as progress when it reads one that hadn't been read since the epic was last fully read. The count is kept in `pullRequestStatusEpicChildReads.ts`. - Only updates that read nothing new count toward the limit, which is now `MAX_STALLED_EPIC_CHILD_READ_PASSES` (20 in a row). These happen when sub-PRs keep changing as fast as they are read. Any progress resets the count, and so does an update that leaves nothing unread. - Sub-PRs not yet read are always read first, so a stable epic of any size keeps scheduling updates until all are read. - New tests: - **211 sub-PRs, end to end:** the epic was published as working, and the scheduled updates alone must reach `⚠️ 1 of 213 issues merged · #911 … waits to be merged`. The old code stopped one sub-PR short. - **Unit tests (new file `pullRequestStatusEpicChildReads.test.ts`):** - 211 and 1000 stable sub-PRs are each read in `ceil(N/10)` updates. - Sub-PRs that change every time they are read stop the updates after 2 progressing updates plus 20 stalled ones. - A newly opened sub-PR resets the stall count. - **Mutation check:** I removed the limit on purpose, and two of the unit tests failed. I also added one clause each about the readiness check and the continuation rule to the epic section of `docs/docs/features/pr-status.md`. There's no visual change, so I made no preview. **Separate finding, not fixed:** with 10 reads per update, statuses kept for 10 minutes, and updates at least 5 seconds apart, roughly 1,000 open sub-PRs or more can't all be held at once. Statuses expire as fast as they're read, so the stall limit ends the updates and the epic keeps its published state. The list check allows up to 1,000 pull requests into an epic branch, so this sits at the extreme end of what's accepted. Fixing it would need a larger per-update read budget or a longer retention time, which is a design choice outside F9. PR: #3540 Comment by: @propr-ultrafix (ID: 0) Model: Claude Opus 5.5
…w updated even if nothing on the sub-PR can be rewritten (title setting off, description too full for the status block). The four new regression tests fail on the previous code and pass now. All 8 pull-request status suites pass (182 tests), core typecheck is clean, and the changed source files lint with zero warnings. I didn't run the full CI suite. I fixed F10. When a sub-PR's status changes, its epic is now updated even if nothing on the sub-PR can be rewritten (title setting off, description too full for the status block). The four new regression tests fail on the previous code and pass now. All 8 pull-request status suites pass (182 tests), core typecheck is clean, and the changed source files lint with zero warnings. I didn't run the full CI suite. **The fix** - Each sub-PR's status sync now records the sub-PR's computed status in Redis: the state, activity and the wording the epic shows. The code is in a new file, `pullRequestStatusEpicChildRecord.ts`. - `syncEpicSideOfStatus` signals the epic when that recorded status changes, as well as when the block or title changes. It goes through the same epic-branch filter and 1-second debounce as before. - The record only follows what the epic shows, so a sub-PR's check counts ticking over causes no extra epic updates. - If the record can't be written, the sync signals the epic anyway rather than risk missing a change. **The same bug in the kept sub-PR statuses** An epic with more than 10 open sub-PRs reuses statuses it read in earlier updates. It reused them as long as the sub-PR's GitHub update time hadn't changed, which assumed every status change rewrites the sub-PR. That's the same wrong assumption. So even after the signal, the epic could keep showing the old status for up to 10 minutes. Now a kept status is only reused while it still matches the sub-PR's record. If the record can't be read, the epic reads those sub-PRs again. **Tests added** - **The finding's scenario, end to end:** a sub-PR's Ultrafix loop finishes with the title setting off and a full description. Through the real signal and lookup, the epic goes from 🔨 to⚠️ naming the sub-PR waiting to be merged, with no write to the sub-PR. On the old code the epic stayed at 🔨. - **Signals:** a status that can't be written sends one signal when it changes and none when it stays the same. - **More than 10 sub-PRs:** the changed sub-PR is read again instead of its kept status being reused. - **Unit checks:** what the record tracks, how changes are detected, Redis failures, and when a kept status is reused. I also updated two sentences in `docs/docs/features/pr-status.md`. **Limits that remain** - **Lost signal:** the status is recorded before the in-process signal finishes looking up the epic PR. If the process stops in that second or so, or the lookup fails, the epic waits for the next change. This was already true when the evidence was the write to the sub-PR. - **No sync, no update:** a status change with no event on the sub-PR still isn't noticed. - **After deploy:** each sub-PR's first sync sends one extra epic update, because nothing is recorded for it yet. There's no visual change, so I made no preview. PR: #3540 Comment by: @propr-ultrafix (ID: 0) Model: Claude Opus 5.5
…red among the epic's reads, and a pass that waited for the progress lease drops its snapshot when a later pass already wrote newer activity. F11 is fixed. A status pass's activity snapshot is now ordered among the epic's reads, and a pass that waited for the progress lease drops its snapshot when a later pass already wrote newer activity. **How the correction works** - **Ordering under the body lock.** The new module `pullRequestStatusEpicActivityFence.ts` gives each epic update a sequence number from a per-epic Redis counter. The pass claims it right after reading its facts, while it still holds the body lock, and then confirms the lock is still held. Two passes never read facts concurrently under that lock, so the sequence order is the facts order. - **Fencing under the progress lease.** Before writing the comment, the pass checks a per-epic fence. If a higher sequence was already written or found current, the pass returns a superseded outcome and writes nothing. After its own write, it advances the fence. Both steps run under the lease the merge progress update shares, so they cannot interleave with another writer. - **A lock lost before the claim** leaves the ordering unknown. That pass writes no comment and signals a fresh epic update through the existing epic signal path, which rereads the facts. - **No Redis or a Redis failure** leaves the write unfenced, exactly as before this change, rather than suppressing it. A counter that expires takes its fence with it, so a stale fence cannot reject every later snapshot. - The ordering lives in `epicSyncNotes`, which moved into the epic sync module so the sync file stays under the 400-line limit. **Sibling check.** The merge progress writer keeps the existing activity lines and never writes a snapshot of its own, so it has no equivalent defect. The undetermined-epic update path builds its notes through the same function and is covered. **Verification** - Three new tests in the epic sync suite: pass A waits on the lease while B publishes "ready to merge", and A then leaves B's lines in place and writes nothing; the newest read wins in either arrival order and an unchanged write still fences; a lapsed body lock yields an unordered snapshot that takes no lease and signals the epic branch. - With the fence check disabled, the two ordering tests fail; with it restored, they pass. - The eight pull request status and epic suites pass, 321 tests in total. Core and API typecheck are clean, and the three changed source files lint with zero warnings. - One sentence was added to the "Epic pull requests" docs describing the ordering. There is no visual change, so no preview was produced. I did not run the full CI suite. PR: #3540 Comment by: @propr-ultrafix (ID: 0) Model: Claude Fable 5.1
…ss lease now schedules a fresh epic status update instead of trusting the lease holder to signal one. I fixed F12. A status pass that gives up on the epic progress lease now schedules a fresh epic status update instead of trusting the lease holder to signal one. **What changed** - In `pullRequestStatusEpicSync.ts`, the lease-busy branch of `syncEpicProgressCommentForStatus` no longer only logs. It now calls a new helper that requests an epic status sync for the epic branch, the same signal the `unordered` branch already uses. The helper first checks the activity fence. If a later pass already wrote newer activity, the comment is current and no update is scheduled, so a superseded pass does not cost an extra GitHub round. - The holder of the lease can be an older status writer, which writes its own older activity and signals nothing. The scheduled refresh reads the epic again and converges the comment to the latest activity. - I added a test hook that shortens the five-second lease wait, so the regression runs in under half a second. - Doc comments in both files no longer claim the lease holder is always the merge progress writer. **Sibling check** The lease helper `withEpicProgressLeaseIfFree` has one caller, the status pass. The merge progress writer keeps its own durable retry obligation in Redis when the lease is busy, so it is not affected. The `unordered` branch already re-signals. No other site carried the same assumption. **Tests** - New regression: older pass A holds the lease past newer pass B's wait. B writes nothing and signals the epic branch. A then publishes its older activity and releases the lease. The scheduled refresh publishes the ready activity, and no lease is left held. This test fails on the previous code with "B asked for a fresh epic update instead of leaving its activity to A". - New regression: a pass whose snapshot is already superseded gives up on a held lease and sends no signal. - Epic sync suite, the two other epic status suites, and both merge progress suites pass. | Suite | Tests | |---|---| | pullRequestStatusEpicSync, pullRequestStatusEpic, pullRequestStatusEpicChildReads | 82 pass | | epicMergeProgressRetry | included above | | epicMergeProgressHandler (needs `--experimental-test-module-mocks`) | 52 pass | Core typecheck is clean. Both changed source files lint with zero warnings. I didn't run the full CI suite. **Independent discovery, not fixed** The `catch` branch of the same function, which handles a thrown GitHub error during the comment write, also logs only, with the message that the next epic update writes it. That drops the pass's activity until another event, the same shape of silent loss as F12 but for GitHub failures rather than lease contention. Re-signalling there without backoff could loop on a persistent GitHub failure, so it needs a design decision and I left it alone. No preview was created. The change has no visible UI effect. PR: #3540 Comment by: @propr-ultrafix (ID: 0) Model: Claude Fable 5.1
Checks FailedValidation failed during setup, tests, CLI packaging, linting, or build checks. View Logs |
Contributor
Author
|
✅ Applied the requested follow-up changes in commit 2b6d61d Summary of ChangesI fixed the Validate Changes failure. It came from one lint warning: Cause: main had already brought that file to exactly 400 lines. This branch added one line on top, the Fix: I combined two separate Verified: There's no visual change, so I didn't make a preview. Review digestRisk-tiered paths
Validation evidence
Diff size: 1 file changed, +1/−2 lines 🤖 Implementation Details
Undo Changes • View Task Execution 💡 ProPR Slash Commands
🤖 Generated by ProPR |
… warning: `src/jobs/processMergeConflictJob.ts` was 401 lines, and the root lint allows at most 400 with no warnings.
I fixed the Validate Changes failure. It came from one lint warning: `src/jobs/processMergeConflictJob.ts` was 401 lines, and the root lint allows at most 400 with no warnings.
**Cause:** main had already brought that file to exactly 400 lines. This branch added one line on top, the `stripStatusSlot` import from `@propr/shared` that keeps the status slot out of the PR title used in merge-conflict tasks. That pushed it to 401. The other CI stages (UI, docs, core package, CLI, API, OpenAPI) passed.
**Fix:** I combined two separate `@propr/core` imports into one line (`import { logger, getAuthenticatedOctokit } from '@propr/core';`), which brings the file back to 400 lines. Nothing else changed.
**Verified:** `npm run lint` (`eslint src/`, the same check that failed in CI) now finishes with no warnings.
There's no visual change, so I didn't make a preview.
PR: #3538
Comment by: @propr-dev[bot] (ID: 6876560454)
Model: Claude Opus 5.5
Contributor
Author
|
ℹ️ Analyzed the follow-up request by @github-actions[bot] Analysis SummaryThis needed no new change. The only failure in the report was the 400-line limit warning on On the current branch, root I made no code changes and created no previews, because nothing visible changed. No code changes were necessary based on the current state of the branch. 🤖 Analysis Details
💡 ProPR Slash Commands
🤖 Generated by ProPR |
propr-dev
Bot
changed the title
propr-dev
Bot
changed the title
Contributor
Author
|
CI failed: Validate Changes Please investigate and fix this CI failure.
Failure evidence
|
Checks FailedValidation failed during setup, tests, CLI packaging, linting, or build checks. View Logs |
Contributor
Author
|
✅ Applied the requested follow-up changes in commit 85ea2b6 Processed 2 comments:
Summary of ChangesI fixed the Validate Changes failure: the edit-plan scope-estimate browser test was timing-dependent, not broken by this PR's code. Cause: this PR doesn't touch the planner. The test watches every request the page sends and expected none while it steps the Context Scope slider. But an existing draft does save its slider level one second after the last change ( Fix: only Verification:
There's no visual change, so I made no previews. Review digestRisk-tiered paths
Validation evidence
Diff size: 1 file changed, +17/−1 lines 🤖 Implementation Details
Undo Changes • View Task Execution 💡 ProPR Slash Commands
🤖 Generated by ProPR |
…stimate browser test was timing-dependent, not broken by this PR's code. I fixed the Validate Changes failure: the edit-plan scope-estimate browser test was timing-dependent, not broken by this PR's code. **Cause:** this PR doesn't touch the planner. The test watches every request the page sends and expected none while it steps the Context Scope slider. But an existing draft does save its slider level one second after the last change (`SETTINGS_SAVE_DEBOUNCE` in `setupWizardHooks.ts`). On a fast machine each slider step comes within that second, the save keeps getting pushed back, and nothing is sent before the test ends. Main now runs browser spec files on several workers at once (`PROPR_BROWSER_TEST_WORKERS` in `playwright.config.ts`), and CI tests the PR merged with main. That made the test slower, so the second ran out between steps and the saves at level 60 and 70 failed it. **Fix:** only `propr-ui/e2e/planner-scope-estimate.pw.ts` changed. The test records each level the slider steps to. Slider saves are now allowed, but only if the request body holds nothing but the draft settings, its level is one the slider actually stepped to, and every other setting is unchanged. Any other request still fails the test. The new-plan and enlarged-text tests are unchanged. **Verification:** - All 6 tests in the spec pass locally (Chromium). - I made temporary copies of the spec that wait 1.5s after each slider step, which recreates the slow CI run. The old check fails there with the same PUT requests seen in CI; the new check passes. The copies were deleted afterwards. - ESLint and the UI typecheck are clean. There's no visual change, so I made no previews. PR: #3538 Comment by: @propr-dev[bot] (ID: 6876976064) Comment by: @github-actions[bot] (ID: 6876976580) Model: Claude Opus 5.5
propr-dev
Bot
changed the title
Owner
|
/review |
propr-dev
Bot
changed the title
Contributor
Author
|
✅ AI Code Review Complete requested by @integry Posted 1 review: |
Contributor
Author
🔍 AI Code Review — codex:gpt-6-astraOverall EvaluationThis PR adds a shared status computation, live body updates, title slots, epic progress, and MCP exposure. It needs a localized correction before merge: the issue-PR retry creation path still opens a PR without the required status block. ✅ Shared computation — Body synchronization and the three MCP surfaces converge on ✅ Preservation and synchronization — The changed implementation preserves surrounding body text, recognizes its own webhook edits, and serializes status and epic-comment writers. ✅ Epic circle coverage — Shared and UI tests cover threshold boundaries, legacy hammer replacement, and slot removal. Acceptance criteria:
This is a static review of the supplied partial diff. Merge blockersEvery finding below was introduced by this PR and must be resolved before merging. F1: 🔴 AC3: Include the block in retry-created PRs
SuggestionsThese are optional follow-ups and are not sent to S3: 🟢 Remove stale release wording
S4: 🟢 Bound local provenance caches
ScoreThe implementation has substantial relevant coverage and passing current checks, but one explicit creation-path requirement remains unmet. The correction appears localized. Score: 6/10 🤖 Review Details
Files omitted from review diff1 file was omitted from the review diff. Large, binary, generated, and lockfile changes are deprioritized. Did not fit the review context budget
Copy and edit to choose records from this review: 🤖 Review by ProPR |
propr-dev
Bot
changed the title
Owner
|
/fix |
propr-dev
Bot
changed the title
Contributor
Author
|
✅ Applied the requested follow-up changes in commit ad2bd57
Summary of ChangesF1 is fixed: PRs created by the API retry now open with the status block already at the top of the body. Change ( Regression test (
Verification:
Other places that create PRs, checked for the same gap:
Separate finding, not changed: There's nothing visual to preview for this change. Review digestRisk-tiered paths
Validation evidence
Diff size: 2 files changed, +77/−4 lines 🤖 Implementation Details
Undo Changes • View Task Execution 💡 ProPR Slash Commands
🤖 Generated by ProPR |
… status block already at the top of the body. F1 is fixed: PRs created by the API retry now open with the status block already at the top of the body. **Change** (`src/jobs/issueJobPRRetry.ts`): `retryPRCreationViaAPI` now builds its body the same way normal issue PR creation does in `issueJobHelpers.ts`. It gets the block from `initialPullRequestStatusBlock(repository)` and puts it first with `withPullRequestStatusBlock`, so the POST body is `<status block>\n\nResolves #N\n\n_PR created via retry mechanism_`. If the process doesn't maintain the status, there is no block and the body stays exactly as before. The retry body is short and fixed, so it doesn't need the body-length reserve that the template path uses. **Regression test** (`test/issueJobPRRetryStatusBlock.test.ts`): - The retry POST body starts with the status block, and the title still gets the `🔨` slot. - With no block available, the body is unchanged. **Verification:** - The new test and the existing `issueJobRecoveredPRAutoAssignment` suite pass (8/8). - I ran the new test against the previous `issueJobPRRetry.ts` and it fails there. - `eslint --max-warnings 0` on the changed source and root `tsc --noEmit` are clean. Root lint doesn't cover `test/`, so the new test file wasn't linted. **Other places that create PRs**, checked for the same gap: - The epic PR path (`packages/core/src/services/epicPRService.ts`) and normal issue creation (`issueJobHelpers.ts`) already add the block. - The goal draft PR and continuation PR paths aren't issue PRs, so AC3 doesn't apply to them and I left them alone. **Separate finding, not changed:** `createPullRequest` / `createPullRequestRobust` in `src/github/prOperations.ts` (reached through `createPRWithRetry` and `src/githubService.ts`) also create issue PRs with the new title slot but no status block. Nothing in the repo imports `githubService.ts` or `prOperations.ts`, so no running path hits this. Fixing it would also mean reserving room for the block in their body budget. I didn't change them; say if you want that done as its own fix. There's nothing visual to preview for this change. PR: #3538 Comment by: @integry (ID: 6880400681) Model: Claude Opus 5.5
propr-dev
Bot
changed the title
Owner
|
/review |
propr-dev
Bot
changed the title
Contributor
Author
|
✅ AI Code Review Complete requested by @integry Posted 1 review: |
Contributor
Author
🔍 AI Code Review — codex:gpt-6-astraOverall EvaluationThe PR implements shared PR status computation, live body updates, title slots, and epic progress. The supplied code appears merge-ready within the reviewed scope; no verified blocker was found. Current-head checks report 38 passed, none failed or pending. This review is partial: ✅ Shared computation — Body synchronization and all three MCP surfaces route through ✅ Protected updates — Body writes use shared locking, re-read the PR after gathering facts, and preserve content outside the status block. ✅ Epic circle coverage — Shared tests cover threshold boundaries, legacy hammer replacement, unchanged attention slots, and ordinary PR behavior; UI tests cover stripping circles.
Merge blockersNo merge blockers. SuggestionsThese are optional follow-ups and are not sent to S5: 🟢 Remove stale release wording
S6: 🟢 Bound provenance memory
ScoreThe reviewed implementation has substantial targeted coverage and passing current-head checks. The omitted test file and truncated issue text limit the review’s completeness, but the supplied code establishes no merge blocker. Score: 8/10 🤖 Review Details
Files omitted from review diff1 file was omitted from the review diff. Large, binary, generated, and lockfile changes are deprioritized. Did not fit the review context budget
Copy and edit to choose records from this review: 🤖 Review by ProPR |
propr-dev
Bot
changed the title
propr-dev
Bot
changed the title
Contributor
Author
✅ Epic merged: PR status at a glanceEpic PR #3538 has been merged into main. 3 out of 3 PRs merged into main through this epic PR. All planned epic work is now merged. Posted automatically by ProPR |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
PR status at a glance
This Epic PR aggregates all changes for the plan: PR status at a glance
Issues in this Epic
Auto-close
When this PR is merged, the following issues will be automatically closed:
Fixes #3529
Fixes #3530
Fixes #3531
Created automatically by ProPR