Repository navigation
Conversation
Railway and similar log viewers emphasize message text, so the generic webhook message hid the event type and provider unless operators expanded structured properties.\n\nInclude the normalized event type and provider directly in each accepted webhook message while retaining the existing structured fields.\n\nCo-authored-by: OpenAI <noreply@openai.com>
…ssage auth: clarify Resend webhook log messages
Quote the normalized event type and use provider metadata to preserve the lowercase structured ID while rendering the provider's human-readable name in log messages.\n\nCo-authored-by: OpenAI <noreply@openai.com>
…ssage-format auth: format Resend webhook event messages
The error banners on the OAuth login, account-login, and recovery forms were plain divs / paragraphs. When an error appeared (or updated) screen-reader users got no announcement — the error silently changed and the user kept thinking the form had accepted their input. Add ARIA live-region semantics so updates announce naturally: - Main login flash banner (#error-msg): role=status aria-live=polite (the polite intent is correct — errors should announce on update without interrupting whatever the user was doing) - Server-rendered <p class="error"> banners on account-login and recovery: role=alert (these are static at render time, so the default assertive announcement on first render is appropriate) Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The demo's sign-in error banner was a plain <div>. When an error appeared screen-reader users got no announcement — visual users saw the red banner, screen-reader users had no idea anything had changed. role="alert" matches the auth-service error banners in the previous commits (which got role=status / role=alert) and tells assistive tech to announce the message immediately. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The shared renderError() helper produces every styled HTML fallback page (Sign-in session expired, internal failures, session not found, …). The error message <p> was a plain paragraph; screen-reader users got no announcement when these pages loaded. role="alert" tells assistive tech to announce immediately on page render. Matches the role=alert treatment we just gave the inline server-rendered error banners on account-login and recovery. One-character behaviour change; no test fixtures depend on the exact HTML. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Buttons on the OTP form had :hover styles but no :focus-visible styles — so keyboard users tabbing through the form had no idea which button was focused (the browser default focus ring is suppressed by some user-agent stylesheets and the .btn-primary explicitly sets `border: none`). Add :focus-visible styles on .btn-primary and .btn-secondary that match the existing --focus-border CSS custom property the input fields already use, with a 2px outline + 2px offset so the ring sits clear of the rounded button edges. Pure CSS change; no behaviour change. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Same focus-visible treatment that the main login page got in the previous commit, applied to the standalone account-login form, the recovery form, and the handle-picker buttons. All three had :hover styles and no :focus-visible — keyboard users tabbing through had no idea where focus was. Pure CSS change. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
resolveClientMetadata returned `{ client_name: clientId }` when the
clientId failed to parse as a URL, or used a non-http(s) protocol.
The intent was probably "give the caller something to display" —
but the upshot was that a malformed query-string client_id ("Sign
in to not-a-url") or a schemeless one ("Sign in to javascript:…")
showed up verbatim in:
- the auth-service login page <title> tag
- the consent screen header
- the OTP form's "Sign in to <appName>" heading
— effectively letting the URL bar shape user-visible text.
Return `{}` from those branches instead, so the caller's
fallback chain (`client_name || extractDomain(clientId) ||
'an application'`) ends in the static "an application" string.
Tests: two existing assertions on the leaky behaviour are
inverted to assert the fallback path.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…s-rings P1: fix(a11y): show keyboard focus rings on auth controls
…plete P1: fix(auth): use accurate autocomplete hints
…n-back P1: fix(auth): clear email when returning from OTP entry
The handle picker now reports unavailable results accurately, but its CSS class and demo theme override still used taken terminology. Rename the selector, call site, and related comments so reserved or policy-rejected handles are not described as claimed. Co-authored-by: OpenAI <noreply@openai.com>
Adding role=status/aria-live to #error-msg was not sufficient on its own — the way showFlash mutated the region defeated it in two ways. The region is display:none until an error fires, and showFlash wrote textContent *before* setting display:block. A hidden element is excluded from the accessibility tree, so the text landed while the live region had no "before" state to diff against; assistive tech could legitimately announce nothing. Unhide first, then mutate. Assigning textContent in place also meant a repeated identical message was not a DOM mutation at all. Re-submitting a bad OTP yields the same "Invalid code" string, so the second failure was silent — precisely the "user thinks the form accepted their input" case this branch set out to fix. Build content off-DOM and swap it in via replaceChildren so every failure is a real mutation. Both concerns are now funnelled through one setFlash() helper, which also fixes the double-announcement in showErrorWithAction and the aborted-flow notice: each appended its action button *after* showFlash had already made the region live, so the message and its button announced as two separate updates. Building the whole content in a fragment makes each of them a single announcement. textContent remains the only sink for caller-supplied strings, so reflected better-auth errors still cannot be interpolated as HTML. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The choose-handle page was missed by the earlier commits in this
branch. Two separate gaps:
The server-rendered error banner used the same <div class="error">
pattern as account-login and recovery, so it gets the same
role=alert. Only the populated branch needs it — the empty
placeholder is never written to by this page's script.
More significantly, #handle-status had no live-region semantics at
all. It is the sole feedback channel for whether a handle is free
("Checking…" / "Available!" / "Already taken." / format errors) and
it gates the Create button via submitBtn.disabled. Screen-reader
users therefore got no availability feedback whatsoever, and hit a
button that had been silently disabled with no stated reason.
aria-live=polite rather than assertive because setStatus() fires on
every debounced keystroke; assertive would interrupt the user
mid-type. aria-describedby also associates the status with the input
so it is read on focus, not only on change.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Widens the existing changeset to match what the branch now does: handle selection is a newly covered surface, and repeated identical failures announcing each time is a user-visible behaviour change worth calling out separately from the initial role/aria additions. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…ror-announcements P0: fix(a11y): announce authentication errors
Without this change, addresses such as gmial.com and hotmal.com proceed directly to code delivery and leave users unable to receive their sign-in code. Add a conservative one-edit domain check for common providers. Both email entry points now pause before sending, offer the corrected address, and still allow the original address explicitly. Behavioral tests cover corrected and original sends. Refs HYPER-545. Co-authored-by: OpenAI Codex <noreply@openai.com>
…lable-copy P1: fix(auth): use truthful unavailable-handle copy
…-guard [HYPER-545] suggest corrections for mistyped email domains
--muted-foreground was #999, which carries the 13px terms line, the "Recover with backup email" link, and the "powered by Certified" footer. Measured against the two backgrounds those actually sit on: #999 on #F8F8F8 (card) 2.68:1 #999 on #E8E8E8 (page) 2.33:1 Both are far below the 4.5:1 WCAG AA minimum for normal-size text, and below even the 3.0:1 large-text threshold that does not apply at 13px anyway. Use #666 (5.41:1 and 4.69:1). #6b6b6b was the value originally proposed alongside the spam-folder hint in #221, but it clears AA only on the card: .powered-by lives in .page-wrap outside .container, so it renders on the page background where #6b6b6b reaches just 4.35:1 and still fails. #666 passes on both, and is a conventional shorthand. Extracted from #221, where it rode along as an undocumented global change. The hint that motivated it is gone — superseded by the resend-time message — but the contrast defect predates that PR and stands on its own. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
.divider ("or continue with", "or use email") hard-coded color: #999
at 13px instead of using the variable, so darkening
--muted-foreground left it behind at 2.68:1 — the same defect this
branch exists to fix, in the one place that had opted out of the
variable.
Point it at var(--muted-foreground). Side effect worth noting: the
divider now honours a branding.css override of --muted-foreground,
which it previously ignored.
Caught by Copilot in review of #232.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Both shots are of real Railway preview deployments rather than a local mock: the "before" is PR #229's preview, which still serves --muted-foreground: #999, and the "after" is this branch's own preview at #666. Captured on the OTP step so three of the variable's consumers are in frame at once — the terms line, the "Recover with backup email" link, and the "Powered by Certified" footer, the last of which sits on the page background rather than the card and is the reason #666 was needed over #6b6b6b. Computed colours at capture time confirm the swap reached every consumer: rgb(153,153,153) before, rgb(102,102,102) after. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The changeset filed the --muted-foreground override advice under
Operators, but branding.css is not an operator surface: it is a field
in the client's own client-metadata.json, and docs/configuration.md,
docs/tutorial.md and every prior CHANGELOG entry that mentions it
address client app developers.
An operator's only involvement is gating which client_ids may inject
CSS at all, via PDS_OAUTH_TRUSTED_CLIENTS — they never author the
file, so the adaptation instruction was pointed at the wrong reader.
Reattribute to Client app developers and reword accordingly ("your
override" rather than a third party's). Also note explicitly that a
value chosen to sit alongside the old #999 may itself be below AA,
which is the actionable part for anyone who did set it.
Raised by @aspiers in review of #232.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Applies the three review suggestions verbatim. Note this now differs from the other pending changesets, which start lowercase after the bold label. Worth settling one way in the skill so the generated changelog reads consistently.
The atproto upgrade in #233 changed both halves of the confirmation call. `confirmEmail` takes positional arguments and now also takes the address — `confirmEmail(did, email, token)` — and `Account.sub` became `Account.did`. `createEmailToken` takes a `DidString`. Upstream now compares the supplied address against the account's current one and throws `InvalidEmail` on a mismatch. That closes the TOCTOU gap CodeRabbit raised on this PR: pass the address whose control was actually proved and the check becomes atomic and upstream-enforced, rather than something this code has to re-derive. So thread the proved address through instead of only the DID: - the callback passes the HMAC-signed `email`, not the account's stored address, so a change between signing and confirmation is rejected; - the backfill passes each candidate's scanned address, so a row whose email moved between scan and write is reported as a failure rather than confirmed on stale evidence. The test fake records the address it receives, so a future signature change breaks these tests rather than passing silently. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…ception `String(err)` collapsed the error to its message, discarding the type and stack — exactly the context an operator needs when a one-off backfill dies. Use the repo's `createLogger` so pino serialises `err` in full. `process.exit(1)` is unchanged, so scripted runs still see the failure. The account enumeration this script does was defended in a code comment, but AGENTS.md states the account-manager boundary as an unqualified rule. Record the exception where the rule lives instead: `AccountManager` offers only `getAccount`, `getAccounts(dids)` and `getAccountByEmail`, all requiring the identifier up front, so there is no supported way to list every account. Noted as something to re-check on each `@atproto/pds` upgrade. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Three statements aimed at operators were inaccurate, all in ways that would mislead during the coordinated deploy this change requires. The signCallback docstring claimed the sentinel scheme keeps the payload "stable across releases". Adding required email_verified deliberately breaks that: the payload is positional, so signer and verifier must ship together. Say so, rather than implying releases are interchangeable. The changeset described a mixed pair as simply rejecting sign-in. pds-core now returns an explicit 400 naming the missing parameter, which is the difference between an operator recognising a mixed-version rollout and hunting a phantom secret mismatch. The changeset, docs/deployment.md and markEmailConfirmed's docstring all said existing accounts are repaired on their owner's next sign-in. Repair requires email_verified='1', so a sign-in by any other means leaves the account unconfirmed — which is exactly when the backfill is still needed. Also refreshed the PR-facing test count and the account-enumeration note now that the exception is recorded in AGENTS.md. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The exception was recorded only in AGENTS.md, but docs/design/pds-white-boxing.md is the catalogue for exactly this — reliance on internal, non-exported or undocumented upstream behaviour — and it already carries the instruction to check the list on every upstream bump. Recording a separate "re-check on each @atproto/pds upgrade" note in AGENTS.md duplicated a review trigger that already existed, in a place nobody consults during an upgrade. Add it as item 19 under High Risk, next to item 6 (the accountManager methods it bypasses), with the reason no supported alternative exists, the scope of the exposure, and what an upstream rename would break. AGENTS.md now states the rule and points at the catalogue rather than restating the rationale. Also correct item 2 while here: it claimed createAccount returns an Account with a `.sub` property. That broke in the 0.5.23 upgrade — the property is now `.did`, and every call site was updated. A catalogue of breakage risks is worth more if it records the breakages that already happened. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…onfirmedat-on-epds-account-after-otp-verified [HYPER-219] record emailConfirmedAt after OTP-verified sign-up
prepare for release v0.8.0
pds: upgrade upstream PDS and OAuth packages
pds/provider ui branding fix
…skill docs: simplify ePDS OAuth login skill
prepare for release v0.9.0
Upgrade dev.certified.app epds to v0.9.0
|
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configuration
You can disable this status message by setting the Use the checkbox below for a quick retry:
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
Review the following changes in direct dependencies. Learn more about Socket for GitHub.
|
|
Ashex
approved these changes
Oct 20, 2026
This branch was successfully deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.