From f3c84c9b2ce7b2aa807e6df4e289bbc9a97c837c Mon Sep 17 00:00:00 2001 From: Test Date: Sat, 4 Apr 2026 22:18:10 -0300 Subject: [PATCH] fix(agent-sync): surface workspace resolution failures, stop tmp path poisoning MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Root cause of "my agent exists on disk but genie spawn can't find it": startAgentSync() in serve.ts silently returned null when findWorkspace() returned null, disabling the entire auto-discovery subsystem (initial sync + file watcher) for the whole serve lifetime. Users saw new agents in workspace/agents/ never register and received no diagnostic anywhere. The reason findWorkspace() returned null in practice: test runs persist their tmpdir workspace paths to ~/.genie/config.json via saveWorkspaceRoot(). Once a serve boots from outside the real workspace, walk-up fails, the fallback hits the stale tmp path (now removed), existsSync returns false, returns null. Fixes: - workspace.ts: saveWorkspaceRoot() refuses to persist paths under os.tmpdir(), so test runs can't poison the global config. - workspace.ts: loadWorkspaceRoot() self-heals — if the saved path no longer has a .genie/workspace.json marker, scrub it from config instead of returning a stale fallback forever. - workspace.ts: GENIE_HOME is now resolved lazily via genieHome() so tests can override it via process.env.GENIE_HOME per-test. - serve.ts: startAgentSync() logs loudly when workspace is unresolved, when sync has per-agent errors, and when the watcher fails to start. Also includes the resolved workspace in the success log so "which workspace did serve attach to" is visible at a glance. - workspace.test.ts: two new tests covering tmp-path guard and stale-path self-heal; tests now isolate GENIE_HOME so they don't touch the developer's real ~/.genie/config.json. --- src/lib/workspace.test.ts | 47 ++++++++++++++++++++++++++++++++++- src/lib/workspace.ts | 50 +++++++++++++++++++++++++++++++++----- src/term-commands/serve.ts | 22 +++++++++++++++-- 3 files changed, 110 insertions(+), 9 deletions(-) diff --git a/src/lib/workspace.test.ts b/src/lib/workspace.test.ts index 1a0df8b13..269842b17 100644 --- a/src/lib/workspace.test.ts +++ b/src/lib/workspace.test.ts @@ -1,18 +1,30 @@ import { afterEach, beforeEach, describe, expect, test } from 'bun:test'; -import { mkdirSync, rmSync, writeFileSync } from 'node:fs'; +import { existsSync, mkdirSync, readFileSync, rmSync, writeFileSync } from 'node:fs'; import { tmpdir } from 'node:os'; import { join } from 'node:path'; import { findWorkspace, getWorkspaceConfig, scanAgents } from './workspace.js'; let testDir: string; +let fakeGenieHome: string; +const originalGenieHome = process.env.GENIE_HOME; beforeEach(() => { testDir = join(tmpdir(), `genie-workspace-test-${Date.now()}-${Math.random().toString(36).slice(2)}`); mkdirSync(testDir, { recursive: true }); + // Isolate ~/.genie/config.json from the developer's real config for every test. + fakeGenieHome = join(testDir, '.fake-genie-home'); + mkdirSync(fakeGenieHome, { recursive: true }); + process.env.GENIE_HOME = fakeGenieHome; }); afterEach(() => { rmSync(testDir, { recursive: true, force: true }); + if (originalGenieHome === undefined) { + // biome-ignore lint/performance/noDelete: process.env assignment coerces undefined→"undefined"; delete is the only correct unset + delete process.env.GENIE_HOME; + } else { + process.env.GENIE_HOME = originalGenieHome; + } }); function makeWorkspace(root: string, config: Record = { name: 'test-ws' }) { @@ -95,6 +107,39 @@ describe('getWorkspaceConfig', () => { }); }); +describe('workspaceRoot persistence', () => { + test('does not persist workspaces under tmpdir() to GENIE_HOME config', () => { + // testDir is under tmpdir() by construction — walk-up will succeed, but + // saveWorkspaceRoot must refuse to write it, so the global config stays clean. + makeWorkspace(testDir); + findWorkspace(testDir); + + const configPath = join(fakeGenieHome, 'config.json'); + if (existsSync(configPath)) { + const config = JSON.parse(readFileSync(configPath, 'utf-8')); + expect(config.workspaceRoot).toBeUndefined(); + } + // (if the file doesn't exist at all, that's also a pass — nothing was persisted) + }); + + test('clears stale workspaceRoot from config when the saved path is gone', () => { + // Simulate a prior run that persisted a path which has since vanished. + const vanished = join(testDir, 'was-here-now-gone'); + const configPath = join(fakeGenieHome, 'config.json'); + writeFileSync(configPath, JSON.stringify({ workspaceRoot: vanished })); + + // findWorkspace from an unrelated dir triggers the fallback → stale path detected → cleared. + const unrelated = join(testDir, 'unrelated'); + mkdirSync(unrelated, { recursive: true }); + const result = findWorkspace(unrelated); + expect(result).toBeNull(); + + // The poisoned entry should be scrubbed from the config so future calls don't keep hitting it. + const config = JSON.parse(readFileSync(configPath, 'utf-8')); + expect(config.workspaceRoot).toBeUndefined(); + }); +}); + describe('scanAgents', () => { test('lists agents with AGENTS.md', () => { makeWorkspace(testDir); diff --git a/src/lib/workspace.ts b/src/lib/workspace.ts index 0c021aa2c..629724339 100644 --- a/src/lib/workspace.ts +++ b/src/lib/workspace.ts @@ -6,7 +6,7 @@ */ import { existsSync, mkdirSync, readFileSync, readdirSync, writeFileSync } from 'node:fs'; -import { homedir } from 'node:os'; +import { homedir, tmpdir } from 'node:os'; import { dirname, join, resolve, sep } from 'node:path'; // ─── Types ──────────────────────────────────────────────────────────────────── @@ -62,15 +62,43 @@ export function findWorkspace(cwd?: string): WorkspaceInfo | null { return null; } -const GENIE_HOME = process.env.GENIE_HOME ?? join(homedir(), '.genie'); +/** Resolved lazily so GENIE_HOME env overrides in tests take effect. */ +function genieHome(): string { + return process.env.GENIE_HOME ?? join(homedir(), '.genie'); +} + +/** True if `root` is under the OS temp directory (avoids persisting test workspaces). */ +function isTempPath(root: string): boolean { + const normalizedTmp = resolve(tmpdir()); + const normalizedRoot = resolve(root); + return normalizedRoot === normalizedTmp || normalizedRoot.startsWith(normalizedTmp + sep); +} function saveWorkspaceRoot(root: string): void { + // Never persist tmp paths — tests run from tmpdir() and would poison the + // global fallback, causing subsequent daemons to resolve a stale path. + if (isTempPath(root)) return; try { - const configPath = join(GENIE_HOME, 'config.json'); + const home = genieHome(); + const configPath = join(home, 'config.json'); const config = existsSync(configPath) ? JSON.parse(readFileSync(configPath, 'utf-8')) : {}; if (config.workspaceRoot === root) return; config.workspaceRoot = root; - mkdirSync(GENIE_HOME, { recursive: true }); + mkdirSync(home, { recursive: true }); + writeFileSync(configPath, `${JSON.stringify(config, null, 2)}\n`, 'utf-8'); + } catch { + /* best-effort */ + } +} + +/** Clear the saved workspaceRoot from /config.json. */ +function clearWorkspaceRoot(): void { + try { + const configPath = join(genieHome(), 'config.json'); + if (!existsSync(configPath)) return; + const config = JSON.parse(readFileSync(configPath, 'utf-8')); + if (config.workspaceRoot === undefined) return; + config.workspaceRoot = undefined; writeFileSync(configPath, `${JSON.stringify(config, null, 2)}\n`, 'utf-8'); } catch { /* best-effort */ @@ -79,10 +107,20 @@ function saveWorkspaceRoot(root: string): void { function loadWorkspaceRoot(): string | null { try { - const configPath = join(GENIE_HOME, 'config.json'); + const configPath = join(genieHome(), 'config.json'); if (!existsSync(configPath)) return null; const config = JSON.parse(readFileSync(configPath, 'utf-8')); - return typeof config.workspaceRoot === 'string' ? config.workspaceRoot : null; + const saved = typeof config.workspaceRoot === 'string' ? config.workspaceRoot : null; + if (!saved) return null; + + // Self-heal: if the saved path no longer has a workspace marker (e.g. a + // test cleanup removed it, or the workspace was moved), clear it rather + // than keep returning a broken fallback. + if (!existsSync(join(saved, WORKSPACE_MARKER))) { + clearWorkspaceRoot(); + return null; + } + return saved; } catch { return null; } diff --git a/src/term-commands/serve.ts b/src/term-commands/serve.ts index f8dee0548..f2a1129f1 100644 --- a/src/term-commands/serve.ts +++ b/src/term-commands/serve.ts @@ -371,13 +371,29 @@ async function startAgentSync(): Promise<{ close: () => void } | null> { try { const { findWorkspace } = require('../lib/workspace.js') as typeof import('../lib/workspace.js'); const ws = findWorkspace(); - if (!ws) return null; + if (!ws) { + // Loud failure — silent return used to hide the whole discovery subsystem + // when serve booted from outside a workspace (or with a stale saved root). + console.warn(' Agent sync: DISABLED — no workspace found from cwd or ~/.genie/config.json'); + console.warn(' Fix: `cd && genie serve restart`, or run `genie init` to bootstrap one'); + return null; + } const { syncAgentDirectory, watchAgentDirectory } = await import('../lib/agent-sync.js'); const syncResult = await syncAgentDirectory(ws.root); const synced = syncResult.registered.length + syncResult.updated.length; if (synced > 0) { - console.log(` Agent sync: ${syncResult.registered.length} registered, ${syncResult.updated.length} updated`); + console.log( + ` Agent sync: ${syncResult.registered.length} registered, ${syncResult.updated.length} updated (workspace: ${ws.root})`, + ); + } else { + console.log(` Agent sync: up to date (workspace: ${ws.root})`); + } + if (syncResult.errors.length > 0) { + console.warn(` Agent sync: ${syncResult.errors.length} error(s) — these agents were NOT registered:`); + for (const e of syncResult.errors) { + console.warn(` ${e.name}: ${e.error}`); + } } const watcher = watchAgentDirectory(ws.root, { @@ -387,6 +403,8 @@ async function startAgentSync(): Promise<{ close: () => void } | null> { }); if (watcher) { console.log(' Agent watcher started (watching agents/ directory)'); + } else { + console.warn(' Agent watcher: FAILED to start — new agents will not be auto-registered'); } return watcher; } catch (err) {