🟡 Medium pullRequest/usePullRequestFilesViewed.ts:69

Switching environments or pull requests while a write is pending can apply the previous PR's queued presses to the new environment and revert the new PR's optimistic state. referenceKey omits environmentId, so the reset does not clear environment-specific queued data, and the in-flight completion still mutates the shared overlay and inFlight.current after the reference changes. Include environmentId in the reset scope and ignore or cancel completions belonging to the previous environment/reference.

✅ Resolved in 2bc61d8a7abfa166a29b4eea0fcc89065ebb63a9

Viewed checkbox skips viewer rerender

Medium Severity

Ticking Viewed on a file that is already folded (the default) does not change collapse, so item.version stays the same. The diff viewer only redraws a file when version changes, so the header checkbox can stay out of date even though the toolbar count moved. The same stall happens when unticking a file that is already expanded.

^{Reviewed by Cursor Bugbot for commit 97f4901f65cba6895982edd88bea5fcd7e12930c. Configure here.}

Viewed overlay ignores environment scope

Medium Severity

Local viewed overlay, queue, and in-flight presses reset on projectId, repository, and number only. The RPC query already keys by environmentId, so switching connected environments while those fields stay the same keeps ticks from the previous server and can apply them to a different project that reused the same ids.

^{Triggered by learned rule: Multi-environment identity scoping: keys and lookups must carry environmentId}

^{Reviewed by Cursor Bugbot for commit 97f4901f65cba6895982edd88bea5fcd7e12930c. Configure here.}

🟠 High sourceControl/githubGraphQlBudget.ts:112

A failed mutation permanently consumes its estimated points, so a transient github.execute failure can leave subsequent reads rejected by the reserve check until the rate-limit window resets. The debit at line 112 is committed before execution and this budget has no rollback path; apply it only after successful execution or provide a rollback when execution fails.

No longer relevant as of 81df73df683623b51f586400d3fb73b5890de4e6

Failed batch undoes later in-flight ticks

Medium Severity

A failed setFilesViewed request drops matching inFlight entries before revert, and revertFileViewedOverlay only treats queued paths as superseded. A later flush for the same path is already in inFlight (the serial command has not run yet), so an earlier failure can clear the optimistic overlay and show an error toast while a newer press is still pending. If that later press sent the same viewed value, the checkbox jumps back to the host until the queued write finishes.

^{Reviewed by Cursor Bugbot for commit 2bc61d8a7abfa166a29b4eea0fcc89065ebb63a9. Configure here.}

An interrupt-only failure lands here as _tag === "Failure" too — the RPC client's scope closes on every reconnect/environment teardown and resumes pending requests with Exit.interrupt — so a flush in flight during a reconnect reverts the reader's ticks and raises a bare "Could not update viewed files" toast for something the server never rejected. The convention elsewhere (ServerUpdateAction.tsx:117-127, useThreadActionMenu.ts:180-188) is: create the command with { reportFailure: false } (otherwise useAtomCommand also console-warns alongside the toast), skip isAtomCommandInterrupted(result), and describe the rest with squashAtomCommandFailure(result).

Smallest fix — keep the revert, drop the toast for interrupts:

with import { isAtomCommandInterrupted } from "@t3tools/client-runtime/state/runtime"; and useAtomCommand(pullRequestEnvironment.setFilesViewed, { reportFailure: false }).

^{Posted via Macroscope — UI Consistency}

Real, and fixed in 3c279bf30.

isAtomCommandInterrupted(result) now gates the toast, and the command is created with { reportFailure: false } to match every sibling in this directory (PullRequestReactions.tsx:69, PullRequestReviewBar.tsx:71, and the three in PullRequestSummaryTab.tsx), which was an inconsistency worth closing on its own.

Two notes on the suggestion as written:

• The console-warn was not actually happening on this path. reportAtomCommandResult returns early when Cause.hasInterruptsOnly(result.cause), so an interrupt never warned regardless of reportFailure. reportFailure: false is here for consistency with the neighbours and to keep the warn from doubling up with the toast on a genuine rejection, not to silence interrupts.
• I left the toast title-only rather than adding squashAtomCommandFailure(result) as a description. Every error toast in this directory is title-only; failureToast with a description is the thread-menu and server-update convention, not this one.

The revert stays on interrupt, as you suggested: the write's fate is genuinely unknown there, so falling back to the last state the host confirmed is the honest thing to show, and the reconnect refreshes it anyway.

🟠 High pullRequest/azureDevOpsPullRequestJson.ts:386

Pull requests with more than 2,000 changed entries return only the first page, so later files are silently omitted from the diff and getFileRevisions/viewed-state reads. RawChangePageSchema retains only changeEntries, discarding Azure's nextSkip/nextTop continuation data, and the callers therefore treat that page as complete; preserve the paging fields and follow nextSkip until all pages are processed.

🟠 High pullRequest/AzureDevOpsPullRequestProvider.ts:31

Enabling diff causes Azure renames to be rendered with the wrong base-side path: when the response provides originalPath and omits sourceServerItem, the decoder sets oldPath === path, so the Code tab compares the new path at the merge base or labels the rename as newPath → newPath. Update the iteration-change decoder to use Azure's originalPath when it differs from the current path, before enabling this capability.

Fixed in cd7cccc. RawChangeEntrySchema now reads originalPath as well, and the decoder takes sourceServerItem ?? originalPath ?? path so both spellings of the same fact work, with the current path standing in only when neither is present. azureDevOpsPullRequestJson.test.ts covers the originalPath shape.

🟡 Medium pullRequest/azureDevOpsDiff.ts:114

The 512 KiB guard compares UTF-16 code-unit counts, so non-ASCII files can exceed MAX_FILE_BYTES on the wire while passing this check and being fully diffed, producing unexpectedly large responses. Measure both contents in UTF-8 bytes before deciding whether to mark the file truncated.

Fixed in cd7cccc. The 512 KiB guard now measures Buffer.byteLength(contents, "utf8") on both sides rather than String.length, and azureDevOpsDiff.test.ts covers a file of three-byte glyphs that a code-unit count would have let straight through.

🟡 Medium pullRequest/PullRequestService.ts:1511

Overlapping environment-backed writes can leave a file viewed after the user unticks it: a later viewed: false deletion may finish before an earlier viewed: true call completes fileRevisionsOf, allowing the stale insert at filesViewedStore.set to run afterward. Serialize updates per file scope or add a version/ordering guard so older writes cannot overwrite the latest action.

🟡 Medium

https://github.com/pingdotgg/t3code/blob/bbf04bb840919a8f9e485d16bf2b5ca73f876f20/apps/server/src/pullRequest/PullRequestService.ts#L735

When the CLI account changes from user A to user B within the ten-minute viewersByHost TTL, viewerOf returns A's cached login, so environment-backed filesViewed and setFilesViewed read and modify A's marks as B. Use a fresh viewer lookup for the persistence scope, or invalidate this cache when authentication changes.

🟠 High pullRequest/AzureDevOpsPullRequestProvider.ts:409

getDiffFileContents reads from the newest iteration, so after a paged diff for iteration N is followed by push N+1, expanding omitted context combines N+1 file contents with the displayed N hunks and shows incorrect lines and content. Carry the originating iteration (or its revisions) through the diff expansion request and read that iteration instead of unconditionally using scope.iterations.at(-1).

🟡 Medium pullRequest/BitbucketPullRequestProvider.ts:241

getFileRevisions can return the previous blob id for up to 60 seconds, so marking a file immediately after a push stores a stale revision and it is reported as dismissed when the cache refreshes. The fresh write path must bypass or invalidate the patch-derived cache before stamping the viewed file.

Azure binaries shown as text

Medium Severity

Item content is read only as content, and binary detection looks for a NUL in that string. Azure’s JSON items envelope marks binaries in contentMetadata and often base64-encodes the body, so images and other binaries render as a text hunk instead of “Binary files differ”.

^{Reviewed by Cursor Bugbot for commit bbf04bb840919a8f9e485d16bf2b5ca73f876f20. Configure here.}

Azure viewed marks collide

Medium Severity

Environment viewed rows for Azure are keyed by dev.azure.com plus the short repository name. Two orgs or projects with the same repo name and PR number share one SQLite row, so ticks from one change request appear on the other.

^{Reviewed by Cursor Bugbot for commit bbf04bb840919a8f9e485d16bf2b5ca73f876f20. Configure here.}

🟡 Medium pullRequest/azureDevOpsPullRequestJson.ts:542

decodeItemContentJson always reports isBinary: false in production because the items request enables includeContent=true but not includeContentMetadata=true; Azure therefore omits contentMetadata.isBinary. Binary blobs are passed to patch generation as text instead of being marked binary. Add includeContentMetadata=true to the request.

🟠 High pullRequest/AzureDevOpsPullRequestCli.ts:592

A non-advancing, non-null nextSkip returns truncated: false, so consumers treat the partial changes set as exhaustive and can mark unlisted paths as deleted. Return truncated: true when this guard stops pagination.

Sorry, I'm unable to act on this request because you do not have permissions within this repository.

🟠 High pullRequest/azureDevOpsDiff.ts:145

A single getDiff request can spend up to 2 seconds on every timed-out file, so many highly divergent files can monopolize the server for minutes or hours despite timeout: MAX_FILE_DIFF_MILLIS. structuredPatch returns undefined for each timeout and the loop continues after line 149; add a total deadline or file-count budget, or stop processing the slice after a timeout.

🟠 High pullRequest/AzureDevOpsPullRequestProvider.ts:400

Effect.orElseSucceed(() => null) converts rate-limit, authentication, and missing-tool failures from readItemContent into a successful partial diff. As a result, getDiff never propagates the provider error, so the host is not paused and users see only an unreadable-file header. Recover only errors that are safe to degrade per file; let systemic CLI failures reach the outer error handler.

Fixed in b060805da7. The per-file fallback now recovers only the failures that are one files own: a path azwill not carry, a blob it will not hand over, an answer that came back unreadable. A missingaz`, a signed-out CLI and a rate limit reach the outer handler as before, so the host is paused rather than the change rendering as files with no hunks. Two tests cover it, one either way.

Sorry, I'm unable to act on this request because you do not have permissions within this repository.

🟡 Medium pullRequest/GitLabPullRequestCli.ts:1113

When the GraphQL response has project: null or missing repository/blobs/nodes, decodeRepositoryBlobsJson returns an empty map, and fileRevisions converts every requested path to "". The service interprets "" as a deleted file, so an authorization or nullable-field failure incorrectly marks all viewed files as changed. Distinguish an unavailable blob query from a confirmed missing path, and only synthesize "" for the latter.

🟡 Medium

https://github.com/pingdotgg/t3code/blob/2db1bd7ff4539eacd7bdd5cf2be79d0722976b3c/apps/server/src/pullRequest/PullRequestService.ts#L2803

Successful head-changing mutations such as update-branch leave heldFileRevisions valid, so the following filesViewed read can report cleared files as viewed using revisions from before the head changed. invalidatedByMutation only bumps refEpochs, while file revisions use the independent heldFileRevisions cache; invalidate the affected file-revision scope for head-changing mutations (or every mutation).

Fixed in b938067c1. invalidatedByMutation now calls forgetFileRevisions alongside the epoch bumps.

heldFileRevisions is a plain map rather than an epoch-keyed cache, so it was the one piece of state a head-moving mutation left behind: the filesViewed read key does include refEpoch and so re-ran, but fileRevisionsOf answered it from the stale held entry for the rest of its 60s TTL.

Covered by forgets what the head had of a marked file once a mutation moves the head in PullRequestService.test.ts.

🟡 Medium persistence/PullRequestFilesViewed.ts:110

set can report failure after earlier files in the same input.files batch have already been committed, so a reload exposes only a partial set of viewed marks. Effect.forEach executes each INSERT/DELETE independently without a transaction; wrap the batch in a database transaction so all changes commit or roll back together.

🟡 Medium pullRequest/azureDevOpsDiff.ts:82

getDiff can return a slice far above the advertised MAX_DIFF_SLICE_BYTES limit for non-ASCII content because it increments bytes with file.section.length, which counts UTF-16 code units rather than UTF-8 bytes. Use Buffer.byteLength(file.section, "utf8") when updating the budget so multibyte text cannot bypass the response-size bound.

Revision cache drops known versions

Medium Severity

A later truncated getFileRevisions answer that omits a path deletes that path from the held map instead of leaving the last known head version. environmentFilesViewed then treats the miss as still viewed, so a file already reported as dismissed can lose its Changed badge.

^{Reviewed by Cursor Bugbot for commit 2db1bd7ff4539eacd7bdd5cf2be79d0722976b3c. Configure here.}

Viewed write lock can split

Medium Severity

inFilesViewedOrder yields inside Semaphore.make before inserting the gate, and it deletes the gate when pending hits zero. Two overlapping setFilesViewed calls can each create a semaphore, so a tick and a later untick are no longer ordered and the earlier write can win.

^{Reviewed by Cursor Bugbot for commit 2db1bd7ff4539eacd7bdd5cf2be79d0722976b3c. Configure here.}

🟡 Medium pullRequest/PullRequestService.ts:1537

filesViewed can report an older changed file as viewed indefinitely. recordFileRevisions refreshes the single scope-wide at timestamp for a fetch covering only newly marked paths while carrying forward revisions for older marks, so repeatedly marking other files keeps a within the freshness window without re-reading its revision; track freshness per path (or avoid refreshing at for carried revisions) so a is revalidated and becomes dismissed.

✅ Resolved in e8c53a02c04863c559e9d19e0b40682f30a8f1a7

🟡 Medium src/pullRequest.ts:922

PullRequestFileViewed changes valid file paths such as " leading.ts" to "leading.ts", so viewed-state responses no longer match the real diff path and can collide with another file. Use a non-empty string schema that preserves the path verbatim instead of TrimmedNonEmptyString.

✅ Resolved in e8c53a02c04863c559e9d19e0b40682f30a8f1a7