Uh oh!
There was an error while loading. Please reload this page.
fix: the six known bugs, and a typecheck that will notice the next one - #36
Merged
Conversation
**A review taken by an unrelated one.** With no branch to go on, every base ref shares a scope, so a second pull request opened in the same detached checkout looked like the same review continuing and took the first one's findings. Work may now only move between sessions when a branch connects them, or when the ref is identical. Redirecting a stale tab is left alone: showing somebody a newer session is recoverable, moving their findings is not. **A detached checkout called its branch HEAD.** `rev-parse --abbrev-ref` says `HEAD` when detached, which is not a branch name, and recorded as one it matched nothing — so a session opened before `gh pr checkout` put the worktree on a real branch was stranded with its findings. Unknown is what it is. **The wrong diff decided where a comment could go.** `gh pr diff --patch` is the commit series in mbox form, not the pull request's diff: one section per commit, hunks against that commit's parent, files under their historical paths. A file touched twice appears twice and only the last survives being collected. On our own #34 that left 25 of 89 commentable lines in one file, and the review API rejected the rest. The message blamed the reader's line arithmetic; it now names the diff. **Text typed into the page was rewritten.** Shell unescaping sat in the storage layer, which the browser routes share, so a comment about `split('\n')` reached the database with a real line break. It belongs at the CLI boundary, where a shell string actually arrives. **And the two type errors** nothing was running: a `string | undefined` passed as a ref, and a `loading` read off a suspense hook that has none. The reason both survived is that no typecheck ran. `packages/cli` had no typecheck script at all, and the UI's failed on four TS6059 errors from its own tsconfig, so its output had been noise for long enough to be ignored. Both are fixed, `npm test` runs them, and the count is zero. v0.9.11. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018PkYQzbsnMihHesafWvXKs
…annot A thread carries forward across a commit; its file path did not. So a commit that renamed a file left every open finding on it naming a path that no longer exists — and since a thread is rendered inside the block for its file, one naming a file that is not there is rendered by nothing. The finding did not look wrong. It was gone. Renames are followed from git's own detection, between the commit the finding was written against and the one it is carried to. Not the review's own diff, which shows the file only under the name it ends up with, so the rename is not in it to find. Content matching was the other option and this is better: a thread only moves to a file git already called the same file. That leaves deletions, and moves git does not report. Those now appear at the top of the diff instead of nowhere, through the same component that already handles a thread whose line has gone. v0.9.12. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018PkYQzbsnMihHesafWvXKs
Self-review. `donors` counts an unfinished review run, and `carryReviewRun` never finishes the donor's, so a donor keeps qualifying for as long as a review is open — and `/api/info` polls every five seconds. Behind that I had put a full `git diff` and a `parseDiff`. `--name-status --diff-filter=R` answers the same question in one line. Copies are out. git only reports them under `-C`, which is not asked for, so the branch was unreachable — and wrong if it ever ran, since a copy leaves the original where it is. v0.9.13. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018PkYQzbsnMihHesafWvXKs
fiddur
marked this pull request as ready for review
August 26, 2026 10:27
fiddur added a commit
that referenced
this pull request
Aug 27, 2026
Self-review. The idle watch asked `getCurrentSession()` whether anyone was still working — the ambient file shared by every worktree using one data directory. Both `agent.ts` and `server.ts` already carry comments saying not to trust it; this was the third reader and the only one that acts destructively on the answer. So a server could read another review's session, find no listener and no review in progress there, and exit while an agent was parked on its own session or a review of its own was half written. Both guards defeated at once, in exactly the setup that produced fifteen servers. Resolved from the server's own ref, once, when the watch starts. Per tick was the alternative and is the mistake made on #36: `findOrCreateSession` re-anchors and writes. The test timings move from environment variables to `startServer` options, so the shipped build has no undocumented knob and the seam is in the signature. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018PkYQzbsnMihHesafWvXKs
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 freeto join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
Everything on the known list, plus the reason two of them were invisible.
A review taken by an unrelated one
reviewScopemaps every base sha to one scope, because a branch's base moves as the branch is updated. With no branch that reasoning is gone — so a second PR opened in the same detached checkout looked like the same review continuing, andgatherOpenWorkmoved the first one's findings into it. Work may now only move when a branch connects two sessions, or when the ref is identical.Redirecting a stale tab is deliberately not guarded: pointing someone at a newer session shows them something they can check, and the legacy-migration tests depend on it. Only taking work is narrowed.
A detached checkout called its branch HEAD
rev-parse --abbrev-ref HEADreturns the literalHEAD, which matches nothing, stranding any session opened beforegh pr checkoutmoved the worktree onto a real branch. Recorded as unknown now, which is what it is.The wrong diff decided where a comment could go
gh pr diff --patchis the mbox commit series, not the PR diff. Measured on our own PRs:--patchgithub-resolution.test.tsgithub-resolution.tsagent.tsTwo of #31's "files" were historical paths from before a rename. And the error message sent you auditing your own line numbers; it names the diff now.
Text typed into the page was rewritten
Shell unescaping lived in
threads.ts/tours.ts, which the browser routes share, so a comment discussingsplit('\n')was stored with a real line break. Moved to the CLI boundary. The CLI still unescapes — verified end to end both ways.The two type errors, and why they survived
packages/clihad no typecheck script at all, and the UI's failed on four TS6059 errors from its own tsconfig (rootDir: srcagainst anincludethat reaches outside it), so its output had been noise long enough to ignore. Both fixed,npm testnow runstypecheckfirst, and the count is zero.Mutation-checked: reverting each session fix turns the new tests red (1 and 4 of 4 respectively), and putting the unescape back in storage turns 2 of 3 red.
parser 43, git 44, github 21, ui 390, cli 179, scripts 4. v0.9.11.
🤖 Generated with Claude Code
https://claude.ai/code/session_018PkYQzbsnMihHesafWvXKs