From 10c04ba8475eaf042e368f41795c40e9bdcf88a8 Mon Sep 17 00:00:00 2001 From: hamr0 Date: Thu, 23 Jul 2026 18:50:23 +0200 Subject: [PATCH 1/7] F49: reject nested-quantifier ReDoS patterns at the plan validation gate The agent-authored artifact-written `pattern` was compiled but not screened for catastrophic backtracking; evalExits runs it with no wall-clock bound, so a nested unbounded quantifier ((a+)+, (\d*)*, (x+){1,}) could hang the exit evaluator. LOW self-DoS (agent authors both pattern and body; no arbiter compromise), but a real hang: measured (a+)+$ NOT finishing on a 33-char body in 120s. Fix at the validation gate (before any tokens): plan.js runs a vanilla hasNestedQuantifier() state-machine scan after the compile check and reds invalid-value on the exponential class. Skips escaped atoms and character classes; bounded outer repeats stay legal. Input-bounding rejected as theater (the blowup needs only tens of chars); a JS regex timeout as disproportionate (worker_threads/RE2 dep) for a LOW issue. TDD: 12-bad/17-good detector battery + validator red-case + detail-names-the- footgun test. 555/555, typecheck + build clean. Co-Authored-By: Claude Opus 4.8 (1M context) --- src/plan.js | 68 +++++++++++++++++++++++++++++++++++++++++++++- tests/plan.test.js | 35 +++++++++++++++++++++++- 2 files changed, 101 insertions(+), 2 deletions(-) diff --git a/src/plan.js b/src/plan.js index 52180f0..27426c1 100644 --- a/src/plan.js +++ b/src/plan.js @@ -49,6 +49,65 @@ const EXIT_FIELDS = { }; const SLUG_RE = /^[a-z0-9][a-z0-9-]*$/; +/** Length of an UNBOUNDED quantifier token at `src[i]` (`*`, `+`, or `{n,}`, + * plus an optional trailing lazy `?`), or 0. Bounded forms (`?`, `{n}`, + * `{n,m}`) return 0: they cannot drive exponential backtracking (a bounded + * outer repeat is polynomial at worst, and the body is self-authored — F49). + * @param {string} src @param {number} i */ +function unboundedQuantLen(src, i) { + let len = src[i] === '*' || src[i] === '+' ? 1 : 0; + if (!len && src[i] === '{') { const m = /^\{\d+,\}/.exec(src.slice(i)); if (m) len = m[0].length; } + if (len && src[i + len] === '?') len++; // the lazy modifier is part of the same token + return len; +} + +/** + * F49 — a heuristic reject for the catastrophic-backtracking footgun: an + * unbounded quantifier applied to a group whose body itself repeats unboundedly + * (`(a+)+`, `(\d*)*`, `([a-z]+){1,}`). Such a pattern can hang `RegExp.test` for + * seconds on a short crafted body, and `evalExits` runs the agent-authored + * `artifact-written` pattern with NO timeout. This is self-DoS only — the agent + * authors both the pattern and (via the worker) the artifact, so a hang burns + * only its own run's wall-clock; there is NO arbiter compromise (it cannot + * escape the fence, forge a green, or leak a secret). So the reject targets the + * dominant exponential class and fails it as a mechanical gap at the validation + * gate, before any tokens burn. A full ReDoS analyzer needs a real regex engine + * (an external native dep we do not take for a LOW issue); exotic + * overlapping-alternation blowup is out of scope by decision. + * + * The input is guaranteed to compile as a RegExp (checked first by the caller), + * so this scan assumes valid, balanced JS regex syntax. + * @param {string} src a compiled regex source string + * @returns {boolean} true iff a nested unbounded quantifier is present + */ +export function hasNestedQuantifier(src) { + /** @type {{ quant: boolean }[]} */ + const stack = []; + for (let i = 0; i < src.length; i++) { + const c = src[i]; + if (c === '\\') { i++; continue; } // escaped atom — the next char is a literal + if (c === '[') { // character class: quantifier chars inside are literals + i++; + if (src[i] === '^') i++; + if (src[i] === ']') i++; // a leading ] is a literal member, not the close + while (i < src.length && src[i] !== ']') { if (src[i] === '\\') i++; i++; } + continue; + } + if (c === '(') { stack.push({ quant: false }); continue; } + if (c === ')') { + const g = stack.pop(); + const qlen = unboundedQuantLen(src, i + 1); + if (g && g.quant && qlen) return true; // group repeats unboundedly AND its body did too + if (qlen && stack.length) stack[stack.length - 1].quant = true; // the group is an unbounded-repeated atom of its parent + i += qlen; + continue; + } + const qlen = unboundedQuantLen(src, i); // a quantifier applying to the preceding atom, inside the current group + if (qlen) { if (stack.length) stack[stack.length - 1].quant = true; i += qlen - 1; } + } + return false; +} + /** @typedef {{code: string, path: string, detail?: string, verb?: string}} Red */ /** @@ -221,8 +280,15 @@ function validateExit(s, at, red, { checkNames, fence, insideFence, writeStep }) if (e.type === 'artifact-written' && e.pattern !== undefined) { if (!isNonEmptyString(e.pattern)) red('invalid-value', `${eAt}.pattern`, 'regex source string'); else { - try { new RegExp(e.pattern, 'm'); } + let compiled = false; + try { new RegExp(e.pattern, 'm'); compiled = true; } catch { red('invalid-value', `${eAt}.pattern`, 'must compile as a RegExp'); } + // F49: a compiled-but-catastrophic pattern (nested unbounded + // quantifier) can hang the untimed exit evaluator — reject it here as + // a mechanical gap so the replan rewrites it, before any tokens burn. + if (compiled && hasNestedQuantifier(e.pattern)) { + red('invalid-value', `${eAt}.pattern`, `nested unbounded quantifier (e.g. (a+)+ , (\\d*)* , (x+){1,}) — a catastrophic-backtracking footgun that can hang the exit evaluator (F49); rewrite without a repeated group inside a repeat`); + } } } } diff --git a/tests/plan.test.js b/tests/plan.test.js index 41b823c..da63071 100644 --- a/tests/plan.test.js +++ b/tests/plan.test.js @@ -10,7 +10,7 @@ import { test } from 'node:test'; import assert from 'node:assert/strict'; -import { validatePlan, EXIT_TYPES, MAX_EXITS_PER_STEP, MAX_PLAN_STEPS, WRITE_VERBS } from '../src/plan.js'; +import { validatePlan, EXIT_TYPES, MAX_EXITS_PER_STEP, MAX_PLAN_STEPS, WRITE_VERBS, hasNestedQuantifier } from '../src/plan.js'; import { validateJob } from '../src/job.js'; // The signed side: a validateJob-green four-field spec (job #4's shape) — the @@ -231,6 +231,7 @@ const RED_CASES = [ ['artifact-written path outside the fence', (p) => { p.steps[0].exit[0] = { type: 'artifact-written', path: 'docs/notes.md' }; }, 'scope-escape:steps.0.exit.0.path'], ['artifact-written path escaping the run dir', (p) => { p.steps[0].exit[0] = { type: 'artifact-written', path: '../notes.md' }; }, 'invalid-value:steps.0.exit.0.path'], ['artifact-written pattern that does not compile', (p) => { p.steps[0].exit[0].pattern = 'def ('; }, 'invalid-value:steps.0.exit.0.pattern'], + ['artifact-written pattern with a nested unbounded quantifier (ReDoS footgun, F49)', (p) => { p.steps[0].exit[0].pattern = '(a+)+$'; }, 'invalid-value:steps.0.exit.0.pattern'], ['tree-changed without a scope', (p) => { p.steps[0].exit[1] = { type: 'tree-changed' }; }, 'invalid-value:steps.0.exit.1.scope'], ['tree-changed scope outside the fence', (p) => { p.steps[0].exit[1] = { type: 'tree-changed', scope: 'src/**' }; }, 'scope-escape:steps.0.exit.1.scope'], ['tree-changed scope escaping the run dir', (p) => { p.steps[0].exit[1] = { type: 'tree-changed', scope: '../**' }; }, 'invalid-value:steps.0.exit.1.scope'], @@ -252,6 +253,38 @@ for (const [name, fn, want] of RED_CASES) { }); } +// F49 — the catastrophic-backtracking detector. A pure-logic algorithm, so a +// direct good/bad battery is the right instrument (Testing Trophy: unit tests +// for algorithms). BAD = an unbounded quantifier applied to a group that +// already repeats unboundedly (the exponential class); GOOD = everything the +// agent legitimately writes, including bounded repetition and escaped/classed +// quantifier chars that must NOT be read as nesting. +const REDOS_BAD = [ + '(a+)+', '(a+)*', '(a*)+', '(a*)*$', '(\\d+)+', '([a-z]+)*', '(\\w+){1,}', + '((a+)+)', '(a+\\w*)+', '(a+?)+?', '(foo|ba+r)+', '(\\s+)*end', +]; +const REDOS_GOOD = [ + 'def ', 'a+', '(abc)+', '(a+)', '(a+)?', '(a+){2}', '(a+){1,3}', + '\\(a+\\)+', '[a+]+', '[+*]{2,}', 'foo|bar', '^\\d{3}-\\d{4}$', + '(a+)b+', '(a+)(b+)', '(?:abc)+', 'class \\w+\\(', '(a{2,4})+', +]; +for (const src of REDOS_BAD) { + test(`hasNestedQuantifier flags the footgun: ${src}`, () => { + assert.equal(hasNestedQuantifier(src), true, `${src} should be flagged`); + }); +} +for (const src of REDOS_GOOD) { + test(`hasNestedQuantifier passes the safe pattern: ${src}`, () => { + assert.equal(hasNestedQuantifier(src), false, `${src} should NOT be flagged`); + }); +} + +test('the ReDoS red detail names the footgun (the gap must let the replan rewrite, not guess)', () => { + const r = validatePlan(mut((p) => { p.steps[0].exit[0].pattern = '(a+)+$'; }), OPTS); + assert.match(r.reds[0].detail ?? '', /quantifier/i); + assert.match(r.reds[0].detail ?? '', /F49/); +}); + test('check-unknown detail names the SIGNED menu (the gap must aim the replan, not taunt it)', () => { const r = validatePlan(mut((p) => { p.steps[1].exit[1] = { type: 'check-passes', name: 'my-clever-check' }; }), OPTS); assert.match(r.reds[0].detail ?? '', /clean-run/); From 4cf91eff00a7c67e422e977ecfc7c4ea9425884f Mon Sep 17 00:00:00 2001 From: hamr0 Date: Thu, 23 Jul 2026 18:50:37 +0200 Subject: [PATCH 2/7] F50: wire Layer R into the accepted plan-v1 flow (was silently unwired) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit layerRoot was wired only into the legacy steps[] path (interpret.js); the accepted plan flow (runPlan) silently ignored it, so a plan-shape job could never emit root-injected and the pre-registered ON-vs-OFF default-flip read was impossible on the go-forward surface — the blind-instrument class as a silently-ignored optional param. Wire it per step (each micro-wheel is the Layer-1 atom): the tee (stage/settle/ discard + onToolResult outcome probe) is mirrored from interpret.js into mkWorker; executeStep creates one root per step and calls observe + injects the note in the middle. Two plan-flow-specific calls, both traced not assumed: - red-set = the exit evaluator's OWN gap (gapKeep '\S', the whole normalized complaint) — a check's ^-anchored gapKeep does not survive the exit wrapper (check "x" red: FAILED …) and would degrade to the dangerous writes-only mode. - the tee is REQUIRED, not just for verbatim: a step rewrites its one target every attempt, and the cumulative audit dedups by path, so without the tee the summary stage never fires either. Native/clipipe excluded (no onToolResult seam; F48 fallback surface). Still OFF by default (F41). TDD: 5 tests through the real plan flow — summary fires + reaches the next prompt, a third fixation escalates to verbatim surfacing the worker's own teed bytes, OFF emits nothing, native inert. Docs: CHANGELOG, LAYERS ⚠ note, FINDINGS F49→resolved + F50, context. Co-Authored-By: Claude Opus 4.8 (1M context) --- CHANGELOG.md | 17 ++++++ bareloop.context.md | 20 ++++--- docs/01-product/LAYERS.md | 29 ++++++---- docs/FINDINGS.md | 83 +++++++++++++++++++++++++--- src/planrun.js | 110 +++++++++++++++++++++++++++++++++++--- src/run.js | 2 +- tests/planrun.test.js | 89 +++++++++++++++++++++++++++++- 7 files changed, 316 insertions(+), 34 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 999447a..b2b1c03 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,23 @@ feature lands, **patch** = docs, fixes, scaffolding. ## [Unreleased] +### Added +- **Layer R wired into the plan-v1 flow** (F50). The within-run ratchet (`layerRoot`) now + engages per step in `runPlan`, not only on the legacy `steps[]` path — so a plan-shape job + can emit `root-injected` and the pre-registered ON-vs-OFF default-flip read becomes possible + on the accepted surface. Red-set = the exit evaluator's own gap; the write-tee is wired so + same-path target rewrites are visible to the detector. Still OFF by default (`layerRoot: + false`, F41). Excluded on native/clipipe (no `onToolResult` seam — F48 fallback surface). + +### Fixed +- **F49 (security-hardening): the agent-authored `artifact-written` regex can no longer hang + the exit evaluator.** `plan.js` rejects nested-quantifier ReDoS patterns (`(a+)+`, `(\d*)*`, + `(x+){1,}`) at the validation gate, before any tokens burn. LOW self-DoS, no arbiter + compromise; input-bounding was rejected as theater (a 33-char body hangs) and a JS regex + timeout as disproportionate (worker/RE2 dep). +- **F50: `runJob` no longer silently ignores `layerRoot` for plan-shape jobs** (was accepted + and dropped — an advertised no-op). + ## [0.5.0] — 2026-07-23 ### Added diff --git a/bareloop.context.md b/bareloop.context.md index e83a7c0..9319e10 100644 --- a/bareloop.context.md +++ b/bareloop.context.md @@ -376,14 +376,18 @@ through `Loop`'s `onToolResult`). So a repeat that never applied is told its anc missed, and only content actually in the file is ever described as having landed. Spine event `root-injected` carries `{stage, mode, streak, paths, redSetSize}` — counts and paths only, NEVER content (the spine is append-only). State dies with the run: this is within-run scratch, not -across-run memory. `layerRoot: true` (on `interpret` and `runJob`) is the ON/experimental -arm; the default is OFF. Status (F41): the ratchet is MEASURED inert on every current -job — two frozen probes + an archive sweep read 0 fixated pairs in 14 (the F21 repetition -disease was a broken-loop symptom, since cured). Because ON has therefore never won its -own A/B, it ships **OFF by default (decided 2026-07-21)** — armed and correct, but not -default-enabled until a stuck job (Layer 2, or a manufactured-fixation probe) shows ON -beats OFF. Flip the default to `true` the day that evidence lands; a `root-injected` event -on your spine (when you pass `layerRoot: true`) is a signal worth reading, not noise. +across-run memory. `layerRoot: true` is the ON/experimental arm; the default is OFF. It is +wired on BOTH surfaces (F50): the legacy `steps[]` path (`interpret`) and the accepted +plan-v1 flow (`runPlan`, one root per step). On the plan flow the red-set is the exit +evaluator's OWN gap (the whole normalized complaint) rather than a single close's gapKeep, +and the write-tee makes same-path target rewrites visible to the detector; it is NOT wired +on the native/clipipe worker (no `onToolResult` seam — F48 fallback surface). Status (F41): +the ratchet is MEASURED inert on every current job — two frozen probes + an archive sweep +read 0 fixated pairs in 14 (the F21 repetition disease was a broken-loop symptom, since +cured). Because ON has therefore never won its own A/B, it ships **OFF by default (decided +2026-07-21)** — armed and correct, but not default-enabled until a stuck job shows ON beats +OFF. Flip the default to `true` the day that evidence lands; a `root-injected` event on your +spine (when you pass `layerRoot: true`) is a signal worth reading, not noise. ### `runJob(spec, { approvals, workdir, provider, nativeProvider?, emit, target?, capRuns?, shellCapUsd?, closeTimeoutMs?, execCmd?, layerRoot? })` → outcome — `src/run.js` diff --git a/docs/01-product/LAYERS.md b/docs/01-product/LAYERS.md index 83e8930..b64602f 100644 --- a/docs/01-product/LAYERS.md +++ b/docs/01-product/LAYERS.md @@ -145,15 +145,26 @@ default-enabled (`layerRoot: false`; pass `true` for the ON/experimental arm). I read (repetition drop, ON vs OFF) DEFERS to the first run whose spine records `root-injected`. -> **⚠ Layer 2 TODO — decide the Layer R default.** Layer 2's narrow micro-wheel steps are -> the expected pressure point that finally produces natural fixation (a stuck run). The -> day a Layer 2 job records `root-injected`, run the pre-registered ON-vs-OFF acceptance -> read on it. **That result decides whether the `layerRoot` default flips to `true` -> (ON helps → keep it on) or stays `false` (no lift → keep it off).** Until then the -> default is provisional, not settled. (A cheaper alternative that does NOT need Layer 2: -> a manufactured-fixation probe — force a real worker to repeat and measure whether the -> note breaks the loop; caveat F41 — strong models resist fixating, so the probe may -> struggle to produce its own precondition honestly.) +**Wired into the plan-v1 flow (2026-07-23, F50).** Until then the ratchet was wired only +into the legacy `steps[]` path (`interpret.js`); the accepted plan flow silently ignored +`layerRoot`, so it could never emit `root-injected`. Now `runPlan` engages one root per +step (each micro-wheel is the Layer-1 atom): red-set = the exit evaluator's own gap +(`gapKeep '\S'`, the whole normalized complaint — format-independent, since a check's +`^`-anchored gapKeep does not survive the exit wrapper `check "x" red: …`), and the +write-tee is wired so same-path target rewrites are visible to the detector (the cumulative +audit dedups by path and cannot see them alone). Excluded on native/clipipe (no +`onToolResult` seam; F48 fallback surface, not the experiment surface). + +> **⚠ Layer 2/3 TODO — decide the Layer R default.** The wiring now EXISTS on the accepted +> surface (F50); the experiment is possible but not yet run. Layer 2's narrow micro-wheel +> steps are the expected pressure point that finally produces natural fixation (a stuck +> run). The day a real plan-flow job records `root-injected`, run the pre-registered +> ON-vs-OFF acceptance read on it. **That result decides whether the `layerRoot` default +> flips to `true` (ON helps → keep it on) or stays `false` (no lift → keep it off).** Until +> then the default is provisional, not settled. (A cheaper alternative that does NOT need a +> natural stuck run: a manufactured-fixation probe — force a real worker to repeat and +> measure whether the note breaks the loop; caveat F41 — strong models resist fixating, so +> the probe may struggle to produce its own precondition honestly.) ### Layer 2 — micro-wheels (the road) The workflow becomes a **sequence of small wheels**, each with one goal and only the verbs diff --git a/docs/FINDINGS.md b/docs/FINDINGS.md index d642d5f..544b965 100644 --- a/docs/FINDINGS.md +++ b/docs/FINDINGS.md @@ -2433,7 +2433,7 @@ hypothesis into a refutation (spent $7, escalated on behavior), which no cheaper could have settled. The API is the only guaranteed surface; clipipe is a taxed Claude-only fallback; local LLMs are an unbuilt, unmeasured future bet. -## F49 — OPEN (logged follow-up): the agent-authored `artifact-written` regex runs unbounded in the exit evaluator +## F49 — RESOLVED (2026-07-23): the agent-authored `artifact-written` regex runs unbounded in the exit evaluator **The gap (security scan, v0.5.0 pre-release).** `src/exits.js` evaluates an `artifact-written` exit's optional `pattern` with `new RegExp(e.pattern, 'm').test(body)` @@ -2451,13 +2451,80 @@ author's input), filed as the honest "bound every reachable path" invariant appl CPU, not a privilege/exposure vuln. Operator-authored regex (`judged.pattern`, `gapKeep`, check bodies) is out of scope — the operator is trusted and those are signed. -**Fix DEFERRED (multiple shapes, needs a decision — hamr's call at v0.5.0):** (a) bound the -tested input size, (b) run the match under a timeout (needs a worker/subprocess in JS), or -(c) reject nested-quantifier patterns at validation (incomplete). (a) changes match -semantics for patterns meant to hit late in a large file; (b) is the most faithful but the -heaviest; (c) cannot be complete. Recorded as the next Layer 2 hardening item; the v0.5.0 -release proceeded because the arbiter is uncompromised. +**Fix shapes weighed (three, at v0.5.0):** (a) bound the tested input size, (b) run the +match under a timeout (a JS worker/subprocess), or (c) reject nested-quantifier patterns at +validation. Two were disqualified on evidence: (a) is THEATER for classic exponential ReDoS +— the blowup needs only tens of characters (measured: `(a+)+$` did NOT finish on a **33-char** +body in 120s), so a length cap does nothing; (b) needs `worker_threads` or an external RE2 +engine — disproportionate weight (and a new native dep) for a LOW self-DoS. That leaves (c), +which is the doctrine-clean place anyway: a mechanical reject at the validation gate, before +any tokens burn, that the replan can rewrite. + +**Fix SHIPPED (2026-07-23), option (c):** `plan.js` now runs `hasNestedQuantifier(pattern)` +after the compile check and reds an `invalid-value:…pattern` when a group repeating unboundedly +(`*`, `+`, `{n,}`) is itself wrapped in an unbounded quantifier (`(a+)+`, `(\d*)*`, +`(x+){1,}`) — the dominant exponential class. It is a vanilla state-machine scan (no dep): +skips escaped atoms and character classes (so `\(a+\)+` and `[a+]+` are safe), treats bounded +outer repeats (`(a+)?`, `(a+){2}`, `(a+){1,3}`) as safe (polynomial, self-authored body), and +is honest about its bound — exotic overlapping-alternation blowup is out of scope BY DECISION +(self-DoS only, no arbiter compromise). TDD: a 12-bad / 17-good detector battery + a validator +red-case + a detail-names-the-footgun test (31 tests), plus the empirical 33-char hang above +as the "the test can fail" proof. Full suite green. **Lesson.** A security scan's value is the coverage table, not just the hits: the one finding here is a LOW self-DoS, and naming it against a CLEAN arbiter-integrity sweep is -what makes "clean" auditable rather than asserted. +what makes "clean" auditable rather than asserted. And the fix-shape choice was itself an +evidence call — two of the three candidates died to a 2-line measurement (input-bounding is +theater; a 33-char body hangs), which is cheaper than shipping the wrong remedy. + +## F50 — RESOLVED (2026-07-23): Layer R was silently unwired on the accepted plan-v1 flow + +**The gap.** Layer R (`layerRoot`) was wired only into the LEGACY `steps[]` path +(`interpret.js` → `createRoot`, on staged sunset). The ACCEPTED plan-v1 flow that shipped +v0.5.0 (`runPlan`/`planrun.js`) never created a root — `runJob` accepted `layerRoot` and +silently IGNORED it for plan-shape jobs. Two consequences, both real: a Layer 2 job could +NEVER emit `root-injected`, so the LAYERS.md ⚠ pre-registered ON-vs-OFF default-flip read was +impossible to satisfy on the go-forward surface; and `layerRoot: true` on a plan job was a +silent no-op — an advertised capability that did not fire. The blind-instrument class in its +plainest form: the ratchet lived only on the path that is being retired. + +**Why it mattered now.** The legacy `steps[]` path is a sunset candidate but NOT yet +retirable — plan-v1 admits only the `green` verdict, so `hitl` (the draft-PR flow) and +`soft-green` still run ONLY on the legacy path (the later non-code-jobs goal). So the ratchet +was stranded on the exact path scheduled to disappear. + +**Fix (2026-07-23).** Wired Layer R into the plan flow, scoped PER STEP (each micro-wheel is +the Layer-1 atom). The tee (stage/settle/discard + `onToolResult` outcome probe) is mirrored +from `interpret.js` into `mkWorker`; `executeStep` creates one root per step and calls +`observe` + injects its note in the middle. Two design calls specific to the plan flow: + +- **The red-set is the exit evaluator's OWN gap** (`gapKeep: '\S'` — every non-blank line), + not a reused check gapKeep. A check's `^`-anchored `gapKeep` (`^FAILED`) does not match once + the exit wrapper prefixes it (`check "x" red: FAILED …`), which would silently degrade the + detector to the dangerous writes-only mode. "The exit evaluator's complaint is byte-identical" + is the honest, format-independent red-set for a step. +- **The tee is REQUIRED here, not just for the verbatim stage.** A plan step rewrites its ONE + `target` every attempt; the cumulative gate audit dedups by path, so a same-path rewrite adds + nothing to the write-set and the detector would see an empty delta. The tee is what makes the + rewrite visible — so without it the summary stage never fires either (traced, then tested). + +**Native excluded, by construction.** The clipipe native worker exposes no `onToolResult` +seam, so the tee cannot settle and same-path rewrites are blind — `root` is `null` for native +(documented, not silent; F48 already ruled native OUT-as-peer, so this is the fallback surface, +not the experiment surface). + +**Validation.** 5 TDD tests through the REAL plan flow (real gate, real spawned close/check, +real audit + tee): summary fires on an unmoved-red-set same-file rewrite and is injected into +the next attempt; a third consecutive fixation escalates to VERBATIM surfacing the worker's own +teed bytes back; OFF by default emits nothing; native stays inert. 555/555, typecheck + build +clean. + +**Status of the default-flip.** Still deferred, but now POSSIBLE: the day a real plan-flow job +emits `root-injected`, run the pre-registered ON-vs-OFF acceptance read (LAYERS.md ⚠). F41 +stands — fixation is extinct on every current job — so the default remains `false` until that +evidence lands. + +**Lesson.** "Wired into the shipped path" is a separate claim from "the code exists," and the +gap hid because the parameter threaded cleanly to a DIFFERENT (legacy) path. When a capability +has two dispatch paths, check which one the ACCEPTED surface actually takes — a silently-ignored +optional param is the blind-instrument class wearing an API's clothes. diff --git a/src/planrun.js b/src/planrun.js index 0ffaa73..064c373 100644 --- a/src/planrun.js +++ b/src/planrun.js @@ -15,6 +15,7 @@ import { createRequire } from 'node:module'; import { readFileSync } from 'node:fs'; +import { createHash } from 'node:crypto'; import { join, resolve } from 'node:path'; import { Gate, redact } from 'bareguard'; import { LiteCtx } from 'litectx'; @@ -22,6 +23,7 @@ import { runClose, ralph, CLOSE_FAULTS } from './ralph.js'; import { validatePlan } from './plan.js'; import { WRITE_VERBS, EXIT_TYPES, MAX_EXITS_PER_STEP, MAX_PLAN_STEPS } from './plan.js'; import { snapshotScope, evalExits } from './exits.js'; +import { createRoot } from './root.js'; import { TOOL_MENU } from './job.js'; import { TOOL_BY_VERB, CTX_TOOLS, createCtxTools, toolAction, PERSONA_TOOLS, RETRIEVAL_STRATEGY, EDIT_STRATEGY } from './interpret.js'; import { globToPrefix, SECRET_PATTERNS } from './validate.js'; @@ -127,11 +129,20 @@ ${scoutBlob || '(no scout notes)'}`; * @param {number} [opts.capRuns] shell-owned per-step attempt cap * @param {number} [opts.closeTimeoutMs] close/check wall-clock cap (shell territory) * @param {number} [opts.maxStepRounds] the shell's per-step rounds ceiling (validatePlan's bound) + * @param {boolean} [opts.layerRoot=false] Layer R — the within-run ratchet (src/root.js), + * scoped PER STEP's ralph loop (each micro-wheel is the Layer-1 atom). Shell-assembled from + * the step's own books: per-attempt write-sets from the F32 workerWrites audit (teed for + * same-path rewrites, which the cumulative audit alone cannot see) and the red-set from the + * exit evaluator's own gap. Defaults OFF (decided 2026-07-21, F41): fixation is extinct on + * every current job, so ON has never won its A/B; `true` is the ON/experimental arm, and the + * first plan-flow job to emit `root-injected` runs the pre-registered ON-vs-OFF acceptance + * read (the Layer R default-flip, LAYERS.md ⚠). Excluded on native (clipipe): the native + * worker has no onToolResult seam, so the tee cannot settle and same-path rewrites are blind. * @returns {Promise} 'green' | 'already-green' | 'escalated' | 'plan-red' | * 'check-red' | 'close-red' | 'close-unsupported' | 'pricing-red' | 'cap-halt' | * 'provider-red' | 'interpreter-red' | `step-red:` */ -export async function runPlan(job, { workdir, provider, nativeProvider, emit, remainingUsd, isUnpriced = () => false, capRuns = 3, closeTimeoutMs, maxStepRounds = 40 }) { +export async function runPlan(job, { workdir, provider, nativeProvider, emit, remainingUsd, isUnpriced = () => false, capRuns = 3, closeTimeoutMs, maxStepRounds = 40, layerRoot = false }) { workdir = resolve(workdir); const scrub = (/** @type {string} */ s) => redact(s, { patterns: SECRET_PATTERNS }); @@ -228,9 +239,13 @@ export async function runPlan(job, { workdir, provider, nativeProvider, emit, re * denied, the wallet as its budget), granted tools only (the menu IS the * grant), per-attempt round bound via loop.stop() (F20), every round metered * with a phase label (F12). - * @param {{granted: string[], phase: string, attemptRounds: number, attempts: number, writable: boolean}} o + * @param {{granted: string[], phase: string, attemptRounds: number, attempts: number, writable: boolean, root?: ReturnType|null}} o + * `root` (Layer R): when present, the worker's write-class actions are teed + * for the within-run ratchet — staged before the gate decides, discarded on + * deny/halt, settled to landed-or-not after execution (the two axes, F43/F7). + * Wired ONLY on the Loop path: the native session exposes no onToolResult seam. */ - async function mkWorker({ granted, phase, attemptRounds, attempts, writable }) { + async function mkWorker({ granted, phase, attemptRounds, attempts, writable, root = null }) { const gate = new Gate({ fs: { writeScope: writable ? fencePrefixes : [], @@ -259,7 +274,69 @@ export async function runPlan(job, { workdir, provider, nativeProvider, emit, re return [...paths]; } catch { return []; } }; - const { policy, onLlmResult } = wireGate(gate, { actionTranslator: (/** @type {string} */ n, /** @type {any} */ a) => toolAction(n, a, workdir) }); + // Layer R tee (design record 2026-07-19; created per-step by executeStep). + // The translator STAGES write/edit content and snapshots the pre-write hash + // BEFORE the gate decides (Finding 6); the policy wrapper DISCARDS on a deny + // or a halt (the tool never runs); onToolOutcome SETTLES landed-vs-not after + // execution (Finding 7 — the gate's allow is intent, the file is outcome). + // All no-ops when root is null: every scout/drafter and every native worker. + const fileHash = (/** @type {string} */ p) => { + try { return createHash('sha256').update(readFileSync(p)).digest('hex'); } catch { return null; } + }; + /** @type {{path: string, content: string, type: string, before: string|null}|null} the write-class action awaiting its OUTCOME */ + let pendingProbe = null; + const teeingTranslator = (/** @type {string} */ n, /** @type {any} */ a) => { + // classify by action TYPE, never a name list (the workerWrites filter uses + // the same write|edit test — a third enumeration would let a future + // write-class verb bypass the tee, the blind-instrument class) + const act = toolAction(n, a, workdir); + if (root && (act.type === 'write' || act.type === 'edit')) { + const path = /** @type {string} */ (act.path); + const content = String((act.type === 'edit' ? a?.newText : a?.content) ?? ''); + root.stageWrite(path, content); // content read off RAW args, never onto the action (the audit stays bytes-only) + pendingProbe = { path, content, type: act.type, before: fileHash(path) }; + } + return act; + }; + const { policy: gatePolicy, onLlmResult } = wireGate(gate, { actionTranslator: teeingTranslator }); + // Settle the stage on the verdict. A DENY/HALT means the tool never runs, so + // the outcome seam below never fires for it — discard here (an allow is NOT + // settled here: the bytes are not written yet; settling on the verdict is the + // exact Finding 7 defect). The catch RE-THROWS so cap-halt routing still reads + // the throw. Non-write actions stage nothing, so both settlements are no-ops. + const policy = root + ? async (/** @type {string} */ n, /** @type {any} */ a, /** @type {any} */ c) => { + try { + const verdict = await gatePolicy(n, a, c); + if (verdict !== true) { root.discardWrite(); pendingProbe = null; } + return verdict; + } catch (e) { root.discardWrite(); pendingProbe = null; throw e; } + } + : gatePolicy; + /** + * Finding 7 — settle the staged write on what the tool ACTUALLY did. Fires + * after every tool.execute (bare-agent loop.js), success or error, and tool + * calls run strictly sequentially, so exactly one probe is ever in flight. A + * probe that never reaches here (a halt out of a tool body) leaves the stage + * unsettled — correct: nothing landed, and the attempt boundary drops it. + * @param {{result?: any}} [info] the tool's return value + */ + const onToolOutcome = async (info) => { + if (!root || !pendingProbe) return; + const { path: p, content, type, before } = pendingProbe; + pendingProbe = null; + let after = null; + try { after = readFileSync(p, 'utf8'); } catch { /* absent → after stays null */ } + let landed = after !== null && createHash('sha256').update(after, 'utf8').digest('hex') !== before; + if (!landed && after !== null) { + // no-byte-change: a write always writes (identical bytes ⇒ content IS the + // file, exact-equal never substring); a no-op edit is an idempotent apply + // OR a missed anchor — the tool RESULT is the only honest signal (F2) + if (type === 'write') landed = after === content; + else landed = typeof info?.result === 'string' && info.result.startsWith('edited '); + } + root.settleWrite(landed); + }; /** @type {number|string|undefined} */ let roundIteration; let roundsThisAttempt = 0; @@ -383,7 +460,7 @@ export async function runPlan(job, { workdir, provider, nativeProvider, emit, re } return onLlmResult(arg); }; - const loop = new Loop({ provider: loopProvider, system, policy, onLlmResult: metered }); + const loop = new Loop({ provider: loopProvider, system, policy, onLlmResult: metered, onToolResult: onToolOutcome }); /** @param {string} prompt @param {typeof toolDefs} [defs] */ const ask = async (prompt, defs = toolDefs) => { let r; @@ -507,13 +584,33 @@ export async function runPlan(job, { workdir, provider, nativeProvider, emit, re for (const e of step.exit) { if (e.type === 'tree-changed') for (const [k, v] of await snapshotScope(workdir, e.scope)) snapshot.set(k, v); } - const w = await mkWorker({ granted: step.tools, phase: `step:${step.id}`, attemptRounds: step.rounds, attempts: capRuns, writable: true }); + // Layer R — one root per step's ralph loop (each micro-wheel is the Layer-1 + // atom, LAYERS.md). The red-set is the exit evaluator's OWN gap: fixation + // here means "the worker rewrote the same file(s) AND the exit evaluator's + // complaint is byte-identical" — so the whole normalized gap is the + // comparable set (gapKeep `\S` = every non-blank line). That is more robust + // than reusing a check's `^`-anchored gapKeep, which the exit wrapper + // (`check "x" red: …`) would break. writesInformative is true (tool mode, + // always) but never fires on write-overlap ALONE — a plan step rewrites its + // one target every attempt, so only a KNOWN-unmoved red-set can distinguish + // repetition from progress (Finding 3). Native is excluded (no onToolResult + // seam → the tee cannot settle → same-path rewrites are blind). + const root = layerRoot && !native + ? createRoot({ gapKeep: '\\S', redact: scrub, writesInformative: true }) + : null; + const w = await mkWorker({ granted: step.tools, phase: `step:${step.id}`, attemptRounds: step.rounds, attempts: capRuns, writable: true, root }); let lastText = ''; let iterationNow = 0; /** @param {number} iteration @param {string} [gap] */ const middle = async (iteration, gap) => { w.setIteration(iteration); iterationNow = iteration; + // Layer R observe: finalize the prior attempt from the books (workerWrites + // audit + teed same-path rewrites), run the fixation detector, and inject + // its escalating note into THIS attempt's prompt (null = inert). The event + // carries counts and paths only — never content (the spine is forever). + const rootInj = root ? root.observe({ iteration, gap, writes: w.workerWrites() }) : null; + if (rootInj) emit('root-injected', { step: step.id, ...rootInj.event }); const r = await w.ask([ step.action, `Repository root (absolute): ${workdir}\nEvery path you pass to a tool MUST be absolute and inside this root — a relative path resolves against a different directory and will be denied by the gate.`, @@ -522,6 +619,7 @@ export async function runPlan(job, { workdir, provider, nativeProvider, emit, re gap && `Previous attempt failed this step's checks:\n${gap}`, w.wasBounded() === iteration - 1 && `Your previous attempt was CUT OFF after ${step.rounds} tool rounds. Reading is bounded; writing is not. Form a hypothesis EARLY and make the change.`, + rootInj && rootInj.note, ].filter(Boolean).join('\n\n')); lastText = scrub(r.text ?? '').slice(0, ARTIFACT_MAX); }; diff --git a/src/run.js b/src/run.js index 84e34ea..fe67944 100644 --- a/src/run.js +++ b/src/run.js @@ -303,7 +303,7 @@ export async function runJob(rawSpec, { approvals, workdir, target, provider, na // money contract is identical to the legacy path. if (planShape) { const outcome = await runPlan(job, { - workdir, provider, nativeProvider, emit: meter, capRuns, closeTimeoutMs, + workdir, provider, nativeProvider, emit: meter, capRuns, closeTimeoutMs, layerRoot, remainingUsd: () => Math.min(shellCapUsd, job.budgetUsd - spentUsd), isUnpriced: () => unpriced, // F6: let the plan flow bail in-flight, not just after it returns }); diff --git a/tests/planrun.test.js b/tests/planrun.test.js index dcd78b7..d5f3e5e 100644 --- a/tests/planrun.test.js +++ b/tests/planrun.test.js @@ -71,11 +71,11 @@ const collector = () => { return { events, emit: (type, data = {}) => { const e = { type, ...data }; events.push(e); return e; } }; }; -async function go(wd, provider, { job = JOB(wd), capRuns = 3 } = {}) { +async function go(wd, provider, { job = JOB(wd), capRuns = 3, layerRoot = false } = {}) { const jv = validateJob(job); assert.deepEqual(jv.reds, [], 'the test job must be validateJob-green'); const { events, emit } = collector(); - const outcome = await runPlan(jv.job, { workdir: wd, provider, emit, capRuns, remainingUsd: () => 1.5 }); + const outcome = await runPlan(jv.job, { workdir: wd, provider, emit, capRuns, layerRoot, remainingUsd: () => 1.5 }); return { outcome, events }; } @@ -387,6 +387,91 @@ test('a step SETUP fault is recorded on the plan-executed spine with the SAME ca assert.equal(outcome, 'interpreter-red'); }); +// ── Layer R (the within-run ratchet, src/root.js) wired into the plan flow. +// A fixation script: the worker rewrites its ONE target with non-'ok' content +// twice (same file, same failing check ⇒ identical red-set), then converts on +// attempt 3. The detector must fire the SUMMARY stage at the start of attempt 3 +// (comparing attempts 1 and 2), inject its note, and stay OFF by default. +// NOTE: content must not contain the substring 'ok' (the check greens on it) — +// and 'broken' does, so the stubs read 'placeholder N'. +const fixationScript = (wd) => scriptedProvider([ + { text: 'src/mod.mjs exports x; tests/ is empty.' }, // scout + { text: PLAN(wd) }, // plan draft + { toolCalls: [tcall('t1', 'shell_write', { path: join(wd, 'tests', 'test_x.mjs'), content: 'placeholder 1 — no assertion yet\n' })] }, + { text: 'attempt 1' }, + { toolCalls: [tcall('t2', 'shell_write', { path: join(wd, 'tests', 'test_x.mjs'), content: 'placeholder 2 — still no assertion\n' })] }, + { text: 'attempt 2' }, + { toolCalls: [tcall('t3', 'shell_write', { path: join(wd, 'tests', 'test_x.mjs'), content: 'ok — asserts x now\n' })] }, + { text: 'attempt 3 fixed it' }, +]); + +test('Layer R ON: a worker that rewrites the same file with an unmoved red-set fires the SUMMARY ratchet, injected into the next attempt', async (t) => { + const wd = makePatient(t); + const { outcome, events } = await go(wd, fixationScript(wd), { layerRoot: true }); + const inj = events.filter((e) => e.type === 'root-injected'); + assert.ok(inj.length >= 1, 'the ratchet fired at least once'); + assert.equal(inj[0].stage, 'summary', 'first fire is the capped summary (streak 1)'); + assert.equal(inj[0].step, 'write-test', 'the event names its step'); + assert.equal(outcome, 'green', 'the ratchet does not break convergence — attempt 3 still greens'); +}); + +test('Layer R ON: the ratchet note is injected into the third attempt\'s prompt (the worker actually sees it)', async (t) => { + const wd = makePatient(t); + const provider = fixationScript(wd); + await go(wd, provider, { layerRoot: true }); + // provider.calls: [scout, plan, a1-turn1, a1-turn2, a2-turn1, a2-turn2, a3-turn1, ...] + // the attempt-3 opening prompt is the one carrying the ratchet note + assert.ok(provider.calls.some((p) => typeof p === 'string' && p.includes('RATCHET')), + 'the worker was told, in-prompt, that it is repeating itself'); +}); + +test('Layer R ON: a THIRD consecutive fixated attempt escalates to VERBATIM — the worker\'s own teed content is surfaced back (the full tee path, end-to-end)', async (t) => { + const wd = makePatient(t); + // four attempts: three non-'ok' rewrites of the one target (fixation each + // comparison), then the fix. streak 1 (summary) at attempt 3, streak 2 + // (verbatim) at attempt 4 — the verbatim note carries attempt 3's own bytes. + const provider = scriptedProvider([ + { text: 'scout' }, + { text: PLAN(wd) }, + { toolCalls: [tcall('t1', 'shell_write', { path: join(wd, 'tests', 'test_x.mjs'), content: 'placeholder 1\n' })] }, { text: 'a1' }, + { toolCalls: [tcall('t2', 'shell_write', { path: join(wd, 'tests', 'test_x.mjs'), content: 'placeholder 2\n' })] }, { text: 'a2' }, + { toolCalls: [tcall('t3', 'shell_write', { path: join(wd, 'tests', 'test_x.mjs'), content: 'SENTINEL placeholder 3\n' })] }, { text: 'a3' }, + { toolCalls: [tcall('t4', 'shell_write', { path: join(wd, 'tests', 'test_x.mjs'), content: 'ok — fixed\n' })] }, { text: 'a4' }, + ]); + const { outcome, events } = await go(wd, provider, { layerRoot: true, capRuns: 4 }); + const stages = events.filter((e) => e.type === 'root-injected').map((e) => e.stage); + assert.deepEqual(stages, ['summary', 'verbatim'], 'the ratchet escalated summary → verbatim across two stuck episodes'); + // the verbatim note carries the worker's OWN previous content (attempt 3's bytes) + assert.ok(provider.calls.some((p) => typeof p === 'string' && p.includes('STILL repeating') && p.includes('SENTINEL placeholder 3')), + 'the verbatim stage surfaced the worker\'s own teed content back to it'); + assert.equal(outcome, 'green'); +}); + +test('Layer R OFF (default): the SAME fixation script emits NO root-injected — armed only when asked', async (t) => { + const wd = makePatient(t); + const provider = fixationScript(wd); + const { outcome, events } = await go(wd, provider); // layerRoot defaults false + assert.equal(events.filter((e) => e.type === 'root-injected').length, 0, 'inert by default'); + assert.ok(!provider.calls.some((p) => typeof p === 'string' && p.includes('RATCHET')), 'no note reaches the worker'); + assert.equal(outcome, 'green'); +}); + +test('Layer R + NATIVE (clipipe): excluded — the native worker has no onToolResult seam, so the ratchet stays inert even under fixation', async (t) => { + const wd = makePatient(t); + const jv = validateJob(JOB(wd, { provider: 'clipipe-subscription' })); + const nativeProvider = scriptedNativeFactory([ + { turns: [{ text: 'scout' }] }, + { turns: [{ text: PLAN(wd) }] }, + { turns: [{ tool: 'shell_write', args: { path: join(wd, 'tests', 'test_x.mjs'), content: 'placeholder 1\n' } }, { text: 'a1' }] }, + { turns: [{ tool: 'shell_write', args: { path: join(wd, 'tests', 'test_x.mjs'), content: 'placeholder 2\n' } }, { text: 'a2' }] }, + { turns: [{ tool: 'shell_write', args: { path: join(wd, 'tests', 'test_x.mjs'), content: 'ok now\n' } }, { text: 'a3' }] }, + ]); + const { events, emit } = collector(); + const outcome = await runPlan(jv.job, { workdir: wd, nativeProvider, emit, capRuns: 3, layerRoot: true, remainingUsd: () => 1.5 }); + assert.equal(events.filter((e) => e.type === 'root-injected').length, 0, 'Layer R is not wired on the native surface (F48 fallback, not the experiment surface)'); + assert.equal(outcome, 'green'); +}); + // ── module 4d: NATIVE clipipe (BA-16). The plan flow is provider-agnostic — // only the WORKER differs. Live-POC-proven that the REAL provider+gate governs; // these drive OUR executor branch deterministically via a scripted native From b0c660410cf394c52bb7ac70ea3d4ec0a321c1cf Mon Sep 17 00:00:00 2001 From: hamr0 Date: Thu, 23 Jul 2026 19:13:36 +0200 Subject: [PATCH 3/7] review (QA pass): wire Layer R into the close-fix loop; document F49 over-rejection MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Second-model review of the F49+F50 diff surfaced two real MEDIUM findings (three others investigated and refuted — no stale-pendingProbe leak, no content leak, faithful tee mirror). Both fixed TDD-first: 1. F50 gap: runPlan's outer close-fix loop (a full ralph loop judged by the REAL close) was still root-less under layerRoot — the same silent-no-op F50 fixes, in the plan flow's likeliest fixation site (full menu, command-judged). Wired with red-set = the close's own gapKeep (raw close output, so the ^-anchor works). New event phase 'fix'; a fix-loop fixation test drives it end-to-end. 2. F49 false-positive class named symmetrically with the false-negative one: anchor-disambiguated repeated-record patterns ((?:^- .+$\n?)+) run linearly but are flagged by shape. Fail-safe direction (never admits an exponential), cost is one redraft. Three REDOS_OVERREJECTED regression tests lock them as accepted limitations so a future "smarter" detector can't turn them into false negatives (the dangerous direction). Detector unchanged. 559/559, typecheck + build clean. Docs: FINDINGS F49/F50, LAYERS, CHANGELOG, context. Co-Authored-By: Claude Opus 4.8 (1M context) --- CHANGELOG.md | 9 +++++---- bareloop.context.md | 9 +++++---- docs/01-product/LAYERS.md | 13 +++++++------ docs/FINDINGS.md | 20 ++++++++++++++++++++ src/plan.js | 13 +++++++++++++ src/planrun.js | 15 ++++++++++++++- tests/plan.test.js | 15 +++++++++++++++ tests/planrun.test.js | 29 +++++++++++++++++++++++++++++ 8 files changed, 108 insertions(+), 15 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index b2b1c03..a263320 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,10 +9,11 @@ feature lands, **patch** = docs, fixes, scaffolding. ### Added - **Layer R wired into the plan-v1 flow** (F50). The within-run ratchet (`layerRoot`) now - engages per step in `runPlan`, not only on the legacy `steps[]` path — so a plan-shape job - can emit `root-injected` and the pre-registered ON-vs-OFF default-flip read becomes possible - on the accepted surface. Red-set = the exit evaluator's own gap; the write-tee is wired so - same-path target rewrites are visible to the detector. Still OFF by default (`layerRoot: + engages in `runPlan` — one root per EXECUTE step (red-set = the exit evaluator's own gap) + AND in the outer close-fix loop (red-set = the close's own `gapKeep`) — not only on the + legacy `steps[]` path. So a plan-shape job can emit `root-injected` and the pre-registered + ON-vs-OFF default-flip read becomes possible on the accepted surface. The write-tee is wired + so same-path target rewrites are visible to the detector. Still OFF by default (`layerRoot: false`, F41). Excluded on native/clipipe (no `onToolResult` seam — F48 fallback surface). ### Fixed diff --git a/bareloop.context.md b/bareloop.context.md index 9319e10..a46cf82 100644 --- a/bareloop.context.md +++ b/bareloop.context.md @@ -378,10 +378,11 @@ missed, and only content actually in the file is ever described as having landed spine is append-only). State dies with the run: this is within-run scratch, not across-run memory. `layerRoot: true` is the ON/experimental arm; the default is OFF. It is wired on BOTH surfaces (F50): the legacy `steps[]` path (`interpret`) and the accepted -plan-v1 flow (`runPlan`, one root per step). On the plan flow the red-set is the exit -evaluator's OWN gap (the whole normalized complaint) rather than a single close's gapKeep, -and the write-tee makes same-path target rewrites visible to the detector; it is NOT wired -on the native/clipipe worker (no `onToolResult` seam — F48 fallback surface). Status (F41): +plan-v1 flow (`runPlan`, one root per EXECUTE step plus one in the outer close-fix loop). On +the plan flow the per-step red-set is the exit evaluator's OWN gap (the whole normalized +complaint) rather than a single close's gapKeep, while the fix loop uses the close's own +gapKeep (raw close output); the write-tee makes same-path target rewrites visible to the +detector. It is NOT wired on the native/clipipe worker (no `onToolResult` seam — F48 fallback). Status (F41): the ratchet is MEASURED inert on every current job — two frozen probes + an archive sweep read 0 fixated pairs in 14 (the F21 repetition disease was a broken-loop symptom, since cured). Because ON has therefore never won its own A/B, it ships **OFF by default (decided diff --git a/docs/01-product/LAYERS.md b/docs/01-product/LAYERS.md index b64602f..a5c39e6 100644 --- a/docs/01-product/LAYERS.md +++ b/docs/01-product/LAYERS.md @@ -148,12 +148,13 @@ read (repetition drop, ON vs OFF) DEFERS to the first run whose spine records **Wired into the plan-v1 flow (2026-07-23, F50).** Until then the ratchet was wired only into the legacy `steps[]` path (`interpret.js`); the accepted plan flow silently ignored `layerRoot`, so it could never emit `root-injected`. Now `runPlan` engages one root per -step (each micro-wheel is the Layer-1 atom): red-set = the exit evaluator's own gap -(`gapKeep '\S'`, the whole normalized complaint — format-independent, since a check's -`^`-anchored gapKeep does not survive the exit wrapper `check "x" red: …`), and the -write-tee is wired so same-path target rewrites are visible to the detector (the cumulative -audit dedups by path and cannot see them alone). Excluded on native/clipipe (no -`onToolResult` seam; F48 fallback surface, not the experiment surface). +EXECUTE step (each micro-wheel is the Layer-1 atom; red-set = the exit evaluator's own gap, +`gapKeep '\S'`, the whole normalized complaint — since a check's `^`-anchored gapKeep does +not survive the exit wrapper `check "x" red: …`) AND one in the outer close-fix loop (red-set += the close's own `gapKeep`, the raw close output where the anchor works). The write-tee is +wired so same-path target rewrites are visible to the detector (the cumulative audit dedups +by path and cannot see them alone). Excluded on native/clipipe (no `onToolResult` seam; F48 +fallback surface, not the experiment surface). > **⚠ Layer 2/3 TODO — decide the Layer R default.** The wiring now EXISTS on the accepted > surface (F50); the experiment is possible but not yet run. Layer 2's narrow micro-wheel diff --git a/docs/FINDINGS.md b/docs/FINDINGS.md index 544b965..c5832f3 100644 --- a/docs/FINDINGS.md +++ b/docs/FINDINGS.md @@ -2471,6 +2471,17 @@ is honest about its bound — exotic overlapping-alternation blowup is out of sc red-case + a detail-names-the-footgun test (31 tests), plus the empirical 33-char hang above as the "the test can fail" proof. Full suite green. +**Named over-rejection (review 2026-07-23).** The shape-only scan also has a false-POSITIVE +class, now documented symmetrically with the false-negative one: anchor/delimiter-disambiguated +repeated-record patterns (`(?:^- .+$\n?)+`, `(?:CHANGELOG:.+\n)+`) run LINEARLY in a real engine +(review measured 100k reps → 6ms) but are flagged by the nested-quantifier SHAPE. This is the +FAIL-SAFE direction — the reject never ADMITS an exponential pattern — and the cost is a single +mechanical redraft (the drafter drops the outer `+`). Detecting "safe because anchored" needs the +real engine we declined, and a wrong guess would admit an exponential pattern, so the shape reject +stands. Three `REDOS_OVERREJECTED` regression tests lock these as accepted limitations — they must +stay flagged, because a future "smarter" detector turning them into false NEGATIVES is the +dangerous direction. + **Lesson.** A security scan's value is the coverage table, not just the hits: the one finding here is a LOW self-DoS, and naming it against a CLEAN arbiter-integrity sweep is what makes "clean" auditable rather than asserted. And the fix-shape choice was itself an @@ -2508,6 +2519,15 @@ from `interpret.js` into `mkWorker`; `executeStep` creates one root per step and nothing to the write-set and the detector would see an empty delta. The tee is what makes the rewrite visible — so without it the summary stage never fires either (traced, then tested). +**The close-fix loop is wired too (review 2026-07-23).** The first cut wired only the EXECUTE +micro-wheels; the QA pass caught that `runPlan`'s outer close-fix loop (a full ralph loop judged +by the REAL close after all steps green) still ran root-less — the SAME silent-no-op this finding +exists to kill, and arguably the likeliest place fixation manifests (the fix worker holds the +full menu and is judged by a command, not a form-only exit). Now wired with its red-set = the +CLOSE's own `gapKeep` (the gap here is the raw close output, unwrapped, so the `^`-anchored +pattern matches — unlike the exec steps' exit-eval gap, which uses `\S`). Its event carries +`phase: 'fix'`; a dedicated test drives a fix-loop fixation end-to-end. + **Native excluded, by construction.** The clipipe native worker exposes no `onToolResult` seam, so the tee cannot settle and same-path rewrites are blind — `root` is `null` for native (documented, not silent; F48 already ruled native OUT-as-peer, so this is the fallback surface, diff --git a/src/plan.js b/src/plan.js index 27426c1..46b0f0c 100644 --- a/src/plan.js +++ b/src/plan.js @@ -75,6 +75,19 @@ function unboundedQuantLen(src, i) { * (an external native dep we do not take for a LOW issue); exotic * overlapping-alternation blowup is out of scope by decision. * + * SCOPE IS ASYMMETRIC, both directions named on purpose: + * - false NEGATIVE: overlapping-alternation blowup (`(a|ab)+`-class) is not + * detected — out of scope (self-DoS only, no arbiter compromise). + * - false POSITIVE: a group whose repetitions are disambiguated by a literal + * anchor/delimiter (`(?:^- .+$\n?)+`, `(?:CHANGELOG:.+\n)+`) is FLAGGED even + * though a real engine runs it linearly — the scan sees the nested-quantifier + * SHAPE, not the disambiguation. Rejecting it is the FAIL-SAFE direction (it + * never admits an unsafe pattern), and the cost is bounded: the plan drafter + * gets a mechanical gap and rewrites (drop the outer `+`, or match once). + * Detecting "safe because anchored" needs the same real engine we declined, + * and guessing it wrong would ADMIT an exponential pattern — so the shape + * reject stands, and the over-rejection is a named, accepted limitation. + * * The input is guaranteed to compile as a RegExp (checked first by the caller), * so this scan assumes valid, balanced JS regex syntax. * @param {string} src a compiled regex source string diff --git a/src/planrun.js b/src/planrun.js index 064c373..3f7dca1 100644 --- a/src/planrun.js +++ b/src/planrun.js @@ -749,16 +749,29 @@ export async function runPlan(job, { workdir, provider, nativeProvider, emit, re emit('fix-loop', { gapBytes: Buffer.byteLength(post.gap ?? '') }); let fixOutcome; try { - const w = await mkWorker({ granted: ceiling, phase: 'fix', attemptRounds: maxStepRounds, attempts: capRuns, writable: true }); + // Layer R for the close-fix loop — the plan flow's single ralph loop judged + // by the REAL close (the closest analog to interpret.js's loop, and the + // likeliest place fixation manifests: the fix worker has the full menu and + // is judged by a command, not a form-only exit). The red-set is the CLOSE's + // own gapKeep (the gap here is the raw close output, unwrapped — so the + // `^`-anchored pattern matches, unlike the exec steps' exit-eval gap). Same + // native exclusion (no onToolResult seam ⇒ the tee cannot settle). + const fixRoot = layerRoot && !native + ? createRoot({ gapKeep: job.close.gapKeep, redact: scrub, writesInformative: true }) + : null; + const w = await mkWorker({ granted: ceiling, phase: 'fix', attemptRounds: maxStepRounds, attempts: capRuns, writable: true, root: fixRoot }); /** @param {number} iteration @param {string} [gap] */ const middle = async (iteration, gap) => { w.setIteration(iteration); + const rootInj = fixRoot ? fixRoot.observe({ iteration, gap, writes: w.workerWrites() }) : null; + if (rootInj) emit('root-injected', { phase: 'fix', ...rootInj.event }); await w.ask([ 'The job\'s final verification is failing. Fix the repository so it passes.', `Repository root (absolute): ${workdir}\nEvery path you pass to a tool MUST be absolute and inside this root.`, artifacts.length > 0 && `Working context (read-only) — the plan's steps produced:\n${artifacts.map((a) => `[${a.id}] ${a.text}`).join('\n\n')}`, !gap && post.gap && `The verification's output on the tree as it stands (not an attempt of yours):\n${post.gap}`, gap && `Previous attempt failed the verification:\n${gap}`, + rootInj && rootInj.note, ].filter(Boolean).join('\n\n')); }; fixOutcome = await ralph({ diff --git a/tests/plan.test.js b/tests/plan.test.js index da63071..e8aad1e 100644 --- a/tests/plan.test.js +++ b/tests/plan.test.js @@ -279,6 +279,21 @@ for (const src of REDOS_GOOD) { }); } +// Named, ACCEPTED over-rejection (F49 false-positive class, review 2026-07-23): +// anchor/delimiter-disambiguated repeated-record patterns run LINEARLY in a real +// engine (measured: `(?:^- .+$\n?)+` on 100k reps → 6ms) but the shape-only scan +// flags them. This is the FAIL-SAFE direction — it never admits an exponential +// pattern — and the cost is one mechanical redraft. Locked here so the reject is +// a documented limitation, not a silent surprise: if a future change makes the +// detector "smarter", these must stay SAFE (false-negatives are the dangerous +// direction). The drafter's escape hatch: drop the outer `+` (match one record). +const REDOS_OVERREJECTED = ['(?:^- .+$\\n?)+', '(?:CHANGELOG:.+\\n)+', '(\\d+\\.\\d+)+']; +for (const src of REDOS_OVERREJECTED) { + test(`hasNestedQuantifier over-rejects (accepted, fail-safe): ${src}`, () => { + assert.equal(hasNestedQuantifier(src), true, `${src} is flagged by shape — an accepted over-rejection, never a false negative`); + }); +} + test('the ReDoS red detail names the footgun (the gap must let the replan rewrite, not guess)', () => { const r = validatePlan(mut((p) => { p.steps[0].exit[0].pattern = '(a+)+$'; }), OPTS); assert.match(r.reds[0].detail ?? '', /quantifier/i); diff --git a/tests/planrun.test.js b/tests/planrun.test.js index d5f3e5e..0ebfcb7 100644 --- a/tests/planrun.test.js +++ b/tests/planrun.test.js @@ -456,6 +456,35 @@ test('Layer R OFF (default): the SAME fixation script emits NO root-injected — assert.equal(outcome, 'green'); }); +test('Layer R ON: the outer close-fix loop ALSO ratchets — fixation there (judged by the REAL close) fires root-injected with phase:fix', async (t) => { + const wd = makePatient(t); + // the check greens on "ok" (so the step greens), but the CLOSE additionally + // wants a DONE marker — so the step passes, the outer close reds, and the fix + // loop runs. The fix worker rewrites the file twice without DONE (same close + // red-set) then adds it: the fix loop's own ratchet must fire at attempt 3. + writeFileSync(join(wd, 'close.mjs'), `import { existsSync, readFileSync } from 'node:fs'; +const p = new URL('./tests/test_x.mjs', import.meta.url).pathname; +const t = existsSync(p) ? readFileSync(p, 'utf8') : ''; +if (t.includes('DONE')) process.exit(0); +console.log('FAILED close: the test is missing the DONE marker'); process.exit(1);\n`); + const job = JOB(wd, { close: { type: 'predicate', cmd: 'node close.mjs', expect: 0, gapKeep: '^FAILED' } }); + const provider = scriptedProvider([ + { text: 'scout' }, + { text: PLAN(wd) }, + { toolCalls: [tcall('t1', 'shell_write', { path: join(wd, 'tests', 'test_x.mjs'), content: 'ok — asserts x\n' })] }, { text: 'step done' }, + // fix loop: two rewrites still missing DONE (fixation), then the fix + { toolCalls: [tcall('t2', 'shell_write', { path: join(wd, 'tests', 'test_x.mjs'), content: 'ok stub 1\n' })] }, { text: 'f1' }, + { toolCalls: [tcall('t3', 'shell_write', { path: join(wd, 'tests', 'test_x.mjs'), content: 'ok stub 2\n' })] }, { text: 'f2' }, + { toolCalls: [tcall('t4', 'shell_write', { path: join(wd, 'tests', 'test_x.mjs'), content: 'ok DONE\n' })] }, { text: 'f3' }, + ]); + const { outcome, events } = await go(wd, provider, { job, layerRoot: true }); + const inj = events.filter((e) => e.type === 'root-injected'); + assert.ok(inj.some((e) => e.phase === 'fix' && e.stage === 'summary'), + `the fix loop ratchets under fixation; got ${JSON.stringify(inj)}`); + assert.ok(provider.calls.some((p) => typeof p === 'string' && p.includes('RATCHET')), 'the note reached the fix worker'); + assert.equal(outcome, 'green'); +}); + test('Layer R + NATIVE (clipipe): excluded — the native worker has no onToolResult seam, so the ratchet stays inert even under fixation', async (t) => { const wd = makePatient(t); const jv = validateJob(JOB(wd, { provider: 'clipipe-subscription' })); From 6f463f6969105f6b284836c38f37ec94cd1f5ae7 Mon Sep 17 00:00:00 2001 From: hamr0 Date: Thu, 23 Jul 2026 23:25:44 +0200 Subject: [PATCH 4/7] =?UTF-8?q?PRD=20v1.23:=20the=20two=20workflow=20shape?= =?UTF-8?q?s=20=E2=80=94=20plan-v1=20is=20the=20target=20for=20all=20verdi?= =?UTF-8?q?cts?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Corrects v1.22's premature "steps[] sunset on landing": Layer 2 landed for the GREEN verdict only; plan-v1 locks soft-green/hitl, which still run only on the legacy steps[] path (hitl is the only working draft-PR flow). Records that the green→plan-v1 / soft-green+hitl→legacy split is a build-order artifact, not a design — plan-v1 is the go-forward home for all three verdicts (the agent authors the workflow; legacy is operator-hardcoded scaffolding). Honest sunset gate: legacy retires when plan-v1 admits+implements hitl + soft-green (the verdict-classes rung, post-Layer-3, with the RSI judged-floor caveat for rubric), as a rewrite-deletion parked for hamr's explicit go. Notes F50 moved the last stranded capability (Layer R) onto plan-v1. Co-Authored-By: Claude Opus 4.8 (1M context) --- docs/01-product/PRD.md | 55 ++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 55 insertions(+) diff --git a/docs/01-product/PRD.md b/docs/01-product/PRD.md index d71afae..fcf40f6 100644 --- a/docs/01-product/PRD.md +++ b/docs/01-product/PRD.md @@ -1313,3 +1313,58 @@ plan (the plan-as-executed spine already holds the checkpoint; not yet wired), a **separate clipipe-subscription battery** to validate the native surface (module 4d) on its own baseline. Process note (F47): run 1 fired without the frozen pre-fire health probe (4 casualties), and a single-message liveness probe is not a sustained-load throughput check. + +## Addendum v1.23 — 2026-07-23 (the two workflow shapes: plan-v1 is the target for ALL verdicts; legacy `steps[]` sunsets at the verdict-classes rung, not at Layer 2 landing — F49/F50, hamr) + +**Corrects v1.22.** v1.22 recorded "`steps[]` and config-v1 sunset on landing." That was +premature for `steps[]` (config-v1 is genuinely dead — F22). Layer 2 landed for the **green +verdict only**: plan-v1 v1 admits `green` and LOCKS `soft-green`/`hitl` (`LOCKED_VERDICTS` — +declaring one is a `request-red`). So the legacy `steps[]` path could NOT retire at landing — +it uniquely hosts the two locked verdicts, and one of them (`hitl`) is the only *working* +human-verdict path (the draft-PR flow, `run.js` `openDraftPr`). + +**The mapping today (honest, no papering over):** + +| verdict | shape it runs on | status | +|---|---|---| +| `green` | **plan-v1** (`planrun.js`, agent-authored plan) | shipped v0.5.0, ACCEPTED (F47) | +| `soft-green` (rubric) | legacy `steps[]` (`interpret.js`, operator-authored) | locked in plan-v1 | +| `hitl` (draft-PR) | legacy `steps[]` (`interpret.js`) | locked in plan-v1 | + +**Is the split principled? No — it is a build-order artifact, not a design.** There is no +reason `soft-green`/`hitl` "belong" on the old shape; plan-v1 is the go-forward home for ALL +three verdicts. The new way is better on the product's own thesis — **the agent authors the +workflow** (validator-gated before tokens, one wallet, in-run self-checks F46/F47, and now +Layer R F50). Legacy is operator-HARDCODED steps: a human writes the workflow, the exact +thing bareloop exists to not require ("automate this — I don't know the best workflow"). If +you already know the steps you do not need the emergence; that is relayfact's job, not +bareloop's (§8). **Nothing about legacy is better for the product's goals.** The one real +reason it stays is pragmatic sequencing: its hitl middle is working code today, and hitl is +not needed until the non-code-jobs goal (§8 later-goal) — keeping it is cheaper than +rebuilding hitl before it is wanted, NOT a reason to preserve the split. + +**Sunset criteria (the honest gate — supersedes v1.22's "on landing"):** legacy `steps[]` +retires when plan-v1 **admits AND implements** both locked verdicts: +1. **`hitl` in the plan flow** — the draft-PR / human-verdict path ported onto `planrun.js` + (the door to non-code jobs: resume/LinkedIn, §8 later-goal). +2. **`soft-green` in the plan flow** — a rubric close, WITH its RSI caveat first: a rubric + close is self-consistency in disguise and needs a judged-floor analog before it can gate + anything (RSI-LEARNINGS / v1.21). + +That is the **verdict-classes rung**, sequenced after Layer 3 (the soft-green/hitl ladder +follows the deterministic-close infra, §8/§10). Retirement is a **rewrite-deletion** (the +`run.js` legacy for-loop, `interpret.js`'s legacy dispatch, `job.js`'s `steps[]` validation, +`interpret.test.js`) done in one deliberate gated pass — never a copy (graduation is a +rewrite). It is a **product-shape decision parked for hamr's explicit go**, not shipped +unilaterally. + +**This session (F49/F50), landing in v0.5.1.** Layer R was wired onto plan-v1 — one root per +EXECUTE step (red-set = the exit evaluator's own gap) and one in the outer close-fix loop +(red-set = the close's own `gapKeep`) — moving the **last capability that was stranded on +legacy** onto the go-forward surface (so it does not retire with legacy). A plan-flow job can +now emit `root-injected`, making the pre-registered ON-vs-OFF default-flip read (v1.20) +possible on the accepted surface; default stays OFF until that evidence lands (F41). F49 +hardened the agent-authored `artifact-written` exit regex — a nested-quantifier ReDoS reject +at the validation gate (LOW self-DoS, no arbiter compromise; input-bounding refuted as +theater by a 33-char-body hang). After v0.5.1, the ONLY legacy-unique surface left is the two +locked verdicts above — so the sunset gate is exactly (1)+(2). From 840798535b798421c27d9f1f8361342c9bba7c92 Mon Sep 17 00:00:00 2001 From: hamr0 Date: Fri, 24 Jul 2026 00:01:26 +0200 Subject: [PATCH 5/7] review (medium): close F49 wrapped-group ReDoS false negative MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit /code-review medium branch flagged that hasNestedQuantifier missed the redundant-wrapper class — ((a+))+, (?:(a+))*, (((a+)))+, ((\w+))* — the same exponential class as (a+)+ with an extra grouping level. The flat-only scan propagated an inner repeat to the parent only when the group was DIRECTLY re-quantified, so the wrapped forms validated and could still hang the untimed exit evaluator (each measured >8s on ~29 chars). Fix: on a group close, propagate the inner-repeat flag up through the enclosing group when the group's own body already repeats. MONOTONIC — only ever sets quant=true, so it can only ADD rejections (the fail-safe direction) and provably cannot introduce a new false negative (the dangerous one). All 17 REDOS_GOOD and 3 REDOS_OVERREJECTED cases unchanged; +5 wrapper cases in REDOS_BAD. Full suite 564/564, typecheck, build:types clean. Docs: CHANGELOG / PRD v1.23 / bareloop.context.md (adopter contract) / FINDINGS F49 updated to record the wrapper class + the monotonic close. README unchanged (pitch-only; a detector detail is the wrong genre). Co-Authored-By: Claude Opus 4.8 (1M context) --- CHANGELOG.md | 10 +++++++--- bareloop.context.md | 2 +- docs/01-product/PRD.md | 5 ++++- docs/FINDINGS.md | 21 +++++++++++++++++++-- src/plan.js | 15 +++++++++++++-- tests/plan.test.js | 6 ++++++ 6 files changed, 50 insertions(+), 9 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index a263320..c72f795 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -19,9 +19,13 @@ feature lands, **patch** = docs, fixes, scaffolding. ### Fixed - **F49 (security-hardening): the agent-authored `artifact-written` regex can no longer hang the exit evaluator.** `plan.js` rejects nested-quantifier ReDoS patterns (`(a+)+`, `(\d*)*`, - `(x+){1,}`) at the validation gate, before any tokens burn. LOW self-DoS, no arbiter - compromise; input-bounding was rejected as theater (a 33-char body hangs) and a JS regex - timeout as disproportionate (worker/RE2 dep). + `(x+){1,}`, and redundant-wrapper forms like `((a+))+` / `(?:(a+))*` / `(((a+)))+`) at the + validation gate, before any tokens burn. The wrapper class was a review-caught false negative + in the first (flat-only) scan — each measured to hang `RegExp.test` >8s on ~29 chars — closed + by propagating an inner repeat up through the enclosing group; the change is MONOTONIC (it only + ever adds rejections, the fail-safe direction, and cannot introduce a new false negative). LOW + self-DoS, no arbiter compromise; input-bounding was rejected as theater (a 33-char body hangs) + and a JS regex timeout as disproportionate (worker/RE2 dep). - **F50: `runJob` no longer silently ignores `layerRoot` for plan-shape jobs** (was accepted and dropped — an advertised no-op). diff --git a/bareloop.context.md b/bareloop.context.md index a46cf82..e09c007 100644 --- a/bareloop.context.md +++ b/bareloop.context.md @@ -173,7 +173,7 @@ middle writes; `validatePlan` gates it against the SIGNED job spec before tokens | `steps[].tools` | non-empty unique subset of the SPEC ceiling | a verb beyond the ceiling reds `verb-escape` with the verb as structured data (overreach, distinct from the operator-side `request-red`); `run` is `verb-escape` at every layer | | `steps[].rounds` | int 1..shell cap (default 40) | the step's per-attempt tool-round bound (the Gate's `maxTurns` natively) | | `steps[].target` | path inside the fence | v1.18 deliverable; REQUIRED on write-granted steps | -| `steps[].exit` | 1..2 items (`MAX_EXITS_PER_STEP`), ALL must pass (AND-only, no OR/NOT) | closed menu (`EXIT_TYPES`): `artifact-written(path, pattern?)` · `tree-changed(scope)` · `json-valid(path)` · `check-passes(name)`. `check-passes` must name a SIGNED check (`check-unknown` red names the signed menu); on a write-granted step it must be paired with `tree-changed` (`exit-illegal` — the seed tree is green, a lone check would pass untouched, F17/F46). Exits verify FORM, not truth — progress gates; the operator's close stays the one arbiter | +| `steps[].exit` | 1..2 items (`MAX_EXITS_PER_STEP`), ALL must pass (AND-only, no OR/NOT) | closed menu (`EXIT_TYPES`): `artifact-written(path, pattern?)` · `tree-changed(scope)` · `json-valid(path)` · `check-passes(name)`. `check-passes` must name a SIGNED check (`check-unknown` red names the signed menu); on a write-granted step it must be paired with `tree-changed` (`exit-illegal` — the seed tree is green, a lone check would pass untouched, F17/F46). `artifact-written.pattern` must compile AND survive a ReDoS shape check — an unbounded quantifier over a group that itself repeats unboundedly (`(a+)+`, `(\d*)*`, even wrapped: `((a+))+`) is an `invalid-value` red (F49, catastrophic-backtracking footgun; rewrite without a repeated group inside a repeat). Exits verify FORM, not truth — progress gates; the operator's close stays the one arbiter | Red vocabulary (all three validators): `parse-error`, `unknown-field`, `missing-required`, `invalid-value`, `bounds`, `duplicate-id`, `close-type`, `close-hierarchy`, diff --git a/docs/01-product/PRD.md b/docs/01-product/PRD.md index fcf40f6..11f7da3 100644 --- a/docs/01-product/PRD.md +++ b/docs/01-product/PRD.md @@ -1366,5 +1366,8 @@ now emit `root-injected`, making the pre-registered ON-vs-OFF default-flip read possible on the accepted surface; default stays OFF until that evidence lands (F41). F49 hardened the agent-authored `artifact-written` exit regex — a nested-quantifier ReDoS reject at the validation gate (LOW self-DoS, no arbiter compromise; input-bounding refuted as -theater by a 33-char-body hang). After v0.5.1, the ONLY legacy-unique surface left is the two +theater by a 33-char-body hang), extended after a follow-up review that caught a false +negative in the flat-only scan (redundant-wrapper forms like `((a+))+` slipped through and +still hang >8s) — closed monotonically by propagating an inner repeat up through the +wrapping group, the fail-safe direction that only ever adds rejections. After v0.5.1, the ONLY legacy-unique surface left is the two locked verdicts above — so the sunset gate is exactly (1)+(2). diff --git a/docs/FINDINGS.md b/docs/FINDINGS.md index c5832f3..9c9b834 100644 --- a/docs/FINDINGS.md +++ b/docs/FINDINGS.md @@ -2467,8 +2467,8 @@ after the compile check and reds an `invalid-value:…pattern` when a group repe skips escaped atoms and character classes (so `\(a+\)+` and `[a+]+` are safe), treats bounded outer repeats (`(a+)?`, `(a+){2}`, `(a+){1,3}`) as safe (polynomial, self-authored body), and is honest about its bound — exotic overlapping-alternation blowup is out of scope BY DECISION -(self-DoS only, no arbiter compromise). TDD: a 12-bad / 17-good detector battery + a validator -red-case + a detail-names-the-footgun test (31 tests), plus the empirical 33-char hang above +(self-DoS only, no arbiter compromise). TDD: a 17-bad / 17-good detector battery + a validator +red-case + a detail-names-the-footgun test, plus the empirical 33-char hang above as the "the test can fail" proof. Full suite green. **Named over-rejection (review 2026-07-23).** The shape-only scan also has a false-POSITIVE @@ -2482,6 +2482,23 @@ stands. Three `REDOS_OVERREJECTED` regression tests lock these as accepted limit stay flagged, because a future "smarter" detector turning them into false NEGATIVES is the dangerous direction. +**Review-caught FALSE NEGATIVE, closed monotonically (2026-07-24, `/code-review medium branch`).** +The first (flat-only) scan claimed the redundant-wrapper class was covered, but it was NOT: a +group repeating unboundedly is the SAME exponential class whether the outer quantifier sits +directly on it (`(a+)+`) or one level of wrapping away (`((a+))+`, `(?:(a+))*`, `(((a+)))+`, +`((\w+))*`), and the shape scan only propagated the inner repeat when the group was DIRECTLY +re-quantified — so the wrapped forms slipped through and validated. Each was MEASURED to hang +`RegExp.test` >8s on ~29 chars (the `((\d*))*` reading needed a digit body — a letter body and a +dropped backslash made it read "fast", a harness confound caught before believing it). Fix: on a +group close, propagate the inner-repeat flag up through the enclosing group when the group's own +body already repeats (not only when it is directly re-quantified). The change is MONOTONIC — it +only ever SETS `quant=true`, so it can only ever ADD rejections: it widens over-rejection (the +fail-safe direction) and provably CANNOT introduce a new false negative (the dangerous one). All +17 `REDOS_GOOD` and 3 `REDOS_OVERREJECTED` cases stay unchanged; 5 wrapper cases added to +`REDOS_BAD`. This is distinct from the "never sharpen the detector" rule above: that rule bars +chasing false POSITIVES (which risks false negatives); closing a false NEGATIVE by adding +rejections is the same fail-safe direction the rule protects. + **Lesson.** A security scan's value is the coverage table, not just the hits: the one finding here is a LOW self-DoS, and naming it against a CLEAN arbiter-integrity sweep is what makes "clean" auditable rather than asserted. And the fix-shape choice was itself an diff --git a/src/plan.js b/src/plan.js index 46b0f0c..95c8ad7 100644 --- a/src/plan.js +++ b/src/plan.js @@ -64,7 +64,10 @@ function unboundedQuantLen(src, i) { /** * F49 — a heuristic reject for the catastrophic-backtracking footgun: an * unbounded quantifier applied to a group whose body itself repeats unboundedly - * (`(a+)+`, `(\d*)*`, `([a-z]+){1,}`). Such a pattern can hang `RegExp.test` for + * (`(a+)+`, `(\d*)*`, `([a-z]+){1,}`) — including through a redundant wrapping + * group (`((a+))+`, `(?:(a+))*`), which is the SAME exponential class as `(a+)+` + * and is caught by propagating the inner repeat up through the wrapper. Such a + * pattern can hang `RegExp.test` for * seconds on a short crafted body, and `evalExits` runs the agent-authored * `artifact-written` pattern with NO timeout. This is self-DoS only — the agent * authors both the pattern and (via the worker) the artifact, so a hang burns @@ -111,7 +114,15 @@ export function hasNestedQuantifier(src) { const g = stack.pop(); const qlen = unboundedQuantLen(src, i + 1); if (g && g.quant && qlen) return true; // group repeats unboundedly AND its body did too - if (qlen && stack.length) stack[stack.length - 1].quant = true; // the group is an unbounded-repeated atom of its parent + // The group is an unbounded-repeated atom of its parent when it is directly + // re-quantified (qlen) OR its own body already repeats unboundedly (g.quant): + // a redundant wrapper — ((a+))+ , (?:(a+))* , (((\d*)))+ — is the SAME + // exponential class as (a+)+, so an inner repeat must propagate THROUGH the + // enclosing group or the outer quantifier's close never sees it. Propagation + // is MONOTONIC (it only ever SETS quant=true → only ever adds rejections): it + // can widen over-rejection — the named FAIL-SAFE direction — but can never + // introduce a false negative, which is the dangerous one (F49). + if ((qlen || (g && g.quant)) && stack.length) stack[stack.length - 1].quant = true; i += qlen; continue; } diff --git a/tests/plan.test.js b/tests/plan.test.js index e8aad1e..70f77ea 100644 --- a/tests/plan.test.js +++ b/tests/plan.test.js @@ -262,6 +262,12 @@ for (const [name, fn, want] of RED_CASES) { const REDOS_BAD = [ '(a+)+', '(a+)*', '(a*)+', '(a*)*$', '(\\d+)+', '([a-z]+)*', '(\\w+){1,}', '((a+)+)', '(a+\\w*)+', '(a+?)+?', '(foo|ba+r)+', '(\\s+)*end', + // redundant WRAPPING group — the inner repeat is nested one level deeper than + // the outer quantifier, the same exponential class as (a+)+ (measured: each + // hangs RegExp.test >10s on ~29 chars). Caught by propagating the inner + // repeat up through the wrapper; a false-negative here is the dangerous + // direction (F49, review 2026-07-23). + '((a+))+', '(?:(a+))+', '((\\d*))*', '(((a+)))+', '((\\w+))*', ]; const REDOS_GOOD = [ 'def ', 'a+', '(abc)+', '(a+)', '(a+)?', '(a+){2}', '(a+){1,3}', From 12d96347b3f948b3c68f8e0686eab6539cdbd00d Mon Sep 17 00:00:00 2001 From: hamr0 Date: Fri, 24 Jul 2026 00:04:58 +0200 Subject: [PATCH 6/7] PRD: park medium review #2 (Layer R tee duplicated across interpret/planrun) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Record the parked decision: the tee wiring (Finding 6/7 settle logic) is duplicated ~60 lines across the legacy and plan-v1 paths, currently faithful. Not extracted — legacy is a scheduled rewrite-deletion at the verdict-classes rung, so the duplication resolves at that deletion; refactoring code slated for deletion is negative-value and risks reintroducing the blind-instrument class. Accepted cost: a drift window (both files change together) until legacy retires. Co-Authored-By: Claude Opus 4.8 (1M context) --- docs/01-product/PRD.md | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/docs/01-product/PRD.md b/docs/01-product/PRD.md index 11f7da3..7365d01 100644 --- a/docs/01-product/PRD.md +++ b/docs/01-product/PRD.md @@ -1371,3 +1371,16 @@ negative in the flat-only scan (redundant-wrapper forms like `((a+))+` slipped t still hang >8s) — closed monotonically by propagating an inner repeat up through the wrapping group, the fail-safe direction that only ever adds rejections. After v0.5.1, the ONLY legacy-unique surface left is the two locked verdicts above — so the sunset gate is exactly (1)+(2). + +**Parked (medium review #2, 2026-07-24): the Layer R tee wiring is duplicated across +`interpret.js` (legacy) and `planrun.js` (plan-v1)** — `fileHash` / `teeingTranslator` / +the deny-discard `policy` wrapper / `onToolOutcome` (the Finding 6/7 settle logic), ~60 +lines, currently faithful (no live defect). NOT extracted into a shared helper, deliberately: +legacy is a scheduled **rewrite-deletion** at the verdict-classes rung (above), so one of the +two copies is *deleted*, not refactored — extracting shared arbiter-adjacent wiring out of code +slated for deletion is negative-value, and reconciling the two shapes (interpret gates on +`mode === 'tools'`; planrun is tool-only) risks reintroducing the blind-instrument class the tee +guards against (a future write-class verb bypassing the tee). Accepted cost: a **drift window** +until legacy retires — any Finding-6/7 change or new write-class verb must be applied to BOTH +files. The clean resolution is the legacy deletion itself; revisit only if legacy outlives +Layer 3 or the tee logic starts changing often. From f54be7dc25fc1208e2da926547d6c229c37fbdc7 Mon Sep 17 00:00:00 2001 From: hamr0 Date: Fri, 24 Jul 2026 00:10:32 +0200 Subject: [PATCH 7/7] =?UTF-8?q?release:=20v0.5.1=20=E2=80=94=20Layer=20R?= =?UTF-8?q?=20on=20plan-v1=20(F50)=20+=20F49=20ReDoS=20hardening?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-Authored-By: Claude Opus 4.8 (1M context) --- CHANGELOG.md | 2 ++ package.json | 2 +- 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index c72f795..d89d2a2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,8 @@ feature lands, **patch** = docs, fixes, scaffolding. ## [Unreleased] +## [0.5.1] — 2026-07-24 + ### Added - **Layer R wired into the plan-v1 flow** (F50). The within-run ratchet (`layerRoot`) now engages in `runPlan` — one root per EXECUTE step (red-set = the exit evaluator's own gap) diff --git a/package.json b/package.json index 6599c61..a3c35b1 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "bareloop", - "version": "0.5.0", + "version": "0.5.1", "description": "Workflows that earn their own design, with receipts — agent-authored workflows for repeated, long, verifiable jobs that improve across runs under an un-gameable gate.", "type": "module", "main": "./src/index.js",