From c8ffb9cc6f64fcaceef0dbfafb111b522f4cb7bd Mon Sep 17 00:00:00 2001 From: Danny Kim Date: Mon, 19 Oct 2026 02:30:54 +0900 Subject: [PATCH] Replace NodeJS dict aliases with maps and named field reads --- src/agents/auth.ts | 17 +++--- src/agents/opencode.ts | 20 ++++---- src/common/index.ts | 3 +- src/common/json.test.ts | 23 +++++++++ src/common/json.ts | 17 ++++-- src/post/post.ts | 59 +++++++++++---------- src/processes/isolation.test.ts | 13 +++-- src/profiles/profiles.test.ts | 76 +++++++++++++++------------ src/profiles/profiles.ts | 22 ++++---- src/run/index.ts | 83 ++++++++++++++++-------------- src/runtime/credentials.ts | 9 ++-- src/runtime/runtime.ts | 12 ++--- src/testing/index.ts | 20 ++++++-- tests/entrypoints.test.ts | 6 ++- tests/workflow.test.ts | 91 +++++++++++++++++---------------- 15 files changed, 275 insertions(+), 196 deletions(-) create mode 100644 src/common/json.test.ts diff --git a/src/agents/auth.ts b/src/agents/auth.ts index 9d6a2f7..8170a2a 100644 --- a/src/agents/auth.ts +++ b/src/agents/auth.ts @@ -12,7 +12,7 @@ import { } from "node:fs/promises"; import { dirname, join } from "node:path"; -import { isNotFound, isObject } from "#common"; +import { field, isNotFound, isObject } from "#common"; interface CodexApiKey { kind: "api-key"; @@ -89,15 +89,16 @@ async function readCodexAuth(path: string): Promise { } catch { throw new Error("invalid Codex login"); } - const tokens = isObject(value) ? value["tokens"] : undefined; + const authMode = field(value, "auth_mode"); + const tokens = field(value, "tokens"); if ( - !isObject(value) || - typeof value["auth_mode"] !== "string" || - value["auth_mode"] === "" || + typeof authMode !== "string" || + authMode === "" || !isObject(tokens) || - !["access_token", "refresh_token", "id_token", "account_id"].every( - (name) => typeof tokens[name] === "string" && tokens[name] !== "", - ) + !["access_token", "refresh_token", "id_token", "account_id"].every((name) => { + const token = field(tokens, name); + return typeof token === "string" && token !== ""; + }) ) { throw new Error("invalid Codex login"); } diff --git a/src/agents/opencode.ts b/src/agents/opencode.ts index 56778ab..0cc5489 100644 --- a/src/agents/opencode.ts +++ b/src/agents/opencode.ts @@ -1,4 +1,4 @@ -import { isObject } from "#common"; +import { field, isObject } from "#common"; import { AgentAuthError } from "#profiles"; export function openCodeArgs(model: string, prompt: string): string[] { @@ -56,13 +56,14 @@ export function openCodeError(output: string): Error | null { } catch { continue; } - if (!isObject(event)) continue; - const { type, error } = event; - if (type !== "error" || !isObject(error)) { + const error = field(event, "error"); + if (field(event, "type") !== "error" || !isObject(error)) { continue; } - const { name, data } = error; - const { message, statusCode } = isObject(data) ? data : {}; + const name = field(error, "name"); + const data = field(error, "data"); + const message = field(data, "message"); + const statusCode = field(data, "statusCode"); const detail = `opencode ${typeof name === "string" ? name : "error"}${ typeof statusCode === "number" ? ` (API error ${statusCode})` : "" }: ${typeof message === "string" ? message : "unknown error"}`; @@ -81,10 +82,9 @@ export function parseOpenCodeEvents(output: string): string { for (const line of output.split("\n")) { if (line.trim() === "") continue; const event: unknown = JSON.parse(line); - if (!isObject(event)) continue; - const { type, part } = event; - if (type !== "text" || !isObject(part)) continue; - const { text } = part; + const part = field(event, "part"); + if (field(event, "type") !== "text" || !isObject(part)) continue; + const text = field(part, "text"); if (typeof text === "string" && text.trim() !== "") result = text; } if (result === null) { diff --git a/src/common/index.ts b/src/common/index.ts index 71a7e51..77199dc 100644 --- a/src/common/index.ts +++ b/src/common/index.ts @@ -1,4 +1,3 @@ // Helpers every phase shares: error and JSON value checks. export { isNotFound } from "./errors.ts"; -export { isMapping, isObject } from "./json.ts"; -export type { JsonObject } from "./json.ts"; +export { field, isMapping, isObject } from "./json.ts"; diff --git a/src/common/json.test.ts b/src/common/json.test.ts new file mode 100644 index 0000000..ed7880d --- /dev/null +++ b/src/common/json.test.ts @@ -0,0 +1,23 @@ +import { expect, test } from "vite-plus/test"; + +import { field, isMapping, isObject } from "./json.ts"; + +test("reads a named field of a parsed object", () => { + const value: unknown = JSON.parse('{"login":"bot","user":{"login":"nested"}}'); + expect(field(value, "login")).toBe("bot"); + expect(field(field(value, "user"), "login")).toBe("nested"); + expect(field(value, "missing")).toBeUndefined(); +}); + +test("reads no field of a value that is not an object", () => { + for (const value of [null, undefined, "text", 1, true]) { + expect(field(value, "login")).toBeUndefined(); + } +}); + +test("counts arrays as objects but not as mappings", () => { + expect(isObject([])).toBe(true); + expect(isMapping([])).toBe(false); + expect(isMapping({})).toBe(true); + expect(field(["first"], "0")).toBe("first"); +}); diff --git a/src/common/json.ts b/src/common/json.ts index a75f091..be94440 100644 --- a/src/common/json.ts +++ b/src/common/json.ts @@ -1,12 +1,19 @@ -// Type guards for values read from JSON, which arrive as `unknown`. -export type JsonObject = NodeJS.Dict; +// Checks and field reads for values parsed from JSON, which arrive as +// `unknown`. Callers read the fields they know by name into their own +// declared interfaces. -// Any non-null object, arrays included: reading a missing key yields undefined. -export function isObject(value: unknown): value is JsonObject { +// Any non-null object, arrays included. +export function isObject(value: unknown): value is object { return typeof value === "object" && value !== null; } // A plain mapping, which an array is not. -export function isMapping(value: unknown): value is JsonObject { +export function isMapping(value: unknown): value is object { return isObject(value) && !Array.isArray(value); } + +// The named field of an object, or undefined for a missing field or a value +// that is not an object. +export function field(value: unknown, name: string): unknown { + return isObject(value) ? Reflect.get(value, name) : undefined; +} diff --git a/src/post/post.ts b/src/post/post.ts index 324b351..6d667d0 100644 --- a/src/post/post.ts +++ b/src/post/post.ts @@ -2,7 +2,7 @@ import type { Stats } from "node:fs"; import { lstat, readFile, rm } from "node:fs/promises"; import { join } from "node:path"; -import { isNotFound, isObject } from "#common"; +import { field, isNotFound, isObject } from "#common"; import { failureFooter, type FailureReason, type ProfileFailure, withProfile } from "#profiles"; import { readRuntimeManifest, type RuntimeManifest, type RuntimeOutcome } from "#runtime"; @@ -22,10 +22,10 @@ interface Anchor { const ANSI = new RegExp(`${String.fromCodePoint(27)}\\[[0-9;]*[A-Za-z]`, "gu"); const stripAnsi = (text: string): string => text.replace(ANSI, ""); -const VERDICTS: NodeJS.Dict<[event: string, mark: string]> = { - Approved: ["APPROVED", "✅"], - "Changes requested": ["REQUEST_CHANGES", "🛑"], -}; +const VERDICTS: Map = new Map([ + ["Approved", ["APPROVED", "✅"]], + ["Changes requested", ["REQUEST_CHANGES", "🛑"]], +]); const PROFILE_TYPES: Set = new Set(["claude", "codex", "opencode"]); const FAILURE_REASONS: Set = new Set(["sign-in failed", "usage exhausted", "failed"]); const isProfileType = (value: unknown): value is ProfileFailure["type"] => PROFILE_TYPES.has(value); @@ -46,7 +46,11 @@ function parseAnchors(text: string, sourcePath: string): Anchor[] { if (!isObject(entry)) { throw new TypeError(`${at}: not an object`); } - const { path, line, side = "new", body } = entry; + const path = field(entry, "path"); + const line = field(entry, "line"); + const givenSide = field(entry, "side"); + const side = givenSide === undefined ? "new" : givenSide; + const body = field(entry, "body"); if (typeof path !== "string" || path === "") { throw new TypeError(`${at}.path: expected a repository path`); } @@ -68,43 +72,42 @@ async function readOutcome(path: string): Promise { if (!isObject(value) || !("ok" in value)) { throw new Error("invalid agent outcome"); } - const outcome = value; - if (outcome["ok"] === false) { + const ok = field(value, "ok"); + if (ok === false) { // Only known labels reach the comment, never raw agent output. - const failures: unknown[] = Array.isArray(outcome["failures"]) ? outcome["failures"] : []; + const listed = field(value, "failures"); + const failures: unknown[] = Array.isArray(listed) ? listed : []; return { ok: false, failures: failures.flatMap((entry: unknown) => { - const { type, model, reason } = isObject(entry) ? entry : {}; + const type = field(entry, "type"); + const model = field(entry, "model"); + const reason = field(entry, "reason"); return isProfileType(type) && typeof model === "string" && isFailureReason(reason) ? [{ type, model, reason }] : []; }), }; } + const body = field(value, "body"); + const reviewPath = field(value, "reviewPath"); + const anchorsPath = field(value, "anchorsPath"); + const profile = field(value, "profile"); if ( - outcome["ok"] !== true || - typeof outcome["body"] !== "string" || - typeof outcome["reviewPath"] !== "string" || - typeof outcome["anchorsPath"] !== "string" || - !isObject(outcome["profile"]) + ok !== true || + typeof body !== "string" || + typeof reviewPath !== "string" || + typeof anchorsPath !== "string" || + !isObject(profile) ) { throw new Error("invalid agent outcome"); } - const profile = outcome["profile"]; - if (!isProfileType(profile["type"]) || typeof profile["model"] !== "string") { + const type = field(profile, "type"); + const model = field(profile, "model"); + if (!isProfileType(type) || typeof model !== "string") { throw new Error("invalid agent outcome"); } - return { - ok: true, - body: outcome["body"], - reviewPath: outcome["reviewPath"], - anchorsPath: outcome["anchorsPath"], - profile: { - type: profile["type"], - model: profile["model"], - }, - }; + return { ok: true, body, reviewPath, anchorsPath, profile: { type, model } }; } async function readAgentArtifact( @@ -188,7 +191,7 @@ export async function postRuntime( } const [verdict = "", ...rest] = review.split("\n"); - const [event, mark] = VERDICTS[verdict.trim()] ?? ["COMMENT", "💬"]; + const [event, mark] = VERDICTS.get(verdict.trim()) ?? ["COMMENT", "💬"]; const anchorText = await readAgentArtifact(manifest, outcome.anchorsPath, "anchors.json"); const anchors = anchorText === null ? [] : parseAnchors(anchorText, outcome.anchorsPath); const postReview = (entries: Anchor[]): Promise => diff --git a/src/processes/isolation.test.ts b/src/processes/isolation.test.ts index 3493a55..22353ec 100644 --- a/src/processes/isolation.test.ts +++ b/src/processes/isolation.test.ts @@ -25,14 +25,14 @@ function runAs( selected: RuntimeProfile, command: string, args: string[], - env?: NodeJS.ProcessEnv, + env?: ReadonlyMap, ): SpawnSyncReturns { return spawnSync(command, args, { cwd: "/", uid: selected.uid, gid: selected.gid, encoding: "utf8", - ...(env !== undefined && { env }), + ...(env !== undefined && { env: Object.fromEntries(env) }), }); } @@ -104,9 +104,12 @@ test.skipIf(!IS_ROOT)("cleanup leaves no runnable daemonized process", async () test.skipIf(!IS_ROOT)("the agent process cannot read its coordinator environment", () => { const selected = profile(20_020, "/tmp"); - const result = runAs(selected, "/usr/bin/cat", [`/proc/${process.pid}/environ`], { - PARENT_SECRET: "must-not-cross-boundary", - }); + const result = runAs( + selected, + "/usr/bin/cat", + [`/proc/${process.pid}/environ`], + new Map([["PARENT_SECRET", "must-not-cross-boundary"]]), + ); expect(result.status).not.toBe(0); }); diff --git a/src/profiles/profiles.test.ts b/src/profiles/profiles.test.ts index 067c48a..6b9d337 100644 --- a/src/profiles/profiles.test.ts +++ b/src/profiles/profiles.test.ts @@ -157,20 +157,22 @@ test("does not mistake incidental quota text for exhaustion", async () => { }); test("exposes only the selected profile credential to the agent", () => { - const base = { - PATH: "/usr/bin", - HOME: "/home/bot", - UNRELATED_SECRET: "must-not-reach-agent", - BOT_AGENTS_MASTER_KEY: "master", - PROFILES: "contains every secret", - REVIEWER_TOKEN: "reviewer", - CLAUDE_CODE_OAUTH_TOKEN: "old-claude", - ANTHROPIC_API_KEY: "old-anthropic-key", - ANTHROPIC_AUTH_TOKEN: "old-anthropic-token", - OPENAI_API_KEY: "old-openai", - CODEX_API_KEY: "old-codex-key", - CODEX_ACCESS_TOKEN: "old-codex-token", - }; + const base = new Map( + Object.entries({ + PATH: "/usr/bin", + HOME: "/home/bot", + UNRELATED_SECRET: "must-not-reach-agent", + BOT_AGENTS_MASTER_KEY: "master", + PROFILES: "contains every secret", + REVIEWER_TOKEN: "reviewer", + CLAUDE_CODE_OAUTH_TOKEN: "old-claude", + ANTHROPIC_API_KEY: "old-anthropic-key", + ANTHROPIC_AUTH_TOKEN: "old-anthropic-token", + OPENAI_API_KEY: "old-openai", + CODEX_API_KEY: "old-codex-key", + CODEX_ACCESS_TOKEN: "old-codex-token", + }), + ); const profile = { index: 0, type: "claude" as const, @@ -181,15 +183,19 @@ test("exposes only the selected profile credential to the agent", () => { tokenFile: "/runtime/profile-0/token", }; - expect(agentEnvironment(base, profile, "selected")).toStrictEqual({ - PATH: "/usr/bin", - HOME: "/runtime/profile-0", - CLAUDE_CODE_OAUTH_TOKEN: "selected", - }); - expect(sanitizedEnvironment(base)).toStrictEqual({ - PATH: "/usr/bin", - HOME: "/home/bot", - }); + expect(agentEnvironment(base, profile, "selected")).toStrictEqual( + new Map([ + ["PATH", "/usr/bin"], + ["HOME", "/runtime/profile-0"], + ["CLAUDE_CODE_OAUTH_TOKEN", "selected"], + ]), + ); + expect(sanitizedEnvironment(base)).toStrictEqual( + new Map([ + ["PATH", "/usr/bin"], + ["HOME", "/home/bot"], + ]), + ); }); test("uses a private Codex home without inventing an API credential", () => { @@ -203,11 +209,13 @@ test("uses a private Codex home without inventing an API credential", () => { tokenFile: null, }; - expect(agentEnvironment({ PATH: "/bin" }, profile)).toStrictEqual({ - PATH: "/bin", - HOME: "/runtime/profile-1", - CODEX_HOME: "/runtime/profile-1/.codex", - }); + expect(agentEnvironment(new Map([["PATH", "/bin"]]), profile)).toStrictEqual( + new Map([ + ["PATH", "/bin"], + ["HOME", "/runtime/profile-1"], + ["CODEX_HOME", "/runtime/profile-1/.codex"], + ]), + ); }); test("exposes an OpenRouter key only to the OpenCode profile", () => { @@ -221,9 +229,11 @@ test("exposes an OpenRouter key only to the OpenCode profile", () => { tokenFile: "/runtime/profile-2/token", }; - expect(agentEnvironment({ PATH: "/bin" }, profile, "selected")).toStrictEqual({ - PATH: "/bin", - HOME: "/runtime/profile-2", - OPENROUTER_API_KEY: "selected", - }); + expect(agentEnvironment(new Map([["PATH", "/bin"]]), profile, "selected")).toStrictEqual( + new Map([ + ["PATH", "/bin"], + ["HOME", "/runtime/profile-2"], + ["OPENROUTER_API_KEY", "selected"], + ]), + ); }); diff --git a/src/profiles/profiles.ts b/src/profiles/profiles.ts index 80613bb..65d4f4a 100644 --- a/src/profiles/profiles.ts +++ b/src/profiles/profiles.ts @@ -80,8 +80,12 @@ export function runProfiles( return attemptFrom(profiles, attempt, failures, 0); } -export function sanitizedEnvironment(environment: NodeJS.ProcessEnv): NodeJS.ProcessEnv { - const result: NodeJS.ProcessEnv = {}; +// Environments are maps from variable name to value, converted to the object +// form only where a child process is spawned. +export function sanitizedEnvironment( + environment: ReadonlyMap, +): Map { + const result: Map = new Map(); for (const name of [ "PATH", "HOME", @@ -108,27 +112,27 @@ export function sanitizedEnvironment(environment: NodeJS.ProcessEnv): NodeJS.Pro "SSL_CERT_DIR", "NODE_EXTRA_CA_CERTS", ]) { - const value = environment[name]; - if (value !== undefined) result[name] = value; + const value = environment.get(name); + if (value !== undefined) result.set(name, value); } return result; } export function agentEnvironment( - environment: NodeJS.ProcessEnv, + environment: ReadonlyMap, profile: ProfileHome, token?: string, -): NodeJS.ProcessEnv { +): Map { const result = sanitizedEnvironment(environment); - result["HOME"] = profile.home; - if (profile.type === "codex") result["CODEX_HOME"] = `${profile.home}/.codex`; + result.set("HOME", profile.home); + if (profile.type === "codex") result.set("CODEX_HOME", `${profile.home}/.codex`); if (token !== undefined && token !== "") { const tokenName = { claude: "CLAUDE_CODE_OAUTH_TOKEN", codex: "OPENAI_API_KEY", opencode: "OPENROUTER_API_KEY", }[profile.type]; - result[tokenName] = token; + result.set(tokenName, token); } return result; } diff --git a/src/run/index.ts b/src/run/index.ts index a4820e4..53e5bc8 100644 --- a/src/run/index.ts +++ b/src/run/index.ts @@ -24,7 +24,7 @@ import { retryInvalidToken, stageCodexAuth, } from "#agents"; -import { isObject } from "#common"; +import { field, isObject } from "#common"; import { echo, exitCode, killProfileProcesses, readProfileFile } from "#processes"; import { AgentAuthError, @@ -62,11 +62,11 @@ const env = (name: string): string => { return value; }; -function definedEnvironment(): NodeJS.ProcessEnv { - const result: NodeJS.ProcessEnv = {}; +function definedEnvironment(): Map { + const result: Map = new Map(); for (const [name, value] of Object.entries(process.env)) { if (value !== undefined) { - result[name] = value; + result.set(name, value); } } return result; @@ -104,17 +104,21 @@ async function postAuthorComment(body: string): Promise { } async function runRoot(command: string, args: string[]): Promise { - const code = await exitCode(spawn(command, args, { env: PROCESS_ENV, stdio: "ignore" })); + const code = await exitCode( + spawn(command, args, { env: Object.fromEntries(PROCESS_ENV), stdio: "ignore" }), + ); if (code !== 0) throw new Error(`${command} ${args[0]} exited with ${code}`); } async function configureGitAuthor(): Promise { const user: unknown = await (await gitea("user")).json(); - if (!isObject(user) || typeof user["login"] !== "string" || typeof user["email"] !== "string") { - throw new Error("user: unexpected Gitea response"); + const login = field(user, "login"); + const email = field(user, "email"); + if (typeof login !== "string" || typeof email !== "string") { + throw new TypeError("user: unexpected Gitea response"); } - await runRoot("git", ["config", "user.name", user["login"]]); - await runRoot("git", ["config", "user.email", user["email"]]); + await runRoot("git", ["config", "user.name", login]); + await runRoot("git", ["config", "user.email", email]); } async function readProfileToken(profile: RuntimeProfile): Promise { @@ -130,7 +134,7 @@ async function runAgentCommand( ): Promise { const child = spawn(command, args, { cwd: WORKSPACE, - env: agentEnvironment(PROCESS_ENV, profile, token), + env: Object.fromEntries(agentEnvironment(PROCESS_ENV, profile, token)), uid: profile.uid, gid: profile.gid, stdio: "ignore", @@ -159,8 +163,9 @@ async function prepareProfile(profile: RuntimeProfile): Promise { // One issue comment as the prompt quotes it. function formatComment(comment: unknown): string { - const { user, created_at: createdAt, body } = isObject(comment) ? comment : {}; - const login = isObject(user) ? user["login"] : undefined; + const login = field(field(comment, "user"), "login"); + const createdAt = field(comment, "created_at"); + const body = field(comment, "body"); return `## ${String(login)} at ${String(createdAt)}\n\n${String(body)}\n`; } @@ -173,22 +178,22 @@ async function renderPrompt(profile: RuntimeProfile): Promise { if (!Array.isArray(comments)) { throw new TypeError("comments: unexpected Gitea response"); } - const values: NodeJS.ProcessEnv = { - COMMENT: env("COMMENT"), - ISSUE_COMMENTS: comments.map((comment: unknown) => formatComment(comment)).join("\n"), - CODE_RULES: await (await gitea(RULES_PATH)).text(), - EVENT_NAME: EVENT, - GITEA_API_URL: API, - GITEA_REPOSITORY: REPO, - ISSUE_INDEX: INDEX, - REVIEW_PATH: reviewPath, - ANCHORS_PATH: anchorsPath, - }; + const values: Map = new Map([ + ["COMMENT", env("COMMENT")], + ["ISSUE_COMMENTS", comments.map((comment: unknown) => formatComment(comment)).join("\n")], + ["CODE_RULES", await (await gitea(RULES_PATH)).text()], + ["EVENT_NAME", EVENT], + ["GITEA_API_URL", API], + ["GITEA_REPOSITORY", REPO], + ["ISSUE_INDEX", INDEX], + ["REVIEW_PATH", reviewPath], + ["ANCHORS_PATH", anchorsPath], + ]); const template = await readFile(new URL("../../prompt.md", import.meta.url), "utf8"); return { prompt: template.replaceAll( /\$\{(\w+)\}/gu, - (match: string, name: string) => values[name] ?? match, + (match: string, name: string) => values.get(name) ?? match, ), reviewPath, anchorsPath, @@ -210,10 +215,7 @@ async function runClaude(profile: RuntimeProfile, token: string, prompt: string) ], { cwd: WORKSPACE, - env: { - ...agentEnvironment(PROCESS_ENV, profile, token), - IS_SANDBOX: "1", - }, + env: Object.fromEntries(agentEnvironment(PROCESS_ENV, profile, token).set("IS_SANDBOX", "1")), uid: profile.uid, gid: profile.gid, stdio: ["ignore", "pipe", "pipe"], @@ -234,17 +236,20 @@ async function runClaude(profile: RuntimeProfile, token: string, prompt: string) if (line !== "") { const event: unknown = JSON.parse(line); if (isObject(event)) { - const content = isObject(event["message"]) ? event["message"]["content"] : undefined; + const content = field(field(event, "message"), "content"); for (const part of Array.isArray(content) ? content : []) { if (isObject(part)) { - const text = part["thinking"] ?? part["text"] ?? part["name"]; + const text = field(part, "thinking") ?? field(part, "text") ?? field(part, "name"); if (typeof text === "string" && text !== "") { console.log(text); } } } - if (event["type"] === "result") { - const { is_error: isError, result: answer, subtype, api_error_status: apiStatus } = event; + if (field(event, "type") === "result") { + const isError = field(event, "is_error"); + const answer = field(event, "result"); + const subtype = field(event, "subtype"); + const apiStatus = field(event, "api_error_status"); result = { isError: typeof isError === "boolean" ? isError : undefined, result: typeof answer === "string" ? answer : undefined, @@ -289,11 +294,11 @@ async function pollForInstructions( async function codexDeviceLogin( profile: RuntimeProfile, - environment: NodeJS.ProcessEnv, + environment: ReadonlyMap, ): Promise { const login = spawn("codex", ["login", "--device-auth"], { cwd: WORKSPACE, - env: environment, + env: Object.fromEntries(environment), uid: profile.uid, gid: profile.gid, stdio: ["ignore", "pipe", "pipe"], @@ -318,12 +323,12 @@ async function codexDeviceLogin( async function codexApiLogin( profile: RuntimeProfile, - environment: NodeJS.ProcessEnv, + environment: ReadonlyMap, token: string, ): Promise { const login = spawn("codex", ["login", "--with-api-key"], { cwd: WORKSPACE, - env: environment, + env: Object.fromEntries(environment), uid: profile.uid, gid: profile.gid, stdio: ["pipe", "inherit", "pipe"], @@ -357,7 +362,7 @@ async function runCodex( } else { const check = spawn("codex", ["login", "status"], { cwd: WORKSPACE, - env: environment, + env: Object.fromEntries(environment), uid: profile.uid, gid: profile.gid, stdio: "ignore", @@ -379,7 +384,7 @@ async function runCodex( ], { cwd: WORKSPACE, - env: environment, + env: Object.fromEntries(environment), uid: profile.uid, gid: profile.gid, stdio: ["ignore", "inherit", "pipe"], @@ -420,7 +425,7 @@ async function runOpenCode( ): Promise { const child = spawn("opencode", openCodeArgs(profile.model, prompt), { cwd: WORKSPACE, - env: agentEnvironment(PROCESS_ENV, profile, token), + env: Object.fromEntries(agentEnvironment(PROCESS_ENV, profile, token)), uid: profile.uid, gid: profile.gid, stdio: ["ignore", "pipe", "pipe"], diff --git a/src/runtime/credentials.ts b/src/runtime/credentials.ts index 09a96d1..d594d2f 100644 --- a/src/runtime/credentials.ts +++ b/src/runtime/credentials.ts @@ -1,6 +1,6 @@ import { webcrypto } from "node:crypto"; -import { isMapping } from "#common"; +import { field, isMapping } from "#common"; import type { BotProfile } from "#profiles"; interface DefaultModels { @@ -80,7 +80,9 @@ async function parseProfile(entry: unknown, index: number, masterKey: string): P throw new Error(`${at}.${key}: unknown field`); } } - const { type, model, token } = entry; + const type = field(entry, "type"); + const model = field(entry, "model"); + const token = field(entry, "token"); if (type !== "claude" && type !== "codex" && type !== "opencode") { throw new Error(`${at}.type: expected "claude", "codex", or "opencode"`); } @@ -112,7 +114,8 @@ export async function parseCredentials(text: string, masterKey: string): Promise throw new Error(`${key}: unknown field`); } } - const { "reviewer-token": reviewerToken, profiles } = value; + const reviewerToken = field(value, "reviewer-token"); + const profiles = field(value, "profiles"); if (typeof reviewerToken !== "string") { throw new TypeError("reviewer-token: expected an encrypted credential"); } diff --git a/src/runtime/runtime.ts b/src/runtime/runtime.ts index edafa40..fba9fc3 100644 --- a/src/runtime/runtime.ts +++ b/src/runtime/runtime.ts @@ -1,7 +1,7 @@ import { chmod, chown, mkdir, mkdtemp, readFile, rm, writeFile } from "node:fs/promises"; import { join } from "node:path"; -import { isMapping } from "#common"; +import { field, isMapping } from "#common"; import type { ProfileFailure, ProfileIdentity } from "#profiles"; import { parseCredentials } from "./credentials.ts"; @@ -107,11 +107,11 @@ export async function prepareRuntime( function isRuntimeManifest(value: unknown): value is RuntimeManifest { return ( isMapping(value) && - typeof value["runtimeDir"] === "string" && - typeof value["manifestPath"] === "string" && - typeof value["reviewerFile"] === "string" && - typeof value["outcomeFile"] === "string" && - Array.isArray(value["profiles"]) + typeof field(value, "runtimeDir") === "string" && + typeof field(value, "manifestPath") === "string" && + typeof field(value, "reviewerFile") === "string" && + typeof field(value, "outcomeFile") === "string" && + Array.isArray(field(value, "profiles")) ); } diff --git a/src/testing/index.ts b/src/testing/index.ts index 2558862..0284e01 100644 --- a/src/testing/index.ts +++ b/src/testing/index.ts @@ -1,5 +1,5 @@ -// Test support: the current user's IDs, a fresh temporary directory, and a -// plain command runner. It imports no source module, so the repository-level +// Test support: the current user's IDs, a fresh temporary directory, the +// inherited environment, and a plain command runner. It imports no source module, so the repository-level // tests in tests/, which reach src/ only through `#testing`, never touch // internals. import { spawn } from "node:child_process"; @@ -33,14 +33,26 @@ export interface CommandResult { stderr: string; } +// This process's environment as a map from variable name to value. +export function inheritedEnvironment(): Map { + const result: Map = new Map(); + for (const [name, value] of Object.entries(process.env)) { + if (value !== undefined) result.set(name, value); + } + return result; +} + // Runs a command with stdin closed and only the given environment, and // collects its stderr. export async function runCommand( command: string, args: string[], - env: NodeJS.ProcessEnv, + env: ReadonlyMap, ): Promise { - const child = spawn(command, args, { env, stdio: ["ignore", "ignore", "pipe"] }); + const child = spawn(command, args, { + env: Object.fromEntries(env), + stdio: ["ignore", "ignore", "pipe"], + }); const [stderr, closed] = await Promise.all([text(child.stderr), once(child, "close")]); const exitCode: unknown = closed[0]; return { success: exitCode === 0, stderr }; diff --git a/tests/entrypoints.test.ts b/tests/entrypoints.test.ts index 8f33d95..ed7af66 100644 --- a/tests/entrypoints.test.ts +++ b/tests/entrypoints.test.ts @@ -15,7 +15,11 @@ const PHASES: [phase: string, firstVariable: string][] = [ test.each(PHASES)("Node loads the %s phase", async (phase, firstVariable) => { const path = fileURLToPath(new URL(`../src/${phase}/index.ts`, import.meta.url)); - const result = await runCommand(process.execPath, [path], { PATH: process.env["PATH"] ?? "" }); + const result = await runCommand( + process.execPath, + [path], + new Map([["PATH", process.env["PATH"] ?? ""]]), + ); expect(result.success).toBe(false); expect(result.stderr).toContain(`${firstVariable} is not set`); }); diff --git a/tests/workflow.test.ts b/tests/workflow.test.ts index 0028e2f..0c96fa1 100644 --- a/tests/workflow.test.ts +++ b/tests/workflow.test.ts @@ -3,24 +3,22 @@ import process from "node:process"; import { assert, expect, test } from "vite-plus/test"; import { parse } from "yaml"; -import { makeTempDir, runCommand } from "#testing"; +import { inheritedEnvironment, makeTempDir, runCommand } from "#testing"; -type JsonObject = NodeJS.Dict; +// A parsed YAML or JSON mapping, keyed by its dynamic field names. +type Mapping = Map; -const isMapping = (value: unknown): value is JsonObject => - typeof value === "object" && value !== null && !Array.isArray(value); - -const mapping = (value: unknown, at: string): JsonObject => { - assert(isMapping(value), at); - return value; +const mapping = (value: unknown, at: string): Mapping => { + assert(typeof value === "object" && value !== null && !Array.isArray(value), at); + return new Map(Object.entries(value)); }; -const sequence = (value: unknown, at: string): JsonObject[] => { +const sequence = (value: unknown, at: string): Mapping[] => { assert(Array.isArray(value), at); return value.map((entry: unknown, index) => mapping(entry, `${at}[${index}]`)); }; -async function yaml(path: string): Promise { +async function yaml(path: string): Promise { const value: unknown = parse(await readFile(path, "utf8")); return mapping(value, path); } @@ -33,8 +31,8 @@ async function yamlFiles(): Promise { .filter((path) => !/^(?:\.git|\.superpowers|node_modules)\//u.test(path)); } -const findStep = (steps: JsonObject[], key: string, value: string): JsonObject => { - const step = steps.find((entry) => entry[key] === value); +const findStep = (steps: Mapping[], key: string, value: string): Mapping => { + const step = steps.find((entry) => entry.get(key) === value); assert(step !== undefined, `${key}: ${value}`); return step; }; @@ -47,11 +45,12 @@ test("all committed YAML parses", async () => { test("the action exposes only the master key and scopes it to preparation", async () => { const action = await yaml("action.yml"); - expect(Object.keys(mapping(action["inputs"], "inputs"))).toStrictEqual(["master-key"]); - const steps = sequence(mapping(action["runs"], "runs")["steps"], "runs.steps"); + expect([...mapping(action.get("inputs"), "inputs").keys()]).toStrictEqual(["master-key"]); + const steps = sequence(mapping(action.get("runs"), "runs").get("steps"), "runs.steps"); for (const step of steps) { - if (typeof step["run"] === "string") { - expect(step["run"].includes("\n"), String(step["name"])).toBe(false); + const run = step.get("run"); + if (typeof run === "string") { + expect(run.includes("\n"), String(step.get("name"))).toBe(false); } } const checkout = findStep(steps, "uses", "actions/checkout@v4"); @@ -59,16 +58,16 @@ test("the action exposes only the master key and scopes it to preparation", asyn const agent = findStep(steps, "id", "agent"); const post = findStep(steps, "id", "post"); findStep(steps, "id", "source"); - expect(mapping(checkout["with"], "checkout.with")["token"]).toBe("${{ gitea.token }}"); - expect(mapping(prepare["env"], "prepare.env")["BOT_AGENTS_MASTER_KEY"]).toBe( + expect(mapping(checkout.get("with"), "checkout.with").get("token")).toBe("${{ gitea.token }}"); + expect(mapping(prepare.get("env"), "prepare.env").get("BOT_AGENTS_MASTER_KEY")).toBe( "${{ inputs.master-key }}", ); - expect(JSON.stringify(agent).includes("BOT_AGENTS_MASTER_KEY")).toBe(false); - expect(JSON.stringify(post).includes("BOT_AGENTS_MASTER_KEY")).toBe(false); - expect(mapping(agent["env"], "agent.env")["GITEA_TOKEN"]).toBe("${{ gitea.token }}"); - assert(String(post["if"]).includes("always()")); + expect(JSON.stringify(Object.fromEntries(agent)).includes("BOT_AGENTS_MASTER_KEY")).toBe(false); + expect(JSON.stringify(Object.fromEntries(post)).includes("BOT_AGENTS_MASTER_KEY")).toBe(false); + expect(mapping(agent.get("env"), "agent.env").get("GITEA_TOKEN")).toBe("${{ gitea.token }}"); + assert(String(post.get("if")).includes("always()")); for (const step of [prepare, agent, post]) { - assert(String(step["run"]).includes("steps.source.outputs.path")); + assert(String(step.get("run")).includes("steps.source.outputs.path")); } }); @@ -76,12 +75,11 @@ test("the source protection script stages immutable action files", async () => { const temp = await makeTempDir(); const outputFile = `${temp}/output`; try { - const result = await runCommand("/bin/bash", ["scripts/protect-source.sh"], { - ...process.env, - ACTION_PATH: process.cwd(), - GITHUB_OUTPUT: outputFile, - RUNNER_TEMP: temp, - }); + const environment = inheritedEnvironment(); + environment.set("ACTION_PATH", process.cwd()); + environment.set("GITHUB_OUTPUT", outputFile); + environment.set("RUNNER_TEMP", temp); + const result = await runCommand("/bin/bash", ["scripts/protect-source.sh"], environment); assert(result.success, result.stderr); const path = (await readFile(outputFile, "utf8")).trim().slice(5); @@ -119,27 +117,30 @@ test("module-relative runtime assets exist", async () => { test("the repository uses its trusted main action", async () => { const workflow = await yaml(".gitea/workflows/agents.yml"); - const job = mapping(mapping(workflow["jobs"], "jobs")["agent"], "jobs.agent"); - const permissions = mapping(job["permissions"], "jobs.agent.permissions"); - expect(permissions).toStrictEqual({ + const job = mapping(mapping(workflow.get("jobs"), "jobs").get("agent"), "jobs.agent"); + const permissions = mapping(job.get("permissions"), "jobs.agent.permissions"); + expect(Object.fromEntries(permissions)).toStrictEqual({ code: "write", issues: "write", "pull-requests": "write", }); - const steps = sequence(job["steps"], "jobs.agent.steps"); - expect(steps.findIndex((step) => step["uses"] === "./")).toStrictEqual(-1); + const steps = sequence(job.get("steps"), "jobs.agent.steps"); + expect(steps.findIndex((step) => step.get("uses") === "./")).toStrictEqual(-1); const trusted = findStep(steps, "uses", "https://git.capsulizers.com/commons/bot-agents@main"); - expect(mapping(trusted["with"], "trusted.with")["master-key"]).toBe( + expect(mapping(trusted.get("with"), "trusted.with").get("master-key")).toBe( "${{ secrets.BOT_AGENTS_MASTER_KEY }}", ); - expect(JSON.stringify(workflow).includes("gitea.actor != 'bot'")).toBe(true); + expect(JSON.stringify(Object.fromEntries(workflow)).includes("gitea.actor != 'bot'")).toBe(true); }); test("CI checks every phase and the root isolation suite", async () => { const workflow = await yaml(".gitea/workflows/check.yml"); - const job = mapping(mapping(workflow["jobs"], "jobs")["check"], "jobs.check"); - const commands = sequence(job["steps"], "jobs.check.steps") - .map((step) => (typeof step["run"] === "string" ? step["run"] : "")) + const job = mapping(mapping(workflow.get("jobs"), "jobs").get("check"), "jobs.check"); + const commands = sequence(job.get("steps"), "jobs.check.steps") + .map((step) => { + const run = step.get("run"); + return typeof run === "string" ? run : ""; + }) .join("\n"); // `vp check` type-checks every phase, and `vp test` runs each of them under // Node as well as the root-only isolation tests. @@ -176,8 +177,12 @@ test("profile preparation does not receive provider credentials", async () => { test("central profiles fall back from Codex to Claude to OpenCode", async () => { const credentials: unknown = JSON.parse(await readFile("profiles.json", "utf8")); - const profiles = sequence(mapping(credentials, "profiles.json")["profiles"], "profiles"); - expect(profiles.map((profile) => profile["type"])).toStrictEqual(["codex", "claude", "opencode"]); - expect(profiles[2]?.["model"]).toBe("openrouter/qwen/qwen3.8-27b"); - assert(String(profiles[2]?.["token"]).startsWith("v1:")); + const profiles = sequence(mapping(credentials, "profiles.json").get("profiles"), "profiles"); + expect(profiles.map((profile) => profile.get("type"))).toStrictEqual([ + "codex", + "claude", + "opencode", + ]); + expect(profiles[2]?.get("model")).toBe("openrouter/qwen/qwen3.8-27b"); + assert(String(profiles[2]?.get("token")).startsWith("v1:")); });