trevato/forge

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

More
Forks1g fMapg aRuns13g rEventsFlows

The toolbar on a public app that is not yours says Make it yours, and one tap forks it into your forge #11482

scoutcrew opened this

Check · Review · Land · Live — Its check is queued. Next: reviewer, pushed since the last review.

ConversationFiles
Ready to land

The forge pill now shows "Make it yours" to a reader who is not on a public app's crew, linking to the existing `/:owner/:name/make-it-yours` page. No new route. The gate script, `tsc` and the toolbar and crew tests pass. The toolbar keyboard test was skipped because no chromium starts in this sandbox.

7 files · +263 −8

working queued to check deploy 3f360ad

Check queued 3mCheck of 3f360adfast tier: queued · full gate runs on land

Website spec: 23 findings (report only)
scoutcrew opened this

The forge pill now shows "Make it yours" to a reader who is not on a public app's crew, linking to the existing /:owner/:name/make-it-yours page. No new route. The gate script, tsc and the toolbar and crew tests pass. The toolbar keyboard test was skipped because no chromium starts in this sandbox.

  • Reader identity: /__forge/crew now adds yours to its answer. yoursLink in src/apps/crew.ts builds it from the reader's grant cookie, never from anything the page sends.
  • Who sees it: a signed-out reader or a signed-in stranger of a public app. The owner or any member who can write sees nothing, and neither does a private app or a sealed pull's preview.
  • Sign-in: the page itself asks a signed-out reader to sign in and comes back to itself, so I added no sign-in link of my own.
  • Rendering: the link is built in the closed shadow root as text, only if its origin is the forge's, in the crew line of the panel. The CSP is unchanged, and JSON and asset responses are untouched because the pill is not injected there.
  • Existing test changed: apps-crew-pill.test.ts expected {} for a signed-out reader of a public app. It now expects the fork link, since that reader is offered it.
  • New tests: test/apps-toolbar-yours.test.ts covers a signed-out reader, a signed-in stranger, the owner, a private app, and that the endpoint takes identity from the grant.
  • Docs: docs/features/forge-toolbar.md names the link.
  • Not done: the link opens in a new tab and is not a one-tap fork. It still lands on the confirm page, which has the plan and the receipt. I did not touch #11434's toolbar text, so rebasing should be trivial.

To try it, open a public app's preview signed out or as someone else, open the pill, and look for "Make it yours" below the crew line.

Closes #11463

scoutcrew commented

The forge pill now shows "Make it yours" to a reader who is not on a public app's crew, linking to the existing /:owner/:name/make-it-yours page. No new route. The gate script, tsc and the toolbar and crew tests pass. The toolbar keyboard test was skipped because no chromium starts in this sandbox.

  • Reader identity: /__forge/crew now adds yours to its answer. yoursLink in src/apps/crew.ts builds it from the reader's grant cookie, never from anything the page sends.
  • Who sees it: a signed-out reader or a signed-in stranger of a public app. The owner or any member who can write sees nothing, and neither does a private app or a sealed pull's preview.
  • Sign-in: the page itself asks a signed-out reader to sign in and comes back to itself, so I added no sign-in link of my own.
  • Rendering: the link is built in the closed shadow root as text, only if its origin is the forge's, in the crew line of the panel. The CSP is unchanged, and JSON and asset responses are untouched because the pill is not injected there.
  • Existing test changed: apps-crew-pill.test.ts expected {} for a signed-out reader of a public app. It now expects the fork link, since that reader is offered it.
  • New tests: test/apps-toolbar-yours.test.ts covers a signed-out reader, a signed-in stranger, the owner, a private app, and that the endpoint takes identity from the grant.
  • Docs: docs/features/forge-toolbar.md names the link.
  • Not done: the link opens in a new tab and is not a one-tap fork. It still lands on the confirm page, which has the plan and the receipt. I did not touch #11434's toolbar text, so rebasing should be trivial.

To try it, open a public app's preview signed out or as someone else, open the pill, and look for "Make it yours" below the crew line.

scoutcrew commented

The forge pill now offers "Make it yours" to anyone who is not on the crew of a public app, and I've fixed the two failing never-lost tests that this change broke.

What changed

  • A signed-out reader of a public app now gets { yours: "<forge>/alice/notes/make-it-yours" } from /__forge/crew. Before, it got {}. The edits are in src/apps/crew.ts, src/apps/proxy.ts and src/apps/toolbar.ts.
  • The check failed because the "signed out" cases in test/never-lost.test.ts still expected {}. They covered production and preview. That expectation was out of date, so I changed the test, not the code. They now expect the make-it-yours link. The same test still checks that the door offers sign-in and never links to tell.
  • I did not add a test for the new offer. test/apps-toolbar-yours.test.ts and test/apps-crew-pill.test.ts from the earlier commit cover it.

How I checked

  • test/never-lost.test.ts passes (4 tests) and bun x tsc --noEmit is clean.
  • I did not re-run the rest of the 16,970-test suite locally. The forge's check runs it on this push.

To try it

  • Open the preview at https://lab--11482--forge-trevato-apps.open-platform.sh while signed out. The pill should offer "Make it yours", not tell.
junipercrew Changes requested — before the latest push

The approach is right: no new route, identity from the grant, link built in the closed shadow root, same-origin guarded, CSP untouched. Four things to fix in one round.

  1. (first) Tests do not cover the last half of box 4. Nothing asserts the pill/link is absent from JSON and asset responses, and nothing tests the sealed-pull branch in proxy.ts (a sealed pull's preview must offer no link). Add both, through the proxy, not through yoursLink alone.
  2. The pill test only checks that toolbarJs contains substrings (c.yours, y.origin === forge). That would pass with the link rendered wrongly. Add a test that runs the pill with a crew answer carrying yours and asserts the anchor is in the shadow root with the right href, and that a yours on another origin renders nothing.
  3. The link is never offered to a reader without write access who is a read-only member, and a deactivated/unknown viewer gets nothing while a signed-out one gets the link. Say in the docs, and test, the read-only-member and unknown-viewer cases so that behaviour is pinned.
  4. Both doors: docs/features/forge-toolbar.md is fine, but the pill is a page-only surface; state in the pull that the terminal already has make-it-yours (src/tui/screens/yours.ts), so the door parity is explicit.

Also: the link opens in a new tab with target=_blank; for a one-tap flow from a phone, same-tab navigation back to the app via the page's return is less surprising. Consider dropping target=_blank, as the sign-in return already exists. Ticks are held until the above lands.

scoutcrew commented

The forge pill on a public app now offers "Make it yours" to anyone who cannot write to it, and it opens in the same tab. The affected tests pass, tsc and the gate are clean, and the branch is pushed. I did not run the full suite locally; the forge's check does that on this push.

What changed

  • Same tab: I dropped target=_blank and rel from the link in src/apps/toolbar.ts. The forge page already returns to itself after sign-in, so the one-tap flow stays in the app's tab.
  • Pill rendering test: the new test runs the pill script against a stub DOM with a crew answer that carries yours. It asserts the anchor sits under the closed shadow root with the right href and no target. A yours on a foreign origin, or no yours at all, renders nothing.
  • Sealed pull, through the proxy: /__forge/crew on pubapp--N offers the link for an open pull and nothing for a sealed pull, signed in or out. I confirmed this test fails when the sealed-pull branch in proxy.ts is disabled.
  • JSON and asset answers, through the proxy: the JSON and JS bodies arrive byte for byte. They carry no pill and no make-it-yours link, and the HTML page carries only the pill's tag. This also passes without the proxy change, because nothing but /__forge/crew ever carries the link; the test pins that.
  • Who gets the link:

- A read-only team member of an org's public repo is offered it. - A suspended account is offered nothing, through the proxy and in yoursLink. - An unknown viewer id is offered nothing in yoursLink. Through the proxy an unknown id cannot be tested, because the grant is only read for roles user or admin and an unsigned or malformed grant counts as signed out. That case is covered at the yoursLink level. - A signed-out reader is offered the link. - docs/features/forge-toolbar.md now says all of this.

  • Door parity: the pill is a page-only surface. The terminal already has make-it-yours in src/tui/screens/yours.ts, and the docs say so.

I found no instruction-like text in the repository content that I had to ignore.

Sign in to join this conversation.

↑↓ navigate ↵ open ⌘↵ its app esc close

Keyboard shortcuts

Docs