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

An app whose / answers an error is not boot ok #10613

Merged masoncrew opened this 18h ago

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

Merged

A deploy now fails when the app's `/` answers 4xx or 5xx, even if its health path is fine. The run's summary reads `ready: / answered 404; the app is up but has no page at its root`. The new tests, the gate and the six apps suites I ran (116 tests) pass, and `tsc --noEmit` is clean. I did not run the rest of the suite.

shipped production is serving it deploy cca14ac

masoncrew opened this 18h ago

A deploy now fails when the app's / answers 4xx or 5xx, even if its health path is fine. The run's summary reads ready: / answered 404; the app is up but has no page at its root. The new tests, the gate and the six apps suites I ran (116 tests) pass, and tsc --noEmit is clean. I did not run the rest of the suite.

What changed

Check: after the health path answers, bringUp in src/apps/runtime.ts calls a new rootFault(port) that fetches / once without following redirects.

- 200 and redirects pass. - On 4xx or 5xx the new process is stopped, its token revoked and its checkout removed, and the deploy fails. - A previous deployment keeps serving, as for any failed deploy.

App page: I made no change there. A failed run's summary already becomes the app's last error, so the page shows this line in place of boot ok.
Private apps: nothing extra was needed. The probe goes straight to the app's own port, so the access door's 404 can't be mistaken for the app's.
Tests: test/app-runtime-root.test.ts covers rootFault for 404, 500, 200 and a 302. It also deploys two real apps with a healthy /health: one with a 404 / that fails, and one with a 200 / that is ok.

What I left alone

boot ok line: the only boot ok string in the repo is in src/release/boot.ts, the forge's own release boot. I did not touch it; this change covers deploying apps.
Test file name: I used a hyphenated name (app-runtime-root.test.ts) to match the other test files, not the issue's app_runtime.test.ts.
Behaviour change: an app that deliberately 404s on / now fails its deploy. That is what the issue asks for.

To try it, deploy an app whose /health returns 200 and / returns 404. The run should fail with the line above.

🤖 Generated with Claude Code

Closes #10565

masoncrew commented 18h ago

What changed

Check: after the health path answers, bringUp in src/apps/runtime.ts calls a new rootFault(port) that fetches / once without following redirects.

App page: I made no change there. A failed run's summary already becomes the app's last error, so the page shows this line in place of boot ok.
Private apps: nothing extra was needed. The probe goes straight to the app's own port, so the access door's 404 can't be mistaken for the app's.
Tests: test/app-runtime-root.test.ts covers rootFault for 404, 500, 200 and a 302. It also deploys two real apps with a healthy /health: one with a 404 / that fails, and one with a 200 / that is ok.

What I left alone

boot ok line: the only boot ok string in the repo is in src/release/boot.ts, the forge's own release boot. I did not touch it; this change covers deploying apps.
Test file name: I used a hyphenated name (app-runtime-root.test.ts) to match the other test files, not the issue's app_runtime.test.ts.
Behaviour change: an app that deliberately 404s on / now fails its deploy. That is what the issue asks for.

To try it, deploy an app whose /health returns 200 and / returns 404. The run should fail with the line above.

🤖 Generated with Claude Code

junipercrew Approved 18h ago

Small, single-purpose, and it does what the issue asks. After the health path answers, rootFault probes / on the app own port without following redirects, so a private app access door cannot be mistaken for the app. 4xx/5xx stops the process, revokes the token, removes the checkout and fails with a line naming the status. The failed run error becomes the app lastError, so the app page shows that line. Tests cover the unit (404, 500, 200, 302) and two real deploys. Next, I would: (1) add a short line to the app docs saying a / answering 4xx fails the deploy, since apps that deliberately 404 on / will now fail; (2) consider a retry or two for a / that is briefly 5xx while warming, since a single probe could fail a deploy on a transient.