trevato/forge

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

More
Forks1g fMapg aRuns16g rEventsFlowsAPI

The boundary review's open items: what an app, a pull and a hosted forge may… #12220

Merged masoncrew opened this in answer to trevatoowner

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

ConversationFiles
Merged

mason/forge:mason/12218-the-boundary-review-s-open-items-what-an → main 4 commits fork 2 threads · 2 unresolved

  • Merged. mason/12218-the-boundary-review-s-open-items-what-an was merged into main as d5b2fe2.

Review

Sign in to review this change.
12 files changed +66 −26
docs/APPS.md +11 −6
@@ -395,7 +395,7 @@ around it. None is advisory.
395395 plus variables set for scope `preview`, which win over `all` and
396396 `production` of the same name (`core/app_env.ts`). Set a payment key
397397 once for `production` (live) and once for `preview` (sandbox); a name
398 left in `all` reaches previews too, so a live key must never be there.
398 left in `all` reaches previews too (a new variable is production only unless its owner chooses "Also in previews"), so a live key must never be there.
399399 The forge does not know which names are payment keys and does not refuse
400400 one; the procedure is in `docs/features/platform-app-move.md`, step 4.
401401 4. **A deployment is a check.** Every deploy is a run; the run's state is
@@ -932,14 +932,19 @@ does not blank the log; a credential that short is not one to rely on.
932932
933933 The same list is in the terminal: an admin's repository screen has
934934 *Variables* in its `:` palette, which lists names and scopes, sets one with
935 `a` (a name, a value typed masked, a scope) and unsets one with `d` after a
935 `a` (a name, a value typed masked, a scope, production first) and unsets one with `d` after a
936936 confirm, refused in the page's words (`actions/app_env_words.ts`). MCP
937937 `app_env` does the same for an agent.
938938
939 A variable has a **scope**: everywhere (`all`, the default), `production`
940 only, or `preview` only — the select beside the value, or `"scope"` in the
941 `PUT` body and `?scope=` on the `DELETE`. The same name may be set once in
942 each scope, and where two apply the narrower one wins: a test key for
939 A variable has a **scope**: `production` (the default: a pull's preview
sentinelcrew outdated

Wrap this line like the rest of the paragraph, and state that an unscoped API/MCP call now means production.
940 never reads a secret its owner did not offer it), everywhere (`all`, offered
941 as "Also in previews") or `preview` only — the select beside the value,
942 or `"scope"` in the `PUT` body and `?scope=` on the `DELETE`. A call that
943 names no scope (API or MCP) now means production; it used to mean everywhere,
944 so a caller that relied on previews reading the value must send `"all"`.
945 The terminal's picker lists the same three, production first, and Enter on
946 the first sets production. The same name may be set once in each scope, and
947 where two apply the narrower one wins: a test key for
943948 previews, the live one for production, and neither process ever holds the
944949 other's. Every preview is one scope, whichever pull it serves.
945950
src/actions/app_env_words.ts +10
@@ -13,6 +13,16 @@ export const SCOPE_WORDS: Record<AppEnvScope, string> = {
1313 preview: "previews",
1414 };
1515
16 /**
17 * What a scope is called where one is chosen, the default first; the page's
18 * select says the same.
19 */
20 export const SCOPE_CHOICES: readonly { scope: AppEnvScope; label: string }[] = [
21 { scope: "production", label: "Production only" },
22 { scope: "all", label: "Also in previews" },
23 { scope: "preview", label: "Previews only" },
24 ];
25
1626 /** `setAppEnvVar`'s refusals, by code. */
1727 export const APP_ENV_ERRORS: Record<string, string> = {
1828 invalid_name: `A name is capital letters, digits and underscores, starting with a letter — ADMIN_PASSWORD, STRIPE_SECRET_KEY — and at most ${NAME_MAX} characters.`,
src/actions/apps.ts +3 −2
@@ -47,6 +47,7 @@ import {
4747 type UpdateAppError,
4848 } from "../core/apps.ts";
4949 import {
50 DEFAULT_ENV_SCOPE,
5051 deleteAppEnv,
5152 isAppEnvScope,
5253 listAppEnvNames,
@@ -168,7 +169,7 @@ export async function setAppEnvVar(
168169 repo: Repo,
169170 name: string,
170171 value: string,
171 scope: string = "all",
172 scope: string = DEFAULT_ENV_SCOPE,
172173 ): Promise<Result<AppEnvName, "not_found" | AppEnvError>> {
173174 const admin = requireRepoAdmin(app, actor, repo);
174175 if (!admin.ok) return admin;
@@ -199,7 +200,7 @@ export async function deleteAppEnvVar(
199200 actor: User | undefined,
200201 repo: Repo,
201202 name: string,
202 scope: string = "all",
203 scope: string = DEFAULT_ENV_SCOPE,
203204 ): Promise<Result<void, "not_found" | "no_such_var">> {
204205 const admin = requireRepoAdmin(app, actor, repo);
205206 if (!admin.ok) return admin;
src/core/app_env.ts +6
@@ -30,6 +30,12 @@ export function isAppEnvScope(value: string): value is AppEnvScope {
3030 return (APP_ENV_SCOPES as readonly string[]).includes(value);
3131 }
3232
33 /**
34 * What a variable with no scope named gets: production only. A preview runs a
35 * pull's own code, so a secret reaches one only when its owner says so.
36 */
37 export const DEFAULT_ENV_SCOPE: AppEnvScope = "production";
38
3339 /** The scope an environment's own variables live under. */
3440 export function scopeOfEnv(env: string): Exclude<AppEnvScope, "all"> {
3541 return env === "production" ? "production" : "preview";
src/http/api/app.ts +3 −2
@@ -20,6 +20,7 @@ import type { Ctx, Handler } from "../context.ts";
2020 import { holds, weakEtag } from "../etag.ts";
2121 import { BUILD_ASKS_WINDOW_MS } from "../../actions/build_preview.ts";
2222 import { apiError, body, json, notFound, requireScope, respondAccess, str, tryAgain, unauthorized } from "../respond.ts";
23 import { DEFAULT_ENV_SCOPE } from "../../core/app_env.ts";
2324 import type { Router } from "../router.ts";
2425
2526 function appPatch(input: Record<string, unknown>): AppSettingsInput {
@@ -256,7 +257,7 @@ export function appRoutes(router: Router<Handler>): void {
256257 repo,
257258 ctx.params.var ?? "",
258259 str(input.value),
259 str(input.scope) || "all",
260 str(input.scope) || DEFAULT_ENV_SCOPE,
260261 );
261262 if (!set.ok) return respondAccess(set.error, 422);
262263 return json(set.value);
@@ -275,7 +276,7 @@ export function appRoutes(router: Router<Handler>): void {
275276 ctx.auth.user,
276277 repo,
277278 ctx.params.var ?? "",
278 ctx.url.searchParams.get("scope") ?? "all",
279 ctx.url.searchParams.get("scope") ?? DEFAULT_ENV_SCOPE,
279280 );
280281 if (!removed.ok) return respondAccess(removed.error, 422);
281282 return json({}, 200);
src/http/mcp/tools/app_env.ts +3 −2
@@ -1,10 +1,11 @@
11 import { scopeAllows } from "../../../core/tokens.ts";
22 import { deleteAppEnvVar, listAppEnv, setAppEnvVar } from "../../../actions/apps.ts";
33 import { type Answer, good, bad, type Tool, REPO_ARG, repoArg, isAnswer } from "../shared.ts";
4 import { DEFAULT_ENV_SCOPE } from "../../../core/app_env.ts";
45
56 export const tool: Tool = {
67 name: "app_env",
7 doc: "`{ repo, action, name?, value?, scope? }`: `list` an app's variable names and scopes, `set` one, or `unset` it, in `all` (default), `production` or `preview`; the API's rules and errors, an admin of the repository with an admin-scope token; a value is never read back",
8 doc: "`{ repo, action, name?, value?, scope? }`: `list` an app's variable names and scopes, `set` one, or `unset` it, in `production` (default), `all` or `preview`; the API's rules and errors, an admin of the repository with an admin-scope token; a value is never read back",
89 description:
910 "An app's environment variables: list names, set or unset by scope; write-only. Admin.",
1011 inputSchema: {
@@ -25,7 +26,7 @@ export const tool: Tool = {
2526 const repo = repoArg(ctx, user, args);
2627 if (isAnswer(repo)) return repo;
2728 const action = args["action"];
28 const scope = typeof args["scope"] === "string" ? args["scope"] : "all";
29 const scope = typeof args["scope"] === "string" ? args["scope"] : DEFAULT_ENV_SCOPE;
2930 const name = typeof args["name"] === "string" ? args["name"] : "";
3031 // The API's errors, by code: never the value.
3132 const refused = (error: string): Answer =>
src/tui/screens/appenv.ts +4 −3
@@ -13,9 +13,10 @@ import {
1313 APP_ENV_ERRORS,
1414 APP_ENV_NO_SUCH,
1515 APP_ENV_NOT_SET,
16 SCOPE_CHOICES,
1617 SCOPE_WORDS,
1718 } from "../../actions/app_env_words.ts";
18 import { APP_ENV_SCOPES, type AppEnvName } from "../../core/app_env.ts";
19 import type { AppEnvName } from "../../core/app_env.ts";
1920 import { getAppByRepo } from "../../core/apps.ts";
2021 import { canAdmin, type Repo } from "../../core/repos.ts";
2122 import type { Ctx, Screen } from "../app/app.ts";
@@ -69,7 +70,7 @@ export function appEnvScreen(f: Session, ctx: Ctx, repo: Repo): Screen {
6970 reload();
7071 };
7172 const where = (v: { scope: AppEnvName["scope"] }): string =>
72 v.scope === "all" ? "everywhere" : `for ${SCOPE_WORDS[v.scope]}`;
73 v.scope === "all" ? "in production and previews" : `for ${SCOPE_WORDS[v.scope]}`;
7374
7475 const add = (): Screen =>
7576 promptScreen(ctx, screen, {
@@ -87,7 +88,7 @@ export function appEnvScreen(f: Session, ctx: Ctx, repo: Repo): Screen {
8788 later(ctx, () =>
8889 pickerScreen(ctx, screen, {
8990 title: `Where ${name} goes`,
90 items: APP_ENV_SCOPES.map((s) => ({ id: s, label: SCOPE_WORDS[s] })),
91 items: SCOPE_CHOICES.map((c) => ({ id: c.scope, label: c.label })),
9192 onPick: ([scope]) =>
9293 runAsync(ctx, async () => {
9394 const set = await setAppEnvVar(f.app, f.user, repo, name, value, scope);
src/web/pages/settings.ts +4 −3
@@ -42,6 +42,7 @@ import { allowBlocked, blockedView, ignoreBlocked, withLiveOnly, type Allowed, t
4242 import { blockedNote } from "../studio/blocked.ts";
4343 import type { DnsRecord } from "../../apps/domains.ts";
4444 import {
45 DEFAULT_ENV_SCOPE,
4546 listAppEnvNames,
4647 NAME_MAX,
4748 type AppEnvName,
@@ -888,8 +889,8 @@ export function appEnvView(base: string, names: AppEnvName[]): Html {
888889 required
889890 />
890891 <select class="input" name="scope" aria-label="Where it applies">
891 <option value="all">Everywhere</option>
892892 <option value="production">Production only</option>
893 <option value="all">Also in previews</option>
893894 <option value="preview">Previews only</option>
894895 </select>
895896 ${btn("Set", { variant: "primary", size: "sm", icon: "plus" })}
sentinelcrew outdated

Principle 1: this sentence explains what the select labels already say. Remove it.
@@ -1195,7 +1196,7 @@ export async function appEnvSet(ctx: Ctx): Promise<Response> {
11951196 repo,
11961197 name,
11971198 field(form, "value"),
1198 field(form, "scope") || "all",
1199 field(form, "scope") || DEFAULT_ENV_SCOPE,
11991200 );
12001201 if (!set.ok && set.error === "not_found") return repoNotFound(ctx);
12011202 return flashTo(
@@ -1225,7 +1226,7 @@ export async function appEnvDelete(ctx: Ctx): Promise<Response> {
12251226 ctx.auth.user,
12261227 repo,
12271228 name,
1228 field(form, "scope") || "all",
1229 field(form, "scope") || DEFAULT_ENV_SCOPE,
12291230 );
12301231 if (!removed.ok && removed.error === "not_found") return repoNotFound(ctx);
12311232 return flashTo(
test/app-env.test.ts +16
@@ -10,7 +10,9 @@ import { mkdtemp, rm } from "node:fs/promises";
1010 import { tmpdir } from "node:os";
1111 import { join } from "node:path";
1212 import { createApp, type App } from "../src/app.ts";
13 import { setAppEnvVar, deleteAppEnvVar } from "../src/actions/apps.ts";
1314 import { createRepo } from "../src/actions/repos.ts";
15 import { getRepoById } from "../src/core/repos.ts";
1416 import { appEnv } from "../src/apps/process.ts";
1517 import {
1618 deleteAppEnv,
@@ -24,6 +26,7 @@ import {
2426 deleteApp,
2527 type AppRecord,
2628 } from "../src/core/apps.ts";
29 import type { User } from "../src/core/users.ts";
2730 import { createUser } from "../src/core/users.ts";
2831 import { quietLogs } from "./helpers/globals.ts";
2932
@@ -34,6 +37,7 @@ describe("an app's environment", () => {
3437 let app: App;
3538 let hello: AppRecord;
3639 let twin: AppRecord;
40 let aliceUser: User;
3741
3842 const stored = (appId: string, name: string): string | undefined =>
3943 app.db
@@ -53,6 +57,7 @@ describe("an app's environment", () => {
5357 adminIfFirst: true,
5458 });
5559 if (!alice.ok) throw new Error(alice.error);
60 aliceUser = alice.value;
5661 for (const name of ["hello", "twin"]) {
5762 const repo = await createRepo(app, alice.value, {
5863 name,
@@ -279,6 +284,17 @@ describe("an app's environment", () => {
279284 expect(deleteAppEnv(app.db, twin.id, "DEBUG", "preview")).toBe(true);
280285 });
281286
287 test("a variable set with no scope is for production: a preview never reads it", async () => {
288 const user = aliceUser;
289 const repo = getRepoById(app.db, twin.repoId)!;
290 const set = await setAppEnvVar(app, user, repo, "NEW_KEY", "v");
291 expect(set.ok && set.value.scope).toBe("production");
292 expect(resolveAppEnv(app.db, dataDir, twin.id, "production")["NEW_KEY"]).toBe("v");
293 expect(resolveAppEnv(app.db, dataDir, twin.id, "pull-3")["NEW_KEY"]).toBeUndefined();
294 // And an unset with no scope removes what that set made.
295 expect((await deleteAppEnvVar(app, user, repo, "NEW_KEY")).ok).toBe(true);
296 });
297
282298 test("a scope is one of three words", () => {
283299 const bad = setAppEnv(app.db, dataDir, twin.id, "X_KEY", "v", "staging");
284300 expect(bad.ok).toBe(false);
test/apps-api.test.ts +3 −3
@@ -655,8 +655,8 @@ printf '%s\\n' '{"is_error":false,"result":"{\\"summary\\":\\"Build it\\",\\"che
655655 >("SELECT payload FROM event WHERE kind = 'app.env_set' ORDER BY id DESC")
656656 .get();
657657 // The name and where it applies; never the value.
658 expect(event?.payload).toBe('{"name":"GREETING","scope":"all"}');
659 expect(answer).toContain('"scope":"all"');
658 expect(event?.payload).toBe('{"name":"GREETING","scope":"production"}');
659 expect(answer).toContain('"scope":"production"');
660660
661661 // A scope is one of three words, and a preview-only value never
662662 // reaches production: the process that serves keeps the one for all.
@@ -679,7 +679,7 @@ printf '%s\\n' '{"is_error":false,"result":"{\\"summary\\":\\"Build it\\",\\"che
679679 items: { name: string; scope: string }[];
680680 }
681681 ).items.map((i) => `${i.name}@${i.scope}`),
682 ).toEqual(["GREETING@all", "GREETING@preview"]);
682 ).toEqual(["GREETING@production", "GREETING@preview"]);
683683 expect(
684684 (await api(`${env}/GREETING?scope=preview`, { method: "DELETE" })).status,
685685 ).toBe(200);
test/tui-app-env.test.ts +2 −3
@@ -31,7 +31,6 @@ quietLogs();
3131
3232 const SECRET = "sk_live_b4n4n4s";
3333 const RIGHT = "\x1b[C";
34 const DOWN = "\x1b[B";
3534
3635 let dir: string;
3736 let app: App;
@@ -96,8 +95,8 @@ afterAll(async () => {
9695 });
9796
9897 describe("an app's variables, in the terminal", () => {
99 test("an admin sets one: masked as it is typed, then listed by name and scope", async () => {
100 const text = await drive(alice, [...open, "a", "STRIPE_KEY", "\r", SECRET, "\r", DOWN, "\r"]);
98 test("an admin sets one: masked as it is typed, Enter on the first choice is production, then listed by name and scope", async () => {
99 const text = await drive(alice, [...open, "a", "STRIPE_KEY", "\r", SECRET, "\r", "\r"]);
101100 expect(vars().map((v) => [v.name, v.scope])).toEqual([["STRIPE_KEY", "production"]]);
102101 expect(text).toContain("STRIPE_KEY is set for production.");
103102 expect(text).toContain("STRIPE_KEY");
test/tui-appenv.test.ts +1 −2
@@ -26,7 +26,6 @@ import { proveAll } from "./helpers/verified.ts";
2626
2727 const SECRET = "hunter2_Zq9xW";
2828 const RIGHT = "\x1b[C";
29 const DOWN = "\x1b[B";
3029 const open = [":", "Variables", "\r"];
3130
3231 let dir: string;
@@ -93,7 +92,7 @@ afterAll(async () => {
9392
9493 describe("the terminal's variables screen", () => {
9594 test("an admin adds a masked value: name and scope listed, the value nowhere", async () => {
96 const text = await drive(admin, [...open, "a", "API_TOKEN", "\r", SECRET, "\r", DOWN, "\r"]);
95 const text = await drive(admin, [...open, "a", "API_TOKEN", "\r", SECRET, "\r", "\r"]);
9796 expect(vars().map((v) => [v.name, v.scope])).toEqual([["API_TOKEN", "production"]]);
9897 expect(text).toContain("API_TOKEN is set for production.");
9998 expect(text).toContain("production · set");

↑↓ navigate ↵ open ⌘↵ its app esc close

Keyboard shortcuts

Docs