Claude: changes for issue #39 #59

Merged
Grandiras merged 2 commits from claude/issue-39 into main 2026-07-25 02:40:54 +00:00
Collaborator

Closes #39.

Closes #39.
feat: let MapAtProtoPds exclude or decorate individual endpoints (closes #39)
All checks were successful
CI / build-and-test (pull_request) Successful in 40s
9f6658c29c
MapAtProtoPds() mapped all twelve PDS XRPC endpoints unconditionally, so a
host that needed to own one of them (e.g. com.atproto.server.createAccount
for real invite-code validation) could only intercept it with terminal
middleware ahead of the call — bypassing endpoint routing, metadata, and
route-level auth policies.

Add a MapAtProtoPds(Action<PdsEndpointOptions>) overload:

- Exclude(...) skips mapping an endpoint so the host can map its own
  implementation on the same route with no ambiguous-match conflict
- Only(...) maps just the listed subset; Exclude wins over Only
- Configure(nsid, ...) / ConfigureAll(...) apply route conventions
  (authorization, endpoint filters, rate limiting, metadata)

NSIDs are validated against PdsEndpointNames.All and throw at startup on a
typo. Mapped endpoints now carry their NSID as the endpoint display name.
The parameterless overload is unchanged and still maps everything.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
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/46); the reply will link the full session transcript.
Owner

@claude-bot Please fix the small conflict.

@claude-bot Please fix the small conflict.
Author
Collaborator

🤖 On it — follow along in the action run; the reply will link the full session transcript.

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

🤖 On it — follow along in [the action run](https://git.grandiras.net/Grandiras/claude-bot/actions/runs/48); 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/issue-59/run-48.md) — updating every ~5 minutes while the task runs.
Merge remote-tracking branch 'origin/main' into claude/issue-39
All checks were successful
CI / build-and-test (pull_request) Successful in 39s
Sync Closures to GitHub / sync-closure (pull_request) Successful in 6s
5b6e762d5b
# Conflicts:
#	CHANGELOG.md
Grandiras deleted branch claude/issue-39 2026-07-25 02:40:55 +00:00
Grandiras referenced this pull request from a commit 2026-07-25 02:40:55 +00:00
Author
Collaborator

Conflict resolved and pushed to claude/issue-39 (merge commit 5b6e762).

What the conflict was: main moved ahead with #58 (issue #41), which added a bullet at the same spot in the ### Added section of CHANGELOG.md that this PR's two #39 bullets occupy. Purely adjacent additions — no semantic overlap.

Resolution: kept both sides, with the OAuthClientMetadata.ToJson (#41) bullet ahead of the MapAtProtoPds(Action<PdsEndpointOptions>) / PdsEndpointNames (#39) bullets. No other file conflicted; the merge brought in main's OAuthModels.cs, docs/oauth.md, and OAuthModelsTests.cs changes cleanly.

Verified: no conflict markers remain, and dotnet test tests/ATProtoNet.Tests/ -p:EnableSourceControlManagerQueries=false on the merged tree passes — 1033 passed, 0 failed, 0 skipped. (The container had no SDK, so I installed .NET 10.0.302 first.) The PR should now show as mergeable.


⏱ 1m 33s · 14 turns · tokens: 20.3k in / 2.7k out (+414.2k cache-read)
📜 Full session transcript

Conflict resolved and pushed to `claude/issue-39` (merge commit `5b6e762`). **What the conflict was:** `main` moved ahead with #58 (issue #41), which added a bullet at the same spot in the `### Added` section of `CHANGELOG.md` that this PR's two `#39` bullets occupy. Purely adjacent additions — no semantic overlap. **Resolution:** kept both sides, with the `OAuthClientMetadata.ToJson` (#41) bullet ahead of the `MapAtProtoPds(Action<PdsEndpointOptions>)` / `PdsEndpointNames` (#39) bullets. No other file conflicted; the merge brought in `main`'s `OAuthModels.cs`, `docs/oauth.md`, and `OAuthModelsTests.cs` changes cleanly. **Verified:** no conflict markers remain, and `dotnet test tests/ATProtoNet.Tests/ -p:EnableSourceControlManagerQueries=false` on the merged tree passes — **1033 passed, 0 failed, 0 skipped**. (The container had no SDK, so I installed .NET 10.0.302 first.) The PR should now show as mergeable. --- ⏱ 1m 33s · 14 turns · tokens: 20.3k in / 2.7k out (+414.2k cache-read) 📜 [Full session transcript](https://git.grandiras.net/Grandiras/claude-bot/src/branch/claude-logs/transcripts/Grandiras/ATProto.NET/issue-59/run-48.md)
Author
Collaborator

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

Clean, well-scoped addition of PdsEndpointOptions/PdsEndpointNames and the new MapAtProtoPds(Action<PdsEndpointOptions>) overload. The refactor of PdsHostingExtensions.MapAtProtoPds to route every registration through the local Map helper is mechanical and preserves each handler's original body verbatim — verified by diffing the full post-change file against the original structure. Exclude/Only/Configure/ConfigureAll compose sensibly (Exclude correctly wins over Only, matching both the doc and the test Only_CombinedWithExclude_ExcludeWins), NSIDs are validated against PdsEndpointNames.All so typos fail fast at startup, and the display-name change (WithDisplayName(nsid)) is a reasonable diagnostics improvement. Test coverage is thorough: real TestServer-backed tests for exclusion/inclusion/route-conflict-avoidance/display-name/conventions, plus fast unit tests on PdsEndpointOptions itself. Docs (pds.md, xrpc-handlers.md) and CHANGELOG.md are updated consistently with the new API and correctly reflect the Added/Changed split per repo convention. No correctness, security, or omission issues found.


⏱ 1m 4s · 10 turns · tokens: 33.0k in / 4.3k out (+542.1k 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): Clean, well-scoped addition of `PdsEndpointOptions`/`PdsEndpointNames` and the new `MapAtProtoPds(Action<PdsEndpointOptions>)` overload. The refactor of `PdsHostingExtensions.MapAtProtoPds` to route every registration through the local `Map` helper is mechanical and preserves each handler's original body verbatim — verified by diffing the full post-change file against the original structure. `Exclude`/`Only`/`Configure`/`ConfigureAll` compose sensibly (`Exclude` correctly wins over `Only`, matching both the doc and the test `Only_CombinedWithExclude_ExcludeWins`), NSIDs are validated against `PdsEndpointNames.All` so typos fail fast at startup, and the display-name change (`WithDisplayName(nsid)`) is a reasonable diagnostics improvement. Test coverage is thorough: real `TestServer`-backed tests for exclusion/inclusion/route-conflict-avoidance/display-name/conventions, plus fast unit tests on `PdsEndpointOptions` itself. Docs (`pds.md`, `xrpc-handlers.md`) and `CHANGELOG.md` are updated consistently with the new API and correctly reflect the `Added`/`Changed` split per repo convention. No correctness, security, or omission issues found. --- ⏱ 1m 4s · 10 turns · tokens: 33.0k in / 4.3k out (+542.1k cache-read) 📜 [Full session transcript](https://git.grandiras.net/Grandiras/claude-bot/src/branch/claude-logs/transcripts/Grandiras/ATProto.NET/pr-59/run-46.md) <!-- claude-reviewed-head:9f6658c29c3d88d88b609c262df15de4fd5d6247 -->
Sign in to join this conversation.
No description provided.