diff --git a/.github/workflows/ocr-compatibility.yml b/.github/workflows/ocr-compatibility.yml index 8e5142d..254ef47 100644 --- a/.github/workflows/ocr-compatibility.yml +++ b/.github/workflows/ocr-compatibility.yml @@ -87,7 +87,9 @@ jobs: - uses: actions/setup-python@5fda3b95a4ea91299a34e894583c3862153e4b97 # v7.0.0 with: python-version: "3.14" - - name: Qualify candidate + - id: qualify + name: Qualify candidate + continue-on-error: true env: CANDIDATE_TAG: ${{ matrix.tag }} COMPARISON_VERSION: ${{ matrix.comparison_version }} @@ -100,15 +102,21 @@ jobs: --comparison-version "${COMPARISON_VERSION}" \ --tested-baseline-version "${TESTED_BASELINE_VERSION}" \ --output /tmp/ocr-compat/evidence.json \ - --issue-body /tmp/ocr-compat/issue.md + --issue-body /tmp/ocr-compat/issue.md \ + --status-output /tmp/ocr-compat/status.json - name: Upsert qualification issue + if: ${{ !cancelled() }} env: GITHUB_TOKEN: ${{ github.token }} REPOSITORY: ${{ github.repository }} RUN_URL: ${{ github.server_url }}/${{ github.repository }}/actions/runs/${{ github.run_id }} run: | + input=(--status /tmp/ocr-compat/status.json) + if [ -f /tmp/ocr-compat/evidence.json ]; then + input=(--evidence /tmp/ocr-compat/evidence.json) + fi PYTHONPATH=src python scripts/ocr_compat.py upsert-issue \ - --evidence /tmp/ocr-compat/evidence.json \ + "${input[@]}" \ --repository "${REPOSITORY}" \ --run-url "${RUN_URL}" \ --output-number /tmp/ocr-compat/issue-number.txt @@ -117,11 +125,15 @@ jobs: ''|*[!0-9]*) echo "invalid qualification issue number: ${issue}" >&2; exit 1 ;; esac - uses: actions/upload-artifact@043fb46d1a93c77aae656e7c1c64a875d1fc6a0a # v7.0.1 + if: ${{ always() && !cancelled() }} with: name: ocr-compatibility-${{ matrix.tag }} path: /tmp/ocr-compat/ if-no-files-found: error retention-days: 7 + - name: Preserve qualification failure + if: steps.qualify.outcome == 'failure' + run: exit 1 aggregate: name: assess-complete-chain diff --git a/PLANS.md b/PLANS.md index ca171a2..84529dd 100644 --- a/PLANS.md +++ b/PLANS.md @@ -4,4 +4,509 @@ Use this file for active or blocked repository work. Update it before implementa ## Active Work -No active or blocked repository work. +### OCR 1.10.0 integration, review effort, and compatibility failure recovery + +Status: `active` +Owner: Codex +Release classification: `release-required` +Target stable version: `0.8.2` +Plan Origin: `plan_mode_approved` +Last Updated: 2026-08-25 + +#### Goal + +Qualify and integrate checksum-pinned Open Code Review 1.10.0, expose its bounded +review-effort control with a toolkit default of `medium`, preserve toolkit-owned +result, DLP, receipt, and approval boundaries around new group/round data, repair +the compatibility workflow so failed qualifications retain a canonical issue and +bounded artifact, complete the activated BL-017 signal-ownership audit without a +second telemetry layer, update the PATH-effective local OCR binary safely, and +deliver the result through the protected toolkit 0.8.2 release lifecycle. + +#### Requested Scope + +- Create `codex/v0.8.2-ocr-1.10.0` from synchronized `main`, make this plan the + first repository write, commit it with an SSH signature, push it, and open a + Draft PR before product implementation. +- Create milestone `v0.8.2`, one canonical OCR 1.10.0 qualification issue with + the stable workflow marker, and bounded linked work for compatibility failure + recovery and the BL-017 measurement audit. +- Repair the deterministic OCR compatibility gateway for OCR 1.10.0's required + comment `path`, semantic grouping, group-level filtering, and multi-round + lifecycle; qualify the exact hosted Linux amd64 artifact before promotion. +- Preserve a failed qualification as a closed schema-versioned status, update + the canonical issue, upload the artifact, and still fail the qualification + job so aggregation and promotion cannot proceed from incomplete evidence. +- Promote manifest, preflight, public GitLab example, documentation, and machine + evidence from OCR 1.9.10 to exact OCR 1.10.0 with hosted asset/checksum parity. +- Add `OCR_REVIEW_EFFORT` with exact values `low`, `medium`, or `high`; default + it to `medium`, write OCR's root `effort` config, and preserve an explicit OCR + `--effort` CLI flag as the per-run override. +- Keep `OCR_LLM_MAX_COMPLETION_TOKENS` unset by default. Qualify and document + inherited OpenAI completion caps separately: 58,888 for OCR 1.9.10 and 16,384 + for OCR 1.10.0, while retaining explicit `4096` as the operator workaround for + gateways that reserve spending against the requested output cap. +- Reject caller-owned OCR `--output`, `--output=...`, `-o`, and attached short + forms before OCR execution. Continue capturing stdout through the toolkit's + pre-opened owner-only result descriptor and keep `--result` toolkit-owned. +- Treat additive OCR `groups` labels and paths as untrusted private result data. + They may be privately sanitized but never enter findings, GitLab text, receipt + v5, severity, fingerprints, lifecycle commands, toolkit telemetry, or approval. +- Document semantic grouping, review rounds, budget/cost effects, output-path + ownership, completion-cap migration, git-error privacy, and OCR telemetry + cardinality without adopting a second exporter or trusting MR content as a + configuration authority. +- Activate and complete the bounded BL-017 audit. Keep BL-016 parked, BL-018 + conditional, and BL-019/BL-020 inactive; reconcile backlog, strategy, roadmap, + engineering navigation, and execution history only to the achieved truth. +- Update the PATH-effective local `/opt/homebrew/bin/ocr` atomically from its + current 1.8.10 Darwin arm64 binary to exact 1.10.0 after checksum and contract + acceptance, without changing the user's OCR config, credentials, or HOME. +- Produce separate agent- and human-readable Towncrier fragments for maintenance, + feature, and bug-fix outcomes, then run the protected 0.8.2 lifecycle through + exact-head hosted validation, merge, registry publication, and external readback. + +#### Requirement Traceability + +- `REQ-001` (`done`): materialize the approved full plan first, create the + feature branch, signed planning commit, initial push, Draft PR, milestone, and + linked issue structure. Covered by `WQ-01` and `WQ-02`. +- `REQ-002` (`done`): make failed compatibility qualification produce a + bounded status, canonical issue update, and artifact while the job remains red + and aggregate remains blocked. Covered by `WQ-03`. +- `REQ-003` (`done`): adapt deterministic qualification to OCR 1.10.0 comment + paths, grouping, filters, effort rounds, usage, budget, and version-specific + completion caps. Covered by `WQ-03` and `WQ-04`. +- `REQ-004` (`done`): independently source-audit, checksum-verify, and qualify + OCR 1.10.0 Linux amd64, then promote every manifest, preflight, example, and + evidence owner without changing historical OCR 1.9.10 evidence. Covered by + `WQ-04` and `WQ-05`. +- `REQ-005` (`done`): add the exact documented environment contract + `OCR_REVIEW_EFFORT=medium`, closed validation, root-config projection, and + explicit CLI override precedence without MR-controlled routing. Covered by + `WQ-05`. +- `REQ-006` (`done`): reject caller OCR output-path controls and preserve the + existing safe result descriptor, cleanup, and failure behavior. Covered by + `WQ-05`. +- `REQ-007` (`done`): prove new group/round fields remain provider-neutral, + private, DLP-bounded, receipt-independent, and unable to alter severity, + findings, approval, posting, or lifecycle commands. Covered by `WQ-06`. +- `REQ-008` (`done`): complete the BL-017 source-to-signal audit with a + no-new-layer conclusion unless evidence demonstrates a separately scoped gap; + preserve BL-016/018/019/020 activation boundaries. Covered by `WQ-07`. +- `REQ-009` (`done`): publish version-separated, deployment-actionable docs + and changelog text covering added, changed, rejected, inherited, telemetry, + privacy, and migration behavior. Covered by `WQ-07` and `WQ-08`. +- `REQ-010` (`done`): atomically update local OCR to checksum-verified Darwin + arm64 1.10.0 and pass isolated no-LLM checks without modifying user config. + Covered by `WQ-09`. +- `REQ-011` (`pending`): complete focused and one final full local validation, + exact-head hosted PR checks, record the owner-waived local LLM non-claim, and + complete the protected 0.8.2 release/readback lifecycle without restoring + validation duplication removed by #132. Covered by `WQ-10` through `WQ-12`. + +#### Explicit Non-Goals + +- Do not fork or patch upstream OCR, run `ocr scan`, adopt OCR's `--output` file + ownership, or add a second review engine, grouping implementation, or result + writer. +- Do not derive effort, model, provider, completion cap, approval, suppression, + severity, or lifecycle state from merge-request title, description, paths, + diff, discussions, group labels, model prose, or raw provider errors. +- Do not implement automatic effort/profile routing or activate BL-016/BL-018; + explicit `medium` is an operator-owned default, not an inferred policy. +- Do not publish raw group labels, file paths, provider identities, response + bodies, request IDs, stderr, failed compatibility exception text, or rejected + DLP values in issues, notes, receipts, or toolkit telemetry. +- Do not create a general configuration file framework, fuzzing campaign, new + coverage framework, new telemetry exporter, or broad test-directory rewrite. +- Do not weaken or disable Bandit, Gitleaks, CodeQL, dependency review, coverage, + protected-branch, artifact, provenance, or registry gates. Do not run a + separate Codex Security scan unless a later validated finding requires it. +- Do not edit the user's OCR config or credentials, invoke a real model during + the local binary replacement, or preserve downloaded binaries in the repo. +- Do not close issues, milestone, active plan, or stable-release state before + independent external reconciliation confirms the exact published artifacts. + +#### Constraints + +- `main` is clean and equals `origin/main` at stable toolkit v0.8.1 commit + `b0ffdd3c324afe9095ee966b339748d5944b029b`; `.next-version` is `0.8.2`. +- The approved effort decision supersedes the earlier recommended `low`: toolkit + default is exactly `medium`; `low` and `high` remain explicit alternatives. +- Each logical commit requires focused tests, full slice diff self-review, + requirements and trust/data-flow reconciliation, `git diff --check`, and an + SSH-signed commit. New non-trivial tests receive concise behavioral docstrings; + docstrings name a concrete OCR version only when the assertion is versioned. +- Preserve the #132 ownership split: focused checks per slice, one complete local + quality gate before final publication, one hosted coverage owner, one PR build + owner, and complete stable-release gates only at the release boundary. +- The hosted Linux amd64 compatibility probe is primary version evidence. The + Darwin arm64 probe is an independent local wire/CLI check with an isolated + temporary HOME and deterministic HTTP peer, never a real LLM call. +- Qualification failures retain only closed status codes in public artifacts; + raw exception detail may remain only in the bounded workflow log for synthetic + compatibility fixtures and may not include credentials or private repository + content. +- Receipt schema stays v5. Existing DLP distinction between private sanitization + and publication filtering, partial-review approval blocking, posting rollback, + fingerprinting, and human ownership must remain intact. +- A real-model exact-head qualification may use only an owner-configured provider + with `OCR_LLM_MAX_COMPLETION_TOKENS=4096`; the user explicitly waived this + local gate because the current environment has no provider access. Preserve + the non-claim instead of substituting Codex credentials or model output. + +#### Inputs And Sources + +- User-approved v0.8.2 plan and the later explicit choice of default + `OCR_REVIEW_EFFORT=medium`. +- Root `AGENTS.md`, engineering-workflow 0.8.1, `docs/development.md`, + `docs/release.md`, project principles, compatibility policy, public + configuration/operations/GitLab contracts, and the #132 validation split. +- Stable main v0.8.1, issues #129/#130/#132, current OCR support manifest and + evidence, existing compatibility harness, result/DLP/receipt/approval code, + environment-contract tests, and GitLab example. +- Official upstream OCR v1.10.0 release, compare from v1.9.10, commit + `a66240084b382ed97a47590bdec13a6a34df0743`, GitHub asset digests, and + `sha256sum.txt`. +- Failed scheduled workflow run 32815275725: discovery passed; qualification + failed because the existing deterministic response emitted no accepted + comment; issue upsert and artifact upload were then skipped. +- Read-only exact Darwin arm64 probe during planning: reported OCR v1.10.0 and + reproduced `candidate full review did not emit the synthetic comment` without + a real LLM or user-config change. +- BL-016 through BL-020, toolkit strategy/roadmap, and engineering signal owners + for backlog reconciliation. + +#### User Decisions And Answers + +- Ship OCR 1.10.0 in the next toolkit release and update the local OCR binary. +- Investigate every upstream change and prioritize what materially benefits the + toolkit; track demonstrated backlog relationships without overengineering. +- Fix the compatibility Action failure and the missing canonical issue/artifact, + rather than rerunning the unchanged failing workflow repeatedly. +- Default `OCR_REVIEW_EFFORT` to `medium`, accepting up to two review rounds and + the associated cost/latency change; document `low` as the explicit economy + choice and `high` as deliberate deeper review. +- Preserve provider-neutral architecture, DLP/approval independence, safe handling + of every MR-controlled text source, accurate release-note categorization, and + agent-readable deployment/migration language. +- Keep tests in their existing thematic owners, add meaningful boundary evidence, + and avoid mechanical file/subdirectory reorganization or percentage-only tests. +- Preserve efficient validation ownership from #132 instead of repeating the + full suite locally, on every push, after main merge, and again without a new + trust boundary. +- Skip the local LLM-backed review in this environment. This owner waiver removes + the release blocker but does not convert deterministic OCR compatibility or + hosted CI evidence into a claim about a production model/provider response. + +#### Completed Baseline State + +- Toolkit v0.8.1 is stably released and `main` is synchronized and clean. +- OCR 1.9.10 is the exact current recommended/preflight/example version with + immutable compatibility evidence; OCR 1.9.9 remains historical predecessor. +- `OCR_LLM_MAX_COMPLETION_TOKENS` already provides closed protocol-aware override + mapping and is unset by default; provider failures already publish safe static + GitLab guidance without raw provider fields or stderr. +- Receipt v5, canonical publication/DLP projection, partial-review approval + blocking, context-store/MCP boundaries, provider-neutral codehost contracts, + and GitLab posting transactions are implemented and covered. +- Combined branch coverage floor is 85% with four risk-group floors; PR and + release validation ownership was deduplicated in #132. +- The PATH-effective local OCR is user-owned Darwin arm64 v1.8.10 at + `/opt/homebrew/bin/ocr`; it is not managed by a Homebrew formula or cask. +- Latest upstream stable OCR is v1.10.0. Official SHA-256 values relevant here + are Linux amd64 `f8f99ea071bed77dbcaa15fdd2083287bb8ae408d5928b3943ebe0788d191b6b` + and Darwin arm64 `c8f51b17c2be193ca178ecce6b5bcc1e38a5614629fbe81c6e1c95af5ede12e4`. + +#### Current Work Queue + +1. `WQ-01` (`done`): pass plan fidelity, create the feature branch, perform + planning self-review/checks, and make the signed planning commit. +2. `WQ-02` (`done`): push planning head, open Draft PR, create/read back + milestone and canonical/sub-issue coordination. +3. `WQ-03` (`done`): implement bounded failure status plus always-run issue and + artifact handling; add workflow/CLI tests and preserve final failure outcome. +4. `WQ-04` (`done`): adapt the gateway and real OCR contracts for path, + grouping, filtering, effort rounds, budget, usage, and completion caps; push + the signed qualification checkpoint and run hosted Linux qualification. +5. `WQ-05` (`done`): validate hosted evidence, promote OCR 1.10.0, add + `OCR_REVIEW_EFFORT=medium`, reject OCR output-path ownership, and update exact + environment/config/installed-artifact contracts. +6. `WQ-06` (`done`): add focused groups/DLP/receipt/approval/result regressions + and repair only real contract violations exposed by them. +7. `WQ-07` (`done`): complete BL-017 audit and reconcile backlog, strategy, + roadmap, telemetry privacy/cardinality, and no-new-layer conclusion. +8. `WQ-08` (`done`): update public docs, examples, compatibility text, test + evidence matrix, and separate Towncrier feature/bugfix/maintenance fragments. +9. `WQ-09` (`done`): checksum-verify and atomically install local Darwin arm64 + OCR 1.10.0; run version/help/no-LLM isolated contract checks and clean temporary + artifacts with rollback on failure. +10. `WQ-10` (`done`): perform holistic requirements/privacy/architecture/data- + flow/telemetry/docs self-review and one final local quality/security/manifest/ + changelog gate; update plan to exact implementation truth and final commit. +11. `WQ-11` (`done`): push final signed history, wait for exact-head hosted PR + checks, fix only evidence-backed failures through the same commit gate, and + record the user-waived local LLM qualification as an explicit non-claim. +12. `WQ-12` (`in_progress`): ready and merge the protected feature PR, verify the + TestPyPI development artifact, execute protected release/v0.8.2, independently + reconcile PyPI/TestPyPI/provenance/tag/GitHub Release/receipt/install state, + close issues and milestone through release automation, archive this plan with + `scripts/plan_lifecycle.py`, and synchronize clean local `main`. + +#### Locked Decisions + +- Toolkit target is 0.8.2; exact OCR target is 1.10.0. +- `OCR_REVIEW_EFFORT` is a closed lower-case enum with exact default `medium`. +- Explicit OCR `--effort` remains the per-run override; no second CLI wrapper flag + and no MR-derived automatic routing are introduced. +- OCR `groups` is optional untrusted private data, not a toolkit public contract + or approval/receipt/telemetry input. +- OCR `--output/-o` is not adopted because it transfers path creation/truncation + ownership across the toolkit's existing safe result boundary. +- OCR 1.10.0 inherited completion cap is documented and qualified as 16,384, but + toolkit completion-cap default remains unset and explicit 4,096 remains the + recommended gateway-specific workaround. +- Compatibility failure status uses closed phase/reason values and the same stable + version marker; success and failure never own separate issues. +- BL-017 completes as a bounded ownership audit with no new exporter unless the + audit proves a separate gap; current evidence expects `no-new-layer`. +- Local OCR replacement occurs only after exact checksum and contract acceptance, + uses an atomic rollback transaction, and never edits user configuration. + +#### Verification + +- Planning: plan-fidelity check, full `PLANS.md` diff review, `git diff --check`, + signed commit verification, remote branch/Draft PR/issue/milestone readback. +- Workflow failure: unit tests for success/failure status schemas, bounded public + rendering, duplicate issue prevention, failed-job preservation, always-run + issue/artifact steps, cancelled behavior, and aggregate blocking; YAML parse. +- OCR contracts: exact Linux hosted and Darwin local binaries, asset digest plus + checksum-file agreement, version/help/preview, two-file grouping, path-aware + finding, low/medium rounds, early stop, group filter, budget partial, token/tool + accounting, inherited/explicit completion caps, and target-rule selection. +- Runtime config: exact environment set/default tests, closed effort enum, + generated root config, CLI precedence documentation, installed-artifact + configure/preflight/review checks, and caller output-option rejection forms. +- Privacy/approval: safe and hostile group metadata, PII/secret/laundering/private + sanitization, no publication/receipt/log/toolkit-telemetry projection, unchanged + finding fingerprint/severity, safe auto-approval parity, and fail-closed partial, + malformed, budget, or publication-filtered cases. +- Documentation/backlog: current-version/default/checksum consistency, BL-017 + source-to-signal matrix, BL-016/018/019/020 status checks, rendered Towncrier + categories, links/index checks, and deployment-agent language review. +- Final local: focused suites while iterating; once at final head run + `scripts/quality.sh check`, `scripts/gitleaks.sh`, `uv lock --check`, + `scripts/ocr_compat.py validate`, Towncrier draft, `git diff --check`, and the + repository privacy scan. Do not repeat clean multi-Python installs locally + because hosted Build artifacts and release gates own that boundary. +- Hosted/delivery: all required feature-PR checks, explicit local-model non-claim, + protected merge, TestPyPI development build/provenance/install readback, + protected stable release, immutable registry/GitHub/tag/receipt readback, and + supported-Python install verification. + +#### Latest Validation Results + +- `2026-08-25`: `main` and `origin/main` both resolve to + `b0ffdd3c324afe9095ee966b339748d5944b029b`; worktree is clean and next version + is 0.8.2. +- `2026-08-25`: engineering-workflow 0.8.1 audit found all canonical files and + required documentation indexes; no index errors. Audit noise is confined to + ignored disposable `.quality-logs` environments, which remain untouched. +- `2026-08-25`: GitHub CLI is authenticated with repository/workflow scope. + Scheduled compatibility run 32815275725 discovered v1.10.0, then failed its + full-review contract; issue upsert and artifact upload were skipped and the + aggregate job did not run. +- `2026-08-25`: official release metadata and upstream source comparison identify + semantic grouping/rounds, path-aware comments, output-file support, and git + diagnostic changes; exact public asset hashes are recorded above. +- `2026-08-25`: an isolated checksum-verified Darwin arm64 1.10.0 planning probe + reproduced the compatibility comment failure without using a real LLM or + changing the installed OCR/config; all temporary probe/source directories were + removed afterward. +- `2026-08-25`: the complete schema-v2 active plan passed engineering-workflow + `plan_lifecycle.py check`; full plan diff self-review and `git diff --check` + passed with no product or external mutation before the planning checkpoint. +- `2026-08-25`: signed planning commit + `4cc7d6427cfebd26db6ff26739f8710d4b8ae134` was pushed and opened Draft PR + #134. Milestone `v0.8.2` contains canonical OCR issue #135 and linked sub-issues + #136 (failed-qualification retention) and #137 (BL-017 audit); GitHub API + readback confirms all three open issues and both parent-child relationships. +- `2026-08-25`: failed qualification now emits only the closed + `ocr-toolkit.compatibility-status/v1` projection to issue automation, retains + raw diagnostics in the private job log, always attempts canonical issue and + artifact handling, and explicitly restores the red job outcome. Ruff, mypy, + 83 focused compatibility/workflow tests, YAML parsing, plan validation, and + `git diff --check` pass; self-review also added the pre-manifest failure path. +- `2026-08-25`: an isolated checksum-verified Darwin arm64 OCR 1.10.0 contract + run passed the adapted real-binary gateway: path-aware comments, one semantic + grouping call, two default-medium review rounds, one filter call, inherited + `max_completion_tokens=16384`, explicit override `4096`, the existing partial + budget contract, result consumers, and telemetry-off environment. The binary, + HOME, repositories, and receipt were temporary and removed; installed OCR and + user configuration remain unchanged. +- `2026-08-25`: hosted Linux run 32825123658 passed on exact head `3c49968`, + updated canonical issue #135, retained its seven-day artifact, and produced + human-review-required evidence with Linux amd64 SHA-256 + `f8f99ea071bed77dbcaa15fdd2083287bb8ae408d5928b3943ebe0788d191b6b`. The reviewed source and + wire contract were accepted and promoted to manifest/preflight/example owners. + `OCR_REVIEW_EFFORT` now defaults to root-config `medium`, explicit CLI effort + remains authoritative, caller output paths fail before preview, and 272 focused + tests plus 104 subtests, installed wheel/sdist checks, Ruff, mypy, manifest + validation, and `git diff --check` pass. Promotion also exposed and fixed the + relative-manifest success-reporting bug in the compatibility CLI. +- `2026-08-25`: additive OCR group labels, file lists, and round diagnostics have + explicit boundary regressions: safe values leave the canonical result and + auto-approval decision unchanged; PII and recognized secrets are sanitized in + the private result without becoming a publication failure; neither safe nor + sanitized values enter receipt v5; and receipt extensions fail closed. The 129 + focused review/approval tests plus 71 subtests, Ruff, and `git diff --check` + pass. Direct mypy invocation over test files remains non-owner validation and + reports pre-existing test-module export errors; the repository quality owner + remains the final typed gate. +- `2026-08-25`: the BL-017 source-to-signal matrix maps provider, request, + latency, cost, tokens, grouping, rounds, tools, coverage, context, DLP, + posting, approval, and compatibility to their existing owners. OCR 1.10.0 + group span names/attributes contain sorted changed paths or model-produced + labels, so explicitly enabled OCR telemetry is classified as repository- + derived and high-cardinality. The audit concludes `no-new-layer`; BL-016 stays + parked, BL-018 conditional, and BL-019/BL-020 retain their activation gates. + Documentation contracts pass 23 focused tests, Ruff, plan validation, and + `git diff --check`. +- `2026-08-25`: public configuration, compatibility, GitLab, operations, + security, example, README, and test-evidence owners now separate OCR 1.9.10's + inherited `58888` completion cap from the OCR 1.10.0 target's `16384`, explain + explicit `4096`, `medium`/two-round effort, grouping cost and telemetry, + private group/result authority, and rejected caller output ownership. Five + category-correct fragments distinguish feature, compatibility maintenance, + workflow/runtime fixes, audit maintenance, and direct deployment without an + intermediate OCR version. The rendered Towncrier 0.8.2 section and 53 focused + documentation/configuration/integration tests pass with Ruff and + `git diff --check`. +- `2026-08-25`: the PATH-effective local OCR was atomically replaced from + user-owned Darwin arm64 1.8.10 with official OCR 1.10.0 commit `a66240084`. + Installed size `54644290` and SHA-256 + `c8f51b17c2be193ca178ecce6b5bcc1e38a5614629fbe81c6e1c95af5ede12e4` + match GitHub Release metadata and the compatibility manifest. Version/help + confirms root effort with default `medium` and upstream output-file support; + the installed-binary isolated no-LLM gateway contract passes. User + `.opencodereview` directory metadata is byte-for-byte unchanged, rollback and + probe artifacts were removed, and no config, credential, or HOME was edited. +- `2026-08-25`: required inputs in the public configuration tables are now + bold only where the row's scoped `Required` contract applies; optional and + conditionally alternative inputs remain distinguishable. The exact display + set is protected by the environment-contract test rather than presentation- + blind string checks. +- `2026-08-25`: holistic review rechecked the upstream-artifact-to-gateway-to- + private-result-to-DLP-to-receipt/approval flow, provider-neutral ownership, + compatibility failure projection, workflow permissions, documentation, and + release notes. It found and closed one impossible qualification-status gap by + binding each failed phase to its exact closed reason set; no further product, + privacy, approval, or telemetry defect remains. +- `2026-08-25`: the single final local gate passes 1,256 tests plus 310 + subtests at 86.14% combined branch coverage. Risk groups pass at 84%, 82%, + 85%, and 87%; Ruff format/lint, strict MyPy, Bandit, checksum-pinned Gitleaks + 8.24.3, lock resolution, OCR manifest/evidence validation, the rendered 0.8.2 + Towncrier section, `git diff --check`, and plan lifecycle validation pass. + The temporary scanner archive matched SHA-256 + `b90f13bb8c90ab72083d9b0c842e39dafb82c0e5c3f872f407366b7a58909013` + and was removed without changing the global 8.30.1 installation. Clean + package rebuild/install matrices are deliberately left to the single hosted + Build artifacts owner and stable release gates under the #132 validation split. +- `2026-08-25`: exact pushed head + `c9df0cb087a84efd978a88477d49930799f1af04` and tree + `6c4fb8c2acd4ecbe72aee0682fb25a31956350f2` pass all 13 hosted PR checks, + including Linux/macOS Python 3.12-3.14, the single package owner, CodeQL, + dependency review/audit, Bandit, secrets, and coverage quality. The task and + macOS launch environments expose no OCR/OpenAI/Anthropic provider variables + and the owner OCR home contains no config file. The user therefore explicitly + waived local LLM execution; no production-model behavior is claimed. + +#### Risks And Recovery + +- Risk: the gateway fixture accidentally models OCR internals instead of the + consumed public wire contract. Recovery: keep stage detection structural, + assert real binary output/counters, and separately source-audit upstream logic. +- Risk: failure handling masks the qualification exit. Recovery: preserve the + failed step outcome, run only issue/artifact cleanup afterward, then use an + explicit terminal step to fail the job; aggregate continues to require success. +- Risk: medium effort silently changes cost or turns budgeted runs partial. + Recovery: document exact default/round count, retain explicit low override, + count grouping/round usage, and keep partial outcomes approval-ineligible. +- Risk: group labels/paths leak through additive fields, private diagnostics, or + upstream telemetry. Recovery: exclude them from the canonical projection, + re-sanitize private readback, test every sink, default OCR telemetry off, and + document upstream path/cardinality exposure when operators enable it. +- Risk: OCR `--output` bypasses safe file ownership. Recovery: reject every long, + equals, short, and attached form before preview or model execution. +- Risk: local replacement fails or changes config. Recovery: verify the new file + before rename, retain the old executable inside the atomic transaction, restore + it on any failed check, use isolated HOME for probes, and compare user config + metadata before/after without reading or rewriting credential values. +- Risk: the waived live-provider run leaves a model-behavior evidence gap. + Recovery: retain the explicit non-claim in plan/PR/release notes and rely only + on deterministic real-OCR wire checks plus protected hosted/release gates; + never describe these as a production provider review. +- Risk: validation duplication returns. Recovery: follow #132 ownership, use + focused local checks per slice and one final full gate, and retain repetition + only where PR, platform, artifact, release, or registry boundaries differ. + +#### Resume Point + +Continue `WQ-12`: commit and push the owner-waiver reconciliation, wait for the +new exact plan-only head checks, ready and merge protected feature PR #134, +verify its TestPyPI development publication, and prepare the protected +`release/v0.8.2` PR according to `docs/release.md`. + +#### Plan Fidelity Check + +- [x] Every user-requested outcome and the later `medium` decision has a stable + requirement and queue owner. +- [x] Product, workflow, local-install, external-issue, qualification, release, + and post-release outcomes are distinguished. +- [x] Inputs, authoritative documents, upstream evidence, current baseline, and + rejected alternatives are recorded. +- [x] Data flow, trust boundaries, DLP/approval independence, telemetry privacy, + result-file ownership, and provider-neutral reuse are explicit. +- [x] Focused, final local, hosted, waived-model non-claim, artifact, and release + validation responsibilities are mapped without undoing #132. +- [x] Risks have bounded recovery paths and the first safe unfinished action is + exact. + +#### Reconciliation Check + +- [x] `PLANS.md` was empty before this activation; no prior active work was + overwritten. +- [x] Stable v0.8.1 baseline, closed #129/#130/#132, next version 0.8.2, current + OCR 1.9.10 support, failed v1.10.0 workflow, and local OCR 1.8.10 agree. +- [x] BL-017 is ready and its trigger is met; BL-016/018/019/020 remain outside + implementation scope unless later evidence and user approval change them. +- [x] Documentation indexes are complete; no unrelated instruction migration or + test reorganization is pending. + +#### Closure Gate + +- [ ] All requirements and queue items are `done` or explicitly `out_of_scope`. +- [ ] Final exact-head validation, self-review, hosted checks, waived-model + non-claim, stable publication, external reconciliation, issue/milestone closure, + and local OCR verification are recorded. +- [ ] Backlog, roadmap, strategy, public docs, changelog, manifest/evidence, and + execution history describe the same delivered state. +- [ ] `scripts/plan_lifecycle.py check` passes before the checked close/archive + transition; `PLANS.md` is not manually marked done. + +#### Post-Close Delivery + +- Protected feature PR merge, TestPyPI development verification, stable release + PR, PyPI/TestPyPI publication, provenance/attestation, annotated tag, immutable + GitHub Release and release receipt, supported-Python registry installs, issue + receipts, milestone closure, and clean synchronized `main` are in scope for the + complete requested release and remain pending until independently verified. + +#### Handoff Notes + +- Resume from the first non-terminal WQ item and update this plan before every + signed commit, push, external qualification, release transition, or handoff. +- Do not infer success from repository prose, a green aggregate summary, or an + installed version string alone; retain exact commit/tree, asset hashes, job + conclusions, artifacts, and registry/GitHub readback for each boundary. diff --git a/README.md b/README.md index aeed16a..9a74ef7 100644 --- a/README.md +++ b/README.md @@ -22,6 +22,7 @@ ocr-ci --help The exact recommended OCR release and its verified asset checksums live in the [versioned compatibility manifest](compatibility/ocr-support.json). CI should pin that release and checksum before execution. The [versioned compatibility policy](docs/compatibility.md) records tested assets and evidence and describes the conservative Dependabot-like qualification workflow for later upstream releases. Review output defaults to English. `OCR_REVIEW_LANGUAGE` accepts another explicit language name when a project needs localized review output; for example, `OCR_REVIEW_LANGUAGE=Russian`. +The current OCR 1.10.0 integration defaults `OCR_REVIEW_EFFORT` to `medium` for two review rounds. `low` and `high` are explicit one- and three-round alternatives; see the [configuration reference](docs/configuration.md#review-effort) for cost, budget, and precedence boundaries. Stable distributions are published to [PyPI](https://pypi.org/project/open-code-review-toolkit/) and mirrored as checksum-listed, provenance-attested assets in the corresponding [GitHub Release](https://github.com/xeonvs/open-code-review-toolkit/releases). Development snapshots are published only to TestPyPI. diff --git a/ROADMAP.md b/ROADMAP.md index 16adc14..67acd74 100644 --- a/ROADMAP.md +++ b/ROADMAP.md @@ -36,7 +36,7 @@ flowchart LR | M3 External MCP hardening | Established | Qualify and document the safe-use envelope and residual limits of the shipped generic external-MCP composition boundary. | Existing external MCP and built-in composition plus BL-011 real-OCR qualification. | Canonical security and configuration guidance records the direct-composition trust boundaries, tool-name allowlist limits, server-owned object authorization, shared plan/main exposure, response/session persistence, failure degradation, and receipt non-claims observed with checksum-verified OCR and a real synthetic stdio peer. Managed OAuth remains conditional. | | M4 Policy and project guidance | Established | Supply relevant target-branch decisions and guidance without allowing self-whitelisting. | Evidence scoping and target/source snapshots. | Stable delivery independently proves backward-compatible structured decisions, bounded target-derived guidance, one read-only MCP lifecycle, and closure of the tracked release work. | | M5 Bounded review-context enrichment | Established | Extend invocation evidence with bounded forge discussions, verified remediation history, and optional external records through one provider-neutral, capability-constrained context lifecycle, without a second review engine. | Established M1, M3, and M4 boundaries plus the v0.7.0 BL-023 delivery. | v0.7.0 establishes bounded discussion/reference acquisition. The v0.8.0 release tree adds policy-v2 remediation selection, context-store v2 and fixed MCP projection, live bot-root/mention identity, DLP isolation, comment-only remediation, and provider-neutral reuse boundaries. The owner waived the separate enriched OCR+LLM qualification, so no receipt proves model-time `context_list`/`context_get`, still-present/evidence-resolved scenarios, or receipt-level raw-data leakage inspection. Protected release publication and independent external readback remain mandatory delivery evidence but do not replace that absent qualification. | -| M6 Profiles and quality measurement | Planned / conditional | Audit current OCR telemetry and result-derived review signals; add model-profile aliases only after demonstrated operational need. | Established receipt-v5 result, discussion, coverage, posting, normalized token, and reconciled MCP/evidence-use signals; a demonstrated alias need and owner-approved matrix are required only for profile implementation. | The audit either proves current bounded reporting sufficient or isolates a separately scoped provider-neutral gap; any later model profiles remain independent from explicit coverage and budget controls. | +| M6 Profiles and quality measurement | Established / conditional | Keep the completed review-signal ownership audit current; add model-profile aliases only after demonstrated operational need. | The BL-017 audit establishes OCR/provider telemetry and toolkit lifecycle-signal ownership; a demonstrated alias need and owner-approved matrix are required only for profile implementation. | The audit concludes `no-new-layer`; any later model profiles remain conditional and independent from explicit coverage and budget controls. | | M7 Later and conditional work | Conditional | Activate routing, more ecosystems, fuzzing, configuration, forge adapters, or governance work only from demonstrated need. | Milestone-specific activation signals and stable preceding contracts. | Each item meets its own trigger and ships as a coherent validated slice without weakening core invariants. | ## Ordering notes @@ -46,7 +46,7 @@ flowchart LR - M2 is established through independently verified stable delivery of its framework plugins, template rules, scoped evidence, deltas, and built-in MCP projection. Conditional future ecosystem packs remain in M7 and do not reopen M2. - M4 is established through independently verified v0.6.0 artifacts and later protected-target identity improvements. M5 consumes but does not reopen its policy boundary. - M5's foundation is established by the v0.7.0 BL-023 release content; the v0.8.0 release tree completes its remediation/provider-neutral extension under an explicit owner waiver for the separate enriched OCR+LLM receipt. The waiver is a permanent evidence limitation, not a successful qualification: model-time `context_list`/`context_get`, still-present/evidence-resolved scenarios, and receipt-level raw-data leakage inspection remain unproven. Direct provider-specific MCP enforcement is not a second M5 path: external records stay behind the broker, while generic direct MCP retains the established M3 operator boundary. Its dependency graph is exactly `M1 -> M5`, `M3 -> M5`, and `M4 -> M5`; it is not a gate for M6 or M7. The protected release workflow, immutable receipt, and independent registry/GitHub readback remain mandatory external delivery closure and do not erase the waiver. -- The M6 measurement-gap audit can begin from receipt v5, privacy-safe normalized token buckets, and reconciled MCP/evidence-action counts delivered in v0.7.1. BL-017 may inventory M5 receipts if they exist, but M5 does not create a second telemetry implementation or exporter. BL-016 remains parked because OCR already exposes direct run-level selection. +- The completed M6 BL-017 audit maps receipt v5, privacy-safe normalized token buckets, reconciled MCP/evidence-action counts, and OCR 1.10.0 grouping/round telemetry to their existing owners and concludes `no-new-layer`. M5 does not create a second telemetry implementation or exporter. BL-016 remains parked because OCR already exposes direct run-level selection; BL-018 remains conditional. - Versioned documentation remains a separate MCP integration: the toolkit supplies package/version evidence but does not store documentation. - Additional code-hosting adapters remain conditional and GitLab-first M5 does not depend on them. - Historical roadmap names, release plans, changelog entries, closed issues/PRs, and receipts retain their original identities. BL-022 is historical and is not reused. diff --git a/changelog.d/135.bugfix.md b/changelog.d/135.bugfix.md new file mode 100644 index 0000000..2e12472 --- /dev/null +++ b/changelog.d/135.bugfix.md @@ -0,0 +1 @@ +**Fixed:** Reject caller-owned OCR `--output`, `--output=...`, `-o`, and attached short forms before preview so the new upstream output flag cannot bypass the toolkit-owned result descriptor, atomic validation, DLP, cleanup, or posting handoff. Also make successful compatibility promotion report a caller-supplied relative manifest path without crashing after the update was written. diff --git a/changelog.d/135.feature.md b/changelog.d/135.feature.md new file mode 100644 index 0000000..4199d4b --- /dev/null +++ b/changelog.d/135.feature.md @@ -0,0 +1,4 @@ +Expose OCR 1.10.0 review depth and grouping with explicit operator ownership. + +- **Added:** `OCR_REVIEW_EFFORT` is a closed `low|medium|high` setting written to OCR's root `effort` key. Its exact default is `medium`, selecting up to two review rounds; `low` selects one and `high` selects three. A caller-supplied OCR `--effort` remains the per-run override, and merge-request text cannot select it. +- **Changed:** OCR may semantically group related changed files and filter findings per group before additional rounds. Group labels, path-derived keys, membership, and round diagnostics remain untrusted private result data and cannot change findings, severity, fingerprints, lifecycle commands, receipt v5, toolkit telemetry, posting, or automatic approval. diff --git a/changelog.d/135.maintenance.md b/changelog.d/135.maintenance.md new file mode 100644 index 0000000..a76ebf8 --- /dev/null +++ b/changelog.d/135.maintenance.md @@ -0,0 +1,6 @@ +Qualify and promote the next OCR runtime without losing predecessor or deployment context. + +- **OCR 1.9.10 — inherited predecessor:** Toolkit 0.8.0 and 0.8.1 used exact OCR 1.9.10, whose unset OpenAI completion cap was observed as `max_completion_tokens=58888`. Its checksum-pinned evidence remains unchanged; toolkit 0.8.2 does not require installing or requalifying it. +- **OCR 1.10.0 — changed target:** Toolkit 0.8.2 preflight and the GitLab example now require checksum-verified OCR 1.10.0. Qualification covers semantic file grouping, path-aware comments, group filtering, one/two/three review rounds, partial-budget reporting, tool/token accounting, the new `--output` flag, private git diagnostics, and the new inherited OpenAI completion cap `max_completion_tokens=16384`. Linux amd64 SHA-256 is `f8f99ea071bed77dbcaa15fdd2083287bb8ae408d5928b3943ebe0788d191b6b`. +- **Telemetry:** OCR remains authoritative for provider, request, latency, cost, grouping, round, and tool telemetry. OCR 1.10.0 group spans can contain sorted changed paths and model-produced labels; the toolkit defaults OCR telemetry off, does not ingest those values, and adds no exporter or approval/routing signal. +- **Deployment/Migration:** Deploy toolkit 0.8.2 directly with OCR 1.10.0; do not install OCR 1.9.10 as an intermediate step. Unset `OCR_LLM_MAX_COMPLETION_TOKENS` inherits `16384`; set an explicit value such as `4096` when a gateway-specific invariant is required. Caller `--output`/`-o` remains unsupported because `ocr-ci review --result` owns the private result lifecycle. diff --git a/changelog.d/136.bugfix.md b/changelog.d/136.bugfix.md new file mode 100644 index 0000000..3b3e67b --- /dev/null +++ b/changelog.d/136.bugfix.md @@ -0,0 +1 @@ +**Fixed:** A failed OCR compatibility qualification now still updates the canonical version issue and uploads a bounded `ocr-toolkit.compatibility-status/v1` artifact before the job returns red. Public coordination receives only closed phase, reason, version, and run identity; raw exceptions remain in the private job log, and aggregate promotion stays blocked. diff --git a/changelog.d/137.maintenance.md b/changelog.d/137.maintenance.md new file mode 100644 index 0000000..aef2c69 --- /dev/null +++ b/changelog.d/137.maintenance.md @@ -0,0 +1 @@ +**Changed:** Complete the BL-017 review-signal ownership audit with a `no-new-layer` result. OCR retains provider/review telemetry ownership; toolkit receipt, context, DLP, posting, and approval projections retain deterministic lifecycle ownership. No exporter, automatic routing, developer scoring, or duplicate group/round metric schema is added. diff --git a/compatibility/evidence/ocr-1.10.0.json b/compatibility/evidence/ocr-1.10.0.json new file mode 100644 index 0000000..8f6735e --- /dev/null +++ b/compatibility/evidence/ocr-1.10.0.json @@ -0,0 +1,132 @@ +{ + "assets": [ + { + "name": "opencodereview-darwin-amd64", + "sha256": "6936af725318aa67bebe4486dad6fcd4fdcf7e38dccdf8415fefcd0300651a86", + "size": 57592192 + }, + { + "name": "opencodereview-darwin-arm64", + "sha256": "c8f51b17c2be193ca178ecce6b5bcc1e38a5614629fbe81c6e1c95af5ede12e4", + "size": 54644290 + }, + { + "name": "opencodereview-linux-amd64", + "sha256": "f8f99ea071bed77dbcaa15fdd2083287bb8ae408d5928b3943ebe0788d191b6b", + "size": 55849122 + }, + { + "name": "opencodereview-linux-arm64", + "sha256": "15f6ec310103caf103aad48bb81bd1e10b1489353d4dc9a4423724d7fbe1edf8", + "size": 52625570 + }, + { + "name": "opencodereview-windows-amd64.exe", + "sha256": "c355d6c5966dc7d06922c45ca8772c95ee2c781247e120520951979863249601", + "size": 57309184 + }, + { + "name": "opencodereview-windows-arm64.exe", + "sha256": "09f09d7f63211c47100f340dc51ddc2856fd386fdf3f4f7daab6751103658900", + "size": 53292032 + }, + { + "name": "sha256sum.txt", + "sha256": "8ecdf209dd79446488dd35bab858e6b3902ebda7e82279b7a4ba3b9e5775829e", + "size": 572 + } + ], + "classification": "human-review-required", + "classification_reasons": [ + "candidate is not a newer patch in the tested major/minor line", + "release notes contain a material or ambiguous compatibility signal" + ], + "comparison_version": "1.9.10", + "contracts": { + "comment_thinking_probe": { + "additive_field_preserved": true, + "posting_exposes_thinking": false, + "result": "passed" + }, + "completion_cap_probe": { + "explicit": 4096, + "inherited": 16384, + "result": "passed", + "wire_field": "max_completion_tokens" + }, + "optional_capabilities": [ + "llm_result_identity", + "per_run_model_override", + "per_run_provider_override", + "review_effort", + "semantic_grouping" + ], + "preview_probe": { + "format": "json", + "path": "example.py", + "result": "passed", + "session_store_created": false + }, + "required_review_flags": [ + "--audience", + "--background-file", + "--effort", + "--format", + "--from", + "--max-tokens-budget", + "--preview", + "--rule", + "--to" + ], + "result_contract_probe": { + "additive_fields_allowed": true, + "comment_fields": [ + "category", + "content", + "end_line", + "existing_code", + "path", + "severity", + "start_line", + "thinking" + ], + "manifest_schema": "ocr.run-manifest/v1", + "normalized_outcome": "clean", + "result": "passed" + }, + "review_budget_probe": { + "budget": 30000, + "completed": 2, + "failed_budget": 1, + "partial_findings_preserved": true, + "result": "passed", + "selected": 3 + }, + "semantic_grouping_probe": { + "default_effort": "medium", + "filter_requests": 1, + "grouping_requests": 1, + "main_requests": 3, + "result": "passed", + "review_rounds": 2 + }, + "target_rule_selection_probe": { + "format": "json", + "from_to_unchanged": true, + "path": "synthetic-template.ocrfixture", + "result": "passed", + "source_exclusion": "unsupported_ext", + "target_selected": true + }, + "version_probe": "passed" + }, + "published_at": "2026-08-25T05:45:43Z", + "release_changes": "## 🚀 Features\n\n- feat(agent): group semantically related files for multi-file review (#808)\n- feat(cli): add --output flag to write review/scan results to a file (#852)\n\n## 🐛 Bug Fixes\n\n- fix(diff): surface git's own message when a diff command fails (#1039)\n\n**Full Changelog**: https://github.com/alibaba/open-code-review/compare/v1.9.10...v1.10.0", + "release_notes_sha256": "8d0f599600b34b30163d834878518758fa18e41be6f06851f131a2543e9a15cc", + "result": "compatible", + "schema_version": 2, + "tag": "v1.10.0", + "tested_baseline_version": "1.9.10", + "upstream_repository": "alibaba/open-code-review", + "version": "1.10.0" +} diff --git a/compatibility/ocr-support.json b/compatibility/ocr-support.json index dc41f82..a50ccb6 100644 --- a/compatibility/ocr-support.json +++ b/compatibility/ocr-support.json @@ -1,6 +1,6 @@ { - "monitoring_floor": "1.9.10", - "recommended_version": "1.9.10", + "monitoring_floor": "1.10.0", + "recommended_version": "1.10.0", "releases": [ { "assets": [ @@ -1134,6 +1134,59 @@ "release_url": "https://github.com/alibaba/open-code-review/releases/tag/v1.9.10", "status": "tested", "version": "1.9.10" + }, + { + "assets": [ + { + "name": "opencodereview-darwin-amd64", + "sha256": "6936af725318aa67bebe4486dad6fcd4fdcf7e38dccdf8415fefcd0300651a86", + "size": 57592192 + }, + { + "name": "opencodereview-darwin-arm64", + "sha256": "c8f51b17c2be193ca178ecce6b5bcc1e38a5614629fbe81c6e1c95af5ede12e4", + "size": 54644290 + }, + { + "name": "opencodereview-linux-amd64", + "sha256": "f8f99ea071bed77dbcaa15fdd2083287bb8ae408d5928b3943ebe0788d191b6b", + "size": 55849122 + }, + { + "name": "opencodereview-linux-arm64", + "sha256": "15f6ec310103caf103aad48bb81bd1e10b1489353d4dc9a4423724d7fbe1edf8", + "size": 52625570 + }, + { + "name": "opencodereview-windows-amd64.exe", + "sha256": "c355d6c5966dc7d06922c45ca8772c95ee2c781247e120520951979863249601", + "size": 57309184 + }, + { + "name": "opencodereview-windows-arm64.exe", + "sha256": "09f09d7f63211c47100f340dc51ddc2856fd386fdf3f4f7daab6751103658900", + "size": 53292032 + }, + { + "name": "sha256sum.txt", + "sha256": "8ecdf209dd79446488dd35bab858e6b3902ebda7e82279b7a4ba3b9e5775829e", + "size": 572 + } + ], + "capabilities": [ + "llm_result_identity", + "per_run_model_override", + "per_run_provider_override", + "review_effort", + "semantic_grouping" + ], + "evidence": "compatibility/evidence/ocr-1.10.0.json", + "evidence_sha256": "50f9668b68dbcc4dc83f379a55e81aa58bc7188d1be88d44abb32a509361e761", + "human_conclusion": "Reviewed OCR 1.9.10...1.10.0 source and hosted Linux wire evidence: semantic grouping, path-aware comments, default medium two-round review, private additive group metadata, output-file capability, git diagnostics, and the 16384 completion cap are compatible with toolkit boundaries; toolkit retains exclusive result-path ownership and explicit 4096 cap support.", + "published_at": "2026-08-25T05:45:43Z", + "release_url": "https://github.com/alibaba/open-code-review/releases/tag/v1.10.0", + "status": "tested", + "version": "1.10.0" } ], "schema_version": 1, diff --git a/docs/codex/TASKS_BACKLOG.md b/docs/codex/TASKS_BACKLOG.md index bee8f81..c949b41 100644 --- a/docs/codex/TASKS_BACKLOG.md +++ b/docs/codex/TASKS_BACKLOG.md @@ -22,6 +22,7 @@ Statuses are `ready`, `planned`, `parked`, `conditional`, or `owner action`. Rel | File-based user configuration | Retained and clarified | BL-020 remains parked; M5 owns only its narrow protected-target context/DLP policy, not a general configuration framework. | | Additional provider adapters | Retained and clarified | BL-021 remains conditional; future forge parity includes discussion and snapshot capabilities without blocking GitLab-first M5. | | M5 bounded review-context enrichment (BL-023) | Completed and removed | The v0.7.0 release establishes the protected policy, GitLab discussion, broker/store/handle, fixed context-tool, containment, publication-DLP, receipt, setup-diagnostic, and CI-uncertainty boundaries tracked by #107-#111. The complete plan and release checkpoint are preserved in the execution-history archive. | +| Review measurement gaps (BL-017) | Completed and removed | The toolkit 0.8.2 source-to-signal audit concludes `no-new-layer`: OCR retains provider/review telemetry ownership, while toolkit receipts and count-only DLP events retain deterministic lifecycle ownership. Group labels and path-derived keys are explicitly classified as untrusted, high-cardinality upstream telemetry. | ## M3 External MCP hardening @@ -60,27 +61,12 @@ Provider/network telemetry remains outside M1 and M5. OCR owns token, cost, budg - **Validation:** Profile matrix, precedence, preflight, rendering, and compatibility tests. - **Release classification expectation:** `release-required`. -### BL-017: Audit remaining review measurement gaps - -- **Status:** ready -- **Priority:** medium -- **Roadmap theme:** M6 Profiles and quality measurement -- **Dependencies:** Established receipt-v5 result, discussion/fingerprint, coverage, posting, normalized token, and reconciled MCP/evidence-use signals. BL-016 is not required for the audit. -- **Activation trigger:** Met for an audit with current OCR telemetry and toolkit result-derived receipts. -- **Upstream overlap:** OCR remains authoritative for deterministic tool rendering, provider/model identity, session correlation, diff-review usage/budgets, and request/tool latency. Toolkit v0.7.1 only validates and renders a closed privacy-safe subset plus reconciled count-only evidence use. Full-repository `scan` signals do not widen toolkit scope. -- **Goal:** Decide whether privacy-safe toolkit telemetry is needed before implementing metrics or routing. -- **Scoped deliverables:** Inventory OCR token, cost, budget, latency, request, tool-call, and provider/model identity alongside established review health, failed-file coverage, findings, suppression, omission, posting, MCP-use receipts, and M5 context receipts only if they exist. Document only genuinely missing lifecycle, evidence degradation, repeated-discussion, compatibility, or review-value gaps and their privacy/cardinality limits; conclude no-new-layer or create a separately scoped follow-up. -- **Acceptance criteria:** The audit maps every signal to its authoritative source, distinguishes derived from missing data, and reaches an explicit no-new-layer or separately scoped conclusion. OCR remains authoritative for token, cost, budget, request, latency, and tool-call telemetry; toolkit receipt/summary/log events remain authoritative only for deterministic lifecycle and publication filtering. The audit adds no second context telemetry implementation and introduces an exporter only under a separately approved contract. -- **Exclusions:** User surveillance, developer ranking, automatic routing, or mandatory external telemetry. -- **Validation:** Representative result/discussion fixtures, privacy review, and source-to-signal matrix. -- **Release classification expectation:** `no-release` for the audit. - ### BL-018: Evaluate conservative automatic profile routing - **Status:** conditional - **Priority:** low - **Roadmap theme:** M6 Profiles and quality measurement -- **Dependencies:** BL-016, BL-017, and an owner-approved quality/cost policy; M5 is not a dependency. +- **Dependencies:** BL-016, the completed BL-017 ownership audit, and an owner-approved quality/cost policy; M5 is not a dependency. - **Activation trigger:** Representative metrics demonstrate a stable deterministic rule that improves an explicit objective without reducing safety. - **Goal:** Select one run-level profile conservatively from trusted bounded inputs. - **Scoped deliverables:** Document the decision rule, inputs, fallback, observability, and opt-out; implement only after replay evaluation and owner approval. diff --git a/docs/compatibility.md b/docs/compatibility.md index 1cdb664..32494f8 100644 --- a/docs/compatibility.md +++ b/docs/compatibility.md @@ -29,11 +29,21 @@ Evidence records optional capabilities separately from required compatibility. O OCR 1.9.9 was adopted and qualified for toolkit 0.7.1. It makes file background override inline background and names each static main-loop stop trigger in the failed-item reason. The toolkit therefore rejects caller `--background` and `--background-file` inputs and supplies only its own evidence bootstrap file. The new reason literals remain bounded data inside the existing `ocr.run-manifest/v1` failed-item contract: retention, publication projection, DLP, and posting diagnostics already preserve or redact that field without deriving authority from its prose. Expanded upstream provider `tool_choice` tests do not change a consumed wire contract. Its evidence remains historical and does not require an intermediate 1.9.9 installation when deploying toolkit 0.8.0. -### OCR 1.9.10 — toolkit 0.8.0 target +### OCR 1.9.10 — toolkit 0.8.0 target and 0.8.2 predecessor -OCR 1.9.10 is the only version accepted by toolkit 0.8.0 preflight and pinned by the GitLab example. Its terminal retry summary groups noteworthy requests by review stage, but JSON retains the unchanged structured `ocr.llm-retry-report/v1`; the OCR result and `ocr.run-manifest/v1` contracts are unchanged. The report remains private OCR diagnostics and is not toolkit telemetry, a DLP or receipt input, finding evidence, a severity input, an outcome signal, or an automatic-approval signal. The upstream background-compression join and safe resume-checkpoint changes apply to `ocr scan`; the toolkit invokes `ocr review`. The VS Code merge-file change is outside the toolkit contract. +OCR 1.9.10 is the historical exact toolkit 0.8.0 and 0.8.1 dependency. Its terminal retry summary groups noteworthy requests by review stage, but JSON retains the unchanged structured `ocr.llm-retry-report/v1`; the OCR result and `ocr.run-manifest/v1` contracts are unchanged. The report remains private OCR diagnostics and is not toolkit telemetry, a DLP or receipt input, finding evidence, a severity input, an outcome signal, or an automatic-approval signal. The upstream background-compression join and safe resume-checkpoint changes apply to `ocr scan`; the toolkit invokes `ocr review`. The VS Code merge-file change is outside the toolkit contract. With no toolkit completion-cap override, the qualified OpenAI wire request used `max_completion_tokens=58888`. -Deploy toolkit 0.8.0 directly with OCR 1.9.10. The public Linux amd64 example verifies SHA-256 `359e5bafda1438a47ef389399f4994350e1016371eac1dc17a2c428acb228e6c`; the manifest records the complete hosted asset set and qualification evidence. +Deploy toolkit 0.8.0 or 0.8.1 directly with OCR 1.9.10. The historical public Linux amd64 example verified SHA-256 `359e5bafda1438a47ef389399f4994350e1016371eac1dc17a2c428acb228e6c`; its manifest entry and evidence remain unchanged. + +### OCR 1.10.0 — toolkit 0.8.2 target + +OCR 1.10.0 is the only version accepted by toolkit 0.8.2 preflight and pinned by its GitLab example. It groups related changed files before review, uses path-aware findings, and supports one, two, or three review rounds through `effort=low|medium|high`. Toolkit 0.8.2 writes an explicit `medium` default, so a normal run permits two rounds; operators may select `low` for one round or `high` for three. Grouping, filtering, and additional rounds can add provider requests, latency, and token cost. Aggregate budget and manifest completeness remain authoritative, and incomplete or budget-stopped reviews remain ineligible for automatic approval. + +Group labels are model-produced and group keys contain sorted changed paths. Additive group/round result fields remain untrusted private diagnostics: the toolkit sanitizes them when needed but excludes them from findings, GitLab text, receipt v5, fingerprints, severity, lifecycle commands, toolkit telemetry, and approval. OCR's own opt-in telemetry can include these repository-derived high-cardinality values; see the [signal-ownership audit](engineering/review_signal_ownership.md). + +OCR 1.10.0 also adds `--output`; `ocr-ci review` rejects `--output`, `--output=...`, `-o`, and attached short forms before preview because the toolkit retains sole ownership of the private result descriptor and atomic validation/cleanup. Improved upstream git errors remain private OCR stderr and do not widen GitLab diagnostics. With no toolkit completion-cap override, the qualified OpenAI wire request now uses `max_completion_tokens=16384`, down from 58,888 in OCR 1.9.10. `OCR_LLM_MAX_COMPLETION_TOKENS=4096` remains an explicit operator workaround for gateways that reserve spending against the requested output cap. + +Deploy toolkit 0.8.2 directly with OCR 1.10.0; do not install OCR 1.9.10 as an intermediate step. The public Linux amd64 example verifies SHA-256 `f8f99ea071bed77dbcaa15fdd2083287bb8ae408d5928b3943ebe0788d191b6b`; the manifest records the complete hosted asset set and qualification evidence. The contiguous 1.9.7 through 1.9.8 promotion combines the repository-owned hosted Linux qualification with adjacent upstream source review. It also repeats the required 1.9.8 contract probes through the checksum-verified Darwin arm64 artifact. Multi-tool MCP initialization, fixed tool discovery and dispatch, and owner-only session persistence remain compatible with the M5 design: the toolkit may expose several fixed tools from its one built-in process, while an isolated `HOME` contains OCR's `.opencodereview/sessions` state for deterministic cleanup. Gemini and Bedrock are optional upstream provider capabilities that the toolkit neither selects nor configures; mirror-aware installation is outside the toolkit-owned download path; native severity retains the structured result fields already consumed; and human-audience progress routing does not affect the toolkit's agent-audience invocation. Therefore this promotion requires no toolkit CI behavior adaptation beyond version and checksum pins. These source observations establish capability only; the M5 production-path tests remain responsible for proving the toolkit's composition and containment. diff --git a/docs/configuration.md b/docs/configuration.md index 02bd7c9..646e628 100644 --- a/docs/configuration.md +++ b/docs/configuration.md @@ -1,6 +1,6 @@ # Environment configuration -Open Code Review Toolkit uses environment variables for CI/runtime configuration. Empty values are generally treated as absent. Exact defaults and safety caps are enforced by the runtime modules. +Open Code Review Toolkit uses environment variables for CI/runtime configuration. Empty values are generally treated as absent. Exact defaults and safety caps are enforced by the runtime modules. **Bold variable names are required in the command, mode, example, or declaration scope stated in their `Required` cell.** ## Toolkit runtime variables @@ -8,9 +8,9 @@ These are the complete supported toolkit-owned runtime inputs. `Required` is sco | Variable | Source / owner | Required | Exact default | Behavior | | --- | --- | --- | --- | --- | -| `OCR_LLM_URL` | Operator / configure and preflight | Yes for review | None | Absolute credential-free HTTPS API root or compatible terminal inference endpoint; normalized through the shared provider owner. | -| `OCR_LLM_TOKEN` | Operator secret / `ocr-ci configure` | Yes for review | None | LLM credential; never written into generated context or receipts. | -| `OCR_LLM_MODEL` | Operator / configure and preflight | Yes for review | None | Exact model identifier passed to OCR and optional model validation. | +| **`OCR_LLM_URL`** | Operator / configure and preflight | Yes for review | None | Absolute credential-free HTTPS API root or compatible terminal inference endpoint; normalized through the shared provider owner. | +| **`OCR_LLM_TOKEN`** | Operator secret / `ocr-ci configure` | Yes for review | None | LLM credential; never written into generated context or receipts. | +| **`OCR_LLM_MODEL`** | Operator / configure and preflight | Yes for review | None | Exact model identifier passed to OCR and optional model validation. | | `OCR_LLM_PROTOCOL` | Operator / `ocr-ci configure` | No | `openai` | Closed protocol: `openai`, `openai-responses`, or `anthropic`. | | `OCR_LLM_AUTH_HEADER` | Operator / configure and preflight | No | `Authorization` | Valid HTTP header name used for the bearer credential. | | `OCR_LLM_EXTRA_HEADERS` | Operator / configure and preflight | No | Empty object | JSON object of additional string headers; cannot duplicate the auth header. | @@ -18,10 +18,11 @@ These are the complete supported toolkit-owned runtime inputs. `Required` is sco | `OCR_LLM_MAX_COMPLETION_TOKENS` | Operator / `ocr-ci configure` | No | Unset (inherits OCR) | Positive decimal integer from `1` through `1000000`; sets the protocol-specific completion/output cap without changing prompt/context or aggregate review budgets. | | `OCR_ANTHROPIC_DISABLE_THINKING` | Operator / `ocr-ci configure` | No | `false` | With the Anthropic protocol, exact `true` adds `thinking.type=disabled`. | | `OCR_REVIEW_LANGUAGE` | Operator / shared language resolver | No | `English` | Allowed language label or BCP-47 tag used for the review. | +| `OCR_REVIEW_EFFORT` | Operator / `ocr-ci configure` | No | `medium` | Closed OCR quality preset: `low`, `medium`, or `high`; maps to one, two, or three review rounds. | | `OCR_LLM_VALIDATE_MODEL` | Operator / `ocr-ci preflight` | No | `false` | `true` validates through `/models`; `auto` may use the offline allowlist; false values skip validation. | | `OCR_LLM_MODELS_URL` | Operator / `ocr-ci preflight` | No | Derived from `OCR_LLM_URL` | Explicit absolute credential-free HTTPS metadata URL when validation is enabled or inference query parameters make derivation ambiguous. | | `OCR_LLM_ALLOWED_MODELS` | Operator / `ocr-ci preflight` | No | Empty list | Comma-separated exact model identifiers for offline or `auto` validation. | -| `OCR_TELEMETRY_ENABLED` | Operator / `ocr-ci configure` | No | `false` | Exact `true` enables OCR telemetry configuration. | +| `OCR_TELEMETRY_ENABLED` | Operator / `ocr-ci configure` | No | `false` | Exact `true` enables OCR telemetry configuration; OCR 1.10.0 spans may include path-derived group keys and model-produced labels. | | `OCR_TELEMETRY_CONTENT_LOGGING` | Operator / `ocr-ci configure` | No | `false` | Exact `true` enables OCR content logging; keep disabled for private review data. | | `OCR_TELEMETRY_EXPORTER` | Operator / `ocr-ci configure` | No | Empty string | Exporter name written only when telemetry is enabled. | | `OCR_TELEMETRY_OTLP_ENDPOINT` | Operator / `ocr-ci configure` | No | Unset | OTLP endpoint written only when telemetry is enabled and the value is non-empty. | @@ -57,7 +58,15 @@ Since 0.8.0, `OCR_USE_ANTHROPIC` is not a compatibility alias. Any presence fail If `OCR_LLM_EXTRA_BODY` already owns that field, an exactly equal JSON integer is deduplicated. A different value, or a boolean, string, float, or null at that field, fails configuration with a migration error; remove the duplicate field or keep the same integer in both places. Other `OCR_LLM_EXTRA_BODY` members are preserved. For example, set `OCR_LLM_MAX_COMPLETION_TOKENS=4096` when a gateway accepts short probes but rejects a full review before generation because it reserves spending against the requested output cap. -Toolkit 0.8.1 does not derive this value from `/models.max_completion_tokens`. That metadata is a model capability boundary, not an account spending limit or proof of how a gateway reserves request cost. +Toolkit 0.8.2 does not derive this value from `/models.max_completion_tokens`. That metadata is a model capability boundary, not an account spending limit or proof of how a gateway reserves request cost. + +The inherited value is version-owned and therefore changes with a qualified OCR upgrade. The toolkit observed `max_completion_tokens=58888` from OCR 1.9.10 and `16384` from OCR 1.10.0 when the variable was unset. Deployments that require an invariant gateway-specific cap must set `OCR_LLM_MAX_COMPLETION_TOKENS` explicitly rather than depending on either OCR default. + +### Review effort + +`OCR_REVIEW_EFFORT` defaults to `medium` and is written to OCR's root `effort` configuration key. OCR 1.10.0 maps `low`, `medium`, and `high` to one, two, and three review rounds respectively. The environment is operator-owned; merge-request text cannot change it. An explicit caller `--effort` passed after `ocr-ci review --` has normal OCR CLI precedence over the generated config, while an unknown environment value fails configuration before preview or model execution. + +Effort controls review depth, not the prompt/context ceiling, per-call completion cap, aggregate token budget, or per-round tool limit. Semantic grouping and filtering can add requests even at `low`; higher effort can add further rounds until OCR stops early, reaches a coverage/budget boundary, or completes the configured depth. ## GitLab and provider variables @@ -65,17 +74,17 @@ GitLab supplies the `CI_*` values in merge-request pipelines. The operator suppl | Variable | Source / owner | Required | Exact default | Behavior | | --- | --- | --- | --- | --- | -| `GITLAB_API_TOKEN` | Operator secret | Yes for provider reads and posting | None | Dedicated GitLab API credential used with `PRIVATE-TOKEN`. | -| `CI_API_V4_URL` | GitLab predefined | One of this or `CI_SERVER_URL` for provider reads | Derived as `${CI_SERVER_URL}/api/v4` | Absolute HTTPS GitLab API v4 root. | -| `CI_SERVER_URL` | GitLab predefined | Yes for posting; alternative owner for API root | `https://gitlab.com` in posting only | Absolute HTTPS GitLab server root. GitLab CI normally always defines it. | -| `CI_PROJECT_ID` | GitLab predefined | Yes in merge-request mode | None | Bounded numeric project identity used for provider APIs and receipts. | -| `CI_MERGE_REQUEST_IID` | GitLab predefined | Yes in merge-request mode | None | Bounded numeric merge-request identity and mode signal. | -| `CI_MERGE_REQUEST_SOURCE_BRANCH_SHA` | GitLab predefined | Yes for the recommended review range | Falls back to `CI_COMMIT_SHA` only where explicitly documented | Exact reviewed source head used by the review, receipt, and posting revalidation. | -| `CI_MERGE_REQUEST_DIFF_BASE_SHA` | GitLab predefined | Yes for the recommended review range | None | Exact merge-request diff base passed to OCR and evidence collection. | +| **`GITLAB_API_TOKEN`** | Operator secret | Yes for provider reads and posting | None | Dedicated GitLab API credential used with `PRIVATE-TOKEN`. | +| **`CI_API_V4_URL`** | GitLab predefined | One of this or `CI_SERVER_URL` for provider reads | Derived as `${CI_SERVER_URL}/api/v4` | Absolute HTTPS GitLab API v4 root. | +| **`CI_SERVER_URL`** | GitLab predefined | Yes for posting; alternative owner for API root | `https://gitlab.com` in posting only | Absolute HTTPS GitLab server root. GitLab CI normally always defines it. | +| **`CI_PROJECT_ID`** | GitLab predefined | Yes in merge-request mode | None | Bounded numeric project identity used for provider APIs and receipts. | +| **`CI_MERGE_REQUEST_IID`** | GitLab predefined | Yes in merge-request mode | None | Bounded numeric merge-request identity and mode signal. | +| **`CI_MERGE_REQUEST_SOURCE_BRANCH_SHA`** | GitLab predefined | Yes for the recommended review range | Falls back to `CI_COMMIT_SHA` only where explicitly documented | Exact reviewed source head used by the review, receipt, and posting revalidation. | +| **`CI_MERGE_REQUEST_DIFF_BASE_SHA`** | GitLab predefined | Yes for the recommended review range | None | Exact merge-request diff base passed to OCR and evidence collection. | | `CI_COMMIT_SHA` | GitLab predefined | No | None | Fallback head identity when the MR-specific source SHA is unavailable; it does not replace the diff base. | | `CI_PIPELINE_ID` | GitLab predefined | No | Omitted | Optional bounded invocation identity stored as non-authoritative evidence. | | `CI_JOB_ID` | GitLab predefined | No | Omitted | Optional bounded invocation identity stored as non-authoritative evidence. | -| `CI_PIPELINE_SOURCE` | GitLab predefined / example rules | Yes for example job selection | None | The public example runs review jobs only for `merge_request_event`. | +| **`CI_PIPELINE_SOURCE`** | GitLab predefined / example rules | Yes for example job selection | None | The public example runs review jobs only for `merge_request_event`. | ## Example-local variables @@ -83,23 +92,23 @@ These names belong to `examples/gitlab/ocr-review.gitlab-ci.yml`; they are shell | Variable | Source / owner | Required | Exact default | Behavior | | --- | --- | --- | --- | --- | -| `OCR_VERSION` | Example pipeline | Yes | `v1.9.10` | Checksum-pinned recommended OCR binary release for toolkit 0.8.1. | -| `OCR_SHA256` | Example pipeline | Yes | `359e5bafda1438a47ef389399f4994350e1016371eac1dc17a2c428acb228e6c` | Expected Linux AMD64 OCR binary digest. | -| `OCR_TOOLKIT_VERSION` | Example pipeline | Yes | `0.8.1` | Exact toolkit wheel release installed by the current published example. | -| `OCR_TOOLKIT_CHECKSUMS_URL` | Example pipeline | Yes | Release URL derived from `OCR_TOOLKIT_VERSION` | Toolkit `SHA256SUMS` URL. | +| **`OCR_VERSION`** | Example pipeline | Yes | `v1.10.0` | Checksum-pinned recommended OCR binary release for toolkit 0.8.2. | +| **`OCR_SHA256`** | Example pipeline | Yes | `f8f99ea071bed77dbcaa15fdd2083287bb8ae408d5928b3943ebe0788d191b6b` | Expected Linux AMD64 OCR binary digest. | +| **`OCR_TOOLKIT_VERSION`** | Example pipeline | Yes | `0.8.1` | Exact toolkit wheel release installed by the current published example. | +| **`OCR_TOOLKIT_CHECKSUMS_URL`** | Example pipeline | Yes | Release URL derived from `OCR_TOOLKIT_VERSION` | Toolkit `SHA256SUMS` URL. | | `OCR_TOOLKIT_WHEEL` | Example shell | Computed | `open_code_review_toolkit-${OCR_TOOLKIT_VERSION}-py3-none-any.whl` | Exact wheel filename selected from the release. | | `OCR_TOOLKIT_WHEEL_SHA256` | Example shell | Computed | Matching value from `SHA256SUMS` | Digest checked before installing the toolkit wheel. | -| `OCR_MAX_TOOLS` | Example pipeline / OCR CLI | No | `30` | Positive maximum OCR tool-request rounds per file; the example keeps the OCR 1.9.10 default and passes it explicitly. | +| `OCR_MAX_TOOLS` | Example pipeline / OCR CLI | No | `30` | Positive maximum OCR tool requests per review loop; the example owns and passes this bounded value explicitly. | | `OCR_MAX_TOKENS_BUDGET` | Example pipeline / OCR CLI | No | `0` | Non-negative aggregate OCR token ceiling; `0` is unlimited. | ## Dynamic adapter and MCP inputs | Variable | Source / owner | Required | Exact default | Behavior | | --- | --- | --- | --- | --- | -| Names declared by adapter `env_from` | Operator / `OCR_REVIEW_CONTEXT_ADAPTERS_JSON` | Yes when declared | None | Inject an adapter environment value by exact variable name; missing names fail closed. | -| Names declared by adapter `headers_from` | Operator / `OCR_REVIEW_CONTEXT_ADAPTERS_JSON` | Yes when declared | None | Supply a remote adapter header without persisting its secret value in configuration. | -| Names declared by MCP `env_from` | Operator / `OCR_MCP_SERVERS_JSON` | Yes when declared | None | Inject a local-profile stdio MCP environment value by exact variable name. | -| Names declared by MCP `headers_from` | Operator / `OCR_MCP_SERVERS_JSON` | Yes when declared | None | Supply a remote MCP header by reference for OCR expansion at connection time. | +| **Names declared by adapter `env_from`** | Operator / `OCR_REVIEW_CONTEXT_ADAPTERS_JSON` | Yes when declared | None | Inject an adapter environment value by exact variable name; missing names fail closed. | +| **Names declared by adapter `headers_from`** | Operator / `OCR_REVIEW_CONTEXT_ADAPTERS_JSON` | Yes when declared | None | Supply a remote adapter header without persisting its secret value in configuration. | +| **Names declared by MCP `env_from`** | Operator / `OCR_MCP_SERVERS_JSON` | Yes when declared | None | Inject a local-profile stdio MCP environment value by exact variable name. | +| **Names declared by MCP `headers_from`** | Operator / `OCR_MCP_SERVERS_JSON` | Yes when declared | None | Supply a remote MCP header by reference for OCR expansion at connection time. | ## MCP composition and trust boundary @@ -194,7 +203,7 @@ environment variables for policy thresholds or category lists in this release. ## Repository evidence -`ocr-ci review` owns this lifecycle. Before OCR starts it collects the exact immutable `--from`/`--to` refs (or the parent/commit pair selected by `--commit`), writes bounded redacted schema-versioned evidence, builds OCR's MCP registry with the mandatory evidence entry plus each independently configured optional server, reads the registry back, self-queries the evidence summary/list/get contract, and supplies the matching compact bootstrap to OCR. Those parent-process preflight calls are not counted as model use. The same preflight-qualified OCR executable first receives the exact production refs, rules, selection options, and background under `review --preview`; there is no toolkit threshold setting or duplicated OCR threshold constant. An exact recognized OCR soft background warning is copied into the bounded CI log and the atomically finalized result `warnings`, which also blocks automatic approval. An exact recognized hard character/file-size rejection stops before model execution and leaves only an identity-bound numeric `ocr.pre-execution-status/v2` outcome for static GitLab reporting; the private path and raw OCR diagnostic are not published. Unknown preview failures fail closed through the generic diagnostic path, and the actual review independently revalidates the background. During OCR, the built-in MCP atomically records only completed `summary`, `list`, and `get` counts without arguments, IDs, paths, results, or content. The parent reads and removes that private receipt before cleanup and exposes the breakdown only when its total exactly matches OCR's `tool_calls.by_tool.ocr_toolkit_evidence`; missing, malformed, raced, or mismatched attribution is explicitly unavailable rather than zero. Since OCR 1.9.9, `--background-file` takes precedence over inline `--background`, so `ocr-ci review` rejects caller forms of both options, including split and `--option=value` syntax, and remains the sole owner of the bootstrap input; caller `--preview` is likewise rejected because the toolkit owns this gate. Toolkit 0.8.0 requires OCR 1.9.10; its stage-grouped terminal retry report does not enter toolkit telemetry, receipts, DLP, findings, severity, outcomes, or approval. A completed OCR review is accepted only when structured `tool_calls.by_tool` proves at least one `ocr_toolkit_evidence` call; a legitimately skipped no-supported-files review remains exempt. +`ocr-ci review` owns this lifecycle. Before OCR starts it collects the exact immutable `--from`/`--to` refs (or the parent/commit pair selected by `--commit`), writes bounded redacted schema-versioned evidence, builds OCR's MCP registry with the mandatory evidence entry plus each independently configured optional server, reads the registry back, self-queries the evidence summary/list/get contract, and supplies the matching compact bootstrap to OCR. Those parent-process preflight calls are not counted as model use. The same preflight-qualified OCR executable first receives the exact production refs, rules, selection options, and background under `review --preview`; there is no toolkit threshold setting or duplicated OCR threshold constant. An exact recognized OCR soft background warning is copied into the bounded CI log and the atomically finalized result `warnings`, which also blocks automatic approval. An exact recognized hard character/file-size rejection stops before model execution and leaves only an identity-bound numeric `ocr.pre-execution-status/v2` outcome for static GitLab reporting; the private path and raw OCR diagnostic are not published. Unknown preview failures fail closed through the generic diagnostic path, and the actual review independently revalidates the background. During OCR, the built-in MCP atomically records only completed `summary`, `list`, and `get` counts without arguments, IDs, paths, results, or content. The parent reads and removes that private receipt before cleanup and exposes the breakdown only when its total exactly matches OCR's `tool_calls.by_tool.ocr_toolkit_evidence`; missing, malformed, raced, or mismatched attribution is explicitly unavailable rather than zero. Since OCR 1.9.9, `--background-file` takes precedence over inline `--background`, so `ocr-ci review` rejects caller forms of both options, including split and `--option=value` syntax, and remains the sole owner of the bootstrap input; caller `--preview` is likewise rejected because the toolkit owns this gate. OCR 1.10.0 adds `--output`, but `ocr-ci review` rejects its long, equals, short, and attached forms because the toolkit must remain the sole owner of the private result descriptor, atomic parsing, cleanup, and posting handoff. The 1.9.10 stage-grouped terminal retry report remains private and does not enter toolkit telemetry, receipts, DLP, findings, severity, outcomes, or approval. A completed OCR review is accepted only when structured `tool_calls.by_tool` proves at least one `ocr_toolkit_evidence` call; a legitimately skipped no-supported-files review remains exempt. The private `.review-context/evidence.json`, `.review-context/bootstrap.md`, repository-policy `.review-context/policy-rules.json`, and count-only evidence-action receipt/lock are internal implementation details, not public path configuration. Keep `.review-context/` ignored. The directory is mode `0700`, regular files are mode `0600`, and symlink, hard-link, non-regular, or unexpectedly permissive receipt targets are rejected. In GitLab MR pipelines, the provider adapter captures the current protected target SHA, fetches that exact immutable object when needed, and materializes only an in-repository `--rule` blob from it; explicit absolute rules outside the repository remain operator-owned. OCR still reviews the original forge diff-base-to-source-head range. The collector reads Git objects without checkout, does not follow repository symlinks or submodules, never executes repository content, and treats source-ref policy changes as untrusted. diff --git a/docs/engineering/README.md b/docs/engineering/README.md index 9d7869d..a8045a5 100644 --- a/docs/engineering/README.md +++ b/docs/engineering/README.md @@ -6,6 +6,7 @@ This index routes maintainers to canonical engineering owners without duplicatin - [evidence_migration_matrix.md](evidence_migration_matrix.md) - [m5_context_contracts.md](m5_context_contracts.md) - [project_principles.md](project_principles.md) +- [review_signal_ownership.md](review_signal_ownership.md) - [test_evidence_matrix.md](test_evidence_matrix.md) - [toolkit_strategy.md](toolkit_strategy.md) @@ -14,6 +15,7 @@ This index routes maintainers to canonical engineering owners without duplicatin - [Toolkit strategy](toolkit_strategy.md) — durable product boundaries, architecture, and non-goals. - [Project principles](project_principles.md) — cross-cutting trust, ownership, review, and delivery invariants. +- [Review signal ownership](review_signal_ownership.md) — completed BL-017 source-to-signal and telemetry privacy audit. - [Roadmap](../../ROADMAP.md) — ordered outcomes and current milestone state. ## Current contracts and evidence diff --git a/docs/engineering/review_signal_ownership.md b/docs/engineering/review_signal_ownership.md new file mode 100644 index 0000000..acd68a0 --- /dev/null +++ b/docs/engineering/review_signal_ownership.md @@ -0,0 +1,50 @@ +# Review signal ownership + +This matrix records the completed BL-017 audit for toolkit 0.8.2 and the +qualified OCR 1.10.0 runtime. It separates provider/review telemetry from the +toolkit's deterministic control-plane receipts. It is an ownership map, not a +new telemetry API. + +## Source-to-signal matrix + +| Signal | Authoritative source | Toolkit projection | Privacy and authority boundary | +| --- | --- | --- | --- | +| Provider/model identity, request/session correlation, retries, latency, HTTP outcome, and cost | OCR and its configured provider telemetry | None, except the closed provider-failure class used for static failure text | Raw identities, request IDs, response text, and provider codes do not enter GitLab notes, receipt v5, DLP signals, or approval. | +| Prompt, completion, cached, reasoning, and total tokens | OCR result and OCR telemetry | Closed non-negative provider-neutral token buckets in the result summary and canonical publication comparison | Unknown fields are ignored; malformed or contradictory counters are unavailable. Token counts never authorize approval or automatic routing. | +| Review effort and executed rounds | Operator-owned root `effort` config and OCR runtime | `OCR_REVIEW_EFFORT` writes one closed `low`, `medium`, or `high` setting; the toolkit does not publish inferred round telemetry | Merge-request content cannot select effort. Budget or incomplete coverage remains approval-ineligible through the existing result contract. | +| Semantic grouping, group file membership, per-group spans, and filter activity | OCR runtime and OCR telemetry | Additive private result fields may be DLP-sanitized; no group or round field enters receipt v5, GitLab text, fingerprints, severity, lifecycle commands, toolkit telemetry, or approval | Group labels are model-produced. Group keys are sorted changed paths, so both are untrusted and potentially high-cardinality. | +| Tool requests and MCP use | OCR result for aggregate tool calls; each toolkit MCP owner for verified local use | Bounded known-server counts and mandatory evidence-action attribution in receipt v5 and the summary | Tool names and counts are closed; arguments, paths, IDs, results, headers, and content are excluded. | +| Selection, completed/reused/failed/waived coverage, and aggregate-budget stop | OCR run manifest | Closed result outcome, summary, receipt validation, and approval blockers | Incomplete, malformed, failed, waived, or budget-stopped coverage fails closed; no duplicate toolkit budget metric is needed. | +| Findings, severity, fingerprints, suppression, resolution, and repeated discussions | OCR findings plus toolkit-owned posting snapshots and human commands | GitLab discussions, summary, exact fingerprints, and closed lifecycle state | Remediation text and additive group metadata cannot change severity, prove resolution, suppress findings, or issue commands. | +| Context admission, degradation, mutation, and evidence use | Toolkit broker/store and fixed MCP receipts | Count-only receipt-v5 context/evidence state | Raw merge-request title, description, discussions, provider identities, rejected text, and record contents do not enter the receipt or telemetry event. | +| Publication DLP and posting transaction state | Toolkit result projection and GitLab transaction owner | Receipt-v5 publication state, one parseable summary marker, and one local count-only log event | `private-sanitized` can preserve approval only when the canonical projection is unchanged; `publication-filtered` is partial and blocks approval. No rejected value or location is emitted. | +| OCR compatibility qualification | Compatibility workflow and checksum-pinned evidence | Canonical issue plus bounded success or failure artifact | Public failure status contains only closed phase/reason/version/run fields. Raw qualification exceptions stay in the job log. | + +## OCR telemetry privacy + +OCR telemetry remains opt-in. The toolkit defaults both +`OCR_TELEMETRY_ENABLED` and `OCR_TELEMETRY_CONTENT_LOGGING` to `false` and adds +no exporter of its own. + +OCR 1.10.0 constructs group span names from sorted changed paths and attaches +group path keys, model-produced labels, file counts, round numbers, churn, and +filter counters to upstream spans and events. Disabling content logging must not +be treated as removing those identifiers: operators who enable OCR telemetry +must regard the configured exporter as receiving repository-derived, +high-cardinality data and apply their own retention, access, and redaction +policy. The toolkit does not ingest those spans or turn them into receipt, +approval, routing, or quality signals. + +## Audit conclusion + +The established owners already cover provider operations, completeness, +evidence use, context degradation, publication safety, posting, and approval. +The remaining data is either provider-specific telemetry already owned by OCR +or untrusted high-cardinality group data that should not be duplicated. +Therefore BL-017 concludes `no-new-layer`: toolkit 0.8.2 adds no exporter, +metric schema, context telemetry implementation, or automatic routing. + +No safe stable objective currently supports automatic profile routing or a +generic review-quality score. BL-016 remains parked, BL-018 remains conditional +on an owner-approved objective and representative evidence, and BL-019 and +BL-020 retain their existing activation requirements. diff --git a/docs/engineering/test_evidence_matrix.md b/docs/engineering/test_evidence_matrix.md index a6ceaf2..de59061 100644 --- a/docs/engineering/test_evidence_matrix.md +++ b/docs/engineering/test_evidence_matrix.md @@ -95,6 +95,16 @@ The M5 negative suite is an attacker matrix, not merely a replay of observed rev | Configure/preflight provider boundary stays canonical and secret-safe | `provider_config.ProviderConfig` shared by configure and preflight | explicit protocol, normalized HTTPS root/terminal endpoint, query handling, auxiliary URL, headers, and request-body controls agree; credentials/fragments/mismatches fail closed | `test_runtime_helpers.py`, `test_environment_contract.py`, and installed-artifact tests | local metadata peers and environment fixtures prove parser/transport wiring, not a live provider's endpoint policy | proven | | Non-zero retry diagnostics become a closed GitLab failure note | bounded result owner -> `provider_failure.parse_retry_report_failure` -> posting renderer | only validated class/phase/status/terminal outcome selects static text; classified raw result/stderr stays private; previous review remains; findings and approval are unreachable | `test_provider_failure.py`, `test_posting_helpers.py`, and `test_review_runner.py`, including the required status/failure/malformed/oversize/privacy matrices | result fixtures and mocked GitLab writes prove strict parsing, control flow, and rendered payload; no live GitLab write or provider semantic claim | proven for deterministic toolkit policy | +## v0.8.2 OCR 1.10.0 evidence + +| Requirement or boundary | Production owner and entry point | Required observable result | Evidence | Double boundary and claim limit | State | +| --- | --- | --- | --- | --- | --- | +| Exact OCR 1.10.0 review contract | compatibility gateway -> hosted Linux binary -> manifest/evidence promotion | path-aware comments, semantic grouping/filtering, default-medium two rounds, token/tool accounting, partial budget, inherited `16384` and explicit `4096` completion caps | hosted run 32825123658; checksum-pinned `compatibility/evidence/ocr-1.10.0.json`; independent Darwin arm64 no-LLM gateway probe | deterministic gateway proves the consumed wire/result contract, not provider quality or production spending policy | proven for exact qualified OCR wire contract | +| Operator effort stays closed and source-independent | `configure.review_effort` -> generated root `effort` -> OCR CLI precedence | unset/empty becomes `medium`; only `low|medium|high` is accepted; explicit OCR `--effort` remains the per-run override | environment/configuration, runtime-helper, integration, compatibility, and installed wheel/sdist tests | environment and local gateway fixtures prove configuration/round wiring; they do not prescribe a cost-quality choice for every repository | proven | +| Group/round diagnostics cannot gain posting or approval authority | private result -> recursive DLP -> canonical projection -> receipt v5 -> approval | safe group data leaves the canonical projection and eligibility unchanged; PII/secrets are privately sanitized; receipt extensions fail closed | `test_review_runner.py`, `test_posting_approval.py`, and BL-017 source-to-signal audit | result fixtures prove deterministic toolkit isolation, not the semantic quality of OCR grouping | proven | +| Result-file ownership remains toolkit-only | review argument validation -> pre-opened result descriptor -> atomic finalization/cleanup | caller `--output`, `--output=...`, `-o`, and attached short forms fail before preview | `test_review_runner.py` and installed-artifact tests | subprocess doubles prove rejection timing; the exact OCR binary independently proves the new flag exists | proven | +| Failed qualification retains bounded coordination evidence without becoming green | qualification status owner -> always-run issue/artifact handling -> terminal failure step | closed phase/reason/version/run reaches the canonical issue/artifact; raw exception remains job-log-only; aggregate cannot run from failure | compatibility CLI/workflow tests and the repaired workflow contract | static Action tests prove control flow; the next real failed candidate run remains operational readback evidence | proven for workflow contract | + ## Complete suite module audit Every top-level test module is classified below. A module can contain more than one evidence class; the strongest class applies only to the named boundary, never to all tests in that file. diff --git a/docs/engineering/toolkit_strategy.md b/docs/engineering/toolkit_strategy.md index 6f1c415..b810989 100644 --- a/docs/engineering/toolkit_strategy.md +++ b/docs/engineering/toolkit_strategy.md @@ -151,6 +151,13 @@ The evidence engine selects applicable root and ancestor guidance before immutab Qualified OCR releases expose explicit per-run provider/model overrides, additive result identity, and an independent aggregate review budget. Toolkit v0.7.1 normalizes only closed privacy-safe token buckets and distinguishes OCR-wide tool totals from verified MCP-server and evidence-action counts; OCR remains authoritative for provider telemetry, and the toolkit adds no exporter. Direct operator settings remain the current contract. Model-profile aliases such as `economy`, `standard`, or `strong` are parked until repeated use demonstrates that aliases are needed and the owner approves a closed matrix. A profile cannot hide aggregate, per-file, or tool limits that change review completeness; those remain explicit inputs with partial coverage reported normally. +The completed [BL-017 signal-ownership audit](review_signal_ownership.md) reaches +`no-new-layer`. OCR owns provider, request, latency, cost, grouping, round, and +tool telemetry. The toolkit owns only deterministic closed result, receipt, +context, publication-DLP, posting, and approval projections. OCR 1.10.0 group +keys contain changed paths and its labels are model-produced, so neither becomes +a toolkit metric, routing input, or approval authority. + Automatic routing is conditional on stable evidence, latency, token, and review-quality metrics. If activated, it is deterministic, conservative, observable, never changes explicit coverage controls, and never routes a merge request to a full-repository scan. ## OCR compatibility policy diff --git a/docs/gitlab.md b/docs/gitlab.md index e3b7cae..28b8dfc 100644 --- a/docs/gitlab.md +++ b/docs/gitlab.md @@ -18,7 +18,9 @@ The complete variable inventory, owner, requirement, exact default, and behavior The public pipeline stores the OCR binary checksum as the non-secret `OCR_SHA256` pin. Store actual credentials as masked, protected CI variables; do not place their values in YAML, command arguments, repository evidence, or the generated bootstrap. GitLab job tokens are not accepted for posting. -`OCR_REVIEW_LANGUAGE` defaults to `English`; `Russian` is one example of an explicit review language. The example passes `OCR_MAX_TOOLS=30`, matching OCR 1.9.10's per-file tool-round default; increase it deliberately only when a reviewed repository needs more tool interaction. `OCR_MAX_TOKENS_BUDGET` defaults to `0`, meaning unlimited; a positive budget may stop dispatch and produce an explicitly partial, automatic-approval-ineligible review. `OCR_LLM_MAX_COMPLETION_TOKENS` defaults to unset and separately controls only the provider request's completion/output cap; `4096` is a practical explicit value for gateways that reserve spending against a larger requested maximum. +`OCR_REVIEW_LANGUAGE` defaults to `English`; `Russian` is one example of an explicit review language. Toolkit 0.8.2 pins OCR 1.10.0 and sets `OCR_REVIEW_EFFORT=medium`, allowing two review rounds; `low` and `high` explicitly select one or three. OCR may stop early when a round adds no finding. The example passes `OCR_MAX_TOOLS=30` as a separate bound on tool interaction inside a review round; increase it deliberately only when a reviewed repository needs more tool interaction. `OCR_MAX_TOKENS_BUDGET` defaults to `0`, meaning unlimited; a positive budget may stop dispatch and produce an explicitly partial, automatic-approval-ineligible review. `OCR_LLM_MAX_COMPLETION_TOKENS` defaults to unset and separately controls only the provider request's completion/output cap; `4096` is a practical explicit value for gateways that reserve spending against a larger requested maximum. + +OCR 1.10.0 semantically groups related changed files before review and filters candidates per group. Grouping, filtering, and multiple rounds can increase provider requests, latency, and token cost, while the manifest and aggregate budget continue to report completeness. Group labels and path-derived keys are untrusted private OCR diagnostics; the toolkit does not publish or use them for severity, fingerprints, lifecycle commands, receipts, telemetry, or approval. OCR's separately configured telemetry may export those high-cardinality repository-derived values, so keep `OCR_TELEMETRY_ENABLED=false` unless the exporter and retention policy are approved. ## Choose one operating mode diff --git a/docs/operations.md b/docs/operations.md index 670b746..d38f74a 100644 --- a/docs/operations.md +++ b/docs/operations.md @@ -34,9 +34,9 @@ suppressed. `OCR_MAX_TOKENS_BUDGET` can set an aggregate input-plus-output token ceiling for the OCR diff review. The default `0` is unlimited. A positive ceiling is approximate rather than a hard billing cutoff because already-running work may complete; when it stops further dispatch, completed findings remain publishable and unreviewed files stay explicit as budget-attributed failed coverage. Such a run is partial and cannot automatically approve. -This aggregate budget is separate from both OCR's prompt/context `max_tokens` ceiling and the provider request's completion/output cap. The toolkit does not add an environment alias for OCR's prompt/context control. `OCR_LLM_MAX_COMPLETION_TOKENS` defaults to unset and, when set, overlays only the protocol-specific output field. A gateway may reserve cost against that requested maximum before generation even when the eventual response would be short. The `/models` capability value does not reveal an account spending limit or reservation policy, so the toolkit never selects the cap from it automatically. +This aggregate budget is separate from both OCR's prompt/context `max_tokens` ceiling and the provider request's completion/output cap. The toolkit does not add an environment alias for OCR's prompt/context control. `OCR_LLM_MAX_COMPLETION_TOKENS` defaults to unset and, when set, overlays only the protocol-specific output field. The inherited OpenAI value was 58,888 in OCR 1.9.10 and is 16,384 in OCR 1.10.0. A gateway may reserve cost against that requested maximum before generation even when the eventual response would be short. The `/models` capability value does not reveal an account spending limit or reservation policy, so the toolkit never selects the cap from it automatically. Set an explicit cap when a deployment must not change with the qualified OCR version. -The published GitLab example also passes `OCR_MAX_TOOLS=30`, matching OCR 1.9.10's maximum tool-request rounds per file. A file that reaches this bound without the model finishing is explicit failed coverage, so the review is partial. Raise the value only after inspecting the repository/model behavior; repeated exhaustion at a deliberately raised bound is a diagnostic signal, not a reason to increase the ceiling indefinitely. +`OCR_REVIEW_EFFORT=medium` is the toolkit default for OCR 1.10.0 and permits two review rounds; `low` permits one and `high` permits three. This is a review-depth choice, not one of the three token controls. OCR first groups related changed files and may make group-filter requests; additional rounds can add requests, latency, and cost, but may stop early when they add no finding. The published GitLab example separately passes `OCR_MAX_TOOLS=30` for tool interaction inside each review round. Exhausting that limit, an aggregate budget stop, or incomplete manifest coverage remains explicit and approval-ineligible; increasing either effort or tools is never a way to hide partial coverage. The outcome wording distinguishes skipped, complete, complete-with-warnings, incomplete, token-budget, and failed reviews while preserving the finding state in that same line. A complete clean review is visibly positive; a complete review with findings or only reviewer-suppressed findings is neutral; warning, partial, budget, and failed states never look clean. Findings withheld by the posting limit remain counted even when the limit allows no individual finding note. Recommended focus areas ranks only its copy of already-published findings by the closed severity, category, safe repository location, and stable-identity order before its existing display cap; inline and fallback discussion order, suppression, counts, security focus, and approval policy remain unchanged. OCR 1.8.5 and later manifest failures provide the canonical failed-file receipt; legacy warnings are a bounded fallback, and `summary.files_reviewed` is never treated as proof of successful coverage. Technical details label the aggregate as all OCR tool calls, separately label toolkit-verified MCP-server calls, and show built-in evidence `summary`/`list`/`get` counts only after exact reconciliation; unavailable attribution is not displayed as zero. Zero-valued token counters and configured-but-unused MCP servers are omitted. Token usage renders only validated input/output/cached/reasoning/total/derived-other buckets; malformed or contradictory counters are unavailable and unknown provider keys are not published. Status and aggregate semantic-category emoji are enabled by default and can be disabled together with `OCR_POST_EMOJI=false`; finding labels remain text unless their separate badge mode is enabled. @@ -130,6 +130,8 @@ Suppression checks both the recorded inline position and compatible fingerprints Run OCR through `ocr-ci review --result PATH --stderr PATH -- ...`. This wrapper does not post to GitLab: it creates private artifacts, acquires enriched context when selected, asks the exact resolved and preflight-qualified OCR executable to preview the production refs/rules/selection/background without an LLM, then runs the model review only if OCR accepts that background. OCR owns the current warning and rejection thresholds; the toolkit has no threshold setting. A recognized soft warning appears in the CI log and result summary and makes automatic approval ineligible. A recognized hard character/file-size rejection stops before the model and lets `ocr-ci post` publish only a static numeric failure summary; the OCR path and raw diagnostic remain private. Unknown preview failures use the generic fail-closed diagnostic path. The ordinary review still validates the same background, the wrapper validates the complete output, and context/session/configuration data is removed. On an unclassified ordinary failure it prints only a bounded redacted stderr excerpt to the runner log; a classified provider failure keeps that excerpt private. Pass the paths and captured exit code to `ocr-ci post` afterward. Set `OCR_POST_ERROR_DETAILS=1` only when the generic path's safe excerpt should also appear in the merge-request failure note. Cleanup uncertainty blocks result publication. DLP atomically converts unsafe publication output into a safe `completed_with_errors` subset, but sanitizes unsafe private-only result fields without discarding an otherwise valid manifest or finding set. Safe findings are posted, unsafe finding content/warnings and unsafe optional fields are omitted, previous OCR comments remain, and matching prior findings are consumed one-for-one rather than duplicated. Receipt v5 and the `ocr.publication-dlp-signal/v2` marker distinguish `private-sanitized`, where the canonical published and approval-relevant projection is unchanged, from partial approval-ineligible `publication-filtered`. The same count-only JSON is logged as `OCR toolkit telemetry event` for optional CI collection/alerting. It is not an OTLP/network exporter and contains no rejected value or location. Never interpret a filtered subset as a full review. +OCR 1.10.0 may add group labels, file membership, and round diagnostics to its private result. Safe values remain private; DLP removes or replaces unsafe values before atomic retention. These fields are deliberately absent from the canonical finding/posting projection and receipt v5, so private-only sanitization does not block an otherwise eligible auto-approval. If any group or round field appears inside receipt v5, the receipt is invalid and approval fails closed. Caller `--output`/`-o` is rejected before preview: only `ocr-ci review --result` owns the result descriptor and posting handoff. + When OCR exits nonzero with a valid bounded `ocr.llm-retry-report/v1`, the toolkit reads only its closed error class, failure phase, terminal outcome, and HTTP status. It maps those facts to `authentication`, `authorization`, `rate-or-spending-limit`, `overloaded`, `timeout`, `network`, `endpoint-or-model-not-found`, `request-rejected`, `provider-unavailable`, `invalid-response`, `cancelled`, `mixed`, or `unknown`, then writes a completely toolkit-authored note. A runtime `404` remains `endpoint-or-model-not-found` because safely distinguishing the endpoint from the model would require trusting the raw response body. For `429`, the note says that ordinary throttling, an account or API-key spending limit, or cost reservation from the requested output cap are all possible. Retry later and check provider limits. If short probes pass while a full review fails before generation, try an explicit `OCR_LLM_MAX_COMPLETION_TOKENS`, for example `4096`; this is a diagnostic workaround, not a claim that the cap was the cause. diff --git a/docs/security.md b/docs/security.md index 902aed7..24df672 100644 --- a/docs/security.md +++ b/docs/security.md @@ -38,6 +38,7 @@ An operator controls CI configuration, direct MCP servers, adapter commands/endp - Context policy cannot come from the source branch, context budgets cannot evict repository evidence, and model-facing context cannot add a network, arbitrary ID/URL, search, traversal, or write path. - Publication and retention are independent from retrieval/model egress. Cleanup uncertainty blocks publication. DLP selects exact posting sinks separately from private OCR metadata. Unsafe sinks produce an explicit safe partial result; unsafe private-only keys/values are removed or replaced before retention without discarding a still-valid manifest or safe findings. Both paths atomically destroy the rejected value/location and expose only closed counts. Publication filtering preserves prior review state, matches repeated fingerprints one-for-one, and blocks approval; private-only sanitization may continue through all existing approval gates only after exact canonical equivalence. - Classified provider failure reporting consumes only validated retry class, phase, terminal outcome, and HTTP status. Provider/model identity, URL, response body, error code or message, request ID, path, warning text, token, and stderr cannot enter the static note or become a receipt, DLP, telemetry, severity, finding, or approval signal. +- OCR semantic-group labels, path-derived group keys, file membership, and round diagnostics are untrusted private data. They may be DLP-sanitized in the owner-only result but cannot enter findings, GitLab text, receipt v5, fingerprints, severity, lifecycle commands, toolkit telemetry, or approval. Receipt extensions fail closed. - Provider mutations bind reviewed identity where supported; ambiguous inline creates use one author-bound readback without retry, and unresolved ambiguity preserves prior state. - Secrets remain outside repository-controlled context, public notes, fixtures, and release artifacts. @@ -113,6 +114,7 @@ Policy v2 adds optional toolkit-owned remediation threads without changing these - **Identity spoofing and privacy:** represent authors by provider-declared class plus run-local pseudonym; omit name, email, avatar, and profile URL. Unknown classification fails closed. - **TOCTOU/replay/cache poisoning:** bind snapshots and handles to version/ETag or digest, policy version, run, and expiry; use atomic store commits and uniform unavailable outcomes. - **PII/secret bypass and output laundering:** make retrieval, model egress, publication sinks, and private retention separate decisions. Minimize before model egress; publication DLP cannot undo prior disclosure. Scan decoded source and rendered forms so HTML comments/entities/tags, Markdown inline/reference/autolink destinations, controls, and Unicode formatting cannot hide exact protected values. Sink uncertainty yields a safe partial result; private-only uncertainty yields an atomically sanitized valid result or falls back to that partial form. Expose only low-cardinality receipt/summary/log signals in ordinary runs. The explicit local-only diagnostic switch retains owner-only raw private state, emits no receipt, and is rejected by the validated GitLab MR profile. +- **Upstream telemetry cardinality:** OCR 1.10.0 can place sorted changed paths in group span names/attributes and attach model-produced group labels and round counters. OCR telemetry is disabled by default. Enabling it authorizes the configured exporter to receive repository-derived high-cardinality data; content-logging disablement is not a promise that these identifiers disappear. The toolkit neither ingests nor duplicates them. - **Local executable and subprocess confused deputy:** accept the OCR executable only through absolute search-path entries and reject resolution inside the reviewed repository. Stdio adapter deadlines cover request delivery as well as response/exit, use a clean environment and isolated directory, and terminate the process group on uncertainty. - **Marker and retry spoofing:** recognize ownership, run-summary, and setup identities only in the exact toolkit preamble. Repository/provider text containing marker lookalikes has no authority. Duplicate findings consume visible prior occurrences one-for-one; successful setup retries replace only previous toolkit setup-pending notes. - **Approval/suppression manipulation:** every admitted remediation thread blocks automatic approval; partial, required-degraded, or DLP-rejected context cannot prove absence or restore eligibility. Safe MR metadata, generic discussions, and adapter records do not themselves block an otherwise eligible receipt. Remediation text cannot change severity, prove a fix, suppress or resolve a finding, issue a lifecycle command, or authorize approval. Existing suppression, exact slash/mention commands, fingerprints, and discussion ownership remain separate deterministic consumers. diff --git a/examples/gitlab/README.md b/examples/gitlab/README.md index d72fb27..df4ecf4 100644 --- a/examples/gitlab/README.md +++ b/examples/gitlab/README.md @@ -2,6 +2,8 @@ [`ocr-review.gitlab-ci.yml`](ocr-review.gitlab-ci.yml) is the complete checksum-pinned pipeline. It defaults to identity-only review and contains the install, preflight, configure, review, and post lifecycle. Copy one mode file below into the pipeline's top-level `variables` mapping, or include exactly one file after reviewing its trust boundary. +The pipeline pins OCR 1.10.0 and explicitly sets `OCR_REVIEW_EFFORT=medium`, which permits two review rounds. Use `low` for one round when latency/cost is the priority, or `high` for three only after accepting the additional provider work. Semantic grouping and filtering are OCR behavior shared by every context mode; they do not change which merge-request text a mode admits. `OCR_MAX_TOOLS`, `OCR_MAX_TOKENS_BUDGET`, and the optional completion cap remain independent controls. + | Mode | Recipe | MR text admitted | External access | Automatic approval | | --- | --- | --- | --- | --- | | Identity only | [`modes/identity-only.gitlab-ci.yml`](modes/identity-only.gitlab-ci.yml) | None | None | May remain enabled | diff --git a/examples/gitlab/ocr-review.gitlab-ci.yml b/examples/gitlab/ocr-review.gitlab-ci.yml index e604437..0b437e2 100644 --- a/examples/gitlab/ocr-review.gitlab-ci.yml +++ b/examples/gitlab/ocr-review.gitlab-ci.yml @@ -6,10 +6,10 @@ default: image: python:3.12-slim variables: - OCR_VERSION: "v1.9.10" + OCR_VERSION: "v1.10.0" OCR_TOOLKIT_VERSION: "0.8.1" OCR_TOOLKIT_CHECKSUMS_URL: "https://github.com/xeonvs/open-code-review-toolkit/releases/download/v${OCR_TOOLKIT_VERSION}/SHA256SUMS" - OCR_SHA256: "359e5bafda1438a47ef389399f4994350e1016371eac1dc17a2c428acb228e6c" + OCR_SHA256: "f8f99ea071bed77dbcaa15fdd2083287bb8ae408d5928b3943ebe0788d191b6b" OCR_POST_MODE: "draft" OCR_STRICT_POSTING: "true" # Default-on receipt-bound approval; external MCP always remains comment-only. @@ -20,7 +20,9 @@ variables: # OCR_POST_BADGES: "shields" OCR_LLM_VALIDATE_MODEL: "false" OCR_LLM_ALLOWED_MODELS: "" - # OCR 1.9.10 default; raise deliberately for repositories that need more tool rounds. + # Two review rounds by default; low and high remain explicit operator choices. + OCR_REVIEW_EFFORT: "medium" + # Bounded example cap; raise deliberately for repositories that need more tool interaction. OCR_MAX_TOOLS: "30" # Optional aggregate review ceiling; 0 keeps OCR's unlimited default. OCR_MAX_TOKENS_BUDGET: "0" diff --git a/scripts/ocr_compat.py b/scripts/ocr_compat.py index c4b547b..eb2e885 100644 --- a/scripts/ocr_compat.py +++ b/scripts/ocr_compat.py @@ -21,10 +21,10 @@ import urllib.error import urllib.parse import urllib.request -from collections.abc import Iterator +from collections.abc import Callable, Iterator from dataclasses import dataclass from pathlib import Path -from typing import Any, NoReturn +from typing import Any, NoReturn, TypeVar ROOT = Path(__file__).resolve().parents[1] DEFAULT_MANIFEST = ROOT / "compatibility" / "ocr-support.json" @@ -80,13 +80,41 @@ "llm_result_identity", "per_run_model_override", "per_run_provider_override", + "review_effort", + "semantic_grouping", } +QUALIFICATION_STATUS_SCHEMA = "ocr-toolkit.compatibility-status/v1" +QUALIFICATION_PHASES = {"metadata", "artifact", "contracts", "evidence", "complete"} +QUALIFICATION_REASONS = { + "metadata-invalid", + "metadata-request-failed", + "artifact-verification-failed", + "contract-probe-failed", + "evidence-write-failed", + "compatible", +} +QUALIFICATION_FAILURE_REASONS = { + "metadata": frozenset({"metadata-invalid", "metadata-request-failed"}), + "artifact": frozenset({"artifact-verification-failed"}), + "contracts": frozenset({"contract-probe-failed"}), + "evidence": frozenset({"evidence-write-failed"}), +} +T = TypeVar("T") class CompatibilityError(Exception): """OCR compatibility metadata or qualification failed closed.""" +class QualificationStageError(CompatibilityError): + """Carry one closed public qualification stage alongside private detail.""" + + def __init__(self, message: str, *, phase: str, reason: str) -> None: + super().__init__(message) + self.phase = phase + self.reason = reason + + class _NoRedirectHandler(urllib.request.HTTPRedirectHandler): """Reject metadata redirects so the reviewed API origin cannot change.""" @@ -255,6 +283,37 @@ def validate_manifest(manifest: dict[str, Any], root: Path = ROOT) -> None: "selected": 3, }: _fail(f"evidence does not qualify partial review budget behavior for {version}") + if _version(version) >= (1, 10, 0): + contracts = evidence.get("contracts") + required_flags = ( + contracts.get("required_review_flags") if isinstance(contracts, dict) else None + ) + capabilities = ( + contracts.get("optional_capabilities") if isinstance(contracts, dict) else None + ) + if not isinstance(required_flags, list) or "--effort" not in required_flags: + _fail(f"evidence does not qualify the review effort flag for {version}") + if not isinstance(capabilities, list) or not { + "review_effort", + "semantic_grouping", + }.issubset(capabilities): + _fail(f"evidence does not qualify effort and grouping for {version}") + if contracts.get("semantic_grouping_probe") != { + "default_effort": "medium", + "filter_requests": 1, + "grouping_requests": 1, + "main_requests": 3, + "result": "passed", + "review_rounds": 2, + }: + _fail(f"evidence does not qualify semantic grouping behavior for {version}") + if contracts.get("completion_cap_probe") != { + "explicit": 4_096, + "inherited": 16_384, + "result": "passed", + "wire_field": "max_completion_tokens", + }: + _fail(f"evidence does not qualify the completion cap for {version}") evidence_assets = evidence.get("assets") if not isinstance(evidence_assets, list): _fail(f"evidence assets are missing for {version}") @@ -614,7 +673,44 @@ class _StubHandler(http.server.BaseHTTPRequestHandler): request_count = 0 tokens_per_request = 2 + grouping_tokens_per_request = 2 + grouping_mode = "singletons" completion_caps: list[object] = [] + request_stages: list[str] = [] + + @staticmethod + def _message_contents(messages: list[Any]) -> list[str]: + """Return only text message content from one bounded probe request.""" + + return [ + content + for message in messages + if isinstance(message, dict) and isinstance((content := message.get("content")), str) + ] + + @classmethod + def _grouping_files(cls, messages: list[Any]) -> list[str]: + """Extract the public grouping prompt's changed-file inventory.""" + + paths: list[str] = [] + for content in cls._message_contents(messages): + for match in re.finditer( + r"(?m)^([^\r\n]+) \((?:ADDED|MODIFIED|DELETED|RENAMED), \+[0-9]+/-[0-9]+\)$", + content, + ): + candidate = match.group(1) + if candidate not in paths: + paths.append(candidate) + return paths + + @classmethod + def _review_path(cls, messages: list[Any]) -> str | None: + """Extract the first path from OCR's public review-files XML block.""" + + for content in cls._message_contents(messages): + if match := re.search(r'', content): + return match.group(1) + return None def do_POST(self) -> None: if self.path != "/v1/chat/completions": @@ -645,7 +741,21 @@ def do_POST(self) -> None: function = tool.get("function") if isinstance(tool, dict) else None if isinstance(function, dict) and isinstance(function.get("name"), str): tool_names.add(function["name"]) - if "approve_all_comments" in tool_names: + message: dict[str, Any] + if not tool_names: + paths = type(self)._grouping_files(messages) + if not paths: + self.send_error(400) + return + groups = ( + [{"label": "compatibility-group", "files": paths}] + if type(self).grouping_mode == "combined" + else [{"label": path, "files": [path]} for path in paths] + ) + message = {"role": "assistant", "content": json.dumps(groups)} + finish_reason = "stop" + stage = "grouping" + elif "approve_all_comments" in tool_names: message = { "role": "assistant", "content": None, @@ -658,7 +768,9 @@ def do_POST(self) -> None: ], } finish_reason = "tool_calls" + stage = "filter" else: + contents = type(self)._message_contents(messages) prior_comment = any( isinstance(message, dict) and any( @@ -669,13 +781,19 @@ def do_POST(self) -> None: ) for message in messages ) - if not prior_comment: + later_round = any("### Previously Confirmed Findings" in item for item in contents) + if not prior_comment and not later_round: + path = type(self)._review_path(messages) + if path is None: + self.send_error(400) + return arguments = json.dumps( { "comments": [ { "content": "Synthetic compatibility finding.", "existing_code": " return 2", + "path": path, "thinking": "Synthetic private compatibility reasoning.", "category": "maintainability", "severity": "low", @@ -700,6 +818,13 @@ def do_POST(self) -> None: ], } finish_reason = "tool_calls" + stage = "main" + type(self).request_stages.append(stage) + usage_tokens = ( + type(self).grouping_tokens_per_request + if stage == "grouping" + else type(self).tokens_per_request + ) payload = json.dumps( { "id": f"compat-{type(self).request_count}", @@ -707,9 +832,9 @@ def do_POST(self) -> None: "model": "synthetic-model", "choices": [{"index": 0, "message": message, "finish_reason": finish_reason}], "usage": { - "prompt_tokens": type(self).tokens_per_request - 1, + "prompt_tokens": usage_tokens - 1, "completion_tokens": 1, - "total_tokens": type(self).tokens_per_request, + "total_tokens": usage_tokens, }, } ).encode() @@ -726,14 +851,24 @@ def log_message(self, _format: str, *_args: Any) -> None: @contextlib.contextmanager -def _stub_gateway(*, tokens_per_request: int = 2) -> Iterator[str]: +def _stub_gateway( + *, + tokens_per_request: int = 2, + grouping_tokens_per_request: int = 2, + grouping_mode: str = "singletons", +) -> Iterator[str]: """Serve deterministic responses with configurable real usage accounting.""" - if tokens_per_request < 2: + if tokens_per_request < 2 or grouping_tokens_per_request < 2: _fail("stub gateway token usage must be at least two") + if grouping_mode not in {"singletons", "combined"}: + _fail("stub gateway grouping mode is invalid") _StubHandler.request_count = 0 _StubHandler.tokens_per_request = tokens_per_request + _StubHandler.grouping_tokens_per_request = grouping_tokens_per_request + _StubHandler.grouping_mode = grouping_mode _StubHandler.completion_caps = [] + _StubHandler.request_stages = [] server = http.server.ThreadingHTTPServer(("127.0.0.1", 0), _StubHandler) thread = threading.Thread(target=server.serve_forever, daemon=True) thread.start() @@ -749,6 +884,8 @@ def detect_optional_capabilities(help_output: str, sample: dict[str, Any]) -> li """Validate additive OCR identity fields and return observed optional capabilities.""" optional_capabilities: set[str] = set() + if "--effort" in help_output: + optional_capabilities.add("review_effort") if "--model" in help_output: optional_capabilities.add("per_run_model_override") if "--provider" in help_output: @@ -766,9 +903,48 @@ def detect_optional_capabilities(help_output: str, sample: dict[str, Any]) -> li ): _fail("candidate full review emitted an invalid additive LLM provider identity") optional_capabilities.add("llm_result_identity") + groups = sample.get("groups") + if groups is not None: + _validate_file_groups(groups) + optional_capabilities.add("semantic_grouping") return sorted(optional_capabilities) +def _validate_file_groups(value: Any, expected_paths: set[str] | None = None) -> None: + """Validate bounded additive group metadata without retaining its labels.""" + + if not isinstance(value, list) or not value or len(value) > 100: + _fail("candidate full review emitted invalid additive file groups") + observed: list[str] = [] + for group in value: + if not isinstance(group, dict): + _fail("candidate full review emitted an invalid file group") + label = group.get("label") + files = group.get("files") + if ( + not isinstance(label, str) + or not label + or len(label) > 500 + or any(ord(character) < 32 for character in label) + or not isinstance(files, list) + or not files + or len(files) > 10 + ): + _fail("candidate full review emitted an invalid file group") + for path in files: + if ( + not isinstance(path, str) + or not path + or len(path) > 1_000 + or any(ord(character) < 32 for character in path) + or path in observed + ): + _fail("candidate full review emitted an invalid grouped path") + observed.append(path) + if expected_paths is not None and set(observed) != expected_paths: + _fail("candidate semantic grouping did not cover the expected paths") + + def _budget_result_probe(binary: Path, directory: Path) -> dict[str, object]: """Drive the real OCR review budget gate and validate its partial manifest.""" @@ -867,7 +1043,85 @@ def _budget_result_probe(binary: Path, directory: Path) -> dict[str, object]: } -def _completion_cap_probe(binary: Path, directory: Path) -> dict[str, object]: +def _semantic_grouping_probe(binary: Path, directory: Path) -> dict[str, object]: + """Drive one real two-file group through grouping and medium review rounds.""" + + root = directory / "semantic-grouping-probe" + root.mkdir() + git_env = _isolated_probe_environment(root / "git-home") + repo = root / "review" + repo.mkdir() + _run(["git", "init", "--initial-branch=main"], cwd=repo, env=git_env) + _run(["git", "config", "user.name", "Synthetic Reviewer"], cwd=repo, env=git_env) + _run(["git", "config", "user.email", "reviewer@example.com"], cwd=repo, env=git_env) + paths = ("first.py", "second.py") + for path in paths: + (repo / path).write_text("def value():\n return 1\n", encoding="utf-8") + _run(["git", "add", *paths], cwd=repo, env=git_env) + _run(["git", "commit", "-m", "grouping baseline"], cwd=repo, env=git_env) + base = _run(["git", "rev-parse", "HEAD"], cwd=repo, env=git_env).strip() + for path in paths: + (repo / path).write_text("def value():\n return 2\n", encoding="utf-8") + _run(["git", "commit", "-am", "group related changes"], cwd=repo, env=git_env) + head = _run(["git", "rev-parse", "HEAD"], cwd=repo, env=git_env).strip() + + env = _isolated_probe_environment(root / "review-home") + with _stub_gateway(grouping_mode="combined") as gateway_url: + env.update( + { + "OCR_LLM_URL": gateway_url, + "OCR_LLM_TOKEN": "synthetic-token", + "OCR_LLM_MODEL": "synthetic-model", + "OCR_LLM_PROTOCOL": "openai", + "OCR_TELEMETRY_ENABLED": "false", + } + ) + output = _run( + [ + str(binary), + "review", + "--from", + base, + "--to", + head, + "--format", + "json", + "--audience", + "agent", + "--concurrency", + "1", + ], + cwd=repo, + env=env, + ) + stages = list(_StubHandler.request_stages) + try: + sample = json.loads(output) + except json.JSONDecodeError as exc: + raise CompatibilityError("semantic grouping review did not emit JSON") from exc + if not isinstance(sample, dict): + _fail("semantic grouping review emitted an unsupported result object") + _validate_file_groups(sample.get("groups"), set(paths)) + groups = sample["groups"] + if ( + len(groups) != 1 + or groups[0].get("label") != "compatibility-group" + or groups[0].get("files") != list(paths) + ): + _fail("semantic grouping review did not preserve the accepted group") + if stages != ["grouping", "main", "main", "filter", "main"]: + _fail(f"default medium review emitted an unexpected stage sequence: {stages!r}") + return { + "default_effort": "medium", + "filter_requests": 1, + "grouping_requests": 1, + "main_requests": 3, + "result": "passed", + "review_rounds": 2, + } + + +def _completion_cap_probe(binary: Path, version: str, directory: Path) -> dict[str, object]: """Observe the real OCR chat-completions output cap with and without an override.""" probe_root = directory / "completion-cap-probe" @@ -875,7 +1129,8 @@ def _completion_cap_probe(binary: Path, directory: Path) -> dict[str, object]: git_env = _isolated_probe_environment(probe_root / "git-home") repo, base, head = _synthetic_repo(probe_root, git_env) observed: dict[str, int] = {} - for label, expected in (("inherited", 58_888), ("explicit", 4_096)): + inherited = 16_384 if _version(version) >= (1, 10, 0) else 58_888 + for label, expected in (("inherited", inherited), ("explicit", 4_096)): env = _isolated_probe_environment(probe_root / f"{label}-home") with _stub_gateway() as gateway_url: env.update( @@ -1053,7 +1308,10 @@ def run_contracts(binary: Path, version: str, directory: Path) -> dict[str, Any] if re.search(rf"(?= (1, 10, 0): + required_review_flags.add("--effort") + missing = sorted(flag for flag in required_review_flags if flag not in help_output) if missing: _fail(f"candidate review help is missing required flags: {', '.join(missing)}") preview_home = directory / "preview-home" @@ -1108,6 +1366,7 @@ def run_contracts(binary: Path, version: str, directory: Path) -> dict[str, Any] cwd=repo, env=env, ) + review_stages = list(_StubHandler.request_stages) try: sample = json.loads(result_output) except json.JSONDecodeError as exc: @@ -1128,6 +1387,10 @@ def run_contracts(binary: Path, version: str, directory: Path) -> dict[str, Any] comments = sample.get("comments") if not isinstance(comments, list) or len(comments) != 1 or not isinstance(comments[0], dict): _fail("candidate full review did not emit the synthetic comment") + if _version(version) >= (1, 10, 0): + _validate_file_groups(sample.get("groups"), {"example.py"}) + if review_stages != ["main", "main", "filter", "main"]: + _fail(f"default medium review emitted an unexpected stage sequence: {review_stages!r}") sample["future_additive_field"] = {"accepted": True} from ocr_toolkit.posting.comments import comment_line from ocr_toolkit.posting.formatting import ( @@ -1163,7 +1426,7 @@ def run_contracts(binary: Path, version: str, directory: Path) -> dict[str, Any] "review_budget_probe": _budget_result_probe(binary, directory), "target_rule_selection_probe": _target_rule_selection_probe(binary, version, directory), "version_probe": "passed", - "required_review_flags": sorted(REQUIRED_REVIEW_FLAGS), + "required_review_flags": sorted(required_review_flags), "preview_probe": { "format": "json" if json_preview else "text", "path": "example.py", @@ -1178,8 +1441,10 @@ def run_contracts(binary: Path, version: str, directory: Path) -> dict[str, Any] "result": "passed", }, } + if _version(version) >= (1, 10, 0): + contracts["semantic_grouping_probe"] = _semantic_grouping_probe(binary, directory) if _version(version) >= (1, 9, 10): - contracts["completion_cap_probe"] = _completion_cap_probe(binary, directory) + contracts["completion_cap_probe"] = _completion_cap_probe(binary, version, directory) if thinking_probe is not None: contracts["comment_thinking_probe"] = thinking_probe return contracts @@ -1245,6 +1510,91 @@ def issue_plain_text(value: Any, field: str, max_chars: int = 500) -> str: return html.escape(cleaned, quote=False).replace("@", "@\u200b") +def qualification_status( + *, + tag: str, + comparison_version: str, + tested_baseline_version: str, + result: str, + phase: str, + reason: str, +) -> dict[str, str]: + """Build the closed public status retained for one qualification attempt.""" + + if VERSION_RE.fullmatch(tag) is None: + _fail("qualification status tag is invalid") + version = tag.removeprefix("v") + comparison = comparison_version.removeprefix("v") + baseline = tested_baseline_version.removeprefix("v") + _version(comparison) + _version(baseline) + if result not in {"compatible", "failed"}: + _fail("qualification status result is invalid") + if phase not in QUALIFICATION_PHASES or reason not in QUALIFICATION_REASONS: + _fail("qualification status phase or reason is invalid") + if (result == "compatible") != (phase == "complete" and reason == "compatible"): + _fail("qualification status result is inconsistent") + if result == "failed" and reason not in QUALIFICATION_FAILURE_REASONS.get(phase, frozenset()): + _fail("qualification status failure phase and reason are inconsistent") + return { + "comparison_version": comparison, + "phase": phase, + "reason": reason, + "result": result, + "schema": QUALIFICATION_STATUS_SCHEMA, + "tag": f"v{version}", + "tested_baseline_version": baseline, + "version": version, + } + + +def validate_qualification_status(value: dict[str, Any]) -> dict[str, str]: + """Reject any status that contains open-ended or inconsistent public data.""" + + expected_keys = { + "comparison_version", + "phase", + "reason", + "result", + "schema", + "tag", + "tested_baseline_version", + "version", + } + if set(value) != expected_keys or value.get("schema") != QUALIFICATION_STATUS_SCHEMA: + _fail("qualification status schema is invalid") + status = qualification_status( + tag=value.get("tag") if isinstance(value.get("tag"), str) else "", + comparison_version=( + value.get("comparison_version") + if isinstance(value.get("comparison_version"), str) + else "" + ), + tested_baseline_version=( + value.get("tested_baseline_version") + if isinstance(value.get("tested_baseline_version"), str) + else "" + ), + result=value.get("result") if isinstance(value.get("result"), str) else "", + phase=value.get("phase") if isinstance(value.get("phase"), str) else "", + reason=value.get("reason") if isinstance(value.get("reason"), str) else "", + ) + if value.get("version") != status["version"]: + _fail("qualification status version is inconsistent") + return status + + +def _qualification_stage(phase: str, reason: str, function: Callable[[], T]) -> T: + """Run one private qualification stage and expose only its closed identity.""" + + try: + return function() + except QualificationStageError: + raise + except CompatibilityError as exc: + raise QualificationStageError(str(exc), phase=phase, reason=reason) from exc + + def qualify_release( release: dict[str, Any], manifest: dict[str, Any], @@ -1255,21 +1605,36 @@ def qualify_release( ) -> dict[str, Any]: """Download, verify, execute, and classify one upstream release.""" - tag = release.get("tag_name") - if not isinstance(tag, str) or VERSION_RE.fullmatch(tag) is None: - _fail("candidate tag is not a stable semantic version") - version = tag.removeprefix("v") - assets = release_assets(release) - if os.environ.get("RUNNER_OS") != "Linux": - _fail("candidate execution requires a Linux runner; checksum validation is cross-platform") + def metadata() -> tuple[str, str, list[Asset]]: + tag = release.get("tag_name") + if not isinstance(tag, str) or VERSION_RE.fullmatch(tag) is None: + _fail("candidate tag is not a stable semantic version") + if os.environ.get("RUNNER_OS") != "Linux": + _fail( + "candidate execution requires a Linux runner; checksum validation is cross-platform" + ) + return tag, tag.removeprefix("v"), release_assets(release) + + tag, version, assets = _qualification_stage("metadata", "metadata-invalid", metadata) with tempfile.TemporaryDirectory(prefix="ocr-compat-") as temp_value: temp = Path(temp_value) - downloaded = {asset.name: _download(asset, temp) for asset in assets} - checksums = parse_checksum_file(downloaded["sha256sum.txt"]) - binaries = {asset.name: asset.sha256 for asset in assets if asset.name != "sha256sum.txt"} - if checksums != binaries: - _fail("upstream checksum file and GitHub asset digests disagree") - contracts = run_contracts(downloaded["opencodereview-linux-amd64"], version, temp) + + def artifacts() -> dict[str, Path]: + downloaded = {asset.name: _download(asset, temp) for asset in assets} + checksums = parse_checksum_file(downloaded["sha256sum.txt"]) + binaries = { + asset.name: asset.sha256 for asset in assets if asset.name != "sha256sum.txt" + } + if checksums != binaries: + _fail("upstream checksum file and GitHub asset digests disagree") + return downloaded + + downloaded = _qualification_stage("artifact", "artifact-verification-failed", artifacts) + contracts = _qualification_stage( + "contracts", + "contract-probe-failed", + lambda: run_contracts(downloaded["opencodereview-linux-amd64"], version, temp), + ) raw_notes = release.get("body") notes = raw_notes if isinstance(raw_notes, str) else "" tested_baseline = tested_baseline_version or str(manifest["recommended_version"]) @@ -1304,8 +1669,15 @@ def qualify_release( "release_changes": release_changes_excerpt(notes), "release_notes_sha256": hashlib.sha256(notes.encode()).hexdigest(), } - output.parent.mkdir(parents=True, exist_ok=True) - output.write_bytes(canonical_json(evidence)) + + def write_evidence() -> None: + try: + output.parent.mkdir(parents=True, exist_ok=True) + output.write_bytes(canonical_json(evidence)) + except OSError as exc: + raise CompatibilityError("cannot write compatibility evidence") from exc + + _qualification_stage("evidence", "evidence-write-failed", write_evidence) return evidence @@ -1649,6 +2021,34 @@ def render_workflow_issue(evidence: dict[str, Any], run_url: str) -> str: return body +def render_workflow_failure_issue(status: dict[str, Any], run_url: str) -> str: + """Render a failed attempt from closed status values, never private detail.""" + + checked = validate_qualification_status(status) + if checked["result"] != "failed": + _fail("failure issue requires a failed qualification status") + version = checked["version"] + return ( + f"\n" + f"## OCR v{version} compatibility qualification failed\n\n" + "- machine result: **failed**\n" + f"- closed phase: `{checked['phase']}`\n" + f"- closed reason: `{checked['reason']}`\n" + f"- current tested baseline: `v{checked['tested_baseline_version']}`\n" + f"- compare: https://github.com/{UPSTREAM_REPOSITORY}/compare/" + f"v{checked['comparison_version']}...v{version}\n" + f"- release: https://github.com/{UPSTREAM_REPOSITORY}/releases/tag/v{version}\n\n" + "The candidate was not classified as compatible. Raw process diagnostics are " + "not copied into this issue.\n\n" + f"### Workflow evidence\n\n- run: {run_url}\n\n" + "### Resume checklist\n\n" + "- [ ] Inspect the bounded workflow artifact and private job log.\n" + "- [ ] Correct or explicitly classify the failed compatibility boundary.\n" + "- [ ] Rerun the same candidate; update this issue instead of creating a duplicate.\n" + "- [ ] Promote only after compatible evidence and protected review.\n" + ) + + def find_qualification_issue(repository: str, marker: str) -> int | None: """Find one exact marker through bounded direct issue listing, without search indexing.""" @@ -1699,10 +2099,19 @@ def find_qualification_issue(repository: str, marker: str) -> int | None: return matches[0] if matches else None -def upsert_qualification_issue(*, repository: str, evidence: dict[str, Any], run_url: str) -> int: +def upsert_qualification_issue( + *, + repository: str, + run_url: str, + evidence: dict[str, Any] | None = None, + status: dict[str, Any] | None = None, +) -> int: """Create or update the sole issue owned by one OCR version marker.""" - version = evidence.get("version") + if (evidence is None) == (status is None): + _fail("issue upsert requires exactly one evidence or status input") + record = evidence if evidence is not None else validate_qualification_status(status or {}) + version = record.get("version") if not isinstance(version, str) or VERSION_RE.fullmatch(version) is None: _fail("qualification evidence version is invalid") expected_run_prefix = f"https://github.com/{repository}/actions/runs/" @@ -1715,7 +2124,11 @@ def upsert_qualification_issue(*, repository: str, evidence: dict[str, Any], run issue_number = find_qualification_issue(repository, marker) payload: dict[str, Any] = { "title": f"[OCR compatibility] Qualify v{version}", - "body": render_workflow_issue(evidence, run_url), + "body": ( + render_workflow_issue(evidence, run_url) + if evidence is not None + else render_workflow_failure_issue(status or {}, run_url) + ), } if issue_number is None: payload["labels"] = ["dependencies"] @@ -1767,6 +2180,7 @@ def main(argv: list[str] | None = None) -> int: qualify.add_argument("--tested-baseline-version") qualify.add_argument("--output", type=Path, required=True) qualify.add_argument("--issue-body", type=Path) + qualify.add_argument("--status-output", type=Path) assess = subparsers.add_parser("assess-chain") assess.add_argument("--evidence", type=Path, action="append", required=True) assess.add_argument("--output", type=Path, required=True) @@ -1779,11 +2193,14 @@ def main(argv: list[str] | None = None) -> int: probe_local.add_argument("--version", required=True) probe_local.add_argument("--output", type=Path, required=True) upsert_issue = subparsers.add_parser("upsert-issue") - upsert_issue.add_argument("--evidence", type=Path, required=True) + upsert_input = upsert_issue.add_mutually_exclusive_group(required=True) + upsert_input.add_argument("--evidence", type=Path) + upsert_input.add_argument("--status", type=Path) upsert_issue.add_argument("--repository", required=True) upsert_issue.add_argument("--run-url", required=True) upsert_issue.add_argument("--output-number", type=Path, required=True) args = parser.parse_args(argv) + manifest: dict[str, Any] | None = None try: manifest = load_json(args.manifest) validate_manifest(manifest, args.manifest.resolve().parents[1]) @@ -1820,13 +2237,15 @@ def main(argv: list[str] | None = None) -> int: ) print("prepared OCR compatibility update:") for path in changed: - print(path.relative_to(ROOT)) + print(path.resolve().relative_to(ROOT)) return 0 if args.command == "upsert-issue": - evidence = load_json(args.evidence) + evidence = load_json(args.evidence) if args.evidence is not None else None + status = load_json(args.status) if args.status is not None else None issue_number = upsert_qualification_issue( repository=args.repository, evidence=evidence, + status=status, run_url=args.run_url, ) args.output_number.write_text(f"{issue_number}\n", encoding="utf-8") @@ -1844,7 +2263,12 @@ def main(argv: list[str] | None = None) -> int: args.output.write_bytes(canonical_json(receipt)) print(f"local OCR {version} contract probes passed") return 0 - release = _request_json(f"{UPSTREAM_API}/releases/tags/{args.tag}") + try: + release = _request_json(f"{UPSTREAM_API}/releases/tags/{args.tag}") + except CompatibilityError as exc: + raise QualificationStageError( + str(exc), phase="metadata", reason="metadata-request-failed" + ) from exc if not isinstance(release, dict): _fail("upstream tag response must be an object") evidence = qualify_release( @@ -1856,9 +2280,41 @@ def main(argv: list[str] | None = None) -> int: ) if args.issue_body is not None: args.issue_body.write_text(render_issue(evidence), encoding="utf-8") + if args.status_output is not None: + status = qualification_status( + tag=args.tag, + comparison_version=args.comparison_version or str(manifest["recommended_version"]), + tested_baseline_version=args.tested_baseline_version + or str(manifest["recommended_version"]), + result="compatible", + phase="complete", + reason="compatible", + ) + args.status_output.write_bytes(canonical_json(status)) print(f"qualified OCR {evidence['version']}: {evidence['classification']}") return 0 except CompatibilityError as exc: + if args.command == "qualify" and args.status_output is not None: + phase = exc.phase if isinstance(exc, QualificationStageError) else "metadata" + reason = exc.reason if isinstance(exc, QualificationStageError) else "metadata-invalid" + manifest_version = ( + str(manifest["recommended_version"]) + if isinstance(manifest, dict) and "recommended_version" in manifest + else "" + ) + try: + status = qualification_status( + tag=args.tag, + comparison_version=args.comparison_version or manifest_version, + tested_baseline_version=args.tested_baseline_version or manifest_version, + result="failed", + phase=phase, + reason=reason, + ) + args.status_output.parent.mkdir(parents=True, exist_ok=True) + args.status_output.write_bytes(canonical_json(status)) + except (CompatibilityError, OSError) as status_exc: + print(f"OCR compatibility status write failed: {status_exc}", file=sys.stderr) print(f"OCR compatibility qualification failed: {exc}", file=sys.stderr) return 1 diff --git a/src/ocr_toolkit/configure.py b/src/ocr_toolkit/configure.py index c38b774..15c2f00 100644 --- a/src/ocr_toolkit/configure.py +++ b/src/ocr_toolkit/configure.py @@ -19,6 +19,9 @@ class OCRRuntimeConfigError(Exception): """OCR runtime config from CI env is invalid.""" +REVIEW_EFFORTS = frozenset({"low", "medium", "high"}) + + def _env(name: str, default: str = "") -> str: return os.environ.get(name, default).strip() @@ -27,6 +30,15 @@ def _bool_env(name: str) -> bool: return _env(name).lower() == "true" +def review_effort() -> str: + """Return the closed OCR review-effort preset, defaulting to medium.""" + + value = (_env("OCR_REVIEW_EFFORT") or "medium").lower() + if value not in REVIEW_EFFORTS: + raise OCRRuntimeConfigError("OCR_REVIEW_EFFORT must be one of: low, medium, high") + return value + + def build_config_updates() -> dict[str, Any]: """Build OCR config updates from already-normalized CI environment.""" @@ -41,6 +53,7 @@ def build_config_updates() -> dict[str, Any]: request_controls = provider.request_controls updates: dict[str, Any] = { + "effort": review_effort(), "language": review_language, "llm.url": llm_url, "llm.auth_token": llm_token, diff --git a/src/ocr_toolkit/preflight.py b/src/ocr_toolkit/preflight.py index 90dcde5..0a4e7b2 100644 --- a/src/ocr_toolkit/preflight.py +++ b/src/ocr_toolkit/preflight.py @@ -28,7 +28,7 @@ "Accept": "application/json", "User-Agent": "open-code-review-ci-preflight/1.0", } -EXPECTED_OCR_VERSION = "1.9.10" +EXPECTED_OCR_VERSION = "1.10.0" class PreflightError(Exception): diff --git a/src/ocr_toolkit/review_runner.py b/src/ocr_toolkit/review_runner.py index bbb0710..93cf277 100644 --- a/src/ocr_toolkit/review_runner.py +++ b/src/ocr_toolkit/review_runner.py @@ -1101,8 +1101,8 @@ def _immutable_review_refs(refs: ReviewRefs) -> ReviewRefs: return ReviewRefs(reader.resolve_commit(refs.base), reader.resolve_commit(refs.head)) -def _reject_owned_background(args: list[str]) -> None: - """Reject caller attempts to compete with the toolkit-owned bootstrap.""" +def _reject_owned_review_options(args: list[str]) -> None: + """Reject caller attempts to compete with toolkit-owned review artifacts.""" background_files = _option_values(args, "--background-file") backgrounds = _option_values(args, "--background") @@ -1118,6 +1118,16 @@ def _reject_owned_background(args: list[str]) -> None: raise ReviewRunnerError( "--preview is managed by ocr-ci review as a pre-model background qualification" ) + if any( + argument in {"--output", "-o"} + or argument.startswith("--output=") + or (argument.startswith("-o") and len(argument) > 2) + for argument in args + ): + raise ReviewRunnerError( + "--output is managed by ocr-ci review; OCR stdout must use the toolkit-owned " + "private result artifact" + ) def _read_bounded_artifact(path: Path, *, limit: int, label: str) -> bytes: @@ -1794,7 +1804,7 @@ def run_evidence_review( except OSError as exc: raise ReviewRunnerError("OCR private pre-execution state is unsafe") from exc refs = _immutable_review_refs(_review_refs(ocr_args)) - _reject_owned_background(ocr_args) + _reject_owned_review_options(ocr_args) _prepare_review_output_artifacts(result_path, stderr_path) print("OCR evidence preflight: collecting immutable review refs", file=sys.stderr) previous_home = os.environ.get("HOME") diff --git a/tests/test_environment_contract.py b/tests/test_environment_contract.py index 1eb8bdb..2f7ee41 100644 --- a/tests/test_environment_contract.py +++ b/tests/test_environment_contract.py @@ -29,6 +29,7 @@ "OCR_LLM_MAX_COMPLETION_TOKENS": "Unset (inherits OCR)", "OCR_ANTHROPIC_DISABLE_THINKING": "false", "OCR_REVIEW_LANGUAGE": "English", + "OCR_REVIEW_EFFORT": "medium", "OCR_LLM_VALIDATE_MODEL": "false", "OCR_LLM_MODELS_URL": "Derived from `OCR_LLM_URL`", "OCR_LLM_ALLOWED_MODELS": "Empty list", @@ -68,8 +69,8 @@ } EXAMPLE_DEFAULTS = { - "OCR_VERSION": "v1.9.10", - "OCR_SHA256": "359e5bafda1438a47ef389399f4994350e1016371eac1dc17a2c428acb228e6c", + "OCR_VERSION": "v1.10.0", + "OCR_SHA256": "f8f99ea071bed77dbcaa15fdd2083287bb8ae408d5928b3943ebe0788d191b6b", "OCR_TOOLKIT_VERSION": STABLE_TOOLKIT_VERSION, "OCR_TOOLKIT_CHECKSUMS_URL": "Release URL derived from `OCR_TOOLKIT_VERSION`", "OCR_TOOLKIT_WHEEL": "open_code_review_toolkit-${OCR_TOOLKIT_VERSION}-py3-none-any.whl", @@ -85,6 +86,25 @@ "Names declared by MCP `headers_from`", } +REQUIRED_DISPLAY_NAMES = { + "OCR_LLM_URL", + "OCR_LLM_TOKEN", + "OCR_LLM_MODEL", + "GITLAB_API_TOKEN", + "CI_API_V4_URL", + "CI_SERVER_URL", + "CI_PROJECT_ID", + "CI_MERGE_REQUEST_IID", + "CI_MERGE_REQUEST_SOURCE_BRANCH_SHA", + "CI_MERGE_REQUEST_DIFF_BASE_SHA", + "CI_PIPELINE_SOURCE", + "OCR_VERSION", + "OCR_SHA256", + "OCR_TOOLKIT_VERSION", + "OCR_TOOLKIT_CHECKSUMS_URL", + *DYNAMIC_INPUTS, +} + REDACTION_ONLY = { "OCR_LLM_AUTH_TOKEN", "OPENAI_API_KEY", @@ -104,6 +124,8 @@ def _display_cell(raw: str) -> str: value = raw.strip() + if value.startswith("**") and value.endswith("**"): + value = value[2:-2] if value.startswith("`") and value.endswith("`") and value.count("`") == 2: return value[1:-1] return value @@ -185,6 +207,21 @@ def test_documented_environment_tables_are_complete_and_exact() -> None: assert documented_names.isdisjoint(REMOVED_PUBLIC_INPUTS | {"OCR_USE_ANTHROPIC"}) +def test_required_environment_inputs_are_visually_distinct() -> None: + """Bold only names whose table scope requires a supplied or predefined value.""" + + configuration = (PROJECT_ROOT / "docs" / "configuration.md").read_text(encoding="utf-8") + bold_names: set[str] = set() + for line in configuration.splitlines(): + if not line.startswith("|"): + continue + first_cell = line.strip("|").split("|", maxsplit=1)[0].strip() + if first_cell.startswith("**") and first_cell.endswith("**"): + bold_names.add(_display_cell(first_cell)) + + assert bold_names == REQUIRED_DISPLAY_NAMES + + def test_source_environment_inventory_matches_the_documented_contract() -> None: source_names = _literal_source_environment_names() # CI_PIPELINE_SOURCE belongs solely to the public example's GitLab rules. @@ -214,6 +251,7 @@ def test_runtime_defaults_match_the_documented_contract(monkeypatch: pytest.Monk try: updates = configure.build_config_updates() assert updates["llm.protocol"] == "openai" + assert updates["effort"] == "medium" assert updates["llm.auth_header"] == "Authorization" assert updates["telemetry.enabled"] is False assert updates["telemetry.content_logging"] is False @@ -261,11 +299,12 @@ def test_example_local_defaults_match_the_pipeline() -> None: encoding="utf-8" ) for name, value in { - "OCR_VERSION": "v1.9.10", + "OCR_VERSION": "v1.10.0", "OCR_SHA256": EXAMPLE_DEFAULTS["OCR_SHA256"], "OCR_TOOLKIT_VERSION": STABLE_TOOLKIT_VERSION, "OCR_MAX_TOOLS": "30", "OCR_MAX_TOKENS_BUDGET": "0", + "OCR_REVIEW_EFFORT": "medium", }.items(): assert f'{name}: "{value}"' in workflow assert ( diff --git a/tests/test_installed_policy_e2e.py b/tests/test_installed_policy_e2e.py index f6c68b6..42728e9 100644 --- a/tests/test_installed_policy_e2e.py +++ b/tests/test_installed_policy_e2e.py @@ -161,6 +161,7 @@ def test_installed_wheel_and_sdist_expose_target_policy_through_real_mcp( generated_config = json.loads( (config_home / ".opencodereview" / "config.json").read_text(encoding="utf-8") ) + assert generated_config["effort"] == "medium" assert generated_config["llm"]["url"] == "https://gateway.example/v1" assert generated_config["llm"]["extra_body"] == {"max_completion_tokens": 4096} protocol_environment = { diff --git a/tests/test_integration_contracts.py b/tests/test_integration_contracts.py index 87b3805..da7b620 100644 --- a/tests/test_integration_contracts.py +++ b/tests/test_integration_contracts.py @@ -128,6 +128,7 @@ def test_gitlab_example_preserves_review_gating_and_manual_self_test() -> None: assert workflow.index(" - lint") < workflow.index(" - ai_review") assert 'OCR_LLM_VALIDATE_MODEL: "false"' in workflow assert 'OCR_MAX_TOKENS_BUDGET: "0"' in workflow + assert 'OCR_REVIEW_EFFORT: "medium"' in workflow assert 'OCR_MAX_TOOLS: "30"' in workflow assert '--max-tools "${OCR_MAX_TOOLS:-30}"' in review_job assert '--max-tokens-budget "${OCR_MAX_TOKENS_BUDGET:-0}"' in review_job @@ -171,6 +172,8 @@ def test_gitlab_docs_match_the_current_review_surface() -> None: assert "OCR_MAX_TOKENS_BUDGET" in workflow assert "OCR_MAX_TOOLS" in configuration assert "OCR_MAX_TOOLS" in workflow + assert "OCR_REVIEW_EFFORT" in configuration + assert "OCR_REVIEW_EFFORT" in workflow assert "--preserve-private-artifacts" in configuration assert "without a posting receipt" in configuration assert "rejects this flag before OCR execution" in configuration diff --git a/tests/test_ocr_compat.py b/tests/test_ocr_compat.py index 721a989..dd17082 100644 --- a/tests/test_ocr_compat.py +++ b/tests/test_ocr_compat.py @@ -43,8 +43,8 @@ def test_committed_manifest_is_valid_and_has_recommended_tested_baseline() -> No module.validate_manifest(manifest, PROJECT_ROOT) - assert manifest["recommended_version"] == "1.9.10" - assert manifest["monitoring_floor"] == "1.9.10" + assert manifest["recommended_version"] == "1.10.0" + assert manifest["monitoring_floor"] == "1.10.0" assert [(item["version"], item["status"]) for item in manifest["releases"]] == [ ("1.7.17", "tested"), ("1.8.0", "tested"), @@ -69,6 +69,7 @@ def test_committed_manifest_is_valid_and_has_recommended_tested_baseline() -> No ("1.9.8", "tested"), ("1.9.9", "tested"), ("1.9.10", "tested"), + ("1.10.0", "tested"), ] @@ -153,9 +154,9 @@ def test_discovery_filters_known_prerelease_and_old_versions() -> None: def test_discovery_pages_until_the_monitoring_floor() -> None: module = load_script() manifest = module.load_json(MANIFEST) - first_page = [release("1.9.11")] + first_page = [release("1.10.1")] first_page.extend({"draft": True} for _ in range(module.MAX_RELEASES_PER_PAGE - 1)) - second_page = [release("1.9.10")] + second_page = [release("1.10.0")] requested: list[str] = [] def fake_request(url: str) -> list[dict[str, Any]]: @@ -165,14 +166,14 @@ def fake_request(url: str) -> list[dict[str, Any]]: with patched_attr(module, "_request_json", fake_request): unseen = module.discover_unseen(manifest) - assert [item["tag_name"] for item in unseen] == ["v1.9.11"] + assert [item["tag_name"] for item in unseen] == ["v1.10.1"] assert len(requested) == 2 def test_discovery_fails_when_bounded_pages_do_not_reach_floor() -> None: module = load_script() manifest = module.load_json(MANIFEST) - page = [release("1.9.11")] + page = [release("1.10.1")] page.extend({"draft": True} for _ in range(module.MAX_RELEASES_PER_PAGE - 1)) with patched_attr(module, "_request_json", lambda _url: page): @@ -217,14 +218,14 @@ def test_qualification_matrix_accepts_the_next_manual_patch() -> None: module = load_script() manifest = module.load_json(MANIFEST) - matrix = module.qualification_matrix(manifest, [release("1.9.11")]) + matrix = module.qualification_matrix(manifest, [release("1.10.1")]) assert matrix == { "include": [ { - "comparison_version": "1.9.10", - "tag": "v1.9.11", - "tested_baseline_version": "1.9.10", + "comparison_version": "1.10.0", + "tag": "v1.10.1", + "tested_baseline_version": "1.10.0", } ] } @@ -365,6 +366,80 @@ def test_issue_body_uses_stable_marker_and_safe_release_changes() -> None: assert "current tested baseline: `v1.7.16`" in body +def test_failed_qualification_status_is_closed_and_consistent() -> None: + """Public failure status accepts only reviewed enum values and exact keys.""" + + module = load_script() + status = module.qualification_status( + tag="v1.10.0", + comparison_version="1.9.10", + tested_baseline_version="1.9.10", + result="failed", + phase="contracts", + reason="contract-probe-failed", + ) + + assert module.validate_qualification_status(status) == status + with pytest.raises(module.CompatibilityError, match="schema is invalid"): + module.validate_qualification_status({**status, "raw_error": "token=/private/path"}) + with pytest.raises(module.CompatibilityError, match="inconsistent"): + module.validate_qualification_status({**status, "result": "compatible"}) + + +@pytest.mark.parametrize( + ("phase", "reason"), + [ + ("complete", "contract-probe-failed"), + ("artifact", "metadata-invalid"), + ("contracts", "evidence-write-failed"), + ("evidence", "artifact-verification-failed"), + ], +) +def test_failed_qualification_status_rejects_impossible_stage_pairs( + phase: str, reason: str +) -> None: + """Fail closed when individually valid status enums contradict each other.""" + + module = load_script() + + with pytest.raises(module.CompatibilityError, match="phase and reason are inconsistent"): + module.qualification_status( + tag="v1.10.0", + comparison_version="1.9.10", + tested_baseline_version="1.9.10", + result="failed", + phase=phase, + reason=reason, + ) + + +def test_failed_qualification_issue_contains_no_private_diagnostics() -> None: + """The issue renderer projects closed status without a raw diagnostic field.""" + + module = load_script() + status = module.qualification_status( + tag="v1.10.0", + comparison_version="1.9.10", + tested_baseline_version="1.9.10", + result="failed", + phase="contracts", + reason="contract-probe-failed", + ) + + body = module.render_workflow_failure_issue( + status, + "https://github.com/synthetic/repository/actions/runs/123", + ) + + assert "" in body + assert "closed phase: `contracts`" in body + assert "closed reason: `contract-probe-failed`" in body + assert "actions/runs/123" in body + assert "Raw process diagnostics are not copied" in body + assert "token=" not in body + assert "/private/" not in body + + def test_optional_capabilities_validate_additive_llm_identity() -> None: module = load_script() @@ -382,6 +457,43 @@ def test_optional_capabilities_validate_additive_llm_identity() -> None: module.detect_optional_capabilities("review", {"llm": {"model": ""}}) +def test_optional_capabilities_validate_effort_and_semantic_groups() -> None: + """Optional capabilities remain additive, bounded, and path-complete.""" + + module = load_script() + sample = { + "groups": [ + { + "label": "application flow", + "files": ["src/app.py", "tests/test_app.py"], + } + ] + } + + capabilities = module.detect_optional_capabilities("review --effort low", sample) + + assert capabilities == ["review_effort", "semantic_grouping"] + module._validate_file_groups(sample["groups"], {"src/app.py", "tests/test_app.py"}) + + +def test_semantic_groups_reject_duplicate_or_unexpected_paths() -> None: + """One file cannot silently appear in multiple model-produced groups.""" + + module = load_script() + duplicate = [ + {"label": "one", "files": ["src/app.py"]}, + {"label": "two", "files": ["src/app.py"]}, + ] + + with pytest.raises(module.CompatibilityError, match="invalid grouped path"): + module._validate_file_groups(duplicate) + with pytest.raises(module.CompatibilityError, match="expected paths"): + module._validate_file_groups( + [{"label": "one", "files": ["src/app.py"]}], + {"src/app.py", "tests/test_app.py"}, + ) + + def test_complete_chain_requires_every_release_to_be_automatic_safe() -> None: module = load_script() manifest = module.load_json(MANIFEST) @@ -525,6 +637,128 @@ def request( assert "fix: synthetic parser correction" in calls[0][2]["body"] +def test_failed_qualification_upsert_updates_the_canonical_issue() -> None: + """A failed attempt reopens and updates the one stable candidate issue.""" + + module = load_script() + calls: list[tuple[str, str, dict[str, Any] | None]] = [] + status = module.qualification_status( + tag="v1.10.0", + comparison_version="1.9.10", + tested_baseline_version="1.9.10", + result="failed", + phase="contracts", + reason="contract-probe-failed", + ) + + def request( + url: str, *, method: str = "GET", payload: dict[str, Any] | None = None + ) -> dict[str, Any]: + calls.append((url, method, payload)) + return {"number": 135} + + with ( + patched_attr(module, "find_qualification_issue", lambda _repo, _marker: 135), + patched_attr(module, "_issue_api_request", request), + ): + number = module.upsert_qualification_issue( + repository="synthetic/repository", + status=status, + run_url="https://github.com/synthetic/repository/actions/runs/123", + ) + + assert number == 135 + assert calls[0][0].endswith("/issues/135") + assert calls[0][1] == "PATCH" + payload = calls[0][2] + assert payload is not None + assert payload["state"] == "open" + assert "contract-probe-failed" in payload["body"] + assert "token=" not in payload["body"] + + +def test_qualify_cli_retains_closed_status_when_private_probe_fails( + tmp_path: Path, capsys: pytest.CaptureFixture[str] +) -> None: + """The CLI records safe status while stderr retains the private diagnostic.""" + + module = load_script() + output = tmp_path / "evidence.json" + status_output = tmp_path / "status.json" + private_detail = "probe token=secret path=/private/workspace/result.json" + + def fail_probe(*_args: object, **_kwargs: object) -> dict[str, Any]: + raise module.QualificationStageError( + private_detail, + phase="contracts", + reason="contract-probe-failed", + ) + + with ( + patched_attr(module, "_request_json", lambda _url: release("1.10.0")), + patched_attr(module, "qualify_release", fail_probe), + ): + result = module.main( + [ + "--manifest", + str(MANIFEST), + "qualify", + "--tag", + "v1.10.0", + "--comparison-version", + "1.9.10", + "--tested-baseline-version", + "1.9.10", + "--output", + str(output), + "--status-output", + str(status_output), + ] + ) + + captured = capsys.readouterr() + status = json.loads(status_output.read_text(encoding="utf-8")) + assert result == 1 + assert status["phase"] == "contracts" + assert status["reason"] == "contract-probe-failed" + assert status["result"] == "failed" + assert private_detail not in status_output.read_text(encoding="utf-8") + assert private_detail in captured.err + assert not output.exists() + + +def test_qualify_cli_retains_status_when_manifest_validation_fails(tmp_path: Path) -> None: + """Explicit baseline inputs keep closed status available before manifest load.""" + + module = load_script() + manifest = tmp_path / "invalid-manifest.json" + status_output = tmp_path / "status.json" + manifest.write_text("{}\n", encoding="utf-8") + + result = module.main( + [ + "--manifest", + str(manifest), + "qualify", + "--tag", + "v1.10.0", + "--comparison-version", + "1.9.10", + "--tested-baseline-version", + "1.9.10", + "--output", + str(tmp_path / "evidence.json"), + "--status-output", + str(status_output), + ] + ) + + status = json.loads(status_output.read_text(encoding="utf-8")) + assert result == 1 + assert status["phase"] == "metadata" + assert status["reason"] == "metadata-invalid" + + def test_qualification_requires_exact_supported_asset_matrix() -> None: module = load_script() candidate = release("1.7.18") @@ -539,6 +773,40 @@ def test_manifest_cli_validate() -> None: assert module.main(["--manifest", str(MANIFEST), "validate"]) == 0 +def test_prepare_update_cli_prints_relative_manifest_path( + tmp_path: Path, + monkeypatch: pytest.MonkeyPatch, + capsys: pytest.CaptureFixture[str], +) -> None: + """A repository-relative manifest remains printable after a successful update.""" + + module = load_script() + evidence = tmp_path / "evidence.json" + evidence.write_text("{}\n", encoding="utf-8") + monkeypatch.chdir(PROJECT_ROOT) + relative_manifest = MANIFEST.relative_to(PROJECT_ROOT) + + with patched_attr( + module, + "prepare_update", + lambda **_kwargs: [relative_manifest], + ): + result = module.main( + [ + "--manifest", + str(relative_manifest), + "prepare-update", + "--evidence", + str(evidence), + "--fragment-number", + "135", + ] + ) + + assert result == 0 + assert str(relative_manifest) in capsys.readouterr().out + + def test_evidence_json_is_canonical() -> None: module = load_script() payload = module.canonical_json({"z": 1, "a": 2}) @@ -900,11 +1168,11 @@ def test_prepare_update_rejects_human_review_candidate(tmp_path: Path) -> None: module = load_script() evidence = { "schema_version": 2, - "version": "1.9.11", + "version": "1.10.1", "result": "compatible", "classification": "human-review-required", - "comparison_version": "1.9.10", - "tested_baseline_version": "1.9.10", + "comparison_version": "1.10.0", + "tested_baseline_version": "1.10.0", } with pytest.raises(module.CompatibilityError, match="bounded conclusion"): @@ -920,11 +1188,11 @@ def test_prepare_update_requires_human_review_for_minor_transition() -> None: module = load_script() evidence = { "schema_version": 2, - "version": "1.10.0", + "version": "1.11.0", "result": "compatible", "classification": "automatic-safe", - "comparison_version": "1.9.10", - "tested_baseline_version": "1.9.10", + "comparison_version": "1.10.0", + "tested_baseline_version": "1.10.0", } with pytest.raises(module.CompatibilityError, match="explicit human review"): @@ -936,7 +1204,7 @@ def test_prepare_update_requires_human_review_for_minor_transition() -> None: ) -@pytest.mark.parametrize("version", ["1.10.0", "2.0.0"]) +@pytest.mark.parametrize("version", ["1.11.0", "2.0.0"]) def test_prepare_update_rejects_schema_one_minor_or_major_transition(version: str) -> None: """Legacy evidence cannot prove a chain across a semantic-version boundary.""" @@ -962,11 +1230,11 @@ def test_prepare_update_rejects_nonadjacent_minor_transition() -> None: module = load_script() evidence = { "schema_version": 2, - "version": "1.11.0", + "version": "1.12.0", "result": "compatible", "classification": "human-review-required", - "comparison_version": "1.9.4", - "tested_baseline_version": "1.9.4", + "comparison_version": "1.10.0", + "tested_baseline_version": "1.10.0", } with pytest.raises(module.CompatibilityError, match="contiguous release sequence"): @@ -974,7 +1242,7 @@ def test_prepare_update_rejects_nonadjacent_minor_transition() -> None: manifest_path=MANIFEST, evidence=evidence, fragment_number=73, - human_conclusions={"1.11.0": "Synthetic reviewed conclusion."}, + human_conclusions={"1.12.0": "Synthetic reviewed conclusion."}, root=PROJECT_ROOT, ) @@ -983,11 +1251,11 @@ def test_prepare_update_rejects_conclusion_outside_evidence_chain() -> None: module = load_script() evidence = { "schema_version": 2, - "version": "1.9.11", + "version": "1.10.1", "result": "compatible", "classification": "automatic-safe", - "comparison_version": "1.9.10", - "tested_baseline_version": "1.9.10", + "comparison_version": "1.10.0", + "tested_baseline_version": "1.10.0", } with pytest.raises(module.CompatibilityError, match="only evidence versions"): @@ -995,7 +1263,7 @@ def test_prepare_update_rejects_conclusion_outside_evidence_chain() -> None: manifest_path=MANIFEST, evidence=evidence, fragment_number=72, - human_conclusions={"1.10.0": "Synthetic unrelated conclusion."}, + human_conclusions={"1.10.2": "Synthetic unrelated conclusion."}, root=PROJECT_ROOT, ) @@ -1007,11 +1275,11 @@ def test_prepare_update_rejects_invalid_optional_reviewed_conclusion( module = load_script() evidence = { "schema_version": 2, - "version": "1.9.11", + "version": "1.10.1", "result": "compatible", "classification": "automatic-safe", - "comparison_version": "1.9.10", - "tested_baseline_version": "1.9.10", + "comparison_version": "1.10.0", + "tested_baseline_version": "1.10.0", } with pytest.raises(module.CompatibilityError, match="bounded plain text"): @@ -1019,7 +1287,7 @@ def test_prepare_update_rejects_invalid_optional_reviewed_conclusion( manifest_path=MANIFEST, evidence=evidence, fragment_number=72, - human_conclusions={"1.9.11": conclusion}, + human_conclusions={"1.10.1": conclusion}, root=PROJECT_ROOT, ) diff --git a/tests/test_operations_docs.py b/tests/test_operations_docs.py index 04d439f..31b659c 100644 --- a/tests/test_operations_docs.py +++ b/tests/test_operations_docs.py @@ -10,6 +10,7 @@ GITLAB_EXAMPLE = PROJECT_ROOT / "examples" / "gitlab" / "ocr-review.gitlab-ci.yml" GITLAB_EXAMPLES = PROJECT_ROOT / "examples" / "gitlab" CODE_OF_CONDUCT = PROJECT_ROOT / "CODE_OF_CONDUCT.md" +SIGNAL_OWNERSHIP = PROJECT_ROOT / "docs" / "engineering" / "review_signal_ownership.md" def test_readme_security_badges_link_to_repository_specific_results() -> None: @@ -67,6 +68,7 @@ def test_documentation_indexes_route_to_canonical_owners() -> None: for phrase in ( "toolkit_strategy.md", "project_principles.md", + "review_signal_ownership.md", "m5_context_contracts.md", "evidence_migration_matrix.md", "test_evidence_matrix.md", @@ -79,6 +81,29 @@ def test_documentation_indexes_route_to_canonical_owners() -> None: assert "docs/README.md" in readme +def test_review_signal_audit_keeps_group_data_outside_toolkit_authority() -> None: + """Keep the completed BL-017 ownership and privacy conclusion explicit.""" + + audit = SIGNAL_OWNERSHIP.read_text(encoding="utf-8") + backlog = (PROJECT_ROOT / "docs" / "codex" / "TASKS_BACKLOG.md").read_text(encoding="utf-8") + roadmap = (PROJECT_ROOT / "ROADMAP.md").read_text(encoding="utf-8") + + for phrase in ( + "Source-to-signal matrix", + "Group labels are model-produced", + "sorted changed paths", + "no exporter of its own", + "concludes `no-new-layer`", + "BL-016 remains parked", + "BL-018 remains conditional", + "BL-019 and", + "BL-020 retain", + ): + assert phrase in audit + assert "Review measurement gaps (BL-017) | Completed and removed" in backlog + assert "M6 Profiles and quality measurement | Established / conditional" in roadmap + + def test_community_conduct_policy_has_a_private_enforcement_route() -> None: """Keep conduct reports private and separate from public issue intake.""" @@ -402,6 +427,8 @@ def test_threat_model_covers_remote_finding_image_boundary() -> None: def test_ocr_compatibility_workflow_is_bounded_and_protected() -> None: + """Qualification retains bounded failure evidence without hiding a red job.""" + workflow = (PROJECT_ROOT / ".github" / "workflows" / "ocr-compatibility.yml").read_text( encoding="utf-8" ) @@ -430,15 +457,24 @@ def test_ocr_compatibility_workflow_is_bounded_and_protected() -> None: assert "git switch -C" in workflow assert "git push --force-with-lease" in workflow assert workflow.count("upsert-issue") == 1 + assert "continue-on-error: true" in workflow + assert "--status-output /tmp/ocr-compat/status.json" in workflow + assert "if: ${{ !cancelled() }}" in workflow + assert "if: ${{ always() && !cancelled() }}" in workflow + assert "if: steps.qualify.outcome == 'failure'" in workflow + assert "run: exit 1" in workflow assert "gh issue create" not in workflow assert 'f"{fragment_number}.maintenance.md"' in qualifier assert 'f"{fragment_number}.feature.md"' not in qualifier for contract in ( "OCR 1.9.9 — inherited predecessor", - "OCR 1.9.10 — toolkit 0.8.0 target", + "OCR 1.9.10 — toolkit 0.8.0 target and 0.8.2 predecessor", + "OCR 1.10.0 — toolkit 0.8.2 target", "ocr.llm-retry-report/v1", "not toolkit telemetry", - "Deploy toolkit 0.8.0 directly with OCR 1.9.10", + "Deploy toolkit 0.8.2 directly with OCR 1.10.0", + "max_completion_tokens=16384", + "do not install OCR 1.9.10 as an intermediate step", ): assert contract in policy assert "--search" not in workflow diff --git a/tests/test_posting_approval.py b/tests/test_posting_approval.py index 5504329..d2b034c 100644 --- a/tests/test_posting_approval.py +++ b/tests/test_posting_approval.py @@ -414,6 +414,26 @@ def test_missing_or_malformed_v5_receipt_fails_closed(self) -> None: "the review-time approval receipt is missing or invalid", ) + def test_group_diagnostics_cannot_extend_the_closed_receipt(self) -> None: + """Reject OCR-owned group or round fields if they enter the approval receipt.""" + + for field, value in ( + ("groups", [{"label": "core", "files": ["src/core.py"]}]), + ("review_rounds", 2), + ): + receipt = receipt_v5() + receipt[field] = value + + decision = approval.evaluate_approval_policy( + settings.BooleanSetting(True), complete_outcome(), [], [], 0, receipt + ) + + self.assertFalse(decision.eligible) + self.assertEqual( + decision.result.reason, + "the review-time approval receipt is missing or invalid", + ) + def test_impossible_v5_capability_and_evidence_states_fail_closed(self) -> None: cases: list[dict[str, Any]] = [] diff --git a/tests/test_release_notes.py b/tests/test_release_notes.py index 6111c17..bd117f0 100644 --- a/tests/test_release_notes.py +++ b/tests/test_release_notes.py @@ -140,6 +140,38 @@ def test_081_release_notes_separate_added_fixed_removed_and_unchanged_contracts( assert contract in notes +def test_082_fragments_are_actionable_for_people_and_deployment_agents() -> None: + """Keep added, changed, inherited, fixed, and migration outcomes distinct.""" + + fragments = "\n".join( + (ROOT / "changelog.d" / name).read_text(encoding="utf-8") + for name in ( + "135.maintenance.md", + "135.feature.md", + "135.bugfix.md", + "136.bugfix.md", + "137.maintenance.md", + ) + ) + + for phrase in ( + "OCR 1.9.10 — inherited predecessor", + "max_completion_tokens=58888", + "OCR 1.10.0 — changed target", + "OCR_REVIEW_EFFORT", + "exact default is `medium`", + "max_completion_tokens=16384", + "Deploy toolkit 0.8.2 directly with OCR 1.10.0", + "do not install OCR 1.9.10 as an intermediate step", + "Caller `--output`/`-o` remains unsupported", + "compatibility-status/v1", + "job returns red", + "no-new-layer", + "No exporter", + ): + assert phrase in fragments + + def test_extracts_only_the_exact_release_section() -> None: changelog = "# Changelog\n\n## 0.2.0 - later\n\nnew\n\n## 0.1.0 - now\n\nfirst\n" diff --git a/tests/test_review_runner.py b/tests/test_review_runner.py index 38ee4ac..d00cdf6 100644 --- a/tests/test_review_runner.py +++ b/tests/test_review_runner.py @@ -1064,6 +1064,90 @@ def test_stage_grouped_retry_report_is_private_and_approval_projection_neutral() assert projected["comments"] == payload["comments"] +def test_review_groups_remain_private_and_cannot_change_approval(tmp_path: Path) -> None: + """Keep additive group and round diagnostics outside every approval authority.""" + + composition = MCPComposition( + payload={}, + capabilities=(MCPCapability("ocr_toolkit_evidence", ("ocr_toolkit_evidence",), True),), + external_servers=(), + secret_values=(), + ) + identity = replace(DEFAULT_IDENTITY, mr_author_id=41) + base_payload: dict[str, object] = { + "status": "complete", + "comments": [], + "warnings": [], + "tool_calls": {"total": 1, "by_tool": {"ocr_toolkit_evidence": 1}}, + "manifest": { + "schema_version": "ocr.run-manifest/v1", + "operation": "review", + "terminal_state": "complete", + "coverage": { + "selected": [{"item_id": "review-a"}], + "completed": [{"item_id": "review-a"}], + "reused": [], + "failed": [], + "waived": [], + }, + }, + } + baseline_projection = review_runner._canonical_result_projection(base_payload) + + for name, groups, expected_state in ( + ( + "safe", + [{"label": "Core behavior", "files": ["src/core.py"], "rounds": 2}], + "passed", + ), + ( + "private", + [ + { + "label": "Contact private@example.invalid", + "files": ["Authorization: Bearer private-group-token"], + "rounds": 2, + } + ], + "private-sanitized", + ), + ): + result = tmp_path / f"{name}.json" + result.write_text( + json.dumps({**base_payload, "groups": groups, "review_rounds": 2}), + encoding="utf-8", + ) + + _usage, blocked, publication = review_runner._finalize_ocr_result( + result, + composition, + identity, + None, + forbidden=(), + ) + + persisted = json.loads(result.read_text(encoding="utf-8")) + receipt = persisted["_ocr_toolkit"] + assert blocked is False + assert publication["state"] == expected_state + assert review_runner._canonical_result_projection(persisted) == baseline_projection + assert "groups" not in receipt + assert "review_rounds" not in receipt + decision = approval.evaluate_approval_policy( + settings.BooleanSetting(True), + parse_result_outcome(persisted), + persisted["comments"], + persisted["warnings"], + 0, + receipt, + ) + assert decision.eligible is True + + private_serialized = (tmp_path / "private.json").read_text(encoding="utf-8") + assert "private@example.invalid" not in private_serialized + assert "private-group-token" not in private_serialized + + def test_warning_objects_are_conservatively_publication_relevant() -> None: payload: dict[str, object] = { "status": "success", @@ -1964,8 +2048,10 @@ def test_immutable_ref_rewrite_preserves_non_diff_ocr_options() -> None: "--format", "json", "--max-comments=20", + "--effort", + "high", ] - ) == ["--format", "json", "--max-comments=20"] + ) == ["--format", "json", "--max-comments=20", "--effort", "high"] @pytest.mark.parametrize( @@ -1979,13 +2065,13 @@ def test_immutable_ref_rewrite_preserves_non_diff_ocr_options() -> None: ) def test_review_rejects_caller_owned_background(arguments: list[str]) -> None: with pytest.raises(review_runner.ReviewRunnerError, match="managed by ocr-ci"): - review_runner._reject_owned_background(arguments) + review_runner._reject_owned_review_options(arguments) @pytest.mark.parametrize("option", ["--background", "--background-file"]) def test_review_rejects_missing_caller_background_value(option: str) -> None: with pytest.raises(review_runner.ReviewRunnerError, match="requires a value"): - review_runner._reject_owned_background([option]) + review_runner._reject_owned_review_options([option]) @pytest.mark.parametrize("argument", ["--preview", "-p", "--preview=true"]) @@ -1993,7 +2079,24 @@ def test_review_rejects_caller_owned_preview(argument: str) -> None: """Reserve preview for the toolkit's installed-OCR background gate.""" with pytest.raises(review_runner.ReviewRunnerError, match="pre-model"): - review_runner._reject_owned_background([argument]) + review_runner._reject_owned_review_options([argument]) + + +@pytest.mark.parametrize( + "arguments", + [ + ["--output", "/tmp/result.json"], + ["--output=/tmp/result.json"], + ["-o", "/tmp/result.json"], + ["-o=/tmp/result.json"], + ["-o/tmp/result.json"], + ], +) +def test_review_rejects_caller_owned_output(arguments: list[str]) -> None: + """Reserve every OCR output spelling for the toolkit result descriptor.""" + + with pytest.raises(review_runner.ReviewRunnerError, match="managed by ocr-ci"): + review_runner._reject_owned_review_options(arguments) @pytest.mark.parametrize( diff --git a/tests/test_runtime_helpers.py b/tests/test_runtime_helpers.py index 4959543..8949138 100644 --- a/tests/test_runtime_helpers.py +++ b/tests/test_runtime_helpers.py @@ -218,6 +218,7 @@ def test_external_server_requires_explicit_tool_allowlist(self) -> None: def test_runtime_config_defaults_review_language_to_english(self) -> None: with ( cleared_env("OCR_REVIEW_LANGUAGE"), + cleared_env("OCR_REVIEW_EFFORT"), patched_env( OCR_LLM_URL="https://gateway.example/v1/chat/completions", OCR_LLM_TOKEN="llm-secret", @@ -227,6 +228,41 @@ def test_runtime_config_defaults_review_language_to_english(self) -> None: updates = ocr_configure.build_config_updates() self.assertEqual(updates["language"], "English") + self.assertEqual(updates["effort"], "medium") + + def test_runtime_config_accepts_closed_review_effort_presets(self) -> None: + """Only upstream's three stable presets enter the generated root config.""" + + for value in ("low", "medium", "high", " HIGH "): + with ( + self.subTest(value=value), + patched_env( + OCR_LLM_URL="https://gateway.example/v1/chat/completions", + OCR_LLM_TOKEN="llm-secret", + OCR_LLM_MODEL="openai/gpt-test", + OCR_REVIEW_EFFORT=value, + ), + ): + updates = ocr_configure.build_config_updates() + + self.assertEqual(updates["effort"], value.strip().lower()) + + def test_runtime_config_rejects_unknown_review_effort(self) -> None: + """Unknown effort never falls through to an OCR-owned implicit default.""" + + with ( + patched_env( + OCR_LLM_URL="https://gateway.example/v1/chat/completions", + OCR_LLM_TOKEN="llm-secret", + OCR_LLM_MODEL="openai/gpt-test", + OCR_REVIEW_EFFORT="extreme", + ), + self.assertRaisesRegex( + ocr_configure.OCRRuntimeConfigError, + "OCR_REVIEW_EFFORT must be one of", + ), + ): + ocr_configure.build_config_updates() def test_runtime_config_rejects_non_https_llm_url_before_storing_token(self) -> None: with patched_env( @@ -1440,7 +1476,7 @@ def test_invalid_json_error_does_not_echo_secret_payload(self) -> None: class PreflightTests(unittest.TestCase): def test_validate_ocr_binary_accepts_supported_version(self) -> None: completed = subprocess.CompletedProcess( - args=["ocr", "--version"], returncode=0, stdout="ocr 1.9.10\n", stderr="" + args=["ocr", "--version"], returncode=0, stdout="ocr 1.10.0\n", stderr="" ) with ( patched_attr(preflight.shutil, "which", lambda _name: "/usr/bin/ocr"),