requesting-review: split the review job out of codex-companion, runtime fail-closed, engine script retired

.claude-plugin/marketplace.json

@@ -9,7 +9,7 @@
     {
       "name": "doperpowers",
       "description": "Emerges by humans, Converges by Agent",
-      "version": "7.46.6",
+      "version": "7.47.0",
       "source": "./",
       "author": {
         "name": "SSFSKIM",

.claude-plugin/plugin.json

@@ -1,7 +1,7 @@
 {
   "name": "doperpowers",
   "description": "Emerges by humans, Converges by Agents",
-  "version": "7.46.6",
+  "version": "7.47.0",
   "author": {
     "name": "SSFSKIM",
     "email": "supremekim17@gmail.com"

.codex-plugin/plugin.json

@@ -1,6 +1,6 @@
 {
   "name": "doperpowers",
-  "version": "7.46.6",
+  "version": "7.47.0",
   "description": "A two-track software-development methodology for coding agents: a human-gated controlled track (brainstorm, plan, TDD, review, ship) plus an autonomous board loop for unattended, well-scoped work.",
   "author": {
     "name": "SSFSKIM",

infra/worker-host/README.md

@@ -129,9 +129,11 @@ dogfood cell end-to-end on the VM and check, in order:
 
 1. `codex exec` smoke turn — auth.json accepted, Landlock sandbox active
    (kernel ≥5.13; Ubuntu 24.04 is 6.8).
-2. Nested-codex TLS — `SSL_CERT_FILE` resolves (`_codex_launch` and
-   `review-engine.sh` probe `/etc/ssl/certs/ca-certificates.crt` since
-   2026-07-12); a review-engine call from inside a worker completes.
+2. Nested-codex TLS — `SSL_CERT_FILE` resolves (`_codex_launch` and the
+   review worker's START ENGINE preamble in
+   `skills/reviewing-prs/SKILL.md` probe `/etc/ssl/certs/ca-certificates.crt`
+   since 2026-07-12); that preamble's review invocation, run from inside a
+   worker, completes.
 3. Board write under env `GH_TOKEN` — a `board-transition.sh` against a test
    issue; push from a worktree uses the remote-wired credential.
 4. Full cell: dispatch an implement worker on a toy ticket → PR → review

package.json

@@ -1,6 +1,6 @@
 {
   "name": "doperpowers",
-  "version": "7.46.6",
+  "version": "7.47.0",
   "description": "Doperpowers skills and runtime bootstrap for coding agents",
   "type": "module",
   "main": ".opencode/plugins/doperpowers.js",

skills/architecting/SKILL.md

@@ -96,8 +96,8 @@ parks carry the quality machinery.
     dispatch doperpowers:critique on the matured design and debate to
     convergence; run the spec-review pass; then give the implementation
     plan the same independent review doperpowers:writing-plans
-    prescribes — doperpowers:codex-companion's `adversarial-review` verb
-    (model `gpt-5.6-sol`, effort `xhigh` via its with-effort wrapper) in
+    prescribes — doperpowers:requesting-review's adversarial review
+    (model `gpt-5.6-sol`, effort `xhigh` via the with-effort wrapper) in
     a background Bash, focused on whether the plan is complete,
     spec-aligned, well-decomposed, and buildable by an engineer with
     zero context. Evaluate its findings rather than accepting them

skills/codex-companion/SKILL.md

@@ -48,12 +48,10 @@ land on stderr, so keep the file and check it only on a nonzero exit.
 
 Verbs, and where each is specified:
 
-- `review` — Codex's native code review of the working tree or a branch
-  (`--base <ref>`); non-steerable by design; reasoning effort is choosable
-  via the `scripts/with-effort.mjs` wrapper; big diffs (~20+ files) route
-  to the `workflow` code-review panel instead → references/reviews.md
-- `adversarial-review` — challenge review of design and assumptions;
-  trailing text is a lens, parallel lenses for big diffs → references/reviews.md
+- `review` / `adversarial-review` — the independent-review job, owned by
+  the sibling skill doperpowers:requesting-review: routing between a
+  single review, an adversarial challenge, and the big-diff code-review
+  panel, plus every invocation and output contract, live there
 - `task` — any prompt to a resumable Codex thread; one-shot delegation
   or a standing multi-turn partner (critique debates, steered execution);
   read-only unless `--write` → references/amigo.md

skills/codex-companion/references/workflows.md

@@ -52,28 +52,9 @@ was the last guard standing.
 ## The bundled code-review panel
 
 `<skill-base>/workflows/code-review.mjs` is a ready-made review workflow —
-the destination references/reviews.md routes big diffs to. One lens-free
-native sweep, up to five scalpel lenses a deriver reads off the diff, and
-one binding verifier over the merged candidate pool:
-
-    … workflow --script "<skill-base>/workflows/code-review.mjs" \
-      --args '{"base":"main"}' --cwd <repo> 2> <scratch>.events.log
-
-`base` is the only required arg. Optional: `lenses` (an array replacing the
-derived set), `finderModel`/`finderEffort` (default `gpt-5.6-sol`/`xhigh`),
-`verifierModel`/`verifierEffort` (default `gpt-5.6-sol`/`high`).
-
-The verb's stdout `result` is `{verdict, findings, coverage, lenses,
-explanation}`. `verdict` is `correct`, `incorrect`, or `interrupted`;
-`findings` carries only verifier-confirmed items —
-`{id, priority, title, file, lines, comment, sources}`, priority-sorted —
-so `incorrect` means confirmed defects, not raw candidates. `interrupted`
-means no verdict about this diff can be asserted: a lane was lost (sweep or
-verifier down, so a clean claim would be hollow) or the target moved — the
-panel pins `merge-base` AND `HEAD` at start and re-resolves both at
-assembly, so a commit landing on the reviewed branch mid-run withholds the
-verdict, with any findings attached as partial evidence. Don't commit to
-the branch under review while a round is in flight.
+one lens-free native sweep, up to five diff-derived scalpel lenses, and one
+binding verifier. When to run it, its invocation, and its result contract
+are owned by the sibling skill doperpowers:requesting-review.
 
 ## The script
 
@@ -124,7 +105,7 @@ Hooks:
 - `review({base, scope, model, effort, lens, label, cwd})` →
   `{reviewText, threadId, status}`. Target selection matches the `review` verb
   (`base`, or `scope` of `auto`/`working-tree`/`branch` — see
-  references/reviews.md). This is Codex's native reviewer, so `lens` is not a
+  doperpowers:requesting-review). This is Codex's native reviewer, so `lens` is not a
   prompt: it rides `developer_instructions` on that worker's private
   app-server, alongside the review protocol rather than replacing it. Keep a
   lens to at most two plain sentences; longer mandates compete with the

skills/codex-companion/runtime/scripts/codex-companion.mjs

@@ -23,6 +23,7 @@ import {
     runAppServerTurn
   } from "./lib/codex.mjs";
 import { resolveClaudeSessionPath } from "./lib/claude-session-transfer.mjs";
+import { assertSandboxUsable } from "./lib/sandbox.mjs";
 import { readStdinIfPiped } from "./lib/fs.mjs";
 import { collectReviewContext, ensureGitRepository, resolveReviewTarget } from "./lib/git.mjs";
 import { pidInstanceVerified, processStartTime } from "./lib/pid.mjs";
@@ -379,6 +380,11 @@ async function executeReviewRun(request) {
       model: request.model,
       onProgress: request.onProgress
     });
+    // Fail closed BEFORE rendering: a review whose fs sandbox never worked
+    // renders hollow findings (see lib/sandbox.mjs for the observed incident).
+    assertSandboxUsable(reviewName, result.stderr, {
+      onDiagnostic: (line) => process.stderr.write(`[codex] sandbox-diagnostic: ${line}\n`)
+    });
     const payload = {
       review: reviewName,
       target,
@@ -422,6 +428,10 @@ async function executeReviewRun(request) {
     outputSchema: readOutputSchema(REVIEW_SCHEMA),
     onProgress: request.onProgress
   });
+  // Same fail-closed guard as the native branch: no verdict from a broken sandbox.
+  assertSandboxUsable(reviewName, result.stderr, {
+    onDiagnostic: (line) => process.stderr.write(`[codex] sandbox-diagnostic: ${line}\n`)
+  });
   const parsed = parseStructuredOutput(result.finalMessage, {
     status: result.status,
     failureMessage: result.error?.message ?? result.stderr

skills/codex-companion/runtime/scripts/lib/sandbox.mjs

@@ -0,0 +1,34 @@
+// Fail-closed guard for the codex fs sandbox. Observed live (ida-worker-1,
+// 2026-08-09): a host that blocks unprivileged userns makes every probe fail
+// (`bwrap: loopback: Failed RTM_NEWADDR`), yet codex exits 0 and renders
+// findings anyway — 22 consecutive review runs passed as "clean". The
+// app-server child's buffered stderr is the machine-emitted trace of that
+// state, so its markers turn the run into a hard error instead of a verdict.
+// Only that channel is scanned: model-authored channels (final message,
+// review text, streamed progress) can quote marker strings innocently.
+export const SANDBOX_FAILURE_MARKERS = /RTM_NEWADDR|shell is unavailable|fs sandbox helper failed/;
+
+export function sandboxFailureLines(stderr) {
+  if (!stderr) return [];
+  return String(stderr).split("\n").filter((line) => SANDBOX_FAILURE_MARKERS.test(line));
+}
+
+// Reports every hit through onDiagnostic first (so journaling survives the
+// throw), then throws terminally — a blocked sandbox does not heal between
+// retries, so a transport retry would spend a full turn to fail identically.
+// Under an OUTER codex sandbox (CODEX_SANDBOX set) probe confinement is
+// expected and the degraded diff-only render is documented behavior: report
+// only, never throw.
+export function assertSandboxUsable(label, stderr, { onDiagnostic } = {}) {
+  const hits = sandboxFailureLines(stderr);
+  if (hits.length === 0) return;
+  for (const line of hits) onDiagnostic?.(line);
+  if (process.env.CODEX_SANDBOX) return;
+  throw Object.assign(
+    new Error(
+      `fs sandbox unavailable during ${label ?? "codex run"}: ${hits[0].trim()} — ` +
+      "findings would be untrustworthy; treat as an engine failure"
+    ),
+    { terminal: true }
+  );
+}

skills/codex-companion/runtime/scripts/lib/workflow/engine.mjs

@@ -7,6 +7,7 @@ import fs from "node:fs";
 import path from "node:path";
 import { pathToFileURL } from "node:url";
 import { runAppServerTurn, runAppServerReview, parseStructuredOutput, resolveReviewTarget } from "../codex.mjs";
+import { assertSandboxUsable, sandboxFailureLines } from "../sandbox.mjs";
 import { processStartTime } from "../pid.mjs";
 import { validateSchema } from "./validate.mjs";
 import {
@@ -181,6 +182,18 @@ export async function runWorkflow(spec) {
           out = await attempt();
         } catch (e1) {
           if (e1?.terminal) throw e1;            // schema-repair exhaustion etc: NEVER a third attempt
+          // A dead app-server's protocol error carries the child's buffered
+          // stderr — forward sandbox markers from it too, or a dying worker's
+          // diagnostics never reach the consumers' fail-closed guards.
+          // Diagnostic ONLY, never guardSandbox: unlike the *.stderr sites, an
+          // error message is not exclusively the machine channel — a rejected
+          // request quotes back the command the model asked to run, so a leaf
+          // reviewing this repo can put a marker here by reading its own test
+          // fixtures. Observed 2026-08-11: a command-policy rejection quoting
+          // `bwrap: … RTM_NEWADDR` out of tests/codex-companion/ killed a live
+          // review lane whose sandbox was fine. Fail closed only where the text
+          // cannot be model-authored.
+          emitSandboxDiagnostics(label, String(e1?.message ?? e1));
           appendEvent(journalPath, { type: "retry", key, error: String(e1?.message ?? e1) });
           emit(`retry ${kind}:${label ?? ""}`);
           out = await attempt();                 // one automatic transport retry, fresh turn
@@ -189,6 +202,7 @@ export async function runWorkflow(spec) {
         emit(`done ${kind}:${label ?? ""}`);
         return out;
       } catch (err) {
+        emitSandboxDiagnostics(label, String(err?.message ?? err));
         appendEvent(journalPath, { type: "finished", key, error: String(err?.message ?? err) });
         emit(`fail ${kind}:${label ?? ""}`);
         throw err;
@@ -243,6 +257,22 @@ export async function runWorkflow(spec) {
       return { ...stamped, uncacheable: runNonce };
     };
 
+    // The app-server buffers its child's stderr and surfaces it only on a
+    // nonzero exit, but the observed fs-sandbox failure (bwrap RTM_NEWADDR —
+    // ida-worker-1, 2026-08-09) exits 0 and renders findings anyway. Every
+    // leaf therefore fails closed on that buffered stderr (lib/sandbox.mjs) —
+    // checked BEFORE the leaf's own success check, so a "successful" turn
+    // whose sandbox never worked becomes a terminal leaf failure instead of a
+    // false-clean result. In the panel that failure surfaces as a lost lane,
+    // i.e. an `interrupted` verdict, never `correct`.
+    const emitSandboxDiagnostics = (label, text) => {
+      for (const line of sandboxFailureLines(text)) emit(`sandbox-diagnostic ${label ?? ""}: ${line}`);
+    };
+    const guardSandbox = (label, stderr) =>
+      assertSandboxUsable(label, stderr, {
+        onDiagnostic: (line) => emit(`sandbox-diagnostic ${label ?? ""}: ${line}`)
+      });
+
     const hooks = {
       args: spec.args,
       log: (m) => { appendEvent(journalPath, { type: "log", message: String(m) }); emit(`log ${m}`); },
@@ -256,6 +286,7 @@ export async function runWorkflow(spec) {
             sandbox: "read-only", persistThread: true,
             outputSchema: opts.schema ?? null, connect
           });
+          guardSandbox(opts.label, turn.stderr);
           assertTurnUsable(turn, "agent turn");
           if (!opts.schema) {
             if (!turn.finalMessage?.trim()) throw new Error("agent turn returned no output");
@@ -270,6 +301,7 @@ export async function runWorkflow(spec) {
             model: opts.model, effort: opts.effort, sandbox: "read-only",
             outputSchema: opts.schema, connect
           });
+          guardSandbox(opts.label, repair.stderr);
           assertTurnUsable(repair, "schema repair turn");
           parsed = parseStructuredOutput(repair.finalMessage);
           errors = parsed.parseError ? [parsed.parseError] : validateSchema(parsed.parsed, opts.schema);
@@ -316,6 +348,7 @@ export async function runWorkflow(spec) {
             model: opts.model, target,
             connect: { disableBroker: true, configOverrides: overrides, onSpawn }
           });
+          guardSandbox(opts.label, res.stderr);
           assertTurnUsable(res, "review");
           if (!res.reviewText?.trim()) throw new Error("review returned no output");
           // review/start takes the SYMBOLIC target — it has no commit parameter —