perf: prefer the DNS answer in handle resolution #209

Grandiras · 2026-10-05T12:59:04Z

Grandiras commented

2026-10-05 12:59:04 +00:00

Closes #193. The owner approved this policy change in chat.

What changed

HandleResolver now follows the handle spec's resolution best practice: "It is ok to attempt both resolution methods in parallel, and to use the first successful result available. If the two methods return conflicting results (aka, different DIDs), the DNS TXT result should be preferred."

Both lookups still start together under one HandleResolutionTimeout budget.
A DNS answer is returned as soon as it arrives, and the HTTPS lookup is cancelled and observed in finally.
The HTTPS well-known is used only when DNS has no answer.
Several distinct did= records in DNS still fail with HandleConflict, which now means only that.
The answer cache from #197 is unchanged.

Security note: an HTTPS answer that disagrees with DNS no longer fails resolution; DNS wins. Control of a domain's DNS already implies control of its web host (A records, ACME DNS-01), and a hijacked web host still cannot override a DNS answer. Bidirectional verification against the DID document is unchanged.

Live latency (cache disabled, median of 5, warm connections)

handle	before	after
bsky.app	123 ms	42 ms
atproto.com	201 ms	23 ms
pfrazee.com	34 ms	34 ms
jcsalterego.bsky.social (HTTPS only)	134 ms	118 ms

A web host that drops packets no longer holds a DNS-configured handle for the whole 5 s budget.

Docs (did-resolution.md, oauth.md), the HandleConflict doc and the CHANGELOG are updated. Tests: the conflict test now asserts that DNS wins, and a new test checks that a DNS answer doesn't wait for a hanging well-known.

🤖 Generated with Claude Code

Closes #193. The owner approved this policy change in chat. ## What changed `HandleResolver` now follows the handle spec's resolution best practice: "It is ok to attempt both resolution methods in parallel, and to use the first successful result available. If the two methods return conflicting results (aka, different DIDs), the DNS TXT result should be preferred." - Both lookups still start together under one `HandleResolutionTimeout` budget. - A DNS answer is returned as soon as it arrives, and the HTTPS lookup is cancelled and observed in `finally`. - The HTTPS well-known is used only when DNS has no answer. - Several distinct `did=` records in DNS still fail with `HandleConflict`, which now means only that. - The answer cache from #197 is unchanged. Security note: an HTTPS answer that disagrees with DNS no longer fails resolution; DNS wins. Control of a domain's DNS already implies control of its web host (A records, ACME DNS-01), and a hijacked web host still cannot override a DNS answer. Bidirectional verification against the DID document is unchanged. ## Live latency (cache disabled, median of 5, warm connections) | handle | before | after | |---|---|---| | bsky.app | 123 ms | 42 ms | | atproto.com | 201 ms | 23 ms | | pfrazee.com | 34 ms | 34 ms | | jcsalterego.bsky.social (HTTPS only) | 134 ms | 118 ms | A web host that drops packets no longer holds a DNS-configured handle for the whole 5 s budget. Docs (`did-resolution.md`, `oauth.md`), the `HandleConflict` doc and the CHANGELOG are updated. Tests: the conflict test now asserts that DNS wins, and a new test checks that a DNS answer doesn't wait for a hanging well-known. 🤖 Generated with [Claude Code](https://claude.com/claude-code)

Grandiras added 1 commit

2026-10-05 12:59:04 +00:00

perf: prefer the DNS answer in handle resolution (closes #193 )

CI / pds-integration (pull_request) Successful in 26s

Details

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

Details

f465a77c0f

- A DNS TXT answer is returned at once and the concurrent HTTPS lookup is
  cancelled; the well-known counts only when DNS has none, as the handle
  spec's resolution best practice recommends.
- HandleConflict now means several DIDs in DNS only.

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

opencode-bot commented

2026-10-05 12:59:06 +00:00

🤖 Reviewing this PR — the review will link the full session transcript.

📜 Live transcript — updating every ~5 minutes while the review runs.

🤖 Reviewing this PR — the review will link the full session transcript.  📜 [Live transcript](https://git.grandiras.net/Grandiras/opencode-bot/src/branch/opencode-logs/transcripts/Grandiras/ATProto.NET/pr-209/run-2026-10-05T12-59-05-520Z.md) — updating every ~5 minutes while the review runs.

claude-bot commented

2026-10-05 12:59:07 +00:00

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

📜 Live transcript — updating every ~5 minutes while the task runs.

🤖 Reviewing this PR — follow along in [the action run](https://git.grandiras.net/Grandiras/claude-bot/actions/runs/739); the reply will link the full session transcript.  📜 [Live transcript](https://git.grandiras.net/Grandiras/claude-bot/src/branch/claude-logs/transcripts/Grandiras/ATProto.NET/pr-209/run-739.md) — updating every ~5 minutes while the task runs.

claude-reviewer reviewed

2026-10-05 12:59:33 +00:00

claude-reviewer left a comment

💬 Verdict: comment — notes only, nothing blocking.

The implementation is correct. DNS is awaited first and a null falls through to the already-running HTTPS task. The finally cancels the budget and observes httpsTask with SuppressThrowing, so there are no unobserved exceptions or leaked requests. A DNS conflict or caller cancellation still propagates. Only DNS-sourced answers can throw HandleConflict now. The new test is a good regression for the hanging well-known.

Remaining items, none blocking:

Stale docs. IHandleResolver.ResolveAsync in IDidResolver.cs:49-50 still says HandleConflict is thrown "when the authorities disagree". IIdentityResolver in the same file (line 63) still says "or to conflicting ones". Both should say that DNS publishes more than one DID. docs/migrating-to-0.7.md:1445 also describes the old disagreement behaviour, so check whether it needs a note.
Behavior change, no migration note. An HTTPS/DNS disagreement used to throw and now silently returns the DNS DID. The CHANGELOG puts it under Changed, which is arguably fine, but callers who relied on the exception for fail-closed behavior are affected. A short note would help.
Trust-root narrowing. DNS now comes through the configured DoH endpoint alone. A compromised or misconfigured DoH endpoint is therefore sufficient to pick the DID, and the HTTPS cross-check is gone. That matches the spec, and bidirectional verification still applies, but it is worth stating in the docs, since the DoH URL is configurable.
Minor. The new paragraph in docs/did-resolution.md has an over-long line.

🧠 claude-sonnet-5-5 · ⏱ 0m 18s · 4 turns · tokens: 18.9k in / 1.7k out (+70.7k cache-read)
📜 Full session transcript

💬 **Verdict: comment** — notes only, nothing blocking. The implementation is correct. DNS is awaited first and a null falls through to the already-running HTTPS task. The `finally` cancels the budget and observes `httpsTask` with `SuppressThrowing`, so there are no unobserved exceptions or leaked requests. A DNS conflict or caller cancellation still propagates. Only DNS-sourced answers can throw `HandleConflict` now. The new test is a good regression for the hanging well-known. Remaining items, none blocking: - **Stale docs.** `IHandleResolver.ResolveAsync` in `IDidResolver.cs:49-50` still says `HandleConflict` is thrown "when the authorities disagree". `IIdentityResolver` in the same file (line 63) still says "or to conflicting ones". Both should say that DNS publishes more than one DID. `docs/migrating-to-0.7.md:1445` also describes the old disagreement behaviour, so check whether it needs a note. - **Behavior change, no migration note.** An HTTPS/DNS disagreement used to throw and now silently returns the DNS DID. The CHANGELOG puts it under Changed, which is arguably fine, but callers who relied on the exception for fail-closed behavior are affected. A short note would help. - **Trust-root narrowing.** DNS now comes through the configured DoH endpoint alone. A compromised or misconfigured DoH endpoint is therefore sufficient to pick the DID, and the HTTPS cross-check is gone. That matches the spec, and bidirectional verification still applies, but it is worth stating in the docs, since the DoH URL is configurable. - **Minor.** The new paragraph in `docs/did-resolution.md` has an over-long line. --- 🧠 claude-sonnet-5-5 · ⏱ 0m 18s · 4 turns · tokens: 18.9k in / 1.7k out (+70.7k cache-read) 📜 [Full session transcript](https://git.grandiras.net/Grandiras/claude-bot/src/branch/claude-logs/transcripts/Grandiras/ATProto.NET/pr-209/run-739.md)

src/ATProtoNet/Identity/HandleResolver.cs

					
				@ -102,1 +101,3 @@

				            await Task.WhenAll(httpsTask, dnsTask).ConfigureAwait(false);

				            answer = (_dnsOverHttpsUrl is null ? null

				                    : await ResolveViaDnsAsync(handle, _dnsOverHttpsUrl, budget.Token, cancellationToken).ConfigureAwait(false))

				                ?? await httpsTask.ConfigureAwait(false);

The logic is fine. Related stale docs: IHandleResolver.ResolveAsync in IDidResolver.cs (lines 49-50) still documents HandleConflict as "when the authorities disagree", and IIdentityResolver (line 63) still says "or to conflicting ones". Update both to match the new meaning (more than one DID in DNS).

The logic is fine. Related stale docs: `IHandleResolver.ResolveAsync` in `IDidResolver.cs` (lines 49-50) still documents `HandleConflict` as "when the authorities disagree", and `IIdentityResolver` (line 63) still says "or to conflicting ones". Update both to match the new meaning (more than one DID in DNS).

opencode-reviewer approved these changes

2026-10-05 13:00:23 +00:00

Dismissed

opencode-reviewer left a comment

✅ Verdict: approve — no blocking issues found.

Clean implementation of the spec's DNS-preferred resolution policy. I verified the new try/finally control flow: a DNS answer or a DNS HandleConflict cancels the budget so the moot HTTPS lookup stops, and ConfigureAwaitOptions.SuppressThrowing (with the correct non-generic Task cast) observes the HTTPS task without letting its cancellation mask the real outcome; caller cancellation still propagates from whichever lookup hits it. The two-record-DNS HandleConflict path this PR narrows is exactly the failure the handle spec calls a failed resolution, and the removal of the HTTPS-vs-DNS conflict throw is the preference the spec explicitly endorses, so the security relaxation is sound — bidirectional DID-document verification, which is the real gate, is untouched.

Two notes, neither blocking:

The interface remark on IHandleResolver.ResolveAsync (src/ATProtoNet/Identity/IDidResolver.cs, lines 49–50) still describes HandleConflict as "when the authorities disagree"; after this change it only means multiple DIDs in DNS. Worth a one-line follow-up.
The new ResolveAsync_DnsAnswers_DoesNotWaitForHttps test (1-minute budget) is the one that actually proves the early return; the pre-existing 300 ms-budget test would also have passed under the old code, so the new test is the load-bearing one. Good that it exists.

Docs (did-resolution.md, oauth.md), the enum doc, and the CHANGELOG are consistent with the new behavior, and the public API surface is unchanged.

⏱ 1m 18s · 7 steps · $0.0136 · tokens: 31.8k in / 1.1k out (+82.9k cache-read) · Go: 5h 0% · wk 1% · mo 91%
📜 Full session transcript

✅ **Verdict: approve** — no blocking issues found. Clean implementation of the spec's DNS-preferred resolution policy. I verified the new try/finally control flow: a DNS answer or a DNS `HandleConflict` cancels the budget so the moot HTTPS lookup stops, and `ConfigureAwaitOptions.SuppressThrowing` (with the correct non-generic `Task` cast) observes the HTTPS task without letting its cancellation mask the real outcome; caller cancellation still propagates from whichever lookup hits it. The two-record-DNS `HandleConflict` path this PR narrows is exactly the failure the handle spec calls a failed resolution, and the removal of the HTTPS-vs-DNS conflict throw is the preference the spec explicitly endorses, so the security relaxation is sound — bidirectional DID-document verification, which is the real gate, is untouched. Two notes, neither blocking: - The interface remark on `IHandleResolver.ResolveAsync` (`src/ATProtoNet/Identity/IDidResolver.cs`, lines 49–50) still describes `HandleConflict` as "when the authorities disagree"; after this change it only means multiple DIDs in DNS. Worth a one-line follow-up. - The new `ResolveAsync_DnsAnswers_DoesNotWaitForHttps` test (1-minute budget) is the one that actually proves the early return; the pre-existing 300 ms-budget test would also have passed under the old code, so the new test is the load-bearing one. Good that it exists. Docs (`did-resolution.md`, `oauth.md`), the enum doc, and the CHANGELOG are consistent with the new behavior, and the public API surface is unchanged. --- ⏱ 1m 18s · 7 steps · $0.0136 · tokens: 31.8k in / 1.1k out (+82.9k cache-read) · Go: 5h 0% · wk 1% · mo 91% 📜 [Full session transcript](https://git.grandiras.net/Grandiras/opencode-bot/src/branch/opencode-logs/transcripts/Grandiras/ATProto.NET/pr-209/run-2026-10-05T12-59-05-520Z.md)

Grandiras added 1 commit

2026-10-05 13:01:53 +00:00

docs: describe HandleConflict as several DIDs in DNS, and the DoH endpoint's role

CI / pds-integration (pull_request) Successful in 26s

Details

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

Details

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

Details

554b46d59e

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

opencode-bot commented

2026-10-05 13:02:54 +00:00

🤖 Reviewing this PR — the review will link the full session transcript.

📜 Live transcript — updating every ~5 minutes while the review runs.

🤖 Reviewing this PR — the review will link the full session transcript.  📜 [Live transcript](https://git.grandiras.net/Grandiras/opencode-bot/src/branch/opencode-logs/transcripts/Grandiras/ATProto.NET/pr-209/run-2026-10-05T13-02-54-657Z.md) — updating every ~5 minutes while the review runs.

claude-bot commented

2026-10-05 13:02:57 +00:00

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

📜 Live transcript — updating every ~5 minutes while the task runs.

🤖 Reviewing this PR — follow along in [the action run](https://git.grandiras.net/Grandiras/claude-bot/actions/runs/740); the reply will link the full session transcript.  📜 [Live transcript](https://git.grandiras.net/Grandiras/claude-bot/src/branch/claude-logs/transcripts/Grandiras/ATProto.NET/pr-209/run-740.md) — updating every ~5 minutes while the task runs.

claude-reviewer approved these changes

2026-10-05 13:03:23 +00:00

claude-reviewer left a comment

✅ Verdict: approve — no blocking issues found.

The items from my last pass are mostly resolved.

Stale XML docs: fixed. IHandleResolver.ResolveAsync and IIdentityResolver.ResolveAsync now say HandleConflict means more than one DID in DNS.
DoH trust root: fixed. docs/did-resolution.md now says the DoH endpoint decides which DID a handle names, and that the DID document must still claim the handle back.
Long line in docs/did-resolution.md: fixed.
Implementation: still correct. DNS is awaited first, and a null falls through to the already-running HTTPS task. The finally cancels the budget and observes httpsTask with SuppressThrowing.

One non-blocking item remains. docs/migrating-to-0.7.md:1444-1445 still says handle resolution fails closed when DNS and HTTPS disagree. That is now wrong, because DNS wins and only several DNS records conflict. Please reword it, since the page describes the 0.7 behaviour that this PR changes.

🧠 claude-sonnet-5-5 · ⏱ 0m 9s · 3 turns · tokens: 18.7k in / 583 out (+42.9k cache-read)
📜 Full session transcript

✅ **Verdict: approve** — no blocking issues found. The items from my last pass are mostly resolved. - **Stale XML docs:** fixed. `IHandleResolver.ResolveAsync` and `IIdentityResolver.ResolveAsync` now say `HandleConflict` means more than one DID in DNS. - **DoH trust root:** fixed. `docs/did-resolution.md` now says the DoH endpoint decides which DID a handle names, and that the DID document must still claim the handle back. - **Long line in `docs/did-resolution.md`:** fixed. - **Implementation:** still correct. DNS is awaited first, and a null falls through to the already-running HTTPS task. The `finally` cancels the budget and observes `httpsTask` with `SuppressThrowing`. One non-blocking item remains. `docs/migrating-to-0.7.md:1444-1445` still says handle resolution fails closed when DNS and HTTPS disagree. That is now wrong, because DNS wins and only several DNS records conflict. Please reword it, since the page describes the 0.7 behaviour that this PR changes. --- 🧠 claude-sonnet-5-5 · ⏱ 0m 9s · 3 turns · tokens: 18.7k in / 583 out (+42.9k cache-read) 📜 [Full session transcript](https://git.grandiras.net/Grandiras/claude-bot/src/branch/claude-logs/transcripts/Grandiras/ATProto.NET/pr-209/run-740.md)

opencode-reviewer approved these changes

2026-10-05 13:05:18 +00:00

opencode-reviewer left a comment

✅ Verdict: approve — no blocking issues found.

Re-review: the follow-ups from the last pass are resolved. The IHandleResolver / IIdentityResolver remarks in IDidResolver.cs now describe HandleConflict as several DIDs in DNS (in line with the enum doc, the OAuth error table and the CHANGELOG), and did-resolution.md now states that the configurable DoH endpoint decides which DID a handle names — the trust-root narrowing flagged earlier is documented.

Re-verified the implementation: the DNS answer (or a DNS HandleConflict) propagates from the awaited DNS task; the finally cancels the budget and observes the moot HTTPS lookup with ConfigureAwaitOptions.SuppressThrowing on the non-generic Task cast, so no unobserved exceptions; caller cancellation still surfaces; the answer cache and the multi-record DNS conflict path are unchanged. The spec explicitly endorses preferring the DNS result, and bidirectional DID-document verification is untouched, so the security relaxation is sound. The new hanging-well-known test is the load-bearing regression for the early return.

One minor non-blocking leftover: docs/migrating-to-0.7.md:1444–1445 still says 0.7 handle resolution "fails closed on a conflict (DNS naming two DIDs, or DNS and HTTPS disagreeing)". Since this PR's CHANGELOG entry is in the same unreleased 0.7 section, that migration bullet now describes behavior 0.7 won't have — worth a one-line touch-up in a follow-up.

⏱ 2m 23s · 7 steps · $0.0079 · tokens: 35.6k in / 1.0k out (+50.6k cache-read) · Go: 5h 1% · wk 1% · mo 91%
📜 Full session transcript

✅ **Verdict: approve** — no blocking issues found. Re-review: the follow-ups from the last pass are resolved. The `IHandleResolver` / `IIdentityResolver` remarks in `IDidResolver.cs` now describe `HandleConflict` as several DIDs in DNS (in line with the enum doc, the OAuth error table and the CHANGELOG), and `did-resolution.md` now states that the configurable DoH endpoint decides which DID a handle names — the trust-root narrowing flagged earlier is documented. Re-verified the implementation: the DNS answer (or a DNS `HandleConflict`) propagates from the awaited DNS task; the `finally` cancels the budget and observes the moot HTTPS lookup with `ConfigureAwaitOptions.SuppressThrowing` on the non-generic `Task` cast, so no unobserved exceptions; caller cancellation still surfaces; the answer cache and the multi-record DNS conflict path are unchanged. The spec explicitly endorses preferring the DNS result, and bidirectional DID-document verification is untouched, so the security relaxation is sound. The new hanging-well-known test is the load-bearing regression for the early return. One minor non-blocking leftover: `docs/migrating-to-0.7.md:1444–1445` still says 0.7 handle resolution "fails closed on a conflict (DNS naming two DIDs, or DNS and HTTPS disagreeing)". Since this PR's CHANGELOG entry is in the same unreleased 0.7 section, that migration bullet now describes behavior 0.7 won't have — worth a one-line touch-up in a follow-up. --- ⏱ 2m 23s · 7 steps · $0.0079 · tokens: 35.6k in / 1.0k out (+50.6k cache-read) · Go: 5h 1% · wk 1% · mo 91% 📜 [Full session transcript](https://git.grandiras.net/Grandiras/opencode-bot/src/branch/opencode-logs/transcripts/Grandiras/ATProto.NET/pr-209/run-2026-10-05T13-02-54-657Z.md)

Grandiras merged commit c9c08939e8 into main

2026-10-05 13:06:50 +00:00

Grandiras deleted branch issue-193-dns-first

2026-10-05 13:06:51 +00:00

Grandiras referenced this pull request from a commit

2026-10-05 13:06:52 +00:00

perf: prefer the DNS answer in handle resolution (#209)

Sign in to join this conversation.

No reviewers