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

forge.json's app.looks lists every top-level page of the forge #2752

Merged opened this

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

Merged

This pull now only raises the `app.looks` cap from 3 to 24. The list of pages and its route test are split out into #2753. That leaves #2735 not fully done, and nothing has run against this push yet.

shipped production is serving it deploy 5f71e3e

opened this

This pull now only raises the app.looks cap from 3 to 24. The list of pages and its route test are split out into #2753. That leaves #2735 not fully done, and nothing has run against this push yet.

Why the check failed: the preview deploy parses forge.json with the forge that is already running, and that forge still caps app.looks at 3. My earlier forge.json with 19 pages was refused (forge.json: app.looks may name at most 3 paths), so the deploy failed and the check came back failed. The new cap can't help until it is deployed, so the list can't ship in the same pull.

What this pull changes

  • Cap: LOOKS_MAX in src/apps/manifest.ts is 24, enough for every top-level page, and docs/APPS.md says so.
  • Test: test/apps-adversarial.test.ts now checks that 24 paths are accepted and 25 are refused. It passes locally (58 tests). I couldn't run the type check here, because bun x tsc can't write to the sandbox's temp directory.
  • forge.json: unchanged, so the preview deploys.

Deferred to #2753: the 19-page app.looks list in forge.json and test/forge-looks.test.ts. That test fails if a top-level page route is missing from app.looks, or if a name in app.looks doesn't answer 200 for an admin. It passed locally earlier. I kept the finished versions and wrote the page list into #2753's description. The test/forge-looks.test.ts file itself isn't saved anywhere in the repo, so whoever picks up #2753 has to rewrite it from that description.

Not linked: I couldn't mark #2753 as waiting on this pull, because the API only links issues to issues. A person needs to start #2753 once this pull is merged and deployed.

Continues #2752

commented

forge.json's app.looks now names 19 top-level pages, up from the home page alone, and the cap on app.looks is raised from 3 to 24. I merged the newer main into the branch (0.7.172, which has design changes only). The new test and the manifest tests still pass on the merge, but I didn't re-run the type check or the rest of the suite.

  • Pages named: /, /board, /boards, /map, /flow, /system, /apps, /explore, /family, /front, /releases, /resilience, /audit, /contribute, /search, /settings, /migrate, /new, /asked. Each answers 200 for a signed-in admin, the viewer a look is taken as.
  • Cap: LOOKS_MAX in src/apps/manifest.ts goes to 24, and docs/APPS.md says 24. The two cap tests in test/apps-adversarial.test.ts now check 24 and 25.
  • Test: test/forge-looks.test.ts fails if a top-level page route is missing from app.looks. It also fails if app.looks names something that isn't a route or doesn't answer 200 for an admin. Routes that aren't pages sit in a list in the test, each with a reason: machine files, signed-out pages, POST-only routes, the event stream and the redirects /asks and /big. A new top-level page has to go in app.looks or in that list.

Left out

  • /usage: the issue names it, but it isn't a top-level route in the router, so I didn't list it. If it lives under another path, say so and I'll add it.
  • /forges: it answers 404 on a forge that isn't a family's, so I left it out.
  • Capture cost: each page is pictured at three widths on every green deploy, so a deploy now takes 19 times the browser starts of the old default. I didn't measure the cost.
  • Pages: unchanged.

Try it Run bun test test/forge-looks.test.ts test/apps-adversarial.test.ts. The next preview deploy pictures every listed page. The real-Chromium capture test in test/looks.test.ts fails in the agent sandbox, with or without this change, because Chromium can't open a socket there; the forge's check runs it.

commented

This pull now only raises the app.looks cap from 3 to 24. The list of pages and its route test are split out into #2753. That leaves #2735 not fully done, and nothing has run against this push yet.

Why the check failed: the preview deploy parses forge.json with the forge that is already running, and that forge still caps app.looks at 3. My earlier forge.json with 19 pages was refused (forge.json: app.looks may name at most 3 paths), so the deploy failed and the check came back failed. The new cap can't help until it is deployed, so the list can't ship in the same pull.

What this pull changes

  • Cap: LOOKS_MAX in src/apps/manifest.ts is 24, enough for every top-level page, and docs/APPS.md says so.
  • Test: test/apps-adversarial.test.ts now checks that 24 paths are accepted and 25 are refused. It passes locally (58 tests). I couldn't run the type check here, because bun x tsc can't write to the sandbox's temp directory.
  • forge.json: unchanged, so the preview deploys.

Deferred to #2753: the 19-page app.looks list in forge.json and test/forge-looks.test.ts. That test fails if a top-level page route is missing from app.looks, or if a name in app.looks doesn't answer 200 for an admin. It passed locally earlier. I kept the finished versions and wrote the page list into #2753's description. The test/forge-looks.test.ts file itself isn't saved anywhere in the repo, so whoever picks up #2753 has to rewrite it from that description.

Not linked: I couldn't mark #2753 as waiting on this pull, because the API only links issues to issues. A person needs to start #2753 once this pull is merged and deployed.

commented

The check on #2752 now passes. The failure you saw was for the earlier head, b40fc297. The current head, 7f9c027f, passed the check in 149 seconds, and its preview deployed. I changed nothing and pushed nothing this turn.

The earlier failure came from the preview deploy, not the tests. The forge that was running still capped app.looks at 3 paths, so it refused the 19-page forge.json. That is why I split the work: this pull only raises the cap from 3 to 24, in src/apps/manifest.ts and docs/APPS.md. It also updates the cap tests in test/apps-adversarial.test.ts, which now check that 24 paths are accepted and 25 refused.

#2735 itself is not finished. The 19-page list in forge.json and the test that every top-level route is in app.looks are tracked in #2753. They can go in once this pull is merged and the new cap is running. I couldn't make #2753 wait on this pull, because the API only links issues to issues, so someone has to start #2753 after the merge.

To try it, run bun test test/apps-adversarial.test.ts.

Approved

Approved. The cap goes from 3 to 24 in src/apps/manifest.ts, docs/APPS.md says 24, and the boundary tests check 24 accepted and 25 refused. The check is green and the change touches no input, auth or write path. The split from #2753 is justified: the running forge would refuse a 19-page forge.json, so the list cannot ship in the same pull. I did not curl the preview or read the screenshots; the diff does not change forge.json, so there is nothing new to see.

Two things to do next:

  1. Start #2753 as soon as this is deployed. The title of #2752 is not true until forge.json lists the pages, and the route-coverage test has to be rewritten from the description.
  2. Measure capture cost before the list lands. 19 pages at 3 widths is 57 browser starts per green deploy. Consider capturing only phone and desktop for the extra pages, or capturing in the background so the deploy is not held up.
commented

This pull (#2752) now only raises the app.looks cap from 3 to 24. The check on its current head, 7f9c027f, is green, and the preview deploy is ok. There was nothing new to push this time.

The earlier failure came from the deploy, not the tests. The deploy reads forge.json with the forge that is already running, which still capped app.looks at 3. My earlier 19-page list was refused, so I took it out of this pull.

  • Changed: LOOKS_MAX in src/apps/manifest.ts is 24, and docs/APPS.md says so. test/apps-adversarial.test.ts now checks that 24 paths are accepted and 25 are refused.
  • Not done: the 19-page list in forge.json and the test that every top-level route is named in it. #2735's checklist is not complete until those land. They are tracked in #2753, with the page list written out in its description.
  • To do: whoever picks up #2753 needs to wait until this pull is merged and the cap is running. I couldn't link #2753 as waiting on this pull, because the API only links issues to issues.
commented

Nothing has moved since my last run, so I changed nothing and pushed nothing.

  • Pull #2752: still open, head 7f9c027f, with a green check and a successful preview deploy. It only raises the app.looks cap from 3 to 24.
  • Cap on main: main still has LOOKS_MAX = 3, so a person needs to merge #2752 before the page list can ship.
  • #2753: still open. The forge.json list of 19 pages and the test that every top-level route is named are held there until the cap is merged and deployed. Putting the list in this pull would make the preview deploy fail again, so #2735's checklist stays incomplete until #2753 lands.
commented

Nothing has changed since my last run, so I made no changes and pushed nothing.

  • Pull #2752: still open at head 7f9c027f, with a green check. It only raises the app.looks cap from 3 to 24.
  • main: still at 621df60b, with LOOKS_MAX = 3.
  • Remaining work: the 19-page list in forge.json and its test are still waiting in #2753. They can't ship until a person merges #2752 and the cap is deployed. Adding the list to this pull earlier made the preview deploy fail, because the running forge refuses more than 3 pages.
commented

Nothing has moved since my last run, so I changed nothing and pushed nothing.

  • Pull #2752: still open at head 7f9c027f with a green check. It only raises the app.looks cap from 3 to 24.
  • main: still at 621df60b, with LOOKS_MAX = 3.
  • Remaining work: the 19-page app.looks list in forge.json and its test stay in #2753. Putting the list in this pull made the preview deploy fail, because the running forge refuses more than 3 pages. #2735's checklist stays incomplete until a person merges #2752 and the cap is deployed.
Sign in to join this conversation.

↑↓ navigate ↵ open ⌘↵ its app esc close

Keyboard shortcuts

Docs