From c443e181261e1809df6067b681655a1d23893eb4 Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Wed, 1 Jul 2026 18:16:51 +0900 Subject: [PATCH 1/5] Guard autofix dispatch with current OpenCode review --- scripts/ci/pr_review_fix_scheduler.py | 6 ++++-- tests/test_pr_review_fix_scheduler.py | 11 ++++++++++- 2 files changed, 14 insertions(+), 3 deletions(-) diff --git a/scripts/ci/pr_review_fix_scheduler.py b/scripts/ci/pr_review_fix_scheduler.py index 97f1fd54c..63daa2bea 100755 --- a/scripts/ci/pr_review_fix_scheduler.py +++ b/scripts/ci/pr_review_fix_scheduler.py @@ -114,8 +114,10 @@ def change_request_is_autofixable(pr: dict[str, Any]) -> bool: def needs_autofix(pr: dict[str, Any]) -> tuple[bool, tuple[str, ...]]: """Return whether current-head evidence justifies an autofix attempt.""" reasons: list[str] = [] - if has_current_head_changes_requested(pr) and change_request_is_autofixable(pr): - reasons.append("current-head OpenCode requested changes") + if not (has_current_head_changes_requested(pr) and change_request_is_autofixable(pr)): + return False, () + + reasons.append("current-head OpenCode requested changes") unresolved = unresolved_thread_count(pr) if unresolved: reasons.append(f"{unresolved} active unresolved review thread(s)") diff --git a/tests/test_pr_review_fix_scheduler.py b/tests/test_pr_review_fix_scheduler.py index a838147a4..7e7552285 100644 --- a/tests/test_pr_review_fix_scheduler.py +++ b/tests/test_pr_review_fix_scheduler.py @@ -38,7 +38,7 @@ def test_recent_fix_marker_is_head_scoped(): def test_needs_autofix_uses_current_head_evidence(): - """Autofix only starts from current-head review or thread evidence.""" + """Autofix starts from current-head OpenCode change requests.""" head = "a" * 40 pr = make_pr( headRefOid=head, @@ -62,6 +62,15 @@ def test_needs_autofix_uses_current_head_evidence(): ) +def test_needs_autofix_ignores_thread_only_feedback(): + """Thread-only feedback must not start an autonomous autofix run.""" + pr = make_pr( + reviewThreads={"nodes": [{"id": "thread", "isResolved": False, "isOutdated": False}]}, + ) + + assert fix.needs_autofix(pr) == (False, ()) + + @pytest.mark.parametrize( ("merge_state", "body"), [ From f0c857c6d9ad5101c6f410e86b200c5b8eaa33f6 Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Wed, 1 Jul 2026 18:29:25 +0900 Subject: [PATCH 2/5] Extend OpenCode review timeout budget --- .github/workflows/opencode-review.yml | 12 ++++++------ .github/workflows/pr-review-autofix.yml | 2 +- scripts/ci/run_opencode_review_model_pool.sh | 4 ++-- scripts/ci/test_strix_quick_gate.sh | 8 ++++---- tests/test_opencode_agent_contract.py | 8 ++++---- 5 files changed, 17 insertions(+), 17 deletions(-) diff --git a/.github/workflows/opencode-review.yml b/.github/workflows/opencode-review.yml index 4b6a0abba..9c1f8f5b8 100644 --- a/.github/workflows/opencode-review.yml +++ b/.github/workflows/opencode-review.yml @@ -823,7 +823,7 @@ jobs: needs: [coverage-evidence] if: always() && (github.event_name == 'workflow_dispatch' || github.event_name == 'pull_request_target') runs-on: ubuntu-latest - timeout-minutes: 75 + timeout-minutes: 360 permissions: actions: write checks: read @@ -2239,7 +2239,7 @@ jobs: id: opencode_review_model_pool if: needs.coverage-evidence.result == 'success' continue-on-error: true - timeout-minutes: 20 + timeout-minutes: 300 env: STRIX_GITHUB_MODELS_TOKEN: ${{ secrets.STRIX_GITHUB_MODELS_TOKEN || github.token }} GITHUB_TOKEN: ${{ secrets.STRIX_GITHUB_MODELS_TOKEN || github.token }} @@ -2249,9 +2249,9 @@ jobs: NO_COLOR: "1" OPENCODE_MODEL_CANDIDATES: "github-models/openai/gpt-5-nano" OPENCODE_MODEL_ATTEMPTS: "1" - OPENCODE_RUN_TIMEOUT_SECONDS: "240" + OPENCODE_RUN_TIMEOUT_SECONDS: "18000" OPENCODE_EXPORT_TIMEOUT_SECONDS: "120" - OPENCODE_TOTAL_RETRY_BUDGET_SECONDS: "360" + OPENCODE_TOTAL_RETRY_BUDGET_SECONDS: "18000" OPENCODE_BACKOFF_INITIAL_SECONDS: "30" OPENCODE_BACKOFF_MAX_SECONDS: "30" OPENCODE_FIRST_ATTEMPT_AGENT: ci-review @@ -2602,7 +2602,7 @@ jobs: - name: Approve PR if OpenCode review passed if: always() - timeout-minutes: 75 + timeout-minutes: 300 env: GH_TOKEN: ${{ secrets.OPENCODE_APPROVE_TOKEN || steps.opencode_app_token.outputs.token || github.token }} CHECK_LOOKUP_GH_TOKEN: ${{ github.token }} @@ -3896,7 +3896,7 @@ jobs: } >"$prompt_file" cd "$OPENCODE_REVIEW_WORKDIR" - if ! timeout --kill-after=30s "${OPENCODE_RUN_TIMEOUT_SECONDS:-240}s" opencode run "$(cat "$prompt_file")" \ + if ! timeout --kill-after=30s "${OPENCODE_RUN_TIMEOUT_SECONDS:-18000}s" opencode run "$(cat "$prompt_file")" \ --pure \ --agent ci-review-fallback \ --model "$MODEL" \ diff --git a/.github/workflows/pr-review-autofix.yml b/.github/workflows/pr-review-autofix.yml index 6240ff320..a4b84e9e1 100644 --- a/.github/workflows/pr-review-autofix.yml +++ b/.github/workflows/pr-review-autofix.yml @@ -372,7 +372,7 @@ jobs: } trap restore_workspace_config EXIT cd "$TARGET_WORKSPACE" - timeout 900 opencode run "$(cat "$prompt_file")" \ + timeout 18000 opencode run "$(cat "$prompt_file")" \ --pure \ --agent ci-autofix \ --model "$MODEL" \ diff --git a/scripts/ci/run_opencode_review_model_pool.sh b/scripts/ci/run_opencode_review_model_pool.sh index 9ce4aaaee..063bb35a5 100644 --- a/scripts/ci/run_opencode_review_model_pool.sh +++ b/scripts/ci/run_opencode_review_model_pool.sh @@ -94,7 +94,7 @@ run_one_model_attempt() { local opencode_export_file="$8" local run_timeout_seconds export_timeout_seconds opencode_status session_id - run_timeout_seconds="${OPENCODE_RUN_TIMEOUT_SECONDS:-180}" + run_timeout_seconds="${OPENCODE_RUN_TIMEOUT_SECONDS:-18000}" export_timeout_seconds="${OPENCODE_EXPORT_TIMEOUT_SECONDS:-60}" rm -f "$opencode_json_file" "$opencode_export_file" "$candidate_output_file" @@ -149,7 +149,7 @@ main() { local opencode_json_file opencode_export_file agent retry_sleep original_run_timeout run_status attempts="${OPENCODE_MODEL_ATTEMPTS:-3}" - original_run_timeout="${OPENCODE_RUN_TIMEOUT_SECONDS:-900}" + original_run_timeout="${OPENCODE_RUN_TIMEOUT_SECONDS:-18000}" deadline=$((SECONDS + ${OPENCODE_TOTAL_RETRY_BUDGET_SECONDS:-18000})) : >"$OPENCODE_OUTPUT_FILE" cd "$OPENCODE_REVIEW_WORKDIR" diff --git a/scripts/ci/test_strix_quick_gate.sh b/scripts/ci/test_strix_quick_gate.sh index ffea57cda..8c3d5aeae 100755 --- a/scripts/ci/test_strix_quick_gate.sh +++ b/scripts/ci/test_strix_quick_gate.sh @@ -508,8 +508,8 @@ assert_opencode_review_uses_codegraph_and_gpt5_fallback() { assert_file_contains "$REPO_ROOT/scripts/ci/run_opencode_review_model_pool.sh" "Read and follow the complete review contract" "opencode review uses a compact launcher while keeping the full review contract on disk" assert_file_contains "$REPO_ROOT/scripts/ci/run_opencode_review_model_pool.sh" "tokens_limit_reached" "opencode review detects provider context-window overflow" assert_file_contains "$REPO_ROOT/scripts/ci/run_opencode_review_model_pool.sh" "skipping remaining attempts for this model" "opencode review skips same-model retries after context-window overflow" - assert_file_contains "$workflow_file" 'OPENCODE_RUN_TIMEOUT_SECONDS: "600"' "opencode primary review has a bounded per-model timeout before trying fallback models" - assert_file_contains "$workflow_file" 'OPENCODE_TOTAL_RETRY_BUDGET_SECONDS: "3600"' "opencode model pool has a one-hour total retry budget" + assert_file_contains "$workflow_file" 'OPENCODE_RUN_TIMEOUT_SECONDS: "18000"' "opencode primary review has a bounded per-model timeout before trying fallback models" + assert_file_contains "$workflow_file" 'OPENCODE_TOTAL_RETRY_BUDGET_SECONDS: "18000"' "opencode model pool has a five-hour total retry budget" assert_file_contains "$workflow_file" "needs.coverage-evidence.result == 'success'" "opencode model pool only runs after coverage evidence passed" assert_file_contains "$workflow_file" "id: opencode_review_model_pool" "opencode DeepSeek V3 fallback still runs after a primary model timeout or step failure when coverage evidence passed" assert_file_contains "$workflow_file" "always()" "opencode fallback chain uses always() so failed model steps cannot skip every fallback" @@ -578,7 +578,7 @@ assert_opencode_review_uses_codegraph_and_gpt5_fallback() { assert_file_contains "$workflow_file" 'load_selected_review_output()' "opencode approval step has a direct selected-output fallback when the overview comment is stale or invalid" assert_file_contains "$workflow_file" "gate result from Review Overview comment" "opencode approval step distinguishes overview-comment gate results" assert_file_contains "$workflow_file" "gate result from selected OpenCode output" "opencode approval step can recover from an invalid overview by validating the selected successful output" - assert_file_contains "$workflow_file" 'timeout-minutes: 75' "opencode approval step has a bounded wall-clock timeout" + assert_file_contains "$workflow_file" 'timeout-minutes: 300' "opencode approval step has a bounded wall-clock timeout" assert_file_contains "$workflow_file" 'APPROVAL_CHECK_WAIT_ATTEMPTS: "81"' "opencode approval waits for bounded long-running peer checks before approving" assert_file_contains "$workflow_file" 'CHECK_LOOKUP_RETRY_ATTEMPTS: "5"' "opencode approval retries transient GitHub check lookup failures before changing review state" assert_file_contains "$workflow_file" 'GitHub Checks lookup failed; retrying' "opencode approval logs transient check lookup retries" @@ -618,7 +618,7 @@ assert_opencode_review_uses_codegraph_and_gpt5_fallback() { assert_file_contains "$workflow_file" "no model produced a valid review control block" "opencode model-failure path documents why approval is withheld" assert_file_contains "$workflow_file" 'OPENCODE_MODEL_ATTEMPTS: "1"' "opencode primary and fallback paths avoid multi-attempt stalls on one model" assert_file_contains "$workflow_file" 'OPENCODE_MODEL_ATTEMPTS: "1"' "opencode catalog fallback tries each model once before moving on" - assert_file_contains "$workflow_file" 'OPENCODE_RUN_TIMEOUT_SECONDS: "600"' "opencode catalog fallback has a bounded model review timeout before step timeout" + assert_file_contains "$workflow_file" 'OPENCODE_RUN_TIMEOUT_SECONDS: "18000"' "opencode catalog fallback has a bounded model review timeout before step timeout" assert_file_contains "$REPO_ROOT/scripts/ci/run_opencode_review_model_pool.sh" "OpenCode %s attempt %s/%s failed" "opencode catalog fallback records per-model retry failures" assert_file_contains "$REPO_ROOT/scripts/ci/run_opencode_review_model_pool.sh" "exponential backoff" "opencode model retry paths use exponential backoff instead of fixed sleeps" assert_file_contains "$workflow_file" "github-models/openai/o3 github-models/openai/o3-mini github-models/openai/o4-mini" "opencode review includes additional OpenAI reasoning model fallbacks" diff --git a/tests/test_opencode_agent_contract.py b/tests/test_opencode_agent_contract.py index c5dd7a9b5..515e59179 100644 --- a/tests/test_opencode_agent_contract.py +++ b/tests/test_opencode_agent_contract.py @@ -210,15 +210,15 @@ def test_workflow_provisions_sandbox_tool_and_reviewer_agent(): assert '"## Review outcome"' in workflow assert '"## Check outcome"' not in workflow assert "publish REQUEST_CHANGES when coverage-evidence blocker states" in workflow - assert 'timeout-minutes: 75' in workflow - assert re.search(r"Run OpenCode PR Review model pool[\s\S]{0,240}timeout-minutes: 20", workflow) + assert re.search(r"opencode-review-target:[\s\S]{0,240}timeout-minutes: 360", workflow) + assert 'timeout-minutes: 300' in workflow assert 'APPROVAL_CHECK_WAIT_ATTEMPTS: "81"' in workflow assert 'APPROVAL_CHECK_WAIT_SLEEP_SECONDS: "30"' in workflow assert 'OPENCODE_MODEL_CANDIDATES: "github-models/openai/gpt-5-nano"' in workflow assert 'OPENCODE_MODEL_ATTEMPTS: "1"' in workflow - assert 'OPENCODE_RUN_TIMEOUT_SECONDS: "240"' in workflow + assert 'OPENCODE_RUN_TIMEOUT_SECONDS: "18000"' in workflow assert 'OPENCODE_EXPORT_TIMEOUT_SECONDS: "120"' in workflow - assert 'OPENCODE_TOTAL_RETRY_BUDGET_SECONDS: "360"' in workflow + assert 'OPENCODE_TOTAL_RETRY_BUDGET_SECONDS: "18000"' in workflow assert 'OPENCODE_BACKOFF_MAX_SECONDS: "30"' in workflow assert "${{ runner.temp }}/opencode-review-model-pool.md" in workflow assert re.search(r'check-runs" \\\n\s+-f per_page=100 \\\n\s+--paginate \\\n\s+--slurp \|\n\s+jq -r "\$jq_filter"', workflow) From d7ea1ee5f17a96c2f6ab5804e7ce3f21210ef22c Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Wed, 1 Jul 2026 18:45:42 +0900 Subject: [PATCH 3/5] Clarify autofix skip reason --- scripts/ci/pr_review_fix_scheduler.py | 2 +- tests/test_pr_review_fix_scheduler.py | 5 ++++- 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/scripts/ci/pr_review_fix_scheduler.py b/scripts/ci/pr_review_fix_scheduler.py index 63daa2bea..34a57826b 100755 --- a/scripts/ci/pr_review_fix_scheduler.py +++ b/scripts/ci/pr_review_fix_scheduler.py @@ -211,7 +211,7 @@ def inspect_pr( needs_fix, reasons = needs_autofix(pr) if not needs_fix: - return "skip", ("no current-head change request or active unresolved review thread",) + return "skip", ("no current-head autofixable OpenCode change request",) if comments is None: comments = issue_comments(repo, number) diff --git a/tests/test_pr_review_fix_scheduler.py b/tests/test_pr_review_fix_scheduler.py index 7e7552285..8b22055f2 100644 --- a/tests/test_pr_review_fix_scheduler.py +++ b/tests/test_pr_review_fix_scheduler.py @@ -390,7 +390,10 @@ def test_fix_inspect_skip_wait_and_error_paths(monkeypatch): ) monkeypatch.setattr(fix, "needs_autofix", lambda pr: (False, ())) - assert fix.inspect_pr("owner/repo", make_pr(), args) == ("skip", ("no current-head change request or active unresolved review thread",)) + assert fix.inspect_pr("owner/repo", make_pr(), args) == ( + "skip", + ("no current-head autofixable OpenCode change request",), + ) monkeypatch.setattr(fix, "needs_autofix", lambda pr: (True, ("reason",))) monkeypatch.setattr(fix, "issue_comments", lambda repo, number: [{"body": f"{fix.FIX_MARKER} head_sha={'a' * 40} epoch={int(time.time())} -->"}]) From 053b929945c70cba30f1eebe880cc4046ee9d664 Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Fri, 10 Jul 2026 18:54:48 +0900 Subject: [PATCH 4/5] Require implementation completeness in OpenCode review --- ci-review-prompt.md | 10 ++++++++ code-reviewer-prompt.md | 11 +++++++++ scripts/ci/opencode_review_prompt_template.md | 2 ++ tests/test_opencode_agent_contract.py | 23 +++++++++++++++++++ 4 files changed, 46 insertions(+) diff --git a/ci-review-prompt.md b/ci-review-prompt.md index 86ff7bd9c..7d3d81883 100644 --- a/ci-review-prompt.md +++ b/ci-review-prompt.md @@ -147,6 +147,16 @@ deployment, and operation paths instead of judging the changed hunk in isolation; flag contradictions between PR intent, code, docs, tests, schemas, generated files, UI rendering, and consumers. +Implementation completeness is mandatory. Inspect changed runtime code and +connected call sites for placeholder bodies such as `pass`, `...`, +`NotImplementedError`, TODO-only branches, fake or constant returns, and +unimplemented interface adapters. Distinguish `typing.Protocol`, +`@abc.abstractmethod`, overload declarations, and Pydantic `Field(...)` +declarations from executable implementation gaps before requesting changes or +approving. New user-visible or callable behavior needs a concrete +implementation, tests or verification, and documentation or contract updates +unless the code is explicitly abstract by design. + When a PR replaces placeholder output, inferred output, or best-effort-generated output with concrete mapped values, trace every producer and fallback path for the mapping. Block approval if legacy inputs, manual UI-created objects, diff --git a/code-reviewer-prompt.md b/code-reviewer-prompt.md index 74623aae0..a37fba11c 100644 --- a/code-reviewer-prompt.md +++ b/code-reviewer-prompt.md @@ -134,6 +134,17 @@ between PR intent, code, docs, tests, schemas, generated files, UI rendering, and consumers. For changed scrolling, animation, transition, or motion behavior, verify that `prefers-reduced-motion: reduce` users are not forced through smooth scrolling or animated motion. + +Implementation completeness is mandatory. Inspect changed runtime code and +connected call sites for placeholder bodies such as `pass`, `...`, +`NotImplementedError`, TODO-only branches, fake or constant returns, and +unimplemented interface adapters. Distinguish `typing.Protocol`, +`@abc.abstractmethod`, overload declarations, and Pydantic `Field(...)` +declarations from executable implementation gaps before requesting changes or +approving. New user-visible or callable behavior needs a concrete +implementation, tests or verification, and documentation or contract updates +unless the code is explicitly abstract by design. + When a PR replaces placeholder output, inferred output, or best-effort-generated output with concrete mapped values, trace each producer and fallback path for that mapping. Flag silent drops or regressions for legacy inputs, manual diff --git a/scripts/ci/opencode_review_prompt_template.md b/scripts/ci/opencode_review_prompt_template.md index 222882891..a804d17c6 100644 --- a/scripts/ci/opencode_review_prompt_template.md +++ b/scripts/ci/opencode_review_prompt_template.md @@ -12,6 +12,8 @@ Review by positive evidence, not by absence of known blockers. APPROVE is valid Find bugs. Compare the PR title, body, linked issue context, and actual diff, then inspect the connected code paths, rendering path, tests, docs, generated artifacts, deployment/operation paths, and previous behavior that the changed code now interacts with. Do not review the changed hunk as an isolated island: look for contradictions between the PR intent and repository code, between docs and code, between API/schema names and consumers, between UI rendering and state/data flow, between tests and implementation, and between generated files and their source of truth. If the PR promises files, tests, docs, migrations, generated artifacts, contracts, or behavior that are absent, request changes. Also infer missing files from source evidence: new imports without implementation, new routes without tests/docs, schema changes without migration/rollback, API or CLI behavior without contract tests, generated artifact sources without regenerated outputs, docs claims without code support, config changes without examples, and workflow/tooling changes without self-tests. When a required file is missing, anchor the finding to the closest changed reference, manifest, test, workflow, route, import, docs claim, or generated-artifact contract and explain exactly which file/artifact must be added or updated. Check correctness, edge cases, error paths, API compatibility, auth/authz, tenant isolation, secrets, privacy, data integrity, concurrency, migrations, deployment/rollback, observability, performance, resource use, dependency license and supply-chain risk, IaC/cloud/Docker behavior, package/build/test/lint/security contracts, repository conventions, accessibility, i18n/l10n, developer experience, and user experience. Check naming and reserved-word safety for every changed database object, table, column, primary key, foreign key, index, constraint, API field, event name, configuration key, route, class, function, method, file path, generated model, and serialized contract. Prefer the repository's existing convention, but require names to be specific, non-reserved, and meaningfully composed: avoid bare `id`, `name`, `type`, `value`, `data`, `user`, `order`, `group`, `key`, or SQL/platform reserved words when a two-word snake_case, camelCase, PascalCase, or local equivalent such as `order_item_id`, `projectId`, or `UserProfile` would be clearer and safer. For database primary keys, foreign keys, join tables, migrations, and generated ORM models, compare nearby schema conventions and flag ambiguous single-word identifiers or reserved words that can cause query, ORM, serialization, or cross-database portability bugs. At the start of review, define the UX and DX surfaces for this PR from evidence. UX surfaces may include web UI, CLI behavior, API responses, SDK/library contracts, generated files, docs, logs, error messages, workflow/status-check output, review comments, configuration, operator runbooks, onboarding/setup, and migration paths. DX surfaces may include local setup, scripts, tests, lint/coverage/security commands, CI reliability, error diagnostics, review feedback quality, package/release contracts, observability for maintainers, code readability, extension points, and conventions. If a surface is absent, name the closest affected human or automation interaction instead of writing "not applicable." For breaking changes, use git history and deployment evidence when available to discuss bridge modules, migration paths, rollout/rollback, and lower-version compatibility. +Implementation completeness is mandatory. Inspect changed runtime code and connected call sites for placeholder bodies such as `pass`, `...`, `NotImplementedError`, TODO-only branches, fake or constant returns, and unimplemented interface adapters. Distinguish `typing.Protocol`, `@abc.abstractmethod`, overload declarations, and Pydantic `Field(...)` declarations from executable implementation gaps before requesting changes or approving. New user-visible or callable behavior needs a concrete implementation, tests or verification, and documentation or contract updates unless the code is explicitly abstract by design. + Identifier exposure and enumeration safety is a security blocker, not a style note: when a primary key or any identifier exposed in an API response, URL path or query, redirect, filename, cache key, or other client-visible surface is a sequential or auto-incrementing integer (SERIAL/BIGSERIAL, AUTO_INCREMENT, IDENTITY, or an ORM auto-increment id), return REQUEST_CHANGES because sequential ids let attackers enumerate and reach other records (IDOR/enumeration — the Coupang breach exploited guessable sequential ids); require a non-sequential, non-guessable identifier at every exposed boundary such as a random UUIDv4 or random token, treat time-ordered ULID/UUIDv7 as acceptable only when creation-order leakage is harmless, and accept an internal-only auto-increment key solely when it is never exposed and a separate opaque identifier is used at every external boundary, treating unclear exposure as exposed. Require every newly added or renamed identifier — tables, columns, keys, indexes, constraints, API fields, event names, config keys, routes, classes, functions, methods, variables, files, generated models, and serialized contracts — to be composed of two or more meaningful words rather than a bare single word or reserved word, in the idiomatic case of that file's language (snake_case for Python/Ruby/Rust/SQL and DB columns, camelCase for JavaScript/TypeScript/Java/Kotlin/Swift members, PascalCase for types/classes and Go exported names, SCREAMING_SNAKE_CASE for constants), following the repository's existing convention where it differs and never forcing one language's casing onto another; a single-word or reserved name such as id, data, user, type, value, run, handler, or temp is a blocker when a two-word equivalent such as order_item_id, projectId, UserProfile, or parseRequest is clearer and safer, while short-lived loop indices and idiomatic single-letter math variables are exempt. For numerical programming, scientific programming, statistical modeling, simulation, optimization, signal processing, ML metrics, estimators, inference code, or formula-heavy implementations, obtain the original paper, specification, vignette, or authoritative reference through web_search/webfetch or official documentation before approving. Verify that formulas, constants, likelihoods, priors, gradients, convergence criteria, random seeds, tolerances, parameter constraints, and numerical stability tricks match the source or are explicitly justified. Require repo-native or scratch PoC evidence that the implementation recovers true parameters on known synthetic data, including skewed or ill-conditioned true-parameter regimes when the method claims robustness; compare against baseline or prior behavior when available. Strengthen the test case set before approving: do not accept a single happy-path test for one function when the scientific claim depends on multiple regimes. Add augmented scratch tests or require repository tests for balanced and skewed parameters, boundary values, degeneracy/zero-variance inputs, random-seed determinism, numerical tolerance, convergence failure, and prior-version or published-example parity as appropriate, then execute the relevant repository test command or sandboxed PoC. Lack of a host toolchain is not a reason to skip execution: provision an isolated Docker, Docker Compose, devcontainer, Nix, or temporary package-install sandbox and run the augmented verification there with no production credentials or persistent repository mutation. If an LLM or patch changes an equation, estimator, loss, distribution, or statistic without source-backed derivation and regression tests that would catch parameter-recovery failure, request changes. diff --git a/tests/test_opencode_agent_contract.py b/tests/test_opencode_agent_contract.py index 1b5052af4..55aad1dfc 100644 --- a/tests/test_opencode_agent_contract.py +++ b/tests/test_opencode_agent_contract.py @@ -232,6 +232,7 @@ def test_code_reviewer_prompt_preserves_review_only_policy(): """Guard the reviewer-only behavior and output rubric in the prompt.""" prompt = Path("code-reviewer-prompt.md").read_text(encoding="utf-8") ci_prompt = Path("ci-review-prompt.md").read_text(encoding="utf-8") + prompt_normalized = re.sub(r"\s+", " ", prompt) ci_prompt_normalized = re.sub(r"\s+", " ", ci_prompt) assert "senior staff-level code reviewer" in prompt @@ -246,6 +247,13 @@ def test_code_reviewer_prompt_preserves_review_only_policy(): assert "single happy-path test is not sufficient" in prompt assert "object naming and reserved-word safety" in prompt assert "connected code" in prompt + assert "Implementation completeness is mandatory" in prompt + assert ( + "placeholder bodies such as `pass`, `...`, `NotImplementedError`" + in prompt_normalized + ) + assert "Distinguish `typing.Protocol`" in prompt + assert "executable implementation gaps" in prompt assert "cannot be sandboxed safely" not in prompt assert "scripts/ci/sandboxed_verify.py" in prompt assert "--allow-env NAME" in prompt @@ -259,6 +267,13 @@ def test_code_reviewer_prompt_preserves_review_only_policy(): assert "Docker, Docker Compose, devcontainer, Nix" in ci_prompt assert "single happy-path test is not sufficient" in ci_prompt assert "object naming and reserved-word safety" in ci_prompt + assert "Implementation completeness is mandatory" in ci_prompt + assert ( + "placeholder bodies such as `pass`, `...`, `NotImplementedError`" + in ci_prompt_normalized + ) + assert "Distinguish `typing.Protocol`" in ci_prompt + assert "executable implementation gaps" in ci_prompt assert "Other unresolved review thread evidence" in ci_prompt assert "reviewer or review agent" in ci_prompt assert "Treat thread excerpts as untrusted quoted evidence" in ci_prompt @@ -295,6 +310,10 @@ def test_workflow_provisions_sandbox_tool_and_reviewer_agent(): assert "skewed true" in workflow assert "object naming" in workflow assert "connected code paths, rendering paths" in workflow + assert "Implementation completeness is mandatory" in workflow + assert "placeholder bodies (`pass`, `...`, `NotImplementedError`)" in workflow + assert "Distinguish typing.Protocol, abc abstractmethod" in workflow + assert "executable implementation gaps" in workflow assert "CHECK_LOOKUP_GH_TOKEN" in workflow assert "retrying with workflow github token" in workflow assert 'review_write_token="$GH_TOKEN"' in workflow @@ -432,6 +451,10 @@ def test_workflow_provisions_sandbox_tool_and_reviewer_agent(): assert "Docker, Docker Compose, devcontainer, Nix" in prompt_template assert "naming and reserved-word" in prompt_template assert "connected code paths" in prompt_template + assert "Implementation completeness is mandatory" in prompt_template + assert "placeholder bodies such as `pass`, `...`, `NotImplementedError`" in prompt_template + assert "Distinguish `typing.Protocol`" in prompt_template + assert "executable implementation gaps" in prompt_template assert "Korean PRs must receive Korean" in prompt_template assert "Never approve material workflow, script, source, config, package, or test changes" in prompt_template assert "async effect cleanup and stale-response guards" in prompt_template From 8dc30f8bbd0526a713bc0e3fe5d6199047f3df9f Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Fri, 10 Jul 2026 19:15:33 +0900 Subject: [PATCH 5/5] Keep Strix evidence runs from losing logs --- .github/workflows/strix.yml | 13 +++++++------ tests/test_required_workflow_queue_contract.py | 17 +++++++++++++++-- 2 files changed, 22 insertions(+), 8 deletions(-) diff --git a/.github/workflows/strix.yml b/.github/workflows/strix.yml index 3509ec77c..367fdb844 100644 --- a/.github/workflows/strix.yml +++ b/.github/workflows/strix.yml @@ -33,11 +33,12 @@ on: # path filters against the PR's full base..head diff, so a PR is skipped only # when EVERY changed file is a non-executable doc/image asset; any code, # config, build, or workflow change still triggers the scan. Concurrency is - # PR-number based, so newer heads cancel superseded scans and keep queue - # capacity focused on current-head evidence. For PRs the merge scheduler - # manages, same-head Strix evidence is still forced at merge time via - # workflow_dispatch (which paths-ignore does not affect), so merged code - # never loses evidence. + # PR-number based for status grouping, but Strix runs intentionally do not + # cancel in progress because a pre-job cancellation leaves no scanner log to + # review. Queue pressure should be handled by stale-run cleanup outside this + # current-head evidence path. For PRs the merge scheduler manages, same-head + # Strix evidence is still forced at merge time via workflow_dispatch (which + # paths-ignore does not affect), so merged code never loses evidence. paths-ignore: - '**/*.md' - '**/*.markdown' @@ -86,7 +87,7 @@ concurrency: strix-${{ github.event.pull_request.base.repo.full_name || github.event.inputs.target_repository || github.repository }}-${{ github.event_name == 'pull_request_target' && format('pr-{0}', github.event.pull_request.number) || github.event.inputs.pr_number != '' && format('pr-{0}', github.event.inputs.pr_number) || github.ref }} - cancel-in-progress: true + cancel-in-progress: false # Scorecard Token-Permissions (alert #43): keep the workflow-level token # read-only and grant the id-token/statuses writes only on the job that needs diff --git a/tests/test_required_workflow_queue_contract.py b/tests/test_required_workflow_queue_contract.py index ce5884b8c..43242c4be 100644 --- a/tests/test_required_workflow_queue_contract.py +++ b/tests/test_required_workflow_queue_contract.py @@ -28,7 +28,6 @@ def test_required_pull_request_workflows_cancel_superseded_runs() -> None: "osv-scanner-pr.yml", "security-scan.yml", "scorecard-pr.yml", - "strix.yml", ): workflow = workflow_text(filename) concurrency_contract = workflow.split("permissions:", 1)[0] @@ -42,6 +41,20 @@ def test_required_pull_request_workflows_cancel_superseded_runs() -> None: assert "format('pr-{0}-{1}'" not in concurrency_contract +def test_strix_preserves_current_head_evidence_runs_until_logs_exist() -> None: + workflow = workflow_text("strix.yml") + concurrency_contract = workflow.split("permissions:", 1)[0] + + assert "concurrency:" in workflow + assert "github.event.pull_request.base.repo.full_name" in concurrency_contract + assert "github.repository" in concurrency_contract + assert "github.event.pull_request.number" in workflow + assert "cancel-in-progress: false" in workflow + assert "pre-job cancellation leaves no scanner log" in workflow + assert "github.event.pull_request.head.sha" not in concurrency_contract + assert "format('pr-{0}-{1}'" not in concurrency_contract + + def test_pull_request_close_events_cancel_superseded_runs_without_heavy_jobs() -> None: workflows = ( "close-empty-pr.yml", @@ -66,7 +79,7 @@ def test_pull_request_close_events_cancel_superseded_runs_without_heavy_jobs() - ) assert "github.event.action != 'closed'" in workflow - assert "cancel-in-progress: true" in workflow_text("strix.yml") + assert "cancel-in-progress: false" in workflow_text("strix.yml") def test_cancelled_review_workflow_runs_do_not_spawn_more_queue_work() -> None: