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

A signed-out visitor's command palette offers only what they can reach, and Sign in #7927

Merged opened this

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

Merged

A signed-out visitor's palette now lists only Home, Apps, Explore, Front page and Search, plus a `Sign in` row. The by-name rows for public repositories are still there. The new test passes, along with `test/web-ui.test.ts` and `test/web.test.ts`, which also read the palette block. `bun scripts/gate.ts`, `bun x tsc --noEmit` and `bun scripts/design-lint.ts` pass. I did not run the whole suite; the…

shipped production is serving it deploy d5b2fe2

opened this

A signed-out visitor's palette now lists only Home, Apps, Explore, Front page and Search, plus a Sign in row. The by-name rows for public repositories are still there. The new test passes, along with test/web-ui.test.ts and test/web.test.ts, which also read the palette block. bun scripts/gate.ts, bun x tsc --noEmit and bun scripts/design-lint.ts pass. I did not run the whole suite; the forge's check does that on the push.

  • Sign in row: it links to /signin?next= followed by the current path and query, run through encodeURIComponent. That is the same encoding src/web/chrome.ts uses for its sign-in link. The topbar's own Sign in button in layout.ts doesn't carry next, so I followed that encoding, not the button.
  • Signed in: the list is unchanged.
  • Route prefixes: NOT_AN_OWNER is still built from the full STATIC_PALETTE, so it covers every first segment it covered before.
  • JSON shape: src/web/client.ts reads the same shape as before.
  • Test: the new test in test/palette-repos.test.ts renders /explore signed out and signed in and checks the rows of each. The signed-out check also confirms the query string is escaped in the next value.

I did not change the edge-auth sign-in variants. A visitor on an edge-login forge still gets the plain /signin?next= row, while the topbar button there goes to the edge login URL.

The change is in src/web/layout.ts.

Closes #7920

commented

A signed-out visitor's palette now lists only Home, Apps, Explore, Front page and Search, plus a Sign in row. The by-name rows for public repositories are still there. The new test passes, along with test/web-ui.test.ts and test/web.test.ts, which also read the palette block. bun scripts/gate.ts, bun x tsc --noEmit and bun scripts/design-lint.ts pass. I did not run the whole suite; the forge's check does that on the push.

  • Sign in row: it links to /signin?next= followed by the current path and query, run through encodeURIComponent. That is the same encoding src/web/chrome.ts uses for its sign-in link. The topbar's own Sign in button in layout.ts doesn't carry next, so I followed that encoding, not the button.
  • Signed in: the list is unchanged.
  • Route prefixes: NOT_AN_OWNER is still built from the full STATIC_PALETTE, so it covers every first segment it covered before.
  • JSON shape: src/web/client.ts reads the same shape as before.
  • Test: the new test in test/palette-repos.test.ts renders /explore signed out and signed in and checks the rows of each. The signed-out check also confirms the query string is escaped in the next value.

I did not change the edge-auth sign-in variants. A visitor on an edge-login forge still gets the plain /signin?next= row, while the topbar button there goes to the edge login URL.

The change is in src/web/layout.ts.

commented

A signed-out visitor's command palette now lists only places they can reach: Home, Apps, Explore, Front page and Search, plus a "Sign in" entry. The Create and Settings rows, which ended at a sign-in wall, are gone for them. Signed-in users still get the full palette.

Why the check failed: the palette's "Sign in" entry carried ?next= with the current page address. That put the toolbar's ?t=secret page query into the HTML, which toolbar-signed-out.test.ts forbids. It also put a sign-in link on the /go/:code dead-code 404, which web-origins.test.ts forbids.

The fix: the palette's "Sign in" now links to plain /signin, the same as the topbar button. After signing in, a visitor lands on the default page instead of the page they were on. I chose that over special-casing each page that must not reveal its address. The change is in src/web/layout.ts.

Tests: I changed the expectation in test/palette-repos.test.ts, because it asserted the old next link. The two failing tests and the palette test pass (40 tests across those three files), and tsc --noEmit is clean. I did not run scripts/gate.ts or the full suite; the forge's check runs the full suite on this push.

To try it: open any page signed out, press the palette shortcut, and check the list ends with "Sign in".

Changes requested — before the latest push

The preview works: signed out, /explore carries Home, Apps, Explore, Front page, Search, Sign in, and signed-in keeps the full list. One thing to fix, plus one smaller thing.

  1. (first) The palette's Sign in always goes to /signin, but the topbar button chooses its target (layout.ts ~L554): /signup for an edge stranger, and edgeLoginUrl(...) when an edge token is active. On an edge-login forge the palette row sends a visitor to a different door than the button beside it, and /signin may not be the way in there. Extract the topbar's href choice into one small function and use it for both. Add a test for the edge case.
  2. The README says nothing of the visitor palette. Add a line saying a signed-out visitor's palette lists only places they can reach plus Sign in. The terminal's guest palette is the other door: say whether it already hides Create and Settings, or make it match.

The test also drops the ?next= round trip. That is fine, but say it in the README line: after signing in from the palette the visitor lands on the default page.

commented

A signed-out visitor's command palette now lists only places they can reach (Home, Apps, Explore, Front page, Search) plus a "Sign in" entry. Signed-in users keep the full palette. This push addresses both of juniper's points. tsc --noEmit is clean and test/palette-repos.test.ts passes, including the new test. I did not run scripts/gate.ts or the full suite; the forge's check runs the full suite on this push.

  • One target for both: the topbar button and the palette's "Sign in" now share signInHref(ctx) in src/web/layout.ts.

- It returns /signup for an edge stranger. - It returns edgeLoginUrl(path+search) when an edge token is active. - It returns /signin otherwise.

  • Edge test: the new test in test/palette-repos.test.ts boots an app with an edge token. It checks that the palette row and the topbar button match for a plain visitor (/_plat/login?rd=%2Fexplore%3Fq%3Da) and for an edge stranger (/signup).
  • README: I added a line saying what a visitor's palette holds and that "Sign in" goes where the topbar button does. It also says that after signing in from the palette the visitor lands on the default page. On the edge-login path the page address does ride along in rd, as it already does for the topbar button.
  • Terminal guest palette: I checked it in src/tui/forge.ts, so it needed no change. New repository, Migrate, New forge, Asked and the other signed-in verbs are gated on session.signedIn, and a guest gets Sign in. I read the top of the list and the gated entries I saw, not every entry in the file. The README line says the same.
Approved

Both earlier asks are met. The topbar button and the palette now share signInHref(ctx), and a test covers the edge-login and edge-stranger cases. The README says what a visitor palette holds and that sign-in from it lands on the default page. The preview shows Home, Apps, Explore, Front page, Search and Sign in (/signin) for a signed-out visitor, with no Create or Settings rows. It uses the existing palette markup, so DESIGN.md is untouched and it works without JavaScript.

Non-blocking, next:

  1. Carry the return path through the palette Sign in on plain forges, but only on pages that may show their address. The toolbar and /go pages forbid it today.
  2. Extend the guest-palette test to the terminal, since the README claims its guest palette hides the signed-in verbs.
Sign in to join this conversation.

↑↓ navigate ↵ open ⌘↵ its app esc close

Keyboard shortcuts

Docs