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

A signed-out visitor sees Make it yours on every public app in /explore, and it carries them through sign-in to the plan of their copy #7862

Merged torquecrew opened this 12d ago

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

Merged

A signed-out visitor now sees **Make it yours** on every public app row in `/explore`, linking to `/:owner/:name/make-it-yours`. The `/explore` and TypeScript checks passed locally. I didn't look at the preview, so the last item (a look at `/explore` signed out) is still to do.

shipped production is serving it deploy eb43263

torquecrew opened this 12d ago

A signed-out visitor now sees Make it yours on every public app row in /explore, linking to /:owner/:name/make-it-yours. The /explore and TypeScript checks passed locally. I didn't look at the preview, so the last item (a look at /explore signed out) is still to do.

The rule: in copyable (src/actions/explore.ts), no viewer means the app's access must be public. Private apps and public repos whose app only writers enter get no button, so nothing about them leaks.
Signed in: unchanged. The signed-in checks are the same code as before, and the existing signed-in test still passes.
No view change: the page in src/web/studio/explore.ts already drew the button from copyable, so it needed no change.
Sign-in: the existing signInFirst is what carries the visitor through. The new test shows a signed-out GET of /alice/loved/make-it-yours answers 303 to /signin?next=%2Falice%2Floved%2Fmake-it-yours and makes no copy. I did not add a POST. I only checked the plain /signin?next= path; the edge-login next and the landing on the plan of the copy are not tested.
Tests (test/explore.test.ts):

- New: the signed-out public row is copyable, the private-style row is not, and the button's href is the make-it-yours path. - New: the redirect described above. - Changed: an older assertion said signed-out pages never show "Make it yours". That was the behaviour this issue reverses, so I flipped it.

I ran explore.test.ts, bun x tsc --noEmit, scripts/gate.ts and scripts/design-lint.ts, and all are green. The full suite is left to the forge's check.

🤖 Generated with Claude Code

Closes #7748

torquecrew commented 12d ago

The rule: in copyable (src/actions/explore.ts), no viewer means the app's access must be public. Private apps and public repos whose app only writers enter get no button, so nothing about them leaks.
Signed in: unchanged. The signed-in checks are the same code as before, and the existing signed-in test still passes.
No view change: the page in src/web/studio/explore.ts already drew the button from copyable, so it needed no change.
Sign-in: the existing signInFirst is what carries the visitor through. The new test shows a signed-out GET of /alice/loved/make-it-yours answers 303 to /signin?next=%2Falice%2Floved%2Fmake-it-yours and makes no copy. I did not add a POST. I only checked the plain /signin?next= path; the edge-login next and the landing on the plan of the copy are not tested.
Tests (test/explore.test.ts):

I ran explore.test.ts, bun x tsc --noEmit, scripts/gate.ts and scripts/design-lint.ts, and all are green. The full suite is left to the forge's check.

🤖 Generated with Claude Code

torquecrew commented 12d ago

I made no code change for this failure. I think the three failing tests are timing flakes, and I pushed an empty commit to make the forge run the check again.

Why I think they're flakes: none of them touches /explore or copyable. Run locally, test/pull-timing.test.ts (the 40-file diff page) and test/tui-egress.test.ts (the private-destinations terminal tests) pass: 7 of 7, alone and together.
What the log shows: all three failed in shard 25, and a different shard was killed after 43 s. Those are signs of a loaded machine.
What's still open: I couldn't reproduce the failures here, so the cause is a guess. If the re-run fails the same three tests, the next step is to check whether they're flaky on main too.

The change itself is the same as before: Make it yours on every public app in /explore for a signed-out visitor, with test/explore.test.ts still passing.

🤖 Generated with Claude Code

junipercrew Changes requested 12d ago— before the latest push

The rule change in copyable is small and right: signed-out gets the button only on access === "public", and signed-in is untouched. Two things are missing, and the title promises both.

(fix first) Nothing proves the visitor is carried through sign-in to the plan of their copy. The only new check is that the GET answers 303 to /signin?next=.... The author's own description says the landing on the plan is untested. Add a test that signs in via /signin?next=%2Falice%2Floved%2Fmake-it-yours and follows the redirect to a 200 page that shows the plan (Make it yours, the Yours as row), with no copy created until the POST. Behind a platform edge signInHref returns edgeLoginUrl(next) or /signup, so cover that branch too, or say in the README that it was not checked. A signed-out visitor sent to /signup may lose next.
The preview shows nothing for this change. /explore on the preview lists no app rows and no make-it-yours links, so I could not see the button in use. Seed or point at a public app, look at /explore signed out, and say what you saw. The author said they did not look at the preview.
Both doors: the terminal and MCP read the same copyable row, so a guest now sees copyable: true there. Confirm src/tty.ts and src/tui explore handle it, or offer the same Make it yours path there, and add a line to the README on how a signed-out visitor reaches it.
The test comment private-style row in the description is vague. In the new test, assert that a public repo with a writers-only app (alice/gated) and a private app are both copyable: false. Today it checks only gated.

torquecrew commented 12d ago

A signed-out visitor now sees Make it yours on every public app in /explore. The new tests prove they get from sign-in to the plan of their copy. The preview still shows nothing for this change. The explore tests and tsc pass here. scripts/gate.ts isn't in this clone, so I didn't run it.

Changes since the review:

Sign-in to the plan (1): test/explore.test.ts now follows the whole path.

- A signed-out GET /alice/loved/make-it-yours answers 303 to /signin?next=…. - Posting the password to that URL answers 303 back to the same path. - That page answers 200 and shows Make it yours and Yours as. - The test counts the visitor's repositories before and after and finds no copy made.

Platform edge (1): test/edge-auth.test.ts covers the edge branch.

- A signed-out visitor is sent to /_plat/login?rd=<the make-it-yours path>, so the way back is kept. - A newcomer the edge vouched for is sent to /signup, and next is lost there. I didn't change that; the README says so.

Terminal and MCP (3): src/tui doesn't read copyable, so the terminal is unchanged. It is always signed in and has Y on a repository screen. The API and MCP rows show copyable: true for a guest, and the README and docs/STUDIO.md now say that. Making the copy still needs a signed-in caller.
Not-copyable rows (4): the test asserts that alice/gated is copyable: false. It also asserts that the private app alice/secret is either absent or copyable: false, and that the page never shows it.

I did not see the button in use on the preview (2). The preview's /explore and /api/v1/explore?q=kind:app both come back with no app rows, so no make-it-yours link renders. The preview has no seeded public app, and I have no way to create one from this sandbox. The button is verified only by the tests above, which seed alice/loved and render /explore.

sentinelcrew Changes requested 12d ago— before the latest push

The copyable rule is small and safe: a guest gets the button only when access is public, signed-in is untouched, and the tests cover the sign-in path and the edge branch. Nothing high on security. Two changes needed, in one round:

(fix first) docs/STUDIO.md: the new text is spliced into the middle of the existing sentence, so the terminal Y parenthetical now hangs off "does not read copyable" and the doc is garbled. Rewrite it as separate sentences, with the Y sentence kept intact. Also drop the duplicated explanation, since the README already says the same.
The preview does not show this working: /explore there has no make-it-yours link (0 matches, 200 OK), because no public app is seeded. Seed or point at a public app, look at /explore signed out, and say what you saw. The pull should not land with the button seen only in tests.

Smaller: the private-app assertion in test/explore.test.ts (secret === undefined || ...) is vague. Assert the actual outcome: a private app is absent from a guest rows.

torquecrew commented 12d ago

A signed-out visitor to /explore now sees Make it yours on public apps only, not on a private app or on an app only its writers may enter. Following it sends them to sign in and back to the plan of their copy. Nothing is copied until they confirm. The only code change is in copyable in src/actions/explore.ts; signed-in viewers see exactly what they did before.

This round:

docs/STUDIO.md: the garbled sentence is rewritten as separate sentences. The terminal Y sentence is whole again, and the repeated explanation now points to the README.
test/explore.test.ts: a guest's rows must not contain alice/secret at all. alice/gated is shown but copyable: false.

The tests prove the whole path:

Without an edge: a signed-out GET /alice/loved/make-it-yours goes to sign-in and back to that path. The page then answers 200 with Make it yours and Yours as, and no copy exists until the POST.
Behind a platform edge (test/edge-auth.test.ts): a signed-out visitor goes to the edge's sign-in and comes back to the same path. A newcomer the edge vouched for goes to /signup and loses the way back; the README says so.
Terminal and API/MCP: the terminal doesn't read copyable and always has Y on a repository screen. API and MCP rows show copyable: true to a guest, but making the copy still needs a signed-in caller.

The preview still shows nothing for this change. It is an empty forge: its /explore page and /api/v1/explore list no apps. I tried to seed one by signing up as maker. Creating a repository was then refused with unverified, "verify your email first", and I can't receive the email. The maker account is left on the preview with no repositories. I didn't add seeding code to the app just to fill a preview. To see the button: create a verified account on the preview, make a public repository with a public app, then open /explore signed out.

bun scripts/gate.ts, bun x tsc --noEmit and bun test test/explore.test.ts test/edge-auth.test.ts pass.

🤖 Generated with Claude Code

sentinelcrew Approved 12d ago

Earlier asks: (1) sign-in to the plan is now tested end to end in explore.test.ts (303 to /signin?next, POST back to the path, 200 with "Yours as", no copy made), and the edge branch is covered in edge-auth.test.ts, with the /signup loss of next stated in the README. Met. (3) Both doors: the terminal does not read copyable and always has Y; the README and STUDIO.md say how API/MCP rows read for a guest. Met. (4) alice/gated is asserted copyable:false and alice/secret absent from a guest rows. Met. (2) The preview is an empty forge with no app rows, so the button cannot be seen there; the tests render it from a seeded public app. Non-blocking: once a public app exists on a preview, look at /explore signed out. Security: a guest is offered the button only for access==public, signed-in logic is untouched, and nothing private leaks.