refactor: replace requireEnv/optionalEnv with env(name, fallback)
Bundles must run without env vars — env vars are overrides, not requirements.
Single function: env(name, fallback) always returns string with a default.
- Removed requireEnv and optionalEnv
- Updated bundle entries, tests, and skill docs
小橘 🍊
This commit is contained in:
@@ -301,28 +301,26 @@ function createLazyAdapter(): AdapterFn {
|
|||||||
}
|
}
|
||||||
\`\`\`
|
\`\`\`
|
||||||
|
|
||||||
### Agent CLI paths: use optionalEnv with defaults
|
### Agent CLI paths: use env() with absolute path defaults
|
||||||
|
|
||||||
When binding agent adapters (cursor-agent, hermes, etc.), **always use \`optionalEnv\` with a sensible default** — never \`requireEnv\`. The worker process may start without the expected env vars, causing a silent crash.
|
Every env var in a bundle must have a sensible default — bundles must run without any env vars set. Use \`env(name, fallback)\` from \`@uncaged/workflow-util\`.
|
||||||
|
|
||||||
Discover the correct CLI path yourself (e.g. \`which cursor-agent\`, \`which hermes\`) and hardcode it as the fallback:
|
Discover the correct CLI path yourself (e.g. \`which cursor-agent\`, \`which hermes\`) and hardcode it as the fallback:
|
||||||
|
|
||||||
\`\`\`typescript
|
\`\`\`typescript
|
||||||
// ❌ WRONG — worker crash if env var missing, thread silently fails with 0 steps
|
import { env } from "@uncaged/workflow-util";
|
||||||
|
|
||||||
|
// ❌ WRONG — requireEnv and optionalEnv no longer exist
|
||||||
const adapter = createCursorAgent({
|
const adapter = createCursorAgent({
|
||||||
command: requireEnv("WORKFLOW_CURSOR_COMMAND", "set it"),
|
command: requireEnv("WORKFLOW_CURSOR_COMMAND", "set it"),
|
||||||
...
|
...
|
||||||
});
|
});
|
||||||
|
|
||||||
// ❌ WRONG — bare command name fails absolute-path validation
|
// ✅ CORRECT — env var is an override, fallback is the discovered absolute path
|
||||||
const adapter = createCursorAgent({
|
const adapter = createCursorAgent({
|
||||||
command: optionalEnv("WORKFLOW_CURSOR_COMMAND") ?? "cursor-agent",
|
command: env("WORKFLOW_CURSOR_COMMAND", "/home/you/.local/bin/cursor-agent"),
|
||||||
...
|
model: env("WORKFLOW_CURSOR_MODEL", "auto"),
|
||||||
});
|
timeout: Number(env("WORKFLOW_CURSOR_TIMEOUT", "300000")),
|
||||||
|
|
||||||
// ✅ CORRECT — use \`which cursor-agent\` to find the path, then write it in
|
|
||||||
const adapter = createCursorAgent({
|
|
||||||
command: optionalEnv("WORKFLOW_CURSOR_COMMAND", "/home/you/.local/bin/cursor-agent"),
|
|
||||||
...
|
...
|
||||||
});
|
});
|
||||||
\`\`\`
|
\`\`\`
|
||||||
|
|||||||
@@ -7,24 +7,20 @@
|
|||||||
import { createCursorAgent } from "@uncaged/workflow-agent-cursor";
|
import { createCursorAgent } from "@uncaged/workflow-agent-cursor";
|
||||||
import { createHermesAgent } from "@uncaged/workflow-agent-hermes";
|
import { createHermesAgent } from "@uncaged/workflow-agent-hermes";
|
||||||
import { createWorkflow } from "@uncaged/workflow-runtime";
|
import { createWorkflow } from "@uncaged/workflow-runtime";
|
||||||
import { optionalEnv } from "@uncaged/workflow-util";
|
import { env } from "@uncaged/workflow-util";
|
||||||
import { buildDevelopDescriptor, developWorkflowDefinition } from "./src/index.js";
|
import { buildDevelopDescriptor, developWorkflowDefinition } from "./src/index.js";
|
||||||
|
|
||||||
const cursorAdapter = createCursorAgent({
|
const cursorAdapter = createCursorAgent({
|
||||||
command: optionalEnv("WORKFLOW_CURSOR_COMMAND", "/home/azureuser/.local/bin/cursor-agent"),
|
command: env("WORKFLOW_CURSOR_COMMAND", "/home/azureuser/.local/bin/cursor-agent"),
|
||||||
model: optionalEnv("WORKFLOW_CURSOR_MODEL"),
|
model: env("WORKFLOW_CURSOR_MODEL", "auto"),
|
||||||
timeout: optionalEnv("WORKFLOW_CURSOR_TIMEOUT")
|
timeout: Number(env("WORKFLOW_CURSOR_TIMEOUT", "0")),
|
||||||
? Number(optionalEnv("WORKFLOW_CURSOR_TIMEOUT"))
|
|
||||||
: 0,
|
|
||||||
workspace: null,
|
workspace: null,
|
||||||
});
|
});
|
||||||
|
|
||||||
const hermesAdapter = createHermesAgent({
|
const hermesAdapter = createHermesAgent({
|
||||||
command: optionalEnv("WORKFLOW_HERMES_COMMAND", "/home/azureuser/.local/bin/hermes"),
|
command: env("WORKFLOW_HERMES_COMMAND", "/home/azureuser/.local/bin/hermes"),
|
||||||
model: optionalEnv("WORKFLOW_HERMES_MODEL"),
|
model: env("WORKFLOW_HERMES_MODEL", "") || null,
|
||||||
timeout: optionalEnv("WORKFLOW_HERMES_TIMEOUT")
|
timeout: Number(env("WORKFLOW_HERMES_TIMEOUT", "0")) || null,
|
||||||
? Number(optionalEnv("WORKFLOW_HERMES_TIMEOUT"))
|
|
||||||
: null,
|
|
||||||
});
|
});
|
||||||
|
|
||||||
const wf = createWorkflow(developWorkflowDefinition, {
|
const wf = createWorkflow(developWorkflowDefinition, {
|
||||||
|
|||||||
@@ -7,14 +7,13 @@
|
|||||||
import { createHermesAgent } from "@uncaged/workflow-agent-hermes";
|
import { createHermesAgent } from "@uncaged/workflow-agent-hermes";
|
||||||
import { workflowAdapter } from "@uncaged/workflow-execute";
|
import { workflowAdapter } from "@uncaged/workflow-execute";
|
||||||
import { createWorkflow } from "@uncaged/workflow-runtime";
|
import { createWorkflow } from "@uncaged/workflow-runtime";
|
||||||
import { optionalEnv } from "@uncaged/workflow-util";
|
import { env } from "@uncaged/workflow-util";
|
||||||
import { buildSolveIssueDescriptor, solveIssueWorkflowDefinition } from "./src/index.js";
|
import { buildSolveIssueDescriptor, solveIssueWorkflowDefinition } from "./src/index.js";
|
||||||
|
|
||||||
const adapter = createHermesAgent({
|
const adapter = createHermesAgent({
|
||||||
model: optionalEnv("WORKFLOW_HERMES_MODEL"),
|
command: env("WORKFLOW_HERMES_COMMAND", "/home/azureuser/.local/bin/hermes"),
|
||||||
timeout: optionalEnv("WORKFLOW_HERMES_TIMEOUT")
|
model: env("WORKFLOW_HERMES_MODEL", "") || null,
|
||||||
? Number(optionalEnv("WORKFLOW_HERMES_TIMEOUT"))
|
timeout: Number(env("WORKFLOW_HERMES_TIMEOUT", "0")) || null,
|
||||||
: null,
|
|
||||||
});
|
});
|
||||||
|
|
||||||
const wf = createWorkflow(solveIssueWorkflowDefinition, {
|
const wf = createWorkflow(solveIssueWorkflowDefinition, {
|
||||||
|
|||||||
@@ -1,44 +1,20 @@
|
|||||||
import { describe, expect, test } from "bun:test";
|
import { describe, expect, it } from "bun:test";
|
||||||
import { optionalEnv, requireEnv } from "../src/env.js";
|
import { env } from "../src/env.js";
|
||||||
|
|
||||||
describe("requireEnv", () => {
|
describe("env", () => {
|
||||||
test("returns value when set", () => {
|
it("returns env value when set", () => {
|
||||||
process.env.TEST_REQ = "hello";
|
process.env.TEST_ENV_SET = "hello";
|
||||||
expect(requireEnv("TEST_REQ", "missing")).toBe("hello");
|
expect(env("TEST_ENV_SET", "default")).toBe("hello");
|
||||||
delete process.env.TEST_REQ;
|
delete process.env.TEST_ENV_SET;
|
||||||
});
|
});
|
||||||
|
|
||||||
test("throws with message when missing", () => {
|
it("returns fallback when missing", () => {
|
||||||
expect(() => requireEnv("TEST_MISSING_XYZ", "need this")).toThrow("need this");
|
expect(env("TEST_ENV_MISSING_XYZ", "fallback")).toBe("fallback");
|
||||||
});
|
});
|
||||||
|
|
||||||
test("throws when empty string", () => {
|
it("returns fallback when empty", () => {
|
||||||
process.env.TEST_EMPTY = "";
|
process.env.TEST_ENV_EMPTY = "";
|
||||||
expect(() => requireEnv("TEST_EMPTY", "cannot be empty")).toThrow("cannot be empty");
|
expect(env("TEST_ENV_EMPTY", "fb")).toBe("fb");
|
||||||
delete process.env.TEST_EMPTY;
|
delete process.env.TEST_ENV_EMPTY;
|
||||||
});
|
|
||||||
});
|
|
||||||
|
|
||||||
describe("optionalEnv", () => {
|
|
||||||
test("returns value when set", () => {
|
|
||||||
process.env.TEST_OPT = "world";
|
|
||||||
expect(optionalEnv("TEST_OPT")).toBe("world");
|
|
||||||
expect(optionalEnv("TEST_OPT", "default")).toBe("world");
|
|
||||||
delete process.env.TEST_OPT;
|
|
||||||
});
|
|
||||||
|
|
||||||
test("returns null when missing and no fallback", () => {
|
|
||||||
expect(optionalEnv("TEST_MISSING_ABC")).toBeNull();
|
|
||||||
});
|
|
||||||
|
|
||||||
test("returns fallback when missing", () => {
|
|
||||||
expect(optionalEnv("TEST_MISSING_ABC", "fallback")).toBe("fallback");
|
|
||||||
});
|
|
||||||
|
|
||||||
test("returns fallback when empty string", () => {
|
|
||||||
process.env.TEST_EMPTY2 = "";
|
|
||||||
expect(optionalEnv("TEST_EMPTY2", "fb")).toBe("fb");
|
|
||||||
expect(optionalEnv("TEST_EMPTY2")).toBeNull();
|
|
||||||
delete process.env.TEST_EMPTY2;
|
|
||||||
});
|
});
|
||||||
});
|
});
|
||||||
|
|||||||
@@ -1,23 +1,14 @@
|
|||||||
/**
|
/**
|
||||||
* Read a required environment variable. Throws with `message` if missing or empty.
|
* Read an environment variable with a required fallback default.
|
||||||
|
* Returns the env value if set and non-empty, otherwise returns `fallback`.
|
||||||
|
*
|
||||||
|
* Every env var in a bundle must have a sensible default — bundles must run
|
||||||
|
* without any env vars set. Env vars are overrides, not requirements.
|
||||||
*/
|
*/
|
||||||
export function requireEnv(name: string, message: string): string {
|
export function env(name: string, fallback: string): string {
|
||||||
const value = process.env[name];
|
const value = process.env[name];
|
||||||
if (value === undefined || value === "") {
|
if (value === undefined || value === "") {
|
||||||
throw new Error(message);
|
return fallback;
|
||||||
}
|
|
||||||
return value;
|
|
||||||
}
|
|
||||||
|
|
||||||
/**
|
|
||||||
* Read an optional environment variable. Returns `fallback` if missing or empty.
|
|
||||||
*/
|
|
||||||
export function optionalEnv(name: string, fallback: string): string;
|
|
||||||
export function optionalEnv(name: string): string | null;
|
|
||||||
export function optionalEnv(name: string, fallback?: string): string | null {
|
|
||||||
const value = process.env[name];
|
|
||||||
if (value === undefined || value === "") {
|
|
||||||
return fallback ?? null;
|
|
||||||
}
|
}
|
||||||
return value;
|
return value;
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -6,7 +6,7 @@ export {
|
|||||||
encodeCrockfordBase32Bits,
|
encodeCrockfordBase32Bits,
|
||||||
encodeUint64AsCrockford,
|
encodeUint64AsCrockford,
|
||||||
} from "./base32.js";
|
} from "./base32.js";
|
||||||
export { optionalEnv, requireEnv } from "./env.js";
|
export { env } from "./env.js";
|
||||||
export { createLogger } from "./logger.js";
|
export { createLogger } from "./logger.js";
|
||||||
export { mergeRefsWithContentHash, normalizeRefsField } from "./refs-field.js";
|
export { mergeRefsWithContentHash, normalizeRefsField } from "./refs-field.js";
|
||||||
export { getDefaultWorkflowStorageRoot, getGlobalCasDir } from "./storage-root.js";
|
export { getDefaultWorkflowStorageRoot, getGlobalCasDir } from "./storage-root.js";
|
||||||
|
|||||||
Reference in New Issue
Block a user