From 652a9d68f1e30dff03a0b380cc27bb4433c40d01 Mon Sep 17 00:00:00 2001 From: Richardson Gunde Date: Wed, 22 Jul 2026 12:35:53 +0530 Subject: [PATCH 1/4] feat(#452): sandbox execution config + authoritative command + evidence mapping MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The Scientist gains the pieces the validate gate needs: - resolveSandboxConfig / loadSandboxConfig read a `sandbox` block from .rstack/rstack.config.json (sibling to guardrails; malformed → safe defaults). - resolveSandboxCommand resolves the command to execute from TRUSTED sources only (task.test_command written by the planner, per-stage config, or the project-global command) — NEVER the untrusted builder's tests_run, or a builder could declare tests_run:["true"] and mint its own green. Returns null → honest 'observed' degrade, never a false PASS. - executionCheck maps a runInSandbox record → a validation check; on FAIL the evidence IS the actual captured logs (not a summary), shaped for #451. - config-validation names unknown keys / bad types / typo'd per-stage ids so a misconfigured command can't silently leave execution unverified. Co-Authored-By: Claude Opus 4.8 --- src/core/harness/config-validation.js | 53 ++++++++++ src/core/harness/sandbox.js | 146 +++++++++++++++++++++++++- 2 files changed, 198 insertions(+), 1 deletion(-) diff --git a/src/core/harness/config-validation.js b/src/core/harness/config-validation.js index a188a568..ae39137c 100644 --- a/src/core/harness/config-validation.js +++ b/src/core/harness/config-validation.js @@ -119,6 +119,59 @@ export function validateRstackConfig(parsed) { if (parsed.enabled_packs != null) { issues.push(...validateEnabledPacksConfig(parsed.enabled_packs)); } + // #452: transient-sandbox execution settings (The Scientist). A typo'd + // per_stage key or command is the silent-failure mode here — the command + // never runs and execution silently stays unverified — so each is named. + if (parsed.sandbox != null) { + issues.push(...validateSandboxConfig(parsed.sandbox)); + } + return issues; +} + +const SANDBOX_KNOWN_KEYS = ['enabled', 'image', 'command', 'network', 'timeout_ms', 'timeoutMs', 'per_stage', 'perStage', 'limits']; + +export function validateSandboxConfig(sandbox) { + const issues = []; + if (!isPlainObject(sandbox)) { + issues.push({ field: 'sandbox', problem: 'must be an object of sandbox execution settings (e.g. { "command": "npm test", "image": "node:20-alpine" })' }); + return issues; + } + for (const key of Object.keys(sandbox)) { + if (!SANDBOX_KNOWN_KEYS.includes(key)) { + issues.push({ field: `sandbox.${key}`, problem: 'unknown sandbox key — this setting is ignored' }); + } + } + if (sandbox.enabled != null && typeof sandbox.enabled !== 'boolean') { + issues.push({ field: 'sandbox.enabled', problem: `must be a boolean, got ${JSON.stringify(sandbox.enabled)} — the default (true) applies` }); + } + if (sandbox.network != null && typeof sandbox.network !== 'boolean') { + issues.push({ field: 'sandbox.network', problem: `must be a boolean (default false = no network), got ${JSON.stringify(sandbox.network)}` }); + } + if (sandbox.image != null && (typeof sandbox.image !== 'string' || !sandbox.image.trim())) { + issues.push({ field: 'sandbox.image', problem: 'must be a non-empty container image reference (e.g. "node:20-alpine") — the default (alpine:3.20) runs shell only' }); + } + if (sandbox.command != null && (typeof sandbox.command !== 'string' || !sandbox.command.trim())) { + issues.push({ field: 'sandbox.command', problem: 'must be a non-empty shell command string (e.g. "npm test") — execution stays unverified without one' }); + } + const timeout = sandbox.timeout_ms ?? sandbox.timeoutMs; + if (timeout != null && !isNonNegativeNumber(timeout)) { + issues.push({ field: 'sandbox.timeout_ms', problem: `must be a non-negative number of milliseconds, got ${JSON.stringify(timeout)} — the default applies` }); + } + const perStage = sandbox.per_stage ?? sandbox.perStage; + if (perStage != null) { + if (!isPlainObject(perStage)) { + issues.push({ field: 'sandbox.per_stage', problem: 'must be an object of canonical-stage-id -> { command } (e.g. "07-code": { "command": "npm test" })' }); + } else { + for (const [stageId, entry] of Object.entries(perStage)) { + if (!getCanonicalStage(stageId)) { + issues.push({ field: `sandbox.per_stage.${stageId}`, problem: 'unknown canonical stage id — this per-stage command will NEVER run; use a canonical 00-14 stage id like "07-code"' }); + } + if (!isPlainObject(entry) || typeof entry.command !== 'string' || !entry.command.trim()) { + issues.push({ field: `sandbox.per_stage.${stageId}`, problem: 'must be an object with a non-empty "command" string' }); + } + } + } + } return issues; } diff --git a/src/core/harness/sandbox.js b/src/core/harness/sandbox.js index 1c807982..c4e455a0 100644 --- a/src/core/harness/sandbox.js +++ b/src/core/harness/sandbox.js @@ -20,7 +20,9 @@ import { spawn, spawnSync } from 'node:child_process'; import { randomUUID } from 'node:crypto'; -import { resolve } from 'node:path'; +import { readFile } from 'node:fs/promises'; +import { existsSync } from 'node:fs'; +import { resolve, join } from 'node:path'; const DEFAULT_TIMEOUT_MS = 120_000; const MAX_TIMEOUT_MS = 600_000; @@ -176,3 +178,145 @@ export function runInSandbox(runDir, { taskId, command, network = false, timeout child.on('close', (code) => finish(code)); }); } + +// --------------------------------------------------------------------------- +// #452 PR2 — wiring the Scientist into the validate gate. +// +// Sandbox execution config lives in .rstack/rstack.config.json under a `sandbox` +// block, sibling to `guardrails` (read the same way loadProjectGuardrails does). +// --------------------------------------------------------------------------- + +export const DEFAULT_SANDBOX_CONFIG = Object.freeze({ + enabled: true, // auto-run whenever a runtime + authoritative command exist + image: 'alpine:3.20', // shell-only default; set node:/python: images to run real suites + network: false, // OFF by default (exfiltration + malicious-install guard) + timeoutMs: DEFAULT_TIMEOUT_MS, + command: null, // project-global authoritative test command (trusted) + perStage: {}, // canonical-stage-id -> { command, image?, network?, timeoutMs? } + limits: { memory: '512m', pids: 256, cpus: 1 }, +}); + +// Merge + validate a raw `sandbox` block into an effective config. Unknown keys +// are ignored; bad-typed values fall back to the default (never silently weaken +// the network/isolation posture). +export function resolveSandboxConfig(raw = {}) { + const cfg = { + ...DEFAULT_SANDBOX_CONFIG, + limits: { ...DEFAULT_SANDBOX_CONFIG.limits }, + perStage: {}, + }; + if (!raw || typeof raw !== 'object' || Array.isArray(raw)) return cfg; + if (typeof raw.enabled === 'boolean') cfg.enabled = raw.enabled; + if (typeof raw.image === 'string' && raw.image.trim()) cfg.image = raw.image.trim(); + if (typeof raw.network === 'boolean') cfg.network = raw.network; + const timeout = Number(raw.timeout_ms ?? raw.timeoutMs); + if (Number.isFinite(timeout) && timeout > 0) cfg.timeoutMs = Math.min(timeout, MAX_TIMEOUT_MS); + if (typeof raw.command === 'string' && raw.command.trim()) cfg.command = raw.command.trim(); + if (raw.limits && typeof raw.limits === 'object' && !Array.isArray(raw.limits)) { + if (typeof raw.limits.memory === 'string' && raw.limits.memory.trim()) cfg.limits.memory = raw.limits.memory.trim(); + if (Number.isFinite(Number(raw.limits.pids))) cfg.limits.pids = Number(raw.limits.pids); + if (Number.isFinite(Number(raw.limits.cpus))) cfg.limits.cpus = Number(raw.limits.cpus); + } + const per = raw.per_stage ?? raw.perStage; + if (per && typeof per === 'object' && !Array.isArray(per)) { + for (const [stageId, entry] of Object.entries(per)) { + if (entry && typeof entry === 'object' && typeof entry.command === 'string' && entry.command.trim()) { + const stageTimeout = Number(entry.timeout_ms ?? entry.timeoutMs); + cfg.perStage[stageId] = { + command: entry.command.trim(), + image: typeof entry.image === 'string' && entry.image.trim() ? entry.image.trim() : undefined, + network: typeof entry.network === 'boolean' ? entry.network : undefined, + timeoutMs: Number.isFinite(stageTimeout) && stageTimeout > 0 ? Math.min(stageTimeout, MAX_TIMEOUT_MS) : undefined, + }; + } + } + } + return cfg; +} + +// Load the effective sandbox config from .rstack/rstack.config.json. A malformed +// file yields the (safe) defaults with a stderr note — mirrors +// loadProjectGuardrails so the two never disagree on config-read semantics. +export async function loadSandboxConfig(projectRoot) { + const configPath = join(projectRoot, '.rstack', 'rstack.config.json'); + if (!existsSync(configPath)) return resolveSandboxConfig(); + try { + const parsed = JSON.parse(await readFile(configPath, 'utf8')); + return resolveSandboxConfig(parsed?.sandbox || {}); + } catch (error) { + if (error instanceof SyntaxError) { + console.error(`[rstack] Ignoring malformed ${configPath} for sandbox config: ${error.message}. Sandbox defaults apply.`); + return resolveSandboxConfig(); + } + throw error; + } +} + +// AUTHORITATIVE command resolution. The command executed for the gate MUST come +// from a trusted source — a plan-time task field or project config — NEVER from +// the untrusted builder's self-reported `tests_run`. If the builder chose the +// command it would just declare `tests_run: ["true"]` and mint itself a green, +// defeating the whole point of running the code. Returns { command, image, +// network, timeoutMs, limits } or null when nothing authoritative is configured +// (→ honest `observed` degrade at the gate, never a false PASS). +export function resolveSandboxCommand({ config = DEFAULT_SANDBOX_CONFIG, stageIds = [], task = {} } = {}) { + // 1. Plan-time task command (written by the planner, not the builder). + const taskCommand = typeof task?.test_command === 'string' && task.test_command.trim() ? task.test_command.trim() : null; + if (taskCommand) { + return { command: taskCommand, image: config.image, network: config.network, timeoutMs: config.timeoutMs, limits: config.limits }; + } + // 2. Per-stage configured command (first matching canonical stage). + for (const stageId of stageIds) { + const entry = config.perStage?.[stageId]; + if (entry?.command) { + return { + command: entry.command, + image: entry.image ?? config.image, + network: entry.network ?? config.network, + timeoutMs: entry.timeoutMs ?? config.timeoutMs, + limits: config.limits, + }; + } + } + // 3. Project-global configured command. + if (config.command) { + return { command: config.command, image: config.image, network: config.network, timeoutMs: config.timeoutMs, limits: config.limits }; + } + return null; +} + +// Map a runInSandbox evidence record → a validation.json check entry, shaped so +// priorCritiqueBlock (#451) can surface the REAL captured output to the next +// builder attempt. On FAIL the evidence IS the actual logs (per the posture: +// "the Evidence must be the actual log output, not a summary"), never a +// paraphrase. +export function executionCheck(record, command) { + if (!record || record.tier === 'unverified' || record.status === 'observed') { + return { + name: 'sandbox_execution', + status: 'WARN', + evidence: `${record?.evidence ?? 'execution unverified'} — gate fell back to contract validation (command: ${command ?? 'n/a'})`, + }; + } + if (record.status === 'PASS') { + return { + name: 'sandbox_execution', + status: 'PASS', + evidence: `sandboxed ${record.tier} run passed: ${record.evidence} — \`${command}\``, + }; + } + const logs = [record.stderr_tail, record.stdout_tail] + .map((section) => String(section ?? '').trim()) + .filter(Boolean) + .join('\n'); + const failure = record.exit_code === null ? 'timed out' : `exit ${record.exit_code}`; + return { + name: 'sandbox_execution', + status: 'FAIL', + root_cause: `sandboxed ${record.tier} run ${failure}`, + evidence: logs + ? `Command \`${command}\` ${failure} in the ${record.tier} sandbox. Actual output:\n${logs}` + : `Command \`${command}\` ${failure} in the ${record.tier} sandbox (no output captured).`, + remediation: 'Fix the failing command shown above — these are the errors from a real sandboxed run, not a self-report.', + }; +} From 7ac567d6e84e1efad8112d8d57e51a73f2da388a Mon Sep 17 00:00:00 2001 From: Richardson Gunde Date: Wed, 22 Jul 2026 12:36:01 +0530 Subject: [PATCH 2/4] feat(#452): run the sandbox in sdlc_validate + feed the critic (#451) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Wire The Scientist into the gate: - runValidationExecution runs the authoritative command in the transient container and returns a validation check + raw execution record. - sdlc_validate folds the REAL exit code into the verdict, replacing the builder's self-reported tests_run signal: FAIL fails validation (its evidence carrying the actual logs), PASS upgrades testsRunOk to observed truth, no runtime / no command degrades to a WARN (never a false green). Runs inside the tasks.json lock (validation is the attempt's serialization point). - The execution record is persisted into validation.json and pinned as an execution_recorded event (tier + exit code). - priorCritiqueBlock now surfaces the failed run's REAL captured output at the top of the retry prompt — the Scientist feeding the Critic — bounded so a huge tail can't dominate, with the duplicate log-dump bullet dropped. Co-Authored-By: Claude Opus 4.8 --- src/integrations/pi/rstack-sdlc.ts | 118 +++++++++++++++++++++++++++-- 1 file changed, 111 insertions(+), 7 deletions(-) diff --git a/src/integrations/pi/rstack-sdlc.ts b/src/integrations/pi/rstack-sdlc.ts index 3f2c3312..2f077dba 100644 --- a/src/integrations/pi/rstack-sdlc.ts +++ b/src/integrations/pi/rstack-sdlc.ts @@ -26,6 +26,10 @@ import { auditRunApprovals, approvalAuditEvent, isSafeArtifactName, trustedAppro import { classifyRetryDecision } from "../../core/harness/retry-policy.js"; import { classifyDestructiveAction, requireApprovalForDestructiveAction, destructiveApprovalArtifact } from "../../core/harness/destructive-actions.js"; import { extractBuilderTelemetry, builderTelemetryEvents, telemetryMetricsUpdate, builderContractKey } from "../../core/harness/telemetry.js"; +// #452 (The Scientist): run the authoritative test command in a transient, +// locked-down container and author execution evidence from the REAL exit code, +// replacing the builder's self-reported tests_run signal at the validate gate. +import { runInSandbox, loadSandboxConfig, resolveSandboxCommand, executionCheck } from "../../core/harness/sandbox.js"; // #136 (BLE-6.2): context-pressure classifier — detects oversized context at // validate time and appends non-blocking context_pressure_warning events. import { classifyContextPressure, loadProjectContextPressureThresholds } from "../../core/harness/context-pressure.js"; @@ -1082,25 +1086,82 @@ export async function priorCritiqueBlock(projectRoot: string, task: any): Promis : Array.isArray(prior.checks) ? prior.checks.filter((c: any) => String(c?.status ?? "").toUpperCase() === "FAIL") : []; - if (!failed.length && !prior.retry_recommendation) return ""; - const notes = failed.slice(0, 12).map((c: any) => { + // #452 — THE SCIENTIST FEEDS THE CRITIC. If the previous attempt's sandboxed + // execution FAILED, surface the REAL captured output at the TOP of the + // critique (the container's actual stderr/stdout, not a paraphrase) so the + // retrying builder sees exactly what broke. Bounded here so a huge tail can't + // dominate the prompt; validation.json keeps the full tail. Rendered from the + // structured `execution` record, so the corresponding sandbox_execution + // bullet is dropped from the list below to avoid duplicating the log dump. + const exec = prior.execution; + let executionSection = ""; + if (exec && String(exec.status ?? "").toUpperCase() === "FAIL") { + const logs = [exec.stderr_tail, exec.stdout_tail] + .map((section: any) => String(section ?? "").trim()) + .filter(Boolean) + .join("\n"); + const boundedLogs = logs.length > 2000 ? `…(${logs.length - 2000} earlier chars omitted)\n${logs.slice(-2000)}` : logs; + const failure = exec.exit_code === null || exec.exit_code === undefined ? "timed out" : `exit ${exec.exit_code}`; + executionSection = [ + `### 🧪 Sandboxed execution FAILED (${exec.tier ?? "container"}, ${failure})`, + "The harness ran your code in an isolated container — this is the REAL result, not a self-report. Make this pass:", + boundedLogs ? "```\n" + boundedLogs + "\n```" : "(no output captured)", + "", + ].join("\n"); + } + if (!failed.length && !prior.retry_recommendation && !executionSection) return ""; + const failedForNotes = failed.filter((c: any) => c?.name !== "sandbox_execution"); + const notes = failedForNotes.slice(0, 12).map((c: any) => { const name = c?.name ?? "check"; - const why = c?.evidence ?? c?.root_cause ?? "failed"; + const rawWhy = String(c?.evidence ?? c?.root_cause ?? "failed"); + const why = rawWhy.length > 600 ? `${rawWhy.slice(0, 600)}…` : rawWhy; const fix = c?.remediation ? ` · fix: ${c.remediation}` : ""; return `- **${name}** — ${why}${fix}`; }); - const more = failed.length > 12 ? `\n- …and ${failed.length - 12} more (see validation.json).` : ""; + const more = failedForNotes.length > 12 ? `\n- …and ${failedForNotes.length - 12} more (see validation.json).` : ""; return [ "## ⚠ Validator critique — your PREVIOUS attempt FAILED. Fix these FIRST.", `Verdict: ${status}. Recommended action: ${prior.retry_recommendation ?? "retry_builder"}.`, + executionSection, "Address every item below before re-attempting; do not repeat the prior approach:", notes.join("\n") + more, - ].join("\n"); + ].filter(Boolean).join("\n"); } catch { return ""; } } +// #452 PR2: run the AUTHORITATIVE test command in the transient sandbox and +// return a validation check (+ the raw execution record for validation.json). +// The command is trusted (task.test_command written by the planner, or the +// project's sandbox config) — NEVER the builder's self-reported tests_run, or a +// builder could declare `tests_run:["true"]` and mint its own green. Degrades to +// a WARN (never a false PASS) when the sandbox is disabled, no command is +// configured, or no container runtime is present. Exported so tests can inject a +// fake spawn/runtime without a real daemon. +export async function runValidationExecution( + { projectRoot, task, stageIds = [], config, deps = {} }: + { projectRoot: string; task: any; stageIds?: string[]; config?: any; deps?: any; }, +): Promise<{ record: any | null; check: any }> { + const sandboxCfg = config ?? await loadSandboxConfig(projectRoot); + if (!sandboxCfg.enabled) { + return { record: null, check: { name: "sandbox_execution", status: "WARN", evidence: "sandbox execution disabled in .rstack/rstack.config.json — tests are self-reported only, not verified" } }; + } + const resolved = resolveSandboxCommand({ config: sandboxCfg, stageIds, task }); + if (!resolved) { + return { record: null, check: { name: "sandbox_execution", status: "WARN", evidence: "no authoritative sandbox command configured (rstack.config.json sandbox.command / sandbox.per_stage, or task.test_command) — execution not verified; self-reported tests_run only" } }; + } + const record = await runInSandbox(projectRoot, { + taskId: task.id, + command: resolved.command, + network: resolved.network, + timeoutMs: resolved.timeoutMs, + image: resolved.image, + limits: resolved.limits, + }, deps); + return { record, check: executionCheck(record, resolved.command) }; +} + export async function builderPrompt(projectRoot: string, task: any, selected: RegistryItem[], runId?: string): Promise { const core = await coreAgentContext(projectRoot); const specialists = await agentContext(projectRoot, selected); @@ -2431,7 +2492,7 @@ export default function (pi: ExtensionAPI) { const tasksPath = join(runsDir(projectRoot), manifest.run_id, "tasks.json"); // Locked read-modify-write: the verdict stamp must not race a concurrent // sdlc_build_next claim on another task (issue #81). - const { task, taskState, checks, status, builderContract, validation, telemetryViolations, retryDecision, claimError } = await withFileLock(tasksPath, async () => { + const { task, taskState, checks, status, builderContract, validation, telemetryViolations, retryDecision, claimError, executionRecord } = await withFileLock(tasksPath, async () => { const taskState = JSON.parse(await readFile(tasksPath, "utf8")); const task = params.task_id ? taskState.tasks.find((t: any) => t.id === params.task_id) : taskState.tasks.find((t: any) => t.status === "IN_PROGRESS"); // #266: no selectable task is an ordinary run state — every FAIL @@ -2461,6 +2522,7 @@ export default function (pi: ExtensionAPI) { let status = "PASS"; let builderContract: any = undefined; let telemetryViolations: any[] = []; + let executionRecord: any = null; // #452: raw sandbox execution evidence // Signals for the mechanical required_checks evaluation (#222) — // captured from the gates below so the evaluator never re-derives // (and never drifts from) what this function already decided. @@ -2532,6 +2594,31 @@ export default function (pi: ExtensionAPI) { checks.push({ name: "builder_contract_json", status: "FAIL", evidence: String(error) }); } } + // #452 PR2 — THE SCIENTIST. Run the authoritative test command in the + // transient container and fold the REAL exit code into the verdict, + // upgrading the builder's self-reported tests_run signal to observed + // truth. A FAIL fails validation and its evidence carries the actual + // captured logs — which priorCritiqueBlock (#451) then feeds to the next + // builder attempt. A missing runtime / unconfigured command degrades to + // a WARN (never a false PASS). NOTE: this runs INSIDE the tasks.json lock + // — validation is the attempt's serialization point, so a real test run + // holds the lock for its bounded timeout and concurrent claims on other + // tasks wait; the gate's correctness (one verdict per claimed attempt) + // outranks parallel-validate throughput. Only runs when a builder + // contract was actually parsed. + if (builderContract) { + try { + const exec = await runValidationExecution({ projectRoot, task, stageIds: taskStageIds(task) }); + checks.push(exec.check); + if (exec.check.status === "FAIL") { status = "FAIL"; requiredSignals.testsRunOk = false; } + else if (exec.check.status === "PASS") { requiredSignals.testsRunOk = true; } + executionRecord = exec.record; + } catch (execError) { + // A sandbox wiring fault must never crash validation — record it as a + // WARN and fall back to the self-reported signal already computed. + checks.push({ name: "sandbox_execution", status: "WARN", evidence: `sandbox execution error: ${String((execError as any)?.message ?? execError)} — self-reported tests_run only` }); + } + } // Select the stage-specific validator profile (#120). Task ids are plan // ids — the profile keys off the task's canonical stage targets, picking // the highest-priority registered stage (or the generic profile). @@ -2642,6 +2729,10 @@ export default function (pi: ExtensionAPI) { status, checks, issues: checks.filter((c: any) => c.status === "FAIL"), + // #452: the raw execution record (real exit code + full captured log + // tail). Kept in the contract so priorCritiqueBlock can surface the + // actual output to the retry, and doctor/dashboard can read the tier. + execution: executionRecord, independence, // An independence-only failure is not the builder's fault — route it // to the policy's escalation (ask_user | block) instead of burning a @@ -2673,7 +2764,7 @@ export default function (pi: ExtensionAPI) { // without going back through the gates. delete task._claim; await writeJsonAtomic(tasksPath, taskState); - return { task, taskState, checks, status, builderContract, validation, telemetryViolations, retryDecision }; + return { task, taskState, checks, status, builderContract, validation, telemetryViolations, retryDecision, executionRecord }; }); if (claimError) { // #405: the requested task exists but is not the actively claimed @@ -2720,6 +2811,19 @@ export default function (pi: ExtensionAPI) { if (canonicalStageIds.length === 0 && getCanonicalStage(task.id)) canonicalStageIds.push(task.id); await appendEvent(projectRoot, manifest.run_id, { type: "task_validated", task_id: task.id, status }); + // #452: pin the real execution outcome (tier + exit code) so the ledger, + // dashboard, and doctor can distinguish a container-verified run from a + // contract-only (unverified) one. Only when the sandbox actually ran. + if (executionRecord) { + await appendEvent(projectRoot, manifest.run_id, { + type: "execution_recorded", + task_id: task.id, + tier: executionRecord.tier, + status: executionRecord.status, + exit_code: executionRecord.exit_code, + duration_ms: executionRecord.duration_ms, + }); + } for (const violation of telemetryViolations) { await appendEvent(projectRoot, manifest.run_id, guardrailEvent(task.id, violation)); } From b65f6603300e2bd6841d023d3dd441a7a7c199d1 Mon Sep 17 00:00:00 2001 From: Richardson Gunde Date: Wed, 22 Jul 2026 12:36:09 +0530 Subject: [PATCH 3/4] test(#452): PR2 sandbox-in-validate coverage + lint fix MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - New tests/sandbox-validate-452.test.js: config resolution, authoritative command priority (never builder tests_run), execution→check mapping with real logs on FAIL, runValidationExecution wiring via injected spawn/runtime (FAIL/ PASS/no-runtime/disabled/no-command), the Scientist→Critic end-to-end through priorCritiqueBlock, and config validation. - Switch the fake child's setImmediate → setTimeout(…,0) here and in the PR1 file: setImmediate isn't in the eslint globals allow-list (a latent lint error that slipped through PR1). Co-Authored-By: Claude Opus 4.8 --- tests/sandbox-execution-452.test.js | 4 +- tests/sandbox-validate-452.test.js | 216 ++++++++++++++++++++++++++++ 2 files changed, 218 insertions(+), 2 deletions(-) create mode 100644 tests/sandbox-validate-452.test.js diff --git a/tests/sandbox-execution-452.test.js b/tests/sandbox-execution-452.test.js index eee3e936..4c3ba13b 100644 --- a/tests/sandbox-execution-452.test.js +++ b/tests/sandbox-execution-452.test.js @@ -23,11 +23,11 @@ function fakeChild({ code = 0, stdout = '', stderr = '', hang = false } = {}) { child.stderr = new EventEmitter(); child.kill = () => { child.killed = true; child.emit('close', null); }; if (!hang) { - setImmediate(() => { + setTimeout(() => { if (stdout) child.stdout.emit('data', Buffer.from(stdout)); if (stderr) child.stderr.emit('data', Buffer.from(stderr)); child.emit('close', code); - }); + }, 0); } return child; } diff --git a/tests/sandbox-validate-452.test.js b/tests/sandbox-validate-452.test.js new file mode 100644 index 00000000..d6be0b14 --- /dev/null +++ b/tests/sandbox-validate-452.test.js @@ -0,0 +1,216 @@ +/** + * Transient Sandbox — "The Scientist" (#452), PR 2: wiring into the validate + * gate + feeding the Critic (#451). + * + * Covers the pure seams (config resolution, AUTHORITATIVE command resolution, + * execution→check mapping) and the exported runValidationExecution wiring with + * an injected spawn/runtime so CI needs no container daemon. Also proves the + * Scientist→Critic loop: a failed sandbox run's REAL logs surface in + * priorCritiqueBlock. + * + * owner: RStack developed by Richardson Gunde + */ + +import test from 'node:test'; +import assert from 'node:assert/strict'; +import { EventEmitter } from 'node:events'; +import { mkdtempSync, mkdirSync, writeFileSync } from 'node:fs'; +import { tmpdir } from 'node:os'; +import { join } from 'node:path'; + +import { + DEFAULT_SANDBOX_CONFIG, + resolveSandboxConfig, + resolveSandboxCommand, + executionCheck, +} from '../src/core/harness/sandbox.js'; +import { runValidationExecution, priorCritiqueBlock } from '../src/integrations/pi/rstack-sdlc.ts'; +import { validateSandboxConfig } from '../src/core/harness/config-validation.js'; + +function fakeChild({ code = 0, stdout = '', stderr = '', hang = false } = {}) { + const child = new EventEmitter(); + child.stdout = new EventEmitter(); + child.stderr = new EventEmitter(); + child.kill = () => { child.killed = true; child.emit('close', null); }; + if (!hang) { + setTimeout(() => { + if (stdout) child.stdout.emit('data', Buffer.from(stdout)); + if (stderr) child.stderr.emit('data', Buffer.from(stderr)); + child.emit('close', code); + }, 0); + } + return child; +} + +// --- resolveSandboxConfig --------------------------------------------------- + +test('resolveSandboxConfig merges valid fields and ignores bad-typed ones', () => { + const cfg = resolveSandboxConfig({ image: 'node:20-alpine', network: true, command: 'npm test', timeout_ms: 5000, enabled: 'nope' }); + assert.equal(cfg.image, 'node:20-alpine'); + assert.equal(cfg.network, true); + assert.equal(cfg.command, 'npm test'); + assert.equal(cfg.timeoutMs, 5000); + assert.equal(cfg.enabled, true, 'bad-typed enabled falls back to the safe default'); +}); + +test('resolveSandboxConfig defaults are safe (enabled, no network, shell image)', () => { + const cfg = resolveSandboxConfig(); + assert.equal(cfg.enabled, DEFAULT_SANDBOX_CONFIG.enabled); + assert.equal(cfg.network, false); + assert.equal(cfg.command, null); + assert.deepEqual(cfg.perStage, {}); +}); + +test('resolveSandboxConfig keeps only per-stage entries with a real command', () => { + const cfg = resolveSandboxConfig({ per_stage: { '07-code': { command: 'pytest -q', image: 'python:3.12-slim' }, '08-testing': { image: 'x' } } }); + assert.equal(cfg.perStage['07-code'].command, 'pytest -q'); + assert.equal(cfg.perStage['07-code'].image, 'python:3.12-slim'); + assert.equal(cfg.perStage['08-testing'], undefined, 'no command → dropped'); +}); + +// --- resolveSandboxCommand (authoritative only) ----------------------------- + +test('resolveSandboxCommand NEVER uses the builder self-report; only trusted sources', () => { + const config = resolveSandboxConfig({ command: 'npm test' }); + // A malicious builder trying to smuggle a trivially-green command via the + // contract has no field here — tests_run is not consulted at all. + const resolved = resolveSandboxCommand({ config, stageIds: [], task: { id: 't', tests_run: ['true'] } }); + assert.equal(resolved.command, 'npm test', 'global config command wins; builder tests_run ignored'); +}); + +test('resolveSandboxCommand priority: task.test_command > per_stage > global', () => { + const config = resolveSandboxConfig({ command: 'global-cmd', per_stage: { '07-code': { command: 'stage-cmd' } } }); + assert.equal(resolveSandboxCommand({ config, stageIds: ['07-code'], task: { test_command: 'task-cmd' } }).command, 'task-cmd'); + assert.equal(resolveSandboxCommand({ config, stageIds: ['07-code'], task: {} }).command, 'stage-cmd'); + assert.equal(resolveSandboxCommand({ config, stageIds: ['02-requirements'], task: {} }).command, 'global-cmd'); +}); + +test('resolveSandboxCommand returns null when nothing authoritative is configured', () => { + assert.equal(resolveSandboxCommand({ config: resolveSandboxConfig(), stageIds: ['07-code'], task: { tests_run: ['npm test'] } }), null); +}); + +// --- executionCheck mapping ------------------------------------------------- + +test('executionCheck maps PASS/FAIL/observed with real logs on FAIL', () => { + const pass = executionCheck({ status: 'PASS', tier: 'docker', evidence: 'exit 0 in 12ms', exit_code: 0 }, 'npm test'); + assert.equal(pass.status, 'PASS'); + + const fail = executionCheck({ status: 'FAIL', tier: 'docker', exit_code: 1, stderr_tail: 'AssertionError: nope', stdout_tail: '1 failing' }, 'npm test'); + assert.equal(fail.status, 'FAIL'); + assert.match(fail.evidence, /AssertionError: nope/, 'evidence is the ACTUAL log output, not a summary'); + assert.match(fail.evidence, /1 failing/); + assert.match(fail.root_cause, /exit 1/); + + const observed = executionCheck({ status: 'observed', tier: 'unverified', evidence: 'no container runtime available' }, 'npm test'); + assert.equal(observed.status, 'WARN', 'no runtime → WARN, never a false PASS'); +}); + +// --- runValidationExecution wiring (injected spawn/runtime) ------------------ + +test('runValidationExecution: FAIL exit → FAIL check carrying real logs (feeds the critic)', async () => { + const { record, check } = await runValidationExecution({ + projectRoot: '/proj', + task: { id: '07-code', test_command: 'pytest -q' }, + stageIds: ['07-code'], + config: resolveSandboxConfig({ command: 'pytest -q' }), + deps: { runtime: 'docker', containerName: 'rstack-sbx-test', spawn: () => fakeChild({ code: 1, stderr: 'E assert 1 == 2' }) }, + }); + assert.equal(check.status, 'FAIL'); + assert.match(check.evidence, /assert 1 == 2/); + assert.equal(record.status, 'FAIL'); + assert.equal(record.exit_code, 1); +}); + +test('runValidationExecution: PASS exit → PASS check', async () => { + const { check } = await runValidationExecution({ + projectRoot: '/proj', + task: { id: '07-code', test_command: 'npm test' }, + config: resolveSandboxConfig({ command: 'npm test' }), + deps: { runtime: 'docker', spawn: () => fakeChild({ code: 0, stdout: '5 passed' }) }, + }); + assert.equal(check.status, 'PASS'); +}); + +test('runValidationExecution: no runtime → WARN, no false green', async () => { + const { record, check } = await runValidationExecution({ + projectRoot: '/proj', + task: { id: '07-code', test_command: 'npm test' }, + config: resolveSandboxConfig({ command: 'npm test' }), + deps: { probe: () => false }, // no docker/podman + }); + assert.equal(check.status, 'WARN'); + assert.equal(record.tier, 'unverified'); +}); + +test('runValidationExecution: disabled → WARN, sandbox never spawned', async () => { + let spawned = false; + const { check } = await runValidationExecution({ + projectRoot: '/proj', + task: { id: '07-code', test_command: 'npm test' }, + config: resolveSandboxConfig({ enabled: false, command: 'npm test' }), + deps: { runtime: 'docker', spawn: () => { spawned = true; return fakeChild({}); } }, + }); + assert.equal(check.status, 'WARN'); + assert.equal(spawned, false, 'disabled config must not execute anything'); +}); + +test('runValidationExecution: no authoritative command → WARN (never runs builder-chosen cmd)', async () => { + let spawned = false; + const { check } = await runValidationExecution({ + projectRoot: '/proj', + task: { id: '07-code', tests_run: ['rm -rf /'] }, // builder self-report — must be ignored + config: resolveSandboxConfig(), + deps: { runtime: 'docker', spawn: () => { spawned = true; return fakeChild({}); } }, + }); + assert.equal(check.status, 'WARN'); + assert.equal(spawned, false, 'no authoritative command → nothing executes'); +}); + +// --- Scientist → Critic (#451) end-to-end ----------------------------------- + +test('priorCritiqueBlock surfaces the sandboxed execution logs at the top', async () => { + const dir = mkdtempSync(join(tmpdir(), 'rstack-critique-')); + const outputDir = join('out'); + mkdirSync(join(dir, outputDir), { recursive: true }); + writeFileSync(join(dir, outputDir, 'validation.json'), JSON.stringify({ + task_id: '07-code', + status: 'FAIL', + retry_recommendation: 'retry_builder', + execution: { status: 'FAIL', tier: 'docker', exit_code: 1, stderr_tail: 'E assert add(2,2) == 5', stdout_tail: '1 failed, 0 passed' }, + issues: [{ name: 'sandbox_execution', status: 'FAIL', evidence: 'Command `pytest` exit 1 ...long dump...' }], + checks: [], + })); + const block = await priorCritiqueBlock(dir, { output_dir: outputDir }); + assert.match(block, /Sandboxed execution FAILED/); + assert.match(block, /assert add\(2,2\) == 5/, 'REAL stderr reaches the retrying builder'); + assert.match(block, /1 failed, 0 passed/); + // The full log dump is rendered once (from the structured record), not also + // duplicated as a sandbox_execution bullet. + assert.equal((block.match(/assert add\(2,2\) == 5/g) || []).length, 1); +}); + +// --- config validation ------------------------------------------------------ + +test('validateSandboxConfig flags unknown keys, bad types, and typo\'d stages', () => { + const issues = validateSandboxConfig({ + enabled: 'yes', + network: 1, + command: '', + bogus: true, + per_stage: { 'not-a-stage': { command: 'x' }, '07-code': { image: 'x' } }, + }); + const fields = issues.map((i) => i.field); + assert.ok(fields.includes('sandbox.enabled')); + assert.ok(fields.includes('sandbox.network')); + assert.ok(fields.includes('sandbox.command')); + assert.ok(fields.includes('sandbox.bogus')); + assert.ok(fields.includes('sandbox.per_stage.not-a-stage')); + assert.ok(fields.includes('sandbox.per_stage.07-code'), 'per-stage entry with no command is flagged'); +}); + +test('validateSandboxConfig passes a well-formed block', () => { + assert.deepEqual( + validateSandboxConfig({ enabled: true, image: 'node:20-alpine', command: 'npm test', network: false, timeout_ms: 60000, per_stage: { '07-code': { command: 'npm test' } } }), + [], + ); +}); From e90e1a9cf99ec85a556b423742c7178374f54427 Mon Sep 17 00:00:00 2001 From: Richardson Gunde Date: Wed, 22 Jul 2026 12:54:55 +0530 Subject: [PATCH 4/4] fix(#452): name bad sandbox.limits sub-fields + over-cap timeout (CodeRabbit #463) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit validateSandboxConfig only allowlisted the `limits` key and never flagged a timeout above the hard cap — contradicting its own goal of NAMING every silent failure. resolveSandboxConfig silently drops bad-typed limits and clamps oversized timeouts, so a typo like limits:{pids:"many"} or an over-cap timeout_ms passed validation clean and was quietly altered. Now each is named, against the single MAX_TIMEOUT_MS exported from the executor it bounds. Co-Authored-By: Claude Opus 4.8 --- src/core/harness/config-validation.js | 22 ++++++++++++++++++++++ src/core/harness/sandbox.js | 2 +- tests/sandbox-validate-452.test.js | 11 +++++++++++ 3 files changed, 34 insertions(+), 1 deletion(-) diff --git a/src/core/harness/config-validation.js b/src/core/harness/config-validation.js index ae39137c..d2ee12b8 100644 --- a/src/core/harness/config-validation.js +++ b/src/core/harness/config-validation.js @@ -14,6 +14,9 @@ import { DEFAULT_LOOP_BOUNDS, LOOP_HARD_CAP } from './goal-loop.js'; import { DEFAULT_CRITICAL_STAGE_IDS } from './checkpoints.js'; import { getCanonicalStage } from './stages.js'; import { rstackStateDir } from './runs.js'; +// #452: the sandbox timeout hard cap lives with the executor it bounds, so the +// config validator and the resolver clamp/name against ONE value. +import { MAX_TIMEOUT_MS } from './sandbox.js'; // #136 (BLE-6.2): context-pressure thresholds live in rstack.config.json under // `context_pressure`; validated field-by-field like every other block. import { validateContextPressureConfig } from './context-pressure.js'; @@ -156,6 +159,25 @@ export function validateSandboxConfig(sandbox) { const timeout = sandbox.timeout_ms ?? sandbox.timeoutMs; if (timeout != null && !isNonNegativeNumber(timeout)) { issues.push({ field: 'sandbox.timeout_ms', problem: `must be a non-negative number of milliseconds, got ${JSON.stringify(timeout)} — the default applies` }); + } else if (timeout != null && Number(timeout) > MAX_TIMEOUT_MS) { + issues.push({ field: 'sandbox.timeout_ms', problem: `exceeds the hard cap of ${MAX_TIMEOUT_MS}ms — it will be clamped to ${MAX_TIMEOUT_MS}ms` }); + } + // limits sub-fields: resolveSandboxConfig silently drops bad-typed values, so + // name them here (the whole point of this validator) instead of leaving a + // typo like { pids: "many" } to pass clean and be ignored downstream. + if (sandbox.limits != null) { + if (!isPlainObject(sandbox.limits)) { + issues.push({ field: 'sandbox.limits', problem: 'must be an object of { memory, pids, cpus }' }); + } else { + if (sandbox.limits.memory != null && (typeof sandbox.limits.memory !== 'string' || !sandbox.limits.memory.trim())) { + issues.push({ field: 'sandbox.limits.memory', problem: `must be a non-empty string (e.g. "512m"), got ${JSON.stringify(sandbox.limits.memory)} — the default applies` }); + } + for (const key of ['pids', 'cpus']) { + if (sandbox.limits[key] != null && !isNonNegativeNumber(sandbox.limits[key])) { + issues.push({ field: `sandbox.limits.${key}`, problem: `must be a non-negative number, got ${JSON.stringify(sandbox.limits[key])} — the default applies` }); + } + } + } } const perStage = sandbox.per_stage ?? sandbox.perStage; if (perStage != null) { diff --git a/src/core/harness/sandbox.js b/src/core/harness/sandbox.js index c4e455a0..d0d7e861 100644 --- a/src/core/harness/sandbox.js +++ b/src/core/harness/sandbox.js @@ -25,7 +25,7 @@ import { existsSync } from 'node:fs'; import { resolve, join } from 'node:path'; const DEFAULT_TIMEOUT_MS = 120_000; -const MAX_TIMEOUT_MS = 600_000; +export const MAX_TIMEOUT_MS = 600_000; const OUTPUT_TAIL_BYTES = 8 * 1024; // bounded like goal-check's maxBuffer precedent const RUNTIMES = ['docker', 'podman']; diff --git a/tests/sandbox-validate-452.test.js b/tests/sandbox-validate-452.test.js index d6be0b14..73f41591 100644 --- a/tests/sandbox-validate-452.test.js +++ b/tests/sandbox-validate-452.test.js @@ -208,6 +208,17 @@ test('validateSandboxConfig flags unknown keys, bad types, and typo\'d stages', assert.ok(fields.includes('sandbox.per_stage.07-code'), 'per-stage entry with no command is flagged'); }); +test('validateSandboxConfig names bad limits sub-fields and an over-cap timeout (CodeRabbit #463)', () => { + const issues = validateSandboxConfig({ timeout_ms: 999_999_999, limits: { memory: '', pids: 'many', cpus: -1 } }); + const fields = issues.map((i) => i.field); + assert.ok(fields.includes('sandbox.timeout_ms'), 'over-cap timeout is named, not silently clamped'); + assert.ok(fields.includes('sandbox.limits.memory')); + assert.ok(fields.includes('sandbox.limits.pids')); + assert.ok(fields.includes('sandbox.limits.cpus')); + // non-object limits is a single clear error + assert.deepEqual(validateSandboxConfig({ limits: [] }).map((i) => i.field), ['sandbox.limits']); +}); + test('validateSandboxConfig passes a well-formed block', () => { assert.deepEqual( validateSandboxConfig({ enabled: true, image: 'node:20-alpine', command: 'npm test', network: false, timeout_ms: 60000, per_stage: { '07-code': { command: 'npm test' } } }),