fix(strix): enforce orchestrator/free and remove heuristic gate decisions - #1668
fix(strix): enforce orchestrator/free and remove heuristic gate decisions#1668seonghobae wants to merge 18 commits into
Conversation
|
Warning Review limit reachedNext included review available in 54 minutes. View limit detailsLimit details: You’ve used the included review currently available. You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Review configuration: ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Team Run ID: 📒 Files selected for processing (15)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Preserve protected main #1665's GitHub-Actions sidecar pool pin while carrying the test-first reusable Strix model-normalization boundary, ADR/doctoring, and one-shot canonical evidence reconciliation forward. No force push or history rewrite.
seonghobae
changed the title
Preserve protected-main Noema diagnostics and org-sweep rate-limit work while carrying the Strix orchestrator/free, retry-allocation, and severity-admission repair forward. The intervening main files do not overlap this PR's semantic delta. No history rewrite or gate weakening.
seonghobae
added
documentation
ChatGPT Codex Connector
# Conflicts: # CHANGELOG.md # docs/product-technical-gap-baseline.md
main had moved substantially since this branch's RED-before-repair
commits and its one-shot source_fix_strix_no_heuristic_retry_severity.py
driver were authored: run_current_target_scan had grown a full
cross-model fallback loop (FALLBACK_MODELS_RAW, per-candidate retry,
PR_FINDINGS_DECISION-gated blocking, a severity_rank/
STRIX_MAX_SEVERITY_RANK threshold check) the driver's simple_scan
template didn't anticipate matching text for, so running it failed
loudly rather than corrupting anything.
Reconciled by hand instead of patching the driver's brittle string
matches: confirmed the fallback-model system was already present at
this branch's own base commit (not a later regression -- gutting it
entirely was always the intended scope), then applied the equivalent
repair directly -- strix_quick_gate.sh's run_current_target_scan now
issues one governed request via run_strix_once and fails closed on
anything else, with no repository-authored retry, model fallback, or
severity-threshold admission; strix.yml drops the outer 3-attempt
backoff loop and the STRIX_LLM_MAX_RETRIES/STRIX_TRANSIENT_RETRY_*/
STRIX_FAIL_ON_MIN_SEVERITY env vars; test_required_workflow_queue_
contract.py's stale assertion is fixed to match.
Fixed the resulting ripple in six tests across four files that
exercised the removed is_transient_same_model_retry_error/
github_models_rate_limit_should_skip_same_model_retry/
run_strix_with_transient_retry orchestration directly: three
(Caido bootstrap timing, ModelBehaviorError, NVIDIA NIM 404) were
narrowed to assert only that each signal classifier still feeds
has_detected_infrastructure_error, since same-model retry and
cross-model fallback no longer exist to wire into; two
multi-attempt-specific assertions in the exempted-finding regression
were removed as testing behavior that no longer exists, keeping the
still-valid single-attempt/never-retries coverage; one now-stale
comment reference to a removed function was corrected.
Removed source_fix_strix_no_heuristic_retry_severity.py and its
companion workflow per the driver's own documented one-shot lifecycle
("deletes it after RED-before-repair and focused GREEN verification
succeed") now that the repair it existed to apply is complete.
Recorded the reconciliation in this doctoring entry's addendum.
Full local triad: 2815 passed (22 pre-existing failures unrelated to
this PR -- the same main-side admission-controller-burst/cron-cadence
gap tracked and fixed in .github#1871, unaffected by this diff);
coverage and docstring gaps match that same pre-existing baseline
exactly, with zero new gaps from this change.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Completed the no-heuristics retry/severity repair (commit
|
|
Superseded by #1880. This draft correctly identified repository-side full-scan retries as queue amplification, but it also changed the shared model normalizer to reject every provider-specific model. The full |
…uest routing The bash self-test still described the pre-migration architecture and reported 949 failures on this branch's head. Drive it to zero without letting the rewrite bless behaviour the migration did not intend to remove. Retargeted, not deleted, wherever the subject survives: - 119 invocations moved to the gateway contract (orchestrator/free, the pinned loopback base, and the single-attempt model/api-base sequences). - 16 provider-routing scenarios now assert the policy boundary itself: a direct provider route exits 2 with "direct provider/model routes are forbidden". These cover the new boundary, which nothing else tested. - preserve-existing-api-base and the untrusted-api-base case had their expected outcome inverted, because the gate got stricter: it now pins the loopback rather than keeping a caller-supplied base, and refuses an api-base file outside the trusted root rather than ignoring it. - Six new fail-closed scenarios replace the deleted fallback cluster's signal coverage (rate limit, resource-exhausted, quota, token limit, timeout), plus a negative control so the provider-unavailable and generic-failure branches cannot silently collapse into one. Deleted only where the mechanism is gone: 86 multi-attempt scenarios, 11 severity-threshold and fallback-config scenarios, the GitHub Models HTTP 410 cluster (unreachable behind is_github_models_api_compatible_model), and five assert_model_requires_vertex_auth calls whose helper normalises internally and can no longer return 0. Also restores three pull-request-scope calls in run_current_target_scan. 15027c1 rewrote that function by hand right after a merge from main introduced evaluate_pull_request_findings, and dropped all of its call sites along with fail_unmapped_threshold_report and should_fail_pull_request_infra_zero_findings. None of those belong to the "repository-authored retry, model fallback, or severity-threshold admission" this change set out to remove; without them the gate fails a pull request on findings confined to files it never modified, and a zero-findings result produced while the provider was failing reads as a clean scan. load_pull_request_head_sha is left alone: it is orphaned on main too. Verified against an unmodified clone of this same branch head: the self-test goes 949 -> 0, and the rest of the suite is unchanged at 22 failed / 2815 passed, coverage 99%, interrogate 98.3%. Those pre-existing failures are branch staleness (34 commits behind main, missing #1883's coverage and docstring repair) and are not touched here. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
자가 테스트를 새 아키텍처에 맞춰 재작성했습니다. 949 → 0, 커밋 무엇을 지웠고 무엇을 재조준했는지, 그리고 테스트가 아닌 프로덕션 코드를 한 곳 건드린 사실을 함께 남깁니다. 재조준 (주제가 살아 있는 것)
경계 거부 16건은 삭제하지 않고 남긴 이유가 있습니다. 기대 결과가 뒤집힌 2건은 게이트가 더 엄격해진 경우입니다.
신설 6건은 삭제한 fallback 클러스터가 덮던 신호 분류(rate limit / resource-exhausted / quota / 413 / timeout)를 단일 요청 계약으로 되살린 것입니다. 음성 대조군을 하나 같이 넣었습니다 — 양성만 있으면 게이트가 모든 실패에 삭제 (기전 자체가 사라진 것)
삭제 판단은 프로덕션 파일의 고아 함수 전수 조사로 했습니다(주석 언급은 참조에서 제외 — 이걸 안 빼면 프로덕션 코드 수정 — 검토 부탁드립니다
이력: 이 브랜치는 되살린 이유: 커밋 메시지가 명시한 제거 대상은 "repository-authored retry, model fallback, or severity-threshold admission" 셋인데, PR 스코프 판정은 그중 어디에도 속하지 않습니다. 이게 없으면 PR이 건드리지도 않은 파일의 finding으로 PR이 실패하고( 되돌리는 법:
검증손대지 않은 같은 브랜치 head 클론을 별도로 만들어 기준선을 재고 비교했습니다. 제 변경이 새로 만든 실패는 0건이고 두 백분율 모두 그대로입니다. 남은 22건과 99%/98.3%는 브랜치가 main보다 34커밋 뒤처져 생긴 staleness입니다 — 99%/98.3%는 오늘 관찰 (수정하지 않음)
|
main already carries docs/adr/0021-hourly-review-repair-single-file-consolidation.md, which arrived on this branch through a merge from main. Two ADRs cannot share a number, and 0021..0027 are all taken on main, so this one becomes 0028. Only the ADR's own file and title move. The other ADR-0021 mentions on this branch (ARCHITECTURE.md, CHANGELOG.md, docs/doctoring/scheduler-target-list-drift-20260902.md) all refer to the hourly-review-repair consolidation and stay as they are. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
|
재오픈합니다. "Superseded by #1880" 폐쇄는 승계 검증을 통과하지 못합니다. 근거를 붙입니다. #1880은 이 PR의 15개 파일 중 2개만 들고 갔습니다#1880이 승계한 것은 strix.yml의 바깥 재시도 루프 제거뿐입니다. 이 PR의 핵심 — 다른 경로로도 들어오지 않았습니다폐쇄 시점부터 지금까지 이 저장소의 AGENTS.md "Supersession and constant-change review"가 정확히 이 형태를 다룹니다 — narrowing a PR isn't verified until you diff the union of successors against the original's full scope. 여기서 union은 2/15입니다. 브랜치에 지금 있는 것ADR 재번호가 필요했던 이유: 병합 전 필요한 것
재오픈은 폐쇄를 되돌리는 것이지 병합 판단이 아닙니다. 리뷰 파이프라인이 지금 멈춰 있어서(#1927 · #1929 · #1931) 체크는 당분간 안 붙습니다. |
One conflict in tests/test_strix_backend_unavailable_after_exempted_finding.py: both sides added a workflow-contract method at the same spot. main's (#1880) is a superset of this branch's -- it also asserts the retry knobs are absent and shares the no-wall-clock-deadline tail -- so main's method is kept and the branch's duplicate header dropped. No production code touched by hand. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
|
Merged The branch was 38 commits behind. One conflict, in Verification on Note for whoever reads the checks: the scheduler guard #1937 is on |
|
Marked ready for review. This PR was a draft, and the pipeline never issues a formal APPROVE on a draft ( |
|
Reverted to draft. The PR body carries an explicit hold that I missed before flipping it: "Keep Draft until all one-shot workflows have self-removed and fresh exact-head tests/security/review gates are terminal. Pending/queued evidence is non-passing." An owner-stated hold stands regardless of whether its condition is currently reachable, so the earlier ready-for-review flip was mine to undo ( One finding is left here as an owner decision, not acted on: the second half of that condition cannot be met while the PR is a draft — |
|
Checked the first half of the draft hold on the current head |
1 participant
Root cause
Protected central Strix provisions
ContextualWisdomLab/contextual-orchestrator, requests exactlyorchestrator/free, forces private-target ZDR, supplies all five bootstrap credential variables, and disables external fallback models. Three reusable Strix surfaces nevertheless remained authoritative below that boundary: (1) generic model normalization still accepted concrete provider/model identifiers; (2) the required workflow allocated repository-authored same-model retries/backoff, including an outer retry loop, even though contextual-orchestrator owns provider failover; and (3) the gate converted Strix severity labels to a hand-selectedMEDIUMmerge threshold.None of those routing/test-time-compute/admission rules has an identified statistical/psychometric model, authoritative standard, or executable experimental calibration. Under the no-heuristics contract, the safe replacement is not a different count/weight/cutoff: reject direct-provider routes, delegate provider failover to the gateway, and fail closed when one governed execution does not produce authoritative evidence or when a current vulnerability artifact exists.
This is separate from the corrected credential boundary:
OPENAI_API_KEYmay remain supplied, registered, and globally discoverable. OpenAI-derived models must be excluded at contextual-orchestrator'sorchestrator/freecandidate boundary rather than by deleting the bootstrap secret.Test-first repair
2083a72dccaa1d96ea423a51af537240fde8a210requires model normalization to admit onlyorchestrator/free/contextual-orchestrator/orchestrator/freeand reject direct OpenAI, OpenRouter, NVIDIA NIM, GitHub Models, Vertex/Gemini, and unqualified model identifiers.10c1ddf822f1e6336b73a9093a56680fea8f4f54moves that reusable model boundary to fail closed before provider credentials or endpoints can become Strix execution authority.618bf6516c792004d84074c00844793ee48c59a1forbids repository-authored Strix retry/test-time-compute allocation in both the central workflow and reusable gate.9dd508276ca5803cc71a261fb151921863dd59ebforbids a hand-selected severity admission threshold and requires a successful Strix execution with any current vulnerability artifact to fail closed.e31d7d21b93b4838b8c0f8e9bb1c2ecac04518f0carries an exact-text guarded repair workflow that first proves those two new contracts RED, removes same-model retry/backoff/outer retry allocation, makes the reusable gate executeorchestrator/freeonce, removes the workflowMEDIUMcutoff, fails closed on any current vulnerability artifact, verifies focused contracts, records causal evidence, and self-removes.ADR-0028 (renumbered from 0021 on 2026-09-05;
mainalready had a different ADR-0028) and the doctoring record preserve the corrected OpenAI credential/candidate separation and ZDR boundary. Canonical product-gap/changelog reconciliation remains one-shot and exact-head gated.Merge discipline
Keep Draft until all one-shot workflows have self-removed and fresh exact-head tests/security/review gates are terminal. Pending/queued evidence is non-passing. Do not bypass, self-approve, force-push, destructively rebase, or weaken required checks. Historical direct-provider/fallback/severity helper code that becomes unreachable after the active-path repair remains cleanup debt until removed with exact-head regression evidence.