Skip to content

Commit 6ba4aa1

Browse files
committed
fix: close 9 findings from Oracle audit wave 3 (dreamer/storage/rpc/system-prompt)
Three parallel read-only Oracles (dreamer subsystem; storage/migrations/sqlite backend; RPC/TUI/system-prompt/conflict/auto-update). Cores verified safe (lease atomicity/reclaim/renewal, circuit breaker, epoch semantics; schema fence, fresh-DB migration, bind-style guard, transaction shim; RPC bearer auth + session-scoped notifications, conflict canonical-name matching, auto-update backgrounding). 9 source-confirmed fixes; design items banked (D1 reinforced, D5–D9). P0: - system-prompt handler fail-open (system-prompt-hash.ts). estimateTokens / updateSessionMeta could throw into the prompt path (the caller just awaits) — a busy/failing DB or pathological tokenizer would FAIL the LLM call instead of losing a telemetry write. Per the project's fail-open-in-per-turn-handlers rule, wrapped both in try/catch; the already-mutated prompt is preserved. +regression test (drops session_meta mid-flight, asserts handler resolves + prompt intact). P1: - Dreamer lease-busy dequeue (runner.ts). With a live lease held (this project or a sibling on the shared queue), the tick still dequeued, failed lease acquisition, incremented retry, and after MAX_LEASE_RETRIES DELETED the queue row — silently dropping a project's pending dream. Now skip the tick while a lease is active; the entry stays queued. - RPC token-file perms (rpc-server.ts). mkdir/writeFile mode only applies on CREATE, so a pre-existing loose-perm dir or stale .tmp could leave the bearer token world-readable. chmod the dir 0o700, rm stale tmp, chmod the final file 0o600 defensively. - node:sqlite migration-conflict breadth (migrations.ts). isSiblingMigrationConflict hard-gated on bun:sqlite error CODES; node:sqlite (Pi/Desktop) can report a different/absent code for the same schema_migrations PK conflict → fail-CLOSE on a legit concurrent-startup race (schema-fence incident class). Switched to the backend-identical error MESSAGE + authoritative row-existence check. P2: - v35 raw ALTER → ensureColumn (migrations.ts): re-check-on-failure tolerates a concurrent sibling adding share_categories. - PRAGMA ordering: busy_timeout before journal_mode=WAL on the dashboard read-write connection (db.rs) and the OpenCode compaction-marker direct-open (compaction-marker.ts) — avoids immediate SQLITE_BUSY on cold-open contention. - Dreamer optional-phase child sessions (runner.ts smart-notes, review-user- memories.ts, identify-key-files.ts) now retained on FAILURE (debugging), mirroring the main-task rule, instead of unconditional delete. - Dashboard enqueue_dream dedup (db.rs): mirror the plugin's "skip if already queued" + normalize to identity, so repeated "dream now" clicks don't pile up duplicate rows a host drains one at a time. - Conflict-disabled config hook early-return (index.ts): when a conflicting plugin disabled the runtime, stop registering /ctx-* commands + hidden agents (pure UX confusion — the runtime won't service them). Gate: plugin 2175/0 (+2), Pi 471/0, dashboard rust 93+21+10+20/0, tsc+biome clean.
1 parent c94798c commit 6ba4aa1

10 files changed

Lines changed: 157 additions & 27 deletions

File tree

packages/dashboard/src-tauri/src/db.rs

Lines changed: 21 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -121,9 +121,12 @@ pub fn open_readonly(path: &PathBuf) -> Result<Connection, rusqlite::Error> {
121121
/// Opens a read-write connection for write operations (memory edits, queue entries).
122122
pub fn open_readwrite(path: &PathBuf) -> Result<Connection, rusqlite::Error> {
123123
let conn = Connection::open(path)?;
124-
conn.pragma_update(None, "journal_mode", "WAL")?;
124+
// busy_timeout MUST come before journal_mode=WAL: setting WAL can itself need
125+
// the file lock, and with the timeout installed last a cold-open under
126+
// contention fails immediately with SQLITE_BUSY instead of waiting.
125127
conn.pragma_update(None, "busy_timeout", 5000)?;
126128
conn.pragma_update(None, "foreign_keys", "ON")?;
129+
conn.pragma_update(None, "journal_mode", "WAL")?;
127130
warn_if_dashboard_schema_requires_upgrade(&conn);
128131
Ok(conn)
129132
}
@@ -4378,10 +4381,26 @@ pub fn enqueue_dream(
43784381
project_path: &str,
43794382
reason: &str,
43804383
) -> Result<i64, rusqlite::Error> {
4384+
// Mirror the plugin's enqueueDream dedup: skip if this project already has ANY
4385+
// queue entry (queued or running). Without this, repeated dashboard "dream now"
4386+
// clicks pile up duplicate rows that a single identity-filtered host drains one
4387+
// at a time. project_path is the resolved identity (the UI passes git:/dir:),
4388+
// matching how hosts dequeue — a raw path would never be drained.
4389+
let identity = normalize_stored_project_path(project_path);
4390+
let existing: Option<i64> = conn
4391+
.query_row(
4392+
"SELECT id FROM dream_queue WHERE project_path = ?1 LIMIT 1",
4393+
rusqlite::params![identity],
4394+
|row| row.get(0),
4395+
)
4396+
.optional()?;
4397+
if let Some(id) = existing {
4398+
return Ok(id);
4399+
}
43814400
let now = chrono::Utc::now().timestamp_millis();
43824401
conn.execute(
43834402
"INSERT INTO dream_queue (project_path, reason, enqueued_at) VALUES (?1, ?2, ?3)",
4384-
rusqlite::params![project_path, reason, now],
4403+
rusqlite::params![identity, reason, now],
43854404
)?;
43864405
Ok(conn.last_insert_rowid())
43874406
}

packages/plugin/src/features/magic-context/compaction-marker.ts

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -158,9 +158,11 @@ function getWritableOpenCodeDb(): Database {
158158
}
159159
}
160160
const db = new Database(dbPath);
161-
db.exec("PRAGMA journal_mode=WAL");
162-
// Allow up to 5s wait when OpenCode holds a write lock
161+
// busy_timeout BEFORE journal_mode=WAL: setting WAL can need the file lock, so
162+
// with the timeout installed first a cold-open while OpenCode holds the lock
163+
// waits up to 5s instead of throwing SQLITE_BUSY immediately.
163164
db.exec("PRAGMA busy_timeout=5000");
165+
db.exec("PRAGMA journal_mode=WAL");
164166
cachedWriteDb = { path: dbPath, db };
165167
return db;
166168
}

packages/plugin/src/features/magic-context/dreamer/runner.ts

Lines changed: 17 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -783,6 +783,11 @@ Only include notes whose conditions you could definitively evaluate against exte
783783

784784
const taskStartedAt = Date.now();
785785
let agentSessionId: string | null = null;
786+
// Retain the child session on failure so its prompt/output/error can be
787+
// inspected — mirrors the main-task cleanup rule. Optional phases used to
788+
// delete unconditionally, losing the evidence for exactly the runs worth
789+
// debugging.
790+
let phaseFailed = false;
786791
const startedAt = Date.now();
787792
let invocationRecorded = false;
788793
const recordInvocation = (params: {
@@ -927,6 +932,7 @@ Only include notes whose conditions you could definitively evaluate against exte
927932
result: `${surfaced} surfaced, ${pending} still pending`,
928933
});
929934
} catch (error) {
935+
phaseFailed = true;
930936
const durationMs = Date.now() - taskStartedAt;
931937
const errorDescription = describeError(error);
932938
args.result.smartNotesSurfaced = 0;
@@ -943,7 +949,8 @@ Only include notes whose conditions you could definitively evaluate against exte
943949
});
944950
} finally {
945951
clearInterval(leaseInterval);
946-
if (agentSessionId && !shouldKeepSubagents()) {
952+
// Keep the child session on failure (debugging) or under keep_subagents.
953+
if (agentSessionId && !phaseFailed && !shouldKeepSubagents()) {
947954
await args.client.session
948955
.delete({
949956
path: { id: agentSessionId },
@@ -999,9 +1006,16 @@ export async function processDreamQueue(args: {
9991006
// would otherwise have its own queue row deleted mid-run. Scope to this project so the
10001007
// cross-process shared queue doesn't reap another host's still-running rows.
10011008
const maxRuntimeMs = args.maxRuntimeMinutes * 60 * 1000;
1002-
if (!hasActiveDreamLease(args.db)) {
1003-
clearStaleEntries(args.db, maxRuntimeMs + 30 * 60 * 1000, args.projectIdentity);
1009+
// A live lease means another dream (this project or a sibling on the shared
1010+
// queue) is actively running. Don't dequeue underneath it: runDream would just
1011+
// fail lease acquisition, increment this entry's retry count, and after
1012+
// MAX_LEASE_RETRIES DELETE the queue row — silently dropping a project's
1013+
// pending dream that never got a fair chance to run. Skip this tick; the entry
1014+
// stays queued for when the lease frees.
1015+
if (hasActiveDreamLease(args.db)) {
1016+
return null;
10041017
}
1018+
clearStaleEntries(args.db, maxRuntimeMs + 30 * 60 * 1000, args.projectIdentity);
10051019
const entry = dequeueNext(args.db, args.projectIdentity);
10061020
if (!entry) {
10071021
return null;

packages/plugin/src/features/magic-context/key-files/identify-key-files.ts

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -477,6 +477,7 @@ async function runKeyFilesLlm(args: {
477477
});
478478
const agentSessionId = typeof created?.id === "string" ? created.id : null;
479479
if (!agentSessionId) throw new Error("Could not create key-file identification session.");
480+
let succeeded = false;
480481
try {
481482
await shared.promptSyncWithModelSuggestionRetry(
482483
args.client,
@@ -504,9 +505,13 @@ async function runKeyFilesLlm(args: {
504505
});
505506
const text = extractLatestAssistantText(messages);
506507
if (!text) throw new Error("Dreamer returned no key-files output.");
508+
succeeded = true;
507509
return { text, messages };
508510
} finally {
509-
if (!shouldKeepSubagents()) {
511+
// Keep the child session on failure (debugging) — mirrors the main-task
512+
// cleanup rule; this try/finally has no catch, so a throw leaves
513+
// succeeded=false and the session is retained.
514+
if (succeeded && !shouldKeepSubagents()) {
510515
await args.client.session
511516
.delete({ path: { id: agentSessionId } })
512517
.catch(() => undefined);

packages/plugin/src/features/magic-context/migrations.ts

Lines changed: 21 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -1363,11 +1363,16 @@ const MIGRATIONS: Migration[] = [
13631363
share_categories TEXT NOT NULL DEFAULT '["CONSTRAINTS"]'
13641364
);
13651365
`);
1366-
if (!columnExists(db, "workspaces", "share_categories")) {
1367-
db.exec(
1368-
`ALTER TABLE workspaces ADD COLUMN share_categories TEXT NOT NULL DEFAULT '["CONSTRAINTS"]'`,
1369-
);
1370-
}
1366+
// ensureColumn (not a guarded raw ALTER): its re-check-on-failure
1367+
// tolerates a concurrent sibling process adding the same column between
1368+
// our existence check and the ALTER (duplicate-column error → re-verify
1369+
// → return). The raw guarded form could throw on that race.
1370+
ensureColumn(
1371+
db,
1372+
"workspaces",
1373+
"share_categories",
1374+
`TEXT NOT NULL DEFAULT '["CONSTRAINTS"]'`,
1375+
);
13711376
db.prepare(
13721377
`UPDATE workspaces
13731378
SET share_categories = '["CONSTRAINTS"]'
@@ -1541,14 +1546,17 @@ function getCurrentVersion(db: Database): number {
15411546
*/
15421547
export function isSiblingMigrationConflict(db: Database, error: unknown, version: number): boolean {
15431548
if (!(error instanceof Error)) return false;
1544-
const code = (error as { code?: unknown }).code;
1545-
if (code !== "SQLITE_CONSTRAINT_PRIMARYKEY" && code !== "SQLITE_CONSTRAINT_UNIQUE") {
1546-
return false;
1547-
}
1548-
// Distinguish "PRIMARY KEY conflict on schema_migrations(version)"
1549-
// from any other UNIQUE/PK collision the migration body could surface.
1550-
// SQLite's better-sqlite3 binding includes the constraint in the
1551-
// message, e.g. "UNIQUE constraint failed: schema_migrations.version".
1549+
// Identify "PRIMARY KEY conflict on schema_migrations(version)" by the SQLite
1550+
// ERROR MESSAGE — which originates from the C library (sqlite3_errmsg) and is
1551+
// identical across bun:sqlite and node:sqlite, e.g.
1552+
// "UNIQUE constraint failed: schema_migrations.version". We deliberately do NOT
1553+
// hard-gate on `error.code`: bun:sqlite reports SQLITE_CONSTRAINT_PRIMARYKEY/
1554+
// _UNIQUE, but node:sqlite (Pi / Desktop) can report a different or absent code
1555+
// for the SAME conflict, and a strict code-only gate would fail-CLOSE a
1556+
// legitimate concurrent-startup race there (the schema-fence incident class).
1557+
// The message guard below already excludes a PK/UNIQUE collision the migration
1558+
// BODY could raise on some other table, and the row-existence check is the
1559+
// authoritative confirmation that a sibling actually applied this version.
15521560
const msg = error.message;
15531561
if (!msg.includes("schema_migrations")) return false;
15541562
if (!msg.toLowerCase().includes("version")) return false;

packages/plugin/src/features/magic-context/user-memory/review-user-memories.ts

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -108,6 +108,8 @@ Return valid JSON (no markdown fencing):
108108
If no promotions are warranted, return empty arrays. Always consume reviewed candidates so they don't accumulate indefinitely.`;
109109

110110
let agentSessionId: string | null = null;
111+
// Keep the child session on failure (debugging), mirroring the main-task rule.
112+
let phaseFailed = false;
111113
const startedAt = Date.now();
112114
let invocationRecorded = false;
113115
const recordInvocation = (params: {
@@ -289,6 +291,7 @@ If no promotions are warranted, return empty arrays. Always consume reviewed can
289291

290292
return result;
291293
} catch (error) {
294+
phaseFailed = true;
292295
const errorDescription = describeError(error);
293296
log(
294297
`[dreamer] user-memories: review failed: ${errorDescription.brief}`,
@@ -297,7 +300,7 @@ If no promotions are warranted, return empty arrays. Always consume reviewed can
297300
return result;
298301
} finally {
299302
clearInterval(leaseInterval);
300-
if (agentSessionId && !shouldKeepSubagents()) {
303+
if (agentSessionId && !phaseFailed && !shouldKeepSubagents()) {
301304
await args.client.session
302305
.delete({
303306
path: { id: agentSessionId },

packages/plugin/src/hooks/magic-context/system-prompt-hash.test.ts

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -224,6 +224,32 @@ describe("system-prompt-hash token estimation (council audit bg_51106601 #2)", (
224224
});
225225
});
226226

227+
describe("system-prompt-hash fail-open (per-turn handler must never throw)", () => {
228+
it("resolves and preserves the mutated prompt when the meta write fails", async () => {
229+
useTempDataHome("sph-fail-open-");
230+
const sessionId = "ses-fail-open";
231+
const { handler } = buildHandler();
232+
const db = openDatabase();
233+
234+
// Pass 1 primes session_meta (hash + tokens) cleanly.
235+
await handler({ sessionID: sessionId }, { system: ["You are a helpful agent."] });
236+
237+
// Now sabotage the persistence layer so the hash-change branch's
238+
// updateSessionMeta throws on pass 2. Dropping the table makes any write
239+
// raise — simulating a busy/failing DB. The handler must NOT propagate it.
240+
db.exec("DROP TABLE session_meta");
241+
242+
const system = ["You are a helpful agent.", "DIFFERENT content forces a hash change"];
243+
// Must not throw — a throw here would fail the LLM call instead of just
244+
// losing a telemetry write.
245+
await handler({ sessionID: sessionId }, { system });
246+
247+
// The prompt was still mutated/injected (guidance present) — failing open
248+
// means we keep what we did, not crash.
249+
expect(system.join("\n")).toContain("## Magic Context");
250+
});
251+
});
252+
227253
describe("system-prompt-hash v2 system prompt contents", () => {
228254
it("keeps project docs, user profile, and key files out of the system prompt", async () => {
229255
useTempDataHome("sph-v2-adjuncts-out-");

packages/plugin/src/hooks/magic-context/system-prompt-hash.ts

Lines changed: 26 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -417,12 +417,33 @@ export function createSystemPromptHashHandler(deps: {
417417

418418
// Estimate system prompt tokens for dashboard visibility only when
419419
// the prompt hash changed; unchanged prompts keep the stored count.
420+
//
421+
// FAIL-OPEN (per-turn handler rule): the prompt has ALREADY been mutated
422+
// in place above (guidance + sticky date). estimateTokens can throw on a
423+
// pathological tokenizer input and updateSessionMeta can throw on a busy/
424+
// failing DB — neither must propagate into the prompt path (a throw here
425+
// would fail the LLM call instead of just losing a telemetry write). Persist
426+
// the hash even if token estimation fails, so the next pass doesn't re-detect
427+
// a phantom hash change and re-flush.
420428
if (currentHash !== previousHash) {
421-
const systemPromptTokens = estimateTokens(systemContent);
422-
updateSessionMeta(deps.db, sessionId, {
423-
systemPromptHash: currentHash,
424-
systemPromptTokens,
425-
});
429+
let systemPromptTokens = sessionMeta.systemPromptTokens;
430+
try {
431+
systemPromptTokens = estimateTokens(systemContent);
432+
} catch (error) {
433+
sessionLog(
434+
sessionId,
435+
"system prompt token estimate failed (using prior count):",
436+
error,
437+
);
438+
}
439+
try {
440+
updateSessionMeta(deps.db, sessionId, {
441+
systemPromptHash: currentHash,
442+
systemPromptTokens,
443+
});
444+
} catch (error) {
445+
sessionLog(sessionId, "system prompt meta persist failed (fail-open):", error);
446+
}
426447
}
427448

428449
// ── Step 4: Drain systemPromptRefreshSessions (one-shot semantics) ──

packages/plugin/src/index.ts

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -466,6 +466,14 @@ const plugin: Plugin = async (ctx) => {
466466
await hooks.magicContext?.["experimental.text.complete"]?.(input, output);
467467
},
468468
config: async (config) => {
469+
// If the runtime is disabled (a conflicting plugin — DCP / OMO /
470+
// OpenCode auto-compaction — was detected and we fail-safed at boot),
471+
// do NOT register the /ctx-* commands or hidden agents. The transform/
472+
// tools/RPC are already no-op'd, so surfacing command entries + hidden
473+
// agents the runtime won't service is pure UX confusion.
474+
if (pluginConfig.enabled !== true) {
475+
return;
476+
}
469477
/**
470478
* Build a hidden-agent config with a deny-everything-by-default
471479
* permission baseline plus an explicit allow-list of tool ids the

packages/plugin/src/shared/rpc-server.ts

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,11 @@
11
import { randomBytes, timingSafeEqual } from "node:crypto";
22
import {
3+
chmodSync,
34
mkdirSync,
45
readdirSync,
56
readFileSync,
67
renameSync,
8+
rmSync,
79
unlinkSync,
810
writeFileSync,
911
} from "node:fs";
@@ -85,7 +87,22 @@ export class MagicContextRpcServer {
8587
// file 0o600. renameSync preserves the tmp file's mode, so
8688
// the 0o600 on the write covers the final file.
8789
mkdirSync(dir, { recursive: true, mode: 0o700 });
90+
// mkdirSync's mode only applies on CREATION — a dir left by an
91+
// older build (or default 0o755 umask) keeps its loose perms, so
92+
// chmod it defensively so the bearer token isn't world-readable.
93+
try {
94+
chmodSync(dir, 0o700);
95+
} catch {
96+
// best-effort
97+
}
8898
const tmpPath = `${this.portFilePath}.tmp`;
99+
// A stale tmp from a crashed write could exist with loose perms;
100+
// writeFileSync's mode only applies on create, so remove it first.
101+
try {
102+
rmSync(tmpPath, { force: true });
103+
} catch {
104+
// best-effort
105+
}
89106
writeFileSync(
90107
tmpPath,
91108
JSON.stringify({
@@ -97,6 +114,13 @@ export class MagicContextRpcServer {
97114
{ encoding: "utf-8", mode: 0o600 },
98115
);
99116
renameSync(tmpPath, this.portFilePath);
117+
// renameSync preserves the tmp's mode, but chmod the final path
118+
// defensively in case the token file pre-existed with loose perms.
119+
try {
120+
chmodSync(this.portFilePath, 0o600);
121+
} catch {
122+
// best-effort
123+
}
100124
log(`[rpc] server listening on 127.0.0.1:${this.port}`);
101125
} catch (err) {
102126
log(`[rpc] failed to write port file: ${err}`);

0 commit comments

Comments
 (0)