diff --git a/src/commands/mine-local.ts b/src/commands/mine-local.ts index b1113bbd1..8eb8e9d84 100644 --- a/src/commands/mine-local.ts +++ b/src/commands/mine-local.ts @@ -38,6 +38,7 @@ import { import { extractPairs, type Pair } from "../skillify/extractors/index.js"; import { findAgentBin, type Agent } from "../skillify/gate-runner.js"; import { extractJsonBlock } from "../skillify/gate-parser.js"; +import { permissionFlags, type ClaudeGrants } from "../hooks/wiki-worker-spawn.js"; import { resolveSkillsRoot, writeNewSkill, listSkills, parseFrontmatter } from "../skillify/skill-writer.js"; import { detectAgentSkillsRoots } from "../skillify/agent-roots.js"; import { fanOutSymlinks } from "../skillify/pull.js"; @@ -91,6 +92,12 @@ function runGateViaStdin(opts: { bin: string; prompt: string; timeoutMs: number; + /** + * Dirs/tools to grant explicitly instead of the blanket `bypassPermissions`, + * which an enterprise policy can disable. See ClaudeGrants in + * ../hooks/wiki-worker-spawn.ts. + */ + grants?: ClaudeGrants; }): Promise<{ stdout: string; stderr: string; errored: boolean; errorMessage?: string }> { return new Promise((resolve) => { if (opts.agent !== "claude_code") { @@ -116,7 +123,7 @@ function runGateViaStdin(opts: { "-p", "--no-session-persistence", "--model", "haiku", - "--permission-mode", "bypassPermissions", + ...permissionFlags(opts.grants), ]; const child = spawn(opts.bin, args, { stdio: ["pipe", "pipe", "pipe"], @@ -586,7 +593,16 @@ async function runMineLocalImpl(args: string[]): Promise { const prompt = buildSessionPrompt(tail, s, verdictPath); writeFileSync(join(sessionTmp, "prompt.txt"), prompt); - const gate = await runGateViaStdin({ agent: gateAgent, bin: gateBin, prompt, timeoutMs: GATE_TIMEOUT_MS }); + // sessionTmp holds the verdict path named in the prompt and lives outside + // the session cwd, so grant it explicitly rather than relying on the + // bypass an enterprise policy can disable. + const gate = await runGateViaStdin({ + agent: gateAgent, + bin: gateBin, + prompt, + timeoutMs: GATE_TIMEOUT_MS, + grants: { addDirs: [sessionTmp], allowedTools: ["Read", "Write"] }, + }); try { writeFileSync(join(sessionTmp, "gate-stdout.txt"), gate.stdout); if (gate.stderr) writeFileSync(join(sessionTmp, "gate-stderr.txt"), gate.stderr); diff --git a/src/hooks/codex/wiki-worker.ts b/src/hooks/codex/wiki-worker.ts index ff2745874..9353a6793 100644 --- a/src/hooks/codex/wiki-worker.ts +++ b/src/hooks/codex/wiki-worker.ts @@ -7,13 +7,13 @@ * Invoked by stop.ts as: node wiki-worker.js */ -import { readFileSync, writeFileSync, existsSync, appendFileSync, mkdirSync, rmSync } from "node:fs"; +import { readFileSync, writeFileSync, existsSync, statSync, utimesSync, appendFileSync, mkdirSync, rmSync } from "node:fs"; import { execFileSync } from "node:child_process"; import { buildTrailingPromptInvocation } from "../wiki-worker-spawn.js"; import { dirname, join } from "node:path"; import { fileURLToPath } from "node:url"; import { finalizeSummary, releaseLock, readState } from "../summary-state.js"; -import { capLinesByBytes, newRowsFromWindow, stampOffset, WIKI_FALLBACK_MAX_ROWS, WIKI_JSONL_MAX_BYTES } from "../wiki-offset.js"; +import { capLinesByBytes, markSummaryUnwritten, newRowsFromWindow, stampOffset, summaryWasWritten, WIKI_FALLBACK_MAX_ROWS, WIKI_JSONL_MAX_BYTES } from "../wiki-offset.js"; import { redactSecrets } from "../shared/redact.js"; import { uploadSummary } from "../upload-summary.js"; import { log as _log } from "../../utils/debug.js"; @@ -239,6 +239,7 @@ async function main(): Promise { wlog("running codex exec"); let execSucceeded = false; const summaryBeforeExec = existsSync(tmpSummary) ? readFileSync(tmpSummary, "utf-8") : null; + const summaryBaseline = markSummaryUnwritten(tmpSummary, { existsSync, utimesSync, statSync }); try { const inv = buildTrailingPromptInvocation(cfg.codexBin, [ "exec", @@ -271,6 +272,15 @@ async function main(): Promise { : "codex exec failed without producing a new summary; skipping upload"); return; } + // Exit 0 is not proof of work: a child that cannot reach tmpDir (or simply + // declines) exits 0 having written nothing, leaving the pre-seeded base + // summary in place. Uploading it unchanged would still advance the offset + // and slice those events away forever, which is how a session gets stuck + // as a header-only placeholder run after run. + if (!summaryWasWritten(tmpSummary, summaryBaseline, summaryChanged, { statSync })) { + wlog("codex exec exited 0 but never wrote the summary; skipping upload to avoid advancing the offset"); + return; + } if (raw.trim()) { // Stamp the offset ourselves so the persisted summary is authoritative // and never depends on the LLM echoing the bookkeeping line. diff --git a/src/hooks/cursor/wiki-worker.ts b/src/hooks/cursor/wiki-worker.ts index c7a03c27b..e46204dcd 100644 --- a/src/hooks/cursor/wiki-worker.ts +++ b/src/hooks/cursor/wiki-worker.ts @@ -12,7 +12,7 @@ * differs: codex shells `codex exec`, we shell `cursor-agent --print --model X`. */ -import { readFileSync, writeFileSync, existsSync, appendFileSync, mkdirSync, rmSync } from "node:fs"; +import { readFileSync, writeFileSync, existsSync, statSync, utimesSync, appendFileSync, mkdirSync, rmSync } from "node:fs"; import { execFileSync } from "node:child_process"; import { buildTrailingPromptInvocation } from "../wiki-worker-spawn.js"; import { dirname, join } from "node:path"; @@ -20,7 +20,7 @@ import { fileURLToPath } from "node:url"; import { finalizeSummary, releaseLock, readState } from "../summary-state.js"; import { readSessionEventCache } from "../session-event-cache.js"; import { buildSessionPath } from "../../utils/session-path.js"; -import { capLinesByBytes, newRowsFromWindow, stampOffset, WIKI_FALLBACK_MAX_ROWS, WIKI_JSONL_MAX_BYTES } from "../wiki-offset.js"; +import { capLinesByBytes, markSummaryUnwritten, newRowsFromWindow, stampOffset, summaryWasWritten, WIKI_FALLBACK_MAX_ROWS, WIKI_JSONL_MAX_BYTES } from "../wiki-offset.js"; import { redactSecrets } from "../shared/redact.js"; import { uploadSummary } from "../upload-summary.js"; import { log as _log } from "../../utils/debug.js"; @@ -274,6 +274,7 @@ async function main(): Promise { wlog(`running cursor-agent --print (model=${cfg.cursorModel})`); let execSucceeded = false; const summaryBeforeExec = existsSync(tmpSummary) ? readFileSync(tmpSummary, "utf-8") : null; + const summaryBaseline = markSummaryUnwritten(tmpSummary, { existsSync, utimesSync, statSync }); try { // cursor-agent --print is the non-interactive headless mode. --force // auto-allows tools (matches the bypass-approvals semantic codex used). @@ -312,6 +313,15 @@ async function main(): Promise { : "cursor-agent --print failed without producing a new summary; skipping upload"); return; } + // Exit 0 is not proof of work: a child that cannot reach tmpDir (or simply + // declines) exits 0 having written nothing, leaving the pre-seeded base + // summary in place. Uploading it unchanged would still advance the offset + // and slice those events away forever, which is how a session gets stuck + // as a header-only placeholder run after run. + if (!summaryWasWritten(tmpSummary, summaryBaseline, summaryChanged, { statSync })) { + wlog("cursor-agent --print exited 0 but never wrote the summary; skipping upload to avoid advancing the offset"); + return; + } if (raw.trim()) { // Stamp the offset ourselves so the persisted summary is authoritative // and never depends on the LLM echoing the bookkeeping line. diff --git a/src/hooks/hermes/wiki-worker.ts b/src/hooks/hermes/wiki-worker.ts index 335348ca5..f1c878b60 100644 --- a/src/hooks/hermes/wiki-worker.ts +++ b/src/hooks/hermes/wiki-worker.ts @@ -12,14 +12,14 @@ * differs: codex shells `codex exec`, we shell `hermes -z --provider X -m Y`. */ -import { readFileSync, writeFileSync, existsSync, appendFileSync, mkdirSync, rmSync } from "node:fs"; +import { readFileSync, writeFileSync, existsSync, statSync, utimesSync, appendFileSync, mkdirSync, rmSync } from "node:fs"; import { execFileSync } from "node:child_process"; import { dirname, join } from "node:path"; import { fileURLToPath } from "node:url"; import { finalizeSummary, releaseLock, readState } from "../summary-state.js"; import { readSessionEventCache } from "../session-event-cache.js"; import { buildSessionPath } from "../../utils/session-path.js"; -import { capLinesByBytes, newRowsFromWindow, stampOffset, WIKI_FALLBACK_MAX_ROWS, WIKI_JSONL_MAX_BYTES } from "../wiki-offset.js"; +import { capLinesByBytes, markSummaryUnwritten, newRowsFromWindow, stampOffset, summaryWasWritten, WIKI_FALLBACK_MAX_ROWS, WIKI_JSONL_MAX_BYTES } from "../wiki-offset.js"; import { redactSecrets } from "../shared/redact.js"; import { uploadSummary } from "../upload-summary.js"; import { log as _log } from "../../utils/debug.js"; @@ -276,6 +276,7 @@ async function main(): Promise { wlog(`running hermes -z (provider=${cfg.hermesProvider}, model=${cfg.hermesModel})`); let execSucceeded = false; const summaryBeforeExec = existsSync(tmpSummary) ? readFileSync(tmpSummary, "utf-8") : null; + const summaryBaseline = markSummaryUnwritten(tmpSummary, { existsSync, utimesSync, statSync }); try { // hermes -z (--oneshot) is the non-interactive mode. --yolo // auto-approves tool use within the spawned hermes process. @@ -324,6 +325,15 @@ async function main(): Promise { : "hermes -z failed without producing a new summary; skipping upload"); return; } + // Exit 0 is not proof of work: a child that cannot reach tmpDir (or simply + // declines) exits 0 having written nothing, leaving the pre-seeded base + // summary in place. Uploading it unchanged would still advance the offset + // and slice those events away forever, which is how a session gets stuck + // as a header-only placeholder run after run. + if (!summaryWasWritten(tmpSummary, summaryBaseline, summaryChanged, { statSync })) { + wlog("hermes -z exited 0 but never wrote the summary; skipping upload to avoid advancing the offset"); + return; + } if (raw.trim()) { // Stamp the offset ourselves so the persisted summary is authoritative // and never depends on the LLM echoing the bookkeeping line. diff --git a/src/hooks/pi/wiki-worker.ts b/src/hooks/pi/wiki-worker.ts index 8b03fb66f..fbbeb5bea 100644 --- a/src/hooks/pi/wiki-worker.ts +++ b/src/hooks/pi/wiki-worker.ts @@ -16,13 +16,13 @@ * we shell `pi --print --provider

--model `. Same query/upload paths. */ -import { readFileSync, writeFileSync, existsSync, appendFileSync, mkdirSync, rmSync } from "node:fs"; +import { readFileSync, writeFileSync, existsSync, statSync, utimesSync, appendFileSync, mkdirSync, rmSync } from "node:fs"; import { execFileSync } from "node:child_process"; import { buildTrailingPromptInvocation } from "../wiki-worker-spawn.js"; import { dirname, join } from "node:path"; import { fileURLToPath } from "node:url"; import { finalizeSummary, releaseLock, readState } from "../summary-state.js"; -import { capLinesByBytes, newRowsFromWindow, stampOffset, WIKI_FALLBACK_MAX_ROWS, WIKI_JSONL_MAX_BYTES } from "../wiki-offset.js"; +import { capLinesByBytes, markSummaryUnwritten, newRowsFromWindow, stampOffset, summaryWasWritten, WIKI_FALLBACK_MAX_ROWS, WIKI_JSONL_MAX_BYTES } from "../wiki-offset.js"; import { redactSecrets } from "../shared/redact.js"; import { uploadSummary } from "../upload-summary.js"; import { log as _log } from "../../utils/debug.js"; @@ -224,6 +224,7 @@ async function main(): Promise { wlog(`running pi --print (provider=${cfg.piProvider}, model=${cfg.piModel})`); let execSucceeded = false; const summaryBeforeExec = existsSync(tmpSummary) ? readFileSync(tmpSummary, "utf-8") : null; + const summaryBaseline = markSummaryUnwritten(tmpSummary, { existsSync, utimesSync, statSync }); try { // pi --print is the non-interactive mode; it bypasses extension // discovery (modes/print-mode.js doesn't import ExtensionRunner), @@ -263,6 +264,15 @@ async function main(): Promise { : "pi --print failed without producing a new summary; skipping upload"); return; } + // Exit 0 is not proof of work: a child that cannot reach tmpDir (or simply + // declines) exits 0 having written nothing, leaving the pre-seeded base + // summary in place. Uploading it unchanged would still advance the offset + // and slice those events away forever, which is how a session gets stuck + // as a header-only placeholder run after run. + if (!summaryWasWritten(tmpSummary, summaryBaseline, summaryChanged, { statSync })) { + wlog("pi --print exited 0 but never wrote the summary; skipping upload to avoid advancing the offset"); + return; + } if (raw.trim()) { // Stamp the offset ourselves so the persisted summary is authoritative // and never depends on the LLM echoing the bookkeeping line. diff --git a/src/hooks/wiki-offset.ts b/src/hooks/wiki-offset.ts index 633c7b5da..b5a6ea7b5 100644 --- a/src/hooks/wiki-offset.ts +++ b/src/hooks/wiki-offset.ts @@ -126,3 +126,60 @@ function truncateUtf8(s: string, maxBytes: number): string { const decoder = new TextDecoder("utf-8", { fatal: false }); return decoder.decode(buf.subarray(0, maxBytes)).replace(/�+$/, ""); } + +/** + * Did THIS run's agent actually write the summary? + * + * Exit 0 is not proof of work: a child that cannot reach the scratch dir (an + * enterprise policy disabling bypassPermissions, say) exits 0 having written + * nothing, leaving the pre-seeded summary in place. Uploading that unchanged + * placeholder still advances the offset and slices the unread events away + * forever, so the worker must be able to tell "wrote nothing" from "wrote". + * + * Content equality alone cannot: an agent that legitimately regenerates the + * same text would read as a no-op and freeze the offset. So the baseline + * stamps the file a minute into the past and the check compares timestamps — + * any real write lands far outside any plausible filesystem granularity. + * + * When the filesystem will not cooperate (utimes throwing or silently doing + * nothing) the timestamp proves nothing, and the check falls back to content. + * That errs toward skipping: re-summarizing the same rows next run wastes work, + * whereas a wrong upload destroys events. Never the other way round. + */ +export interface SummaryBaseline { + mtimeMs: number; + /** False when utimes threw or left the timestamp unchanged. */ + trusted: boolean; +} + +const SUMMARY_BACKDATE_MS = 60_000; + +export function markSummaryUnwritten( + path: string, + fs: Pick, +): SummaryBaseline { + if (!fs.existsSync(path)) return { mtimeMs: 0, trusted: true }; + const sentinel = new Date(Date.now() - SUMMARY_BACKDATE_MS); + try { + fs.utimesSync(path, sentinel, sentinel); + const mtimeMs = fs.statSync(path).mtimeMs; + // A filesystem that ignores utimes reports something far from the sentinel. + return { mtimeMs, trusted: Math.abs(mtimeMs - sentinel.getTime()) < 2_000 }; + } catch { + return { mtimeMs: 0, trusted: false }; + } +} + +export function summaryWasWritten( + path: string, + baseline: SummaryBaseline, + contentChanged: boolean, + fs: Pick, +): boolean { + if (!baseline.trusted) return contentChanged; + try { + return fs.statSync(path).mtimeMs !== baseline.mtimeMs; + } catch { + return contentChanged; + } +} diff --git a/src/hooks/wiki-worker-spawn.ts b/src/hooks/wiki-worker-spawn.ts index 79f374cea..066f70f4d 100644 --- a/src/hooks/wiki-worker-spawn.ts +++ b/src/hooks/wiki-worker-spawn.ts @@ -2,13 +2,10 @@ import type { ExecFileSyncOptions } from "node:child_process"; import { binNeedsShell, shellFile } from "../utils/resolve-cli-bin.js"; /** Fixed flags for the summary-generation `claude -p` call (no user input). */ -const CLAUDE_FLAGS = [ - "--no-session-persistence", - "--model", - "haiku", - "--permission-mode", - "bypassPermissions", -] as const; +const CLAUDE_BASE_FLAGS = ["--no-session-persistence", "--model", "haiku"] as const; + +/** Blanket grant, used only when the caller names no explicit grants. */ +const CLAUDE_BYPASS_FLAGS = ["--permission-mode", "bypassPermissions"] as const; export interface ClaudeInvocation { file: string; @@ -16,6 +13,43 @@ export interface ClaudeInvocation { options: ExecFileSyncOptions; } +/** + * Explicit grants for call sites that read/write files OUTSIDE the session cwd. + * + * `bypassPermissions` is NOT honored under an enterprise policy that sets + * `"disableBypassPermissionsMode": "disable"` (macOS: + * /Library/Application Support/ClaudeCode/managed-settings.json). The child then + * falls back to normal permissioning, refuses every path outside the working + * directory — the wiki worker's `$TMPDIR/deeplake-wiki-*` scratch dir, or a + * backfill's transcript + staging dir — and exits 0 having written nothing. On + * such a machine every summary stays a header-only stub and every backfill + * reports `no-summary`. + * + * Naming the dirs and tools instead is both policy-proof and least-privilege + * (the summarizer only ever needs Read + Write), so a caller that supplies + * grants gets them INSTEAD of the bypass, not in addition to it. + */ +export interface ClaudeGrants { + /** Directories to expose to the child (each becomes `--add-dir

`). */ + addDirs?: string[]; + /** Tools to pre-approve, e.g. `["Read", "Write"]`. */ + allowedTools?: string[]; +} + +/** + * `quotePaths` is for the Windows `.cmd` branch, where args are re-joined into a + * shell command line: a temp dir there routinely contains spaces + * (`C:\Users\First Last\AppData\Local\Temp`) and would otherwise split. + */ +export function permissionFlags(grants: ClaudeGrants | undefined, quotePaths = false): string[] { + if (!grants) return [...CLAUDE_BYPASS_FLAGS]; + const flags: string[] = []; + for (const dir of grants.addDirs ?? []) flags.push("--add-dir", quotePaths ? `"${dir}"` : dir); + if (grants.allowedTools?.length) flags.push("--allowedTools", ...grants.allowedTools); + // An empty grants object would otherwise leave the child with no grant at all. + return flags.length > 0 ? flags : [...CLAUDE_BYPASS_FLAGS]; +} + /** * Build the `execFileSync` descriptor for the summary-generation claude call. * @@ -30,11 +64,15 @@ export interface ClaudeInvocation { * prompt as a positional arg, no shell — so the already-working path stays * byte-identical. */ -export function buildClaudeInvocation(claudeBin: string, prompt: string): ClaudeInvocation { +export function buildClaudeInvocation( + claudeBin: string, + prompt: string, + grants?: ClaudeGrants, +): ClaudeInvocation { if (binNeedsShell(claudeBin)) { return { file: shellFile(claudeBin), - args: ["-p", ...CLAUDE_FLAGS], + args: ["-p", ...CLAUDE_BASE_FLAGS, ...permissionFlags(grants, true)], // windowsHide: the wiki worker is a detached, console-less process, so // without CREATE_NO_WINDOW Windows allocates a visible console window // (titled after the CLI exe) for the child. No-op on POSIX. @@ -43,7 +81,7 @@ export function buildClaudeInvocation(claudeBin: string, prompt: string): Claude } return { file: claudeBin, - args: ["-p", prompt, ...CLAUDE_FLAGS], + args: ["-p", prompt, ...CLAUDE_BASE_FLAGS, ...permissionFlags(grants)], options: { stdio: ["ignore", "pipe", "pipe"], windowsHide: true }, }; } @@ -102,6 +140,14 @@ export function buildStdinPromptInvocation(bin: string, flags: string[], prompt: } /** Claude variant of {@link buildStdinPromptInvocation} (same fixed flags as the argv path). */ -export function buildClaudeStdinInvocation(claudeBin: string, prompt: string): ClaudeInvocation { - return buildStdinPromptInvocation(claudeBin, ["-p", ...CLAUDE_FLAGS], prompt); +export function buildClaudeStdinInvocation( + claudeBin: string, + prompt: string, + grants?: ClaudeGrants, +): ClaudeInvocation { + return buildStdinPromptInvocation( + claudeBin, + ["-p", ...CLAUDE_BASE_FLAGS, ...permissionFlags(grants)], + prompt, + ); } diff --git a/src/hooks/wiki-worker.ts b/src/hooks/wiki-worker.ts index 637b85520..525a3c08d 100644 --- a/src/hooks/wiki-worker.ts +++ b/src/hooks/wiki-worker.ts @@ -7,7 +7,7 @@ * Invoked by session-end.ts as: node wiki-worker.js */ -import { readFileSync, writeFileSync, existsSync, appendFileSync, mkdirSync, rmSync } from "node:fs"; +import { readFileSync, writeFileSync, existsSync, statSync, utimesSync, appendFileSync, mkdirSync, rmSync } from "node:fs"; import { execFileSync } from "node:child_process"; import { buildClaudeInvocation } from "./wiki-worker-spawn.js"; import { dirname, join } from "node:path"; @@ -19,7 +19,7 @@ const dlog = (msg: string) => _log("wiki-worker", msg); import { finalizeSummary, releaseLock, readState } from "./summary-state.js"; import { readSessionEventCache } from "./session-event-cache.js"; import { buildSessionPath } from "../utils/session-path.js"; -import { capLinesByBytes, newRowsFromWindow, stampOffset, WIKI_FALLBACK_MAX_ROWS, WIKI_JSONL_MAX_BYTES } from "./wiki-offset.js"; +import { capLinesByBytes, markSummaryUnwritten, newRowsFromWindow, stampOffset, summaryWasWritten, WIKI_FALLBACK_MAX_ROWS, WIKI_JSONL_MAX_BYTES } from "./wiki-offset.js"; import { redactSecrets } from "./shared/redact.js"; import { uploadSummary } from "./upload-summary.js"; import { embedSummaryWithWarmup } from "../embeddings/embed-summary.js"; @@ -324,8 +324,16 @@ async function main(): Promise { wlog("running claude -p"); let execSucceeded = false; const summaryBeforeExec = existsSync(tmpSummary) ? readFileSync(tmpSummary, "utf-8") : null; + const summaryBaseline = markSummaryUnwritten(tmpSummary, { existsSync, utimesSync, statSync }); try { - const inv = buildClaudeInvocation(cfg.claudeBin, prompt); + // tmpDir holds both the session JSONL to read and the summary to write, + // and lives outside the session cwd — grant it explicitly rather than + // relying on bypassPermissions, which an enterprise policy can disable + // (see ClaudeGrants in wiki-worker-spawn.ts). + const inv = buildClaudeInvocation(cfg.claudeBin, prompt, { + addDirs: [tmpDir], + allowedTools: ["Read", "Write"], + }); execFileSync(inv.file, inv.args, { ...inv.options, timeout: 120_000, @@ -355,6 +363,15 @@ async function main(): Promise { : "claude -p failed without producing a new summary; skipping upload"); return; } + // Exit 0 is not proof of work: a child that cannot reach tmpDir (or simply + // declines) exits 0 having written nothing, leaving the pre-seeded base + // summary in place. Uploading it unchanged would still advance the offset + // and slice those events away forever, which is how a session gets stuck + // as a header-only placeholder run after run. + if (!summaryWasWritten(tmpSummary, summaryBaseline, summaryChanged, { statSync })) { + wlog("claude -p exited 0 but never wrote the summary; skipping upload to avoid advancing the offset"); + return; + } if (raw.trim()) { // Stamp the offset ourselves so the persisted summary is authoritative // and never depends on the LLM echoing the bookkeeping line. diff --git a/src/skillify/gate-runner.ts b/src/skillify/gate-runner.ts index 8c60726ca..fa7704da5 100644 --- a/src/skillify/gate-runner.ts +++ b/src/skillify/gate-runner.ts @@ -6,7 +6,7 @@ * codex / cursor / hermes installed never needs `claude` in PATH. * * Per-agent invocation: - * claude_code → `claude -p --no-session-persistence --model haiku --permission-mode bypassPermissions` + * claude_code → `claude -p --no-session-persistence --model haiku ` * codex → `codex exec --dangerously-bypass-approvals-and-sandbox ` * cursor → `cursor-agent --print --model --force --output-format text ` * hermes → `hermes -z --provider -m --yolo --ignore-user-config` @@ -14,9 +14,15 @@ * The worker passes a verdict-write path inside the prompt; the runner * captures stdout regardless so the worker's stdout-fallback path still * works on agents whose models don't reliably use the Write tool. + * + * That verdict path lives outside the session cwd, so the claude_code caller + * names it via `grants` (`--add-dir` + `--allowedTools`). Callers that name no + * grants keep the blanket `--permission-mode bypassPermissions`. */ import { existsSync } from "node:fs"; + +import { permissionFlags, type ClaudeGrants } from "../hooks/wiki-worker-spawn.js"; import { createRequire } from "node:module"; // We need `child_process.execFileSync` to actually spawn the agent CLI for @@ -61,6 +67,14 @@ export interface GateRunOptions { piModel?: string; /** Max wall-clock for the CLI call; default 120s. */ timeoutMs?: number; + /** + * claude_code only — dirs/tools to grant explicitly instead of the blanket + * `bypassPermissions`, which an enterprise policy can disable. The gate + * prompt names a verdict path outside the session cwd, so a caller that + * wants the Write-tool branch to work under such a policy must grant that + * dir. See ClaudeGrants in ../hooks/wiki-worker-spawn.ts. + */ + grants?: ClaudeGrants; } export interface GateRunResult { @@ -153,7 +167,7 @@ export function buildArgs(agent: Agent, prompt: string, opts: GateRunOptions): s "-p", prompt, "--no-session-persistence", "--model", "haiku", - "--permission-mode", "bypassPermissions", + ...permissionFlags(opts.grants), ]; case "codex": return [ diff --git a/src/skillify/skillify-worker.ts b/src/skillify/skillify-worker.ts index 65363d699..28d98e6b9 100644 --- a/src/skillify/skillify-worker.ts +++ b/src/skillify/skillify-worker.ts @@ -399,6 +399,10 @@ async function main(): Promise { agent: gateAgent, prompt, bin: cfg.gateBin, + // The prompt offers the model a verdict path inside tmpDir, which lives + // outside the session cwd. Grant it explicitly so the Write-tool branch + // survives an enterprise policy that disables bypassPermissions. + grants: { addDirs: [tmpDir], allowedTools: ["Read", "Write"] }, cursorModel: cfg.cursorModel, hermesProvider: cfg.hermesProvider, hermesModel: cfg.hermesModel, diff --git a/src/skillify/stage-memory.ts b/src/skillify/stage-memory.ts index 5b3013dee..81f8221de 100644 --- a/src/skillify/stage-memory.ts +++ b/src/skillify/stage-memory.ts @@ -18,10 +18,10 @@ import { spawn } from "node:child_process"; import { existsSync, mkdirSync, readFileSync, writeFileSync, unlinkSync } from "node:fs"; -import { join } from "node:path"; +import { dirname, join } from "node:path"; import { WIKI_PROMPT_TEMPLATE } from "../hooks/spawn-wiki-worker.js"; -import { buildClaudeInvocation } from "../hooks/wiki-worker-spawn.js"; +import { buildClaudeInvocation, type ClaudeGrants } from "../hooks/wiki-worker-spawn.js"; import { resolveCliBin } from "../utils/resolve-cli-bin.js"; import { EmbedClient } from "../embeddings/client.js"; import { embeddingsDisabled } from "../embeddings/disable.js"; @@ -62,7 +62,12 @@ export interface StageOptions { * Injectable for tests; defaults to the real `claude -p` spawn. A real * agent writes the summary file named in the prompt as a side effect. */ - runAgent?: (claudeBin: string, prompt: string, timeoutMs: number) => Promise; + runAgent?: ( + claudeBin: string, + prompt: string, + timeoutMs: number, + grants?: ClaudeGrants, + ) => Promise; /** Embed `text` locally; null when unavailable. Injectable for tests. */ embed?: (text: string) => Promise; } @@ -124,12 +129,17 @@ export function planClaudeSpawn(inv: ReturnType): }; } -function runClaude(claudeBin: string, prompt: string, timeoutMs: number): Promise { +function runClaude( + claudeBin: string, + prompt: string, + timeoutMs: number, + grants?: ClaudeGrants, +): Promise { // Reuse the live wiki-worker's invocation builder so the prompt-as-arg vs // prompt-over-stdin (Windows `.cmd` shim) handling stays identical to the // proven SessionEnd path. A bare `spawn(bin, ["-p", prompt, ...])` cannot // launch a `.cmd` shim and would blow the command-line length on Windows. - const plan = planClaudeSpawn(buildClaudeInvocation(claudeBin, prompt)); + const plan = planClaudeSpawn(buildClaudeInvocation(claudeBin, prompt, grants)); return new Promise((resolve) => { const child = spawn(plan.file, plan.args, { stdio: plan.stdio, @@ -213,7 +223,13 @@ export async function stageSession(input: StageSessionInput, opts: StageOptions) } const runAgent = opts.runAgent ?? runClaude; - const ran = await runAgent(opts.claudeBin, prompt, opts.timeoutMs); + // The transcript and the staging dir both live outside the session cwd, so the + // agent needs them granted explicitly — bypassPermissions alone is silently + // ignored under an enterprise policy and every session stages as `no-summary`. + const ran = await runAgent(opts.claudeBin, prompt, opts.timeoutMs, { + addDirs: [dirname(input.jsonlPath), stagingDir], + allowedTools: ["Read", "Write"], + }); if (!existsSync(summaryPath)) { return { sessionId: key, ok: false, embedded: false, reason: ran ? "no-summary" : "claude-failed" }; } diff --git a/tests/claude-code/mine-local-orchestrator.test.ts b/tests/claude-code/mine-local-orchestrator.test.ts index e3795769f..af56dc4b3 100644 --- a/tests/claude-code/mine-local-orchestrator.test.ts +++ b/tests/claude-code/mine-local-orchestrator.test.ts @@ -239,6 +239,18 @@ describe("runMineLocal: orchestrator branches", () => { await mod.runMineLocal([]); expect(spawnCalls).toHaveLength(1); + // The gate prompt offers the model a verdict path inside the per-session + // tmp dir, which is outside the session cwd. That dir must be granted by + // name: `bypassPermissions` is silently ignored under an enterprise policy + // (disableBypassPermissionsMode), and the child then refuses the path. + expect(spawnCalls[0].args).not.toContain("--permission-mode"); + expect(spawnCalls[0].args).not.toContain("bypassPermissions"); + // Pin the granted VALUES: the dir must be THIS session's tmp dir (the one + // holding the verdict path named in the prompt), not merely some dir. + const addDirAt = spawnCalls[0].args.indexOf("--add-dir"); + expect(addDirAt).toBeGreaterThan(-1); + expect(spawnCalls[0].args[addDirAt + 1]).toMatch(/[/\\]mine-local-\d+[/\\]s-sess-aaa$/); + expect(spawnCalls[0].args.slice(addDirAt + 2)).toEqual(["--allowedTools", "Read", "Write"]); expect(writeNewSkill).toHaveBeenCalledTimes(1); expect(writeNewSkill.mock.calls[0][0].name).toBe("useful-skill"); expect(fanOutSymlinks).toHaveBeenCalledTimes(1); diff --git a/tests/claude-code/skillify-gate-runner.test.ts b/tests/claude-code/skillify-gate-runner.test.ts index e0b7984f9..aacff5001 100644 --- a/tests/claude-code/skillify-gate-runner.test.ts +++ b/tests/claude-code/skillify-gate-runner.test.ts @@ -74,7 +74,7 @@ describe("runGate dispatch", () => { // produces. (Previously these spawned `/usr/bin/echo` and grepped stdout, // which only works on POSIX; buildArgs is the deterministic, cross-platform // seam for the same contract.) - it("constructs claude_code invocation with --model haiku + bypassPermissions", () => { + it("constructs claude_code invocation with --model haiku + bypassPermissions when no grants are named", () => { const args = buildArgs("claude_code", "PROMPT_MARKER", { agent: "claude_code", prompt: "PROMPT_MARKER" }); expect(args).toContain("PROMPT_MARKER"); expect(args).toContain("--model"); @@ -82,6 +82,36 @@ describe("runGate dispatch", () => { expect(args).toContain("bypassPermissions"); }); + it("swaps bypassPermissions for the named grants when the caller supplies them", () => { + // The worker names the tmp dir holding the verdict path. bypassPermissions + // is silently ignored under an enterprise policy that sets + // disableBypassPermissionsMode, so it must not be the only grant. + const args = buildArgs("claude_code", "PROMPT_MARKER", { + agent: "claude_code", + prompt: "PROMPT_MARKER", + grants: { addDirs: ["/tmp/skillify-x"], allowedTools: ["Read", "Write"] }, + }); + expect(args).not.toContain("--permission-mode"); + expect(args).not.toContain("bypassPermissions"); + expect(args).toEqual([ + "-p", "PROMPT_MARKER", + "--no-session-persistence", + "--model", "haiku", + "--add-dir", "/tmp/skillify-x", + "--allowedTools", "Read", "Write", + ]); + }); + + it("ignores grants for non-claude agents (their CLIs have no --add-dir)", () => { + const args = buildArgs("codex", "PROMPT_MARKER", { + agent: "codex", + prompt: "PROMPT_MARKER", + grants: { addDirs: ["/tmp/skillify-x"], allowedTools: ["Read", "Write"] }, + }); + expect(args).not.toContain("--add-dir"); + expect(args).toContain("--dangerously-bypass-approvals-and-sandbox"); + }); + it("constructs codex invocation with exec + --dangerously-bypass-approvals-and-sandbox", () => { const args = buildArgs("codex", "PROMPT_MARKER", { agent: "codex", prompt: "PROMPT_MARKER" }); expect(args).toContain("PROMPT_MARKER"); diff --git a/tests/claude-code/stage-memory.test.ts b/tests/claude-code/stage-memory.test.ts index c2f86fa4c..c07095755 100644 --- a/tests/claude-code/stage-memory.test.ts +++ b/tests/claude-code/stage-memory.test.ts @@ -8,7 +8,7 @@ import { describe, it, expect, beforeEach, afterEach } from "vitest"; import { mkdtempSync, writeFileSync, readFileSync, existsSync, rmSync, chmodSync, mkdirSync } from "node:fs"; import { tmpdir } from "node:os"; -import { join } from "node:path"; +import { dirname, join } from "node:path"; import { stageSession, resolveClaudeBin, planClaudeSpawn, type StageOptions } from "../../src/skillify/stage-memory.js"; import { readPendingMemoryManifest } from "../../src/skillify/pending-memory-manifest.js"; @@ -48,6 +48,27 @@ function opts(over: Partial = {}): StageOptions { } describe("stageSession", () => { + it("grants the transcript dir and the staging dir explicitly, with Read+Write", async () => { + // The backfill hands the agent two paths outside the session cwd. Without + // an explicit grant those are refused under an enterprise policy that + // disables bypassPermissions, and every session stages as `no-summary`. + // The other tests inject a runAgent that ignores its grants argument, so + // dropping the grants entirely would leave them green — this one pins it. + let seen: any; + await stageSession(input(), opts({ + runAgent: async (_bin, prompt, _timeout, grants) => { + seen = grants; + const m = prompt.match(/SUMMARY FILE to write: (\S+)/); + if (m) writeFileSync(m[1], "# Session s1\n## What Happened\nreal content\n"); + return true; + }, + })); + expect(seen).toBeDefined(); + expect(seen.allowedTools).toEqual(["Read", "Write"]); + expect(seen.addDirs).toContain(dirname(jsonlPath)); + expect(seen.addDirs).toContain(stagingDir); + }); + it("stages summary + manifest row on success", async () => { const r = await stageSession(input(), opts()); expect(r).toMatchObject({ ok: true, embedded: false }); diff --git a/tests/claude-code/wiki-worker.test.ts b/tests/claude-code/wiki-worker.test.ts index 14648adb0..2197620ad 100644 --- a/tests/claude-code/wiki-worker.test.ts +++ b/tests/claude-code/wiki-worker.test.ts @@ -290,8 +290,17 @@ describe("wiki-worker — happy path", () => { expect(calledArgs).toContain("--no-session-persistence"); expect(calledArgs).toContain("--model"); expect(calledArgs).toContain("haiku"); - expect(calledArgs).toContain("--permission-mode"); - expect(calledArgs).toContain("bypassPermissions"); + // The scratch dir is granted explicitly instead of relying on + // bypassPermissions, which an enterprise policy can disable. + expect(calledArgs).not.toContain("--permission-mode"); + expect(calledArgs).not.toContain("bypassPermissions"); + // Pin the granted VALUES, not just the flag names: a grant naming the + // wrong directory reads identically to a correct one under a presence-only + // assertion, and would leave the child just as unable to reach tmpDir. + expect(calledArgs.slice(calledArgs.indexOf("--add-dir"))).toEqual([ + "--add-dir", tmpDir, + "--allowedTools", "Read", "Write", + ]); // Prompt template was expanded with real values const prompt = calledArgs[1]; @@ -378,6 +387,68 @@ describe("wiki-worker — happy path", () => { expect(releaseLockMock).toHaveBeenCalledWith("sid-worker"); }); + it("does NOT upload or advance the offset when claude -p exits 0 having written nothing", async () => { + // The field failure, reproduced at the seam that made it permanent. Under an enterprise policy that disables bypassPermissions the + // child cannot reach tmpDir, cannot prompt (print mode), and exits 0 with + // the summary file untouched — leaving the pre-seeded prior summary on disk. + // Treating that as success re-uploaded the placeholder verbatim AND stamped + // lastSummaryCount, slicing the unread events away forever, so every later + // run summarized nothing. Exit 0 is not proof of work. + mkFetch(undefined, 1, true, 14); + // Captured inside the mock: the worker rmSync's tmpDir on the way out, so + // the pre-seed can only be observed while the child is "running". + let preSeeded: string | null = null; + execFileSyncMock.mockImplementation((_bin: string, args: string[]) => { + const summaryPath = args[1].match(/SUMMARY=(\S+)/)![1]; + preSeeded = existsSync(summaryPath) ? readFileSync(summaryPath, "utf-8") : null; + return Buffer.from(""); // exit 0, wrote nothing + }); + await runWorker(); + + // The worker did pre-seed the prior summary (otherwise this test would + // pass for the wrong reason — an absent file, not an unchanged one). + expect(preSeeded).toContain("JSONL offset"); + expect(uploadSummaryMock).not.toHaveBeenCalled(); + expect(finalizeSummaryMock).not.toHaveBeenCalled(); + const log = readFileSync(join(hooksDir, "wiki.log"), "utf-8"); + expect(log).toContain("claude -p exited 0 but never wrote the summary; skipping upload to avoid advancing the offset"); + expect(releaseLockMock).toHaveBeenCalledWith("sid-worker"); + }); + + it("still uploads when the agent legitimately rewrites the summary to the SAME bytes", async () => { + // Content equality alone cannot tell "wrote nothing" from "correctly + // regenerated identical text". Treating the second as the first would + // freeze the offset and re-summarize those rows on every future run, so + // the guard also checks whether the file was touched. + mkFetch(undefined, 1, true, 14); + execFileSyncMock.mockImplementation((_bin: string, args: string[]) => { + const summaryPath = args[1].match(/SUMMARY=(\S+)/)![1]; + // A plain rewrite of the same bytes, with NO mtime manipulation — the + // worst case for a content-only check, and the case a coarse filesystem + // clock would hide if the guard compared against "just now". + writeFileSync(summaryPath, readFileSync(summaryPath, "utf-8")); + return Buffer.from(""); + }); + await runWorker(); + expect(uploadSummaryMock).toHaveBeenCalledTimes(1); + expect(finalizeSummaryMock).toHaveBeenCalledWith("sid-worker", 14); + }); + + it("still uploads and advances the offset when claude -p rewrites the pre-seeded summary", async () => { + // The mirror of the guard above: a real run that DOES rewrite the file must + // keep uploading and stamping, so the fix cannot silently freeze summaries. + mkFetch(undefined, 1, true, 14); + execFileSyncMock.mockImplementation((_bin: string, args: string[]) => { + const summaryPath = args[1].match(/SUMMARY=(\S+)/)![1]; + writeFileSync(summaryPath, "# Session X\n\n## What Happened\nfresh body written by this run.\n"); + return Buffer.from(""); + }); + await runWorker(); + + expect(uploadSummaryMock).toHaveBeenCalledTimes(1); + expect(finalizeSummaryMock).toHaveBeenCalledWith("sid-worker", 14); + }); + it("defaults to /sessions/unknown/ when the path SELECT returns no rows", async () => { mkFetch(undefined, 0); execFileSyncMock.mockImplementation((_bin: string, args: string[]) => { diff --git a/tests/codex/codex-wiki-worker.test.ts b/tests/codex/codex-wiki-worker.test.ts index 13ce5b4a6..6056da647 100644 --- a/tests/codex/codex-wiki-worker.test.ts +++ b/tests/codex/codex-wiki-worker.test.ts @@ -270,6 +270,22 @@ describe("codex wiki-worker — happy path", () => { expect(releaseLockMock).toHaveBeenCalledWith("sid-codex"); }); + it("does NOT upload or advance the offset when codex exec exits 0 having written nothing", async () => { + // Exit 0 is not proof of work. A child that cannot reach tmpDir (or simply + // declines) exits 0 with the summary file untouched, leaving the pre-seeded + // prior summary in place. Treating that as success re-uploaded the + // placeholder verbatim AND stamped lastSummaryCount, slicing the unread + // events away forever — so every later run summarized nothing. + mkFetch(1, true, 9); + execFileSyncMock.mockImplementation(() => Buffer.from("")); + await runWorker(); + expect(uploadSummaryMock).not.toHaveBeenCalled(); + expect(finalizeSummaryMock).not.toHaveBeenCalled(); + const log = readFileSync(join(hooksDir, "wiki.log"), "utf-8"); + expect(log).toContain("codex exec exited 0 but never wrote the summary; skipping upload to avoid advancing the offset"); + expect(releaseLockMock).toHaveBeenCalledWith("sid-codex"); + }); + it("ignores a stale sidecar offset when no existing summary was loaded", async () => { // Sidecar says 3 processed, but SELECT summary returns nothing (row gone). // Without a base summary the offset is meaningless — regenerate from scratch diff --git a/tests/cursor/cursor-wiki-worker.test.ts b/tests/cursor/cursor-wiki-worker.test.ts index 11eea2eae..94acf9efa 100644 --- a/tests/cursor/cursor-wiki-worker.test.ts +++ b/tests/cursor/cursor-wiki-worker.test.ts @@ -174,6 +174,38 @@ describe("cursor wiki-worker — behavior", () => { expect(releaseLockMock).toHaveBeenCalledWith("sid-cursor"); }); + it("does NOT upload or advance the offset when the agent exits 0 having written nothing", async () => { + // Exit 0 is not proof of work. A child that cannot reach tmpDir (or simply + // declines) exits 0 with the summary file untouched, leaving the pre-seeded + // prior summary in place. Treating that as success re-uploaded the + // placeholder verbatim AND stamped lastSummaryCount, slicing the unread + // events away forever — so every later run summarized nothing. + fetchMock.mockImplementation(async (_u: string, init: any) => { + const sql = JSON.parse(init.body).query as string; + if (sql.startsWith("SELECT count(*) AS n")) return jsonResp({ columns: ["n"], rows: [[9]] }); + if (sql.startsWith("SELECT message, creation_date")) { + return jsonResp({ + columns: ["message", "creation_date"], + rows: Array.from({ length: 9 }, (_, i) => [JSON.stringify({ type: "user_message", content: `hello cursor ${i}` }), "2026-04-20T00:00:00Z"]), + }); + } + if (sql.startsWith("SELECT DISTINCT path")) { + return jsonResp({ columns: ["path"], rows: [["/sessions/alice/alice_org_default_sid-cursor.jsonl"]] }); + } + if (sql.startsWith("SELECT summary FROM")) { + return jsonResp({ columns: ["summary"], rows: [["# Session X\n- **JSONL offset**: 7\n\n## What Happened\nprior"]] }); + } + throw new Error(`unexpected query: ${sql}`); + }); + execFileSyncMock.mockImplementation(() => Buffer.from("")); + await runWorker(); + expect(uploadSummaryMock).not.toHaveBeenCalled(); + expect(finalizeSummaryMock).not.toHaveBeenCalled(); + const log = readFileSync(join(hooksDir, "wiki.log"), "utf-8"); + expect(log).toContain("cursor-agent --print exited 0 but never wrote the summary; skipping upload to avoid advancing the offset"); + expect(releaseLockMock).toHaveBeenCalledWith("sid-cursor"); + }); + it("reads events from the local cache and issues NO self-session SELECTs", async () => { readCacheMock.mockReturnValue( Array.from({ length: 4 }, (_, i) => JSON.stringify({ type: "user_message", content: `cache ${i}` })), diff --git a/tests/hermes/hermes-wiki-worker.test.ts b/tests/hermes/hermes-wiki-worker.test.ts index c0bbf729a..18f77f479 100644 --- a/tests/hermes/hermes-wiki-worker.test.ts +++ b/tests/hermes/hermes-wiki-worker.test.ts @@ -180,6 +180,38 @@ describe("hermes wiki-worker — behavior", () => { expect(releaseLockMock).toHaveBeenCalledWith("sid-hermes"); }); + it("does NOT upload or advance the offset when the agent exits 0 having written nothing", async () => { + // Exit 0 is not proof of work. A child that cannot reach tmpDir (or simply + // declines) exits 0 with the summary file untouched, leaving the pre-seeded + // prior summary in place. Treating that as success re-uploaded the + // placeholder verbatim AND stamped lastSummaryCount, slicing the unread + // events away forever — so every later run summarized nothing. + fetchMock.mockImplementation(async (_u: string, init: any) => { + const sql = JSON.parse(init.body).query as string; + if (sql.startsWith("SELECT count(*) AS n")) return jsonResp({ columns: ["n"], rows: [[9]] }); + if (sql.startsWith("SELECT message, creation_date")) { + return jsonResp({ + columns: ["message", "creation_date"], + rows: Array.from({ length: 9 }, (_, i) => [JSON.stringify({ type: "user_message", content: `hello hermes ${i}` }), "2026-04-20T00:00:00Z"]), + }); + } + if (sql.startsWith("SELECT DISTINCT path")) { + return jsonResp({ columns: ["path"], rows: [["/sessions/alice/alice_org_default_sid-hermes.jsonl"]] }); + } + if (sql.startsWith("SELECT summary FROM")) { + return jsonResp({ columns: ["summary"], rows: [["# Session X\n- **JSONL offset**: 7\n\n## What Happened\nprior"]] }); + } + throw new Error(`unexpected query: ${sql}`); + }); + execFileSyncMock.mockImplementation(() => Buffer.from("")); + await runWorker(); + expect(uploadSummaryMock).not.toHaveBeenCalled(); + expect(finalizeSummaryMock).not.toHaveBeenCalled(); + const log = readFileSync(join(hooksDir, "wiki.log"), "utf-8"); + expect(log).toContain("hermes -z exited 0 but never wrote the summary; skipping upload to avoid advancing the offset"); + expect(releaseLockMock).toHaveBeenCalledWith("sid-hermes"); + }); + it("reads events from the local cache and issues NO self-session SELECTs", async () => { readCacheMock.mockReturnValue( Array.from({ length: 4 }, (_, i) => JSON.stringify({ type: "user_message", content: `cache ${i}` })), diff --git a/tests/pi/pi-wiki-worker.test.ts b/tests/pi/pi-wiki-worker.test.ts index 8aaf85b73..634368238 100644 --- a/tests/pi/pi-wiki-worker.test.ts +++ b/tests/pi/pi-wiki-worker.test.ts @@ -169,6 +169,38 @@ describe("pi wiki-worker — behavior", () => { expect(uploadSummaryMock).not.toHaveBeenCalled(); expect(releaseLockMock).toHaveBeenCalledWith("sid-pi"); }); + + it("does NOT upload or advance the offset when the agent exits 0 having written nothing", async () => { + // Exit 0 is not proof of work. A child that cannot reach tmpDir (or simply + // declines) exits 0 with the summary file untouched, leaving the pre-seeded + // prior summary in place. Treating that as success re-uploaded the + // placeholder verbatim AND stamped lastSummaryCount, slicing the unread + // events away forever — so every later run summarized nothing. + fetchMock.mockImplementation(async (_u: string, init: any) => { + const sql = JSON.parse(init.body).query as string; + if (sql.startsWith("SELECT count(*) AS n")) return jsonResp({ columns: ["n"], rows: [[9]] }); + if (sql.startsWith("SELECT message, creation_date")) { + return jsonResp({ + columns: ["message", "creation_date"], + rows: Array.from({ length: 9 }, (_, i) => [JSON.stringify({ type: "user_message", content: `hello pi ${i}` }), "2026-04-20T00:00:00Z"]), + }); + } + if (sql.startsWith("SELECT DISTINCT path")) { + return jsonResp({ columns: ["path"], rows: [["/sessions/alice/alice_org_default_sid-pi.jsonl"]] }); + } + if (sql.startsWith("SELECT summary FROM")) { + return jsonResp({ columns: ["summary"], rows: [["# Session X\n- **JSONL offset**: 7\n\n## What Happened\nprior"]] }); + } + throw new Error(`unexpected query: ${sql}`); + }); + execFileSyncMock.mockImplementation(() => Buffer.from("")); + await runWorker(); + expect(uploadSummaryMock).not.toHaveBeenCalled(); + expect(finalizeSummaryMock).not.toHaveBeenCalled(); + const log = readFileSync(join(hooksDir, "wiki.log"), "utf-8"); + expect(log).toContain("pi --print exited 0 but never wrote the summary; skipping upload to avoid advancing the offset"); + expect(releaseLockMock).toHaveBeenCalledWith("sid-pi"); + }); }); const promptOf = (a: string[]) => a.find((x) => typeof x === "string" && x.includes("SUMMARY="))!; diff --git a/tests/shared/claude-permission-grants.test.ts b/tests/shared/claude-permission-grants.test.ts new file mode 100644 index 000000000..42515d6a2 --- /dev/null +++ b/tests/shared/claude-permission-grants.test.ts @@ -0,0 +1,145 @@ +import { afterEach, describe, expect, it } from "vitest"; + +/** + * Contract for the explicit permission grants that replaced the blanket + * `--permission-mode bypassPermissions` on the summarizer's `claude -p` calls. + * + * Why the swap: an enterprise policy can set + * `"disableBypassPermissionsMode": "disable"`, after which the child ignores + * the bypass, falls back to normal permissioning, refuses every path outside + * the session cwd — which is exactly where the wiki worker's scratch dir and + * the backfill's transcript/staging dirs live — and, because print mode cannot + * prompt, exits 0 having written nothing. Naming the dirs and tools is both + * policy-proof and least-privilege. + * + * Two halves matter equally: grants must REPLACE the bypass when supplied, and + * the bypass must survive untouched for every caller that supplies none. + */ + +import { + buildClaudeInvocation, + buildClaudeStdinInvocation, + permissionFlags, +} from "../../src/hooks/wiki-worker-spawn.js"; + +const realPlatform = process.platform; +function setPlatform(p: NodeJS.Platform): void { + Object.defineProperty(process, "platform", { value: p, configurable: true }); +} +afterEach(() => { + Object.defineProperty(process, "platform", { value: realPlatform, configurable: true }); +}); + +describe("permissionFlags", () => { + it("falls back to the blanket bypass when no grants are named", () => { + expect(permissionFlags(undefined)).toEqual(["--permission-mode", "bypassPermissions"]); + }); + + it("emits one --add-dir per granted dir, then the allowed tools", () => { + expect(permissionFlags({ addDirs: ["/a", "/b"], allowedTools: ["Read", "Write"] })) + .toEqual(["--add-dir", "/a", "--add-dir", "/b", "--allowedTools", "Read", "Write"]); + }); + + it("accepts dirs without tools, and tools without dirs", () => { + expect(permissionFlags({ addDirs: ["/a"] })).toEqual(["--add-dir", "/a"]); + expect(permissionFlags({ allowedTools: ["Read"] })).toEqual(["--allowedTools", "Read"]); + }); + + it("falls back to the bypass rather than leaving the child with no grant at all", () => { + // An empty grants object is a caller bug; degrading to today's behaviour + // beats silently spawning a child that can reach nothing. + expect(permissionFlags({})).toEqual(["--permission-mode", "bypassPermissions"]); + expect(permissionFlags({ addDirs: [], allowedTools: [] })).toEqual(["--permission-mode", "bypassPermissions"]); + }); + + it("quotes granted paths only when asked (the Windows shell branch)", () => { + const dir = "C:\\Users\\First Last\\AppData\\Local\\Temp\\deeplake-wiki-1"; + expect(permissionFlags({ addDirs: [dir] }, false)).toEqual(["--add-dir", dir]); + expect(permissionFlags({ addDirs: [dir] }, true)).toEqual(["--add-dir", `"${dir}"`]); + }); +}); + +describe("buildClaudeInvocation — grants vs bypass", () => { + it("replaces the bypass with the grants on the POSIX argv path", () => { + setPlatform("linux"); + const inv = buildClaudeInvocation("/usr/local/bin/claude", "PROMPT", { + addDirs: ["/tmp/deeplake-wiki-1"], + allowedTools: ["Read", "Write"], + }); + expect(inv.args).toEqual([ + "-p", "PROMPT", + "--no-session-persistence", + "--model", "haiku", + "--add-dir", "/tmp/deeplake-wiki-1", + "--allowedTools", "Read", "Write", + ]); + expect(inv.args).not.toContain("bypassPermissions"); + }); + + it("leaves the bypass in place for a caller that names no grants", () => { + setPlatform("linux"); + const inv = buildClaudeInvocation("/usr/local/bin/claude", "PROMPT"); + expect(inv.args).toEqual([ + "-p", "PROMPT", + "--no-session-persistence", + "--model", "haiku", + "--permission-mode", "bypassPermissions", + ]); + }); + + it("quotes the granted dir on the Windows .cmd branch, where args are re-joined into a command line", () => { + // A Windows temp dir routinely contains spaces; unquoted it would split + // into two args and the grant would silently name the wrong directory. + setPlatform("win32"); + const dir = "C:\\Users\\First Last\\AppData\\Local\\Temp\\deeplake-wiki-1"; + const inv = buildClaudeInvocation("C:\\npm\\claude.cmd", "PROMPT", { + addDirs: [dir], + allowedTools: ["Read", "Write"], + }); + expect(inv.args).toEqual([ + "-p", + "--no-session-persistence", + "--model", "haiku", + "--add-dir", `"${dir}"`, + "--allowedTools", "Read", "Write", + ]); + // The prompt still rides stdin, never the command line. + expect(inv.args).not.toContain("PROMPT"); + expect(inv.options.input).toBe("PROMPT"); + }); + + it("keeps the bypass on the Windows .cmd branch when no grants are named", () => { + setPlatform("win32"); + const inv = buildClaudeInvocation("C:\\npm\\claude.cmd", "PROMPT"); + expect(inv.args).toEqual([ + "-p", + "--no-session-persistence", + "--model", "haiku", + "--permission-mode", "bypassPermissions", + ]); + }); +}); + +describe("buildClaudeStdinInvocation — grants vs bypass", () => { + it("carries the grants through the stdin variant", () => { + setPlatform("linux"); + const inv = buildClaudeStdinInvocation("/usr/local/bin/claude", "PROMPT", { + addDirs: ["/tmp/staging"], + allowedTools: ["Read", "Write"], + }); + expect(inv.args).toEqual([ + "-p", + "--no-session-persistence", + "--model", "haiku", + "--add-dir", "/tmp/staging", + "--allowedTools", "Read", "Write", + ]); + expect(inv.options.input).toBe("PROMPT"); + }); + + it("keeps the bypass in the stdin variant when no grants are named", () => { + setPlatform("linux"); + const inv = buildClaudeStdinInvocation("/usr/local/bin/claude", "PROMPT"); + expect(inv.args).toContain("bypassPermissions"); + }); +}); diff --git a/tests/shared/wiki-summary-written.test.ts b/tests/shared/wiki-summary-written.test.ts new file mode 100644 index 000000000..a040d5930 --- /dev/null +++ b/tests/shared/wiki-summary-written.test.ts @@ -0,0 +1,98 @@ +import { describe, expect, it } from "vitest"; +import { mkdtempSync, rmSync, writeFileSync, existsSync, utimesSync, statSync } from "node:fs"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; + +import { markSummaryUnwritten, summaryWasWritten } from "../../src/hooks/wiki-offset.js"; + +/** + * "Did the agent actually write?" is the decision that separates a real summary + * from the placeholder-forever bug: answering "yes" when the child wrote nothing + * uploads the pre-seeded stub AND advances the offset, destroying the unread + * events. These pin the answer on every filesystem the workers can land on. + */ + +const realFs = { existsSync, utimesSync, statSync }; + +function withFile(body: (path: string) => void): void { + const dir = mkdtempSync(join(tmpdir(), "summary-written-")); + try { body(join(dir, "summary.md")); } finally { rmSync(dir, { recursive: true, force: true }); } +} + +describe("markSummaryUnwritten / summaryWasWritten", () => { + it("reports NOT written when the agent leaves the pre-seeded file alone", () => { + withFile((p) => { + writeFileSync(p, "# placeholder\n"); + const base = markSummaryUnwritten(p, realFs); + expect(base.trusted).toBe(true); + expect(summaryWasWritten(p, base, false, realFs)).toBe(false); + }); + }); + + it("reports WRITTEN when the agent rewrites the identical bytes", () => { + // The regression the timestamp check exists for: content equality alone + // would call this a no-op and freeze the offset forever. + withFile((p) => { + writeFileSync(p, "# same\n"); + const base = markSummaryUnwritten(p, realFs); + writeFileSync(p, "# same\n"); + expect(summaryWasWritten(p, base, false, realFs)).toBe(true); + }); + }); + + it("reports WRITTEN when the agent writes different content", () => { + withFile((p) => { + writeFileSync(p, "# before\n"); + const base = markSummaryUnwritten(p, realFs); + writeFileSync(p, "# after\n"); + expect(summaryWasWritten(p, base, true, realFs)).toBe(true); + }); + }); + + it("treats a first run with no pre-seeded file as written when content appeared", () => { + withFile((p) => { + const base = markSummaryUnwritten(p, realFs); + expect(base).toEqual({ mtimeMs: 0, trusted: true }); + writeFileSync(p, "# fresh\n"); + expect(summaryWasWritten(p, base, true, realFs)).toBe(true); + }); + }); + + it("does not throw — and distrusts the timestamp — when utimes is rejected", () => { + // A read-only or exotic filesystem must not abort the worker: before this + // was routed through the helper, the utimes call sat outside the try and a + // throw killed the run outright. + withFile((p) => { + writeFileSync(p, "# ro\n"); + const fs = { existsSync, statSync, utimesSync: () => { throw new Error("EPERM"); } }; + const base = markSummaryUnwritten(p, fs); + expect(base.trusted).toBe(false); + // Untrusted timestamps fall back to content, erring toward skipping. + expect(summaryWasWritten(p, base, false, realFs)).toBe(false); + expect(summaryWasWritten(p, base, true, realFs)).toBe(true); + }); + }); + + it("distrusts the timestamp when utimes silently does nothing", () => { + // The coarse-clock hole: a filesystem that accepts utimes and ignores it + // would leave the baseline at "now", where a same-tick identical rewrite + // is indistinguishable from no write at all. + withFile((p) => { + writeFileSync(p, "# noop\n"); + const fs = { existsSync, statSync, utimesSync: () => { /* silently ignored */ } }; + const base = markSummaryUnwritten(p, fs); + expect(base.trusted).toBe(false); + expect(summaryWasWritten(p, base, false, realFs)).toBe(false); + }); + }); + + it("falls back to content when the file vanishes before the check", () => { + withFile((p) => { + writeFileSync(p, "# gone\n"); + const base = markSummaryUnwritten(p, realFs); + rmSync(p); + expect(summaryWasWritten(p, base, true, realFs)).toBe(true); + expect(summaryWasWritten(p, base, false, realFs)).toBe(false); + }); + }); +});