trevato/forge

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

More
Forks1g fMapg aRuns20g rEventsFlows

Serve a deployment as soon as it answers; run the app's tests beside it, not in front of it #19

Merged scoutcrew opened this

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

ConversationFiles
Merged

scout/forge:scout/11-serve-a-deployment-as-soon-as-it-answers → main 1 commit fork

  • Merged. scout/11-serve-a-deployment-as-soon-as-it-answers was merged into main as d46a5d9.

Review

Sign in to review this change.
6 files changed +355 −57
docs/APPS.md +16 −6
@@ -69,7 +69,13 @@ around it. None is advisory.
6969 4. **A deployment is a check.** Every deploy is a run; the run's state is
7070 what the pull shows. With `require_checks` on, `mergePull` refuses
7171 `checks_required` until the head's deploy run is `ok`, read live inside
72 the merge — never from a status a caller computed earlier.
72 the merge — never from a status a caller computed earlier. Install still
73 comes first, but `test` and `start` run side by side: the environment is
74 swapped in and serving the moment the app answers, and the run stays
75 `running` — checking, not blocking — until `test` finishes and decides
76 `ok` or `failed`. A deployment that answers is what was asked for; the
77 suite going green is confirmed a few seconds later, not stood in front
78 of the door.
7379 5. **A private app answers only to someone the forge vouches for.** An app's
7480 `access` is `public` or `private` (defaulting to the repository's
7581 visibility), and it is set apart from the repository's own: the code can
@@ -186,9 +192,10 @@ verdict is a run of kind `check`, kept with its log beside the pull, in
186192 the same place a preview's deploy reports and read by the same code: the
187193 inbox badge, the pull's status, the crew. A failing check on an agent's
188194 pull goes back to the agent with the log's tail, as a failed preview does,
189 bounded the same way. An app's own `test` still runs inside its deploy;
190 `check` is for a repository that is not an app, or an app whose check is
191 more than its test. `install` defaults to `bun install` when the package
195 bounded the same way. An app's own `test` still runs inside its deploy,
196 concurrently with `start` rather than before it; `check` is for a
197 repository that is not an app, or an app whose check is more than its
198 test. `install` defaults to `bun install` when the package
192199 declares dependencies; `timeout` is seconds, twenty minutes by default.
193200 A `check` that is present and wrong fails with the reason, so a commit is
194201 never silently unchecked. A check under way when the forge restarts is
@@ -360,8 +367,11 @@ many landed, pulls a person merged, spend, and the mean cost, time and
360367 turns of a run.
361368
362369 On a pull, the runs for the current head are its checks. A deploy run that
363 fails — install, build, test, or the app never answering — fails the check
364 and the preview shows the log where it stopped. A push makes new runs;
370 fails — install, build, or the app never answering — fails the check before
371 anything serves; the preview shows the log where it stopped. A deploy whose
372 app answers but whose `test` then fails is different: the preview keeps
373 serving that commit, but the run still fails the check, and the page says
374 in words that it is serving code whose tests failed. A push makes new runs;
365375 older ones are history, not status.
366376
367377 ## The crew
src/apps/process.ts +46 −18
@@ -1,8 +1,9 @@
11 /**
2 * One deployment's process: install, test, start, and wait until it
3 * answers. Everything the app is handed is listed here and nothing else
4 * reaches it — the forge's own environment carries secrets, and a child
5 * inherits exactly what `childEnv` names (rule 8 in docs/APPS.md).
2 * One deployment's process: install, then start the app and run its tests
3 * side by side, and wait until the app answers. Everything the app is
4 * handed is listed here and nothing else reaches it — the forge's own
5 * environment carries secrets, and a child inherits exactly what `childEnv`
6 * names (rule 8 in docs/APPS.md).
67 *
78 * Commands run through `sh -c` with an `exec` in front, so the shell
89 * replaces itself with the app and a signal to the pid is a signal to the
@@ -48,6 +49,17 @@ export interface StartError {
4849 readonly message: string;
4950 }
5051
52 export interface StartedProcess {
53 readonly handle: ProcessHandle;
54 /**
55 * The `test` step's outcome. Already running by the time this resolves —
56 * it started alongside `start`, not before it — so a caller that wants a
57 * fast preview can swap the handle in now and await this afterwards.
58 * `ok()` immediately when the manifest has no `test`.
59 */
60 readonly tested: Promise<Result<void, string>>;
61 }
62
5163 const STEP_TIMEOUT_MS = 10 * 60_000;
5264 const STOP_GRACE_MS = 5_000;
5365 const HEALTH_INTERVAL_MS = 250;
@@ -177,13 +189,15 @@ export async function runStep(
177189 }
178190
179191 /**
180 * Install, test, start, and wait for the app to answer on its port.
181 * Returns the running process; on failure the process (if it started) is
182 * already gone.
192 * Install, then start the app and run its tests side by side, and wait for
193 * the app to answer on its port. Returns as soon as it does — `tested`
194 * keeps running and is the caller's to await — so a preview is never held
195 * behind its own test suite. On failure the process (if it started) and the
196 * test step (if one was running) are both already gone.
183197 */
184198 export async function startProcess(
185199 input: StartInput,
186 ): Promise<Result<ProcessHandle, StartError>> {
200 ): Promise<Result<StartedProcess, StartError>> {
187201 const { manifest, log } = input;
188202 const stepEnv = childEnv({
189203 CI: "1",
@@ -200,18 +214,28 @@ export async function startProcess(
200214 if (!installed.ok)
201215 return err({ stage: "install", message: installed.error });
202216 }
203 if (manifest.test !== null) {
204 const tested = await runStep("test", manifest.test, {
205 cwd: input.dir,
206 env: stepEnv,
207 log,
208 signal: input.signal,
209 });
210 if (!tested.ok) return err({ stage: "test", message: tested.error });
211 }
212217 if (input.signal.aborted)
213218 return err({ stage: "start", message: "cancelled" });
214219
220 // `test` starts here, alongside the app below, and is aborted on its own
221 // signal — a start failure must not leave it running past its run, and
222 // neither may the caller's abort after this function has already returned.
223 const testAbort = new AbortController();
224 if (input.signal.aborted) testAbort.abort();
225 else
226 input.signal.addEventListener("abort", () => testAbort.abort(), {
227 once: true,
228 });
229 const tested: Promise<Result<void, string>> =
230 manifest.test === null
231 ? Promise.resolve(ok())
232 : runStep("test", manifest.test, {
233 cwd: input.dir,
234 env: stepEnv,
235 log,
236 signal: testAbort.signal,
237 });
238
215239 const port = freePort();
216240 log(`$ ${manifest.run} (PORT=${port})`);
217241 const proc = Bun.spawn(["/bin/sh", "-c", `exec ${manifest.run}`], {
@@ -267,6 +291,10 @@ export async function startProcess(
267291 });
268292 if (!ready.ok) {
269293 await handle.stop();
294 // The app never came up: the test step running beside it has nothing
295 // left to verify, and must not outlive this failed run.
296 testAbort.abort();
297 await tested.catch(() => {});
270298 const tail =
271299 recent.length > 0
272300 ? ` — last output: ${recent.slice(-3).join(" | ")}`
@@ -277,7 +305,7 @@ export async function startProcess(
277305 });
278306 }
279307 log(`ready: ${input.url}`);
280 return ok(handle);
308 return ok({ handle, tested });
281309 }
282310
283311 /**
src/apps/runtime.ts +75 −27
@@ -37,6 +37,7 @@ import { Gate } from "../git/env.ts";
3737 import { resolveOid } from "../git/merge.ts";
3838 import { repoDir } from "../git/repos.ts";
3939 import { createLog } from "../kernel/log.ts";
40 import type { Result } from "../kernel/result.ts";
4041 import { holdLease, STALE_AFTER_MS } from "../runs/lease.ts";
4142 import { appendRunLog } from "../runs/log.ts";
4243 import { exportTree, readFileAt } from "./checkout.ts";
@@ -83,6 +84,13 @@ export interface EnvStatus {
8384 readonly runId?: string | undefined;
8485 readonly since?: string | undefined;
8586 readonly error?: string | undefined;
87 /**
88 * A running environment whose deploy is still checking itself: its tests
89 * are still running, or ran and failed. Never set while `state` is
90 * anything but `running` — a failed test never stops a deployment that
91 * already answers.
92 */
93 readonly note?: string | undefined;
8694 }
8795
8896 interface Live {
@@ -93,6 +101,8 @@ interface Live {
93101 readonly since: string;
94102 /** How this checkout runs — its `tty` is what an ssh session opens. */
95103 readonly manifest: AppManifest;
104 /** Unset while tests are still running or there are none; the message once they fail. */
105 readonly testsFailed?: string | undefined;
96106 }
97107
98108 /** A running environment, as a terminal session needs it. */
@@ -199,7 +209,14 @@ export class AppsRuntime {
199209
200210 private describe(state: Environment): EnvStatus {
201211 const url = this.urlFor(state.app, state.env);
202 if (state.deploying !== undefined) {
212 // Once a deploy's own commit has answered and swapped in, it is
213 // "running" from here — even though its run stays open, checking its
214 // tests, until they finish. Only a build still short of that point (no
215 // live process yet, or the live one is an older commit) is "deploying".
216 const swapped =
217 state.deploying !== undefined &&
218 state.live?.commitOid === state.deploying.run.commitOid;
219 if (state.deploying !== undefined && !swapped) {
203220 return {
204221 env: state.env,
205222 state: "deploying",
@@ -217,8 +234,12 @@ export class AppsRuntime {
217234 commitOid: state.live.commitOid,
218235 url,
219236 port: state.live.handle.port,
220 runId: state.live.runId,
237 runId: swapped ? state.deploying!.run.id : state.live.runId,
221238 since: state.live.since,
239 ...(swapped ? { note: "serving — still verifying its tests" } : {}),
240 ...(state.live.testsFailed === undefined
241 ? {}
242 : { error: `serving — its tests failed: ${state.live.testsFailed}` }),
222243 };
223244 }
224245 return {
@@ -559,29 +580,9 @@ export class AppsRuntime {
559580 error: `deploy failed: ${error instanceof Error ? error.message : String(error)}`,
560581 }));
561582
562 state.deploying = undefined;
563 lease();
564 if (outcome.ok) {
565 const previous = state.live;
566 state.live = outcome.value;
567 state.lastError = undefined;
568 if (previous !== undefined) {
569 write(
570 `stopping the previous deployment (${previous.commitOid.slice(0, 7)})`,
571 );
572 await previous.handle.stop();
573 if (previous.dir !== outcome.value.dir)
574 rmSync(previous.dir, { recursive: true, force: true });
575 }
576 this.watch(state, repo, outcome.value);
577 const url = this.urlFor(state.app, state.env);
578 const done = finishRun(this.app.db, run.id, {
579 state: "ok",
580 summary: `running at ${url}`,
581 });
582 write(`done: ${url}`);
583 if (done) announceRun(this.app, done);
584 } else {
583 if (!outcome.ok) {
584 state.deploying = undefined;
585 lease();
585586 state.lastError = outcome.error;
586587 const done = finishRun(this.app.db, run.id, {
587588 state: "failed",
@@ -593,7 +594,50 @@ export class AppsRuntime {
593594 `the previous deployment (${state.live.commitOid.slice(0, 7)}) keeps serving`,
594595 );
595596 if (done) announceRun(this.app, done);
597 return;
596598 }
599
600 // It answers: swap it in now, before its tests are done. `state.deploying`
601 // stays set — the run is still open, and a stop or a teardown must still
602 // be able to reach in and kill the test step — until they finish below.
603 const previous = state.live;
604 state.live = outcome.value;
605 state.lastError = undefined;
606 if (previous !== undefined) {
607 write(
608 `stopping the previous deployment (${previous.commitOid.slice(0, 7)})`,
609 );
610 await previous.handle.stop();
611 if (previous.dir !== outcome.value.dir)
612 rmSync(previous.dir, { recursive: true, force: true });
613 }
614 this.watch(state, repo, outcome.value);
615 const url = this.urlFor(state.app, state.env);
616 write(`serving ${url}`);
617
618 const tested = await outcome.tested;
619 state.deploying = undefined;
620 lease();
621 if (tested.ok) {
622 const done = finishRun(this.app.db, run.id, {
623 state: "ok",
624 summary: `running at ${url}`,
625 });
626 write(`done: ${url}`);
627 if (done) announceRun(this.app, done);
628 return;
629 }
630 // The tests failed, but the deployment that answered is what was asked
631 // for: it keeps serving, and the run — the check a pull shows — fails.
632 if (state.live === outcome.value)
633 state.live = { ...outcome.value, testsFailed: tested.error };
634 const done = finishRun(this.app.db, run.id, {
635 state: "failed",
636 summary: `test: ${tested.error}`,
637 });
638 write(`failed: test: ${tested.error}`);
639 write(`serving ${url} anyway — its tests failed`);
640 if (done) announceRun(this.app, done);
597641 }
598642
599643 private async bringUp(
@@ -602,7 +646,10 @@ export class AppsRuntime {
602646 run: Run,
603647 write: (line: string) => void,
604648 signal: AbortSignal,
605 ): Promise<{ ok: true; value: Live } | { ok: false; error: string }> {
649 ): Promise<
650 | { ok: true; value: Live; tested: Promise<Result<void, string>> }
651 | { ok: false; error: string }
652 > {
606653 const bare = repoDir(this.app.reposRoot, repo.ownerName, repo.name);
607654 const [forgeJson, packageJson] = await Promise.all([
608655 readFileAt(bare, run.commitOid, "forge.json"),
@@ -668,12 +715,13 @@ export class AppsRuntime {
668715 ok: true,
669716 value: {
670717 commitOid: run.commitOid,
671 handle: started.value,
718 handle: started.value.handle,
672719 dir,
673720 runId: run.id,
674721 since: new Date().toISOString(),
675722 manifest: manifest.value,
676723 },
724 tested: started.value.tested,
677725 };
678726 }
679727
src/web/pages/apps.ts +11 −3
@@ -459,7 +459,9 @@ const envFacts = (base: string, env: EnvStatus): Html =>
459459 html`<a href="${base}/runs/${id}"
460460 >${env.state === "deploying"
461461 ? "deploy in progress"
462 : "last deploy"}</a
462 : env.note !== undefined
463 ? "verifying tests"
464 : "last deploy"}</a
463465 >`,
464466 )}
465467 </div>`;
@@ -472,10 +474,13 @@ function envCard(base: string, env: EnvStatus, writer: boolean): Html {
472474 <p class="env-facts mt">
473475 <span class="mono truncate" title="${env.url}">${env.url}</span>
474476 </p>
477 ${when(env.note, (note) => html`<p class="muted small">${note}</p>`)}
475478 ${when(env.error, (error) => html`<p class="env-error">${error}</p>`)}`,
476479 {
477480 extra: "env-card",
478 ...(env.state === "failed" ? { tone: "danger" as const } : {}),
481 ...(env.state === "failed" || env.error !== undefined
482 ? { tone: "danger" as const }
483 : {}),
479484 head: html`${icon(number === undefined ? "rocket" : "pull")}
480485 <h2>
481486 ${number === undefined
@@ -686,6 +691,7 @@ export function previewCard(ctx: Ctx, repo: Repo, pull: Pull): Html {
686691 }
687692 return card(
688693 html`${envFacts(base, env)}
694 ${when(env.note, (note) => html`<p class="muted small">${note}</p>`)}
689695 ${when(env.error, (error) => html`<p class="env-error">${error}</p>`)}
690696 <div class="bar mt">
691697 ${btn("Open preview", {
@@ -698,7 +704,9 @@ export function previewCard(ctx: Ctx, repo: Repo, pull: Pull): Html {
698704 {
699705 head,
700706 extra: "env-card",
701 ...(env.state === "failed" ? { tone: "danger" as const } : {}),
707 ...(env.state === "failed" || env.error !== undefined
708 ? { tone: "danger" as const }
709 : {}),
702710 },
703711 );
704712 }
test/apps-adversarial.test.ts +131
@@ -1075,6 +1075,137 @@ describe("apps adversarial: teardown races, grants, the proxy, and rule 1/4", ()
10751075 );
10761076 }, 60_000);
10771077
1078 test("install failing fails before test or start ever run — nothing from a half-installed tree", async () => {
1079 const made = await newApp(app, alice, {
1080 name: "badinstall",
1081 visibility: "public",
1082 });
1083 expect(made.ok).toBe(true);
1084 if (!made.ok) return;
1085 const repo = made.value.repo;
1086 await settled(repo.id, (r) => r.env === "production");
1087
1088 const testMarker = join(work, `install-test-marker-${Date.now()}`);
1089 const runMarker = join(work, `install-run-marker-${Date.now()}`);
1090 const committed = await commitToBranch(app, alice, repo, {
1091 branch: "bad-install",
1092 base: repo.defaultBranch,
1093 message: "install fails",
1094 files: [
1095 {
1096 path: "forge.json",
1097 content: `${JSON.stringify(
1098 {
1099 app: {
1100 install: "exit 1",
1101 run: `touch ${runMarker} && bun run src/server.ts`,
1102 test: `touch ${testMarker} && bun test`,
1103 },
1104 },
1105 null,
1106 2,
1107 )}\n`,
1108 },
1109 ],
1110 });
1111 expect(committed.ok).toBe(true);
1112 if (!committed.ok) return;
1113 const opened = await openPull(app, alice, repo, {
1114 title: "Bad install",
1115 baseBranch: repo.defaultBranch,
1116 headBranch: "bad-install",
1117 });
1118 expect(opened.ok).toBe(true);
1119 if (!opened.ok) return;
1120
1121 const failed = await settled(
1122 repo.id,
1123 (r) =>
1124 r.env === `pull-${opened.value.number}` &&
1125 r.commitOid === committed.value.commitOid,
1126 );
1127 expect(failed.state).toBe("failed");
1128 expect(failed.summary).toContain("install");
1129 // Neither `test` nor `run` ever executed.
1130 expect(existsSync(testMarker)).toBe(false);
1131 expect(existsSync(runMarker)).toBe(false);
1132 expect(
1133 (await viaHost(`badinstall--${opened.value.number}.${DOMAIN}`)).status,
1134 ).toBe(502);
1135 }, 60_000);
1136
1137 test("closing a pull whose deploy already answered kills its still-running test step — no orphan `bun test`", async () => {
1138 const made = await newApp(app, alice, {
1139 name: "slowtest",
1140 visibility: "public",
1141 });
1142 expect(made.ok).toBe(true);
1143 if (!made.ok) return;
1144 const repo = made.value.repo;
1145 await settled(repo.id, (r) => r.env === "production");
1146
1147 const marker = join(work, `slowtest-marker-${Date.now()}`);
1148 const committed = await commitToBranch(app, alice, repo, {
1149 branch: "slow-test",
1150 base: repo.defaultBranch,
1151 message: "slow test",
1152 files: [
1153 {
1154 path: "forge.json",
1155 content: `${JSON.stringify(
1156 {
1157 app: {
1158 run: "bun run src/server.ts",
1159 test: `sleep 5 && touch ${marker}`,
1160 },
1161 },
1162 null,
1163 2,
1164 )}\n`,
1165 },
1166 ],
1167 });
1168 expect(committed.ok).toBe(true);
1169 if (!committed.ok) return;
1170 const opened = await openPull(app, alice, repo, {
1171 title: "Slow test",
1172 baseBranch: repo.defaultBranch,
1173 headBranch: "slow-test",
1174 });
1175 expect(opened.ok).toBe(true);
1176 if (!opened.ok) return;
1177
1178 // Wait for the app to answer — well before the test step's sleep ends.
1179 const upDeadline = Date.now() + 10_000;
1180 let up = false;
1181 while (Date.now() < upDeadline) {
1182 if (
1183 (await viaHost(`slowtest--${opened.value.number}.${DOMAIN}`))
1184 .status === 200
1185 ) {
1186 up = true;
1187 break;
1188 }
1189 await Bun.sleep(50);
1190 }
1191 expect(up).toBe(true);
1192
1193 // Close it while the test step is still sleeping.
1194 const closed = await mergePull(
1195 app,
1196 alice,
1197 repo,
1198 getPull(app.db, repo.id, opened.value.number)!,
1199 "merge",
1200 );
1201 expect(closed.ok).toBe(true);
1202
1203 // Wait well past the sleep the test step was doing, and confirm it
1204 // never finished: the marker it would have written is not there.
1205 await Bun.sleep(6_500);
1206 expect(existsSync(marker)).toBe(false);
1207 }, 60_000);
1208
10781209 test("private grant: forged, tampered, expired, wrong-env, and open-redirect are all refused; cookie is HttpOnly/SameSite", async () => {
10791210 const made = await newApp(app, alice, {
10801211 name: "grants",
test/apps.test.ts +76 −3
@@ -391,10 +391,17 @@ describe("apps: production, previews, data, gates, crew", () => {
391391 const refused = await mergePull(app, alice, repo, opened.value, "merge");
392392 expect(refused.ok).toBe(false);
393393 if (!refused.ok) expect(refused.error).toBe("checks_required");
394 // The preview shows why instead of a gateway error.
394 // The app itself never broke, only its tests: the preview answers with
395 // its own page rather than a gateway error, because a deployment that
396 // answers is what was asked for.
395397 const page = await viaHost(`hello--${opened.value.number}.${DOMAIN}`);
396 expect(page.status).toBe(502);
397 expect(await page.text()).toContain("test");
398 expect(page.status).toBe(200);
399 expect(await page.text()).toContain(
400 `preview of pull #${opened.value.number}`,
401 );
402 const status = runtime.envStatus(record, `pull-${opened.value.number}`);
403 expect(status?.state).toBe("running");
404 expect(status?.error).toContain("tests failed");
398405
399406 const fixed = await commitToBranch(app, alice, repo, {
400407 branch: "broken",
@@ -425,6 +432,72 @@ describe("apps: production, previews, data, gates, crew", () => {
425432 expect(merged.ok).toBe(true);
426433 }, 120_000);
427434
435 test("a deploy whose test step sleeps: the preview answers before it finishes, and the run stays open until it does", async () => {
436 const committed = await commitToBranch(app, alice, repo, {
437 branch: "slow-test",
438 base: repo.defaultBranch,
439 message: "Slow test",
440 files: [
441 {
442 path: "forge.json",
443 content: `${JSON.stringify(
444 {
445 app: {
446 run: "bun run src/server.ts",
447 test: "sleep 3 && bun test",
448 tty: "bun run src/tty.ts",
449 },
450 },
451 null,
452 2,
453 )}\n`,
454 },
455 ],
456 });
457 expect(committed.ok).toBe(true);
458 if (!committed.ok) return;
459 const opened = await openPull(app, alice, repo, {
460 title: "Slow test",
461 baseBranch: repo.defaultBranch,
462 headBranch: "slow-test",
463 });
464 expect(opened.ok).toBe(true);
465 if (!opened.ok) return;
466
467 const start = Date.now();
468 const deadline = start + 15_000;
469 let answered = false;
470 while (Date.now() < deadline) {
471 if (
472 (await viaHost(`hello--${opened.value.number}.${DOMAIN}`)).status ===
473 200
474 ) {
475 answered = true;
476 break;
477 }
478 await Bun.sleep(50);
479 }
480 expect(answered).toBe(true);
481 // It answered well inside the test step's own sleep.
482 expect(Date.now() - start).toBeLessThan(3_000);
483
484 const status = runtime.envStatus(record, `pull-${opened.value.number}`);
485 expect(status?.state).toBe("running");
486 expect(status?.note).toContain("verifying its tests");
487
488 const run = listRuns(app.db, repo.id, {
489 env: `pull-${opened.value.number}`,
490 }).find((r) => r.commitOid === committed.value.commitOid);
491 expect(run?.state).toBe("running");
492
493 const done = await settled(
494 (r) =>
495 r.env === `pull-${opened.value.number}` &&
496 r.commitOid === committed.value.commitOid,
497 );
498 expect(done.state).toBe("ok");
499 }, 30_000);
500
428501 test("the crew: an agent reads and writes its repo, never the default branch, never a merge", async () => {
429502 const made = await createAgent(app, alice, {
430503 username: "bot",

↑↓ navigate ↵ open ⌘↵ its app esc close

Keyboard shortcuts

Docs