From 0070eadd53d4d50b06408cc3f5834aec96c7fa41 Mon Sep 17 00:00:00 2001 From: Samuel Reed Date: Thu, 29 Oct 2026 14:16:28 -0400 Subject: [PATCH 1/2] fix: the served gate trusts only the runner's own marker, and a findings JSON must hold one value Root cause: drop_unacceptable_l1_cache decided a record was the runner's own from its keys (error or skipped with findings: []), which a worker can write as easily as the runner. A substituted model's {"error":"unreadable","findings":[]} skipped the served verdict and left its session untriaged. Separately, every findings check used jq -e, which judges only the last value in a file, so a forged object followed by a valid one passed and L2 read the first. - The dispatcher writes .served = runner beside each record it writes itself (unreadable, noise gate, normalize failure, no engine, final-round stub). Under AUTODREAM_L1_REQUIRE_SERVED=1 that marker, plus empty findings, is the only exemption from the verdict. A runner record from before the marker is triaged once more. - A no-engine record is still dropped by shape once its source resolves an engine; keeping one under the gate now takes the marker. - One filter, L1_FINDINGS_OK under jq -s (exactly one value, an object, a findings array), replaces the five jq -e checks. Closes #167 --- CHANGELOG.md | 4 +++ CLAUDE.md | 6 ++--- bin/run.sh | 67 +++++++++++++++++++++++++++++------------------- tests/run-all.sh | 53 ++++++++++++++++++++++++++++++++++++++ 4 files changed, 100 insertions(+), 30 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index e0de137..d176c9e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,10 @@ All notable changes to cc-autodream. Format loosely follows Keep a Changelog. - **`AUTODREAM_L1_MODEL_` is keyed by the engine that makes the call, not by the harness that holds the session.** A model id belongs to an engine, so `AUTODREAM_L1_MODEL_OMP` is the model of every L1 call on the omp engine, whichever harness the session came from, and never reaches the claude CLI. A host whose harnesses each review their own sessions sees no change. One that moves a harness to another engine pins that engine's model, the same keying `AUTODREAM_L2_MODEL_` already had. - **A session with no usable L1 engine is retried once a later run gives it one.** Its error record counted as done, which held while the only cause was an adapter missing on disk. An engine choice naming no accepted adapter, a missing model and a failed login check are config, so the record now carries `no_engine: true`, and a later run of the date (a catch-up or `AUTODREAM_FORCE=1`) that resolves an engine and a model for the session's source drops it and triages the session. +### Fixed +- **`AUTODREAM_L1_REQUIRE_SERVED=1` no longer trusts a findings JSON to say who wrote it (#167).** It kept any record with `error` or `skipped` and no findings as the runner's own, so a substituted model that wrote `{"error":"unreadable","findings":[]}` left its session untriaged. The runner now marks each record it writes itself with `.served` reading `runner`, and that marker is the only thing that exempts a record from the verdict. Runner records written before this change are triaged once more. +- **A findings JSON holding two JSON values is no longer a finished triage.** Every check used `jq -e`, which judges only the last value, so a forged object followed by a valid one passed and L2 read the first. One definition, `L1_FINDINGS_OK` under `jq -s`, now requires exactly one value at every site. + ### Added - **`AUTODREAM_SESSION_ROOTS_`** lists the session roots of one adapter other than claude, colon-separated, in place of its manifest's defaults, for a harness that keeps one store per profile. - **Who each engine runs as, and which model answered each L1 call.** Each engine the run uses reports its login once through a new adapter verb, `whoami` (claude: `claude auth status --json`, no model call), logged and written to run-stats as `login_`. The claude L1 worker runs with `--output-format json`, and the new `served` verb reads the models that answered, the CLI's cost estimate at API prices and a verdict, `match` or `mismatch`, that knows the CLI's naming (`haiku` answers as `claude-haiku-5-5`). Every call goes to `findings//l1-served.tsv`. run-stats gains `l1_engine_`, `l1_served_models`, `l1_served_mismatch`, `l1_served_unmeasured` (calls whose engine reports no model, such as omp), `l1_cost_usd_estimate` and `l1_cost_unmeasured` (calls left out of that sum, so an all-omp night does not read as free). diff --git a/CLAUDE.md b/CLAUDE.md index a969823..d238374 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -109,8 +109,8 @@ plus env `CLAUDE_CODE_DISABLE_CLAUDE_MDS=1 DISABLE_TELEMETRY=1 DISABLE_ERROR_REP - **The source map is held in the environment, not read from the findings dir.** `AUTODREAM_SOURCE_MAP` (hash and adapter per session) and `AUTODREAM_L1_MODELS` (model per adapter) are built before the first model call. An L1 worker holds the Write tool and could rewrite `sessions-source.txt`, and the source decides which binary the next worker executes. `test_l1_engine_cannot_be_redirected_by_a_worker` has a worker do exactly that and checks the next session still runs on the real engine; reading the file instead fails it. - **Haiku 5.5, effort and a replaced system prompt (2026-10-07).** The claude manifest's `l1_model` is `claude-haiku-5-5` (verified: `--model claude-haiku-5-5` and the `haiku` alias both answer as it; an id the CLI does not know falls back silently, so re-check `modelUsage` in `--output-format json` when changing it). `l1-argv` and `warmup-argv` add `--effort` from `AUTODREAM_L1_EFFORT_CLAUDE`, then `AUTODREAM_L1_EFFORT`, then `high`; `off` omits it and an unknown level exits 2. They also pass `--system-prompt` instead of `--append-system-prompt`, which drops Claude Code's default prompt: one-word call 6.9k -> 5.0k input tokens, and a real triage run on a 250 KB slice cost $0.159 against $0.215 with the same number of findings. L2 is untouched. - **The engine, then the model.** A source's L1 engine is `AUTODREAM_L1_ENGINE_`, then `AUTODREAM_L1_ENGINE`, then the run's reviewer (the L2 engine) (`adapter_l1_engine`). The model is keyed by that engine, not by the source: `AUTODREAM_L1_MODEL_`, then `AUTODREAM_L1_MODEL`, then the engine's manifest `l1_model` (`adapter_l1_model`). A model id belongs to one engine, so a source-keyed pin sent omp sessions to the claude CLI with `--model deepseek/...` once a claude reviewer read them (PR #165 review). The engine choice is per source and the model pin per engine, the same as `AUTODREAM_L2_MODEL_`. -- **What answered, and whether it counts.** Every L1 call's stdout goes through the engine adapter's `served` verb with the requested model: it returns the models that answered, the cost estimate and a verdict, `match` or `mismatch`, judged by the adapter because only it knows its CLI's naming (claude: `haiku` answers as `claude-haiku-5-5`). An engine with no `served` verb (omp) is `unmeasured`. The verdict is the sixth column of `l1-served.tsv` (run-stats `l1_served_mismatch`, `l1_served_unmeasured`) and is stored beside every accepted answer, findings JSON or chunk answer, as `.served`. With `AUTODREAM_L1_REQUIRE_SERVED=1` a call whose verdict is not `match` loses its answer, and before L1 `drop_unacceptable_l1_cache` holds the cache to the same rule: a findings JSON or chunk answer from an earlier run with no verdict or a non-`match` one is dropped and triaged again (`l1_cache_dropped`). A record with `error` or `skipped` and `findings: []` is kept, because the runner writes those itself and the post-L1 passes add fields to them, so their shape alone cannot tell them from a worker's. The empty array is the test, not the key: a worker can write either key, but every record the runner writes has no findings, so one with findings is model output and is held to its verdict. -- **No engine is an error for this run's config, not forever.** A session whose engine is not executable, resolves no model, names no accepted adapter or fails `AUTODREAM_REQUIRE_LOGIN` gets a findings record with `no_engine: true` and `error: no L1 engine for this session`, counted by `l1_findings_with_error` and skipped by the retry rounds, since retrying in the same run cannot change the answer, and the run still reports. Every one of those causes is config, which can change between runs, so `drop_unacceptable_l1_cache` drops the record in a later run (a catch-up or `AUTODREAM_FORCE=1`) once that run resolves an engine and a model for the session's source, read from `AUTODREAM_SOURCE_MAP` and never from the record. Records written before the marker are recognised by their error text. Either way the record must have `findings: []`: one with findings is model output, and with `AUTODREAM_L1_REQUIRE_SERVED=1` it is held to its verdict like any other. +- **What answered, and whether it counts.** Every L1 call's stdout goes through the engine adapter's `served` verb with the requested model: it returns the models that answered, the cost estimate and a verdict, `match` or `mismatch`, judged by the adapter because only it knows its CLI's naming (claude: `haiku` answers as `claude-haiku-5-5`). An engine with no `served` verb (omp) is `unmeasured`. The verdict is the sixth column of `l1-served.tsv` (run-stats `l1_served_mismatch`, `l1_served_unmeasured`) and is stored beside every accepted answer, findings JSON or chunk answer, as `.served`. With `AUTODREAM_L1_REQUIRE_SERVED=1` a call whose verdict is not `match` loses its answer, and before L1 `drop_unacceptable_l1_cache` holds the cache to the same rule: a findings JSON or chunk answer from an earlier run with no verdict or a non-`match` one is dropped and triaged again (`l1_cache_dropped`). A record the runner wrote with no model behind it (noise-gate skip, unreadable session, normalize failure, no-engine record, final-round stub) is kept, and the only thing that says so is its `.served` reading `runner`, which the dispatcher writes beside each one. Keys and shape prove nothing: a worker can write `error`, `skipped` or `no_engine` with `findings: []` as easily as the runner can, and that record would have skipped the verdict and left the session untriaged (#167). The empty findings array is kept as a second guard. A runner record from before the marker is triaged once more and comes back marked. +- **No engine is an error for this run's config, not forever.** A session whose engine is not executable, resolves no model, names no accepted adapter or fails `AUTODREAM_REQUIRE_LOGIN` gets a findings record with `no_engine: true` and `error: no L1 engine for this session`, counted by `l1_findings_with_error` and skipped by the retry rounds, since retrying in the same run cannot change the answer, and the run still reports. Every one of those causes is config, which can change between runs, so `drop_unacceptable_l1_cache` drops the record in a later run (a catch-up or `AUTODREAM_FORCE=1`) once that run resolves an engine and a model for the session's source, read from `AUTODREAM_SOURCE_MAP` and never from the record. Records written before the marker are recognised by their error text. Either way the record must have `findings: []`. Its shape is enough to drop it, since a forged one only buys a retriage; keeping it under `AUTODREAM_L1_REQUIRE_SERVED=1` takes the runner marker above. ## Self-pollution (the eating-its-own-tail bug) @@ -293,7 +293,7 @@ A run killed during L2 leaves a complete findings dir and no report, and every s The worker's stdout used to go to `/dev/null`, so every failure looked the same: an `.err` holding the one line `Working...` plus a hand-written sentence. On 2026-09-04 that hid a dead network behind three fine transcripts, and the report blamed their size (`oversized_errored / oversized_total` read 3/4, the number that opens issue #12). Re-run by hand against a live network, those transcripts triaged in 73 to 78 seconds. Now a failed worker's `.err` records its exit code and elapsed time and the last 40 lines of its stdout, or says plainly that stdout was empty. The `.err` and `.out` are deleted only when the worker succeeds. -- **Non-empty is not valid.** A worker that writes malformed JSON, or JSON whose `.findings` is not an array, is a failure with its evidence kept, not a success. `jq -e .findings` is truthy for a string, so the check is `.findings | arrays`, on the way in (idempotency, `l1_missing_count`) and on the way out. The malformed file is removed so it cannot reach L2. +- **Non-empty is not valid.** A worker that writes malformed JSON, or JSON whose `.findings` is not an array, is a failure with its evidence kept, not a success. `jq -e .findings` is truthy for a string and judges only the last value in a file, so a forged object followed by a good one passed and L2 read the first (#167). The check is `L1_FINDINGS_OK` under `jq -s` (one value, an object, a `findings` array), the same rule `merge-chunks.sh --check` applies to chunks, used on the way in (idempotency, `l1_missing_count`), on the way out and by the skill-field pass. The malformed file is removed so it cannot reach L2. - `bin/failure-class.sh` is the single failure predicate sourced by both `run.sh` and `bin/oversized-gate.sh`. It assigns every error stub to one of four classes from its `.err`: `unclassified` when the file is missing, empty, or predates the diff --git a/bin/run.sh b/bin/run.sh index f0adf61..ab2cb0a 100755 --- a/bin/run.sh +++ b/bin/run.sh @@ -2043,6 +2043,12 @@ wait_for_network() { # 0 = network is up, 1 = gave up after the cap; no-op when return 0 } +# The one test for a usable findings JSON, run with jq -s: exactly one JSON value, an object, with a +# findings array. jq -e judges only the LAST value in a file, so a forged object followed by a good +# one passed the old `.findings | arrays` check and L2 read the first (#167). merge-chunks.sh --check +# is the same rule for chunk answers. Exported because the worker is a separate bash -c. +export L1_FINDINGS_OK='length == 1 and (.[0] | type == "object" and (.findings | type) == "array")' + l1_missing_count() { # count sessions in $SESSIONS_LIST that still have no findings JSON local m=0 s h while IFS= read -r s; do @@ -2050,8 +2056,8 @@ l1_missing_count() { # count sessions in $SESSIONS_LIST that still have no findi # An unvalidated hash here counts the session missing forever and the retry # loop re-dispatches it every round. h=$(session_hash "$s") || { m=$((m + 1)); continue; } - # Same check the dispatcher applies on the way out; see the worker for why `arrays`. - jq -e ".findings | arrays" "$FINDINGS_DIR/$h.json" >/dev/null 2>&1 || m=$((m + 1)) + # Same check the dispatcher applies on the way out (L1_FINDINGS_OK). + jq -s -e "$L1_FINDINGS_OK" "$FINDINGS_DIR/$h.json" >/dev/null 2>&1 || m=$((m + 1)) done < "$SESSIONS_LIST" printf '%s' "$m" } @@ -2168,13 +2174,12 @@ reconcile_findings_with_worklist() { # With AUTODREAM_L1_REQUIRE_SERVED=1, a findings JSON or chunk answer a model wrote is held to the # verdict the runner stored beside it (.served, written in dispatch_l1): one with no # verdict, or any verdict but match, came from a run that did not require it, or from before the -# verdict existed, and is dropped so this run triages it again. A record carrying error or skipped -# with an empty findings array is kept: the runner writes those itself with no model behind it (a -# noise-gate skip, an unreadable session, a final-round stub), and the post-L1 passes add fields to -# them, so their shape cannot be told from a worker's. Neither can their keys, since a worker can -# write any of them, so the empty array is what is trusted: every record the runner writes has -# findings: [], and one with findings in it is model output whatever else it carries. That holds -# for the no-engine marker as well, so a no_engine record with findings goes through the verdict. +# verdict existed, and is dropped so this run triages it again. A record the runner wrote itself, +# with no model behind it (a noise-gate skip, an unreadable session, a normalize failure, a +# no-engine record, a final-round stub), is kept when its .served says runner and its findings are +# empty. Provenance comes from that marker, which the runner writes, never from the record's keys +# or shape: a worker can write error, skipped or no_engine as easily as the runner can (#167). A +# runner record from before the marker existed is triaged once more and comes back marked. # # Runs once, after reconcile_findings_with_worklist and before the pre-L1 cache snapshot, so the # dispatcher, l1_missing_count and L1_PRECACHED all see the same set. Counted in run-stats as @@ -2188,12 +2193,14 @@ drop_unacceptable_l1_cache() { [ -n "$_h" ] || continue _f="$FINDINGS_DIR/$_h.json" [ -f "$_f" ] || continue + # Dropping a no-engine record only costs a triage, so its shape is enough to do that. if l1_record_has_no_engine "$_f"; then _src=$(printf '%s\n' "$AUTODREAM_SOURCE_MAP" | awk -F'\t' -v h="$_h" '$1 == h { print $2; exit }') - l1_source_has_engine "$_src" || continue - rm -f "$_f" "$_f.served" - L1_CACHE_DROPPED=$((L1_CACHE_DROPPED + 1)); _unrouted="${_unrouted:+$_unrouted }$_h" - continue + if l1_source_has_engine "$_src"; then + rm -f "$_f" "$_f.served" + L1_CACHE_DROPPED=$((L1_CACHE_DROPPED + 1)); _unrouted="${_unrouted:+$_unrouted }$_h" + continue + fi fi [ "$_served" = 1 ] || continue l1_record_is_runner_only "$_f" && continue @@ -2213,10 +2220,10 @@ drop_unacceptable_l1_cache() { [ -z "$_unverified" ] && [ "$_chunks" -eq 0 ] \ || log "AUTODREAM_L1_REQUIRE_SERVED=1: dropped cached L1 result(s) no served verdict vouches for, to triage again (sessions: ${_unverified:-none}; chunk answers: $_chunks)" } -# 0 for a no-engine record, by its marker or, for one written before the marker, by its error. -# Findings in it make it model output (see l1_record_is_runner_only). +# 0 for a no-engine record, by its no_engine key or, for one written before the key, by its error. +# Only ever used to drop a record, so a worker that forges one buys itself a retriage, nothing more. l1_record_has_no_engine() { # $1=findings JSON - jq -e '(.no_engine == true or ((.error // "") | tostring | startswith("no L1 engine for this session"))) and .findings == []' "$1" >/dev/null 2>&1 + jq -s -e 'length == 1 and (.[0] | (.no_engine == true or ((.error // "") | tostring | startswith("no L1 engine for this session"))) and .findings == [])' "$1" >/dev/null 2>&1 } # 0 when this run resolved an engine and a model for the source, as dispatch_l1 will read them. l1_source_has_engine() { # $1=source adapter @@ -2227,10 +2234,12 @@ l1_source_has_engine() { # $1=source adapter case "$_e" in ""|*[!abcdefghijklmnopqrstuvwxyz0123456789_-]*) return 1 ;; esac [ -n "$_m" ] && [ -x "$ADAPTERS_DIR/$_e/adapter.sh" ] } -# 0 for a record that says the session was not triaged (error) or was not sent to a model (skipped) -# and has no findings. A worker can write either key, so a record with findings is model output. +# 0 for a record the runner wrote with no model behind it: its .served says runner (the dispatcher +# writes that marker beside every such record) and it holds one value with no findings. The empty +# findings array is a second guard, for a worker that rewrites another session's record in place. l1_record_is_runner_only() { # $1=findings JSON - jq -e '(has("error") or has("skipped")) and .findings == []' "$1" >/dev/null 2>&1 + [ "$(served_verdict "$1")" = runner ] || return 1 + jq -s -e 'length == 1 and .[0].findings == []' "$1" >/dev/null 2>&1 } served_verdict() { cut -s -f4 "$1.served" 2>/dev/null | head -1; } # $1=answer -> its stored verdict scan_worklist_leftovers() { @@ -2456,11 +2465,10 @@ dispatch_l1() { # one parallel pass; idempotent worker → only the still-missin # garbage JSON). Treat it as missing so this pass re-dispatches it, rather # than letting it count as done and feed broken records to L2. # `jq -e .findings` is truthy for a STRING or an OBJECT, so {"findings":"oops"} counted as a - # finished session and reached L2 as a result. `arrays` emits nothing for a non-array, so jq - # -e exits non-zero. Written this way rather than as `(.findings | type) == "array"`: this - # body is a single-quoted bash -c where a quote silently breaks the quoting, and the same - # check in l1_missing_count must read as the same check. - jq -e ".findings | arrays" "$output" >/dev/null 2>&1 && exit 0 + # finished session and reached L2 as a result. The filter comes from the environment + # (L1_FINDINGS_OK, defined once in the parent) because this body is a single-quoted bash -c + # where a quote silently breaks the quoting, and l1_missing_count must run the same check. + jq -s -e "$L1_FINDINGS_OK" "$output" >/dev/null 2>&1 && exit 0 # Validate the session is readable BEFORE spawning a worker. A path that find # enumerated but that is gone/unreadable by dispatch time otherwise sends the @@ -2469,6 +2477,7 @@ dispatch_l1() { # one parallel pass; idempotent worker → only the still-missin # is correct — retrying would not help. if [ ! -r "$session" ]; then jq -cn --arg p "$session" "{session_path: \$p, error: \"session file not readable at dispatch\", findings: []}" > "$output" + printf "runner\t-\t-\trunner\n" > "$output.served" rm -f "$errlog" echo "skip (unreadable): $session ($hash)" >&2 exit 0 @@ -2488,6 +2497,7 @@ dispatch_l1() { # one parallel pass; idempotent worker → only the still-missin gate=$(jq -r --argjson min_turns "${AUTODREAM_MIN_USER_TURNS:-2}" --argjson min_minutes "${AUTODREAM_MIN_MINUTES:-1}" "if (.isSidechain == true) or ((.tool_call_count // 0) >= 5) then 0 elif (.user_message_count // 0) < \$min_turns then 1 elif ((.duration_minutes // 0) > 0) and ((.duration_minutes // 0) < \$min_minutes) then 1 else 0 end" "$statsfile" 2>/dev/null) if [ "$gate" = "1" ]; then jq -cn --arg p "$session" "{session_path: \$p, skipped: \"below_noise_gate\", findings: []}" > "$output" + printf "runner\t-\t-\trunner\n" > "$output.served" rm -f "$errlog" echo "gated (below noise threshold): $session ($hash)" >&2 exit 0 @@ -2517,6 +2527,7 @@ dispatch_l1() { # one parallel pass; idempotent worker → only the still-missin # record, so a refusal can be told from an environment fault by reading it. jq -cn --arg p "$session" --arg a "$wsrc" --arg why "$normerr" \ "{session_path: \$p, error: (\"session could not be normalized by the \" + \$a + \" adapter: \" + \$why), findings: []}" > "$output" + printf "runner\t-\t-\trunner\n" > "$output.served" rm -f "$errlog" echo "skip (not normalizable): $session ($hash)" >&2 exit 0 @@ -2586,6 +2597,7 @@ dispatch_l1() { # one parallel pass; idempotent worker → only the still-missin # (drop_unacceptable_l1_cache in the parent). rm -f "$slimfile" "$normfile" "$dayfile" jq -cn --arg p "$session" --arg s "$src" --arg m "$model" "{session_path: \$p, no_engine: true, error: (\"no L1 engine for this session (source [\" + \$s + \"], model [\" + \$m + \"])\"), findings: []}" > "$output" + printf "runner\t-\t-\trunner\n" > "$output.served" rm -f "$errlog" echo "skip (no engine): $session ($hash)" >&2 exit 0 @@ -2765,7 +2777,7 @@ dispatch_l1() { # one parallel pass; idempotent worker → only the still-missin if [ -n "${5:-}" ]; then l1_ok() { bash "$MERGER" --check "$1"; } else - l1_ok() { jq -e ".findings | arrays" "$1" >/dev/null 2>&1; } + l1_ok() { jq -s -e "$L1_FINDINGS_OK" "$1" >/dev/null 2>&1; } fi if [ -s "$2" ] && ! l1_ok "$2"; then printf "worker wrote output with no usable .findings key; treating as a failure\n" >> "$3" @@ -2903,7 +2915,7 @@ dispatch_l1() { # one parallel pass; idempotent worker → only the still-missin # itself. A merge that fails anyway drops every cached answer so the next round cannot # loop on them. if bash "$MERGER" --session "$session" --elided "$elided" "${parts[@]}" > "$output.merge" 2>> "$errlog" \ - && [ -s "$output.merge" ] && jq -e ".findings | arrays" "$output.merge" >/dev/null 2>&1; then + && [ -s "$output.merge" ] && jq -s -e "$L1_FINDINGS_OK" "$output.merge" >/dev/null 2>&1; then mv -f "$output.merge" "$output" # The merged answer is vouched for only as far as its chunks were: match when every chunk # answer says match, mismatch when any says mismatch, unmeasured otherwise (a chunk @@ -2972,6 +2984,7 @@ dispatch_l1() { # one parallel pass; idempotent worker → only the still-missin [ "$chunked" = 1 ] && cmeta="{\"chunks\":$nchunks,\"chunks_elided\":$elided}" jq -cn --arg p "$session" --arg r "${AUTODREAM_L1_ROUNDS:-5}" --argjson b "${sz:-0}" --argjson l "${lines:-0}" --argjson sl "$([ -n "$slimfile" ] && echo true || echo false)" --argjson cm "$cmeta" \ "{session_path: \$p, error: (\"worker exited without findings JSON after \" + \$r + \" rounds\"), meta: ({bytes: \$b, lines: \$l, slimmed: \$sl} + \$cm), findings: []}" > "$output" + printf "runner\t-\t-\trunner\n" > "$output.served" echo "FAIL (metadata stub written): $session ($hash) [$(($(date +%s) - t0))s] — see $errlog" >&2 else echo "FAIL: $session ($hash) [$(($(date +%s) - t0))s] — see $errlog" >&2 @@ -3943,7 +3956,7 @@ PY for fjson in "$FINDINGS_DIR"/*.json; do case "$fjson" in *.stats.json) continue ;; esac [ -s "$fjson" ] || continue - jq -e ".findings | arrays" "$fjson" >/dev/null 2>&1 || continue + jq -s -e "$L1_FINDINGS_OK" "$fjson" >/dev/null 2>&1 || continue sidecar="${fjson%.json}.stats.json" skilltmp="$fjson.skills.tmp" if ! jq -e 'type == "object" and (["skills_invoked", "skills_invoked_count", "skills_invoked_counts", "skills_authored"] - keys | length == 0)' "$sidecar" >/dev/null 2>&1; then diff --git a/tests/run-all.sh b/tests/run-all.sh index 07d3980..2358053 100755 --- a/tests/run-all.sh +++ b/tests/run-all.sh @@ -4389,6 +4389,57 @@ test_require_served_holds_cached_findings_to_their_verdict(){ assert_grep "$fd/run-stats.txt" '^l1_cache_dropped: 0$' "and dropped nothing" rm -rf "$root" } +test_require_served_trusts_only_the_runners_own_marker(){ + echo "# a model answer shaped like a runner record, findings empty, is held to its verdict (#167)" + local root fd h body + for body in '{"error":"unreadable","findings":[]}' '{"skipped":"below_noise_gate","findings":[]}'; do + root=$(setup_env); mk_session "$root" sess1; h=$(hash_of "$root/projects/proj-a/sess1.jsonl") + MOCK_SERVED_MODEL=claude-other-model run_dream "$root"; fd=$(fdir "$root") + printf '%s\n' "$body" > "$fd/$h.json" + AUTODREAM_FORCE=1 AUTODREAM_L1_REQUIRE_SERVED=1 MOCK_CALL_LOG="$root/calls2.log" run_dream "$root" + assert_eq "$(grep -c . "$root/calls2.log" 2>/dev/null)" "1" "$body: run 2 triaged the session again" + assert_grep "$fd/run-stats.txt" '^l1_cache_dropped: 1$' "$body: run-stats counts the drop" + assert_grep "$fd/$h.json" 'always run tests after edits' "$body: real findings replace it" + rm -rf "$root" + done + echo "# so is a no_engine record the runner did not write, while its source has no engine" + root=$(setup_env); mk_session "$root" sess1; h=$(hash_of "$root/projects/proj-a/sess1.jsonl") + MOCK_SERVED_MODEL=claude-other-model run_dream "$root"; fd=$(fdir "$root") + printf '%s\n' '{"no_engine":true,"error":"forged","findings":[]}' > "$fd/$h.json" + AUTODREAM_FORCE=1 AUTODREAM_L1_REQUIRE_SERVED=1 AUTODREAM_L1_ENGINE_CLAUDE=nonesuch run_dream "$root" + assert_grep "$fd/run-stats.txt" '^l1_cache_dropped: 1$' "the forged record is dropped" + assert_grep "$fd/$h.json" 'no L1 engine for this session' "and the runner writes its own in its place" + assert_grep "$fd/$h.json.served" ' runner$' "marked as the runner's" + rm -rf "$root" + echo "# a runner record without the marker (from an older runner) is triaged again, once" + root=$(setup_env); mk_session "$root" sess1; h=$(hash_of "$root/projects/proj-a/sess1.jsonl") + MOCK_MODE=l1_incomplete AUTODREAM_L1_ROUNDS=1 run_dream "$root"; fd=$(fdir "$root") + assert_grep "$fd/$h.json.served" ' runner$' "run 1 marked its final-round stub" + rm -f "$fd/$h.json.served" + AUTODREAM_FORCE=1 AUTODREAM_L1_REQUIRE_SERVED=1 MOCK_CALL_LOG="$root/calls2.log" run_dream "$root" + assert_eq "$(grep -c . "$root/calls2.log" 2>/dev/null)" "1" "run 2 triaged it again" + rm -rf "$root" +} +test_a_findings_json_with_two_values_is_not_a_result(){ + echo "# a findings JSON holding two values is not a finished triage: jq -e judged only the last (#167)" + local root fd h two + two='{"findings":[{"what":"forged by the substituted model"}]} +{"skipped":true,"findings":[]}' + root=$(setup_env); mk_session "$root" sess1; h=$(hash_of "$root/projects/proj-a/sess1.jsonl") + run_dream "$root"; fd=$(fdir "$root") + printf '%s\n' "$two" > "$fd/$h.json" + AUTODREAM_FORCE=1 MOCK_CALL_LOG="$root/calls2.log" run_dream "$root" + assert_eq "$(grep -c . "$root/calls2.log" 2>/dev/null)" "1" "without the gate, the session is triaged again" + assert_nogrep "$fd/$h.json" 'forged by the substituted model' "and the first value never reaches L2" + rm -rf "$root" + root=$(setup_env); mk_session "$root" sess1; h=$(hash_of "$root/projects/proj-a/sess1.jsonl") + MOCK_SERVED_MODEL=claude-other-model run_dream "$root"; fd=$(fdir "$root") + printf '%s\n' "$two" > "$fd/$h.json" + AUTODREAM_FORCE=1 AUTODREAM_L1_REQUIRE_SERVED=1 MOCK_CALL_LOG="$root/calls2.log" run_dream "$root" + assert_eq "$(grep -c . "$root/calls2.log" 2>/dev/null)" "1" "with the gate, the same" + assert_nogrep "$fd/$h.json" 'forged by the substituted model' "and the forged finding is gone" + rm -rf "$root" +} test_a_no_engine_record_is_retried_once_an_engine_resolves(){ echo "# a no-engine record left by a bad engine choice is triaged by a later run whose config resolves one" local root o h fd; root=$(setup_env); o=$(mk_omp_session "$root" xxxx); h=$(hash_of "$o") @@ -5733,6 +5784,8 @@ _run test_l1_provenance_and_served_models_are_recorded _run test_an_engine_failing_its_login_requirement_is_handed_nothing _run test_a_call_answered_by_another_model_loses_its_findings _run test_require_served_holds_cached_findings_to_their_verdict +_run test_require_served_trusts_only_the_runners_own_marker +_run test_a_findings_json_with_two_values_is_not_a_result _run test_omp_nested_child_is_a_sidechain_of_its_bucket _run test_omp_session_that_cannot_be_linearized_is_an_error_record _run test_omp_stats_describe_the_live_branch_only From 13463ca990812df3f77ede762eb2003fae6faea4 Mon Sep 17 00:00:00 2001 From: Samuel Reed Date: Thu, 29 Oct 2026 14:30:51 -0400 Subject: [PATCH 2/2] fix: move a rejected findings JSON aside so it cannot read as this round's result Root cause (Codex round 1 on PR 168): the worker rejected a cached file that failed L1_FINDINGS_OK but left it in place, and every later step treats a non-empty $output as a result. A chunk that kept failing then skipped the final-round stub and the rejected file reached L2. It now moves to .json.rejected, which no *.json glob reaches, before anything else runs. Also documents the limit the served markers share with the match verdict: a worker that rewrites another session's record and .served together is not caught by checks on files in the findings dir. --- bin/run.sh | 14 ++++++++++++++ tests/run-all.sh | 14 ++++++++++++++ 2 files changed, 28 insertions(+) diff --git a/bin/run.sh b/bin/run.sh index ab2cb0a..b1d9d44 100755 --- a/bin/run.sh +++ b/bin/run.sh @@ -2181,6 +2181,12 @@ reconcile_findings_with_worklist() { # or shape: a worker can write error, skipped or no_engine as easily as the runner can (#167). A # runner record from before the marker existed is triaged once more and comes back marked. # +# The limit, which this shares with the match verdict: every .served lives in the findings dir, +# where a worker holding the Write tool can reach it, so a worker that rewrites ANOTHER session's +# record and its .served together is not caught here. The dispatcher replaces each worker's own +# .served after it exits, which covers the worker's own session only. Closing the cross-session +# case needs provenance a worker cannot write at all, not a better check on these files. +# # Runs once, after reconcile_findings_with_worklist and before the pre-L1 cache snapshot, so the # dispatcher, l1_missing_count and L1_PRECACHED all see the same set. Counted in run-stats as # l1_cache_dropped and named in the log. @@ -2469,6 +2475,14 @@ dispatch_l1() { # one parallel pass; idempotent worker → only the still-missin # (L1_FINDINGS_OK, defined once in the parent) because this body is a single-quoted bash -c # where a quote silently breaks the quoting, and l1_missing_count must run the same check. jq -s -e "$L1_FINDINGS_OK" "$output" >/dev/null 2>&1 && exit 0 + # A file that fails the check is moved out of the way now, not left in place: every later + # step reads a non-empty $output as this round having produced a result, so a chunk failure + # with the old file still there skipped the final stub and the rejected file reached L2. + # Kept as .json.rejected (no glob for *.json reaches it) as the evidence of what it was. + if [ -e "$output" ]; then + mv -f "$output" "$output.rejected" 2>/dev/null || rm -f "$output" + rm -f "$output.served" + fi # Validate the session is readable BEFORE spawning a worker. A path that find # enumerated but that is gone/unreadable by dispatch time otherwise sends the diff --git a/tests/run-all.sh b/tests/run-all.sh index 2358053..08c1fd3 100755 --- a/tests/run-all.sh +++ b/tests/run-all.sh @@ -6496,6 +6496,19 @@ test_a_failed_chunker_on_a_transcript_that_was_never_slimmed_is_still_bounded(){ rm -rf "$root" } +test_a_rejected_findings_file_still_ends_in_the_final_stub(){ + local two='{"findings":[{"what":"forged by the substituted model"}]} +{"skipped":true,"findings":[]}' + echo "# a rejected file is moved aside, so a chunk that keeps failing still ends in the final stub" + local root fd h f; root=$(setup_env); f=$(mk_chunky "$root" big 60); h=$(hash_of "$f"); fd=$(fdir "$root") + mkdir -p "$fd"; printf '%s\n' "$two" > "$fd/$h.json" + export MOCK_MODE=chunked MOCK_FAIL_CHUNK=2 AUTODREAM_L1_ROUNDS=1 + run_chunked "$root"; unset MOCK_MODE MOCK_FAIL_CHUNK AUTODREAM_L1_ROUNDS + assert_grep "$fd/$h.json" 'worker exited without findings JSON' "the session gets the final-round stub" + assert_nogrep "$fd/$h.json" 'forged by the substituted model' "and the rejected file does not reach L2" + assert_grep "$fd/$h.json.rejected" 'forged by the substituted model' "it is kept as evidence beside it" + rm -rf "$root" +} test_a_chunk_that_fails_is_retried_alone_and_finished_chunks_are_reused(){ echo "# chunking: a chunk failure blocks the merge, only that chunk is redone, and finished ones are reused" local root; root=$(setup_env) @@ -6777,6 +6790,7 @@ _run test_a_chunk_that_fails_is_retried_alone_and_finished_chunks_are_reused _run test_a_cached_chunk_answer_is_not_reused_after_the_instructions_or_the_session_change _run test_a_cached_chunk_answer_is_not_reused_on_another_engine _run test_require_served_holds_cached_chunk_answers_to_their_verdict +_run test_a_rejected_findings_file_still_ends_in_the_final_stub _run test_a_bad_chunk_answer_is_never_merged_around _run test_a_chunk_failure_is_classified_like_any_worker_failure _run test_a_provider_refusal_on_a_chunk_defers_the_date_and_stops_the_other_chunks