diff --git a/CHANGELOG.md b/CHANGELOG.md index bf192f6a9e..52f7f3c807 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,13 @@ +### Agent-mention sweep regression follows completion order + +- Replace the repository-order assertion with a deterministic completion-order + fixture for `list_recent_pull_requests`. The production iterator already + uses `concurrent.futures.as_completed`; the stale test could pass or fail + with thread scheduling and contradicted the latency boundary that a completed + repository must be yielded without waiting for a slower sibling. The fixture + blocks the first repository until the caller has received the second, while + retaining the two-worker ceiling. + ### Failed-check finding names the Strix sandbox instead of the gateway - `opencode-review-dispatch.yml`'s `emit_strix_provider_failure_finding` rendered one fixed finding for every `STRIX_PROVIDER_UNAVAILABLE` line, whose Root cause read "The contextual-orchestrator gateway or its discovered provider pool was unavailable for this run". `#1953` had just given the Strix sandbox bootstrap failure its own second verdict token (`STRIX_SANDBOX_UNAVAILABLE`) precisely because that attribution is wrong for it -- the sandbox container never reaches its Caido proxy, so the run dies before the gateway serves anything -- and this consumer re-applied the wrong attribution one step downstream, into the review findings and the failure census. The emitter now branches on the second token: a sandbox verdict gets a finding that names Strix's sandbox, says the verdict does not name the gateway, and tells the reader not to change gateway or provider configuration on its strength. A `STRIX_PROVIDER_UNAVAILABLE` line without the token keeps its existing text verbatim, so the gateway class has no regression surface. No test covered this finding text at all before (`gateway or its discovered provider pool` matched nothing under `tests/`); `tests/test_opencode_dispatch_strix_sandbox_finding.py` now runs the production emitter from the published run block and pins both directions plus the no-signal case. Refs #1953, #1935. diff --git a/docs/doctoring/agent-mention-sweep-completion-order.md b/docs/doctoring/agent-mention-sweep-completion-order.md new file mode 100644 index 0000000000..62585fe2ec --- /dev/null +++ b/docs/doctoring/agent-mention-sweep-completion-order.md @@ -0,0 +1,46 @@ +# Agent-mention sweep completion-order regression + +Decision date: **2026-09-08** + +## Problem + +`list_recent_pull_requests` submits bounded repository fetches concurrently and +iterates them with `concurrent.futures.as_completed`. Its regression instead +claimed that results remain in repository order and asserted +`first, second` without controlling completion. The assertion therefore tested +thread timing rather than the production contract. + +A slow first repository is an operational failure scene: if the already-finished +second repository is hidden behind it, the sweep wastes its bounded dispatch +window and may omit actionable mentions before the next rotation. + +## Decision + +Keep the production implementation unchanged. Replace only the stale regression +with a deterministic fixture. The first repository waits on an event; the caller +must receive the second repository before releasing that event. The test retains +the exact two-worker assertion. + +This is the minimum owner repair. It adds no scheduler abstraction, timeout, +retry, ordering buffer, or dependency. + +## Evidence + +- RED `570da463b0ce7f4837727a91557187624a7b37db` makes the completion order + deterministic while retaining the obsolete `first, second` expectation. + The production iterator necessarily yields `second` first. +- GREEN replaces the expectation with `second, first` and names the latency + contract directly. +- Exact-head hosted Checks remain the admission authority. + +## Risks and rollback + +The event wait has a 30-second failure ceiling so a broken executor produces an +actionable failure instead of hanging the suite. Rollback is removal of this +test-only successor; production behavior is unchanged. + +## References + +Python Software Foundation. (2026). *concurrent.futures — Launching parallel +tasks*. Python 3 documentation. +https://docs.python.org/3/library/concurrent.futures.html#concurrent.futures.as_completed diff --git a/docs/product-technical-gap-baseline.md b/docs/product-technical-gap-baseline.md index 1cc9e20313..94d2759685 100644 --- a/docs/product-technical-gap-baseline.md +++ b/docs/product-technical-gap-baseline.md @@ -1,5 +1,20 @@ # Product and Technical Gap Baseline +## Exact-head amendment — 2026-09-08 + +- **Gap:** the agent-mention sweep production iterator yields repository results + with `concurrent.futures.as_completed`, but its current-main regression + claimed and asserted repository order. That scheduler-dependent assertion + could conceal the intended latency boundary or fail nondeterministically. +- **Action/status:** Proposed on + `fix/agent-sweep-completion-order-regression`. A deterministic two-repository + fixture holds the first request until the already-completed second result is + observed, then asserts second-before-first and preserves the two-worker + ceiling. Protected-main integration and exact-head Checks remain required. +- **Boundary:** no production scheduling, retry, credential, merge, or scanner + policy changes. + + 작성 기준일: **2026-08-26 10:35 KST** 대상: **ContextualWisdomLab/.github** 중앙 거버넌스·자동화 레포지터리와 이를 소비하는 naruon 생태계 현재 보호된 `main`: `826b92394c63deb6981c3a8d16a724d71f85a0d7` diff --git a/tests/test_agent_mention_sweep_regressions.py b/tests/test_agent_mention_sweep_regressions.py index 643f562cc0..217bfcebc0 100644 --- a/tests/test_agent_mention_sweep_regressions.py +++ b/tests/test_agent_mention_sweep_regressions.py @@ -4,6 +4,7 @@ import importlib import sys +import threading from datetime import datetime, timezone from pathlib import Path @@ -94,11 +95,25 @@ def test_pull_pagination_stops_at_cutoff_without_loading_later_pages() -> None: assert sweep.flatten_pages([{"number": 1}]) == [{"number": 1}] -def test_recent_pull_requests_use_bounded_parallel_repository_fetches(monkeypatch) -> None: - """Repository fetches are parallel but results remain repository ordered.""" +def test_recent_pull_requests_emit_bounded_parallel_fetches_as_they_finish( + monkeypatch, +) -> None: + """A slow repository cannot hide a completed sibling repository result.""" sweep = module() - client = PagingClient( + second_observed = threading.Event() + + class CompletionOrderClient(PagingClient): + """Hold the first repository until its completed sibling is yielded.""" + + def request(self, args, *, input_payload=None): + """Make repository completion order deterministic for the assertion.""" + + if args[0] == "repos/ContextualWisdomLab/first/pulls": + assert second_observed.wait(timeout=30) + return super().request(args, input_payload=input_payload) + + client = CompletionOrderClient( { ("orgs/ContextualWisdomLab/repos", 1): [[ repository("first"), @@ -120,17 +135,18 @@ def recording_executor(*, max_workers): "ThreadPoolExecutor", recording_executor, ) - results = list( - sweep.list_recent_pull_requests( - client, - organization="ContextualWisdomLab", - repository_source="organization", - since="2026-08-05T00:00:00Z", - ) + issues = sweep.list_recent_pull_requests( + client, + organization="ContextualWisdomLab", + repository_source="organization", + since="2026-08-05T00:00:00Z", ) + first_result = next(issues) + second_observed.set() + results = [first_result, *issues] assert [result["repository"] for result in results] == [ - "ContextualWisdomLab/first", "ContextualWisdomLab/second", + "ContextualWisdomLab/first", ] assert worker_limits == [2]