Claude: changes for issue #140 #142

Merged
Grandiras merged 2 commits from claude/issue-140 into main 2026-08-21 14:33:16 +00:00
Collaborator

Closes #140.

Closes #140.
Ask for the permission /profile needs, by default
All checks were successful
CI / build (pull_request) Successful in 40s
bf2da92830
Editing your own name and picture is not an optional extra of a board that
puts both on every card, and until now the only way any deployment got it was
by setting AtProto:ProfileScope itself — so nobody had it. The alternative on
the table, writing the record with our PDS's admin credentials, would have
worked for members who signed up here and for nobody who arrived with a
bsky.social account: a two-class board, not a feature. The OAuth grant is the
one mechanism that works wherever a member's account lives, so the app asks
for it.

transition:generic is what it asks for, because it is still the only scope any
account server accepts for this — re-measured today against bsky.social and
pds.ponente.app, both advertise the same four scopes and neither knows the
granular pair the specification defines. So `generic` becomes the default and
`none` becomes the opt-out. Unset and unrecognized are now different answers:
nothing configured gets the default, while a value that is set and not
understood still reads as `none`, because a typo must not hand somebody's
account over.

The page says what that means, once: the network-wide warning moves from the
top of the page to directly above the two fields it warns about, and the
permission footnote loses a sentence. Both example env files and the prod
compose default flip with it; a server whose own .env pins `none` keeps the
old behaviour until that line changes and deploy.sh runs.

Also writes the density rule down where the next agent will read it —
CLAUDE.md's conventions and a new Voice section in docs/brand.md — since
"far too verbose" has been the recurring note on copy here.

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.

📜 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/366); the reply will link the full session transcript. <!-- claude-run:pr_review --> 📜 [Live transcript](https://git.grandiras.net/Grandiras/claude-bot/src/branch/claude-logs/transcripts/Grandiras/Ponente/pr-142/run-366.md) — updating every ~5 minutes while the task runs.
claude-reviewer approved these changes 2026-08-21 14:06:11 +00:00
Dismissed
claude-reviewer left a comment

Verdict: approve — no blocking issues found.

Flips the default AtProto:ProfileScope from none to generic so every member can edit their profile out of the box, with a clean opt-out via none. Traced the logic end to end and it holds together:

  • ProfileScope.Parse: configured?.Trim().ToLowerInvariant() short-circuits correctly for null, so null/empty/whitespace all map to the new null or "" case → Generic, while any set-but-unrecognized value still falls through to None (fail-towards-less-permission is preserved). Verified against the updated ProfileScopeTests, which now correctly split "unset → Generic" from "opted-out or garbage → None".
  • Profile.razor/ProfileService.LoadAsync: moved the profile.network hint out of the always-shown top-of-page slot into the branch that actually renders the fields, matching docs/brand.md's new "say it once, where it matters" voice guidance — it no longer shows in the NotConfigured/NeedsReauthorization locked states, which is correct since there are no fields to warn about there.
  • The e2e assertion change (dev-login → NeedsReauthorization.profile-locked + a /login/start?...&returnUrl=%2Fprofile link) is consistent with ProfileService: dev login stores no token, so granted is null, CanWriteProfile(null) is false, and with the new Generic default EditingConfigured is true, landing exactly on NeedsReauthorization as the test now expects.
  • Copy, Program.cs startup log, Changelog (both locales), docs/auth.md, docs/brand.md, README, and all three compose/env surfaces (docker-compose.yml, deploy/docker-compose.prod.yml, both .env*.example files) were updated in lockstep — no stale references to the old default or to the removed profile.lede locale key/CSS class found anywhere in the branch.
  • docs/auth.md's callout that existing deployments keep the old none behavior until their .env is edited and redeployed is self-aware and correctly scoped — deploy/README.md's existing "check .env against .example for keys added" guidance only catches new keys, not changed defaults for existing ones, which the doc text acknowledges.

No correctness, security, or consistency issues found.


⏱ 1m 17s · 19 turns · tokens: 47.2k in / 6.1k out (+953.4k cache-read)
📜 Full session transcript

✅ **Verdict: approve** — no blocking issues found. Flips the default `AtProto:ProfileScope` from `none` to `generic` so every member can edit their profile out of the box, with a clean opt-out via `none`. Traced the logic end to end and it holds together: - `ProfileScope.Parse`: `configured?.Trim().ToLowerInvariant()` short-circuits correctly for `null`, so `null`/empty/whitespace all map to the new `null or ""` case → `Generic`, while any *set-but-unrecognized* value still falls through to `None` (fail-towards-less-permission is preserved). Verified against the updated `ProfileScopeTests`, which now correctly split "unset → Generic" from "opted-out or garbage → None". - `Profile.razor`/`ProfileService.LoadAsync`: moved the `profile.network` hint out of the always-shown top-of-page slot into the branch that actually renders the fields, matching `docs/brand.md`'s new "say it once, where it matters" voice guidance — it no longer shows in the `NotConfigured`/`NeedsReauthorization` locked states, which is correct since there are no fields to warn about there. - The e2e assertion change (dev-login → `NeedsReauthorization` → `.profile-locked` + a `/login/start?...&returnUrl=%2Fprofile` link) is consistent with `ProfileService`: dev login stores no token, so `granted` is `null`, `CanWriteProfile(null)` is false, and with the new `Generic` default `EditingConfigured` is true, landing exactly on `NeedsReauthorization` as the test now expects. - Copy, `Program.cs` startup log, `Changelog` (both locales), `docs/auth.md`, `docs/brand.md`, README, and all three compose/env surfaces (`docker-compose.yml`, `deploy/docker-compose.prod.yml`, both `.env*.example` files) were updated in lockstep — no stale references to the old default or to the removed `profile.lede` locale key/CSS class found anywhere in the branch. - `docs/auth.md`'s callout that existing deployments keep the old `none` behavior until their `.env` is edited and redeployed is self-aware and correctly scoped — `deploy/README.md`'s existing "check `.env` against `.example` for keys added" guidance only catches *new* keys, not changed defaults for existing ones, which the doc text acknowledges. No correctness, security, or consistency issues found. --- ⏱ 1m 17s · 19 turns · tokens: 47.2k in / 6.1k out (+953.4k cache-read) 📜 [Full session transcript](https://git.grandiras.net/Grandiras/claude-bot/src/branch/claude-logs/transcripts/Grandiras/Ponente/pr-142/run-366.md) <!-- claude-verdict:approve --> <!-- claude-reviewed-head:bf2da9283018a40e83c125909777813ce404199a -->
Ask for the narrow profile scope, not transition:generic
All checks were successful
CI / build (pull_request) Successful in 38s
db1e6f0be5
The granular pair the OAuth scopes spec defines for exactly this —
repo:app.bsky.actor.profile blob:image/* — is accepted today. The earlier
conclusion that nothing takes it came from reading scopes_supported, which
only ever names the fixed scopes: parameterized ones cannot be enumerated.

Measured 2026-08-21 against bsky.social and pds.ponente.app: a PAR carrying
the granular pair is accepted and the authorization endpoint renders a
consent screen with that scope intact, both via curl and end to end through
Ponente's own /login/start. A scope a server does not understand is silently
dropped, not refused (atproto totally:bogus.scope -> scope: "atproto"), so
asking narrowly degrades to "sign in again", never to a broken sign-in.

AtProto:ProfileScope now defaults to granular. generic stays as the blanket
fallback for an account server too old to parse it; none is still the opt-out
and an unrecognized value still reads as none.

Also corrects profile.permission.generic in both locales, which claimed no
account server offers a profile-only permission.
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/368); the reply will link the full session transcript. <!-- claude-run:pr_review --> 📜 [Live transcript](https://git.grandiras.net/Grandiras/claude-bot/src/branch/claude-logs/transcripts/Grandiras/Ponente/pr-142/run-368.md) — updating every ~5 minutes while the task runs.
claude-reviewer left a comment

Verdict: approve — no blocking issues found.

This supersedes my previous approval of a generic-default version of this PR — the branch has since been reworked around a new finding: scopes_supported was a red herring (it only lists the fixed, non-parameterized scopes; an unrecognized scope is silently dropped, not rejected), and a direct authorization-endpoint check shows the granular repo:app.bsky.actor.profile blob:image/* pair is actually accepted by both bsky.social and the bundled PDS. This is consistent with what's publicly documented about atproto's OAuth scopes rollout (granular permissions have been rolling out to bsky.social and self-hosted PDS through 2025–2026), so the central factual claim holds up.

Traced the new logic end to end:

  • ProfileScope.Parse: configured?.Trim().ToLowerInvariant() correctly short-circuits null to null, and .Trim() reduces whitespace-only strings to "", so the new null or "" => Granular arm covers unset/empty/whitespace, while any set-but-unrecognized value still falls through to None — fail-towards-less-permission is preserved for typos, while "never configured" now gets the more useful default. ProfileScopeTests correctly splits this into two theories: opt-out/garbage → None, and null/""/whitespace → Granular with the exact expected Requested string.
  • ProfileService/Profile.razor: with the new default, dev-login (which stores no token) now lands on NeedsReauthorization rather than NotConfigured, showing .profile-locked plus a /login/start?...&returnUrl=%2Fprofile link — verified this branch and the link-building code already exist in Profile.razor (from an earlier merged PR) and match exactly what the updated profile.spec.js now asserts.
  • The profile.network hint move (top-of-page → directly above the editable fields, only rendered in the fully-editable branch) matches docs/brand.md's new voice section, and the old profile.lede key/CSS class is fully removed with no leftover references anywhere in the tree.
  • Copy, Program.cs's three-way startup log, Changelog (both locales), docs/auth.md, docs/brand.md, README, and all three compose/env surfaces were updated in lockstep — no stale none-as-default or old "nothing accepts it yet" reasoning left anywhere in the repo outside what this diff changes.
  • docs/auth.md's new callout that already-deployed stacks keep whatever their own .env says until it's edited and redeployed is accurate and appropriately scoped.

No correctness, security, or consistency issues found.


⏱ 1m 54s · 22 turns · tokens: 46.1k in / 7.2k out (+1466.5k cache-read)
📜 Full session transcript

✅ **Verdict: approve** — no blocking issues found. This supersedes my previous approval of a `generic`-default version of this PR — the branch has since been reworked around a new finding: `scopes_supported` was a red herring (it only lists the fixed, non-parameterized scopes; an unrecognized scope is silently dropped, not rejected), and a direct authorization-endpoint check shows the granular `repo:app.bsky.actor.profile blob:image/*` pair is actually accepted by both bsky.social and the bundled PDS. This is consistent with what's publicly documented about atproto's OAuth scopes rollout (granular permissions have been rolling out to bsky.social and self-hosted PDS through 2025–2026), so the central factual claim holds up. Traced the new logic end to end: - `ProfileScope.Parse`: `configured?.Trim().ToLowerInvariant()` correctly short-circuits `null` to `null`, and `.Trim()` reduces whitespace-only strings to `""`, so the new `null or "" => Granular` arm covers unset/empty/whitespace, while any *set-but-unrecognized* value still falls through to `None` — fail-towards-less-permission is preserved for typos, while "never configured" now gets the more useful default. `ProfileScopeTests` correctly splits this into two theories: opt-out/garbage → `None`, and null/""/whitespace → `Granular` with the exact expected `Requested` string. - `ProfileService`/`Profile.razor`: with the new default, dev-login (which stores no token) now lands on `NeedsReauthorization` rather than `NotConfigured`, showing `.profile-locked` plus a `/login/start?...&returnUrl=%2Fprofile` link — verified this branch and the link-building code already exist in `Profile.razor` (from an earlier merged PR) and match exactly what the updated `profile.spec.js` now asserts. - The `profile.network` hint move (top-of-page → directly above the editable fields, only rendered in the fully-editable branch) matches `docs/brand.md`'s new voice section, and the old `profile.lede` key/CSS class is fully removed with no leftover references anywhere in the tree. - Copy, `Program.cs`'s three-way startup log, `Changelog` (both locales), `docs/auth.md`, `docs/brand.md`, README, and all three compose/env surfaces were updated in lockstep — no stale `none`-as-default or old "nothing accepts it yet" reasoning left anywhere in the repo outside what this diff changes. - `docs/auth.md`'s new callout that already-deployed stacks keep whatever their own `.env` says until it's edited and redeployed is accurate and appropriately scoped. No correctness, security, or consistency issues found. --- ⏱ 1m 54s · 22 turns · tokens: 46.1k in / 7.2k out (+1466.5k cache-read) 📜 [Full session transcript](https://git.grandiras.net/Grandiras/claude-bot/src/branch/claude-logs/transcripts/Grandiras/Ponente/pr-142/run-368.md) <!-- claude-verdict:approve --> <!-- claude-reviewed-head:db1e6f0be51cf838c0dbfc8d4cb6443e1137656f -->
Grandiras deleted branch claude/issue-140 2026-08-21 14:33:16 +00:00
Sign in to join this conversation.
No reviewers
No labels
No milestone
No project
No assignees
2 participants
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference
Grandiras/Ponente!142
No description provided.