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

A Deploy to… page and link for a pinned forge (follow-up to #8697) #8794

Merged scoutcrew opened this 11d ago

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

Merged

This adds the `/deploy-to` page and the `Deploy to…` link for a pinned forge. I pushed it; the forge's check hasn't run yet.

shipped production is serving it deploy eb43263

scoutcrew opened this 11d ago

This adds the /deploy-to page and the Deploy to… link for a pinned forge. I pushed it; the forge's check hasn't run yet.

Page: a forge admin gets a server-rendered form with cloud account, app, source, version and name. A link like /deploy-to?app=owner/name&source=https://…&version=v1.2.3 fills the fields in and deploys nothing until the button is pressed. The link is built by deployToHref in src/web/pinned.ts.
Sign-in: signed out, the link redirects through one sign-in and comes back to the same link. A signed-in non-admin gets a 404.
Deploy: the POST checks same-origin and the admin token scope, then calls the same deployPinned as the API, MCP and terminal, so the checks and words are the same. A refusal re-renders the form with the reason (422) and keeps what was typed. Success redirects back with a flash.
Reach: the page is linked from Settings → Cloud accounts as "Deploy a pinned forge". deploy-to is added to the reserved names so nobody can claim it as a username.
Docs: docs/features/pinned-forge.md now describes the web door and no longer lists #8744 under "Not done".
MCP: no new tool and no new dependency.
Tests: test/pinned-page.test.ts covers the sign-in round trip, the link prefill with escaping, the non-admin 404, the cross-origin 403, the 422 on a bad version, and a successful deploy. This test plus the pinned-forge, pinned-recovery, mcp-size, reserved-names, design-lint and cloud tests pass, and bun x tsc --noEmit is clean.

Not done:

The form deploys to DigitalOcean only, because it is the only target that deploys today. Accounts with writes off are not offered, and the page says where to turn them on.
No result URL is shown on success, because the droplet address isn't read back yet (#8745).
scripts/fast.ts found the diff too broad (1559 of 1928 test files), so I ran only the test files above.
bun scripts/gate.ts and the design lint printed no failures. I started both in one command that timed out and moved to the background, and the output file ended at the design-lint line, so the gate's own verdict isn't confirmed.

To try it, open /deploy-to?app=alice/shop&source=https://forge.example.org/trevato/forge&version=v1.0.0 as an admin.

Closes #8744

scoutcrew commented 11d ago

This adds the /deploy-to page and the Deploy to… link for a pinned forge. I pushed it; the forge's check hasn't run yet.

Page: a forge admin gets a server-rendered form with cloud account, app, source, version and name. A link like /deploy-to?app=owner/name&source=https://…&version=v1.2.3 fills the fields in and deploys nothing until the button is pressed. The link is built by deployToHref in src/web/pinned.ts.
Sign-in: signed out, the link redirects through one sign-in and comes back to the same link. A signed-in non-admin gets a 404.
Deploy: the POST checks same-origin and the admin token scope, then calls the same deployPinned as the API, MCP and terminal, so the checks and words are the same. A refusal re-renders the form with the reason (422) and keeps what was typed. Success redirects back with a flash.
Reach: the page is linked from Settings → Cloud accounts as "Deploy a pinned forge". deploy-to is added to the reserved names so nobody can claim it as a username.
Docs: docs/features/pinned-forge.md now describes the web door and no longer lists #8744 under "Not done".
MCP: no new tool and no new dependency.
Tests: test/pinned-page.test.ts covers the sign-in round trip, the link prefill with escaping, the non-admin 404, the cross-origin 403, the 422 on a bad version, and a successful deploy. This test plus the pinned-forge, pinned-recovery, mcp-size, reserved-names, design-lint and cloud tests pass, and bun x tsc --noEmit is clean.

Not done:

The form deploys to DigitalOcean only, because it is the only target that deploys today. Accounts with writes off are not offered, and the page says where to turn them on.
No result URL is shown on success, because the droplet address isn't read back yet (#8745).
scripts/fast.ts found the diff too broad (1559 of 1928 test files), so I ran only the test files above.
bun scripts/gate.ts and the design lint printed no failures. I started both in one command that timed out and moved to the background, and the output file ended at the design-lint line, so the gate's own verdict isn't confirmed.

To try it, open /deploy-to?app=alice/shop&source=https://forge.example.org/trevato/forge&version=v1.0.0 as an admin.

scoutcrew commented 11d ago

Main is merged into the branch and pushed. I haven't seen the forge's check run on this push yet.

Conflict: the only one was the "Not done" list in docs/features/pinned-forge.md. I kept main's wording about the canary (#8746) and dropped the "No Deploy to… page" line, since this pull adds the page.
Earlier check failure: test/forge-looks.test.ts failed because the new /deploy-to route wasn't declared. The form is admin-only and fills from its link, so I listed it in that test's not-a-page list, with test/pinned-page.test.ts as its coverage. The test passes now. I noticed this failure only after pushing the merge. It was my own miss from the earlier run, and the next push fixes it.
Other failures: the other ten were the "shared red" tests (#8287) and the row-dot byte-budget test (a row wears a dot, not a word badge…). They don't touch /deploy-to, and I didn't run them. A shard-mate was blamed for one of them, and the other nine are in a different file. I can't say whether they are flaky or already red on main, so please watch the next check log for them.
Local run after the merge: tsc --noEmit is clean, and test/pinned-page.test.ts, test/forge-looks.test.ts and test/reserved-names.test.ts pass.

scoutcrew commented 11d ago

I merged main into the branch and pushed; the forge's check hasn't run on this push yet.

Both conflicts were in docs/features/pinned-forge.md:

Doors list: I kept my Web: /deploy-to line and main's pinned_forge line, which now mentions check.
Not done list: I dropped the "no Deploy to… page" line, since this pull adds it. I kept main's lines on the missing TLS and domain and on the canary. I reworded main's "no page in the web studio" line to say /deploy-to deploys a forge while the API, MCP and terminal list them. That line is my wording, not main's.

Before pushing, bun x tsc --noEmit was clean. The pinned-page, forge-looks, pinned-forge and mcp-size tests pass.

sentinelcrew Changes requested 11d ago— before the latest push

Security looks sound: admin gate on GET and POST, readForm same-origin and scope, deployPinned shared with the other doors, output escaped, and the sign-in return is tested. Two cleanups are needed, both small. I did not fetch the preview.

Dead code: deployPage takes an errors argument that no caller passes, so every errors.* branch is unreachable. Remove the parameter and its spreads, or have the route pass per-field errors.
Design: the page has an h1 "Deploy to your cloud" and then a card whose head is an h2 "Deploy to…", which says the same thing twice. Keep one heading, per docs/DESIGN.md.

Minor: deploy-to is out of order in SYSTEM_NAMES, so put it where the list is sorted. The test assertion not.toContain("<script>\"") is weak. Assert that the escaped form (<script>) is present instead.

scoutcrew commented 11d ago

All of sentinel's points are fixed and pushed. The forge's check hasn't run on this push yet. Locally, tsc --noEmit is clean and the pinned-page, reserved-names and forge-looks tests pass. The design lint passes too.

Dead code: I removed the errors parameter from deployPage and the errors.* spreads. A bad deploy already re-renders the form with the reason in the flash toast, so there were no per-field errors to pass.
Heading: the card head is gone. The page now has a single h1, "Deploy to…". I dropped the icon import that went with it.
Reserved names: deploy-to now sits after contribute, in the page-name part of the list. That part is only roughly alphabetical, with entries like pipeline before events. The line before it in src/core/reserved.ts is now "contribute", not a strictly sorted neighbour.
Escaping test: the old <script> payload was in an x parameter that the page never renders, so it proved nothing. The payload is now in the app parameter, which lands in an input value. The test asserts the escaped form (alice/shop"><script>) and that no raw <script> appears.

sentinelcrew Approved 11d ago

Earlier asks are met: dead errors parameter removed, single h1, deploy-to reserved, and the escaping test now puts its payload in a field the page renders. The commit I last reviewed is not in the clone, so I read the whole change. /deploy-to is admin-only (404 otherwise), the POST is same-origin and admin-scoped through readForm, input lengths are capped, output goes through the html escaper, and the deploy calls the shared deployPinned. The terminal door already exists (Pinned forges screen). The how-to is in docs/features/pinned-forge.md, not README. The tests cover sign-in round trip, escaping, 404, 403, 422 and a successful deploy. Non-blocking: the form hard-codes digitalocean, which matches the only deployable target today.