Claude: changes for issue #35 #63

Merged
Grandiras merged 2 commits from claude/issue-35 into main 2026-07-25 17:09:07 +00:00
Collaborator

Closes #35.

Closes #35.
fix: make LoginForm's IStringLocalizer injection genuinely optional
All checks were successful
CI / build-and-test (pull_request) Successful in 38s
1f140a1bfa
Blazor's [Inject] property injection requires the service regardless of
the property's nullable annotation, so rendering <LoginForm /> in an app
that had not called services.AddLocalization() threw
InvalidOperationException before parameters were applied — explicit
ButtonText/HandlePlaceholder values could not avoid it.

Resolve the localizer through IServiceProvider.GetService<T>() instead.
Without a registration the built-in English defaults render; with one the
copy is localized exactly as before, and explicit parameters still win.

Adds LoginFormTests covering all three paths via HtmlRenderer (the
no-localization test reproduces the reported exception without the fix).

closes #35
Author
Collaborator

🤖 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/58); 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-63/run-58.md) — updating every ~5 minutes while the task runs.
claude-bot left a comment

The LoginForm fix itself is solid: resolving the localizer via IServiceProvider.GetService<T>() instead of [Inject] correctly makes it optional, and the added tests cover the no-localizer, localizer-registered, and explicit-parameter-override cases well. Docs update is accurate.

However, the CHANGELOG.md edit is corrupted and must be fixed before merge: instead of adding a new bullet for Issue #35, the diff overwrote the existing Issue #41 (OAuthClientMetadata) bullet in place, splicing the new LoginForm description onto the leftover tail of the old OAuth description. This produces one garbled, nonsensical bullet and silently deletes the Issue #41 changelog entry.

  • CHANGELOG.md:43 — This bullet is corrupted: it starts as the new Issue #35 (LoginForm) entry but then splices in the leftover tail of the old Issue #41 (OAuthClientMetadata) entry ("...so authorization servers rejected the client-metadata document** (Issue #41) — The AT Protocol OAuth spec distinguishes absent from null..."), producing one garbled bullet that makes no sense and effectively deletes the original Issue #41 changelog entry. This needs to be a distinct new bullet under ### Fixed, leaving the existing Issue #41 entry intact.

⏱ 0m 35s · 5 turns · tokens: 14.8k in / 2.4k out (+199.2k cache-read)
📜 Full session transcript

The `LoginForm` fix itself is solid: resolving the localizer via `IServiceProvider.GetService<T>()` instead of `[Inject]` correctly makes it optional, and the added tests cover the no-localizer, localizer-registered, and explicit-parameter-override cases well. Docs update is accurate. However, the `CHANGELOG.md` edit is corrupted and must be fixed before merge: instead of adding a new bullet for Issue #35, the diff overwrote the *existing* Issue #41 (`OAuthClientMetadata`) bullet in place, splicing the new LoginForm description onto the leftover tail of the old OAuth description. This produces one garbled, nonsensical bullet and silently deletes the Issue #41 changelog entry. - `CHANGELOG.md:43` — This bullet is corrupted: it starts as the new Issue #35 (`LoginForm`) entry but then splices in the leftover tail of the old Issue #41 (`OAuthClientMetadata`) entry ("...so authorization servers rejected the client-metadata document** (Issue #41) — The AT Protocol OAuth spec distinguishes *absent* from *null*..."), producing one garbled bullet that makes no sense and effectively deletes the original Issue #41 changelog entry. This needs to be a distinct new bullet under `### Fixed`, leaving the existing Issue #41 entry intact. --- ⏱ 0m 35s · 5 turns · tokens: 14.8k in / 2.4k out (+199.2k cache-read) 📜 [Full session transcript](https://git.grandiras.net/Grandiras/claude-bot/src/branch/claude-logs/transcripts/Grandiras/ATProto.NET/pr-63/run-58.md) <!-- claude-reviewed-head:1f140a1bfa53018f80a3d2e16cce5c3cdd65a42e -->
Owner

@claude-bot Please fix the conflict and work on the feedback.

@claude-bot Please fix the conflict and work on the feedback.
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/66); 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-63/run-66.md) — updating every ~5 minutes while the task runs.
Merge origin/main into claude/issue-35
All checks were successful
CI / build-and-test (pull_request) Successful in 39s
Sync Closures to GitHub / sync-closure (pull_request) Successful in 6s
ff0f7d9edf
Resolves the CHANGELOG.md conflict and fixes the corrupted entry flagged in
review: the Issue #35 bullet had been spliced onto the tail of the existing
Issue #41 (OAuthClientMetadata) bullet, garbling it and deleting the #41 entry.

Issue #41's bullet is restored verbatim from main, and Issue #35 is now its own
distinct bullet at the top of ### Fixed.

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

Merged main into claude/issue-35 and fixed the corrupted CHANGELOG entry. Pushed as ff0f7d9 on branch claude/issue-35.

Conflict

The branch was based on e0811cb while main had moved to 60fd3bf. The only conflict was in CHANGELOG.md — and it was exactly the review finding: the branch's single-line edit had replaced the Issue #41 (OAuthClientMetadata) bullet in place, so git saw the new #35 text and the four ### Fixed bullets that landed on main since (#40 ×2, #45 ×2) as competing changes to the same region.

Resolved by:

  • Restoring main's ### Fixed block verbatim, so the Issue #41 bullet is back intact (and #40/#45 entries are preserved).
  • Truncating the spliced text at The services.AddLocalization() workaround remains valid and adding it as a distinct new bullet at the top of ### Fixed. No leftover OAuthClientMetadata tail remains in it.

The diff against main is now exactly one added CHANGELOG line, with zero deletions:

 CHANGELOG.md                                     |  1 +
 docs/blazor.md                                   |  4 ++
 src/ATProtoNet.Blazor/Components/LoginForm.razor | 24 ++++++-
 tests/ATProtoNet.Tests/Blazor/LoginFormTests.cs  | 87 ++++++++++++++++++++++++

The LoginForm fix, docs, and tests were unchanged — the review had no findings against them.

Verified

No .NET SDK was present in this container, so I installed 10.0.302 and ran the canonical checks on the merged tree:

  • dotnet build -p:EnableSourceControlManagerQueries=false0 errors (warnings are pre-existing CS1591/NU1902 noise).
  • dotnet test tests/ATProtoNet.Tests/1330 passed, 0 failed, 0 skipped.
  • --filter "FullyQualifiedName~LoginFormTests"3 passed (no-localizer, localizer-registered, explicit-parameter-override).

⏱ 2m 22s · 22 turns · tokens: 31.6k in / 4.6k out (+803.0k cache-read)
📜 Full session transcript

Merged `main` into `claude/issue-35` and fixed the corrupted CHANGELOG entry. Pushed as `ff0f7d9` on branch **`claude/issue-35`**. ## Conflict The branch was based on `e0811cb` while `main` had moved to `60fd3bf`. The only conflict was in `CHANGELOG.md` — and it was exactly the review finding: the branch's single-line edit had replaced the Issue #41 (`OAuthClientMetadata`) bullet in place, so git saw the new #35 text and the four `### Fixed` bullets that landed on `main` since (#40 ×2, #45 ×2) as competing changes to the same region. Resolved by: - Restoring `main`'s `### Fixed` block verbatim, so the **Issue #41 bullet is back intact** (and #40/#45 entries are preserved). - Truncating the spliced text at `The services.AddLocalization() workaround remains valid` and adding it as a **distinct new bullet** at the top of `### Fixed`. No leftover `OAuthClientMetadata` tail remains in it. The diff against `main` is now exactly one added CHANGELOG line, with zero deletions: ``` CHANGELOG.md | 1 + docs/blazor.md | 4 ++ src/ATProtoNet.Blazor/Components/LoginForm.razor | 24 ++++++- tests/ATProtoNet.Tests/Blazor/LoginFormTests.cs | 87 ++++++++++++++++++++++++ ``` The `LoginForm` fix, docs, and tests were unchanged — the review had no findings against them. ## Verified No .NET SDK was present in this container, so I installed 10.0.302 and ran the canonical checks on the merged tree: - `dotnet build -p:EnableSourceControlManagerQueries=false` — **0 errors** (warnings are pre-existing CS1591/NU1902 noise). - `dotnet test tests/ATProtoNet.Tests/` — **1330 passed, 0 failed, 0 skipped**. - `--filter "FullyQualifiedName~LoginFormTests"` — **3 passed** (no-localizer, localizer-registered, explicit-parameter-override). --- ⏱ 2m 22s · 22 turns · tokens: 31.6k in / 4.6k out (+803.0k cache-read) 📜 [Full session transcript](https://git.grandiras.net/Grandiras/claude-bot/src/branch/claude-logs/transcripts/Grandiras/ATProto.NET/issue-63/run-66.md)
Grandiras deleted branch claude/issue-35 2026-07-25 17:09:07 +00:00
Grandiras referenced this pull request from a commit 2026-07-25 17:09:09 +00:00
Sign in to join this conversation.
No description provided.