-
Notifications
You must be signed in to change notification settings - Fork 0
fix(ci): restore evidence-only review admission #1629
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
5c7b7ea
f7e03f2
4235ecb
93bdbf7
cb3956f
28da4a2
1d52c2a
f756684
6594cb4
ee898e4
49d880d
80d1000
6a0beea
2e57d01
56335de
aaae0cf
e4b08b5
4b12197
38cf798
27c1a37
7d697c6
a43af42
5175935
b9b252d
27a9f0f
6b7477d
3bb1985
7759a86
8d746dd
b561d1e
6781c49
17fa7c3
7d2563c
ec38d6e
50206d8
beead82
56cf1db
0f6e1ee
00602b8
ac0ac05
700d0cd
4041a87
aee16fd
e0af6fc
ae79230
2bb0a0a
998f06d
14f400b
6670953
7afe1cd
add185e
cd65b54
111ce3a
6fc197f
baa212c
f80b0c6
92e8bd0
cf30d06
b7d196c
6df5132
b5d4424
195c5b8
bf846f4
cec8538
1962dfe
0e526cd
34f5b48
7561e1a
449117b
d4ebe48
18fdd72
6a80203
87eab9e
787a24b
ce8bc95
a436bd4
ba9500d
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,3 @@ | ||
| trigger=2026-09-02T08:00:00Z | ||
| contract=fail-closed-provider-default-preflight | ||
| expected-head=ce8bc953141da7250c5bb7e44ea6ed5cfaf2929b | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win
🛡️ 제안: expected-head 검증 단계 - name: Verify trigger pins the checked-out head
shell: bash
run: |
set -euo pipefail
expected="$(sed -n 's/^expected-head=//p' .github/source-fix-1629-no-heuristic-compute.trigger)"
actual="$(git rev-parse HEAD)"
if [ "$expected" != "$actual" ]; then
echo "::error::trigger expected-head ${expected} does not match ${actual}"
exit 1
fi🤖 Prompt for AI Agents |
||
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,91 @@ | ||||||||||||||||||||||||
| name: Source fix PR1629 no-heuristic compute | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| on: | ||||||||||||||||||||||||
| push: | ||||||||||||||||||||||||
| branches: | ||||||||||||||||||||||||
| - fix/no-heuristic-review-admission-current-main | ||||||||||||||||||||||||
| paths: | ||||||||||||||||||||||||
| - .github/source-fix-1629-no-heuristic-compute.trigger | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| jobs: | ||||||||||||||||||||||||
| repair: | ||||||||||||||||||||||||
| permissions: | ||||||||||||||||||||||||
| contents: write | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| runs-on: ubuntu-24.04 | ||||||||||||||||||||||||
| steps: | ||||||||||||||||||||||||
| - uses: actions/checkout@9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0 | ||||||||||||||||||||||||
| with: | ||||||||||||||||||||||||
| fetch-depth: 0 | ||||||||||||||||||||||||
| ref: fix/no-heuristic-review-admission-current-main | ||||||||||||||||||||||||
| persist-credentials: true | ||||||||||||||||||||||||
| - uses: astral-sh/setup-uv@20cfd1bf945f4377ade1205e4dbc17946fc9a30d | ||||||||||||||||||||||||
| with: | ||||||||||||||||||||||||
| version: '0.12.5' | ||||||||||||||||||||||||
| - name: Validate repair driver syntax | ||||||||||||||||||||||||
| run: python -m py_compile scripts/source_fix_1629_no_heuristic_compute_v2.py | ||||||||||||||||||||||||
| - name: Prove compute-allocation contract is RED before repair | ||||||||||||||||||||||||
| shell: bash | ||||||||||||||||||||||||
| run: | | ||||||||||||||||||||||||
| set -euo pipefail | ||||||||||||||||||||||||
| if uv run --group dev python -m pytest -q tests/test_contextual_orchestrator_review_no_heuristic_compute.py; then | ||||||||||||||||||||||||
| echo '::error::no-heuristic compute regression was not RED before production repair' | ||||||||||||||||||||||||
| exit 1 | ||||||||||||||||||||||||
| fi | ||||||||||||||||||||||||
|
Comment on lines
+30
to
+33
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win RED 게이트가 수집 오류를 계약 실패로 오인합니다. 이 조건은 pytest의 모든 비-0 종료 코드를 RED로 처리합니다. pytest는 수집/임포트 오류에 2, 사용법 오류에 4를 반환합니다. 테스트 파일 이름이 바뀌거나 임포트가 깨지면 게이트가 통과하고, 프로덕션 수리가 잘못된 근거로 진행됩니다. 실제 테스트 실패인 종료 코드 1만 허용하십시오. 🐛 제안 수정- if uv run --group dev python -m pytest -q tests/test_contextual_orchestrator_review_no_heuristic_compute.py; then
- echo '::error::no-heuristic compute regression was not RED before production repair'
- exit 1
- fi
+ status=0
+ uv run --group dev python -m pytest -q \
+ tests/test_contextual_orchestrator_review_no_heuristic_compute.py || status=$?
+ if [ "$status" -ne 1 ]; then
+ echo "::error::expected pytest exit code 1 (RED contract), got ${status}"
+ exit 1
+ fi📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||||||||||||||||||||
| - name: Apply causal shared-owner repair | ||||||||||||||||||||||||
| run: python scripts/source_fix_1629_no_heuristic_compute_v2.py | ||||||||||||||||||||||||
| - name: Verify repaired central contracts | ||||||||||||||||||||||||
| shell: bash | ||||||||||||||||||||||||
| run: | | ||||||||||||||||||||||||
| set -euo pipefail | ||||||||||||||||||||||||
| uv run --group dev python -m pytest -q \ | ||||||||||||||||||||||||
| tests/test_contextual_orchestrator_review_no_heuristic_compute.py \ | ||||||||||||||||||||||||
| tests/test_contextual_orchestrator_review_runtime_preflight.py \ | ||||||||||||||||||||||||
| tests/test_contextual_orchestrator_review_preflight_concurrency.py \ | ||||||||||||||||||||||||
| tests/test_contextual_orchestrator_review_policy.py \ | ||||||||||||||||||||||||
| tests/test_contextual_orchestrator_review_sidecar_contract.py | ||||||||||||||||||||||||
| uv run --group dev ruff check \ | ||||||||||||||||||||||||
| scripts/ci/contextual_orchestrator_review_launcher.py \ | ||||||||||||||||||||||||
| tests/test_contextual_orchestrator_review_no_heuristic_compute.py \ | ||||||||||||||||||||||||
| tests/test_contextual_orchestrator_review_runtime_preflight.py | ||||||||||||||||||||||||
| bash -n scripts/ci/contextual_orchestrator_review_sidecar.sh | ||||||||||||||||||||||||
| git diff --check | ||||||||||||||||||||||||
| - name: Commit repair, reconcile current main, self-remove, and push | ||||||||||||||||||||||||
| shell: bash | ||||||||||||||||||||||||
| run: | | ||||||||||||||||||||||||
| set -euo pipefail | ||||||||||||||||||||||||
| rm -f \ | ||||||||||||||||||||||||
| .github/workflows/source-fix-1629-no-heuristic-compute.yml \ | ||||||||||||||||||||||||
| .github/source-fix-1629-no-heuristic-compute.trigger \ | ||||||||||||||||||||||||
| scripts/source_fix_1629_no_heuristic_compute.py \ | ||||||||||||||||||||||||
| scripts/source_fix_1629_no_heuristic_compute_v2.py | ||||||||||||||||||||||||
| git add -A | ||||||||||||||||||||||||
| git diff --cached --check | ||||||||||||||||||||||||
| if git diff --cached --quiet; then | ||||||||||||||||||||||||
| echo '::error::repair produced no tracked change' | ||||||||||||||||||||||||
| exit 1 | ||||||||||||||||||||||||
| fi | ||||||||||||||||||||||||
| git config user.name 'opencode-agent[bot]' | ||||||||||||||||||||||||
| git config user.email '219766164+opencode-agent[bot]@users.noreply.github.com' | ||||||||||||||||||||||||
| git commit -m 'fix(review): remove heuristic inference allocation' | ||||||||||||||||||||||||
| git fetch --no-tags origin main fix/no-heuristic-review-admission-current-main | ||||||||||||||||||||||||
| remote_head="$(git rev-parse origin/fix/no-heuristic-review-admission-current-main)" | ||||||||||||||||||||||||
| if [ "$remote_head" != "$(git rev-parse HEAD)" ] && ! git merge-base --is-ancestor "$remote_head" HEAD; then | ||||||||||||||||||||||||
| git merge --no-edit "$remote_head" | ||||||||||||||||||||||||
| fi | ||||||||||||||||||||||||
| main_head="$(git rev-parse origin/main)" | ||||||||||||||||||||||||
| if ! git merge-base --is-ancestor "$main_head" HEAD; then | ||||||||||||||||||||||||
| git merge --no-edit "$main_head" | ||||||||||||||||||||||||
| fi | ||||||||||||||||||||||||
| uv run --group dev python -m pytest -q \ | ||||||||||||||||||||||||
| tests/test_contextual_orchestrator_review_no_heuristic_compute.py \ | ||||||||||||||||||||||||
| tests/test_contextual_orchestrator_review_runtime_preflight.py \ | ||||||||||||||||||||||||
| tests/test_contextual_orchestrator_review_preflight_concurrency.py \ | ||||||||||||||||||||||||
| tests/test_contextual_orchestrator_review_policy.py \ | ||||||||||||||||||||||||
| tests/test_contextual_orchestrator_review_sidecar_contract.py | ||||||||||||||||||||||||
| uv run --group dev ruff check \ | ||||||||||||||||||||||||
| scripts/ci/contextual_orchestrator_review_launcher.py \ | ||||||||||||||||||||||||
| tests/test_contextual_orchestrator_review_no_heuristic_compute.py \ | ||||||||||||||||||||||||
| tests/test_contextual_orchestrator_review_runtime_preflight.py | ||||||||||||||||||||||||
| bash -n scripts/ci/contextual_orchestrator_review_sidecar.sh | ||||||||||||||||||||||||
| git diff --check | ||||||||||||||||||||||||
| git push origin HEAD:fix/no-heuristic-review-admission-current-main | ||||||||||||||||||||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🟡 Repaired head receives no checks The final Prompt for agentsWas this helpful? React with 👍 or 👎 to provide feedback. |
||||||||||||||||||||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,81 +1,86 @@ | ||
| # Doctoring record: evidence-gated path toward `orchestrator/free` for Strix | ||
| # Doctoring record: Strix `orchestrator/free` reconciliation | ||
|
|
||
| - **Date:** 2026-08-30 | ||
| - **Subject:** The 2026-08-30 owner directive asks that Noema, OpenCode, and | ||
| Strix all route review through `contextual-orchestrator`'s `orchestrator/free` | ||
| pool. Noema and OpenCode already do (`docs/adr/0003-contextual-orchestrator-vendored-free-zdr.md`). | ||
| Strix does not, and stays on `orchestrator/auto` today; this record explains | ||
| why the pin was not flipped on the strength of the instruction alone, and | ||
| what new evidence infrastructure exists so a future, properly reviewed change | ||
| can flip it safely. | ||
| - **Date:** 2026-09-01 | ||
| - **Status:** supersedes the 2026-08-30 diversity-gate proposal | ||
| - **Subject:** Noema, OpenCode, and Strix route required review through | ||
| `ContextualWisdomLab/contextual-orchestrator` using `orchestrator/free`. | ||
| - **Decision record:** [`docs/adr/0003-contextual-orchestrator-vendored-free-zdr.md`](../adr/0003-contextual-orchestrator-vendored-free-zdr.md) | ||
| (2026-08-30 addendum) | ||
| - **Related:** [`docs/product-goal-directive.md`](../product-goal-directive.md) §8 | ||
| and its Follow-up findings note; [`docs/doctoring/noema-orchestrator-free-zdr.md`](noema-orchestrator-free-zdr.md) | ||
|
|
||
| ## Why this needed reconciliation, not a direct edit | ||
| ## Superseded proposal | ||
|
|
||
| `docs/product-goal-directive.md` states its own conflict policy: "Where this | ||
| directive and those documents conflict, resolve the conflict and update | ||
| whichever document is wrong — do not silently pick one." Strix's | ||
| `orchestrator/auto` pin is not an oversight; it is an accepted ADR-0003 | ||
| decision backed by a specific, dated finding: on 2026-08-29, the DiskSage | ||
| exact-head scan showed every discovered free route sharing the OpenRouter | ||
| outage domain, so a strict `orchestrator/free` pin for Strix (which has no | ||
| provider fallback) would have gone dark on that one provider's outage. Silently | ||
| flipping the pin today, on the strength of a general instruction that does not | ||
| re-examine that finding, would reintroduce the exact single-point-of-failure | ||
| risk the ADR was written to avoid — for the workflow whose job is the org's | ||
| required *security* review. Silently keeping the old pin, on the other hand, | ||
| would ignore a legitimate cost/consistency goal the owner restated today. | ||
| The earlier version of this record correctly observed an outage-domain | ||
| concentration incident, but it proposed automatically switching Strix between | ||
| `orchestrator/free` and `orchestrator/auto` when a conceptual | ||
| `free_family_diversity >= 2` condition was met. That historical name referred | ||
| to outage-domain families; it was not, and is not, a runtime evidence field. | ||
| The current runtime emits `free_account_diversity`, which counts credential | ||
| accounts and is not a semantic substitute because multiple accounts can share | ||
| one outage domain. The historical cardinality threshold was not derived from a | ||
|
devin-ai-integration[bot] marked this conversation as resolved.
|
||
| reliability model, statistical estimand, authoritative standard, or | ||
| experimentally validated routing policy. It is therefore not a permitted | ||
| decision rule under the organization no-heuristics contract and must not be | ||
| implemented or revived. | ||
|
|
||
| ## What changed | ||
| Current `free_account_diversity` evidence, and separately any explicitly modeled | ||
| provider/outage-domain observation, may remain diagnostic evidence. Diagnostics | ||
| do not acquire routing authority merely because they are deterministic or | ||
| measured. Any future reliability-aware model selection must identify its | ||
| estimand and be independently evaluated rather than turning an account count or | ||
| an outage-domain count into a routing threshold. | ||
|
|
||
| `scripts/ci/contextual_orchestrator_review_policy.py`'s | ||
| `build_zdr_prioritized_catalog` now reports `free_family_diversity`: the count | ||
| of distinct outage-domain provider families (`provider_family`; the primary | ||
| and secondary NVIDIA NIM keys already collapse into one family) among *all* | ||
| discovered free routes, independent of which `--pool` was requested. This is | ||
| new evidence, not a new decision — it is computed from the same discovery | ||
| report the catalog already validates, and it is present whether the caller | ||
| asked for `--pool free` or `--pool auto`. | ||
| ## Current executable contract | ||
|
|
||
| `tests/test_contextual_orchestrator_review_policy.py` gained | ||
| `test_build_catalog_reports_free_family_diversity` (asserts diversity of 4 for | ||
| the existing five-provider fixture) and | ||
| `test_build_catalog_reports_single_family_free_concentration` (a regression | ||
| test reproducing the 2026-08-29 shape: two NVIDIA keys only, which collapse to | ||
| one family, so diversity is 1). Full suite: 1882 passed, 1 skipped; coverage | ||
| of the changed module remains 100% (`coverage run -m pytest tests` + | ||
| `coverage report --include=scripts/ci/contextual_orchestrator_review_policy.py`). | ||
| Protected-main evidence now records the actual Strix policy: | ||
|
|
||
| `.github/workflows/strix.yml` is unchanged in this PR. It still hard-pins | ||
| `CONTEXTUAL_ORCHESTRATOR_POOL: auto` and its `STRIX_MODEL`/`STRIX_LLM` gates | ||
| still reject anything except `orchestrator/auto`. | ||
| - `.github/workflows/strix.yml` accepts the contextual-orchestrator gateway and | ||
| restricts Strix model overrides to `orchestrator/free`; | ||
| - `tests/test_contextual_orchestrator_review_sidecar_contract.py` asserts | ||
| `CONTEXTUAL_ORCHESTRATOR_POOL: free`; | ||
| - `scripts/ci/strix_quick_gate.sh` and the required-workflow smoke contracts no | ||
| longer treat `orchestrator/auto` as an allowed Strix model route; | ||
| - Noema and Required OpenCode use the same `orchestrator/free` product boundary; | ||
| - private/internal review targets require the sidecar's ZDR policy rather than a | ||
| workflow-local model fallback. | ||
|
|
||
| ## What has to happen before Strix can move to `orchestrator/free` | ||
| The five bootstrap credentials may all be supplied to contextual-orchestrator: | ||
| `BYTEZ_API_KEY`, `NVIDIA_NIM_API_KEY`, `NVIDIA_NIM_API_KEY_SUB`, | ||
| `OPENROUTER_API_KEY`, and `OPENAI_API_KEY`. Receiving, registering, or globally | ||
| discovering through `OPENAI_API_KEY` is not a defect. The invariant is the | ||
| `orchestrator/free` candidate-admission boundary: OpenAI-key-derived models are | ||
| not eligible for free-pool candidate generation, ranking, routing, serving, | ||
| failover, fallback, preflight, or durable free-pool persistence. The four | ||
| free-eligible credential sources still require their explicit zero-cost, | ||
| privacy, and capability evidence; a supplied credential does not fabricate an | ||
| eligible model. | ||
|
|
||
| A follow-up PR to `strix.yml` (or to | ||
| `scripts/ci/contextual_orchestrator_review_sidecar.sh`, whichever the | ||
| implementer finds is the correct evidence-read point) should read | ||
| `free_family_diversity` from the sidecar's `policy-report.json` after | ||
| discovery and select `orchestrator/free` only when it is `>= 2` — i.e. the | ||
| discovered free catalog spans at least two independent outage domains, so one | ||
| provider's outage cannot black out Strix's required review — and fall back to | ||
| `orchestrator/auto` otherwise. That PR was deliberately not bundled into this | ||
| one because `strix.yml` is a `pull_request_target` required workflow | ||
| (`docs/pr-review-and-merge-procedure.md`'s trust-boundary note: PRs that edit | ||
| trusted review workflows run the *base branch's* trusted scripts and can fail | ||
| their own checks until the base branch catches up) and its `STRIX_MODEL` | ||
| allowlist is a deliberate hardened gate, not an oversight to route around in | ||
| the same change that adds the evidence it would depend on. | ||
| ## Admission versus routing | ||
|
|
||
| ## Audit trail | ||
| The central review catalog is an admission boundary. It may enforce explicit | ||
| pool, zero-cost/price evidence, credential-source, capability, and ZDR | ||
| predicates, but it must not turn discovery into a provider quota, family quota, | ||
| candidate-count cap, cost/provider/name ordering, synthesized priority, or | ||
| first-come escalation preference. Every evidence-eligible route remains in the | ||
| catalog. Downstream selection requires identified routing evidence; if that | ||
| evidence is unavailable, the runtime fails closed. | ||
|
|
||
| - `docs/adr/0003-contextual-orchestrator-vendored-free-zdr.md` — 2026-08-30 | ||
| addendum recording the decision and rationale. | ||
| - `docs/product-goal-directive.md` §8 and its Follow-up findings note — the | ||
| directive text and prior CodeRabbit reconciliation this addendum extends. | ||
| - `scripts/ci/contextual_orchestrator_review_policy.py`, | ||
| `tests/test_contextual_orchestrator_review_policy.py` — the evidence change | ||
| and its tests. | ||
| Startup readiness follows the same separation: complete admission evidence | ||
| remains durable, while all admitted routes are probed concurrently and reported | ||
| in catalog order. This removes additive provider latency without turning probe | ||
| completion order into routing authority. | ||
|
|
||
| PR #1629 restores that contract on current protected-main lineage by removing | ||
| the reintroduced catalog cardinality/account caps, ranking, priority synthesis, | ||
| launcher route-count caps, and shared escalation quota while preserving the | ||
| free-only central-review pool. | ||
|
|
||
| ## Evidence trail | ||
|
|
||
| - `.github/workflows/strix.yml` — executable Strix pool and override boundary. | ||
| - `tests/test_contextual_orchestrator_review_sidecar_contract.py` — executable | ||
| `orchestrator/free` sidecar contract. | ||
| - `docs/adr/0003-contextual-orchestrator-vendored-free-zdr.md` — current and | ||
| historical pool decisions, including the later amendment superseding Strix | ||
| `orchestrator/auto`. | ||
| - `docs/product-technical-gap-baseline.md` — current implementation/gap ledger. | ||
| - `scripts/ci/contextual_orchestrator_review_policy.py` — admission evidence, | ||
| not a substantive model router. | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🔴 Stale repair mutates newer branch
When the branch advances before a queued repair starts,
expected-headis never checked. The repair pushes its stale mutation onto unreviewed code.Prompt for agents
Was this helpful? React with 👍 or 👎 to provide feedback.