mvschwarz/openrig
1
0 Fork 0
mirror of https://github.com/mvschwarz/openrig.git synced 2026-11-09 17:32:41 -10:00
Code Issues Packages Projects Releases 1 Wiki Activity

fix(slice-22): drop --openrig-home flag; use process-env pattern (FF1)

Browse Source 
Guard re-review on 5a14029 caught two real architectural findings:

FINDING #1: --openrig-home did NOT deliver the claimed fully-isolated
two-daemon lifecycle. Module-level constants OPENRIG_DIR / STATE_FILE /
LOG_FILE in packages/cli/src/daemon-lifecycle.ts:70-73 resolve from the
parent CLI's OPENRIG_HOME at IMPORT time. startDaemon reads state,
creates the directory, opens logs, and writes daemon.json through
those module-level constants — but the flag was only passed into
buildDaemonEnv for the SPAWNED child. Parent CLI process always
read/wrote default ~/.openrig state regardless of the flag. Two
sequential invocations would collide on ~/.openrig/daemon.json. Core
slice 22 "fully-isolated state" claim was broken.

FINDING #2: Bootstrap script + convention doc referenced
`rig daemon stop --openrig-home` for reset, but the `stop` subcommand
never had that flag. Doc path was non-executable.

FIX: per guard's v0-simplest recommendation (option (b) in dispatch):
drop the --openrig-home flag entirely and use the process-env pattern.
Each `rig` invocation has its own OPENRIG_HOME at process start →
module-level constants resolve to per-invocation paths "for free."
No flag-threading through start/status/stop/logs/preflight needed
(which would multiply the surface to 5+ files; deferred until a real
use case can't be served by env).

CHANGES:
- packages/cli/src/commands/daemon.ts: remove --openrig-home flag +
  opts.openrigHome wiring
- packages/cli/src/commands/up.ts: remove --openrig-home flag +
  opts.openrigHome wiring
- packages/cli/src/daemon-lifecycle.ts: remove openrigHome from
  StartOptions interface and buildDaemonEnv opt + the conditional
  OPENRIG_HOME export
- packages/cli/test/daemon-lifecycle.test.ts: replace the 4 Phase-B
  flag tests with 3 NEW env-contract regression tests
  proving:
    1. getOpenRigHome respects process.env.OPENRIG_HOME (distinct
       values → distinct paths)
    2. getDefaultOpenRigPath threads env-resolved home into derived
       paths (daemon.json + log)
    3. empty OPENRIG_HOME falls back to homedir default (not the
       literal empty string)
  These are the actual function contract the slice's isolation claim
  rests on. Distinct-value discriminators per banked
  feedback_poc_regression_must_discriminate.
- scripts/vm-bootstrap/two-daemon-start.sh: rewrite to use
  `OPENRIG_HOME=$X rig daemon start ...` pattern for both daemons +
  matching `OPENRIG_HOME=$X rig daemon stop` for reset. Adds an
  architecture-note section in the script header explaining the
  module-level-constants gotcha that ruled out the flag approach.
- packages/daemon/assets/vm-preview-fixtures/README.md: update "v0
  scope" section to describe the process-env pattern.

Substrate convention doc updated separately
(<work-tree>/conventions/vm-preview/README.md):
new "How isolation works (process-env pattern)" section + "Architecture
note: module-level constants vs function-level usage" section explaining
the lesson. IMPL-PRD §14 captures the forward-fix narrative.

Tests green:
- daemon-lifecycle.test.ts: 55/55 (was 56; +3 new env-contract /
  -4 flag tests = net 55)
- Full CLI+daemon sweep (env-vars cleaned): 4269/4271 (one
  environmental pre-existing fail unchanged)
- CLI tsc clean
- daemon untouched in this commit

No SC-29 EXCEPTIONS: pure additive/subtractive on existing
OPENRIG_HOME env contract.

Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
This commit is contained in:
mvschwarzandClaude Opus 4.7 committed 2026-05-11 04:54:53 -07:00
1 parent 5a14029739
commit acc6afc358
6 files changed
+97 -109
Download Patch File Download Diff File Expand all files Collapse all files

No files matched your search

packages/cli/src/commands/daemon.ts
+1 -10
View File
@@ -77,11 +77,6 @@ export function daemonCommand(depsOverride?: LifecycleDeps): Command {
.option("--port <port>", "Port to listen on")
.option("--host <host>", "Host to bind on")
.option("--db <path>", "Database path")
// Slice 22 founder-walk-vm-populated-env — operator-explicit
// OPENRIG_HOME directory. Enables running two daemons in the same
// VM with fully isolated state (e.g., blank-slate vs populated
// preview environments).
.option("--openrig-home <path>", "OPENRIG_HOME directory for daemon state (transcripts, plugins, specs, etc.)")
// V0.3.1 slice 05 kernel-rig-as-default — skip the kernel auto-boot
// path. Used by test fixtures, headless CI, and operators who want
// a no-kernel daemon for ad-hoc topology work. The daemon proceeds
@@ -96,7 +91,7 @@ export function daemonCommand(depsOverride?: LifecycleDeps): Command {
// weaker "daemon-ready". Default 60s; override with --wait-for-kernel-ms.
.option("--wait-for-kernel", "After daemon binds, also wait for kernel-agent readiness (default timeout 60s)")
.option("--wait-for-kernel-ms <ms>", "Override --wait-for-kernel timeout in milliseconds")
.action(async (opts: { port?: string; host?: string; db?: string; openrigHome?: string; kernel?: boolean; waitForKernel?: boolean; waitForKernelMs?: string }) => {
.action(async (opts: { port?: string; host?: string; db?: string; kernel?: boolean; waitForKernel?: boolean; waitForKernelMs?: string }) => {
try {
const { ConfigStore } = await import("../config-store.js");
const { SystemPreflight } = await import("../system-preflight.js");
@@ -152,10 +147,6 @@ export function daemonCommand(depsOverride?: LifecycleDeps): Command {
// OPENRIG_NO_KERNEL env var so the daemon's kernel-boot
// check in startup.ts honors the flag.
skipKernelBoot: skipKernel,
// Slice 22 founder-walk-vm-populated-env — operator-explicit
// OPENRIG_HOME directory (omit when undefined so the daemon
// falls through to its built-in default ~/.openrig).
openrigHome: opts.openrigHome,
},
getDeps(),
);
packages/cli/src/commands/up.ts
+1 -8
View File
@@ -31,10 +31,7 @@ Examples:
.option("--target <root>", "Target root directory for package installation (.rigbundle only; does not change agent cwd)")
.option("--existing", "Treat <source> as an existing rig name; bypass library-spec name resolution")
.option("--json", "JSON output for agents")
// Slice 22 founder-walk-vm-populated-env — operator-explicit
// OPENRIG_HOME for the auto-started daemon's state directory.
.option("--openrig-home <path>", "OPENRIG_HOME directory for the auto-started daemon (state, transcripts, plugins, specs)")
.action(async (source: string, opts: { plan?: boolean; yes?: boolean; cwd?: string; target?: string; existing?: boolean; json?: boolean; openrigHome?: string }) => {
.action(async (source: string, opts: { plan?: boolean; yes?: boolean; cwd?: string; target?: string; existing?: boolean; json?: boolean }) => {
const deps = getDepsF();
// Run preflight before auto-start
@@ -99,10 +96,6 @@ Examples:
transcriptsLines: resolvedConfig?.transcripts.lines,
transcriptsPollIntervalSeconds: resolvedConfig?.transcripts.pollIntervalSeconds,
workspaceRoot: resolvedConfig?.workspace.root,
// Slice 22 founder-walk-vm-populated-env — operator-explicit
// OPENRIG_HOME (omit when undefined so the daemon's default
// homedir-relative ~/.openrig still applies).
openrigHome: opts.openrigHome,
}, deps.lifecycleDeps);
status = await getDaemonStatus(deps.lifecycleDeps);
} catch (err) {
packages/cli/src/daemon-lifecycle.ts
-19
View File
@@ -44,10 +44,6 @@ export interface StartOptions {
// Exposed at the CLI as `rig daemon start --no-kernel`; projected
// into the daemon process env as OPENRIG_NO_KERNEL.
skipKernelBoot?: boolean;
// Slice 22 founder-walk-vm-populated-env — operator-explicit
// OPENRIG_HOME directory for the daemon child process. Enables two
// daemons in one VM to have fully isolated state (OQ-4 (a)).
openrigHome?: string;
}
export interface LifecycleDeps {
@@ -273,17 +269,6 @@ export function buildDaemonEnv(
// V0.3.1 slice 05 — projected via OPENRIG_NO_KERNEL env var so
// the daemon's startup.ts kernel-boot path honors the flag.
skipKernelBoot?: boolean;
/**
* Slice 22 founder-walk-vm-populated-env: operator-explicit
* OPENRIG_HOME directory. When defined, sets OPENRIG_HOME in the
* child process so the daemon writes its state (transcripts,
* plugins, context-packs, agent-images, specs) under the chosen
* directory. When undefined, the env-passthrough loop preserves
* whatever OPENRIG_HOME the operator's shell already had (their
* shell-level opt-in still wins). Enables two daemons in the same
* VM to have fully isolated state per OQ-4 (a).
*/
openrigHome?: string;
},
): Record<string, string> {
const env: Record<string, string> = {};
@@ -312,9 +297,6 @@ export function buildDaemonEnv(
if (opts.transcriptsPollIntervalSeconds !== undefined) {
env["OPENRIG_TRANSCRIPTS_POLL_INTERVAL_SECONDS"] = String(opts.transcriptsPollIntervalSeconds);
}
if (opts.openrigHome !== undefined) {
env["OPENRIG_HOME"] = opts.openrigHome;
}
if (opts.skipKernelBoot) {
env["OPENRIG_NO_KERNEL"] = "1";
}
@@ -379,7 +361,6 @@ export async function startDaemon(opts: StartOptions, deps: LifecycleDeps): Prom
transcriptsLines: opts.transcriptsLines,
transcriptsPollIntervalSeconds: opts.transcriptsPollIntervalSeconds,
skipKernelBoot: opts.skipKernelBoot,
openrigHome: opts.openrigHome,
}),
stdio: ["ignore", logFd, logFd],
detached: true,
packages/cli/test/daemon-lifecycle.test.ts
+61 -47
View File
@@ -981,57 +981,71 @@ describe("buildDaemonEnv", () => {
});
});
// Slice 22 founder-walk-vm-populated-env: --openrig-home / OPENRIG_HOME
// flag flows through buildDaemonEnv so two daemons in one VM can have
// fully isolated state directories (OQ-4 resolved to (a) fully isolated).
describe("slice 22: openrigHome (OPENRIG_HOME) env projection", () => {
it("omits OPENRIG_HOME when openrigHome opt is undefined (shell-env passthrough wins)", () => {
const baseEnv: Record<string, string> = { HOME: "/Users/tester", PATH: "/usr/bin" };
const result = buildDaemonEnv(baseEnv, {
port: 7433,
db: "/tmp/test.db",
});
expect(result["OPENRIG_HOME"]).toBeUndefined();
// Slice 22 founder-walk-vm-populated-env forward-fix #1: the slice
// intentionally drops a `--openrig-home` CLI flag in favor of using
// the process-env-pattern (each `rig` invocation gets its own
// OPENRIG_HOME via the shell). The slice's "fully isolated state"
// claim depends on the env contract being honored at module load
// time — specifically that getOpenRigHome() returns the current
// env value (not a hardcoded default) and that getDefaultOpenRigPath
// threads it into derived paths. The module-level constants
// (OPENRIG_DIR / STATE_FILE / LOG_FILE) are a single snapshot of
// these functions at load.
//
// These regression tests prove the function contract with distinct-
// value discriminators per banked feedback_poc_regression_must_discriminate.
describe("slice 22: OPENRIG_HOME env contract for per-process state isolation", () => {
it("getOpenRigHome respects process.env.OPENRIG_HOME (distinct values yield distinct paths)", async () => {
const { getOpenRigHome } = await import("../src/openrig-compat.js");
const saved = process.env["OPENRIG_HOME"];
try {
process.env["OPENRIG_HOME"] = "/Users/example/.openrig-blank";
const blank = getOpenRigHome();
process.env["OPENRIG_HOME"] = "/Users/example/.openrig-populated";
const populated = getOpenRigHome();
expect(blank).toBe("/Users/example/.openrig-blank");
expect(populated).toBe("/Users/example/.openrig-populated");
expect(blank).not.toBe(populated);
} finally {
if (saved === undefined) delete process.env["OPENRIG_HOME"];
else process.env["OPENRIG_HOME"] = saved;
}
});
it("sets OPENRIG_HOME when openrigHome opt is defined (operator opt-in)", () => {
const baseEnv: Record<string, string> = { HOME: "/Users/tester", PATH: "/usr/bin" };
const result = buildDaemonEnv(baseEnv, {
port: 7433,
db: "/tmp/test.db",
openrigHome: "/Users/example/.openrig-blank",
});
expect(result["OPENRIG_HOME"]).toBe("/Users/example/.openrig-blank");
it("getDefaultOpenRigPath threads env-resolved OPENRIG_HOME into derived paths (daemon.json + log)", async () => {
const { getDefaultOpenRigPath } = await import("../src/openrig-compat.js");
const saved = process.env["OPENRIG_HOME"];
try {
process.env["OPENRIG_HOME"] = "/Users/example/.openrig-blank";
const blankState = getDefaultOpenRigPath("daemon.json");
const blankLog = getDefaultOpenRigPath("daemon.log");
process.env["OPENRIG_HOME"] = "/Users/example/.openrig-populated";
const populatedState = getDefaultOpenRigPath("daemon.json");
const populatedLog = getDefaultOpenRigPath("daemon.log");
expect(blankState).toBe("/Users/example/.openrig-blank/daemon.json");
expect(populatedState).toBe("/Users/example/.openrig-populated/daemon.json");
expect(blankLog).toBe("/Users/example/.openrig-blank/daemon.log");
expect(populatedLog).toBe("/Users/example/.openrig-populated/daemon.log");
expect(blankState).not.toBe(populatedState);
expect(blankLog).not.toBe(populatedLog);
} finally {
if (saved === undefined) delete process.env["OPENRIG_HOME"];
else process.env["OPENRIG_HOME"] = saved;
}
});
it("preserves shell-set OPENRIG_HOME through baseEnv when opt is undefined", () => {
const baseEnv: Record<string, string> = {
HOME: "/Users/tester",
PATH: "/usr/bin",
OPENRIG_HOME: "/Users/example/.openrig-shell",
};
const result = buildDaemonEnv(baseEnv, {
port: 7433,
db: "/tmp/test.db",
});
expect(result["OPENRIG_HOME"]).toBe("/Users/example/.openrig-shell");
});
it("opt-set OPENRIG_HOME overrides shell-set baseEnv value (distinct-value discriminator)", () => {
// Per banked feedback_poc_regression_must_discriminate — distinct
// values so we can prove the override branch fired and didn't pass
// vacuously through baseEnv-passthrough.
const baseEnv: Record<string, string> = {
HOME: "/Users/tester",
PATH: "/usr/bin",
OPENRIG_HOME: "/Users/example/.openrig-shell-blank",
};
const result = buildDaemonEnv(baseEnv, {
port: 7433,
db: "/tmp/test.db",
openrigHome: "/Users/example/.openrig-opt-populated",
});
expect(result["OPENRIG_HOME"]).toBe("/Users/example/.openrig-opt-populated");
it("empty OPENRIG_HOME falls back to homedir default (not the literal empty string)", async () => {
const { getOpenRigHome } = await import("../src/openrig-compat.js");
const { homedir } = await import("node:os");
const saved = process.env["OPENRIG_HOME"];
try {
process.env["OPENRIG_HOME"] = "";
const result = getOpenRigHome();
expect(result).toBe(`${homedir()}/.openrig`);
} finally {
if (saved === undefined) delete process.env["OPENRIG_HOME"];
else process.env["OPENRIG_HOME"] = saved;
}
});
});
packages/daemon/assets/vm-preview-fixtures/README.md
+5 -1
View File
@@ -33,7 +33,11 @@ Fixture YAML files use the canonical placeholder paths per banked
Slice 22 (release-0.3.1) ships the INFRASTRUCTURE for the two-daemon model:
- `--openrig-home` CLI flag on `rig daemon start` and `rig up`
- **Process-env pattern**: `OPENRIG_HOME=<dir> rig daemon start ...` — each
`rig` invocation gets its own state directory at process import time.
No CLI flag (an earlier attempt at `--openrig-home` was dropped because
it only threaded into the spawned child, not the parent-side lifecycle
bookkeeping; see substrate convention doc for the architecture note).
- Bootstrap script (`scripts/vm-bootstrap/two-daemon-start.sh`)
- This fixtures directory + a sample workflow YAML
- Convention doc in substrate
scripts/vm-bootstrap/two-daemon-start.sh
+29 -24
View File
@@ -12,15 +12,27 @@
# 3. Open two browser tabs:
# http://<vm-ip>:7433 → blank-slate (default, fresh install UX)
# http://<vm-ip>:7434 → populated (sample rig + qitems for "lived-in" UX)
# 4. To reset: kill both daemons + rm both OPENRIG_HOME dirs + re-run.
# 4. To reset, see "RESET" section at the bottom of this file.
#
# Pre-conditions (operator install before running this):
# - `rig` CLI on PATH
# - Tailscale daemon active OR explicit LAN bind via OPENRIG_HOST (see
# auth-bearer-tailscale-trust slice for the bind rules)
#
# Idempotency: the script refuses to clobber existing daemons; operator
# must clean state dirs to fully reset.
# Architecture note (forward-fix #1): the CLI's parent-process module-
# level constants (OPENRIG_DIR / STATE_FILE / LOG_FILE in
# packages/cli/src/daemon-lifecycle.ts) resolve from OPENRIG_HOME at
# import time. So per-invocation `OPENRIG_HOME=<dir> rig daemon start`
# isolates BOTH the spawned daemon's state AND the CLI's lifecycle
# bookkeeping (daemon.json, daemon.log) in the same dir. The earlier
# attempt to add a `--openrig-home` flag was dropped because it only
# threaded into the spawned child, leaving the parent CLI's state
# writes on the default ~/.openrig path.
#
# Idempotency: each daemon's $OPENRIG_HOME has its own daemon.json, so
# re-running the script after a clean shutdown re-creates state. If a
# daemon is still running, `rig daemon start` (under the matching
# OPENRIG_HOME) refuses to launch a duplicate.
set -euo pipefail
@@ -35,33 +47,20 @@ echo " BLANK_HOME=$BLANK_HOME (port $BLANK_PORT)"
echo " POPULATED_HOME=$POPULATED_HOME (port $POPULATED_PORT)"
echo " FIXTURE_DIR=$FIXTURE_DIR"
# Refuse to overwrite existing daemons — operator action required to reset.
if [ -f "$BLANK_HOME/daemon.json" ]; then
echo "ERROR: blank daemon already running per $BLANK_HOME/daemon.json"
echo " Run: rig daemon stop --openrig-home $BLANK_HOME"
echo " Then: rm -rf $BLANK_HOME && rerun this script."
exit 1
fi
if [ -f "$POPULATED_HOME/daemon.json" ]; then
echo "ERROR: populated daemon already running per $POPULATED_HOME/daemon.json"
echo " Run: rig daemon stop --openrig-home $POPULATED_HOME"
echo " Then: rm -rf $POPULATED_HOME && rerun this script."
exit 1
fi
mkdir -p "$BLANK_HOME" "$POPULATED_HOME"
# Start the blank-slate daemon — pristine first-launch UX.
# Start the blank-slate daemon — pristine first-launch UX. OPENRIG_HOME
# env (NOT a CLI flag) ensures both the spawned daemon AND the CLI's
# own state writes (daemon.json, daemon.log) land under $BLANK_HOME.
echo "==> Starting blank-slate daemon (port $BLANK_PORT)"
rig daemon start \
--openrig-home "$BLANK_HOME" \
OPENRIG_HOME="$BLANK_HOME" rig daemon start \
--port "$BLANK_PORT" \
--db "$BLANK_HOME/openrig.sqlite"
# Start the populated daemon — runs alongside the blank one with its own state.
# Start the populated daemon — runs alongside the blank one with its own
# OPENRIG_HOME so neither sees the other's state.
echo "==> Starting populated daemon (port $POPULATED_PORT)"
rig daemon start \
--openrig-home "$POPULATED_HOME" \
OPENRIG_HOME="$POPULATED_HOME" rig daemon start \
--port "$POPULATED_PORT" \
--db "$POPULATED_HOME/openrig.sqlite"
@@ -71,11 +70,17 @@ echo " OPENRIG_HOME=$POPULATED_HOME OPENRIG_PORT=$POPULATED_PORT \\"
echo " rig up product-team # instantiate a sample rig"
echo
echo " Copy workflow fixtures to operator workspace specs dir:"
echo " cp $FIXTURE_DIR/workflows/*.yaml \$OPENRIG_WORKSPACE_SPECS_ROOT/workflows/"
echo " cp $FIXTURE_DIR/workflows/*.yaml \\"
echo " \$OPENRIG_WORKSPACE_SPECS_ROOT/workflows/"
echo " (slice 11 auto-discovers them on GET /api/specs/library)"
echo
echo "==> Operator UI access:"
echo " Blank-slate: http://127.0.0.1:$BLANK_PORT"
echo " Populated: http://127.0.0.1:$POPULATED_PORT"
echo
echo "==> RESET (when you want a clean start):"
echo " OPENRIG_HOME=$BLANK_HOME rig daemon stop"
echo " OPENRIG_HOME=$POPULATED_HOME rig daemon stop"
echo " rm -rf $BLANK_HOME $POPULATED_HOME"
echo
echo "==> See conventions/vm-preview/README.md for the full workflow + how to extend."