diff --git a/CHANGELOG.md b/CHANGELOG.md index 39c61c142b..da913f0870 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,36 @@ this file. The format follows Keep a Changelog, and versioned releases follow Semantic Versioning where the repository publishes a release. ## [Unreleased] +- Confirm on a live hosted run (`ContextualWisdomLab/contextual-orchestrator#921`) + that the family_cap raise and #1436's `max_tokens` fix work end-to-end: + `noema-review` and `strix` both succeeded on a fresh head, with the job + log confirming a real, complete sidecar cycle (not a vacuous pass). This + is not the same as the outage being fully closed: two further, + independent defects were separately found and verified — both since + fixed by later, already-merged PRs (confirmed directly against their + merged diffs during this rebase, not assumed). First, a run on + `.github`'s own `main` (job `99247611184`) failed because Strix's OpenAI + Agents SDK client sent `stream_options.include_usage=true` with + `tools`/`response_format`, rejected by at least one `orchestrator/free` + candidate with `HTTP 400 invalid_stream_options`, exhausting the pool — + fixed at its root cause by `contextual-orchestrator#925` (merge commit + `7944a3c`), which makes the gateway itself stop rejecting the + combination; `.github#1448`'s earlier client-side `LLM_DISABLE_STREAMING` + workaround for the same symptom was since reverted by `.github#1463` now + that it is unnecessary (and `ORCHESTRATOR_PIN_SHA` bumped to `7944a3c` in + the same PR so the vendored sidecar actually carries `#925`'s fix). + Second, a separate run + (`ContextualWisdomLab/.github#1441` job `99249903390`) got past that + stage entirely and instead hung for the full 120s `curl` timeout with + zero bytes back on the sidecar's post-`healthz` gateway smoke request — + fixed by `.github#1452` (commit `1ff82682`, implementing ADR-0005), which + retries that request up to `REVIEW_PREFLIGHT_GATEWAY_MAX_ATTEMPTS` (3) on + a transport failure instead of failing closed after one single-shot + attempt. See the 2026-08-30 gap-baseline entry (and its 2026-09-01 + update) for the full evidence on both gaps and their fixes, the two + permanently-retired `gemma-3` model ids still admitted into the pool, + and the live-catalog-freshness fix that remains the more complete + answer if 8 candidates ever proves insufficient again. - Harden the review sidecar's per-account catalog cap against silent drift: `contextual_orchestrator_review_launcher.py`'s two `build_zdr_prioritized_catalog` call sites now source their diff --git a/docs/product-technical-gap-baseline.md b/docs/product-technical-gap-baseline.md index ea56ced98d..56df635c08 100644 --- a/docs/product-technical-gap-baseline.md +++ b/docs/product-technical-gap-baseline.md @@ -1255,42 +1255,199 @@ direct-NVIDIA-NIM communication is a removal target. (already dead) or removing the one resilience mechanism keeping a required check alive during a live outage. -## 2026-08-30 pingora_edge_policy.py binary-evidence gap: two competing open fixes +## 2026-08-30 pingora_edge_policy.py binary-evidence gap: resolved for PDF by #1435 -A live failure on `ContextualWisdomLab/contextual-orchestrator#906`'s `required-workflow-bootstrap` +**Resolved — updated from an earlier "two competing open fixes" framing +after #1435 merged (squash, `2026-08-30T11:10:11Z`) with a third, +independent implementation, verified directly against its real diff, not +assumed from a title.** A live failure on +`ContextualWisdomLab/contextual-orchestrator#906`'s `required-workflow-bootstrap` job (`GitHub content evidence for docs/papers/helm-holistic-evaluation-2211.09110.pdf -is not a regular base64 file`) traces to `scripts/ci/pingora_edge_policy.py`'s +is not a regular base64 file`) originally traced to `scripts/ci/pingora_edge_policy.py`'s `_load_file_content`: GitHub's Contents API stops returning inline `encoding: "base64"` once a file crosses roughly 1 MB (returning -`encoding: "none"` + a `download_url` instead), and this policy scanner's -`_needs_content_scan` has no exemption for genuinely binary evidence files in -general — any added/modified file without a `patch` (i.e. any binary file, -regardless of size) reaches `_load_file_content`, which always fails once it -tries `raw.decode("utf-8")`. Two **already-open, independent, partially -conflicting** PRs address pieces of this: - -- **#1420** adds real, structural validation (`_is_recognized_documentation_image`: - PNG magic header, chunk order, CRC, zlib-stream, dimension, and scanline - checks) so an image *suffix* alone cannot exempt a file — consistent with - this policy's own stated principle. Covers `.png` only; does not touch - `.pdf`, so it would not by itself fix `ContextualWisdomLab/contextual-orchestrator#906`. -- **#1427** adds a flat `NON_RUNTIME_BINARY_SUFFIXES` allowlist (`.avif`, - `.gif`, `.ico`, `.jpeg`, `.jpg`, `.pdf`, `.png`, `.webp`) that skips - content-scanning by **extension alone**, no byte-level verification. This - does fix `ContextualWisdomLab/contextual-orchestrator#906`, but for every - suffix in that list (not just `.pdf`) it - reintroduces the exact "extension alone is not an exception" gap #1420 - exists to close for PNG — a shell/config file renamed to `evidence.pdf` - (or `.png`, `.jpg`, ...) would now bypass the Nginx-runtime-artifact scan - entirely. -- Left substantive comments on both PRs (this pass) recommending #1420's - structural-validation pattern be extended to `.pdf` (a bounded magic- - header/`%%EOF`-trailer check, short of full parsing) rather than merging - #1427's blanket suffix-trust list, and that the two PRs coordinate so the - org does not land two divergent implementations of the same policy - surface. Not resolved in code this pass — both PRs are themselves - currently blocked by the sidecar-preflight outage above, so neither could - be re-reviewed to a genuine pass yet regardless of which approach wins. +`encoding: "none"` instead), and the policy scanner had no exemption for +genuinely binary evidence files — any binary file without a diff `patch` +reached `_load_file_content`, which always failed on `raw.decode("utf-8")`. + +- **#1435's approach is better than either of the two PRs this entry + previously compared** (#1420's PNG-only structural validation; #1427's + blanket-suffix-trust list this entry recommended against): it + network-verifies the real `%PDF-` magic prefix whenever the file's bytes + can actually be fetched (`_pdf_evidence_confirms_binary`, + `_load_raw_file_bytes`), so a shell/config file renamed to `.pdf` under a + documentation path is still caught and scanned — it does **not** trust + the extension alone. It falls back to the path+suffix convention only for + the one case that genuinely cannot be verified by content at all: a file + whose declared size exceeds the Contents API's fetch ceiling (the exact, + real research-paper-citation case this exemption exists for), gated + through a new `ContentSizeExceededError` distinct from every other + content-evidence failure, which still fails closed exactly as before. New + regression coverage in `tests/test_pingora_edge_policy.py` covers the + oversized-real-PDF case (using GitHub's actual `encoding: "none"` + response shape, not a synthetic one), a disguised textual `.pdf` with a + patch still getting scanned, a genuine small PDF verified by magic bytes, + and a removed PDF not being fetched at all. + `ContextualWisdomLab/contextual-orchestrator#906`'s original failure is + fixed by this. +- **Scope note, not a gap in #1435**: this resolves `.pdf` specifically. + #1420 (still open) separately covers `.png` with equivalent + content-verified rigor (magic header, chunk/CRC/zlib/scanline checks); + #1427 (still open) still proposes the broader, weaker blanket-suffix-trust + list (`.avif`/`.gif`/`.ico`/`.jpeg`/`.jpg`/`.png`/`.webp`, extension alone, + no verification) for the remaining binary formats. The recommendation + from this investigation's earlier pass stands for whichever of those two + the org picks next: extend #1435/#1420's verified-content pattern to the + remaining formats rather than merging #1427's unverified allowlist as-is. + Neither PR is blocking anything today — the specific live failure that + motivated this whole entry is closed. + +## 2026-08-30 sidecar-preflight outage: family_cap/max_tokens fixes confirmed working end to end, but a separate compatibility gap can still fail Strix + +**Softened from an earlier "confirmed resolved" framing in this same entry** +after a second, independently-found, timestamped counter-example — see +below. #1434 merged (squash `e36a1f71`) with all three fixes from the +entries above: the Strix `orchestrator/auto`→`orchestrator/free` switch, +the `ORCHESTRATOR_CATALOG_FAMILY_CAP` 4→8 raise, and (via the independent +#1436 merged into `main` mid-pass) the gateway preflight `max_tokens` fix. + +- **The family_cap/max_tokens fixes are confirmed working end-to-end on at + least one real run.** Checked `ContextualWisdomLab/contextual-orchestrator#921` + (unrelated to this investigation, picked specifically because it does not + itself touch review-pipeline files, so it can't hit the same + `pull_request_target` self-test trust boundary PR #1434/#1441 do), on a + fresh head (`55832c01...`) pushed after `main` had all three fixes: + `noema-review` — **success**; `strix` — **success**. `noema-review`'s job + log confirms a real, complete sidecar cycle, not a vacuous pass: + `CONTEXTUAL_ORCHESTRATOR_BASE_URL: http://127.0.0.1:18080` was exported, + which only happens after the sidecar's own preflight found a viable route + and the server became healthy — the exact stage that failed 100% of the + time (three independent reproductions) before these fixes landed. + `opencode-review` failed on the same head, but for an already-understood, + benign, unrelated timing gap (it ran before the async + `opencode-review-dispatch` flow had posted a verdict for that exact + head). +- **A further, distinct compatibility gap can still cause Strix failures + depending on which provider gets selected — not yet fixed.** A `strix` + run on `.github`'s own `main` (job `99247611184`, completed + `2026-08-30T11:16:17Z`) failed independently, verified directly from its + job log rather than assumed: Strix's OpenAI Agents SDK client sends + `stream_options.include_usage=true` together with `tools`/ + `response_format` in the same request; whichever `orchestrator/free` + candidate this run routed to (log evidence shows an `integrate. + api.nvidia.com` DNS resolution around the same window, consistent with an + `nvidia_nim` route) rejected that exact combination — + `openai.BadRequestError: 400 {'code': 'invalid_stream_options', 'message': + 'stream_options.include_usage=true is not supported with tools or + response_format'}` — on all retries (two separate penetration-test + attempts, `11:12:34Z` and `11:16:12Z`, both hit the identical error), + ending in `STRIX_PROVIDER_UNAVAILABLE: contextual-orchestrator/orchestrator/free + exhausted; the gateway owns provider discovery and failover`. This + predates PR #1444's original "confirmed resolved" claim (`11:16:17Z` vs. + that PR's `11:21:52Z` creation) and is a genuinely separate defect from + both fixes above — a request-shape incompatibility between the OpenAI + Agents SDK's fixed request parameters and at least one upstream + provider's API, not a candidate-selection or token-budget problem. + Root-causing this has been delegated to a separate, already-in-flight + investigation (contextual-orchestrator agent); not duplicated here. +- **A third, distinct failure mode, also verified directly, also not yet + fixed**: `noema-review` on `ContextualWisdomLab/.github#1441` (job + `99249903390`, head `d877886b`) reached `healthz and provider-route + preflight confirmed after 41s` — the launcher's own internal preflight + found a viable route, past the family-cap fix's stage entirely — logged + the same non-fatal `provider_discovery_failed provider=bytez + code=http_status_500` warning seen elsewhere in this investigation, and + then the shell script's own separate post-`healthz` gateway smoke + request (the one #1436 fixed the `max_tokens` on, now `4096`, up from + `16`) got **zero bytes back for the full 120-second `curl` timeout**: + `curl: (28) Operation timed out after 120002 milliseconds with 0 bytes + received` → `gateway preflight request could not reach the local + sidecar`. Distinct from both defects above: not a bad-request rejection + (no error response at all, just silence) and not Strix's own client code + path (this is the sidecar script's own `curl` probe). Plausible causes, + none confirmed: (a) `max_tokens=4096` now lets a route legitimately spend + much longer generating (heavy internal reasoning, no early return) than + the old `16`-token budget ever could, genuinely exceeding a 120s ceiling + that was never re-examined when the token budget grew 256x; (b) a real + hang/deadlock in the orchestrator server for a specific request shape; + (c) unrelated transient network flakiness. Not pursued further this pass + — noted as evidence for whoever picks this up next. If this recurs across + multiple runs, the 120s `curl --max-time` in + `contextual_orchestrator_review_sidecar.sh`'s gateway-preflight step may + need raising alongside `max_tokens` (same reasoning #1436 already + applied: a bigger token budget can legitimately need more wall-clock + time), or the specific route it lands on needs identifying and + deprioritizing. +- **Correction (Devin Review finding on this PR, verified directly before + editing): the conclusion below previously undercounted the open gaps as + "one further known gap" when the same entry already documented two + separate, unfixed failure modes** (the `stream_options` 400 and the + 120s-timeout gap, both documented above). Also checked directly rather than assumed: + `ContextualWisdomLab/contextual-orchestrator#924` (opened after this + entry was first written, proposing to fix the `stream_options` gap by + removing the upfront rejection since `stream_options` was already + stripped before every upstream provider call) is **still open and + `mergeable_state: blocked` as of this correction — not merged** — so + that gap is not closed yet either, only diagnosed with a fix proposed. +- **Accurate combined conclusion**: the family_cap-driven deterministic + admission of dead/slow candidates, and separately the desynchronized + `max_tokens` on the post-`healthz` smoke request, are fixed and confirmed + by a real hosted run — that specific, previously 100%-reproducible + failure mode is closed. It is not true that `orchestrator/free` is now + reliable in general: with `family_cap=8` giving more provider diversity + per run, *which* candidate a given run draws varies, and **two further, + independent, unfixed gaps remain open**: (1) at least one candidate + family has the live `stream_options`/`tools`/`response_format` + request-compatibility rejection above, with a fix proposed but not yet + merged in `ContextualWisdomLab/contextual-orchestrator#924` (though + evidently not every candidate hits it — + `ContextualWisdomLab/contextual-orchestrator#921`'s `strix` run + succeeded); and (2) the separate 120s-timeout-with-zero-bytes gap above, + cause unconfirmed. Two permanently-retired `google/gemma-3-*-it` model + ids are also still admitted into the pool (see the family-cap entry + above) and will still individually fail when the alphabetical sort + reaches them. None of this changes the scope of what was actually fixed + in this pass; it means "the outage is over" would overclaim, while "the + two diagnosed root causes are fixed and verified, two further known + gaps remain open elsewhere" is accurate. +- **Update (2026-09-01, Devin Review finding on `.github#1444`, verified + directly against the merged diffs rather than trusted from the finding + text alone): both of the two "further, independent, unfixed gaps" in the + conclusion above are now closed.** Neither fix existed yet when this + entry (or PR #1444, which restates it) was originally written, so this is + a genuine update, not a contradiction of the historical narrative above. + (1) The `stream_options.include_usage=true` + `tools`/`response_format` + rejection: `.github#1448` (commit `702392a2`) landed first as a + Strix-side `LLM_DISABLE_STREAMING` workaround, but **that workaround was + itself since reverted by `.github#1463`** once the real, upstream root + cause was fixed at the gateway — `contextual-orchestrator#925` (merge + commit `7944a3c`, "accept stream_options.include_usage=true for tools + passthrough") makes the server itself stop rejecting the combination, so + routing around it client-side became unnecessary. `.github#1463` bumped + `ORCHESTRATOR_PIN_SHA` to `7944a3c` itself (not a later tip, kept + minimal) after Devin Review on that revert caught that the *previously* + vendored pin predated `#925`'s merge and would have silently + reintroduced the original failure had the workaround been removed + first — see this file's own 2026-08-31 `ORCHESTRATOR_PIN_SHA bumped to + carry #925's stream_options/tools fix` entry for the full trail, + confirmed directly via `git merge-base --is-ancestor 7944a3c + 8cd99f13...` (true) against the pin now in place. Net effect is the same + conclusion as originally written here — this gap is fixed — via a + cleaner, upstream mechanism instead of a client-side route-around; the + once-proposed `contextual-orchestrator#924` fix (cited above) is + superseded by `#925`, a different, more direct fix for the identical + symptom. (2) The + 120s-timeout-with-zero-bytes gap is fixed by `.github#1452` (commit + `1ff82682`, merged 2026-08-30T14:54:45Z, implementing ADR-0005): the + sidecar's own code comments now cite a live reproduction of this exact + failure class (`ContextualWisdomLab/.github#1449`, job `99253418179`, + `curl` timing out at exactly 120002ms with zero bytes received — the same + symptom as this entry's job `99249903390`), and the post-`healthz` gateway + smoke request is now retried up to `REVIEW_PREFLIGHT_GATEWAY_MAX_ATTEMPTS` + (default 3) on a transport failure or non-2xx status instead of failing + closed after one single-shot 120s attempt. The two permanently-retired + `gemma-3` model ids gap (see the family-cap entry above) is unrelated to + either fix and remains open. ## 2026-08-30 PR #1347 Devin Review 6건 검증: 4건 실재 결함 수정, 2건 확인 후 해소 diff --git a/tests/test_opencode_agent_contract.py b/tests/test_opencode_agent_contract.py index 79fdba39aa..746b750c16 100644 --- a/tests/test_opencode_agent_contract.py +++ b/tests/test_opencode_agent_contract.py @@ -2660,6 +2660,10 @@ def test_opencode_privileged_review_security_boundaries_are_fail_closed(): '^[A-Za-z0-9_.-]+/[A-Za-z0-9_.-]+$ ]]' ) in metadata_step assert '[ "$live_head_repository" != "$TARGET_REPOSITORY" ]' not in metadata_step + # #1533 briefly relaxed this to a warn-and-proceed check, but #1540 + # reverted it back to the original strict fail-closed equality (no + # rationale recorded beyond the revert itself) -- confirmed against + # main's actual current content, not assumed from the PR history. assert '[ "$SUPPLIED_HEAD_SHA" = "$live_head_sha" ]' in metadata_step assert ( 'live_visibility="$(jq -r \'.base.repo.visibility // empty | ascii_downcase\''