diff --git a/docs/product-technical-gap-baseline.md b/docs/product-technical-gap-baseline.md index 758ef2961a..ea56ced98d 100644 --- a/docs/product-technical-gap-baseline.md +++ b/docs/product-technical-gap-baseline.md @@ -1715,6 +1715,36 @@ string, a bare number) confirmed to fail against the pre-fix script (`KeyError: signature as the original round-4 bug) before passing after the fix. 1930 tests pass; 100% coverage and 100% docstring coverage on `scripts/ci/`. +## 2026-08-31 `ORCHESTRATOR_PIN_SHA` bumped to carry #925's stream_options/tools fix + +**Context**: `#1451` fixed a separate, org-wide `pingora_edge_policy.py` coverage +gap blocking `opencode-review-dispatch.yml`'s own `coverage-evidence` job for +every `.github`-hosted PR. Once that landed and Strix could actually complete +scans again (via `#1448`'s scoped `LLM_DISABLE_STREAMING` workaround), +`ContextualWisdomLab/contextual-orchestrator#925` — the real root-cause fix for +the gateway's `stream_options.include_usage=true` + `tools` rejection — merged +(`7944a3c`). `.github#1463` reverts `#1448`'s workaround now that the gateway +itself no longer rejects that combination. + +**Devin Review correctly caught a real bug in that revert before merge**: the +review sidecar vendors `contextual-orchestrator` at a *pinned* SHA +(`ORCHESTRATOR_PIN_SHA`), not live `main` — and the pin in place at revert time +(`30c6d71680e659f25a0a433d4726ad0d437f9757`) was cut *before* `#925` merged. +Confirmed by `git merge-base --is-ancestor 30c6d716... 7944a3c` (true). Removing +the Strix-side streaming workaround while the vendored gateway still ran the +old, rejecting code would have restored the exact failure `#1448` existed to +route around — every Strix scan through the sidecar would fail again. + +**Fix**: bumped `ORCHESTRATOR_PIN_SHA` to `7944a3cd98f7b60fba9272e7f89c3977a75af746` +(the `#925` merge commit itself — deliberately not `contextual-orchestrator`'s +later tip, to keep this bump minimal and scoped to exactly the fix this revert +depends on) in the three places this repo's own convention requires kept in +sync: `scripts/ci/contextual_orchestrator_review_sidecar.sh`'s default, +`tests/test_contextual_orchestrator_review_sidecar_contract.py`'s pinned-SHA +contract assertion, and `docs/adr/0003-contextual-orchestrator-vendored-free-zdr.md`'s +"today" reference. Landed in the same PR (`#1463`) as the streaming revert, +not split out, since the revert is unsafe without it. + ## 5. 실행 루프와 고객의 다음 행동 각 hourly pass는 아래 순서를 유지한다. diff --git a/scripts/ci/strix_quick_gate.sh b/scripts/ci/strix_quick_gate.sh index 4f0d7b1ca4..57b81f0ad8 100755 --- a/scripts/ci/strix_quick_gate.sh +++ b/scripts/ci/strix_quick_gate.sh @@ -2651,20 +2651,6 @@ run_strix_once() { if ! resolved_target_path="$(resolve_current_target_path "$TARGET_PATH")"; then return 1 fi - # contextual-orchestrator's gateway deliberately rejects any request that - # combines stream_options.include_usage=true with tools (a correctness - # guarantee against silently-incomplete usage accounting; out of scope to - # change here). Strix's agent loop always streams and always sends tools, - # so every call through that gateway hits the rejection immediately. - # Strix itself ships an opt-in for exactly this: LLM_DISABLE_STREAMING=true - # makes each turn a single non-streaming get_response (stream:false on the - # wire, so stream_options is never sent) replayed as one terminal stream - # event; nothing else about the run loop changes. Scope it narrowly to the - # contextual-orchestrator loopback so other providers keep real streaming. - local strix_disable_streaming="false" - if is_contextual_orchestrator_api_base "$llm_api_base_value"; then - strix_disable_streaming="true" - fi local start_epoch start_epoch="$(date +%s)" local child_llm_api_key="" @@ -2696,7 +2682,6 @@ run_strix_once() { STRIX_CHILD_EXECUTABLE_ROOT="$STRIX_EXECUTABLE_ROOT" \ STRIX_CHILD_EXECUTABLE_SHA256="$STRIX_EXECUTABLE_SHA256" \ STRIX_CHILD_REQUIRE_EXECUTABLE_INTEGRITY="${IS_PR_EVIDENCE_RUN:-false}" \ - STRIX_CHILD_DISABLE_STREAMING="$strix_disable_streaming" \ python3 - "$timeout_seconds" "$resolved_target_path" "$SCAN_MODE" "$STRIX_LOG" "$STRIX_SCAN_WORKING_DIR" <<'PY' import hashlib import hmac @@ -2751,12 +2736,6 @@ child_env["LLM_MODEL"] = os.environ["STRIX_CHILD_MODEL"] if os.environ.get("STRIX_CHILD_LLM_API_KEY"): child_env["LLM_API_KEY"] = os.environ["STRIX_CHILD_LLM_API_KEY"] child_env["STRIX_REPORTS_DIR"] = os.environ["STRIX_CHILD_REPORTS_DIR"] -if os.environ.get("STRIX_CHILD_DISABLE_STREAMING", "").strip().lower() == "true": - # See the comment above strix_disable_streaming's assignment in bash: - # this routes only the contextual-orchestrator gateway through Strix's - # own non-streaming fallback so stream_options is never sent alongside - # tools, without touching how Strix talks to any other provider. - child_env["LLM_DISABLE_STREAMING"] = "true" for key, value in os.environ.items(): if key.startswith("FAKE_STRIX_") and value: child_env[key] = value diff --git a/scripts/ci/test_strix_quick_gate.sh b/scripts/ci/test_strix_quick_gate.sh index 4053f4fd53..01b7b19a5a 100644 --- a/scripts/ci/test_strix_quick_gate.sh +++ b/scripts/ci/test_strix_quick_gate.sh @@ -96,6 +96,13 @@ assert_file_not_contains() { fi } +required_workflow_bootstrap_has_if() { + local bootstrap_file="$1" + + awk '/^ required-workflow-bootstrap:$/{p=1; print; next} p && /^ [A-Za-z0-9_-]+:/{exit} p' "$bootstrap_file" | + grep '^[[:space:]]*if:' >/dev/null +} + seal_opencode_test_artifacts() { local runner_temp="$1" local head_sha="$2" @@ -327,9 +334,12 @@ assert_strix_workflow_pr_trigger_hardened() { assert_file_contains "$GATE_SCRIPT" 'child_env["PNPM_CONFIG_IGNORE_SCRIPTS"] = "true"' "strix gate child process disables pnpm lifecycle scripts" assert_file_contains "$GATE_SCRIPT" 'child_env["YARN_ENABLE_SCRIPTS"] = "false"' "strix gate child process disables yarn lifecycle scripts" assert_file_contains "$GATE_SCRIPT" 'child_env["PYTHONWARNINGS"] = "ignore:Pydantic serializer warnings:UserWarning:pydantic.main"' "strix gate child env narrowly filters the known third-party Pydantic serializer warning" - assert_file_contains "$GATE_SCRIPT" 'if is_contextual_orchestrator_api_base "$llm_api_base_value"; then' "strix gate scopes the non-streaming opt-in to the contextual-orchestrator loopback gateway" - assert_file_contains "$GATE_SCRIPT" 'STRIX_CHILD_DISABLE_STREAMING="$strix_disable_streaming"' "strix gate threads the streaming opt-in through to the child process environment" - assert_file_contains "$GATE_SCRIPT" 'child_env["LLM_DISABLE_STREAMING"] = "true"' "strix gate disables Strix's own SDK streaming for the contextual-orchestrator gateway, which rejects stream_options.include_usage alongside tools" + # contextual-orchestrator#925 (merged) fixed the gateway's rejection of + # stream_options.include_usage=true alongside tools -- the actual root + # cause #1448's LLM_DISABLE_STREAMING opt-in routed around. That opt-in is + # reverted (this PR); these guard against it silently reappearing. + assert_file_not_contains "$GATE_SCRIPT" 'STRIX_CHILD_DISABLE_STREAMING="$strix_disable_streaming"' "strix gate no longer threads a streaming opt-in through to the child process environment" + assert_file_not_contains "$GATE_SCRIPT" 'child_env["LLM_DISABLE_STREAMING"] = "true"' "strix gate no longer disables Strix's own SDK streaming for the contextual-orchestrator gateway" assert_file_contains "$GATE_SCRIPT" '[[ "$normalized_changed_file" =~ ^backend/.+\.py$ ]]' "strix gate detects nested backend Python files for PR-scoped import context" assert_file_contains "$GATE_SCRIPT" '[[ "$normalized_changed_file" == scripts/ci/test_*.sh || "$normalized_changed_file" == scripts/ci/*_test.sh ]]' "strix gate excludes large CI test harness scripts from model scan input" assert_file_contains "$GATE_SCRIPT" "Materialized PR-head changed-file scope for Strix scan" "strix gate avoids copying the full PR head tree into privileged scan targets by default" @@ -522,9 +532,31 @@ assert_opencode_review_uses_codegraph_and_contextual_orchestrator() { assert_file_not_contains "$workflow_file" "Wait for trusted OpenCode approval review" "opencode pull_request bridge was removed to avoid duplicate required-check resource use" assert_file_not_contains "$workflow_file" "Trusted OpenCode requested changes for head" "opencode pull_request bridge no longer reconsumes stale trusted review state" assert_file_not_contains "$workflow_file" "github.event.pull_request.number == 240" "opencode review workflow must not hard-code repository-specific PR bypasses" - if awk '/^ required-workflow-bootstrap:$/,/^[^ ]/' "$bootstrap_file" | grep -q '^[[:space:]]*if:'; then + # Match against the full awk output rather than letting `grep -q` close its + # end of the pipe on the first match: a large bootstrap job's piped output + # can exceed the OS pipe buffer, and `grep -q`'s early exit can SIGPIPE the + # still-writing awk producer. Under `set -o pipefail` (top of this file) + # that SIGPIPE (128+13=141) outranks grep's own 0 exit, so the `if` + # incorrectly takes the "no match" branch even though the forbidden `if:` + # key was found. Dropping `-q` makes grep read to completion, so it never + # closes the pipe early and the real exit status is preserved. + if required_workflow_bootstrap_has_if "$bootstrap_file"; then record_failure "opencode required workflow bootstrap must not depend on required-workflow event payload fields" fi + local large_bootstrap_fixture + local fixture_line + large_bootstrap_fixture="$(mktemp)" + { + printf '%s\n' 'jobs:' ' required-workflow-bootstrap:' ' if: forbidden' + for ((fixture_line = 0; fixture_line < 20000; fixture_line++)); do + printf '%s\n' ' # padding forces the producer past the pipe buffer' + done + printf '%s\n' ' next-job:' ' runs-on: ubuntu-latest' + } >"$large_bootstrap_fixture" + if ! required_workflow_bootstrap_has_if "$large_bootstrap_fixture"; then + record_failure "opencode required workflow bootstrap condition detection must survive a job block larger than the pipe buffer" + fi + rm -f "$large_bootstrap_fixture" assert_file_contains "$workflow_file" 'github.event.client_payload.target_repository || github.repository' "opencode review scopes concurrency by target repository" assert_file_contains "$workflow_file" "format('pr-{0}', github.event.client_payload.pr_number)" "opencode review scopes repository_dispatch concurrency by current PR" assert_file_not_contains "$workflow_file" "format('pr-{0}-{1}'" "opencode review does not keep stale head-specific concurrency groups" @@ -1500,8 +1532,12 @@ assert_opencode_review_posts_suggested_diffs_inline() { assert_file_contains "$workflow_file" "GitHub did not accept the inline review comments" "opencode review explains anchor failures instead of copying diffs to the PR body" assert_file_contains "$workflow_file" "publish_request_changes_from_control" "opencode review REQUEST_CHANGES path publishes findings from the control JSON" + # Same SIGPIPE-under-pipefail shape as the required-workflow-bootstrap + # check above: read the piped awk range to completion instead of letting + # `grep -q` close the pipe on its first match, which could otherwise + # SIGPIPE a still-writing awk and flip this check's exit status. if awk '/format_request_changes_body\(\)/,/build_request_changes_review_payload\(\)/ { print }' "$workflow_file" | - grep -Fq '```diff'; then + grep -F '```diff' >/dev/null; then record_failure "opencode review PR-level REQUEST_CHANGES body must not contain fenced suggested diffs" fi } @@ -3292,7 +3328,7 @@ set -euo pipefail printf '%s\n' "${STRIX_LLM:-}" >> "${FAKE_STRIX_CALL_LOG:?}" printf '%s\n' "${LLM_API_BASE:-}" >> "${FAKE_STRIX_API_BASE_LOG:?}" if [ -n "${FAKE_STRIX_RUNTIME_ENV_LOG:-}" ]; then - printf 'LLM_TIMEOUT=%s;STRIX_MEMORY_COMPRESSOR_TIMEOUT=%s;STRIX_REASONING_EFFORT=%s;STRIX_LLM_MAX_RETRIES=%s;GEMINI_LOCATION=%s;PYTHONWARNINGS=%s;NPM_CONFIG_IGNORE_SCRIPTS=%s;PNPM_CONFIG_IGNORE_SCRIPTS=%s;YARN_ENABLE_SCRIPTS=%s;UNRELATED_SECRET=%s;LLM_DISABLE_STREAMING=%s\n' \ + printf 'LLM_TIMEOUT=%s;STRIX_MEMORY_COMPRESSOR_TIMEOUT=%s;STRIX_REASONING_EFFORT=%s;STRIX_LLM_MAX_RETRIES=%s;GEMINI_LOCATION=%s;PYTHONWARNINGS=%s;NPM_CONFIG_IGNORE_SCRIPTS=%s;PNPM_CONFIG_IGNORE_SCRIPTS=%s;YARN_ENABLE_SCRIPTS=%s;UNRELATED_SECRET=%s\n' \ "${LLM_TIMEOUT:-}" \ "${STRIX_MEMORY_COMPRESSOR_TIMEOUT:-}" \ "${STRIX_REASONING_EFFORT:-}" \ @@ -3302,8 +3338,7 @@ if [ -n "${FAKE_STRIX_RUNTIME_ENV_LOG:-}" ]; then "${NPM_CONFIG_IGNORE_SCRIPTS:-}" \ "${PNPM_CONFIG_IGNORE_SCRIPTS:-}" \ "${YARN_ENABLE_SCRIPTS:-}" \ - "${UNRELATED_SECRET:-}" \ - "${LLM_DISABLE_STREAMING:-}" >> "${FAKE_STRIX_RUNTIME_ENV_LOG:?}" + "${UNRELATED_SECRET:-}" >> "${FAKE_STRIX_RUNTIME_ENV_LOG:?}" fi target_path="" @@ -5959,13 +5994,6 @@ PY "$runtime_env_log" \ "LLM_TIMEOUT=90;STRIX_MEMORY_COMPRESSOR_TIMEOUT=10;STRIX_REASONING_EFFORT=minimal;STRIX_LLM_MAX_RETRIES=1;GEMINI_LOCATION=GLOBAL;PYTHONWARNINGS=ignore:Pydantic serializer warnings:UserWarning:pydantic.main;NPM_CONFIG_IGNORE_SCRIPTS=true;PNPM_CONFIG_IGNORE_SCRIPTS=true;YARN_ENABLE_SCRIPTS=false;UNRELATED_SECRET=" \ "scenario=$scenario runtime env forwarding" - # Non-contextual-orchestrator providers (gemini here) never see the - # stream-disabling opt-in: it is scoped narrowly to the gateway that - # rejects stream_options.include_usage alongside tools. - assert_file_contains \ - "$runtime_env_log" \ - "LLM_DISABLE_STREAMING=" \ - "scenario=$scenario non-gateway providers keep real streaming" fi if [ "$scenario" = "custom-openai-compatible-preserves-effort" ]; then assert_file_contains \ @@ -5973,16 +6001,6 @@ PY "STRIX_REASONING_EFFORT=minimal" \ "scenario=$scenario custom compatible endpoint effort" fi - if [ "$scenario" = "contextual-orchestrator-gateway-model-qualification" ]; then - # contextual-orchestrator rejects stream_options.include_usage=true - # alongside tools; Strix's agent loop always sends both, so the gate - # routes this gateway through Strix's own LLM_DISABLE_STREAMING opt-in - # (single non-streaming get_response per turn) instead of streaming. - assert_file_contains \ - "$runtime_env_log" \ - "LLM_DISABLE_STREAMING=true" \ - "scenario=$scenario contextual-orchestrator gateway disables SDK streaming to avoid the stream_options+tools rejection" - fi if [ "$scenario" = "report-known-internal-warning-sanitized" ]; then assert_file_not_contains \