fix(noema): surface real sidecar fail-closed diagnostics past redaction

docs/product-technical-gap-baseline.md

@@ -465,6 +465,106 @@ recurrence" section below out of the file entirely; both are restored here.)
   way this pass found #1417's, and consider merging `main` into #1394's head
   to get it off its stale base.
 
+## 2026-08-30 orchestrator/free pool exhausted by upstream ZDR hardening
+
+- **Root cause (verified by live, end-to-end local reproduction, not log
+  inference).** After #1422 bumped `ORCHESTRATOR_PIN_SHA` to
+  `5f2753ace756ddd81049a5221d55e8977572a416`, the first hosted `noema-review`
+  run on the new pin (`.github` PR #1423, head
+  `954d57b46fd8896ba0fb572a4fc662aa6a684c0a`) failed with `sidecar exited
+  before healthz (status 1); stderr: omitted_unstructured_lines=1` — a new
+  failure signature, distinct from the stale-pin HTTP 502/413 class the
+  2026-08-30 entry above describes. Between the old pin
+  (`b21645116b352967e50fc497b87eb745b9cc8c61`) and the new one, upstream
+  `contextual-orchestrator` commit `952996ec` ("fix(discovery): keep
+  OpenRouter catalog evidence-only") deliberately set
+  `ProviderModelSource(provider_name="openrouter", ...).evidence_only=True`
+  (previously `False`) — an intentional, ZDR-privacy-motivated hardening
+  (OpenRouter routes to many third-party backends with varying retention
+  policies, so it may no longer be used as a *serving* agent, only as a
+  source of per-model ZDR evidence for other providers' matching canonical
+  ids). This is a correct fix on the orchestrator side and must not be
+  reverted or weakened.
+- The org's sidecar (`scripts/ci/contextual_orchestrator_review_launcher.py`)
+  builds the `orchestrator/free` pool only from `is_free=True` routes among
+  the five credentialed providers (`BYTEZ_API_KEY`, `NVIDIA_NIM_API_KEY`,
+  `NVIDIA_NIM_API_KEY_SUB`, `OPENROUTER_API_KEY`, `OPENAI_API_KEY`).
+  `openrouter` was, and had always been, the *only* one of those five whose
+  discovery response carries genuine per-model pricing (`contextual_orchestrator/model_discovery.py`'s `_parse_openai_compatible` reads `row["pricing"]`, present only in OpenRouter's `/v1/models`
+  response shape). NVIDIA NIM, OpenAI, and Bytez publish no pricing via their
+  list-models endpoints at all — confirmed by an unauthenticated live probe
+  of `https://integrate.api.nvidia.com/v1/models` in this session, which
+  returns only `{id, object, created, owned_by}` per model, and by
+  `contextual_orchestrator`'s own `_parse_bytez` docstring ("Bytez prices by
+  GPU-second ... leaving per-1k pricing unset is more honest than a
+  misleading estimate"). `.github`'s own
+  `tests/test_contextual_orchestrator_review_live_discovery_contract.py`
+  already encoded this as `cost_evidence == "unknown"` for openai/nvidia_nim/
+  nvidia_nim_sub/bytez in its live-shape fixture — this was a known,
+  pre-existing structural dependency on OpenRouter for the free pool, not a
+  new assumption. With `openrouter` now `evidence_only`, the launcher's
+  `_routable_discovered_models()` filter drops all 540 OpenRouter rows before
+  the free-pool selection ever runs, so `selected_models` is empty and
+  `main()` raises `SystemExit("review sidecar discovered no eligible models;
+  orchestrator/free would fail closed")` — exit 1, before `serve()`, hence
+  before `/healthz`.
+- **Live reproduction** (this session, real network calls, fake-but-present
+  values for the five secrets, pinned commit `5f2753ac…` installed from its
+  own `requirements.lock`): `discover_all_models()` returned 682 models —
+  `openrouter`: 540 total, 60 genuinely free, but 540/540 `evidence_only`;
+  `nvidia_nim` and `nvidia_nim_sub`: 71 each, 0 free; `openai`/`bytez`:
+  `http_status_401` (fake key, but note neither provider's list endpoint
+  carries pricing regardless of auth outcome). Routable (non-evidence-only)
+  free models: **0**. Running
+  `scripts/ci/contextual_orchestrator_review_launcher.py` directly end-to-end
+  reproduced the exact hosted signature: raw stderr
+  `review sidecar discovered no eligible models; orchestrator/free would
+  fail closed`, exit 1. This is deterministic and structural, not a
+  transient provider/network fluke — every future `noema-review` run with
+  this exact five-secret credential set will fail identically until the free
+  pool gets a real, non-OpenRouter zero-cost source, so this blocks PR review
+  org-wide, not just PR #1423.
+- **Independent bug found and fixed in this pass (safe, no policy
+  tradeoff):** `scripts/ci/sanitize_contextual_orchestrator_sidecar_stream.py`'s
+  `_PREFIX_SUMMARIES` allowlist still matched the launcher's *old* wording
+  ("no zero-cost models"), not the current "no eligible models" text, and had
+  no entry at all for the launcher's missing-auth-token or
+  missing-provider-credential `SystemExit` messages. All three fell through
+  to `omitted_unstructured_lines=N`, which is exactly why PR #1423's hosted
+  log showed only `omitted_unstructured_lines=1` instead of the actionable
+  cause above — the redaction was hiding a real, non-secret diagnostic, not
+  protecting a secret. Fixed the three prefixes/summaries and the matching
+  pinned assertions in
+  `tests/test_contextual_orchestrator_review_runtime_preflight.py`; full
+  `.github` suite (1875 passed, 1 skipped, 25 subtests), `coverage report`
+  (the changed file itself is 100%; the pre-existing repo-wide 99% is the
+  already-tracked `scripts/ci/pingora_edge_policy.py:274` gap owned by
+  #1398, not introduced here), and `interrogate` (100.0%) all pass on this
+  change alone.
+- **What is intentionally NOT fixed by this pass, and needs a product/human
+  decision, not a unilateral code change:** restoring a non-empty
+  `orchestrator/free` pool. Two candidate paths, neither exercised or
+  authorized here: (a) accept real provider spend by pointing
+  `CONTEXTUAL_ORCHESTRATOR_POOL` at `auto` (already fully implemented in the
+  launcher as a priced fallback) — this trades away the "fail-closed
+  zero-cost" guarantee `docs/CWL-MASTER-CONTEXT.md`/`CLAUDE.md` describe for
+  every PR review org-wide, a budget-owner call; or (b) wire in a genuine
+  zero-cost provider — `contextual_orchestrator`'s `opencode_zen` source
+  already cross-references real Models.dev pricing (not a self-reported
+  flag) to compute `is_free` honestly, and its credential
+  (`OPENCODE_ZEN_API_KEY`) already exists as an org secret (used today only
+  by `opencode-review.yml`'s separate OpenCode Zen GitHub Models config, not
+  passed to this sidecar) — but wiring it in also needs a new
+  `scripts/ci/zdr_policy.py` `PROVIDER_ZDR_SCOPE["opencode_zen"]` attestation
+  entry (that table currently `KeyError`s on an unknown provider name by
+  design, so skipping this would crash every ZDR-required — i.e.
+  private/internal-repo — review instead of just noema-review's current
+  public-repo failure) and live verification, with a real key, that
+  opencode.ai/zen's discovered free models are actually
+  general-chat/tool-call-capable and pass the sidecar's runtime preflight —
+  none of which this pass could validate without provisioning real
+  credentials. Neither option is a small, obviously-safe patch, so it is
+  left open here rather than forced.
 ## 2026-08-30 sidecar pin staleness recurrence
 
 - Same class of defect as the 2026-08-29 entry above recurred within one day:

scripts/ci/contextual_orchestrator_review_sidecar.sh

@@ -271,6 +271,22 @@ log "starting review sidecar on ${ORCHESTRATOR_HOST}:${ORCHESTRATOR_PORT}"
 cp "$ORCHESTRATOR_LAUNCHER" "$ORCHESTRATOR_WORK/launch_sidecar.py"
 export ORCHESTRATOR_CATALOG_LIMIT="$CATALOG_LIMIT"
 export ORCHESTRATOR_CATALOG_FAMILY_CAP="$CATALOG_FAMILY_CAP"
+# Stream stdout/stderr through the redacting sanitizer as two named, awaitable
+# processes (not bare `> >(...)` substitutions, whose PIDs bash never exposes)
+# so a failure handler can wait for the sanitizer to finish flushing before it
+# reads the sanitized file — otherwise the read can race the still-draining
+# pipe and silently show an empty/truncated diagnostic (the exact class of bug
+# this sanitizer exists to avoid: see the 2026-08-30 sidecar-diagnostics gap
+# baseline entry).
+exec {orchestrator_stdout_fd}> >("$sidecar_python" -u "$SIDECAR_LOG_SANITIZER" > "$sidecar_stdout")
+stdout_sanitizer_pid=$!
+exec {orchestrator_stderr_fd}> >("$sidecar_python" -u "$SIDECAR_LOG_SANITIZER" > "$sidecar_stderr")
+stderr_sanitizer_pid=$!
+wait_for_sidecar_sanitizers() {
+  wait "$stdout_sanitizer_pid" 2>/dev/null || true
+  wait "$stderr_sanitizer_pid" 2>/dev/null || true
+}
+
 PYTHONPATH="$ORCHESTRATOR_SOURCE:$ORG_REPO_ROOT" \
   CONTEXTUAL_ORCHESTRATOR_TOKEN="$ORCHESTRATOR_TOKEN" \
   "$sidecar_python" "$ORCHESTRATOR_WORK/launch_sidecar.py" \
@@ -283,16 +299,22 @@ PYTHONPATH="$ORCHESTRATOR_SOURCE:$ORG_REPO_ROOT" \
     "${zdr_args[@]}" \
     "${privacy_args[@]}" \
     "${pool_args[@]}" \
-> >("$sidecar_python" -u "$SIDECAR_LOG_SANITIZER" > "$sidecar_stdout") \
-2> >("$sidecar_python" -u "$SIDECAR_LOG_SANITIZER" > "$sidecar_stderr") &
+  >&"$orchestrator_stdout_fd" 2>&"$orchestrator_stderr_fd" &
 sidecar_pid=$!
+# Close our own copies of the write ends now that the sidecar process holds
+# its own duplicated fds. If these stayed open in this shell, the sanitizer
+# process substitutions would never see EOF (and never exit) once the sidecar
+# itself closes its fds, since a process substitution's reader only finishes
+# after every writer has closed.
+exec {orchestrator_stdout_fd}>&- {orchestrator_stderr_fd}>&-
 cleanup_sidecar_on_error() {
   status=$?
   if [ "$status" -ne 0 ]; then
     publish_sidecar_evidence || true
     log "stopping failed sidecar (pid $sidecar_pid)"
     kill "$sidecar_pid" 2>/dev/null || true
     wait "$sidecar_pid" 2>/dev/null || true
+    wait_for_sidecar_sanitizers
   fi
 }
 trap cleanup_sidecar_on_error EXIT
@@ -302,6 +324,11 @@ until curl -fsSL --max-time 2 "http://${ORCHESTRATOR_HOST}:${ORCHESTRATOR_PORT}/
   if ! kill -0 "$sidecar_pid" 2>/dev/null; then
     sidecar_status=0
     wait "$sidecar_pid" || sidecar_status=$?
+    # The sidecar has fully exited (confirmed above), so its stderr pipe has
+    # already sent EOF; draining the sanitizer here cannot hang, and it
+    # guarantees $sidecar_stderr holds everything the sidecar wrote before we
+    # read it for the failure message below.
+    wait_for_sidecar_sanitizers
     fail "sidecar exited before healthz (status ${sidecar_status}); stderr: $(sed -n '1,20p' "$sidecar_stderr")"
   fi
   i=$((i + 1))

scripts/ci/sanitize_contextual_orchestrator_sidecar_stream.py

@@ -15,8 +15,21 @@
     ("review sidecar preflight failed:", "review sidecar preflight failed"),
     ("review sidecar discovery failed:", "review sidecar discovery failed"),
     (
-        "review sidecar discovered no zero-cost models;",
-        "review sidecar discovered no zero-cost models",
+        # Matches contextual_orchestrator_review_launcher.py's actual
+        # SystemExit text ("no eligible models", not "no zero-cost models" --
+        # that stale prefix never matched the launcher's real message, so
+        # this fail-closed diagnostic was silently dropped to
+        # omitted_unstructured_lines instead of reaching CI operators).
+        "review sidecar discovered no eligible models;",
+        "review sidecar discovered no eligible models",
+    ),
+    (
+        "review sidecar requires an explicit --auth-token or the KV credential",
+        "review sidecar auth token unavailable",
+    ),
+    (
+        "review sidecar requires at least one provider credential in the KV",
+        "review sidecar requires at least one provider credential in the KV",
     ),
 )
 

tests/test_contextual_orchestrator_review_runtime_preflight.py

@@ -326,8 +326,15 @@ def test_sidecar_stream_sanitizer_allowlists_only_bounded_diagnostics() -> None:
         "review sidecar discovery failed: https://provider.invalid/?key=sk-secret"
     ) == "review sidecar discovery failed"
     assert sanitize_line(
-        "review sidecar discovered no zero-cost models; orchestrator/free would fail closed"
-    ) == "review sidecar discovered no zero-cost models"
+        "review sidecar discovered no eligible models; orchestrator/free would fail closed"
+    ) == "review sidecar discovered no eligible models"
+    assert sanitize_line(
+        "review sidecar requires an explicit --auth-token or the KV credential "
+        "'CONTEXTUAL_ORCHESTRATOR_TOKEN'"
+    ) == "review sidecar auth token unavailable"
+    assert sanitize_line(
+        "review sidecar requires at least one provider credential in the KV"
+    ) == "review sidecar requires at least one provider credential in the KV"
     assert sanitize_line("provider response sk-secret") is None
 
 

tests/test_contextual_orchestrator_review_sidecar_contract.py

@@ -401,6 +401,26 @@ def test_sidecar_trap_keeps_the_gateway_alive_after_provisioning() -> None:
     assert 'trap \'log "stopping sidecar (pid $sidecar_pid)"; kill "$sidecar_pid"' not in text
 
 
+def test_sidecar_waits_for_sanitizer_drain_before_reading_failure_diagnostics() -> None:
+    """A bare `2> >(sanitizer)` races the failure-path read and can hide the diagnostic; the drain must close that race."""
+    text = _read(SIDECAR)
+    assert "exec {orchestrator_stdout_fd}> >(" in text
+    assert "stdout_sanitizer_pid=$!" in text
+    assert "exec {orchestrator_stderr_fd}> >(" in text
+    assert "stderr_sanitizer_pid=$!" in text
+    assert "exec {orchestrator_stdout_fd}>&- {orchestrator_stderr_fd}>&-" in text
+    assert "wait_for_sidecar_sanitizers" in text
+    # The old bare, unwaited process-substitution redirection must be gone.
+    assert '> >("$sidecar_python" -u "$SIDECAR_LOG_SANITIZER" > "$sidecar_stdout") \\' not in text
+    assert '2> >("$sidecar_python" -u "$SIDECAR_LOG_SANITIZER" > "$sidecar_stderr") &' not in text
+    # The drain must happen strictly before the failure-path read, only in the
+    # branch where the sidecar has already exited (not the healthz-timeout
+    # branch, where it may still be running and draining would hang).
+    exited_branch = text.index("sidecar exited before healthz")
+    drain_call = text.rindex("wait_for_sidecar_sanitizers", 0, exited_branch)
+    assert drain_call < exited_branch
+
+
 def test_noema_review_workflow_provisions_sidecar_with_all_five_secrets() -> None:
     """Required Noema review uses the gateway; the public NIM hardcode is gone."""
     workflow = _read(NOEMA_WORKFLOW)