Claude: changes for issue #42 #56

Merged

Grandiras merged 2 commits from claude/issue-42 into main

2026-07-25 02:25:50 +00:00

claude-bot commented

2026-07-25 02:18:08 +00:00

Collaborator

Requested in #42.

claude-bot added 1 commit

2026-07-25 02:18:08 +00:00

fix(oauth): don't abort sign-in when a handle probe times out (closes #42 )

CI / build-and-test (pull_request) Successful in 40s

Details

4b873688ce

Handle verification at the OAuth callback is best-effort — the authoritative
DID is the token response's `sub`, already in hand by then. But the catch
guards distinguished failures by exception type: a refused connection
(HttpRequestException) left IsHandleVerified = false, while a timed-out probe
(TaskCanceledException, from a parked handle domain or an HttpClient with a
ConnectTimeout) propagated out of CompleteAuthorizationAsync and failed the
whole login.

Both now mean "could not verify". Only the caller's own CancellationToken
aborts the flow, and that path now disposes the pending DPoP key, which
previously leaked whenever an exception escaped step 8.

VerifyDidToAuthServerConsistencyAsync gets the same distinction: caller
cancellation surfaces as cancellation instead of a "verification_failed"
OAuthException. A probe that merely timed out still fails closed there,
since that check is a real security boundary.

The ~100 s stall the issue also reports was fixed for the pre-redirect half
in #52 (raced lookups under a HandleResolutionTimeout budget); this covers
the callback half, where the wait now ends in an unverified handle rather
than an exception.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

claude-bot referenced this pull request

2026-07-25 02:18:09 +00:00

OAuth handle verification re-throws cancellation on a timed-out .well-known/atproto-did probe (and resolution hangs ~100s) for DNS-only handles #42

claude-bot commented

2026-07-25 02:18:25 +00:00

Author

Collaborator

🤖 Reviewing this PR — follow along in the action run; the reply will link the full session transcript.

🤖 Reviewing this PR — follow along in [the action run](https://git.grandiras.net/Grandiras/claude-bot/actions/runs/39); the reply will link the full session transcript.

Grandiras added 1 commit

2026-07-25 02:24:37 +00:00

Merge branch 'main' into claude/issue-42

CI / build-and-test (pull_request) Successful in 36s

Details

Sync Closures to GitHub / sync-closure (pull_request) Successful in 5s

Details

c3c6675d9c

Grandiras merged commit 9cf6436ec9 into main

2026-07-25 02:25:50 +00:00

Grandiras deleted branch claude/issue-42

2026-07-25 02:25:51 +00:00

Grandiras referenced this pull request from a commit

2026-07-25 02:25:52 +00:00

Claude: changes for issue #42 (#56)

claude-bot commented

2026-07-25 14:34:54 +00:00

Author

Collaborator

📝 Retroactive review summary (was stuck as an invisible draft by the APPROVE/APPROVED bug, fixed in v12.2 — verdict was: approve):

Solid, well-scoped fix. The three catch-clause changes consistently apply the intended rule: an OperationCanceledException that stems from the caller's own CancellationToken still propagates (and correctly disposes the pending DPoP key before doing so), while any other OperationCanceledException (e.g. TaskCanceledException from a connect timeout) now falls through to the generic handler and is treated as "could not verify" rather than aborting the login — exactly matching the CHANGELOG description. VerifyDidToAuthServerConsistencyAsync keeps its fail-closed wrap for genuine timeouts while still special-casing caller cancellation, and the pre-existing OAuthException rethrow there is unaffected by catch-order (checked the full method body). New tests cover DID-document timeout, both-authority timeout, the reported silent-well-known/DNS-answers case, wrong-DID mismatch, and caller cancellation — good coverage of the scenarios called out in the changelog. CHANGELOG entry is present and accurately describes the change. No correctness or security issues found.

⏱ 1m 0s · 17 turns · tokens: 26.0k in / 3.8k out (+761.7k cache-read)
📜 Full session transcript

📝 **Retroactive review summary** (was stuck as an invisible draft by the APPROVE/APPROVED bug, fixed in v12.2 — verdict was: approve): Solid, well-scoped fix. The three catch-clause changes consistently apply the intended rule: an `OperationCanceledException` that stems from the caller's own `CancellationToken` still propagates (and correctly disposes the pending DPoP key before doing so), while any other `OperationCanceledException` (e.g. `TaskCanceledException` from a connect timeout) now falls through to the generic handler and is treated as "could not verify" rather than aborting the login — exactly matching the CHANGELOG description. `VerifyDidToAuthServerConsistencyAsync` keeps its fail-closed wrap for genuine timeouts while still special-casing caller cancellation, and the pre-existing `OAuthException` rethrow there is unaffected by catch-order (checked the full method body). New tests cover DID-document timeout, both-authority timeout, the reported silent-well-known/DNS-answers case, wrong-DID mismatch, and caller cancellation — good coverage of the scenarios called out in the changelog. CHANGELOG entry is present and accurately describes the change. No correctness or security issues found. --- ⏱ 1m 0s · 17 turns · tokens: 26.0k in / 3.8k out (+761.7k cache-read) 📜 [Full session transcript](https://git.grandiras.net/Grandiras/claude-bot/src/branch/claude-logs/transcripts/Grandiras/ATProto.NET/pr-56/run-39.md)