fix([triage-gemini-pr515-out-of-scope-findings]): atomic writes for sub-agent index/metadata + quoted-arg shell parsing

.changelog/NEXT.md

@@ -67,5 +67,6 @@ TBD
 - `isExternallyManaged()` no longer false-positives on PortOS-owned venvs created from a PEP 668 base (e.g. Homebrew Python). Inside a venv, `sysconfig.get_path("stdlib")` resolves to the base interpreter's stdlib, so a venv created from Homebrew inherited the `EXTERNALLY-MANAGED` marker even though pip-in-venv ignores PEP 668 entirely. The check now also reads `sys.prefix` and `sys.base_prefix` and short-circuits to `false` when they differ (the canonical "am I in a venv?" test). Symptom this fixes: after switching from Anaconda to Homebrew and clicking "Create PortOS venv", the panel showed the new venv path but still asked to create a venv — because the new venv looked externally-managed too, hiding the regular "Install N missing packages" button.
 - Video Gen status pill no longer lies when the saved Python is missing required packages. Previously `/api/video-gen/status` returned `connected: true` whenever any `pythonPath` was stored, so a Python with no `mflux` / `mlx` / `mlx_video` installed showed a green pill until the user clicked Generate and the renderer crashed. `/status` now probes the imports via `checkPackages()` and returns `connected: false` + a `missingPackages` list when anything is absent, which triggers the same inline `LocalSetupPanel`. The panel's header copy adapts: "Set up Local Python" when no path is selected, "Install missing Python packages" (with the count) when the path is valid but packages are missing.
 - Local Python auto-detection on Apple Silicon Macs now skips x86_64 candidates: a default Anaconda install (`/opt/anaconda3/bin/python3`) was winning over `/opt/homebrew/bin/python3` and then failing at install time with `No matching distribution found for mlx` because `mlx` ships arm64-only wheels. `detectPython()` now probes `platform.machine()` of each candidate on `darwin/arm64` and prefers a matching interpreter. The `/api/image-gen/setup/check` response also gains `interpreterArch`, `hostArch`, `archMismatch`, and `suggestedArm64Python` fields; `LocalSetupPanel` surfaces a warning and a one-click "Switch to detected arm64 Python" button when the user's saved path is x86_64 on an Apple Silicon host.
+- **[triage-gemini-pr515-out-of-scope-findings] Sub-agent index and metadata writes are now crash-safe, and the in-app shell honors quoted arguments.** A mid-write crash during sub-agent completion or pruning no longer leaves a half-written `metadata.json` / agent index on disk — both go through the shared atomic-write helper that swaps via temp file. Quick-commands run from PortOS (`git commit -m "msg with spaces"`, etc.) now keep the quoted argument as a single token instead of splitting on every space and passing the broken tokens to the child process. Same atomic guarantee was added to the small JSON caches used by various settings stores, so a crash mid-save no longer truncates them.
 
 ## Removed

PLAN.md

@@ -8,7 +8,6 @@ _Batch-cleared 2026-05-25: 23 Next Up items shipped together via parallel sub-ag
 
 - [ ] [flux2-multi-reference-python-runner] **FLUX.2 multi-reference Python runner.** The UI + server contract for multi-reference editing shipped 2026-05-17 (slug `multi-reference-image-editing-for-flux-2-ui`); the Python runner (`scripts/flux2_macos.py`) currently ignores the `--reference-images`/`--reference-strengths` args that `local.js` now passes. Wire diffusers' multi-reference API in the runner and swap `server/lib/mediaModels.js#flux2-klein-9b` `tokenizerRepo` to `FLUX.2-klein-9B-kv` (gated repo — requires the user to accept the license on HF). Validate end-to-end with 2–4 uploaded refs. _(Deferred 2026-05-25 from the batch-clear: blocked on gated HF model + GPU validation; can't be verified autonomously.)_
 - [ ] [apple-health-integration-live-sync] **Apple Health integration for MeatSpace.** iOS live sync (HealthKit Shortcut → `POST /api/meatspace/apple-health` endpoint) plus a bulk historical import path for an exported `export.xml`. Wire into existing MeatSpace tabs so steps / sleep / heart rate / VO2 max / resting HR show alongside the alcohol / blood / body / epigenetic tracks already shipped. GOALS.md flags this as a documented Secondary Goal ("Apple Health integration planned") but no implementation tracking existed until this entry. When this lands, also fold an Apple-Health-imported signal into the Capabilities page "Genome & Health" row (`server/lib/capabilityMap.js#genomeRow` + the route's `genome` fetch) so a health-only setup no longer reports "Not set up" purely because no genome is uploaded (codex review of `[codex5-onboarding-capability-map]`, 2026-05-24). _(Deferred 2026-05-25 from the batch-clear: multi-file feature with an iOS Shortcut component that needs a device to verify.)_
-- [ ] [triage-gemini-pr515-out-of-scope-findings] **Triage repo-wide findings gemini surfaced during PR #515 review (UNVERIFIED, out-of-scope from the local-Python-setup PR).** Same root cause as the PR #508 entry below — gemini's `/do:review` reviews the whole repo instead of the branch diff. Items, in files PR #515 does NOT touch: (1) `server/services/commands.js:~28` — `executeCommand` uses naive `split(/\s+/)` instead of `validateCommand` / `parseCommandArgs` from `commandSecurity.js`; quoted args containing spaces break. (2) `server/lib/fileUtils.js:~482` — `createCachedStore.save` uses `writeFile` directly instead of the canonical `atomicWrite`; mid-write crash corrupts the cache file. (3) `server/services/cosAgents.js:~52` — `saveAgentIndex` re-implements atomic-temp-rename logic instead of using `atomicWrite`. (4) `server/services/cosAgents.js:~181` — `completeAgent` writes `metadata.json` non-atomically. (Gemini also flagged `server/index.js` body limit + `*` CORS; both are intentional per CLAUDE.md Security Model and were dismissed.) Surfaced 2026-05-28. Verify each is still reproducible before acting; gemini's `/do:review` has a track record of pattern-matching false-positives.
 - [ ] [triage-gemini-out-of-scope-findings] **Triage repo-wide findings gemini surfaced during PR #508 review (UNVERIFIED, out-of-scope from the voice PR).** A `/do:review` gemini pass reviewed the whole repo instead of the branch diff and flagged these pre-existing items in files PR #508 doesn't touch. Not applied to that PR (scope creep); several may be moot under the single-user/private-network trust model — verify before acting: (1) `server/routes/database.js:~236` — DB sync builds a `bash -c` string with `dumpFile` interpolated; consider `spawn` + piped stdin instead of shell string interpolation. (2) `server/services/apps.js:~304` — `updateAppTaskTypeOverride`/`bulkUpdateAppTaskTypeOverride` don't validate `taskType` against `SELF_IMPROVEMENT_TASK_TYPES`. (3) `server/routes/apps.js:~888` — `open-editor`/`open-claude` use `shell:false` for commands that are `.cmd` shims on Windows (would fail on Windows); needs a `needsShell` check + Windows metachar validation. (4) `server/lib/httpClient.js:~110` — `insecureFetch` does a bare `JSON.parse(buffer)` that can throw synchronously; wrap or reject. (5) `server/index.js` god-file (already noted in Deferred Architecture). Surfaced 2026-05-26.
 - [ ] [voice-code-agent-target-managed-app] **Voice code-agent delegation can't target a managed app yet.** The `dispatch_code_agent` voice tool (`server/services/voice/tools.js`) creates a CoS user task with no `app` set, so the agent always runs against the PortOS repo (the CoS default workspace). Add an optional spoken target ("…in BookLoom") → resolve to an app id and pass `app` through to `addTask` (cos.js already persists `metadata.app`, and `agentLifecycle.js#registerAgent` already reads it). Needs a phrase→app resolver (fuzzy match against managed app names) and a guard for "app not found." Deferred from the initial build (2026-05-26) to keep v1 scoped to the self-repo case.
 - [ ] [voice-code-agent-status-query] **No mid-task voice status query for dispatched coding agents.** Once a task is dispatched via `dispatch_code_agent`, the user can only learn the outcome from the completion announcement — there's no "how's that coding task going?" tool. Add a `code_agent_status` voice tool that reads `data/cos/state.json` for running agents tagged `metadata.voiceDispatch` and reports phase/elapsed. Deferred from the initial build (2026-05-26); the completion announcement covers the common case.

server/lib/fileUtils.js

@@ -482,8 +482,7 @@ export function createCachedStore(filePath, defaultValue, { ttl = 2000, context
   };
 
   const save = async (data) => {
-    await ensureDir(dir);
-    await writeFile(filePath, JSON.stringify(data, null, 2));
+    await atomicWrite(filePath, data);
     cache = data;
     cacheTimestamp = Date.now();
   };

server/services/commands.js

@@ -1,6 +1,6 @@
 import { spawn } from 'child_process';
 import { logAction } from './history.js';
-import { ALLOWED_COMMANDS, DANGEROUS_SHELL_CHARS } from '../lib/commandSecurity.js';
+import { ALLOWED_COMMANDS, validateCommand } from '../lib/commandSecurity.js';
 
 // Track active commands
 const activeCommands = new Map();
@@ -16,40 +16,24 @@ const activeCommands = new Map();
 export function executeCommand(command, workspacePath, onData, onComplete) {
   const commandId = Date.now().toString(36) + Math.random().toString(36).substr(2);
 
-  // Security: Reject empty or whitespace-only commands
-  const trimmedCommand = command?.trim();
-  if (!trimmedCommand) {
-    const error = 'Empty command provided';
-    onComplete?.({ success: false, error, exitCode: 1 });
-    return null;
-  }
-
-  // Parse command to check allowlist
-  const parts = trimmedCommand.split(/\s+/);
-  const baseCommand = parts[0];
-
-  if (!ALLOWED_COMMANDS.has(baseCommand)) {
-    const error = `Command '${baseCommand}' is not in the allowlist`;
-    onComplete?.({ success: false, error, exitCode: 1 });
-    logAction('command', null, trimmedCommand.substring(0, 50), { command: trimmedCommand, workspacePath }, false, error);
-    return null;
-  }
-
-  // Security: Check for dangerous shell metacharacters that could enable command injection
-  // This prevents attacks like: npm; rm -rf / or npm && malicious_cmd or npm | cat /etc/passwd
-  if (DANGEROUS_SHELL_CHARS.test(trimmedCommand)) {
-    const error = 'Command contains disallowed shell characters (security restriction)';
-    onComplete?.({ success: false, error, exitCode: 1 });
-    logAction('command', null, trimmedCommand.substring(0, 50), { command: trimmedCommand, workspacePath }, false, error);
+  const validation = validateCommand(command);
+  if (!validation.valid) {
+    const trimmedForLog = (command || '').trim();
+    onComplete?.({ success: false, error: validation.error, exitCode: 1 });
+    if (trimmedForLog) {
+      logAction('command', null, trimmedForLog.substring(0, 50), { command: trimmedForLog, workspacePath }, false, validation.error);
+    }
     return null;
   }
 
+  const { baseCommand, args } = validation;
   const startTime = Date.now();
   let output = '';
 
-  // Security: Use spawn with array of args (shell:false) to prevent shell injection
-  // The DANGEROUS_SHELL_CHARS check above ensures no metacharacters slip through
-  const child = spawn(baseCommand, parts.slice(1), {
+  // Security: Use spawn with array of args (shell:false) to prevent shell injection.
+  // validateCommand has already rejected shell metacharacters AND parsed quoted args
+  // correctly (e.g. 'git commit -m "msg with spaces"' becomes 4 args, not 5).
+  const child = spawn(baseCommand, args, {
     cwd: workspacePath || process.cwd(),
     env: { ...process.env, FORCE_COLOR: '1' },
     shell: false,

server/services/cosAgents.js

@@ -11,7 +11,7 @@ import { existsSync } from 'fs';
 import { join } from 'path';
 import { cosEvents, emitLog } from './cosEvents.js';
 import { loadState, saveState, withStateLock, AGENTS_DIR } from './cosState.js';
-import { ensureDir, safeJSONParse, tryReadFile } from '../lib/fileUtils.js';
+import { atomicWrite, ensureDir, safeJSONParse, tryReadFile } from '../lib/fileUtils.js';
 import { repairCodexTaskSummary } from './codexSummaryRepair.js';
 
 const INDEX_FILE = join(AGENTS_DIR, 'index.json');
@@ -50,19 +50,15 @@ export async function loadAgentIndex() {
   return agentIndexPromise;
 }
 
-// Persist agent index to disk (atomic write via temp file + rename)
+// Persist agent index to disk via the shared atomicWrite helper (temp file + rename,
+// with Windows backup-swap fallback). Without atomic semantics a mid-write crash
+// truncates index.json and on next boot the date-bucket migration would silently
+// re-run (or worse, drop already-archived agents from the lookup).
 async function saveAgentIndex() {
   if (!agentIndex) return;
   const obj = Object.fromEntries(agentIndex);
-  const tmpFile = `${INDEX_FILE}.tmp`;
-  const written = await writeFile(tmpFile, JSON.stringify(obj)).then(() => true).catch(err => {
+  await atomicWrite(INDEX_FILE, obj).catch(err => {
     console.error(`❌ Failed to save agent index: ${err.message}`);
-    return false;
-  });
-  if (!written) return;
-  await rename(tmpFile, INDEX_FILE).catch(err => {
-    console.error(`❌ Failed to rename agent index: ${err.message}`);
-    rm(tmpFile, { force: true }).catch(() => {});
   });
 }
 
@@ -83,7 +79,7 @@ async function migrateAgentsToDateBuckets() {
 
   if (!existsSync(AGENTS_DIR)) {
     await ensureDir(AGENTS_DIR);
-    await writeFile(INDEX_FILE, '{}');
+    await atomicWrite(INDEX_FILE, {});
     console.log('📂 Created empty agent index (no agents to migrate)');
     return index;
   }
@@ -108,7 +104,7 @@ async function migrateAgentsToDateBuckets() {
   const flatAgentDirs = entries.filter(e => e.isDirectory() && e.name.startsWith('agent-'));
 
   if (flatAgentDirs.length === 0) {
-    await writeFile(INDEX_FILE, JSON.stringify(Object.fromEntries(index)));
+    await atomicWrite(INDEX_FILE, Object.fromEntries(index));
     console.log(`📂 Agent index built: ${index.size} entries (no flat dirs to migrate)`);
     return index;
   }
@@ -185,7 +181,7 @@ async function migrateAgentsToDateBuckets() {
   }
 
   // Persist index
-  await writeFile(INDEX_FILE, JSON.stringify(Object.fromEntries(index)));
+  await atomicWrite(INDEX_FILE, Object.fromEntries(index));
   const uniqueDates = new Set(index.values()).size;
   const parts = [`📦 Migrated ${migrated} agents into date buckets (${uniqueDates} unique dates)`];
   if (skipped > 0) parts.push(`skipped ${skipped} undatable`);
@@ -309,7 +305,7 @@ export async function completeAgent(agentId, result = {}) {
       await ensureDir(flatDir);
     }
     const { output: _output, ...agentWithoutOutput } = state.agents[agentId];
-    await writeFile(join(flatDir, 'metadata.json'), JSON.stringify(agentWithoutOutput, null, 2));
+    await atomicWrite(join(flatDir, 'metadata.json'), agentWithoutOutput);
 
     // Move entire agent dir into date bucket (atomic on same filesystem)
     const targetDir = join(bucketDir, agentId);
@@ -663,7 +659,7 @@ export async function cleanupZombieAgents() {
 
         // Ensure metadata is written before move
         if (!existsSync(flatDir)) await ensureDir(flatDir);
-        await writeFile(join(flatDir, 'metadata.json'), JSON.stringify(agentWithoutOutput, null, 2)).catch(() => {});
+        await atomicWrite(join(flatDir, 'metadata.json'), agentWithoutOutput).catch(() => {});
 
         // Move to date bucket
         const targetDir = join(bucketDir, agentId);
@@ -750,7 +746,7 @@ export async function submitAgentFeedback(agentId, feedback) {
           const raw = safeJSONParse(content, null);
           if (raw) {
             raw.feedback = feedbackData;
-            await writeFile(metaPath, JSON.stringify(raw, null, 2)).catch(() => {});
+            await atomicWrite(metaPath, raw).catch(() => {});
           }
         }
       }
@@ -773,7 +769,7 @@ export async function submitAgentFeedback(agentId, feedback) {
     if (!raw) return { error: 'Agent not found' };
 
     raw.feedback = feedbackData;
-    await writeFile(metaPath, JSON.stringify(raw, null, 2));
+    await atomicWrite(metaPath, raw);
 
     emitLog('info', `Feedback received for agent ${agentId}: ${feedback.rating}`, { agentId, rating: feedback.rating });
     cosEvents.emit('agent:feedback', { agentId, feedback: feedbackData });
@@ -887,7 +883,7 @@ export async function archiveStaleAgents() {
 
         if (existsSync(flatDir) && !existsSync(targetDir)) {
           // Write metadata then move (with cross-filesystem fallback)
-          await writeFile(join(flatDir, 'metadata.json'), JSON.stringify(agentWithoutOutput, null, 2)).catch(() => {});
+          await atomicWrite(join(flatDir, 'metadata.json'), agentWithoutOutput).catch(() => {});
           await rename(flatDir, targetDir).catch(async () => {
             await ensureDir(targetDir);
             const files = await readdir(flatDir).catch(() => []);
@@ -900,7 +896,7 @@ export async function archiveStaleAgents() {
           if (!existsSync(targetDir)) continue; // Skip index update if move failed
         } else if (!existsSync(targetDir)) {
           await ensureDir(targetDir);
-          await writeFile(join(targetDir, 'metadata.json'), JSON.stringify(agentWithoutOutput, null, 2)).catch(() => {});
+          await atomicWrite(join(targetDir, 'metadata.json'), agentWithoutOutput).catch(() => {});
         }
 
         idx.set(id, dateStr);