mirror of
https://github.com/nexu-io/open-design.git
synced 2026-06-01 03:14:35 +07:00
fix(diagnostics): capture daemon/web logs in packaged bundles (#3126)
Packaged diagnostics bundles never contained the daemon or web
`latest.log` — the very logs that hold the agent/critique run flow — so
support exports could not explain "sent prompt to the agent, then
nothing happened" reports.
Root cause: the sidecar `base` means different things per launch path.
tools-dev passes the pre-namespace source root, so
`resolveNamespaceRoot(base, namespace)` is correct. But the packaged
orchestrator launches every child with `base = <namespaceRoot>/runtime`
(apps/packaged/src/{paths,sidecars}.ts) while logs live a level up at
`<namespaceRoot>/logs`. The diagnostics builders re-appended the
namespace and resolved every log to
`<namespaceRoot>/runtime/<namespace>/logs/...` → ENOENT. renderer.log
only survived by accident: the desktop main process wrote it to the
same wrong path the reader looked in.
Add `resolveRuntimeNamespaceRoot(runtime, contract, runtimeMode)` to
`@open-design/sidecar` which walks up out of the `runtime/` dir in
packaged (runtime-mode) launches and falls back to the dev layout
otherwise. Route the desktop renderer-log path and both diagnostics
exporters (desktop IPC + daemon HTTP) through it so writer and reader
stay in lockstep and renderer.log lands next to the desktop log dir.
Tests: sidecar unit specs for both layouts; a daemon export spec that
writes a real `<namespaceRoot>/logs/daemon/latest.log` and asserts the
bundle captures its contents (red on main → ENOENT placeholder, green
here).
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
parent
bc44a8add3
commit
b8cfee0c60
6 changed files with 174 additions and 12 deletions
|
|
@ -13,11 +13,12 @@ import {
|
|||
import {
|
||||
APP_KEYS,
|
||||
OPEN_DESIGN_SIDECAR_CONTRACT,
|
||||
SIDECAR_MODES,
|
||||
type SidecarStamp,
|
||||
} from '@open-design/sidecar-proto';
|
||||
import {
|
||||
resolveLogFilePath,
|
||||
resolveNamespaceRoot,
|
||||
resolveRuntimeNamespaceRoot,
|
||||
type SidecarRuntimeContext,
|
||||
} from '@open-design/sidecar';
|
||||
|
||||
|
|
@ -48,10 +49,14 @@ export const STANDALONE_LAUNCH_WARNING =
|
|||
|
||||
function buildSidecarLogSources(runtime: SidecarRuntimeContext<SidecarStamp> | null): LogSource[] {
|
||||
if (runtime == null) return [];
|
||||
const namespaceRoot = resolveNamespaceRoot({
|
||||
base: runtime.base,
|
||||
// In packaged builds `runtime.base` is `<namespaceRoot>/runtime`, so the log
|
||||
// tree lives a level UP at `<namespaceRoot>/logs`; `resolveRuntimeNamespaceRoot`
|
||||
// accounts for that (a plain `resolveNamespaceRoot` here resolved every
|
||||
// daemon/web log to an ENOENT phantom path and captured none of them).
|
||||
const namespaceRoot = resolveRuntimeNamespaceRoot({
|
||||
contract: OPEN_DESIGN_SIDECAR_CONTRACT,
|
||||
namespace: runtime.namespace,
|
||||
runtime,
|
||||
runtimeMode: SIDECAR_MODES.RUNTIME,
|
||||
});
|
||||
const apps = [APP_KEYS.DAEMON, APP_KEYS.WEB, APP_KEYS.DESKTOP];
|
||||
const sources: LogSource[] = [];
|
||||
|
|
|
|||
|
|
@ -1,6 +1,19 @@
|
|||
import { mkdir, rm, writeFile } from 'node:fs/promises';
|
||||
import { tmpdir } from 'node:os';
|
||||
import { dirname, join } from 'node:path';
|
||||
import { randomUUID } from 'node:crypto';
|
||||
|
||||
import { describe, expect, it } from 'vitest';
|
||||
import JSZip from 'jszip';
|
||||
|
||||
import {
|
||||
APP_KEYS,
|
||||
SIDECAR_MODES,
|
||||
SIDECAR_SOURCES,
|
||||
type SidecarStamp,
|
||||
} from '@open-design/sidecar-proto';
|
||||
import type { SidecarRuntimeContext } from '@open-design/sidecar';
|
||||
|
||||
import {
|
||||
STANDALONE_LAUNCH_WARNING,
|
||||
createDiagnosticsExportHandler,
|
||||
|
|
@ -50,3 +63,55 @@ describe('diagnostics export handler — non-sidecar launch', () => {
|
|||
expect(manifest.files).toEqual([]);
|
||||
});
|
||||
});
|
||||
|
||||
describe('diagnostics export handler — packaged (runtime) layout', () => {
|
||||
// Regression for the namespaceRoot off-by-one that left every packaged
|
||||
// bundle without daemon/web logs (the agent-run flow lives in the daemon
|
||||
// log). In packaged builds the orchestrator launches each child with
|
||||
// `base = <namespaceRoot>/runtime` while the logs live a level up at
|
||||
// `<namespaceRoot>/logs`. The old `resolveNamespaceRoot(base, namespace)`
|
||||
// resolved the daemon log to `<namespaceRoot>/runtime/<namespace>/logs/...`
|
||||
// → ENOENT, so the bundle silently captured nothing.
|
||||
it('captures the daemon log from the real <namespaceRoot>/logs tree', async () => {
|
||||
const root = join(tmpdir(), `od-diag-${randomUUID()}`);
|
||||
const namespaceRoot = join(root, 'namespaces', 'release-stable');
|
||||
const daemonLogPath = join(namespaceRoot, 'logs', APP_KEYS.DAEMON, 'latest.log');
|
||||
const marker = 'DAEMON-LOG-MARKER critique runId=rc100-poster';
|
||||
try {
|
||||
await mkdir(dirname(daemonLogPath), { recursive: true });
|
||||
await writeFile(daemonLogPath, `${marker}\n`, 'utf8');
|
||||
|
||||
const runtime: SidecarRuntimeContext<SidecarStamp> = {
|
||||
app: APP_KEYS.DAEMON,
|
||||
// packaged launches children with base == <namespaceRoot>/runtime
|
||||
base: join(namespaceRoot, 'runtime'),
|
||||
ipc: '/tmp/od-diag-test-daemon.sock',
|
||||
mode: SIDECAR_MODES.RUNTIME,
|
||||
namespace: 'release-stable',
|
||||
source: SIDECAR_SOURCES.PACKAGED,
|
||||
};
|
||||
|
||||
const handler = createDiagnosticsExportHandler({ runtime, projectRoot: '/tmp/test-project' });
|
||||
const res = mockResponse();
|
||||
await handler({} as never, res as never, () => undefined);
|
||||
|
||||
expect(res.capturedStatus).toBe(200);
|
||||
const zip = await JSZip.loadAsync(res.capturedPayload!);
|
||||
|
||||
// The log must be present with its real contents, not a missing-file
|
||||
// placeholder.
|
||||
const daemonEntry = zip.file('logs/daemon/latest.log');
|
||||
expect(daemonEntry).not.toBeNull();
|
||||
expect(await daemonEntry!.async('string')).toContain(marker);
|
||||
|
||||
const manifest = JSON.parse(await zip.file('summary/manifest.json')!.async('string')) as {
|
||||
files: { name: string; bytes: number; error?: string }[];
|
||||
};
|
||||
const daemonFile = manifest.files.find((f) => f.name === 'logs/daemon/latest.log');
|
||||
expect(daemonFile?.error).toBeUndefined();
|
||||
expect(daemonFile?.bytes ?? 0).toBeGreaterThan(0);
|
||||
} finally {
|
||||
await rm(root, { recursive: true, force: true });
|
||||
}
|
||||
});
|
||||
});
|
||||
|
|
|
|||
|
|
@ -7,11 +7,12 @@ import { BrowserWindow, app, dialog, ipcMain, shell } from "electron";
|
|||
import {
|
||||
APP_KEYS,
|
||||
OPEN_DESIGN_SIDECAR_CONTRACT,
|
||||
SIDECAR_MODES,
|
||||
type SidecarStamp,
|
||||
} from "@open-design/sidecar-proto";
|
||||
import {
|
||||
resolveLogFilePath,
|
||||
resolveNamespaceRoot,
|
||||
resolveRuntimeNamespaceRoot,
|
||||
type SidecarRuntimeContext,
|
||||
} from "@open-design/sidecar";
|
||||
import {
|
||||
|
|
@ -40,10 +41,14 @@ function safeUsername(): string | undefined {
|
|||
}
|
||||
|
||||
function buildSidecarLogSources(runtime: SidecarRuntimeContext<SidecarStamp>): LogSource[] {
|
||||
const namespaceRoot = resolveNamespaceRoot({
|
||||
base: runtime.base,
|
||||
// In packaged builds `runtime.base` is `<namespaceRoot>/runtime`, so the log
|
||||
// tree lives a level UP at `<namespaceRoot>/logs`; `resolveRuntimeNamespaceRoot`
|
||||
// accounts for that (a plain `resolveNamespaceRoot` here resolved every
|
||||
// daemon/web log to an ENOENT phantom path and captured none of them).
|
||||
const namespaceRoot = resolveRuntimeNamespaceRoot({
|
||||
contract: OPEN_DESIGN_SIDECAR_CONTRACT,
|
||||
namespace: runtime.namespace,
|
||||
runtime,
|
||||
runtimeMode: SIDECAR_MODES.RUNTIME,
|
||||
});
|
||||
const apps = [APP_KEYS.DAEMON, APP_KEYS.WEB, APP_KEYS.DESKTOP];
|
||||
const sources: LogSource[] = [];
|
||||
|
|
|
|||
|
|
@ -9,6 +9,7 @@ import {
|
|||
OPEN_DESIGN_SIDECAR_CONTRACT,
|
||||
SIDECAR_ENV,
|
||||
SIDECAR_MESSAGES,
|
||||
SIDECAR_MODES,
|
||||
normalizeDesktopSidecarMessage,
|
||||
type DesktopClickInput,
|
||||
type DesktopEvalInput,
|
||||
|
|
@ -27,7 +28,7 @@ import {
|
|||
requestJsonIpc,
|
||||
resolveAppIpcPath,
|
||||
resolveLogFilePath,
|
||||
resolveNamespaceRoot,
|
||||
resolveRuntimeNamespaceRoot,
|
||||
type JsonIpcServerHandle,
|
||||
type SidecarRuntimeContext,
|
||||
} from "@open-design/sidecar";
|
||||
|
|
@ -363,10 +364,17 @@ export async function runDesktopMain(
|
|||
},
|
||||
{ openPath: (path) => shell.openPath(path) },
|
||||
);
|
||||
const namespaceRoot = resolveNamespaceRoot({
|
||||
base: runtime.base,
|
||||
// Resolve the namespace root the same way the diagnostics export does
|
||||
// (apps/desktop/src/main/diagnostics.ts). In packaged builds `runtime.base`
|
||||
// is `<namespaceRoot>/runtime`, so re-appending the namespace via
|
||||
// `resolveNamespaceRoot` would write renderer.log to a phantom
|
||||
// `<namespaceRoot>/runtime/<namespace>/logs/desktop` dir that the export
|
||||
// reader never looks in. Keeping both sides on `resolveRuntimeNamespaceRoot`
|
||||
// co-locates renderer.log with the desktop log dir AND keeps it captured.
|
||||
const namespaceRoot = resolveRuntimeNamespaceRoot({
|
||||
contract: OPEN_DESIGN_SIDECAR_CONTRACT,
|
||||
namespace: runtime.namespace,
|
||||
runtime,
|
||||
runtimeMode: SIDECAR_MODES.RUNTIME,
|
||||
});
|
||||
const desktopLogPath = resolveLogFilePath({
|
||||
app: APP_KEYS.DESKTOP,
|
||||
|
|
|
|||
|
|
@ -180,6 +180,48 @@ export function resolveNamespaceRoot<TStamp extends SidecarStampShape>({
|
|||
return join(resolve(base), contract.normalizeNamespace(namespace));
|
||||
}
|
||||
|
||||
/**
|
||||
* Resolve the namespace root (the directory that holds `logs/`, `runs/`,
|
||||
* `current.json`, …) from a *live* {@link SidecarRuntimeContext}.
|
||||
*
|
||||
* `resolveNamespaceRoot` alone is not enough here because the sidecar `base`
|
||||
* carries a different meaning depending on how the process was launched:
|
||||
*
|
||||
* - **dev (`tools-dev`):** every child is launched with `base` set to the
|
||||
* pre-namespace source runtime root (see `tools/dev/src/config.ts`), so the
|
||||
* namespace root is `base/<namespace>` — exactly what `resolveNamespaceRoot`
|
||||
* computes.
|
||||
* - **packaged (runtime mode):** the orchestrator launches every child with
|
||||
* `base = <namespaceRoot>/runtime` (see `apps/packaged/src/paths.ts` →
|
||||
* `runtimeRoot`, wired in `apps/packaged/src/sidecars.ts`), while the actual
|
||||
* logs live as a sibling at `<namespaceRoot>/logs`. Re-appending the
|
||||
* namespace via `resolveNamespaceRoot` would yield
|
||||
* `<namespaceRoot>/runtime/<namespace>`, so every daemon/web log file
|
||||
* resolved that way is an ENOENT — which is why packaged diagnostics bundles
|
||||
* used to capture none of them.
|
||||
*
|
||||
* Callers pass their contract's runtime-mode constant (e.g.
|
||||
* `SIDECAR_MODES.RUNTIME`) so this generic helper does not have to hardcode
|
||||
* Open Design's mode strings.
|
||||
*/
|
||||
export function resolveRuntimeNamespaceRoot<TStamp extends SidecarStampShape>({
|
||||
contract,
|
||||
runtime,
|
||||
runtimeMode,
|
||||
}: {
|
||||
contract: SidecarContractDescriptor<TStamp>;
|
||||
runtime: Pick<SidecarRuntimeContext<TStamp>, "base" | "mode" | "namespace">;
|
||||
runtimeMode: TStamp["mode"] | string;
|
||||
}): string {
|
||||
if (runtime.mode === runtimeMode) {
|
||||
// packaged: `base` already points INTO the namespace tree
|
||||
// (`<namespaceRoot>/runtime`), so the namespace root is its parent.
|
||||
return dirname(resolve(runtime.base));
|
||||
}
|
||||
// dev / tools-dev: `base` is the pre-namespace source runtime root.
|
||||
return resolveNamespaceRoot({ base: runtime.base, contract, namespace: runtime.namespace });
|
||||
}
|
||||
|
||||
export function resolveRuntimeRoot<TStamp extends SidecarStampShape>({
|
||||
base,
|
||||
contract,
|
||||
|
|
|
|||
|
|
@ -6,8 +6,10 @@ import {
|
|||
createSidecarLaunchEnv,
|
||||
resolveAppIpcPath,
|
||||
resolveAppRuntimePath,
|
||||
resolveLogFilePath,
|
||||
resolveNamespace,
|
||||
resolveNamespaceRoot,
|
||||
resolveRuntimeNamespaceRoot,
|
||||
resolveSidecarBase,
|
||||
resolveSourceRuntimeRoot,
|
||||
type SidecarContractDescriptor,
|
||||
|
|
@ -129,3 +131,38 @@ describe("generic sidecar bootstrap", () => {
|
|||
});
|
||||
});
|
||||
});
|
||||
|
||||
describe("resolveRuntimeNamespaceRoot", () => {
|
||||
// dev / tools-dev: `base` is the pre-namespace source root, so the namespace
|
||||
// is appended — identical to plain `resolveNamespaceRoot`.
|
||||
it("appends the namespace for pre-namespace (dev) bases", () => {
|
||||
const namespaceRoot = resolveRuntimeNamespaceRoot({
|
||||
contract: fakeContract,
|
||||
runtime: { base: "/runtime/base", mode: "dev", namespace: "alpha" },
|
||||
runtimeMode: "prod",
|
||||
});
|
||||
expect(namespaceRoot).toBe(join(resolve("/runtime/base"), "alpha"));
|
||||
});
|
||||
|
||||
// packaged: the orchestrator launches children with `base = <namespaceRoot>/runtime`,
|
||||
// so the namespace root is the PARENT of `base` and logs resolve to
|
||||
// `<namespaceRoot>/logs/...`. Re-appending the namespace (the old bug) would
|
||||
// point at `<namespaceRoot>/runtime/<namespace>/logs/...` → ENOENT.
|
||||
it("walks up out of the runtime dir for packaged bases", () => {
|
||||
const runtime = { base: "/data/ns/alpha/runtime", mode: "prod", namespace: "alpha" } as const;
|
||||
const namespaceRoot = resolveRuntimeNamespaceRoot({
|
||||
contract: fakeContract,
|
||||
runtime,
|
||||
runtimeMode: "prod",
|
||||
});
|
||||
expect(namespaceRoot).toBe(resolve("/data/ns/alpha"));
|
||||
expect(
|
||||
resolveLogFilePath({ app: "api", contract: fakeContract, runtimeRoot: namespaceRoot }),
|
||||
).toBe(join(resolve("/data/ns/alpha"), "logs", "api", "latest.log"));
|
||||
// The old `resolveNamespaceRoot(base, namespace)` path would have produced
|
||||
// a phantom dir nested under `runtime/`.
|
||||
expect(
|
||||
resolveNamespaceRoot({ base: runtime.base, contract: fakeContract, namespace: runtime.namespace }),
|
||||
).toBe(join(resolve("/data/ns/alpha/runtime"), "alpha"));
|
||||
});
|
||||
});
|
||||
|
|
|
|||
Loading…
Reference in a new issue