diff --git a/.github/workflows/agent-mention-router-quality-ci.yml b/.github/workflows/agent-mention-router-quality-ci.yml index 9c36a89119..4e943d74f2 100644 --- a/.github/workflows/agent-mention-router-quality-ci.yml +++ b/.github/workflows/agent-mention-router-quality-ci.yml @@ -14,6 +14,7 @@ on: - "tests/test_agent_mention_*.py" - "tests/test_pr_review_fix_scheduler_coverage.py" - "requirements-opencode-review-ci-hashes.txt" + - "requirements-noema-document-ci-hashes.txt" push: branches: [main] paths: @@ -27,6 +28,7 @@ on: - "tests/test_agent_mention_*.py" - "tests/test_pr_review_fix_scheduler_coverage.py" - "requirements-opencode-review-ci-hashes.txt" + - "requirements-noema-document-ci-hashes.txt" concurrency: group: agent-mention-router-quality-${{ github.repository }}-${{ github.event.pull_request.number || github.ref }} @@ -85,11 +87,14 @@ jobs: with: python-version: "3.14" cache: pip - cache-dependency-path: requirements-opencode-review-ci-hashes.txt + cache-dependency-path: | + requirements-opencode-review-ci-hashes.txt + requirements-noema-document-ci-hashes.txt - name: Install exact hash-locked tooling run: >- python -m pip install --disable-pip-version-check --require-hashes -r requirements-opencode-review-ci-hashes.txt + -r requirements-noema-document-ci-hashes.txt - name: Run complete repository suite and bounded branch coverage shell: bash --noprofile --norc -e -o pipefail {0} run: | diff --git a/.jules/bolt.md b/.jules/bolt.md index 4f20b36047..088c43d3f8 100644 --- a/.jules/bolt.md +++ b/.jules/bolt.md @@ -54,3 +54,7 @@ ## 2026-09-01 - 대용량 문자열 서브스트링 스캐닝 루프 최적화 **Learning:** 긴 텍스트에서 여러 기준 문자열(`candidate`)을 탐색하여 다음 구역의 시작점을 찾을 때, 텍스트 전체에 대해 반복적으로 `text.find(candidate)`를 호출하면 O(N)의 비효율적인 중복 스캐닝 오버헤드가 발생합니다. 특히 가장 가까운 시작점을 찾기 위해 모든 후보를 스캔할 때 이 문제가 심화됩니다. **Action:** 기준점(`start`)을 잡은 후, `idx = text.find(candidate, start, end)`를 사용하여 검색 범위를 동적으로 축소(`end = min(end, idx)`)하십시오. 이렇게 하면 불필요한 스캐닝 오버헤드를 막고 검색 범위를 안전하게 줄여 매우 큰 성능 향상을 얻을 수 있습니다. + +## 2026-09-17 - ThreadPoolExecutor shutdown latency +**Learning:** Using `executor.shutdown(wait=False)` to speed up generator cleanup (e.g. `list_recent_pull_requests`) inside a Python script only hides the blocking until process exit. CPython will still forcefully join all non-daemon worker threads when terminating, so the overall job wall-clock time is not reduced, and the GitHub API requests continue running unconstrained in the background until completion or network timeout. +**Action:** Do not use `wait=False` micro-optimizations to abandon running I/O workers. Instead, rely on cooperative cancellation (e.g., passing a `threading.Event` to interrupt network waits) so workers cleanly abort, allowing both the generator and the interpreter to exit quickly. diff --git a/CHANGELOG.md b/CHANGELOG.md index 34281625cb..b8c5d19aa2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -96,6 +96,12 @@ - Raised `hourly-review-repair.yml`'s discovery ceiling from 50 to 200 while rotating deterministic 50-PR deep-inspection windows by hourly run number. The scheduler hydrates only the selected window and stops immediately after its single dispatch, preserving access to newer PRs without quadrupling expensive review/check/comment work. See `docs/doctoring/hourly-review-repair-single-file-consolidation.md`'s 2026-09-03 follow-up. ## [Unreleased] +- **Repair the Agent Mention full-suite dependency closure.** The quality + workflow now triggers on, caches, and installs both the OpenCode and Noema + hash locks, so repository-wide collection can import `defusedxml`. The + stale `ThreadPoolExecutor.shutdown(wait=False)` proposal is removed because + CPython still joins those workers at process exit while the early return + would let GitHub API work continue after generator cleanup. - **Bind GitHub REST redirect evidence to both production opener chains.** `.github#2279` now feeds a synthetic same-authority 302 through the CodeQL identity and Strix evidence clients' real module-level openers, proving the redirect target is never contacted and the bearer header is never forwarded. Removing `_RejectRedirects` from either opener makes the contract fail on the forbidden second request. Four stale Strix HTTP/transport/JSON fixtures now patch that same production seam; direct handler unit cases and standalone CodeQL materialization remain unchanged. - **Define an evidence-backed repository README quality standard.** Added `docs/repository-readme-quality-standard.md` as the shared review contract for product-first structure, code-current onboarding, authority boundaries, durable quality signals, and repository/source/dependency license due diligence. Product repositories continue to own their own README prose; the standard is linked from the root documentation map and does not centralize or generate product claims. - Include merge-scheduler entrypoint, core, and regression-test changes in diff --git a/scripts/ci/agent_mention_router.py b/scripts/ci/agent_mention_router.py index 68e544a6cf..81a7cf510c 100755 --- a/scripts/ci/agent_mention_router.py +++ b/scripts/ci/agent_mention_router.py @@ -10,6 +10,7 @@ import os import re import subprocess +import threading import time from dataclasses import dataclass from typing import Any, Sequence @@ -150,6 +151,7 @@ def request( args: Sequence[str], *, input_payload: dict[str, Any] | None = None, + cancellation_event: threading.Event | None = None, ) -> Any: """Execute one bounded ``gh api`` request and decode optional JSON. @@ -176,6 +178,8 @@ def request( payload = None if input_payload is None else json.dumps(input_payload) attempt = 0 while True: + if cancellation_event is not None and cancellation_event.is_set(): + raise RuntimeError("gh api request cancelled") attempt += 1 try: completed = subprocess.run( @@ -193,6 +197,8 @@ def request( "gh api timed out after " f"{GITHUB_API_TIMEOUT_SECONDS} seconds" ) from exc + if cancellation_event is not None and cancellation_event.is_set(): + raise RuntimeError("gh api request cancelled") return_code = int(getattr(completed, "returncode", 0)) if not return_code: output = completed.stdout.strip() @@ -202,7 +208,11 @@ def request( diagnostic = "no stderr output" retryable = RATE_LIMIT_DIAGNOSTIC_RE.search(diagnostic) is not None if retryable and attempt < GITHUB_API_MAX_ATTEMPTS: - time.sleep(attempt * 5) + backoff_seconds = attempt * 5 + if cancellation_event is None: + time.sleep(backoff_seconds) + elif cancellation_event.wait(backoff_seconds): + raise RuntimeError("gh api request cancelled") continue suffix = f" after {attempt} attempts" if attempt > 1 else "" raise RuntimeError( diff --git a/scripts/ci/agent_mention_sweep.py b/scripts/ci/agent_mention_sweep.py index 5b56fdcf4f..c969d16442 100755 --- a/scripts/ci/agent_mention_sweep.py +++ b/scripts/ci/agent_mention_sweep.py @@ -31,16 +31,12 @@ # log tail and metrics. Stop dispatching new work with margin to spare so # the sweep exits cleanly and reports what it completed. # -# Returning early only stops NEW work: list_recent_pull_requests' generator -# cleanup still blocks (executor.shutdown(wait=True)) until every currently -# RUNNING repository fetch finishes on its own. GitHubClient's rate-limit -# retry costs up to ~255s worst case for one repository (six attempts, each -# up to the 30s subprocess timeout, plus ~75s of backoff between them), and -# up to max_workers of those can be running concurrently at the moment the -# deadline trips (bounded by that ceiling, not multiplied by it, since they -# run in parallel). Budget = 900s job timeout - ~60s setup/checkout -# overhead - ~255s worst-case cleanup wait, with a further margin still -# unspent. +# Returning early sets one cancellation event shared by repository fetches. +# That event interrupts retry backoff immediately; an already-running gh +# subprocess retains its existing 30s timeout. Cleanup then waits for those +# bounded workers before returning, so no worker can keep using the shared +# client after the sweep reports completion. The 480s dispatch budget leaves +# the 30s worker tail plus setup and reporting margin inside the 900s job. DEFAULT_TIME_BUDGET_SECONDS = 480.0 @@ -204,7 +200,8 @@ def fetch(repository: str) -> list[dict[str, Any]]: "per_page=100", "-f", f"page={page}", - ] + ], + cancellation_event=stop_event, ) pull_requests = flatten_pages(response) if not pull_requests: diff --git a/tests/test_agent_mention_sweep.py b/tests/test_agent_mention_sweep.py index 1489873b76..f075098923 100644 --- a/tests/test_agent_mention_sweep.py +++ b/tests/test_agent_mention_sweep.py @@ -23,9 +23,10 @@ def __init__(self, responses=None) -> None: self.responses = responses or {} self.calls = [] - def request(self, args, *, input_payload=None): + def request(self, args, *, input_payload=None, cancellation_event=None): """Return the response registered for the first API argument.""" + del cancellation_event self.calls.append((list(args), input_payload)) return self.responses.get(args[0]) @@ -677,3 +678,56 @@ def test_main_constructs_clients_and_forwards_options(monkeypatch) -> None: assert captured[0]["lookback_hours"] == 48 assert captured[0]["max_dispatches"] == 3 assert captured[0]["dry_run"] is True + +def test_list_recent_pull_requests_shutdown_behavior(monkeypatch) -> None: + """Generator cleanup cancels workers and joins them before returning.""" + + sweep = module() + client = FakeClient() + monkeypatch.setattr( + sweep, "list_accessible_repositories", lambda *args, **kwargs: ["ContextualWisdomLab/repo", "ContextualWisdomLab/repo2"] + ) + + import concurrent.futures + import threading + shutdown_called_with_wait = False + + # We need a latch to ensure the worker starts running before we close. + worker_started = threading.Event() + worker_can_finish = threading.Event() + + def fake_request(args, *, input_payload=None, cancellation_event=None): + del input_payload + endpoint = args[0] + if endpoint.endswith("repo2/pulls"): + worker_started.set() + assert cancellation_event is not None + cancellation_event.wait(timeout=5) + worker_can_finish.set() + return [] + assert worker_started.wait(timeout=2) + return [{"number": 1, "updated_at": "2026-08-05T00:00:00Z"}] + + monkeypatch.setattr(client, "request", fake_request) + + class MockExecutor(concurrent.futures.ThreadPoolExecutor): + def shutdown(self, wait=True, cancel_futures=False): + nonlocal shutdown_called_with_wait + if wait and cancel_futures: + shutdown_called_with_wait = True + super().shutdown(wait=wait, cancel_futures=cancel_futures) + + monkeypatch.setattr(concurrent.futures, "ThreadPoolExecutor", MockExecutor) + + gen = sweep.list_recent_pull_requests( + client, + organization="ContextualWisdomLab", + repository_source="organization", + since="2026-08-05T00:00:00Z", + ) + + assert next(gen)["number"] == 1 + assert worker_started.is_set() + gen.close() + assert shutdown_called_with_wait + assert worker_can_finish.is_set() diff --git a/tests/test_agent_mention_sweep_regressions.py b/tests/test_agent_mention_sweep_regressions.py index 643f562cc0..4992b018d7 100644 --- a/tests/test_agent_mention_sweep_regressions.py +++ b/tests/test_agent_mention_sweep_regressions.py @@ -40,10 +40,10 @@ def __init__(self, responses) -> None: self.responses = responses self.calls: list[list[str]] = [] - def request(self, args, *, input_payload=None): + def request(self, args, *, input_payload=None, cancellation_event=None): """Return one endpoint/page response or raise its configured error.""" - del input_payload + del input_payload, cancellation_event args = list(args) self.calls.append(args) endpoint = args[0] diff --git a/tests/test_agent_mention_timeout_bounds.py b/tests/test_agent_mention_timeout_bounds.py index 7f294cb259..050c1b9786 100644 --- a/tests/test_agent_mention_timeout_bounds.py +++ b/tests/test_agent_mention_timeout_bounds.py @@ -54,10 +54,10 @@ def __init__(self, names: tuple[str, ...]) -> None: self.names = names - def request(self, args, *, input_payload=None): + def request(self, args, *, input_payload=None, cancellation_event=None): """Return the organization inventory or one repository pull list.""" - del input_payload + del input_payload, cancellation_event endpoint = args[0] if endpoint == "orgs/ContextualWisdomLab/repos": return [repository(name) for name in self.names] @@ -128,6 +128,99 @@ def flaky_run(command, **kwargs): assert sleeps == [5, 10] +def test_github_client_cancellation_interrupts_rate_limit_backoff(monkeypatch) -> None: + """Sweep cancellation stops a retrying request before another subprocess.""" + + router = router_module() + attempts = [] + waits = [] + + def rate_limited(command, **kwargs): + """Return one admission-time rate-limit response.""" + del kwargs + attempts.append(command) + return SimpleNamespace( + stdout="", + stderr="gh: API rate limit exceeded for installation ID 1", + returncode=1, + ) + + class CancellationEvent: + """Expose the Event subset required by the request boundary.""" + + cancelled = False + + def is_set(self) -> bool: + """Return whether the synthetic cancellation was observed.""" + return self.cancelled + + def wait(self, timeout: float) -> bool: + """Record the backoff and interrupt it immediately.""" + waits.append(timeout) + self.cancelled = True + return True + + monkeypatch.setattr(router.subprocess, "run", rate_limited) + with pytest.raises(RuntimeError, match="gh api request cancelled"): + router.GitHubClient("token").request( + ["repos/x/y"], + cancellation_event=CancellationEvent(), + ) + + assert len(attempts) == 1 + assert waits == [5] + + +def test_sweep_process_exits_after_cooperative_worker_cancellation() -> None: + """Closing the real sweep generator also bounds interpreter shutdown.""" + + child_program = f''' +import sys +import threading +import time +sys.path.insert(0, {str(SCRIPTS)!r}) +import agent_mention_sweep as sweep + +worker_started = threading.Event() + +class Client: + def request(self, args, *, input_payload=None, cancellation_event=None): + del input_payload + endpoint = args[0] + if endpoint == "orgs/ContextualWisdomLab/repos": + return [ + {{"full_name": "ContextualWisdomLab/fast", "owner": {{"login": "ContextualWisdomLab"}}, "archived": False, "disabled": False}}, + {{"full_name": "ContextualWisdomLab/slow", "owner": {{"login": "ContextualWisdomLab"}}, "archived": False, "disabled": False}}, + ] + if endpoint == "repos/ContextualWisdomLab/slow/pulls": + worker_started.set() + if cancellation_event is None: + time.sleep(5) + else: + cancellation_event.wait(5) + return [] + assert worker_started.wait(1) + return [{{"number": 1, "updated_at": "2026-08-20T00:00:00Z"}}] + +generator = sweep.list_recent_pull_requests( + Client(), + organization="ContextualWisdomLab", + repository_source="organization", + since="2026-08-19T00:00:00Z", +) +assert next(generator)["number"] == 1 +generator.close() +''' + completed = subprocess.run( + [sys.executable, "-c", child_program], + capture_output=True, + text=True, + check=False, + timeout=2, + ) + assert completed.returncode == 0, completed.stderr + + def test_github_client_fails_closed_after_exhausting_retries(monkeypatch) -> None: """A persistent rate limit still fails after all retries, not silently.""" @@ -234,10 +327,10 @@ def test_repository_fanout_yields_fast_repository_before_slow_one() -> None: class FairnessClient: """Block one repository while allowing the next one to complete.""" - def request(self, args, *, input_payload=None): + def request(self, args, *, input_payload=None, cancellation_event=None): """Return the inventory or one deliberately paced pull list.""" - del input_payload + del input_payload, cancellation_event endpoint = args[0] if endpoint == "orgs/ContextualWisdomLab/repos": return [repository("alpha"), repository("bravo")] @@ -310,14 +403,13 @@ def test_generator_close_stops_additional_pages_after_inflight_request( sweep = sweep_module() page_two_started = threading.Event() - release_page_two = threading.Event() shutdown_started = threading.Event() class ClosingClient: """Keep the second repository in one bounded in-flight request.""" - def request(self, args, *, input_payload=None): - """Return page one or pause page two until the closer releases it.""" + def request(self, args, *, input_payload=None, cancellation_event=None): + """Return page one or pause page two until cancellation.""" del input_payload endpoint = args[0] if endpoint == "orgs/ContextualWisdomLab/repos": @@ -332,7 +424,8 @@ def request(self, args, *, input_payload=None): return [pull(number) for number in range(100, 200)] if page == 2: page_two_started.set() - assert release_page_two.wait(2) + assert cancellation_event is not None + assert cancellation_event.wait(2) return [pull(number) for number in range(200, 300)] raise AssertionError(f"unexpected third page request: {args!r}") @@ -374,7 +467,6 @@ def shutdown(self, *, wait, cancel_futures): closer = threading.Thread(target=generator.close) closer.start() assert shutdown_started.wait(2) - release_page_two.set() closer.join(2) assert not closer.is_alive() diff --git a/tests/test_agent_mention_workflow_contract.py b/tests/test_agent_mention_workflow_contract.py index c5fc4cae54..be110c6da3 100644 --- a/tests/test_agent_mention_workflow_contract.py +++ b/tests/test_agent_mention_workflow_contract.py @@ -60,3 +60,20 @@ def test_quality_workflow_measures_exact_files_without_module_name_warnings() -> assert "source =" not in coverage_config assert "scripts/ci/agent_mention_router.py" in coverage_config assert "scripts/ci/agent_mention_sweep.py" in coverage_config + + +def test_quality_workflow_installs_every_full_suite_dependency_lock() -> None: + """The repository-wide suite installs both trusted hashed lock closures.""" + + text = QUALITY_WORKFLOW.read_text(encoding="utf-8") + header = text.split("\njobs:\n", 1)[0] + assert ' - "requirements-opencode-review-ci-hashes.txt"' in header + assert ' - "requirements-noema-document-ci-hashes.txt"' in header + assert "cache-dependency-path: |" in text + assert "requirements-opencode-review-ci-hashes.txt" in text + assert "requirements-noema-document-ci-hashes.txt" in text + install = text.split("- name: Install exact hash-locked tooling", 1)[1].split( + "- name:", 1 + )[0] + assert "-r requirements-opencode-review-ci-hashes.txt" in install + assert "-r requirements-noema-document-ci-hashes.txt" in install