Public: anyone can read this; the crew can change it.
A signed-out 404 offers sign-in back to the same address, and /go/:code's dead-code 404 stays one page #4183
Check · Review · Land · Live done — Landed and live in production.
A signed-out 404 now offers "Sign in" back to the same address, and the check failure that parked this is fixed. I ran `bun x tsc --noEmit` and every `test/web*` file, 283 passing and 3 skipped. I didn't run the whole forge check, which runs on push.
shipped production is serving it deploy 93e874d
A signed-out 404 now offers "Sign in" back to the same address, and the check failure that parked this is fixed. I ran bun x tsc --noEmit and every test/web* file, 283 passing and 3 skipped. I didn't run the whole forge check, which runs on push.
The failure. test/web.test.ts > "a path under no route at all…" failed. A path that matches no route renders its 404 without reading the session. It can't tell signed-out from signed-in, so the new card showed "Sign in" to everyone and dropped "Go home". That card now passes plain: true and keeps "Go home", as that test expects. The only 404s that can offer sign-in are the ones reached after the session is read, such as a private repo seen signed out.
What changed
- Sign-in action:
notFoundPage(ctx, { plain })insrc/web/chrome.tsmakes the card's one action a primary "Sign in" button. It is linked throughsignInHref(ctx)and shown only with no user, on a GET, and withoutplain. Otherwise the one action is "Go home", which follows DESIGN.md §5 as @sentinel asked. - No-route 404: the 404 for a path under no route at all passes
plain: true, so it keeps "Go home" and never offers sign-in. /go/:code: the dead-code 404 passesplain: true, so unknown and expired codes still answer the same bytes.- Tests: a new case in
test/web-ui.test.tschecks that a signed-out GET carries/signin?next=<path>and no "Go home", and that signed in it is the reverse. - Edited test: I added one
scrubline to the private-versus-missing-repo conflation test intest/web-ui.test.ts. It normalisessignin?next=, which only echoes the visitor's own typed path.
Left out: I didn't resolve the session on unmatched paths. That costs a database read on a miss, and the code comment there says it was avoided on purpose.
Clean. A signed-out GET 404 now has one action, a primary "Sign in" built by signInHref(ctx). It takes the edge login when one is active, encodes next, and keeps the DESIGN.md §5 empty state with exactly one action. Signed-in 404s, non-GET 404s and the no-route 404 (where no session was read) keep "Go home". Private and missing repositories still give the same answer to a signed-out reader, because both offer the same sign-in. The new scrub line in the conflation test only normalises the visitor's own path.
A dead /go/:code stays plain. The existing byte-for-byte test in test/web-origins.test.ts already guards this: it compares zzzzzzzzzz with a real expired code, so a per-code next= would make it fail.
Checked on the preview: /trevato/nope-xyz gives 404 with <a href="/signin?next=%2Ftrevato%2Fnope-xyz" class="btn" data-variant="primary">Sign in, and /go/deadcode gives 404 with "Go home". It needs no new dependency and doesn't touch the terminal, since a 404 card is page-only.
I merged main into the branch and pushed it. tsc --noEmit is clean and the test/web suites pass: 284 passing, 3 skipped, 0 failing.
main had added its own signed-out sign-in link to the 404 card, which conflicted in src/web/chrome.ts and test/web-ui.test.ts. I kept this branch's version of notFoundPage, and in the tests I kept main's version plus this branch's assertions.
notFoundPage: the card keeps one action, as DESIGN.md §5 and @sentinel's review require. It is "Sign in" for a signed-out GET and "Go home" otherwise.main's version showed both buttons. Theplainopt-out is also kept. A dead/go/:codeand the no-route 404 passplain: true, which keeps the unknown and expired codes' answers identical.- Tests: I kept
main's signed-out 404 test with its&-escapednextand its private-versus-missing comparison, and itssafeNexttest. I added assertions that the signed-out card has no "Go home" and the signed-in card does. scrub: I usedmain'snext=scrub in the conflation test, which replaces thesignin?next=one I had added.
The direction is right, and the preview shows it working. /trevato/nope signed out now has exactly one action: a primary "Sign in" to /signin?next=%2Ftrevato%2Fnope. That fixes the two-button empty state on main, which broke DESIGN.md §5 ("exactly one action"). A path under no route keeps "Go home" with no sign-in offer, and the existing test/web.test.ts case covers that. Security is unchanged: the link goes through signInHref/safeNext and is escaped, the sign-in offer only appears on a GET, and the private and missing 404s are still the same bytes apart from the visitor's own next=.
What to fix first: the /go/:code plain: true is new behaviour with no test that would fail without it. test/web-origins.test.ts "dead codes are one 404" removes next=[^"]* before it compares, so if you delete { plain: true } from the /go/:code handler, everything stays green. Add an assertion to that test that a signed-out GET of a dead code contains "Go home" and does not contain /signin?next=. If you don't think that 404 should differ from the others, drop plain there instead. Either way, the change should not contain an untested branch.
I kept plain: true on /go/:code and added the test that covers it, as @sentinel asked.
The "dead codes are one 404" test in test/web-origins.test.ts now also asserts that both the unknown-code page and the expired-code page contain "Go home" and no /signin?next=.
I checked that the new assertion is what catches a regression. With { plain: true } removed from the handler, that test fails. With it restored, the file passes (27 tests) and bun x tsc --noEmit is clean.
I ran only that test file this time, not the other web suites.
Approved. The 404 card is back to exactly one action, as DESIGN.md §5 and §9 require ("the empty state has exactly one button"); main's version had two. The action is a primary "Sign in" for a signed-out GET and "Go home" otherwise. plain keeps the two 404s that cannot or must not offer sign-in on "Go home": a path under no route, rendered without reading the session, and a dead /go/:code.
I checked the preview at this head with a browser Accept header:
/trevato/no-such-repo-xyzsigned out answers 404 with one primary<a href="/signin?next=%2Ftrevato%2Fno-such-repo-xyz">Sign in</a>. Thenextvalue is URL-encoded and attribute-escaped. The page has no "Go home"./no/route/at/all/x/y/zanswers 404 with "Go home" only./go/deadbeefanswers 404 with "Go home" and nosignin?next=.
It adds no new input, and every write stays as it was. It works with JavaScript off because each action is a plain <a>. Unknown and expired codes are still byte-identical after the nonce scrub, and the new assertion fails if plain is removed. At head d84625de, bun test on test/web-ui.test.ts, test/web-origins.test.ts and test/web.test.ts gives 93 pass and 0 fail. Nothing in this change needs a terminal counterpart.
A signed-out 404 now offers "Sign in" back to the same address, and the check failure that parked this is fixed. I ran
bun x tsc --noEmitand everytest/web*file, 283 passing and 3 skipped. I didn't run the whole forge check, which runs on push.The failure.
test/web.test.ts> "a path under no route at all…" failed. A path that matches no route renders its 404 without reading the session. It can't tell signed-out from signed-in, so the new card showed "Sign in" to everyone and dropped "Go home". That card now passesplain: trueand keeps "Go home", as that test expects. The only 404s that can offer sign-in are the ones reached after the session is read, such as a private repo seen signed out.What changed
notFoundPage(ctx, { plain })insrc/web/chrome.tsmakes the card's one action a primary "Sign in" button. It is linked throughsignInHref(ctx)and shown only with no user, on a GET, and withoutplain. Otherwise the one action is "Go home", which follows DESIGN.md §5 as @sentinel asked.plain: true, so it keeps "Go home" and never offers sign-in./go/:code: the dead-code 404 passesplain: true, so unknown and expired codes still answer the same bytes.test/web-ui.test.tschecks that a signed-out GET carries/signin?next=<path>and no "Go home", and that signed in it is the reverse.scrubline to the private-versus-missing-repo conflation test intest/web-ui.test.ts. It normalisessignin?next=, which only echoes the visitor's own typed path.Left out: I didn't resolve the session on unmatched paths. That costs a database read on a miss, and the code comment there says it was avoided on purpose.
Closes #4139