Uh oh!
There was an error while loading. Please reload this page.
feat(github): say what the forge already knows about a thread - #34
Merged
Conversation
A finding that has been posted looked identical to one that had not, and a finding the author had already resolved looked identical to one nobody had read. Both facts existed — `submitted_at` was being written and never shown, and resolution was never fetched at all. The card now carries "Posted to GitHub 15:37", a time of day rather than "19h ago" because the question it answers is whether the author has seen it yet. Resolution comes from GraphQL, since REST does not carry it, and is matched on file and wording rather than on line: GitHub nulls a thread's line once it goes outdated, and outdated is the usual state of a resolved thread, so a line in the key would have made the sync do nothing on exactly the threads it exists for. Pulling now refreshes the page when only resolutions changed, which the old count-based branch did not. v0.9.8. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018PkYQzbsnMihHesafWvXKs
Resolving here does not resolve there, and nothing said so. The notice fires only for a finding that was posted — anything else has nowhere else to be resolved. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018PkYQzbsnMihHesafWvXKs
Self-review found the amend case, which a test of mine was concealing: it passed a thread holding both the old body and the new one, a shape `editComment` cannot produce because it overwrites in place. So the test passed while the behaviour it claimed to protect did not exist, and an amended finding would have stopped matching its thread on the forge entirely — which since the finding-only change is most of the ones carrying an answer. `submitted_body` records what went out, and the matcher compares against that. Threads sent before the column fall back to their current bodies. Two smaller ones. `pullThreadState` returned an empty array for both "nothing is resolved" and "the call failed", so a broken sync read as a clean one; it returns null for the second and the page says so. And dismissing a posted finding is as local as resolving one, so it carries the same notice. v0.9.9. 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 25, 2026 11:54
fiddur added a commit
that referenced
this pull request
Aug 26, 2026
#36) Everything on the known list, plus the reason two of them were invisible. ### A review taken by an unrelated one `reviewScope` maps 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, and `gatherOpenWork` moved 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 HEAD` returns the literal `HEAD`, which matches nothing, stranding any session opened before `gh pr checkout` moved 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 --patch` is the mbox commit series, not the PR diff. Measured on our own PRs: | | `--patch` | correct | |---|---|---| | #34 `github-resolution.test.ts` | 25 lines | **89** | | #34 `github-resolution.ts` | 29 | **59** | | #31 `agent.ts` | 15 | **32** | | #31 file count | 49 | 47 | Two 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 discussing `split('\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/cli` had **no typecheck script at all**, and the UI's failed on four TS6059 errors from its own tsconfig (`rootDir: src` against an `include` that reaches outside it), so its output had been noise long enough to ignore. Both fixed, `npm test` now runs `typecheck` first, 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.com/claude-code) https://claude.ai/code/session_018PkYQzbsnMihHesafWvXKs --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
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.
Two things a thread could not tell you: that it had already been posted, and that the author had already resolved it. Both were invisible in the same way — a finding sitting on a line looked the same whether nobody had read it or it had been ticked off yesterday.
Posted marker.
submitted_atwas already being written and never shown. The card now readsPosted to GitHub 15:37,yesterday 15:37, or12 Aug 15:37. A time of day rather than19h ago, because the question is whether the author has seen it, which is about when it landed.Resolution. REST carries no resolution state, so this is a second call to GraphQL for
reviewThreads { isResolved }, run as part of pulling comments.Matched on file and wording, not line — and this is the part worth reading. GitHub nulls a thread's
lineonce it goes outdated, and outdated is the usual state of a resolved thread: every one of #14378's 13 threads and #14364's came back withline: null. Keyed on the line, this sync would have quietly done nothing on exactly the threads it exists for. Two findings with identical wording in one file would now resolve together, which is the cheaper way to be wrong than missing every resolution.Pulling also refreshes the page when only resolutions changed — the old branch called
onPulled()only when comments arrived, so resolutions would have landed invisibly.Checked against real data, since a GraphQL shape is not unit-testable:
pullThreadStateparses #14378 (13 threads, 13 resolved) and #14380 (12, 6 resolved), and the matcher picks the sent thread while leaving an unsent one alone. Mutation-checked the marker too.parser 43, git 44, github 21, ui 384, cli 166; tsc at its baseline 6. Bumped to v0.9.8 per AGENTS.md.
🤖 Generated with Claude Code
https://claude.ai/code/session_018PkYQzbsnMihHesafWvXKs