From 7899b483fe54cd1da7858bfc501479347867f45a Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Fri, 4 Sep 2026 21:19:13 +0900 Subject: [PATCH] fix(codeql): restore required startup recovery Signed-off-by: Seongho Bae --- .github/workflows/codeql-scan-dispatch.yml | 9 ++-- ...-failure-and-strix-concurrency-20260904.md | 7 +-- docs/org-required-workflow-rollout.md | 37 ++++++++-------- docs/product-technical-gap-baseline.md | 11 ++++- .../ci/audit_central_required_workflows.py | 1 + scripts/ci/pr_review_merge_scheduler_core.py | 2 - ...ntral_required_workflow_exact_inventory.py | 3 +- ...central_required_workflow_ruleset_audit.py | 20 ++++----- ...ode_scanning_required_workflow_contract.py | 16 +++---- ..._codeql_scan_dispatch_workflow_contract.py | 16 +++---- tests/test_pr_review_merge_scheduler.py | 43 ++++++++++++++++++- 11 files changed, 106 insertions(+), 59 deletions(-) diff --git a/.github/workflows/codeql-scan-dispatch.yml b/.github/workflows/codeql-scan-dispatch.yml index 72fc246ed9..2934731071 100644 --- a/.github/workflows/codeql-scan-dispatch.yml +++ b/.github/workflows/codeql-scan-dispatch.yml @@ -1,12 +1,9 @@ # Runs github/codeql-action outside any required-workflow context. GitHub # categorically refuses to admit init/analyze inside a required workflow # (docs/doctoring/codeql-pr-required-workflow-always-fails.md); this file is -# the native execution half of the dispatch+poll design proposed in -# ContextualWisdomLab/.github#1772. -# -# NOT YET WIRED UP: codeql-pr.yml does not dispatch here yet (that rewrite is -# a separate, still-pending follow-up so it can get independent review). Do -# not add workflow_dispatch here to allow manual testing: +# the native execution half of the dispatch+poll design implemented by +# ContextualWisdomLab/.github#1778. Do not add workflow_dispatch here to allow +# manual testing: # test_no_central_workflow_exposes_branch_selected_manual_dispatch (in # tests/test_required_workflow_queue_contract.py) forbids it on every central # workflow, because workflow_dispatch runs the workflow file as it exists on diff --git a/docs/doctoring/startup-failure-and-strix-concurrency-20260904.md b/docs/doctoring/startup-failure-and-strix-concurrency-20260904.md index 382037d59c..e710eb5d1f 100644 --- a/docs/doctoring/startup-failure-and-strix-concurrency-20260904.md +++ b/docs/doctoring/startup-failure-and-strix-concurrency-20260904.md @@ -25,9 +25,10 @@ current head. When any latest PR run has `startup_failure`, it reuses the existing guarded same-tree restamp operation to create one new head and one fresh `synchronize` event. A newer queued or completed run suppresses recovery, and a head whose latest commit is already the recovery restamp is not -restamped again. The retired required -`CodeQL PR` workflow is excluded explicitly; its platform prohibition was -fixed by the existing dispatch-and-poll architecture and must not be retried. +restamped again. The former direct-CodeQL required workflow was excluded while +its platform prohibition remained. The dispatch-and-poll architecture has +since removed all `github/codeql-action` use from the required entrypoint, so +CodeQL now uses the same guarded recovery path as every other pre-job failure. The PR head is re-read immediately before mutation, and the operation remains restricted to same-repository branches plus a credential that GitHub permits to start workflows. diff --git a/docs/org-required-workflow-rollout.md b/docs/org-required-workflow-rollout.md index d57a0fa302..d56be9840c 100644 --- a/docs/org-required-workflow-rollout.md +++ b/docs/org-required-workflow-rollout.md @@ -12,7 +12,8 @@ Use an organization repository ruleset instead of copying workflow files into ea - Target: branch rules on every repository's default branch (`repository_name.include=["~ALL"]`, `ref_name.include=["~DEFAULT_BRANCH"]`) - Required workflow source repository: `ContextualWisdomLab/.github` - Required workflow source repository ID: `1274066402` -- Active required workflow paths (live-verified 2026-09-04, six entries): +- Canonical required workflow paths (seven entries): + - `.github/workflows/codeql-pr.yml` - `.github/workflows/noema-review.yml` - `.github/workflows/opencode-review.yml` - `.github/workflows/pr-review-merge-scheduler.yml` @@ -102,20 +103,18 @@ Keep the OpenCode required workflow active only while the central workflow keeps ## Code scanning required workflow posture -**Superseded (2026-09-03): `codeql-pr.yml` is deliberately no longer required-workflow-injected.** -GitHub categorically disallows `github/codeql-action/init` and `github/codeql-action/analyze` inside a -ruleset-required workflow — every ruleset-injected `codeql-pr.yml` run across every one of the ~71 covered -repositories concluded `startup_failure` with zero check runs ever created (a platform restriction, not a -configuration defect this repo could fix; the REST API surfaces no reason, only the run page's web UI -annotation does; see `docs/product-technical-gap-baseline.md`, item 41). `codeql-pr.yml` was removed from -ruleset `18156473`'s required `workflows` list (verify live via `gh api orgs/ContextualWisdomLab/rulesets/18156473`; -six entries remain, with OSV and Scorecard consolidated under `security-scan.yml`). Coverage now comes -from GitHub's native code-scanning default setup, enabled directly per repository -(`code-scanning/default-setup` state `configured`) rather than through this ruleset — including the 23 -repositories given real coverage as part of the same fix, and 16 more found by a later, wider sweep (item -41's own entry has the full breakdown). **Do not treat the paragraphs below as current operator guidance or -"drift" to restore** — they describe the pre-2026-09-03 design and are kept for history. Do not re-add any -workflow using `github/codeql-action` to a required-workflow ruleset entry. +**Correction (2026-09-04): restore the dispatch-safe CodeQL entrypoint.** +The 2026-09-03 removal was correct for the old workflow, which called +`github/codeql-action` directly and always failed at startup. The current +`codeql-pr.yml` contains no such action. It validates the exact live head, +dispatches the scan to the native `codeql-scan-dispatch.yml`, and waits for an +app-authored `codeql-dispatch/` status. Ruleset `18156473` must require +this dispatch-safe entrypoint after its audit contract reaches protected main. +The scheduler may then same-tree restamp a future CodeQL `startup_failure` just +like any other pre-job failure. Native default setup remains a repository-local +safety net; it does not replace the central required gate. Do not add any +workflow that invokes `github/codeql-action` directly to a required-workflow +ruleset. The org's `default_for_new_repos: "all"` policy (configuration `17`, "GitHub recommended") is supposed to make this automatic for every newly created repository, but item 41's investigation confirmed it is empirically unreliable for this org: 11 non-fork repositories created between 2026-05-09 and 2026-08-18 — @@ -132,10 +131,10 @@ technique (checking out `refs/pull//merge` and uploading SARIF with `sha: pull_request.merge_commit_sha` because the ruleset evaluates that commit, not the ephemeral merge ref OID) before its removal above. -Repository-local `codeql.yml` push/default-branch scans, or GitHub's native -`code-scanning/default-setup`, are now the only source of CodeQL coverage — -PR merge gates cannot rely on a central required-workflow CodeQL check for the -platform reason above. +Repository-local CodeQL and native default setup may coexist with the central +gate only when they do not compete to upload the same SARIF. The central native +dispatch handler analyzes the target head without making the target repository's +default-setup upload path its source of truth. ### Repository-local CodeQL inventory (2026-07-04) — HISTORICAL, superseded 2026-09-03 diff --git a/docs/product-technical-gap-baseline.md b/docs/product-technical-gap-baseline.md index f154bfec0d..c8347f7189 100644 --- a/docs/product-technical-gap-baseline.md +++ b/docs/product-technical-gap-baseline.md @@ -2774,7 +2774,16 @@ prose" convention already stated in `CLAUDE.md`. **Formerly open, gateway-owned — now fixed, PR open.** The missing model/provider attribution on the real-call failure path (`served_model=unknown` where preflight proves the sidecar can report this detail) is root-caused and fixed: `ContextualWisdomLab/contextual-orchestrator#1037` (branch `fix/invoke-failover-attempt-telemetry`, based on `main` @ `f4e5fc67`, open, not yet merged). Root cause: `TaskOrchestrator._invoke`'s failover loop (`contextual_orchestrator/orchestrator.py:7660-7893`) tracked only the single most recent candidate's failure (`last_upstream_error`/`last_provider_response_error`, overwritten on every new candidate), discarding every earlier candidate's `agent_id`/`model`/`provider_name`/failure reason the moment the loop moved on — so a fully-exhausted pool's raised exception could only ever describe the last agent tried, exactly matching the `served_model=unknown` symptom above. Fix: `ProviderUpstreamError.detail` now conditionally surfaces `attempts` (one record per candidate: `agent_id`/`model`/`provider`/`error_code`/`provider_status`/`retryable`/`retry_attempt`, reusing the existing `_record_tool_fallback` shape — never raw exception text) and `stop_reason`, populated at all 3 of `_invoke`'s existing "candidate exhausted" exit points; `server.py`'s error-message helper surfaces the count/reason; a second, compounding bug (the 413 `request_too_large` handler silently dropping `exc.detail` via a missing 4th `_send_error` argument) was fixed alongside it since it shares the same attribution-loss shape. RED-then-GREEN on 3 new tests, regression guards (`test_detail_and_transport_are_preserved_for_callers`, `test_invoke_preserves_final_classified_failure_across_candidates`, `test_all_agents_failing_raises_after_trying_every_candidate`) confirmed unmodified, full suite green. Zero line-range overlap with the concurrently-active PR #1032 (confirmed via diff comparison — #1032 touches `_orchestrated_provider_completion`'s schema-repair accounting; this touches `_invoke`'s failover loop, a different code path), branched from `main` directly rather than stacked. `.github`-side follow-up still needed once both #1661 and #1037 land: `scripts/ci/noema_review_gate.py`'s `call_llm` catches `urllib.error.HTTPError` without calling `exc.read()`, so it cannot see the response body CO now sends on failure, and `_extract_served_model` only reads a top-level `data.get("model")` while CO nests everything under `error.detail`/`error_detail` — the caller needs its own small patch to actually surface what the gateway now provides. -## Item 41: CodeQL PR `startup_failure` blocking merges org-wide — existing-repo gap closed, future-repo gap open +## Item 41: CodeQL PR `startup_failure` blocking merges org-wide — dispatch-safe re-admission in progress + +**2026-09-04 correction.** The emergency ruleset removal below fixed the old +entrypoint, but became stale after `.github#1778` moved `github/codeql-action` +into the native `codeql-scan-dispatch.yml` handler. Seven current PR heads then +materialized every other central workflow but no `CodeQL PR` run because +ruleset `18156473` still omitted the now-safe entrypoint. Completion therefore +requires protected-main audit/recovery contracts, a live ruleset re-add that +preserves every unrelated field, and fresh exact-head runs that do not conclude +`startup_failure`; configuration text alone is not completion evidence. **Problem.** Every ruleset-injected `codeql-pr.yml` run in every repository covered by org ruleset `18156473` (confirmed: bandscope, naruon, aFIPC, pg-erd-cloud, xtrmLLMBatchPython, wardnet, spanning 2026-09-02T20:12:52Z through 2026-09-03T03:15:43Z) concluded `startup_failure` with **zero check runs created** — while every other required workflow in the same PRs at the same time enqueued normally. Example: [wardnet run 33710719228](https://github.com/ContextualWisdomLab/wardnet/actions/runs/33710719228). diff --git a/scripts/ci/audit_central_required_workflows.py b/scripts/ci/audit_central_required_workflows.py index 4d84042d81..cc27d5db7c 100755 --- a/scripts/ci/audit_central_required_workflows.py +++ b/scripts/ci/audit_central_required_workflows.py @@ -24,6 +24,7 @@ # while still being validated from an organization-admin ruleset payload. REQUIRED_EXCLUSION_PROBES = {".github", "noema"} REQUIRED_WORKFLOW_PATHS = ( + ".github/workflows/codeql-pr.yml", ".github/workflows/noema-review.yml", ".github/workflows/opencode-review.yml", ".github/workflows/pr-review-merge-scheduler.yml", diff --git a/scripts/ci/pr_review_merge_scheduler_core.py b/scripts/ci/pr_review_merge_scheduler_core.py index 66a9d0e286..48a5f56019 100644 --- a/scripts/ci/pr_review_merge_scheduler_core.py +++ b/scripts/ci/pr_review_merge_scheduler_core.py @@ -2890,8 +2890,6 @@ def recover_current_head_startup_failures( if run.get("head_sha") == head_sha and run.get("status") == "completed" and run.get("conclusion") == "startup_failure" - and run.get("name") != "CodeQL PR" - and not str(run.get("path") or "").endswith("/codeql-pr.yml") ] if ( retryable diff --git a/tests/test_central_required_workflow_exact_inventory.py b/tests/test_central_required_workflow_exact_inventory.py index ef11acc839..6b3e9b650e 100644 --- a/tests/test_central_required_workflow_exact_inventory.py +++ b/tests/test_central_required_workflow_exact_inventory.py @@ -6,6 +6,7 @@ EXPECTED_REQUIRED_WORKFLOW_PATHS = ( + ".github/workflows/codeql-pr.yml", ".github/workflows/noema-review.yml", ".github/workflows/opencode-review.yml", ".github/workflows/pr-review-merge-scheduler.yml", @@ -16,7 +17,7 @@ def _ruleset_payload() -> dict: - """Build an independent six-workflow live-policy oracle.""" + """Build an independent seven-workflow live-policy oracle.""" return { "id": audit.RULESET_ID, "name": audit.RULESET_NAME, diff --git a/tests/test_central_required_workflow_ruleset_audit.py b/tests/test_central_required_workflow_ruleset_audit.py index cb0a052907..77bbf53305 100644 --- a/tests/test_central_required_workflow_ruleset_audit.py +++ b/tests/test_central_required_workflow_ruleset_audit.py @@ -10,6 +10,7 @@ def ruleset_payload() -> dict: """Return the expected live central required-workflow ruleset shape.""" workflow_paths = ( + "codeql-pr.yml", "noema-review.yml", "opencode-review.yml", "pr-review-merge-scheduler.yml", @@ -112,7 +113,7 @@ def test_expected_central_ruleset_passes(monkeypatch, capsys) -> None: assert audit.main([]) == 0 assert ( - "PASS: ruleset 18156473 enforces 6 central required workflows" + "PASS: ruleset 18156473 enforces 7 central required workflows" in capsys.readouterr().out ) @@ -289,21 +290,19 @@ def test_readded_scorecard_workflow_reports_duplicate_scan() -> None: ) -def test_readded_codeql_workflow_alongside_full_set_reports_unexpected_entry() -> None: +def test_missing_codeql_workflow_reports_exact_drift() -> None: payload = ruleset_payload() workflow_rule = next(rule for rule in payload["rules"] if rule["type"] == "workflows") - workflow_rule["parameters"]["workflows"].append( - { - "repository_id": 1274066402, - "path": ".github/workflows/codeql-pr.yml", - "ref": "refs/heads/main", - } - ) + workflow_rule["parameters"]["workflows"] = [ + workflow + for workflow in workflow_rule["parameters"]["workflows"] + if workflow["path"] != ".github/workflows/codeql-pr.yml" + ] errors = audit.audit_ruleset(payload) assert ( - "unexpected workflow present in required set: .github/workflows/codeql-pr.yml" + "missing central required workflow .github/workflows/codeql-pr.yml" in errors ) @@ -383,6 +382,7 @@ def test_audit_reports_all_structural_and_protection_drift() -> None: "central ruleset repository exclusions drifted: expected ['.github', 'IRT-bibliography-set', 'noema'], got []", "central ruleset does not target every default branch", "expected one workflows rule, found 0", + "missing central required workflow .github/workflows/codeql-pr.yml", "missing central required workflow .github/workflows/noema-review.yml", "missing central required workflow .github/workflows/opencode-review.yml", "missing central required workflow .github/workflows/pr-review-merge-scheduler.yml", diff --git a/tests/test_code_scanning_required_workflow_contract.py b/tests/test_code_scanning_required_workflow_contract.py index 204e234a0d..19933303d8 100644 --- a/tests/test_code_scanning_required_workflow_contract.py +++ b/tests/test_code_scanning_required_workflow_contract.py @@ -38,11 +38,11 @@ def test_consolidated_security_scan_preserves_osv_and_scorecard_evidence() -> No assert "Upload Scorecard SARIF to code scanning" in workflow -def test_ruleset_audit_deliberately_excludes_codeql_pr() -> None: - """codeql-pr.yml must stay out of the required set (github/codeql-action cannot - - run inside a ruleset-required workflow -- see the 2026-09-03 correction in - docs/org-required-workflow-rollout.md). A re-add here would silently - re-introduce the 100% startup_failure regression the removal fixed. - """ - assert ".github/workflows/codeql-pr.yml" not in audit.REQUIRED_WORKFLOW_PATHS +def test_ruleset_requires_dispatch_safe_codeql_pr() -> None: + """Restore the central gate without reintroducing forbidden CodeQL actions.""" + workflow_path = ".github/workflows/codeql-pr.yml" + workflow = (REPOSITORY_ROOT / workflow_path).read_text(encoding="utf-8") + + assert workflow_path in audit.REQUIRED_WORKFLOW_PATHS + assert "uses: github/codeql-action" not in workflow + assert "event_type:\"codeql-scan\"" in workflow diff --git a/tests/test_codeql_scan_dispatch_workflow_contract.py b/tests/test_codeql_scan_dispatch_workflow_contract.py index 5db23c2951..1b2c2ed662 100644 --- a/tests/test_codeql_scan_dispatch_workflow_contract.py +++ b/tests/test_codeql_scan_dispatch_workflow_contract.py @@ -1,10 +1,9 @@ """Structure and shell-syntax contract for the new codeql-scan-dispatch.yml handler. ContextualWisdomLab/.github#1772 designs this file as the native -(non-required-workflow) half of the CodeQL dispatch+poll rewrite. It is not -wired up to codeql-pr.yml yet -- that rewrite is a -separate, still-pending follow-up -- so this only guards the handler's own -structure and shell syntax, mirroring the established pattern in +(non-required-workflow) half of the CodeQL dispatch+poll rewrite, and +ContextualWisdomLab/.github#1778 wires the required entrypoint to it. This +guards the handler's structure and shell syntax, mirroring the established pattern in tests/test_opencode_workflow_shell_syntax.py and tests/test_codeql_pr_workflow_contract.py. """ @@ -18,6 +17,7 @@ import sys from pathlib import Path +from scripts.ci import audit_central_required_workflows as ruleset_audit from tests.test_opencode_workflow_shell_syntax import _extract_run_block REPO_ROOT = Path(__file__).resolve().parents[1] @@ -238,7 +238,7 @@ def test_codeql_scan_dispatch_is_not_in_the_required_workflow_ruleset_scope(): admission restriction documented in docs/doctoring/codeql-pr-required-workflow-always-fails.md. """ - audit_path = REPO_ROOT / "docs/org-required-workflow-rollout.md" - if not audit_path.exists(): - return - assert "codeql-scan-dispatch.yml" not in audit_path.read_text(encoding="utf-8") + required_paths = set(ruleset_audit.REQUIRED_WORKFLOW_PATHS) + + assert ".github/workflows/codeql-pr.yml" in required_paths + assert ".github/workflows/codeql-scan-dispatch.yml" not in required_paths diff --git a/tests/test_pr_review_merge_scheduler.py b/tests/test_pr_review_merge_scheduler.py index 7902e3dcb5..a31a0b03bf 100644 --- a/tests/test_pr_review_merge_scheduler.py +++ b/tests/test_pr_review_merge_scheduler.py @@ -4916,7 +4916,7 @@ def fake_read(args): "owner/repo", make_pr(headRefOid=head_sha), dry_run=False ) - assert recovered == [90, 91] + assert recovered == [90, 91, 92] assert calls == [ ( "owner/repo", @@ -4963,6 +4963,47 @@ def test_recover_current_head_startup_failures_does_not_restamp_twice(monkeypatc ) == [] +@pytest.mark.parametrize( + "workflow_metadata", + ( + {"workflow_id": 12, "name": "CodeQL PR", "path": ".github/workflows/codeql-pr.yml"}, + {"workflow_id": 12, "name": "Renamed CodeQL", "path": ".github/workflows/codeql-pr.yml"}, + {"workflow_id": 12, "name": "CodeQL PR"}, + ), +) +def test_recover_current_head_startup_failures_restamps_codeql_alone( + monkeypatch, workflow_metadata +): + head_sha = "a" * 40 + restamps = [] + run = { + "id": 92, + "event": "pull_request", + "head_sha": head_sha, + "status": "completed", + "conclusion": "startup_failure", + "created_at": "2026-09-04T01:02:00Z", + **workflow_metadata, + } + monkeypatch.setattr( + sched, + "run_github_read", + lambda _args: json.dumps({"workflow_runs": [run]}), + ) + monkeypatch.setattr( + sched, + "restamp_pr_head_after_startup_failure", + lambda repo, pr, **kwargs: restamps.append((repo, pr["headRefOid"], kwargs)), + ) + + recovered = sched.recover_current_head_startup_failures( + "owner/repo", make_pr(headRefOid=head_sha), dry_run=False + ) + + assert recovered == [92] + assert restamps == [("owner/repo", head_sha, {"dry_run": False})] + + def test_inspect_pr_recovers_startup_failure_before_other_actions(monkeypatch): monkeypatch.setenv("GITHUB_ACTIONS", "true") monkeypatch.setattr(