mirror of
https://github.com/mvschwarz/openrig.git
synced 2026-11-08 17:31:56 -10:00
fix(slice-22): path-form rig up <install-internal-spec> defaults cwd to caller dir (OPR.0.3.2.22 Bug 3)
Closes the path-form gap behind the openrig.dev hero-flow paper-cut.
Builtin starter specs declare member-level cwd: "." which resolves
relative to the spec's directory — inside the daemon install root for
shipped builtins. Without --cwd this trips
getOpenRigInstallCwdError at preflight. Bare-name form was already
rescued in packages/cli/src/commands/up.ts:152-154 (entry.sourceType
=== "builtin"). Path-form `rig up <install-internal-spec>` was the
remaining gap that operators copy-pasting from openrig.dev hero
copy-blocks would hit.
The fix has two parts:
(1) New daemon-info surface (packages/daemon/src/routes/info.ts +
server.ts mount at /api/info): returns the daemon's installRoot
computed by cwd-resolution.getOpenRigInstallRoot. Minimal: no
other fields. /api/info has no auth gate so the CLI auto-start
path can consume it before bearer-token plumbing settles in.
(2) CLI consumer (packages/cli/src/commands/up.ts):
- Local isPathInsideRoot helper mirrors the daemon's
cwd-resolution.isPathInsideRoot (kept local — tactical fix
avoids a cross-package import for a 2-line helper).
- Before the /api/up POST, if (!opts.cwd && !isRigName &&
!isRigBundle && defaultLibraryCwdOverride === undefined),
call GET /api/info with a 2000ms timeout (a healthy daemon
answers in <100ms; the timeout protects against stuck-daemon
multi-second stalls on every rig up). If installRoot is
returned and the resolved sourceRef is inside it, set
defaultLibraryCwdOverride = process.cwd() and print the
one-line notice "Defaulting cwd to current directory
because the spec lives inside the OpenRig install."
- Silent fall-through on /api/info error or timeout: the
daemon's own preflight still surfaces
getOpenRigInstallCwdError; operator gets the same
pre-Bug-3 hint they would have gotten.
Structural redesign of how builtin starter specs declare
member.cwd is OUT OF SCOPE (deferred to 0.3.3 per slice-22 triage:
significant refactor across builtins + instantiator + tests, not
bounded).
Test additions:
- packages/daemon/test/info-routes.test.ts (new file): GET
/api/info returns the expected installRoot string shape.
- packages/cli/test/up.test.ts: two new tests pinning the contract:
(a) path-form spec INSIDE install root defaults cwdOverride to
caller cwd + prints notice; (b) path-form spec OUTSIDE install
root leaves cwdOverride undefined.
Test scaffold alignment (packages/cli/test/up.test.ts):
- Two existing tests with custom request handlers ("up --yes sends
autoApprove=true" + "resolves relative path to absolute in POST
body") use path-form sourceRefs (/tmp/rig.yaml + relative/spec.yaml)
and therefore now trigger the /api/info call. Each gets a 404
fallback in its custom handler so the CLI falls through cleanly
(those tests do not exercise the install-internal path-form gate).
Without this alignment, the requests would hang and the test
pollution would cascade to subsequent tests' default handlers.
Verification:
- Full daemon test/ suite: 272 files (271 prior + 1 new
info-routes.test.ts), 3992 PASS + 1 skipped, ZERO failures.
- Full CLI test/ suite: 70 files, 1104 PASS, 3 pre-existing
baseline failures unchanged (DaemonClient --version + 2 preflight
tests; verified by stash-and-rerun against ad6a522b — same 3
fail). Bug 3 changes did not introduce any new CLI failures.
- daemon tsc + cli tsc EXIT=0.
- Commit body verification grep clean per banked discipline (zero
matches against the forbidden-attribution pattern).
- git diff --cached --stat: 5 files (+201 / -0).
Branch: slice-22-rig-up-paper-cut-fix-round off LOCAL main 0c9e63f1.
Chain: 50488427 (Bug 1 BONUS) → a29c7883 (Bug 2 first) → ad6a522b
(Bug 2 teardown follow-up) → this commit. LOCAL only, NO push.
This commit is contained in:
1 parent
ad6a522b69
commit
64f8aa8bc6
5 files changed
+201
No files matched your search
@@ -7,6 +7,14 @@ import type { StatusDeps } from "./status.js";
|
||||
|
||||
const LONG_RUNNING_UP_TIMEOUT_MS = 120_000;
|
||||
|
||||
// OPR.0.3.2.22 Bug 3 helper — mirrors cwd-resolution.isPathInsideRoot in
|
||||
// the daemon so the CLI can decide whether a path-form sourceRef lives
|
||||
// inside the daemon install root without importing the daemon package.
|
||||
function isPathInsideRoot(candidate: string, root: string): boolean {
|
||||
const relative = nodePath.relative(nodePath.resolve(root), nodePath.resolve(candidate));
|
||||
return relative === "" || (!relative.startsWith("..") && !nodePath.isAbsolute(relative));
|
||||
}
|
||||
|
||||
export function upCommand(
|
||||
depsOverride?: StatusDeps & {
|
||||
lifecycleDeps?: LifecycleDeps;
|
||||
@@ -189,6 +197,46 @@ Examples:
|
||||
const isRigBundle = !isRigName && /\.rigbundle$/i.test(sourceRef);
|
||||
const targetRoot = opts.target ?? (isRigBundle ? process.cwd() : undefined);
|
||||
|
||||
// OPR.0.3.2.22 Bug 3 — extend the bare `rig up <builtin>` default-cwd
|
||||
// treatment to path-form. Builtin starter specs declare member-level
|
||||
// cwd: "." which resolves to the spec directory (inside the daemon's
|
||||
// install root). Without --cwd that trips getOpenRigInstallCwdError
|
||||
// at preflight. Bare-name form is already rescued at the
|
||||
// resolveLibrarySpec branch above (entry.sourceType === "builtin");
|
||||
// path-form `rig up <install-internal-spec>` is the remaining gap.
|
||||
//
|
||||
// Detection: source is path-form (not isRigName, not isRigBundle),
|
||||
// no --cwd was given, no defaultLibraryCwdOverride was set by the
|
||||
// bare-name branch above, AND the resolved path lives inside the
|
||||
// daemon's install root (fetched via /api/info). When all hold,
|
||||
// default cwdOverride to process.cwd() so the operator's project
|
||||
// dir is used as launch cwd, and print a one-line notice. If
|
||||
// /api/info is unavailable, fall through silently — the daemon
|
||||
// preflight will still surface the install-cwd error.
|
||||
//
|
||||
// Structural redesign of how builtin starter specs declare cwd is
|
||||
// out of scope (deferred to 0.3.3 per the slice triage).
|
||||
if (!opts.cwd && !isRigName && !isRigBundle && defaultLibraryCwdOverride === undefined) {
|
||||
try {
|
||||
// Short timeout: a healthy daemon answers /api/info in <100ms; if
|
||||
// it doesn't, fall through to the daemon's own preflight error
|
||||
// rather than adding a multi-second stall to every rig up.
|
||||
const infoRes = await client.get<{ installRoot?: string }>("/api/info", { timeoutMs: 2000 });
|
||||
const installRoot = infoRes.data?.installRoot;
|
||||
if (installRoot && isPathInsideRoot(sourceRef, installRoot)) {
|
||||
defaultLibraryCwdOverride = process.cwd();
|
||||
if (!opts.json) {
|
||||
console.log("Defaulting cwd to current directory because the spec lives inside the OpenRig install.");
|
||||
}
|
||||
}
|
||||
} catch {
|
||||
// /api/info unavailable — fall through. The daemon preflight
|
||||
// returns getOpenRigInstallCwdError if this is in fact the
|
||||
// install-internal case; the operator gets the same hint they
|
||||
// would have gotten pre-Bug-3.
|
||||
}
|
||||
}
|
||||
|
||||
const res = await client.post<Record<string, unknown>>("/api/up", {
|
||||
sourceRef,
|
||||
plan: opts.plan ?? false,
|
||||
|
||||
@@ -137,7 +137,13 @@ describe("Up CLI", () => {
|
||||
lastBody = JSON.parse(body);
|
||||
res.writeHead(201, { "Content-Type": "application/json" });
|
||||
res.end(JSON.stringify({ status: "completed", runId: "r", rigId: "g", stages: [], errors: [] }));
|
||||
return;
|
||||
}
|
||||
// OPR.0.3.2.22 Bug 3 — path-form sourceRef triggers a /api/info
|
||||
// call for install-root awareness; respond with a 404 so the CLI
|
||||
// falls through (this test does not exercise the install-internal
|
||||
// path-form gate).
|
||||
res.writeHead(404).end();
|
||||
});
|
||||
|
||||
await captureLogs(async () => {
|
||||
@@ -220,7 +226,10 @@ describe("Up CLI", () => {
|
||||
lastBody = JSON.parse(body);
|
||||
res.writeHead(201, { "Content-Type": "application/json" });
|
||||
res.end(JSON.stringify({ status: "completed", runId: "r", rigId: "g", stages: [], errors: [] }));
|
||||
return;
|
||||
}
|
||||
// OPR.0.3.2.22 Bug 3 /api/info fallback (404 → CLI falls through).
|
||||
res.writeHead(404).end();
|
||||
});
|
||||
|
||||
await captureLogs(async () => {
|
||||
@@ -364,6 +373,104 @@ describe("Up CLI", () => {
|
||||
for (const l of origListeners) server.on("request", l as (...args: unknown[]) => void);
|
||||
});
|
||||
|
||||
// OPR.0.3.2.22 Bug 3 — path-form `rig up <spec-inside-install>` without
|
||||
// --cwd should default cwdOverride to process.cwd() so the spec's
|
||||
// member-level cwd: "." does not resolve into the OpenRig install root
|
||||
// and trip getOpenRigInstallCwdError at preflight. The bare-name form
|
||||
// is already rescued at the resolveLibrarySpec branch (covered by the
|
||||
// earlier library-name test); this pins the path-form gap.
|
||||
it("up path-form spec inside install root defaults cwdOverride to caller cwd and prints notice", async () => {
|
||||
const fs = await import("node:fs");
|
||||
const path = await import("node:path");
|
||||
const os = await import("node:os");
|
||||
|
||||
const installRoot = fs.mkdtempSync(path.join(os.tmpdir(), "up-bug3-install-"));
|
||||
const specPath = path.join(installRoot, "rigs", "demo", "rig.yaml");
|
||||
fs.mkdirSync(path.dirname(specPath), { recursive: true });
|
||||
fs.writeFileSync(specPath, "version: '0.2'\nname: demo\npods: []\nedges: []\n");
|
||||
|
||||
let lastBody: Record<string, unknown> = {};
|
||||
let infoCalls = 0;
|
||||
const origListeners = server.listeners("request");
|
||||
server.removeAllListeners("request");
|
||||
server.on("request", async (req: http.IncomingMessage, res: http.ServerResponse) => {
|
||||
let body = "";
|
||||
for await (const chunk of req) body += chunk;
|
||||
if (req.url === "/api/info" && req.method === "GET") {
|
||||
infoCalls += 1;
|
||||
res.writeHead(200, { "Content-Type": "application/json" });
|
||||
res.end(JSON.stringify({ installRoot }));
|
||||
return;
|
||||
}
|
||||
if (req.url === "/api/up" && req.method === "POST") {
|
||||
lastBody = JSON.parse(body);
|
||||
res.writeHead(201, { "Content-Type": "application/json" });
|
||||
res.end(JSON.stringify({ status: "completed", runId: "r", rigId: "g", stages: [], errors: [] }));
|
||||
return;
|
||||
}
|
||||
res.writeHead(404, { "Content-Type": "application/json" });
|
||||
res.end(JSON.stringify({ error: "not found" }));
|
||||
});
|
||||
|
||||
try {
|
||||
const { logs } = await captureLogs(async () => {
|
||||
await makeCmd().parseAsync(["node", "rig", "up", specPath]);
|
||||
});
|
||||
expect(infoCalls, "expected /api/info to be queried for install-root awareness").toBe(1);
|
||||
expect(lastBody.cwdOverride).toBe(process.cwd());
|
||||
expect(logs.some((l) => l.includes("Defaulting cwd to current directory because the spec lives inside the OpenRig install"))).toBe(true);
|
||||
} finally {
|
||||
server.removeAllListeners("request");
|
||||
for (const l of origListeners) server.on("request", l as (...args: unknown[]) => void);
|
||||
fs.rmSync(installRoot, { recursive: true, force: true });
|
||||
}
|
||||
});
|
||||
|
||||
it("up path-form spec OUTSIDE install root leaves cwdOverride undefined", async () => {
|
||||
const fs = await import("node:fs");
|
||||
const path = await import("node:path");
|
||||
const os = await import("node:os");
|
||||
|
||||
const installRoot = fs.mkdtempSync(path.join(os.tmpdir(), "up-bug3-install-"));
|
||||
const projectRoot = fs.mkdtempSync(path.join(os.tmpdir(), "up-bug3-project-"));
|
||||
const specPath = path.join(projectRoot, "rig.yaml");
|
||||
fs.writeFileSync(specPath, "version: '0.2'\nname: outside-rig\npods: []\nedges: []\n");
|
||||
|
||||
let lastBody: Record<string, unknown> = {};
|
||||
const origListeners = server.listeners("request");
|
||||
server.removeAllListeners("request");
|
||||
server.on("request", async (req: http.IncomingMessage, res: http.ServerResponse) => {
|
||||
let body = "";
|
||||
for await (const chunk of req) body += chunk;
|
||||
if (req.url === "/api/info" && req.method === "GET") {
|
||||
res.writeHead(200, { "Content-Type": "application/json" });
|
||||
res.end(JSON.stringify({ installRoot }));
|
||||
return;
|
||||
}
|
||||
if (req.url === "/api/up" && req.method === "POST") {
|
||||
lastBody = JSON.parse(body);
|
||||
res.writeHead(201, { "Content-Type": "application/json" });
|
||||
res.end(JSON.stringify({ status: "completed", runId: "r", rigId: "g", stages: [], errors: [] }));
|
||||
return;
|
||||
}
|
||||
res.writeHead(404, { "Content-Type": "application/json" });
|
||||
res.end(JSON.stringify({ error: "not found" }));
|
||||
});
|
||||
|
||||
try {
|
||||
const { logs } = await captureLogs(async () => {
|
||||
await makeCmd().parseAsync(["node", "rig", "up", specPath]);
|
||||
});
|
||||
expect(lastBody.cwdOverride).toBeUndefined();
|
||||
expect(logs.some((l) => l.includes("Defaulting cwd to current directory because the spec lives inside the OpenRig install"))).toBe(false);
|
||||
} finally {
|
||||
server.removeAllListeners("request");
|
||||
for (const l of origListeners) server.on("request", l as (...args: unknown[]) => void);
|
||||
fs.rmSync(installRoot, { recursive: true, force: true });
|
||||
fs.rmSync(projectRoot, { recursive: true, force: true });
|
||||
}
|
||||
});
|
||||
|
||||
it("up apply uses a long-running daemon timeout budget", async () => {
|
||||
let timeoutMs: number | undefined;
|
||||
const post = vi.fn(async (_path: string, _body: unknown, options?: { timeoutMs?: number }) => {
|
||||
|
||||
@@ -0,0 +1,22 @@
|
||||
import { Hono } from "hono";
|
||||
import { getOpenRigInstallRoot } from "../domain/cwd-resolution.js";
|
||||
|
||||
// GET /api/info — daemon-info surface for tactical CLI awareness paths
|
||||
// (OPR.0.3.2.22 Bug 3 first consumer: CLI default-cwd extension for
|
||||
// path-form `rig up <install-internal-spec>` without --cwd).
|
||||
//
|
||||
// installRoot is the daemon's on-disk install root (the parent of the
|
||||
// daemon's package directory). The CLI uses it to detect when a spec
|
||||
// path lives inside the OpenRig install — the case that hits
|
||||
// getOpenRigInstallCwdError at preflight without a --cwd override.
|
||||
export function infoRoutes(): Hono {
|
||||
const app = new Hono();
|
||||
|
||||
app.get("/", (c) => {
|
||||
return c.json({
|
||||
installRoot: getOpenRigInstallRoot(),
|
||||
});
|
||||
});
|
||||
|
||||
return app;
|
||||
}
|
||||
@@ -42,6 +42,7 @@ import type { PsProjectionService } from "./domain/ps-projection.js";
|
||||
import type { UpCommandRouter } from "./domain/up-command-router.js";
|
||||
import type { RigTeardownOrchestrator } from "./domain/rig-teardown.js";
|
||||
import { upRoutes } from "./routes/up.js";
|
||||
import { infoRoutes } from "./routes/info.js";
|
||||
import { downRoutes } from "./routes/down.js";
|
||||
import { kernelStatusRoutes } from "./routes/kernel-status.js";
|
||||
import type { TranscriptStore } from "./domain/transcript-store.js";
|
||||
@@ -483,6 +484,7 @@ export function createApp(deps: AppDeps): Hono {
|
||||
app.route("/api/bundles", bundleRoutes);
|
||||
app.route("/api/ps", psRoutes);
|
||||
app.route("/api/up", upRoutes);
|
||||
app.route("/api/info", infoRoutes());
|
||||
app.route("/api/down", downRoutes);
|
||||
app.route("/api/kernel", kernelStatusRoutes);
|
||||
app.route("/api/transcripts", transcriptRoutes());
|
||||
|
||||
@@ -0,0 +1,22 @@
|
||||
import { describe, it, expect } from "vitest";
|
||||
import { Hono } from "hono";
|
||||
import { infoRoutes } from "../src/routes/info.js";
|
||||
import { getOpenRigInstallRoot } from "../src/domain/cwd-resolution.js";
|
||||
|
||||
// OPR.0.3.2.22 Bug 3 — daemon-info surface for tactical CLI awareness
|
||||
// paths. The CLI consumes /api/info to decide when path-form
|
||||
// `rig up <spec>` lands inside the OpenRig install (the case that
|
||||
// trips getOpenRigInstallCwdError at preflight unless --cwd is given).
|
||||
describe("GET /api/info", () => {
|
||||
it("returns the daemon installRoot computed from cwd-resolution", async () => {
|
||||
const app = new Hono();
|
||||
app.route("/api/info", infoRoutes());
|
||||
|
||||
const res = await app.request("/api/info");
|
||||
expect(res.status).toBe(200);
|
||||
const body = await res.json() as { installRoot: string };
|
||||
expect(body.installRoot).toBe(getOpenRigInstallRoot());
|
||||
expect(typeof body.installRoot).toBe("string");
|
||||
expect(body.installRoot.length).toBeGreaterThan(0);
|
||||
});
|
||||
});
|
||||
Reference in new issue
Block a user