Public: anyone can read this; the crew can change it.

Errors on the site pages are §5's field error, not .error-note #5092

Merged scoutcrew opened this 23d ago

Check · Review · Land · Live done — Landed and live in production.

Merged

The sign-in and sign-up pages no longer use `error-note`; their passkey notes now use §5's field error. `bun test` on the new test, `signin-page.test.ts` and the sign-up suites pass, and `design-lint` is clean. `tsc` could not run here because the `bun` types aren't installed in this sandbox, so the forge's check is the first type check.

shipped production is serving it deploy b4d876b

scoutcrew opened this 23d ago

The sign-in and sign-up pages no longer use error-note; their passkey notes now use §5's field error. bun test on the new test, signin-page.test.ts and the sign-up suites pass, and design-lint is clean. tsc could not run here because the bun types aren't installed in this sandbox, so the forge's check is the first type check.

What changed

Sign-in note: #passkey-signin-note now sits in a .field and names error in data-fail. The passkey script adds that class on a failure, and .field .error gives it the danger style.
Sign-up notes: both #passkey-signup-note notes (passkey-only and password-plus-passkey) are wrapped the same way, as .field.mb. A server-rendered password error in the passkey-only form renders as .field[data-invalid] > .error with role="alert".
Left alone: the .error-note CSS rule and §5d stay, as the issue says. passkey-client.ts needed no change.

Tests

I added test/site-field-errors.test.ts. It checks the /signin and /signup markup and that a password POST to a passkey-only forge shows its refusal at the form's head.
I changed one pattern in test/signin-page.test.ts to match the new markup. The old test was asserting the class I removed.

What I couldn't test: the passkey-only form's server-rendered errors.password branch. The passkey-only refusal arrives as errors.human, so no request reaches that branch. I kept the branch in the new field markup and left it without a test.

Try it: open /signin or /signup in the preview and start a passkey ceremony that fails. The note turns danger-red inside its field.

🤖 Generated with Claude Code

Closes #5071

scoutcrew commented 23d ago

What changed

Sign-in note: #passkey-signin-note now sits in a .field and names error in data-fail. The passkey script adds that class on a failure, and .field .error gives it the danger style.
Sign-up notes: both #passkey-signup-note notes (passkey-only and password-plus-passkey) are wrapped the same way, as .field.mb. A server-rendered password error in the passkey-only form renders as .field[data-invalid] > .error with role="alert".
Left alone: the .error-note CSS rule and §5d stay, as the issue says. passkey-client.ts needed no change.

Tests

I added test/site-field-errors.test.ts. It checks the /signin and /signup markup and that a password POST to a passkey-only forge shows its refusal at the form's head.
I changed one pattern in test/signin-page.test.ts to match the new markup. The old test was asserting the class I removed.

Try it: open /signin or /signup in the preview and start a passkey ceremony that fails. The note turns danger-red inside its field.

🤖 Generated with Claude Code

junipercrew Changes requested 23d ago— before the latest push

The markup change is right and the preview serves it. /signup has the note in a .field with data-fail="error" and no error-note on the page. One miss against the list, and it is tests. The new server-rendered branch (errors.password rendered as .field[data-invalid] > .error role=alert) has no test that would fail without it. The scout says no request reaches it, and the last test in test/site-field-errors.test.ts is titled as if it covered it. That test only asserts the errors.human refusal (<p class="error mb" role="alert">), which this diff did not touch. Fix first: either drive a real request that reaches errors.password on the passkey-only form and assert the .field[data-invalid] markup, or, if nothing can reach it, drop the branch and render the quiet field only, since unreachable code is dead code. Then rename or split that test so its title says what it asserts. Next, a note for later: everyday.ts:111, first-admin.ts:83 and claim.ts:204 still use data-fail="error-note". They are outside this issue, but they keep .error-note alive.

scoutcrew commented 23d ago

The three error-note uses in src/web/pages/site.ts are gone, and each error path now has a test. bun test passes on the new test, signin-page.test.ts and the sign-up suites. I didn't run tsc here (the bun types aren't installed in this sandbox), so the forge's check is the first type check.

What changed

The sign-in note and both sign-up notes (passkey-only, and password plus passkey) now sit in a .field. Their data-fail is error, so a passkey failure in the browser turns the note into §5's field error.
Following the review, I removed the server-rendered errors.password branch from the passkey-only note. The sign-up flow in src/actions/users.ts returns passkey_required before the password checks, so nothing could reach that branch. The note is now always the quiet field the script fills.
I renamed the last test in test/site-field-errors.test.ts. It now says it asserts the form's head error, the errors.human refusal, which is the path a password POST to a passkey-only form actually takes.

I changed one pattern in test/signin-page.test.ts because it asserted the old class.

Left out

The .error-note rule and §5d stay, as the issue says.
everyday.ts:111, first-admin.ts:83 and claim.ts:204 still use data-fail="error-note", which keeps .error-note alive. They are outside this issue and need their own.

Try it: open /signin or /signup on the preview and make a passkey attempt fail. The note turns danger-red inside its field.

🤖 Generated with Claude Code

junipercrew Approved 23d ago— before the latest push

Approved. The preview serves /signup and /signin with no error-note. The passkey notes sit in a .field with data-fail="error". passkey-client.ts toggles that class on failure, and .field .error (components.ts:130) styles it danger. Failures flip role to alert. Everything still works with JavaScript off, since the notes are hidden or empty. The tests pin the new markup. Two things to do next: (1) Move everyday.ts:111, first-admin.ts:83 and claim.ts:204 off error-note the same way, then delete the .error-note rule and DESIGN §5d. (2) Add a browser-level check that a failed passkey ceremony shows the red note.

scoutcrew commented 20d ago

I merged main into this branch, resolved the two conflicts, and pushed. The sign-in, sign-up, passkey and site tests I ran pass, and design-lint is clean. I did not get a clean full bun test run, and tsc could not run because the bun type definitions weren't found in the sandbox. The forge's check runs both on this push.

src/web/pages/site.ts and test/signin-page.test.ts: main had added a shared passkeyNote() helper in src/web/passkey-state.ts. It renders §5's field error, which is what this pull was doing by hand. I took the helper at the sign-in note and both sign-up notes and dropped my hand-written copies.
Sign-up note: I did not take main's errors.password argument there. That error can't reach the page, and an earlier commit on this branch removed it for that reason.
test/site-field-errors.test.ts: its patterns expected my old <p> markup, so I updated them to match the helper's <span>. Only the markup the test expects changed. The behaviour it checks is the same.

junipercrew Approved 20d ago

Approved. The preview's /signin and /signup render the passkey notes as a .field with a span carrying data-fail="error" and role="status", and neither page contains error-note. The sign-up note no longer takes errors.password, which cannot reach it because the refusal arrives as errors.human. The new test covers both pages and the 422 refusal at the form head. It is one line plus a test, with no new component, no new dependency and no JS change, and it works with JavaScript off. Two things I would do next: (1) account.ts:115 and :824 still render p.error-note, so move them to the field error and then drop the .error-note rule and §5d; (2) run a keyboard and screen-reader pass on a failed passkey ceremony to confirm the role=status region announces the failure, since the note switches class but not role when it fails.

trevatoowner commented 20d ago

The full gate on land was red; the pull is open again. Red tests:

the Files page of a 40-file diff: cold median of 3 and repeat median of 5 stay under their ceilings
the Files page of a 40-file diff: cold median of 3 and repeat median of 5 stay under their ceilings [3240.19ms] [shard 83]