diff --git a/PLANS.md b/PLANS.md index ca171a2..6a0703c 100644 --- a/PLANS.md +++ b/PLANS.md @@ -4,4 +4,336 @@ Use this file for active or blocked repository work. Update it before implementa ## Active Work -No active or blocked repository work. +### Toolkit 0.8.4 - GitLab summary correctness and OCR 1.10.1 + +#### Goal + +Deliver toolkit `0.8.4` through the complete protected stable-release lifecycle: +finish and independently review the existing Draft feature PR, run one final +local OCR `1.10.1` review, correct confirmed findings, merge the exact hosted- +green feature head, prepare and merge `release/v0.8.4`, and independently +reconcile the published artifacts, provenance, immutable receipt, issues, and +milestone. + +#### Plan Origin + +`plan_mode_approved` + +#### Requested Scope + +- Resolve #145 by separating four GitLab summary inputs: OCR coverage, + publication integrity, ordinary findings/warnings, and a toolkit-owned OCR + core advisory. +- Resolve #146 by recording hosted qualification evidence for OCR `1.10.1`, + promoting the exact version and checksums, documenting relevant upstream + behavior, and updating the direct local OCR binary without changing user + configuration. +- Permit HTAB only in published `existing_code` and `suggestion_code` values + while preserving all remaining publication-DLP checks. +- Avoid duplicating one published finding in `Recommended focus areas` while + preserving the deterministic ranking for two or more findings. +- Preserve the current tool-call and token technical-summary format and emit + those independent non-empty metrics under passed, private-sanitized, and + publication-filtered states. +- Finish with verified stable `0.8.4` artifacts and provenance, Actions-owned + receipts on #145/#146, both issues closed, milestone `v0.8.4` closed, and a + clean local `main` synchronized with `origin/main`. + +#### Requirement Traceability + +| Requirement | Owner | Implementation evidence | Acceptance evidence | +| --- | --- | --- | --- | +| OCR 1.10.1 compatibility and local update | #146 | compatibility evidence, manifest/preflight/example pins, current-version docs, Rules and Maintenance changelog fragments | hosted run `32955196785`, exact checksum validation, isolated no-LLM version/help/background/rule checks | +| Closed OCR advisory | #145 | strict private `ocr.toolkit-advisory/v1` parser/projection and Technical details renderer | spoof/malformed/duplicate/extra-key/no-receipt and approval-independence regressions | +| Correct publication-filtered coverage | #145 | derive original coverage from validated receipt v5 `publication.original`; keep publication integrity separate | complete 5/5 filtering, real partial/budget, no pathless fallback regressions | +| Field-bounded HTAB support | #145 | allow HTAB only for `existing_code` and `suggestion_code` through private and public publication DLP | tab preservation plus secrets/PII/laundering/control-character regressions | +| Non-duplicating reviewer guide | #145 | omit focus-area ranking for one published finding | one- and two-finding renderer regressions | +| Operational handoff | #145, #146 | Draft body and issue checklists with exact head/tree, checksums, validation, Added/Fixed/Changed/Unchanged | hosted-green Draft status, clean merge state, zero unresolved threads, remote/worktree readback | + +#### Explicit Non-Goals + +- No additional OCR qualification campaign or enriched-context qualification; + run only the user-authorized final local OCR review on this repository. +- No receipt v5, publication-DLP signal v2, result, manifest, telemetry, or + approval-contract version changes. +- No weakening of global context DLP or publication controls outside the two + exact code-value fields. +- No adoption of upstream GitHub Action, delegate skill, npm launcher, + provider preset, or upstream repository-local `providers.go` rule. +- No mechanical test-directory reorganization or refactor unrelated to the + two activated issues. +- No consumer-repository, B2B, `core/common`, or shared-template integration. +- No issue or milestone closure before independent stable-release readback. + +#### Constraints + +- Release classification: `release-required`; target stable version: `0.8.4`; + delivery state: `active stable delivery`. +- Branch: `codex/v0.8.4-summary-ocr-1.10.1`, based on clean released `v0.8.3` + `main`; `.next-version` already owns `0.8.4`. +- The first tracked repository write is this complete active plan. +- After the signed planning commit, make one initial push and open the Draft PR; + make no further pushes until all local implementation slices are complete. +- Every logical signed commit requires focused tests, complete slice diff + self-review, trust/data-flow review, and `git diff --check`. +- New tests stay with existing thematic owners and include docstrings. +- Do not change OCR configuration, credentials, or the user's `HOME`. +- The final local OCR review uses the already configured local OCR `1.10.1`, + concurrency `2`, no provider-specific `4096` completion cap, and private + ignored artifacts. Its exit status is insufficient without complete result + and manifest readback. +- Keep long-running test and OCR output in ignored owner-only logs and expose + only bounded summaries in the interactive session. +- Run the full local gate after the final fixes; hosted workflows own the + cross-platform, dependency, Security, and CodeQL validation, while local + release closure additionally owns deterministic package and clean-install + evidence. + +#### Inputs And Sources + +- GitHub issues #145 and #146 and compatibility run `32955196785`. +- OCR `1.10.1` release assets and checksums: + - Linux amd64: `8b806c221d409727a21611b4a7952d8e15edadbbc25f5affccaeb8f677e4055c`. + - Darwin arm64: `8fc24bd825c9d918b894be05c0cf27fac8d30bc549257c812d87337167c7563c`. + - `sha256sum.txt`: `ec72bda51f1227f412ee00602d952868efc57d847cce0ae1586fb97069d4139d`. +- Current public contracts in `docs/configuration.md`, `docs/gitlab.md`, + `docs/operations.md`, `docs/security.md`, and `docs/compatibility.md`. +- Runtime owners under `src/ocr_toolkit/` and their existing thematic tests. + +#### User Decisions And Answers + +- Complete and publish stable toolkit `0.8.4` in this task. +- Run one final local OCR `1.10.1` review, correct confirmed findings, perform a + holistic self-review, then push and move the exact Draft head through the + protected release process. +- Preserve the existing technical-summary format and publish tool-call/token + numeric metrics only when their list/value is non-empty and non-zero. +- Default `OCR_REVIEW_EFFORT` remains `medium`. +- Highlight required environment variables in public documentation where + applicable. +- Avoid duplicate local validation already owned by hosted PR workflows. + +#### Completed Baseline State + +- `main` and `origin/main` both resolve to released `v0.8.3` commit + `4c697fee6eeceb02a50fbed1c150a6eb953a08d6` with a clean worktree. +- `.next-version` contains `0.8.4`; `.release-version` contains `0.8.3`. +- Hosted OCR compatibility run `32955196785` passed the required result, + completion-cap, medium-effort, and max-tools probes for `1.10.1`. +- The direct local OCR binary is `1.10.0`; no Homebrew-managed OCR package is + installed. +- Upstream semantic audit identified bounded session cache keys and `.m` + MATLAB/Objective-C rule resolution as consumed behavior; result/manifest, + completion cap `16384`, explicit cap `4096`, medium effort, and max-tools + semantics remain unchanged. + +#### Current Work Queue + +1. **Plan and Draft coordination - completed** + - Create milestone `v0.8.4`, assign #145/#146 to `xeonvs`, add both issues to + the milestone, commit this plan, push once, and open the Draft PR. +2. **OCR 1.10.1 qualification - completed locally** + - Import canonical evidence, human conclusion, manifest/preflight/example + pins, current-version docs, and separate Maintenance/Rules fragments. + - Atomically update the direct local binary with checksum verification and + rollback on validation failure; run isolated no-LLM checks. +3. **Closed advisory contract - completed locally** + - Remove the accepted background advisory from OCR warnings; validate and + attach a private toolkit-owned numeric advisory after publication DLP; + render it only with a valid receipt in Technical details. +4. **Coverage, publication DLP, and reviewer guide - completed locally** + - Use `publication.original` for filtered coverage; suppress the legacy + warning fallback in filtered state; correct DLP wording; permit HTAB only + in the two code fields; omit focus areas for one finding. +5. **Independent feature review and final OCR - active** + - Review the complete Draft diff and trust/data flows, run focused and full + validation with bounded output, then execute the single final local OCR + `1.10.1` review with concurrency `2`. + - Inspect the OCR result and manifest for complete selected-item coverage; + trace and fix only confirmed findings, rerun their owner tests, and record + the private-safe conclusions without publishing raw provider artifacts. + - The final exact-range run completed all 10 selected items at + `02c2f9d8f76d736ba83deed7700bed9374c4e38d` with no failed, reused, or + waived coverage and produced three confirmed boundary corrections: make + receipt-v5 original outcome/count validation exhaustive; preserve the + original HTAB-bearing code value for secret and forbidden matching while + relaxing only its control-character admission; and carry a valid + receipt-bound OCR core advisory into failed-result Technical details. +6. **Holistic self-review and feature delivery** + - Review the complete post-OCR diff, correct findings, run the final + quality/coverage, lock, manifest, Towncrier, Gitleaks, deterministic-build, + clean-install, and diff gates, then push the exact reviewed head. + - Verify hosted checks and unresolved threads on that head, move #147 to + Ready, and squash-merge it through the protected branch policy. +7. **Release PR and stable publication** + - Verify the protected-main development publication, prepare + `release/v0.8.4` with the canonical release metadata, changelog, issue set, + and plan archive, then validate and squash-merge the exact release head. + - Monitor the Release workflow and independently verify TestPyPI/PyPI/GitHub + bytes, PEP 740 provenance, GitHub attestations, annotated tag target, + immutable Release, receipt identities, and clean Python 3.12-3.14 installs. +8. **External closure** + - Verify Actions-owned receipts, close #145/#146 and milestone `v0.8.4` only + after external reconciliation, re-read planning sources, synchronize clean + local `main`, and remove temporary logs, archives, and unused environments. + +#### Locked Decisions + +- Data flow for the advisory is + `exact preview stderr -> strict parser -> numeric toolkit state -> closed renderer -> Technical details`. +- The accepted advisory is not an OCR warning, DLP input, coverage signal, + receipt input, telemetry field, or approval signal. +- Raw OCR output may not supply the reserved `_ocr_toolkit_advisory` key. +- Advisory schema is exact `ocr.toolkit-advisory/v1` with kind + `background_recommended_limit`, bounded positive non-boolean `actual` and + `recommended`, `actual > recommended`, and unit `characters`. +- Without a valid receipt v5 the advisory is untrusted; malformed, extended, + duplicated, or spoofed forms fail closed. +- Complete OCR coverage plus publication filtering renders + `Review complete with publication filtering`; real failed/budget/partial + coverage retains its existing higher-priority state. +- Publication filtering always blocks auto-approval; a valid advisory alone + does not. +- HTAB is allowed only in values of `comments[*].existing_code` and + `comments[*].suggestion_code`; all secret, PII, forbidden-value, laundering, + budget, and other control-character checks remain active. +- OCR `1.10.1` promotion is Maintenance; MATLAB/Objective-C rule effects are a + separate Rules entry. Historical `1.10.0` records remain immutable. + +#### Verification + +- Per slice: focused owner tests, complete diff review, requirement and + trust/data-flow review, `git diff --check`, signed commit. +- OCR checks: compatibility evidence/manifest validation, exact checksum and + documentation consistency, isolated `--version`/`--help`, soft/hard + background preview, and MATLAB/Objective-C rule resolution without LLM. +- #145 regression matrix: both reported scenarios; advisory spoof/malformed/ + duplicate/extra-key/no-receipt; approval independence; complete filtered and + actual partial/budget coverage; field-specific HTAB and remaining DLP + controls; one/two finding guide; tool-call/token summaries across publication + states. +- One final local gate: `scripts/quality.sh check`, coverage floors, + `uv lock --check`, `scripts/ocr_compat.py validate`, Towncrier draft, + `scripts/gitleaks.sh`, and `git diff --check`. +- Hosted Draft PR: build artifacts, Twine, clean installs, OS/Python matrix, + dependency checks, Security, and CodeQL. + +#### Latest Validation Results + +- Baseline branch/worktree check: clean `main` at + `4c697fee6eeceb02a50fbed1c150a6eb953a08d6`. +- Hosted OCR compatibility run `32955196785`: successful. +- OCR `1.10.1` compatibility slice: 146 focused tests passed; support manifest + and Towncrier draft validated. +- The direct local Darwin arm64 binary now reports OCR `1.10.1` and matches + SHA-256 `8fc24bd825c9d918b894be05c0cf27fac8d30bc549257c812d87337167c7563c`; + upstream `sha256sum.txt` matches + `ec72bda51f1227f412ee00602d952868efc57d847cce0ae1586fb97069d4139d`. +- Isolated no-LLM version/help, hosted/local contract, soft/hard background, + and MATLAB/Objective-C rule-resolution checks passed without changing user + configuration or `HOME`. +- Closed advisory slice: 360 focused tests and Ruff passed. Raw/duplicate/ + malformed/unbound advisory input fails closed; the valid numeric projection + remains outside OCR warnings, publication DLP, coverage, receipt v5, + telemetry, and approval inputs and renders only in Technical details. +- Coverage/publication slice: 342 focused tests and 209 subtests passed. The + complete 5/5 filtered scenario retains its original coverage counts without + legacy pathless failures; real partial/budget state remains stronger; + passed/private-sanitized/publication-filtered tool and token lines remain + independent; field-specific HTAB, hostile controls and remaining DLP checks, + one/two-finding guide behavior, and impossible receipt counts are covered. +- Final quality gate passed: Ruff format/lint, mypy, Bandit, and the complete + pytest run produced 86% combined coverage. Risk groups passed at 85% + (result/preflight), 82% (posting/GitLab transaction), 86% + (review/context/DLP/approval), and 87% (MCP/provider/policy/result). +- `uv lock --check`, OCR support-manifest validation, Towncrier 0.8.4 draft, + and `git diff --check` passed. The rendered draft has separate Bug Fixes + (#145), Maintenance (#146), and Rules (#146) sections with explicit + deployment and unchanged-contract guidance. +- The original product implementation and all three final-OCR corrections are + complete locally. Focused posting/review regressions pass 343 tests plus 217 + subtests; shared DLP callers pass 66 tests plus 8 subtests; Ruff and strict + MyPy pass. The release remains active pending the final holistic gates, push, + hosted CI reconciliation, Draft transition, and protected release lifecycle. +- The single authorized final local OCR `1.10.1` review completed the exact + `origin/main..02c2f9d8f76d736ba83deed7700bed9374c4e38d` range in 6m23s with + concurrency `2`, complete 10/10 coverage, no failed/reused/waived items, + no stderr, and 102 tool calls. Its three candidates were all confirmed as + bounded correctness or trust-boundary defects and are the active corrective + scope above; the retained private result and DLP sidecar remain local-only + remediation evidence and will be deleted after verification. +- Final holistic gates passed after the corrective slice: Gitleaks 8.24.3 + scanned the complete `origin/main..HEAD` feature range; `pip-audit + --skip-editable` found no known dependency vulnerabilities; the focused + public-contract suite passed 25 tests; and `git diff --check` passed. +- Two independent local `0.8.4` builds with one fixed source epoch were + byte-identical. Twine accepted both wheel/sdist pairs, archive inspection + found no private/log/Git/environment/key paths, and clean wheel plus sdist + installations passed `ocr-ci --help` on Python 3.12, 3.13, and 3.14. +- Holistic documentation review replaced the remaining obsolete + tab-normalized-copy wording with the implemented unchanged-value DLP + contract. Required variables remain bold in the canonical tables, and the + example toolkit pin correctly stays at the currently published `0.8.3` + until the release PR advances it. + +#### Risks And Recovery + +- **Advisory spoofing or privilege confusion:** reject reserved input before + toolkit projection; exact-schema validation and receipt-gated rendering fail + closed. Revert the advisory slice if the boundary cannot be proven. +- **Coverage conflation:** use only already-validated receipt v5 original counts + for filtered publication; preserve real partial/budget outcomes. Revert the + summary slice if legacy result compatibility regresses. +- **DLP weakening:** make the allowance field-aware and tab-only; retain every + semantic scanner. Any secret/PII/control regression blocks the commit. +- **Local OCR replacement:** verify release checksum and existing source binary, + stage an adjacent replacement atomically, retain a rollback copy until all + no-LLM checks pass, and restore `1.10.0` on failure. +- **Hosted-only failure:** diagnose from exact job evidence, make a focused + reviewed fix commit, rerun the affected local owner check, and push only the + evidence-driven correction. + +#### Resume Point + +Current action: sign and push the complete post-OCR corrective head, verify the +hosted checks and review threads on that exact head, then move Draft PR #147 to +Ready and squash-merge it through the protected branch policy. + +#### Plan Fidelity Check + +- The active queue maps every accepted requirement to #145 or #146 and retains + all approved trust, DLP, approval, compatibility, and release boundaries. +- No requested product behavior is deferred silently; stable publication alone + is intentionally deferred. +- Scope additions require an explicit plan update before implementation. + +#### Reconciliation Check + +- Before final push, compare implementation, tests, docs, changelog, manifest, + examples, and issue acceptance criteria against this plan. +- After hosted CI, verify the exact remote head/tree, all required checks, + unresolved review threads, Draft state, merge state, and local cleanliness. + +#### Closure Gate + +This objective is complete only when the exact protected release is externally +reconciled: registry and Release bytes agree, provenance and attestations +validate, the annotated tag and immutable receipt bind the reviewed release, +Python 3.12-3.14 clean installs pass, Actions-owned issue receipts exist, +#145/#146 and milestone `v0.8.4` are closed, and local `main` is clean and +synchronized. Earlier feature, merge, development-publication, or workflow-green +states are intermediate only. + +#### Post-Close Delivery + +No post-close repository delivery remains in this objective. Conditional backlog +work stays inactive unless its own trigger and authorization are met. + +#### Handoff Notes + +- Draft PR #147 may move to Ready only after the final OCR fixes, holistic + self-review, exact-head local gates, push, and hosted reconciliation. +- #145, #146, and milestone `v0.8.4` remain open until the stable receipt and + independent external readback prove closure. +- OCR `1.10.1` is the qualified toolkit target; the previous `1.10.0` evidence + remains historical and must not be rewritten. diff --git a/README.md b/README.md index 9a74ef7..b8f23d2 100644 --- a/README.md +++ b/README.md @@ -22,7 +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. +The current OCR 1.10.1 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/changelog.d/145.bugfix.md b/changelog.d/145.bugfix.md new file mode 100644 index 0000000..7fec936 --- /dev/null +++ b/changelog.d/145.bugfix.md @@ -0,0 +1,6 @@ +Correct GitLab review summaries by keeping OCR coverage, publication integrity, findings, and OCR core advisories as independent signals. + +- **Added:** accepted OCR background recommendations are projected as a receipt-bound numeric `ocr.toolkit-advisory/v1` line under Technical details, including failed-result summaries; they no longer become OCR warnings or approval blockers. +- **Fixed:** complete OCR coverage followed by publication filtering now renders `Review complete with publication filtering`, preserves only exhaustively validated original outcome/count combinations, and does not invent partial-coverage or pathless failed-item diagnostics. +- **Changed:** horizontal tabs are retained only in `existing_code` and `suggestion_code` after the unchanged value passes all secret, PII, forbidden-value, laundering, and budget checks, and Recommended focus areas are shown only for two or more published findings. +- **Unchanged:** publication filtering remains warning-bearing and automatic-approval-ineligible; receipt v5, publication-DLP signal v2, manifest/result schemas, secret/PII/laundering/budget checks, ordinary OCR warnings, real partial/budget outcomes, and the existing non-zero tool-call/token summary format remain authoritative. diff --git a/changelog.d/146.maintenance.md b/changelog.d/146.maintenance.md new file mode 100644 index 0000000..e24a454 --- /dev/null +++ b/changelog.d/146.maintenance.md @@ -0,0 +1,8 @@ +Qualify and promote checksum-pinned Open Code Review 1.10.1 for toolkit 0.8.4. + +- **Added:** canonical qualification evidence records the hosted OCR 1.10.1 result, completion-cap, medium-effort, and max-tools probes. +- **Changed:** preflight, the compatibility manifest, and the GitLab example now require OCR 1.10.1. SHA-256 pins are Linux AMD64 `8b806c221d409727a21611b4a7952d8e15edadbbc25f5affccaeb8f677e4055c`, Darwin ARM64 `8fc24bd825c9d918b894be05c0cf27fac8d30bc549257c812d87337167c7563c`, and upstream `sha256sum.txt` `ec72bda51f1227f412ee00602d952868efc57d847cce0ae1586fb97069d4139d`. +- **Consumed:** bounded private session-cache keys reduce upstream cache-key risk without adding a toolkit receipt or telemetry field. +- **Unchanged:** result and manifest schemas, inherited OpenAI completion cap `16384`, explicit `4096` override, default `medium` effort, and behaviorally qualified max-tools semantics remain unchanged. +- **Not consumed:** the upstream GitHub Action, delegate skill, npm launcher, provider preset, and repository-local `providers.go` rule do not alter the toolkit's native-binary, explicit-protocol integration. +- **Deployment:** update directly from OCR 1.10.0 to 1.10.1; no intermediate OCR version is required. diff --git a/changelog.d/146.rules.md b/changelog.d/146.rules.md new file mode 100644 index 0000000..14acb34 --- /dev/null +++ b/changelog.d/146.rules.md @@ -0,0 +1 @@ +OCR 1.10.1 adds `.m` files to its review allowlist and selects MATLAB or Objective-C built-in rules from file content. The upstream repository-local `providers.go` rule is not a toolkit-wide rule. diff --git a/compatibility/evidence/ocr-1.10.1.json b/compatibility/evidence/ocr-1.10.1.json new file mode 100644 index 0000000..dda813d --- /dev/null +++ b/compatibility/evidence/ocr-1.10.1.json @@ -0,0 +1,204 @@ +{ + "assets": [ + { + "name": "opencodereview-darwin-amd64", + "sha256": "259dd57e09ed326ac001ecdb0798296ac06a1ca1ebfd5a55588c4647d2eb75f0", + "size": 57633504 + }, + { + "name": "opencodereview-darwin-arm64", + "sha256": "8fc24bd825c9d918b894be05c0cf27fac8d30bc549257c812d87337167c7563c", + "size": 54677666 + }, + { + "name": "opencodereview-linux-amd64", + "sha256": "8b806c221d409727a21611b4a7952d8e15edadbbc25f5affccaeb8f677e4055c", + "size": 55890082 + }, + { + "name": "opencodereview-linux-arm64", + "sha256": "681d9fb93352f56439b1ac9c64e2a62b1b3c20e753d6ad69dd02f4bd1baa7f3b", + "size": 52625570 + }, + { + "name": "opencodereview-windows-amd64.exe", + "sha256": "66f495d48ee6fb155c9812ba8e8d650e1a8ad34be9cca174831cea1b7151f6e3", + "size": 57351680 + }, + { + "name": "opencodereview-windows-arm64.exe", + "sha256": "b3b0628d61ca980705b2c6eb3166c264af3ce1eac839e1786fe5a968491abf91", + "size": 53334016 + }, + { + "name": "sha256sum.txt", + "sha256": "ec72bda51f1227f412ee00602d952868efc57d847cce0ae1586fb97069d4139d", + "size": 572 + } + ], + "classification": "human-review-required", + "classification_reasons": [ + "release notes contain a material or ambiguous compatibility signal" + ], + "comparison_version": "1.10.0", + "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" + }, + "numeric_cli_probe": { + "max_tokens_budget": { + "cases": { + "invalid_below": { + "effective": null, + "input": -1, + "outcome": "rejected" + }, + "minimum": { + "effective": 1, + "input": 1, + "outcome": "accepted" + }, + "omitted": { + "effective": "unlimited", + "input": null, + "outcome": "accepted" + }, + "representative": { + "effective": 30000, + "input": 30000, + "outcome": "accepted" + }, + "sentinel": { + "effective": "unlimited", + "input": 0, + "outcome": "accepted" + } + }, + "maximum": null, + "owner": "ocr-cli" + }, + "max_tools": { + "cases": { + "invalid_below": { + "effective": null, + "input": -1, + "outcome": "rejected" + }, + "minimum": { + "effective": 100, + "input": 50, + "outcome": "accepted" + }, + "minimum_minus_one": { + "effective": 100, + "input": 49, + "outcome": "normalized", + "reported_normalization": 50 + }, + "omitted": { + "effective": 100, + "input": null, + "outcome": "accepted" + }, + "representative": { + "effective": 101, + "input": 101, + "outcome": "accepted" + }, + "sentinel": { + "effective": 100, + "input": 0, + "outcome": "accepted" + } + }, + "maximum": null, + "owner": "ocr-template-or-higher-cli", + "reported_minimum": 50 + }, + "result": "passed" + }, + "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", + "--max-tools", + "--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-26T08:51:59Z", + "release_changes": "## 🚀 Features\n\n- feat(rules): add Objective-C-specific review rules (#1061)\n- feat(allowlist): add matlab support (#574)\n- feat(rules): add project review rule for providers.go (#1057)\n\n## 🐛 Bug Fixes\n\n- fix(llm): bound session task key length (#1009)\n- fix(skill): handle delegate CLI version skew and background limits (#1047)\n- fix(action): forward review timeout and complete LLM config (#1051)\n- fix(llm): serve GPT-5.6 models via the OpenAI Responses API (#559) (#938)\n- fix(cli): exit 128+signo when the native binary is killed by a signal (#1054)\n\n## 📖 Documentation\n\n- docs(ko): add Korean translation for getting-started pages (#993)\n\n## Other Changes\n\n- ci(workflows): run the contract suite in a dedicated workflow (#1058)\n\n**Full Changelog**: https://github.com/alibaba/open-code-review/compare/v1.10.0...v1.10.1", + "release_notes_sha256": "890f24192f1ccf427d65baa7970721dcef50b933cbcd3777f3843e95dfc64d43", + "result": "compatible", + "schema_version": 2, + "tag": "v1.10.1", + "tested_baseline_version": "1.10.0", + "upstream_repository": "alibaba/open-code-review", + "version": "1.10.1" +} diff --git a/compatibility/ocr-support.json b/compatibility/ocr-support.json index 4b009c4..a0890b6 100644 --- a/compatibility/ocr-support.json +++ b/compatibility/ocr-support.json @@ -1,6 +1,6 @@ { - "monitoring_floor": "1.10.0", - "recommended_version": "1.10.0", + "monitoring_floor": "1.10.1", + "recommended_version": "1.10.1", "releases": [ { "assets": [ @@ -1187,6 +1187,59 @@ "release_url": "https://github.com/alibaba/open-code-review/releases/tag/v1.10.0", "status": "tested", "version": "1.10.0" + }, + { + "assets": [ + { + "name": "opencodereview-darwin-amd64", + "sha256": "259dd57e09ed326ac001ecdb0798296ac06a1ca1ebfd5a55588c4647d2eb75f0", + "size": 57633504 + }, + { + "name": "opencodereview-darwin-arm64", + "sha256": "8fc24bd825c9d918b894be05c0cf27fac8d30bc549257c812d87337167c7563c", + "size": 54677666 + }, + { + "name": "opencodereview-linux-amd64", + "sha256": "8b806c221d409727a21611b4a7952d8e15edadbbc25f5affccaeb8f677e4055c", + "size": 55890082 + }, + { + "name": "opencodereview-linux-arm64", + "sha256": "681d9fb93352f56439b1ac9c64e2a62b1b3c20e753d6ad69dd02f4bd1baa7f3b", + "size": 52625570 + }, + { + "name": "opencodereview-windows-amd64.exe", + "sha256": "66f495d48ee6fb155c9812ba8e8d650e1a8ad34be9cca174831cea1b7151f6e3", + "size": 57351680 + }, + { + "name": "opencodereview-windows-arm64.exe", + "sha256": "b3b0628d61ca980705b2c6eb3166c264af3ce1eac839e1786fe5a968491abf91", + "size": 53334016 + }, + { + "name": "sha256sum.txt", + "sha256": "ec72bda51f1227f412ee00602d952868efc57d847cce0ae1586fb97069d4139d", + "size": 572 + } + ], + "capabilities": [ + "llm_result_identity", + "per_run_model_override", + "per_run_provider_override", + "review_effort", + "semantic_grouping" + ], + "evidence": "compatibility/evidence/ocr-1.10.1.json", + "evidence_sha256": "3377626cbb65f868f397c22da1f6594b13dc2a22b761ae8a2fc267fcde9b4785", + "human_conclusion": "Compatible after semantic review for #146. The toolkit consumes bounded private session cache keys and .m language resolution with MATLAB and Objective-C rules. Result and manifest contracts, inherited completion cap 16384, explicit cap 4096, medium effort, and max-tools semantics remain unchanged. The upstream GitHub Action, delegate skill, npm launcher, built-in provider preset, and repository-local providers.go rule are not consumed because the toolkit uses the native binary with explicit protocol and configuration.", + "published_at": "2026-08-26T08:51:59Z", + "release_url": "https://github.com/alibaba/open-code-review/releases/tag/v1.10.1", + "status": "tested", + "version": "1.10.1" } ], "schema_version": 1, diff --git a/docs/compatibility.md b/docs/compatibility.md index 00d4e49..a71432e 100644 --- a/docs/compatibility.md +++ b/docs/compatibility.md @@ -39,7 +39,7 @@ Deploy toolkit 0.8.0 or 0.8.1 directly with OCR 1.9.10. The historical public Li ### OCR 1.10.0 — toolkit 0.8.2 and 0.8.3 target -OCR 1.10.0 is the only version accepted by toolkit 0.8.2 and 0.8.3 preflight and pinned by the current 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`. The toolkit 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. +OCR 1.10.0 was the only version accepted by toolkit 0.8.2 and 0.8.3 preflight and pinned by their published 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`. The toolkit 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). @@ -49,6 +49,18 @@ Its max-tools CLI reports a minimum and normalization target of `50`, while the Deploy toolkit 0.8.2 or 0.8.3 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. +### OCR 1.10.1 — toolkit 0.8.4 target + +OCR 1.10.1 is the only version accepted by toolkit 0.8.4 preflight and pinned by the current GitLab example. Hosted Linux and independent Darwin arm64 qualification preserve the OCR result and `ocr.run-manifest/v1` contracts, inherited OpenAI completion cap `16384`, explicit `4096` override, default-medium two-round behavior, and effective max-tools values from 1.10.0. The `--max-tools` help text advertises a lower minimum, but actual CLI behavior still normalizes `49` to `50`, the embedded template keeps effective `100`, and explicit `101` raises it; the toolkit uses those behaviorally observed values and keeps the example on sentinel `0`. + +Toolkit 0.8.4 reduces an exact accepted soft background diagnostic to the private numeric `ocr.toolkit-advisory/v1` projection after publication DLP. It is rendered only with a valid receipt v5 in GitLab Technical details and does not enter OCR warnings, coverage, DLP counts, telemetry, or automatic-approval inputs. Raw OCR output cannot supply the reserved field; duplicate, extended, unknown, malformed, or unbound forms fail closed. + +The release bounds private session task/cache keys to 64 characters. Those values remain OCR-owned private routing data and do not enter toolkit telemetry, receipt v5, findings, fingerprints, lifecycle commands, or approval. OCR also adds `.m` to its reviewable allowlist and resolves MATLAB versus Objective-C from file content so the corresponding built-in rules can be selected. The upstream repository-local `providers.go` rule is not part of the toolkit-wide rules contract. + +The upstream GitHub Action, delegate skill, npm launcher, and built-in provider preset are not consumed. The toolkit downloads the native binary in its GitLab example and writes an explicit protocol and provider configuration, so these changes add no implicit provider selection or CI execution path. + +Deploy toolkit 0.8.4 directly with OCR 1.10.1. The public Linux amd64 example verifies SHA-256 `8b806c221d409727a21611b4a7952d8e15edadbbc25f5affccaeb8f677e4055c`; the local Darwin arm64 artifact verifies `8fc24bd825c9d918b894be05c0cf27fac8d30bc549257c812d87337167c7563c`, and the upstream `sha256sum.txt` verifies `ec72bda51f1227f412ee00602d952868efc57d847cce0ae1586fb97069d4139d`. + 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. An automatic-safe result is not an automatic stable release. It must still pass a normal protected compatibility PR and a separate signed stable-release PR. If a dedicated OCR update bot credential is not configured, the workflow publishes the exact patch as an artifact and records the resume action in the issue; the default `GITHUB_TOKEN` is intentionally not used to create a PR that would fail to trigger the full protected workflow set. diff --git a/docs/configuration.md b/docs/configuration.md index 6fc0762..7dbe3b9 100644 --- a/docs/configuration.md +++ b/docs/configuration.md @@ -22,7 +22,7 @@ These are the complete supported toolkit-owned runtime inputs. `Required` is sco | `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 1.10.0 spans may include path-derived group keys and model-produced labels. | +| `OCR_TELEMETRY_ENABLED` | Operator / `ocr-ci configure` | No | `false` | Exact `true` enables OCR telemetry configuration; OCR 1.10.1 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. | @@ -60,11 +60,11 @@ If `OCR_LLM_EXTRA_BODY` already owns that field, an exactly equal JSON integer i The toolkit 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. +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 both OCR 1.10.0 and 1.10.1 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 an 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. +`OCR_REVIEW_EFFORT` defaults to `medium` and is written to OCR's root `effort` configuration key. OCR 1.10.1 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. @@ -92,8 +92,8 @@ 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.10.0` | Checksum-pinned recommended OCR binary release for toolkit 0.8.3. | -| **`OCR_SHA256`** | Example pipeline | Yes | `f8f99ea071bed77dbcaa15fdd2083287bb8ae408d5928b3943ebe0788d191b6b` | Expected Linux AMD64 OCR binary digest. | +| **`OCR_VERSION`** | Example pipeline | Yes | `v1.10.1` | Checksum-pinned recommended OCR binary release for toolkit 0.8.4. | +| **`OCR_SHA256`** | Example pipeline | Yes | `8b806c221d409727a21611b4a7952d8e15edadbbc25f5affccaeb8f677e4055c` | Expected Linux AMD64 OCR binary digest. | | **`OCR_TOOLKIT_VERSION`** | Example pipeline | Yes | `0.8.3` | 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. | @@ -213,7 +213,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. 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. +`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 diagnostic becomes a toolkit-authored numeric `ocr.toolkit-advisory/v1` value only after publication DLP and appears in the bounded CI log plus GitLab Technical details. It is not an OCR warning, receipt or DLP input, coverage signal, telemetry field, or approval blocker. 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. @@ -229,7 +229,7 @@ Evidence-store schema v4 retains v1-v3 readback and adds a distinct immutable po In `metadata` mode, GitLab MR acquisition normalizes only title, description, labels, optional source branch, and the reviewed source SHA into `review.merge_request_context/v1`. Values are complete-field bounded, NFC-normalized, control-stripped, redacted, source-head-bound invocation data. Raw values never enter bootstrap, argv, environment, diagnostics, or receipts; bootstrap lists only field statuses and toolkit-authored comparison guidance. In `off` mode none of those mutable text fields reaches normalization or persistence. OCR may treat matching intent as evidence against an assumption-dependent concern, contradictory intent as mismatch evidence, and missing intent as unknown. The source-branch hint is weaker than an explicit description and cannot establish rollout intent by itself. Metadata cannot authorize tools, policy, suppression, posting, or approval. In `enriched`, references are extracted only from admitted metadata and admitted discussion bodies; adapters authorize them before local handle minting. There is no generic URL, identifier, search, or provider-tool path in the model loop. -The review step writes exact closed receipt v5 inside the private result only after cleanup and the inode-checked atomic publication transformation. It binds source and policy SHA, merge-request author ID where applicable, context mode/state/classes, per-source completeness/degradation, admitted-mutable state, the complete bounded MCP capability inventory, positive known-server and fixed context-tool usage, mandatory-evidence state, publication-DLP result, and cleanup result. The current closed states are `passed`, `private-sanitized`, and `publication-filtered`. A pure canonical projection covers the normalized outcome/message, ordered allowlisted finding fields and warnings, manifest coverage/failure details, displayed tool counters, normalized token telemetry, omission/completeness, and approval inputs. Token telemetry has a closed provider-neutral vocabulary: input, output, cached as a subset of input, reasoning as a subset of output, optional validated total, and mathematically derived other; malformed or contradictory telemetry is unavailable and unknown provider keys are never published. Private sanitization may retain the original complete result and continue through existing approval gates only when that projection is byte-equivalent before and after sanitization. Any changed, malformed, or incomparable projection becomes a safe partial `publication-filtered` result with closed retained/omitted/original counts; it preserves the previous review and cannot authorize approval. Receipt v1-v4 has no posting or approval compatibility. The later GitLab posting step reads v5 instead of rebuilding context or MCP facts from a possibly changed environment. Its summary omits configured-but-unused servers and all zero counters; the receipt/event never stores rejected text/locations, provider/context text, upstream IDs, server URLs, commands, setup, arguments, headers, tool inputs/results, credentials, or repository contents. +The review step writes exact closed receipt v5 inside the private result only after cleanup and the inode-checked atomic publication transformation. It binds source and policy SHA, merge-request author ID where applicable, context mode/state/classes, per-source completeness/degradation, admitted-mutable state, the complete bounded MCP capability inventory, positive known-server and fixed context-tool usage, mandatory-evidence state, publication-DLP result, and cleanup result. The current closed states are `passed`, `private-sanitized`, and `publication-filtered`. A pure canonical projection covers the normalized outcome/message, ordered allowlisted finding fields and warnings, manifest coverage/failure details, displayed tool counters, normalized token telemetry, omission/completeness, and approval inputs. Token telemetry has a closed provider-neutral vocabulary: input, output, cached as a subset of input, reasoning as a subset of output, optional validated total, and mathematically derived other; malformed or contradictory telemetry is unavailable and unknown provider keys are never published. Private sanitization may retain the original complete result and continue through existing approval gates only when that projection is byte-equivalent before and after sanitization. Any changed, malformed, or incomparable projection becomes a safe `publication-filtered` result with closed retained/omitted/original counts; it preserves the previous review and cannot authorize approval. The public projection may be incomplete while its validated original coverage counts still prove a complete OCR run, so the GitLab summary reports those dimensions separately and never feeds filtered warnings into legacy failed-item inference. Horizontal tab is admitted only in the closed `existing_code` and `suggestion_code` finding fields and remains subject to every non-control DLP check; tabs in all other fields and every other control/format character remain invalid. Receipt v1-v4 has no posting or approval compatibility. The later GitLab posting step reads v5 instead of rebuilding context or MCP facts from a possibly changed environment. Its summary omits configured-but-unused servers and all zero counters; the receipt/event never stores rejected text/locations, provider/context text, upstream IDs, server URLs, commands, setup, arguments, headers, tool inputs/results, credentials, or repository contents. ### Accepted project decisions diff --git a/docs/engineering/m5_context_contracts.md b/docs/engineering/m5_context_contracts.md index ddda08b..13d10a8 100644 --- a/docs/engineering/m5_context_contracts.md +++ b/docs/engineering/m5_context_contracts.md @@ -53,7 +53,7 @@ The allowed field vocabulary is closed per generic projection. `model`, `publish When a validated GitLab merge request introduces its configured repository-owned OCR rules path, the source candidate still cannot become policy. If the exact normalized path is absent at both the immutable diff base and captured protected-target policy commit, but exists at the exact source head as a regular blob within the Git reader's byte limit, `review` stops before OCR and atomically writes `ocr.pre-execution-status/v2`. Source contents are not read or validated for this classification. A path that existed at the diff base, an absolute operator-owned path outside the repository, or a missing, symlink, tree, submodule, oversized, ambiguous, or unavailable source object retains the generic fail-closed outcome. -The owner-only v2 status always contains `schema_version`, one closed reason, and the diff-base, source, and captured policy SHAs. `protected_target_rule_path_pending` requires `actual`, `limit`, and `unit` to be null. The two installed-OCR background rejections instead require positive integer `actual`/`limit`, `actual > limit`, and the matching closed `characters` or `bytes` unit. The same preflight-qualified OCR executable derives these values under `review --preview` with exact production refs, rules, selection inputs, and the toolkit-owned background; the toolkit exposes no threshold setting and duplicates no OCR threshold constant. A recognized soft warning enters the CI log and atomically finalized result warnings, while a recognized hard rejection stops before model execution. The status contains no path, ref name, hostname, provider text, exception, stderr, or display wording. `post` hostile-reads the bounded regular single-link file, verifies the current source and diff-base identities, and renders only toolkit-authored text. It deliberately does not replace the captured policy SHA with a newer target-branch head. Missing, stale, malformed, oversized, permission-unsafe, unknown-version/reason/key, or identity-mismatched state falls back to the generic failure note. `OCR_POST_ERROR_DETAILS` never appends stderr to a recognized static note; emoji and strict/advisory exit behavior remain under the existing posting settings. +The owner-only v2 status always contains `schema_version`, one closed reason, and the diff-base, source, and captured policy SHAs. `protected_target_rule_path_pending` requires `actual`, `limit`, and `unit` to be null. The two installed-OCR background rejections instead require positive integer `actual`/`limit`, `actual > limit`, and the matching closed `characters` or `bytes` unit. The same preflight-qualified OCR executable derives these values under `review --preview` with exact production refs, rules, selection inputs, and the toolkit-owned background; the toolkit exposes no threshold setting and duplicates no OCR threshold constant. A recognized soft diagnostic becomes only the exact post-DLP `ocr.toolkit-advisory/v1` numeric projection and does not enter result warnings or approval inputs; a recognized hard rejection stops before model execution. The status contains no path, ref name, hostname, provider text, exception, stderr, or display wording. `post` hostile-reads the bounded regular single-link file, verifies the current source and diff-base identities, and renders only toolkit-authored text. It deliberately does not replace the captured policy SHA with a newer target-branch head. Missing, stale, malformed, oversized, permission-unsafe, unknown-version/reason/key, or identity-mismatched state falls back to the generic failure note. `OCR_POST_ERROR_DETAILS` never appends stderr to a recognized static note; emoji and strict/advisory exit behavior remain under the existing posting settings. ## Recognizers and candidates @@ -99,7 +99,7 @@ OCR runs under a fresh owner-only isolated `HOME` containing only toolkit-valida Publication validation runs after OCR and cleanup, within the same inode-checked atomic read/replace that attaches receipt v5. It compares both decoded source and rendered approximations against whole forbidden/non-publishable values and normalized contiguous excerpts of at least 24 characters. Closed checks cover nested HTML entities, comments/tags, inline/reference/autolink Markdown destinations, escapes/formatting, configured secrets, formatted-phone/email patterns, controls, and Unicode deception. Bare SHAs, build identifiers, and unformatted digit strings are not classified as phone numbers. A comparison that would exceed the fixed work bound is uncertainty. -The result has two explicit DLP projections. Publication sinks are exactly values the posting owner may render: outcome message, allowlisted finding fields, warnings, displayed tool names, and manifest-failure path/reason fields. If one is unsafe, the raw result is atomically replaced by an explicit safe-partial result containing independently passed findings/warnings plus closed reason, omission, original-coverage, and tool-use facts. Non-rendered OCR metadata is a separate private-retention projection: unsafe keys are removed and unsafe string values receive stable non-reversible placeholders before the result is retained. When that sanitized result still satisfies the OCR result contract, its original status, manifest, and safe findings remain intact; a required structural-field loss falls back to the safe-partial form. Neither path retains the rejected value or its location. GitLab may publish the safe result with a filtered signal, while retaining the previous review, consuming prior matching fingerprints one-for-one, replacing only an earlier toolkit setup-pending note on retry, and blocking approval. This is containment of deterministic exact-output classes, not a claim to detect shorter arbitrary excerpts, arbitrary semantic paraphrase, or undo content already sent to the model. +The result has two explicit DLP projections. Publication sinks are exactly values the posting owner may render: outcome message, allowlisted finding fields, warnings, displayed tool names, and manifest-failure path/reason fields. If one is unsafe, the raw result is atomically replaced by an explicit safe filtered result containing independently passed findings/warnings plus closed reason, omission, original-coverage, and tool-use facts. Horizontal tab is admitted as a control character only in `existing_code` and `suggestion_code`; the unchanged code value is retained only after all remaining DLP checks pass. Non-rendered OCR metadata is a separate private-retention projection: unsafe keys are removed and unsafe string values receive stable non-reversible placeholders before the result is retained. When that sanitized result still satisfies the OCR result contract, its original status, manifest, and safe findings remain intact; a required structural-field loss falls back to the filtered form. Neither path retains the rejected value or its location. GitLab may publish the safe result with a filtered signal, while retaining the previous review, consuming prior matching fingerprints one-for-one, replacing only an earlier toolkit setup-pending note on retry, and blocking approval. Validated original coverage kind/counts remain presentation evidence distinct from public-projection completeness; filtered warnings never create legacy coverage failures. This is containment of deterministic exact-output classes, not a claim to detect shorter arbitrary excerpts, arbitrary semantic paraphrase, or undo content already sent to the model. Receipt schema `ocr.toolkit-receipt/v5` stores only closed review/policy identities, context mode, per-source completeness and degradation counts, admitted-mutable flag, fixed tool usage, publication-DLP result, and cleanup result. Its publication states are `passed`, `private-sanitized`, and `publication-filtered`. Private sanitization keeps normal approval evaluation only when the canonical publication/approval projection is byte-equivalent; changed, malformed, or incomparable projections become safe partial results. The corresponding closed counts form an `ocr.publication-dlp-signal/v2` GitLab-summary marker and structured log event; neither is a new network telemetry exporter. The receipt/event never stores rejected text or locations, context text, upstream IDs, URLs, commands, arguments, headers, adapter results, personal display data, or transport diagnostics. v1-v4 result receipts are rejected rather than migrated. diff --git a/docs/engineering/review_signal_ownership.md b/docs/engineering/review_signal_ownership.md index acd68a0..4273181 100644 --- a/docs/engineering/review_signal_ownership.md +++ b/docs/engineering/review_signal_ownership.md @@ -17,7 +17,7 @@ new telemetry API. | 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. | +| 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` makes the public projection incomplete and blocks approval without relabelling independently complete OCR coverage. 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 diff --git a/docs/engineering/test_evidence_matrix.md b/docs/engineering/test_evidence_matrix.md index 14f321d..d037058 100644 --- a/docs/engineering/test_evidence_matrix.md +++ b/docs/engineering/test_evidence_matrix.md @@ -75,7 +75,7 @@ The M5 negative suite is an attacker matrix, not merely a replay of observed rev | --- | --- | --- | --- | | Source branch expands access | `review_runner._prepare_enriched_context` and `context.policy.load_protected_policy` bind the captured policy SHA and exact policy path | real immutable Git loader/wiring tests plus exact-call, source-policy, duplicate/UTF-8/size/schema/object-type cases | proven for deterministic Git/policy wiring; live forge service remains outside local evidence | | Source rules select public setup wording | `review_runner._record_rules_path_setup` plus `pre_execution` atomic envelope/hostile read and `posting.workflow.post_pre_execution_status` | real immutable Git and local-TLS GitLab identity prove absence at base/policy plus metadata-only regular source blob; hostile file/identity matrix and static posting tests cover malformed, stale, unknown, symlink/hard-link, oversize, emoji, stderr exclusion, and strict/advisory behavior | deterministic setup classification and source-to-sink wording proven; same-CI-identity compromise remains a host residual risk | -| Generated background exceeds current OCR guidance | `review_runner._qualify_review_background` invokes the resolved preflight-qualified OCR with the exact production refs, selection arguments, rules, and background under `review --preview`; `pre_execution` and posting own closed rejection persistence/reporting | exact-argv, bounded output, cleanup, near-miss, stale-artifact, warning-to-result/approval, hard-rejection/no-model, hostile status, and static posting tests pass; the exact supported OCR executable independently owns and returns its current warning/rejection thresholds | deterministic toolkit orchestration and closed reporting are proven; parser tests use OCR-shaped diagnostics, while the subsequent actual review remains OCR's independent production revalidation | +| Generated background exceeds current OCR guidance | `review_runner._qualify_review_background` invokes the resolved preflight-qualified OCR with the exact production refs, selection arguments, rules, and background under `review --preview`; result finalization owns the post-DLP advisory and `pre_execution` owns hard-rejection persistence | exact-argv, bounded output, cleanup, near-miss, advisory spoof/schema/receipt/approval tests, hard-rejection/no-model, hostile status, and static posting tests pass; the exact supported OCR executable independently owns and returns its current recommendation/rejection thresholds | deterministic toolkit orchestration and closed reporting are proven; the numeric advisory is not a warning, DLP/coverage/receipt input, telemetry signal, or approval blocker, while the subsequent actual review remains OCR's independent production revalidation | | BOLA/confused deputy/service credential mismatch | `context.adapters.authorize_and_resolve` plus `context.broker.acquire_external_records` before handle minting | real stdio and local-TLS peers exercise operator tenant/resource allowlists, request/run mismatch, uniform unavailable, replacement, and cross-tenant denial | proven for toolkit broker/protocol; adapter service truth remains a deployment non-claim | | Arbitrary URL/ID, SSRF, redirects, traversal, writes | `context.recognizers.recognize`, broker admission, redirect-free fixed transports, and `context.mcp.call_context_tool` | fixed-grammar/malformed-authority/config/redirect tests plus real one-process stdio MCP prove only committed handles reach `context_get`; arbitrary IDs, URLs, malformed/tampered cursors, unknown args/tools and source-bound cursors fail before store access | deterministic toolkit path and real local MCP proven; the real OCR run advertised both tools but made zero context calls, so model-driven list/get is not claimed | | Oversize, omission, TOCTOU, replay | separate request/admission/streaming limits, repeated bounded GitLab digest, aggregate orchestration, and `context.store.ContextStore` binding | real child/TLS no-read, request-write timeout, oversize, partial/multiple frame and bounded-pagination mutation; authorization-before-budget, exact/changed duplicate, hard-link/persistence/recursive-readback, version, lifetime and replay cases | proven through production transport owners, committed store, and local MCP readback | @@ -106,6 +106,14 @@ The M5 negative suite is an attacker matrix, not merely a replay of observed rev | 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 | +## v0.8.4 OCR 1.10.1 evidence + +| Requirement or boundary | Production owner and entry point | Required observable result | Evidence | Double boundary and claim limit | State | +| --- | --- | --- | --- | --- | --- | +| Exact OCR 1.10.1 review contract | compatibility gateway -> hosted Linux binary -> manifest/evidence promotion | result/manifest, inherited `16384` and explicit `4096` completion caps, default-medium two rounds, semantic grouping, and behaviorally observed max-tools values remain compatible | hosted run 32955196785; checksum-pinned `compatibility/evidence/ocr-1.10.1.json`; independent checksum-verified Darwin arm64 local gateway probe | deterministic gateways prove the consumed wire/result/loop contract, not provider quality or production spending policy | proven for exact qualified OCR wire contract | +| New upstream rule selection | exact OCR preview -> allowlist and built-in rule resolver | `.m` input is reviewable and content selects MATLAB or Objective-C behavior without changing the toolkit rule file | isolated Darwin arm64 no-LLM `ocr rules check` readback for MATLAB and Objective-C plus upstream semantic audit | selection evidence proves language/rule routing, not finding quality | proven | +| Bounded upstream session keys stay private | OCR runtime private cache/session owner -> toolkit result boundary | new bounded keys do not enter result, receipt v5, toolkit telemetry, GitLab text, or approval | upstream semantic audit plus unchanged result/manifest probes | source and result evidence prove the consumed boundary, not every upstream internal cache implementation | proven | + ## 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 b810989..261480b 100644 --- a/docs/engineering/toolkit_strategy.md +++ b/docs/engineering/toolkit_strategy.md @@ -37,7 +37,7 @@ The legacy `context/*` Markdown renderer, its CLI/environment contract, and its The built-in evidence MCP is mandatory for ordinary evidence-backed reviews. External stdio and native HTTPS Streamable HTTP servers compose as independent optional entries; replacement mode may discard stale external entries but cannot remove or shadow the built-in server. The compact bootstrap is generated from the same validated capability composition that is written to OCR. -GitLab result normalization and posting are implemented behind provider-oriented modules. They bound and neutralize model-controlled text, use stable finding fingerprints, preserve human-owned discussions, and keep GitLab credentials outside OCR. Receipt v5 separates byte-equivalent private sanitization from publication filtering through one canonical publication/approval projection. Review health, published findings, failed-file coverage, reconciled count-only evidence use, privacy-safe normalized token buckets, deterministic Recommended focus ranking, and collapsed technical details are separate implemented concepts. The current recommended and tested OCR baseline belongs in the operational compatibility contract, not this durable strategy. +GitLab result normalization and posting are implemented behind provider-oriented modules. They bound and neutralize model-controlled text, use stable finding fingerprints, preserve human-owned discussions, and keep GitLab credentials outside OCR. Receipt v5 separates byte-equivalent private sanitization from publication filtering through one canonical publication/approval projection. OCR coverage, public-projection integrity, published findings, core advisories, reconciled count-only evidence use, privacy-safe normalized token buckets, multi-finding Recommended focus ranking, and collapsed technical details are separate implemented concepts. The current recommended and tested OCR baseline belongs in the operational compatibility contract, not this durable strategy. ## Implemented Repository Evidence Engine @@ -71,7 +71,7 @@ Runtime packages follow responsibility rather than file-count boundaries. Pure r ## Implemented compact bootstrap and built-in evidence MCP -The OCR background is a compact, independently bounded bootstrap. The same installed and preflight-qualified OCR executable classifies it under `review --preview` with the exact production refs, rules, and selection inputs, so OCR remains the sole owner of its current recommended and hard byte/character thresholds. The toolkit exposes no threshold setting and duplicates no threshold constant. A recognized OCR soft warning remains reviewable but enters the CI log and finalized result warning list; a recognized hard rejection stops before model execution and produces an identity-bound closed numeric failure summary. The bootstrap contains authoritative constraints and trust instructions, base/head identity, evidence and delta-kind counts, the validated composed MCP capability inventory, relevant accepted decisions, and short project-guidance hints. Bootstrap planning and OCR MCP configuration consume the same composition plan so the instructions cannot advertise unavailable tools or omit available allowlisted tools; detailed evidence remains available through the built-in MCP rather than expanding the bootstrap. +The OCR background is a compact, independently bounded bootstrap. The same installed and preflight-qualified OCR executable classifies it under `review --preview` with the exact production refs, rules, and selection inputs, so OCR remains the sole owner of its current recommended and hard byte/character thresholds. The toolkit exposes no threshold setting and duplicates no threshold constant. A recognized OCR soft diagnostic remains reviewable and becomes only a receipt-gated, post-DLP numeric Technical details advisory; it does not enter OCR warnings or approval inputs. A recognized hard rejection stops before model execution and produces an identity-bound closed numeric failure summary. The bootstrap contains authoritative constraints and trust instructions, base/head identity, evidence and delta-kind counts, the validated composed MCP capability inventory, relevant accepted decisions, and short project-guidance hints. Bootstrap planning and OCR MCP configuration consume the same composition plan so the instructions cannot advertise unavailable tools or omit available allowlisted tools; detailed evidence remains available through the built-in MCP rather than expanding the bootstrap. Complete manifests, dependency inventories, guidance documents, and external issue/page contents do not belong in the bootstrap. Detailed repository facts are available on demand through a built-in server registered under a reserved namespace such as `ocr_toolkit_evidence`, with tools prefixed `ocr_toolkit_`. Candidate tools expose review environment, changed components, dependency state and deltas, framework state, version evidence, and accepted decisions. @@ -154,7 +154,7 @@ Qualified OCR releases expose explicit per-run provider/model overrides, additiv 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 +context, publication-DLP, posting, and approval projections. OCR 1.10.1 group keys contain changed paths and its labels are model-produced, so neither becomes a toolkit metric, routing input, or approval authority. diff --git a/docs/gitlab.md b/docs/gitlab.md index e900aec..84af6cf 100644 --- a/docs/gitlab.md +++ b/docs/gitlab.md @@ -20,9 +20,11 @@ The public pipeline stores the OCR binary checksum as the non-secret `OCR_SHA256 `OCR_REVIEW_LANGUAGE` defaults to `English`; `Russian` is one example of an explicit review language. The example pins its qualified OCR release 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. `OCR_MAX_TOOLS=0` delegates the per-file tool-call limit to the installed OCR template; use a positive value only after checking that release's behaviorally qualified minimum, normalization, and effective value. `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. +OCR 1.10.1 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. -The collapsed technical details keep aggregate input/output/cache token usage separate from OCR tool activity. The existing `all OCR tool calls` line lists every non-zero count for the closed review set (`file_read`, `file_read_diff`, `file_find`, `code_search`, `code_comment`, `task_done`, `ocr_toolkit_evidence`, `context_list`, and `context_get`) and is omitted when that admitted list is empty. These counts help explain whether OCR read context, searched the repository, checked cross-file diffs, consulted toolkit evidence, or emitted review output; they are not per-tool token attribution. Dynamic external MCP tool names remain private and appear only through the existing verified per-server aggregate. +The summary keeps four inputs separate: OCR manifest coverage, publication integrity, published findings, and an optional OCR core advisory. Complete manifest coverage that later loses public fields is labelled `Review complete with publication filtering`; it is warning-bearing and approval-ineligible, but it is not called incomplete OCR coverage and does not invent failed-file diagnostics. Actual partial, failed, waived, or budget-stopped manifest coverage keeps its stronger status. Ordinary OCR warnings retain their own complete-with-warnings state. The reviewer guide ranks bounded focus areas only when at least two findings are published; one finding remains in its inline or fallback discussion without a duplicate snippet. + +The collapsed technical details keep aggregate input/output/cache token usage separate from OCR tool activity. The existing `all OCR tool calls` line lists every non-zero count for the closed review set (`file_read`, `file_read_diff`, `file_find`, `code_search`, `code_comment`, `task_done`, `ocr_toolkit_evidence`, `context_list`, and `context_get`) and is omitted when that admitted list is empty. These counts help explain whether OCR read context, searched the repository, checked cross-file diffs, consulted toolkit evidence, or emitted review output; they are not per-tool token attribution. Dynamic external MCP tool names remain private and appear only through the existing verified per-server aggregate. Tool and token lines remain independent of `passed`, `private-sanitized`, or `publication-filtered` receipt state. If OCR accepts a background above its recommended character count, a separate `OCR core advisory` line contains only the actual and recommended counts. It does not become a warning or change review completeness or approval eligibility. ## Choose one operating mode diff --git a/docs/operations.md b/docs/operations.md index d3b6112..c82030e 100644 --- a/docs/operations.md +++ b/docs/operations.md @@ -34,7 +34,7 @@ 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. 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. +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 and 1.10.1. 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. `OCR_REVIEW_EFFORT=medium` is the toolkit default for the qualified OCR release 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=0`, delegating the effective per-file tool-call limit to the installed OCR template instead of carrying a release-specific minimum. A positive value can raise that limit, but operators should check the release's behavioral qualification because CLI help, normalization text, and effective template value can differ. Exhausting the effective 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. @@ -128,9 +128,9 @@ Suppression checks both the recorded inline position and compatible fingerprints ## OCR diagnostics -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. +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 recommendation and rejection thresholds; the toolkit has no threshold setting. A recognized soft diagnostic is reduced to a toolkit-authored `ocr.toolkit-advisory/v1` enum and two positive character counts. It is attached after publication DLP, rendered only with an exact receipt v5 in Technical details, and does not change warnings, coverage, DLP counts, telemetry, or automatic approval. 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 approval-ineligible `publication-filtered`, where the public projection is incomplete even when the receipt proves OCR coverage was complete. Posting uses only the validated original coverage kind and counts for status and Technical details; it never reconstructs or republishes rejected values. Horizontal tabs survive only in `existing_code` and `suggestion_code`; the unchanged values pass the same secret, PII, forbidden-value, laundering, and budget checks, while tabs elsewhere and all other control/format characters remain blocking. 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 publishable review or as proof that OCR coverage was partial. -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. +OCR 1.10.1 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. diff --git a/docs/review-context.md b/docs/review-context.md index a218efd..09bab0c 100644 --- a/docs/review-context.md +++ b/docs/review-context.md @@ -190,7 +190,7 @@ Both tools read the already committed local store. They have no network, subproc After OCR exits, the toolkit applies separate publication-sink and private-retention projections against forbidden/non-publishable context, configured secrets, closed PII patterns, controls, Markdown destinations, and Unicode/HTML/Markdown laundering forms. Publication sinks are exactly the result values the posting owner can render: finding fields, warnings, outcome message, displayed tool names, and manifest failure path/reason fields. Other OCR metadata remains private but is independently scanned and sanitized before persistence; it cannot make safe publication sinks partial merely because an opaque SHA, UUID, item identity, or bare build number resembles a phone number. The conservative detector can still classify a separator-bearing technical identifier as phone-like. Ordinary receipts intentionally expose only closed aggregate counts; an explicit local preservation run adds a private value-free path/subtype/size/hash decision sidecar so the operator can diagnose that false-positive class against the separately retained raw result without weakening the filter or disclosing the value. The checks compare whole values and normalized contiguous excerpts of at least 24 characters; a work-bound uncertainty filters or sanitizes the affected unit. This does not claim detection of shorter arbitrary excerpts or semantic paraphrases, and it cannot reverse data already sent to the model. -An unsafe result is neither retained raw nor discarded wholesale. In the same inode-checked atomic transformation used to attach the receipt, unsafe publication content produces a safe `completed_with_errors` projection with independently passed finding/warning fields, fixed tool-use counters, original closed coverage counts, and closed DLP reason/retained/omitted counts. Unsafe `content` removes its finding; an unsafe optional finding field is removed without discarding safe finding content. When every publication sink is safe and only private result metadata fails retention DLP, the unsafe private fields are replaced with static non-sensitive values while the original valid status, manifest, warnings, and findings remain. Unsafe values and their locations are never retained. Receipt v5 calls this `private-sanitized` only when a pure canonical publication/approval projection is byte-equivalent before and after sanitization; normal approval evaluation then applies. Any changed, malformed, or incomparable projection is `publication-filtered`, preserves the previous review, emits only closed counts, and cannot automatically approve. +An unsafe result is neither retained raw nor discarded wholesale. In the same inode-checked atomic transformation used to attach the receipt, unsafe publication content produces a safe `completed_with_errors` projection with independently passed finding/warning fields, fixed tool-use counters, original closed coverage counts, and closed DLP reason/retained/omitted counts. Unsafe `content` removes its finding; an unsafe optional finding field is removed without discarding safe finding content. Horizontal tab is permitted only in `existing_code` and `suggestion_code`, where the unchanged value still passes every secret, PII, forbidden-value, laundering, and budget detector before it is retained. When every publication sink is safe and only private result metadata fails retention DLP, the unsafe private fields are replaced with static non-sensitive values while the original valid status, manifest, warnings, and findings remain. Unsafe values and their locations are never retained. Receipt v5 calls this `private-sanitized` only when a pure canonical publication/approval projection is byte-equivalent before and after sanitization; normal approval evaluation then applies. Any changed, malformed, or incomparable projection is `publication-filtered`, preserves the previous review, emits only closed counts, and cannot automatically approve. Its public projection is incomplete, but posting uses the validated original coverage kind/counts so complete OCR coverage is not relabelled as partial and filtered warnings cannot synthesize failed items. The summary contains distinct private-sanitization and publication-filtering details with an exact `ocr.publication-dlp-signal/v2` HTML marker containing only low-cardinality counts. The posting command emits the same JSON as an `OCR toolkit telemetry event` log line so CI log collectors can alert without a new runtime network/exporter path. OCR remains authoritative for provider/token/request telemetry; the toolkit does not add an OTLP client, arbitrary telemetry endpoint, raw value/path, or mandatory external export. The explicit local `ocr-ci review --preserve-private-artifacts` diagnostic retains owner-only raw session/context state, adds a value-free `ocr.private-dlp-decisions/v1` path/reason/detector/size/hash attribution sidecar, and deliberately emits no posting receipt. It is local evidence for investigating conservative false positives, not a CI or publication artifact; the validated GitLab MR profile rejects it. diff --git a/docs/security.md b/docs/security.md index 24df672..b0c2327 100644 --- a/docs/security.md +++ b/docs/security.md @@ -67,6 +67,7 @@ Receipt v5 records the bounded configured capability inventory and positive call - Review context uses a closed `off|metadata|enriched` selector. `off` retains only validated source/protected-target/author identities; `metadata` admits bounded MR fields; `enriched` requires the immutable protected policy and admits only stable bounded discussion/adapter projections. Source policy, unknown fields/classes, raw display identities, arbitrary URLs/IDs, tokens, and ambient environment values cannot expand it. - Generated Markdown neutralizes controls and GitLab quick actions. Actionable suggestions require exact reviewed-head proof; unverifiable replacements retain prose only. - Result and provider reads have byte limits; notes enforce character and UTF-8 byte limits. Position-bearing inline creates reserve independent unguessable markers, classify closed outcomes, and perform at most one complete author-bound reconciliation read with no retry. +- Publication DLP admits ordinary horizontal tab only in `existing_code` and `suggestion_code`. The unchanged code value is retained only after it passes the existing size, secret, PII, forbidden-value, and laundering checks; tabs elsewhere and vertical tab, form feed, NUL, bidi/format controls, and other unsupported controls remain blocking. - LLM provider URLs are normalized by one forge-neutral owner shared by configuration and preflight; only credential-free absolute HTTPS roots or protocol-compatible terminal endpoints are accepted. Explicit protocol remains authoritative, and ambiguous auxiliary URL derivation fails closed. - Automatic approval binds the exact synchronized reviewed head and MR author from receipt v5, skips self-approval, and never removes an existing approval. Partial, warning, non-v5, publication-filtered, omitted, degraded metadata, DLP-rejected selected-source, required context degradation, admitted remediation-context, or direct external-MCP runs are ineligible. - Human replies are ownership boundaries. Merge-request source SHA, protected-target policy SHA, and merge-result SHA remain distinct. @@ -114,7 +115,8 @@ 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. +- **Upstream telemetry cardinality:** OCR 1.10.1 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. +- **OCR advisory spoofing:** raw OCR output cannot supply `_ocr_toolkit_advisory`. The toolkit adds only the exact numeric `ocr.toolkit-advisory/v1` shape after publication DLP; posting requires a valid receipt v5 and rejects duplicate, extended, unknown, or malformed forms. The advisory cannot change warnings, coverage, DLP, telemetry, findings, or approval. - **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 9427a2f..fb0dee4 100644 --- a/examples/gitlab/README.md +++ b/examples/gitlab/README.md @@ -2,7 +2,7 @@ [`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=0` delegates the effective per-file tool-call limit to the installed OCR template. `OCR_MAX_TOKENS_BUDGET` and the optional completion cap remain independent controls. +The pipeline pins OCR 1.10.1 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=0` delegates the effective per-file tool-call limit to the installed OCR template. `OCR_MAX_TOKENS_BUDGET` and the optional completion cap remain independent controls. | Mode | Recipe | MR text admitted | External access | Automatic approval | | --- | --- | --- | --- | --- | diff --git a/examples/gitlab/ocr-review.gitlab-ci.yml b/examples/gitlab/ocr-review.gitlab-ci.yml index eb91c13..99a452f 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.10.0" + OCR_VERSION: "v1.10.1" OCR_TOOLKIT_VERSION: "0.8.3" OCR_TOOLKIT_CHECKSUMS_URL: "https://github.com/xeonvs/open-code-review-toolkit/releases/download/v${OCR_TOOLKIT_VERSION}/SHA256SUMS" - OCR_SHA256: "f8f99ea071bed77dbcaa15fdd2083287bb8ae408d5928b3943ebe0788d191b6b" + OCR_SHA256: "8b806c221d409727a21611b4a7952d8e15edadbbc25f5affccaeb8f677e4055c" OCR_POST_MODE: "draft" OCR_STRICT_POSTING: "true" # Default-on receipt-bound approval; external MCP always remains comment-only. diff --git a/src/ocr_toolkit/context/dlp.py b/src/ocr_toolkit/context/dlp.py index 39b4391..2ae68de 100644 --- a/src/ocr_toolkit/context/dlp.py +++ b/src/ocr_toolkit/context/dlp.py @@ -109,7 +109,7 @@ def compile(cls, values: tuple[str, ...]) -> ForbiddenMatcher: exact: list[str] = [] seen: set[str] = set() for value in values: - candidate = normalize_text(value) + candidate = normalize_text(value, allow_horizontal_tabs=True) if not candidate: continue for representation in (_display_normalize(candidate), _source_normalize(candidate)): @@ -157,14 +157,16 @@ def matches(self, value: str) -> bool: return self.match_reason(value) is not None -def normalize_text(value: object) -> str | None: +def normalize_text(value: object, *, allow_horizontal_tabs: bool = False) -> str | None: """Normalize NFC/newlines and reject unsupported controls.""" if not isinstance(value, str): return None normalized = unicodedata.normalize("NFC", value.replace("\r\n", "\n").replace("\r", "\n")) + allowed_controls = "\n\t" if allow_horizontal_tabs else "\n" if any( - unicodedata.category(character) in {"Cc", "Cf", "Cs", "Zl", "Zp"} and character != "\n" + unicodedata.category(character) in {"Cc", "Cf", "Cs", "Zl", "Zp"} + and character not in allowed_controls for character in normalized ): return None @@ -178,10 +180,11 @@ def check_text( publication: bool = False, forbidden: tuple[str, ...] = (), forbidden_matcher: ForbiddenMatcher | None = None, + allow_horizontal_tabs: bool = False, ) -> DLPResult: """Apply independent units, redaction, PII, and optional publication checks.""" - normalized = normalize_text(value) + normalized = normalize_text(value, allow_horizontal_tabs=allow_horizontal_tabs) if normalized is None: return DLPResult(False, None, "invalid_text", "type_or_control") normalized_bytes = normalized.encode("utf-8") diff --git a/src/ocr_toolkit/ocr_result.py b/src/ocr_toolkit/ocr_result.py index 908852d..d8a14e7 100644 --- a/src/ocr_toolkit/ocr_result.py +++ b/src/ocr_toolkit/ocr_result.py @@ -9,6 +9,7 @@ import stat import sys from collections.abc import Callable, Mapping +from dataclasses import dataclass from pathlib import Path from typing import Any @@ -20,6 +21,11 @@ TOOLKIT_RESULT_KEY = "_ocr_toolkit" TOOLKIT_RESULT_SCHEMA_VERSION = 5 SUPPORTED_TOOLKIT_RESULT_SCHEMA_VERSIONS = frozenset({TOOLKIT_RESULT_SCHEMA_VERSION}) +TOOLKIT_ADVISORY_KEY = "_ocr_toolkit_advisory" +TOOLKIT_ADVISORY_SCHEMA_VERSION = "ocr.toolkit-advisory/v1" +TOOLKIT_ADVISORY_KIND = "background_recommended_limit" +TOOLKIT_ADVISORY_UNIT = "characters" +MAX_TOOLKIT_ADVISORY_VALUE = 999_999_999_999 # The receipt can name the 16 configured external servers plus the mandatory built-in. MAX_TOOLKIT_MCP_USAGE_SERVERS = 17 MAX_TOOLKIT_MCP_TOOLS_PER_SERVER = 128 @@ -57,6 +63,69 @@ class OcrResultTooLarge(Exception): """The OCR result artifact exceeds the configured safety limit.""" +@dataclass(frozen=True, slots=True) +class OcrToolkitAdvisory: + """Carry one validated toolkit-authored numeric OCR advisory.""" + + actual: int + recommended: int + + +def parse_toolkit_advisory(value: Any) -> OcrToolkitAdvisory: + """Parse the exact private toolkit advisory without accepting extensions.""" + + if not isinstance(value, dict) or set(value) != { + "schema_version", + "kind", + "actual", + "recommended", + "unit", + }: + raise OcrResultMalformed("OCR toolkit advisory has an unsupported schema") + actual = value.get("actual") + recommended = value.get("recommended") + if ( + value.get("schema_version") != TOOLKIT_ADVISORY_SCHEMA_VERSION + or value.get("kind") != TOOLKIT_ADVISORY_KIND + or value.get("unit") != TOOLKIT_ADVISORY_UNIT + or not isinstance(actual, int) + or isinstance(actual, bool) + or not isinstance(recommended, int) + or isinstance(recommended, bool) + or not 0 < recommended < actual <= MAX_TOOLKIT_ADVISORY_VALUE + ): + raise OcrResultMalformed("OCR toolkit advisory has invalid closed values") + return OcrToolkitAdvisory(actual=actual, recommended=recommended) + + +def background_recommended_advisory(*, actual: int, recommended: int) -> OcrToolkitAdvisory: + """Construct one validated background recommendation advisory.""" + + return parse_toolkit_advisory( + { + "schema_version": TOOLKIT_ADVISORY_SCHEMA_VERSION, + "kind": TOOLKIT_ADVISORY_KIND, + "actual": actual, + "recommended": recommended, + "unit": TOOLKIT_ADVISORY_UNIT, + } + ) + + +def toolkit_advisory_payload(advisory: OcrToolkitAdvisory) -> dict[str, object]: + """Serialize a validated advisory into its exact private result shape.""" + + payload: dict[str, object] = { + "schema_version": TOOLKIT_ADVISORY_SCHEMA_VERSION, + "kind": TOOLKIT_ADVISORY_KIND, + "actual": advisory.actual, + "recommended": advisory.recommended, + "unit": TOOLKIT_ADVISORY_UNIT, + } + parse_toolkit_advisory(payload) + return payload + + def max_result_bytes() -> int: """Return the maximum OCR JSON artifact size to read into memory.""" @@ -234,7 +303,16 @@ def _decode_result(data: bytes) -> Any: except UnicodeDecodeError as exc: raise OcrResultMalformed(str(exc)) from exc try: - return json.loads(text) + + def reject_duplicate_advisory(pairs: list[tuple[str, Any]]) -> dict[str, Any]: + result: dict[str, Any] = {} + for key, value in pairs: + if key == TOOLKIT_ADVISORY_KEY and key in result: + raise OcrResultMalformed("OCR result repeats the reserved toolkit advisory") + result[key] = value + return result + + return json.loads(text, object_pairs_hook=reject_duplicate_advisory) except (json.JSONDecodeError, RecursionError) as exc: raise OcrResultMalformed(str(exc)) from exc diff --git a/src/ocr_toolkit/posting/approval.py b/src/ocr_toolkit/posting/approval.py index 531eee3..0f2b602 100644 --- a/src/ocr_toolkit/posting/approval.py +++ b/src/ocr_toolkit/posting/approval.py @@ -14,10 +14,11 @@ TOOLKIT_MCP_SERVER_NAME_RE, ) from ocr_toolkit.posting.settings import BooleanSetting -from ocr_toolkit.result_contract import ReviewOutcome +from ocr_toolkit.result_contract import OcrResultContractError, ReviewOutcome ALLOWED_CATEGORIES = frozenset({"style", "documentation", "maintainability"}) MAX_APPROVABLE_FINDINGS = 3 +INVALID_APPROVAL_RECEIPT_REASON = "the review-time approval receipt is missing or invalid" class ApprovalStatus(str, Enum): @@ -218,6 +219,23 @@ def publication_dlp_state(value: Any) -> str | None: ) ): return None + selected = original["selected"] + completed = original["completed"] + reused = original["reused"] + failed = original["failed"] + waived = original["waived"] + outcome = original["outcome"] + derived_outcomes = {"failed"} | ( + {"skipped"} + if selected == 0 + else {"clean", "warning"} + if failed == 0 + else {"failed"} + if failed == selected + else {"partial"} + ) + if selected != completed + reused + failed + waived or outcome not in derived_outcomes: + return None return "publication-filtered" @@ -237,7 +255,7 @@ def _valid_dlp_reason_counts(value: Any) -> bool: def automatic_approval_metadata_reason(toolkit_metadata: Any) -> str: """Return the closed review-time receipt blocker for automatic approval.""" - invalid = "the review-time approval receipt is missing or invalid" + invalid = INVALID_APPROVAL_RECEIPT_REASON if not isinstance(toolkit_metadata, dict): return invalid if toolkit_metadata.get("schema_version") != 5 or set(toolkit_metadata) != { @@ -433,6 +451,51 @@ def automatic_approval_metadata_reason(toolkit_metadata: Any) -> str: return "" +def toolkit_receipt_is_valid(toolkit_metadata: Any) -> bool: + """Return whether metadata is an exact receipt v5, including valid blockers.""" + + return automatic_approval_metadata_reason(toolkit_metadata) != INVALID_APPROVAL_RECEIPT_REASON + + +def publication_outcome_for_summary(outcome: ReviewOutcome, publication: Any) -> ReviewOutcome: + """Recover only validated original coverage facts from a filtered receipt.""" + + if publication_dlp_state(publication) != "publication-filtered": + return outcome + if outcome.kind != "partial" or outcome.manifest_present: + raise OcrResultContractError( + "publication-filtered receipt is not bound to a safe result projection" + ) + original = publication["original"] + kind = original["outcome"] + if outcome.budget_exceeded and kind != "partial": + raise OcrResultContractError( + "publication-filtered receipt contradicts the result budget state" + ) + counts = { + field: original[field] for field in ("selected", "completed", "reused", "failed", "waived") + } + manifest_present = any(counts.values()) + status = { + "clean": "complete" if manifest_present else "success", + "warning": "completed_with_warnings", + "partial": "budget_exceeded" if outcome.budget_exceeded else "completed_with_errors", + "failed": "failed", + "skipped": "skipped", + }[kind] + return ReviewOutcome( + status=status, + kind=kind, + budget_exceeded=outcome.budget_exceeded and kind == "partial", + manifest_present=manifest_present, + selected_count=counts["selected"], + completed_count=counts["completed"], + reused_count=counts["reused"], + failed_count=counts["failed"], + waived_count=counts["waived"], + ) + + def _valid_evidence_actions(value: Any, evidence_calls: Any) -> bool: """Validate verified counts or an explicit unavailable attribution state.""" diff --git a/src/ocr_toolkit/posting/formatting.py b/src/ocr_toolkit/posting/formatting.py index 8974271..42aea9f 100644 --- a/src/ocr_toolkit/posting/formatting.py +++ b/src/ocr_toolkit/posting/formatting.py @@ -23,6 +23,7 @@ PUBLIC_REVIEW_TOOL_CALL_NAMES, SUPPORTED_TOOLKIT_RESULT_SCHEMA_VERSIONS, TOOLKIT_MCP_SERVER_NAME_RE, + OcrToolkitAdvisory, ) from ocr_toolkit.posting.approval import ( ApprovalResult, @@ -599,10 +600,10 @@ def format_publication_dlp_details(signal: dict[str, Any] | None) -> str: omitted = signal["omitted"] carried = signal["carried_forward_comments"] completeness = ( - "One or more publication units were omitted, so this review is partial and cannot " - "authorize automatic approval." + "One or more public projection units were omitted. OCR coverage is reported " + "separately, and automatic approval remains unavailable." if omitted["comments"] or omitted["warnings"] - else "The canonical publication or approval projection changed, so automatic approval remains unavailable." + else "The public projection changed, so automatic approval remains unavailable." ) return "\n".join( [ @@ -830,7 +831,11 @@ def format_reviewer_guide( enumerate(comments), key=lambda item: _guide_comment_rank(item[1], item[0]), ) - guide_comments = [comment for _, comment in ranked_comments[:MAX_REVIEWER_GUIDE_COMMENTS]] + guide_comments = ( + [comment for _, comment in ranked_comments[:MAX_REVIEWER_GUIDE_COMMENTS]] + if len(comments) >= 2 + else [] + ) if guide_comments: lines.append("") lines.append("### Recommended focus areas") @@ -879,6 +884,8 @@ def _review_outcome_line( marker, status_text = "⚠️", "Review stopped at token budget" elif partial_result: marker, status_text = "⚠️", "Review incomplete" + elif outcome_status == "publication-filtered": + marker, status_text = "⚠️", "Review complete with publication filtering" elif outcome_status in {"warning", "completed_with_warnings"} or warning_count: marker, status_text = "⚠️", "Review complete with warnings" elif has_finding_state: @@ -922,6 +929,17 @@ def _review_outcome_line( return f"{prefix}**{status_text} — {result_text}**" +def format_ocr_core_advisory(advisory: OcrToolkitAdvisory | None) -> str: + """Render one validated numeric OCR advisory for Technical details only.""" + + if advisory is None: + return "" + return ( + f"- OCR core advisory: background {advisory.actual} characters; recommended " + f"{advisory.recommended} characters; accepted by OCR core" + ) + + def summarize_result( total: int, inline_count: int, @@ -933,6 +951,7 @@ def summarize_result( tool_calls_summary: str = "", mcp_usage_summary: str = "", token_usage_summary: str = "", + ocr_core_advisory_summary: str = "", publication_dlp_details: str = "", reviewer_guide: str = "", fallback_reasons: Mapping[str, int] | None = None, @@ -1044,7 +1063,12 @@ def summarize_result( ) if reasons: technical.append(f"- Fallback reasons: {reasons}") - for summary in (mcp_usage_summary, tool_calls_summary, token_usage_summary): + for summary in ( + mcp_usage_summary, + tool_calls_summary, + token_usage_summary, + ocr_core_advisory_summary, + ): if summary: technical.append(summary) technical.append(f"- Review mode: `{post_mode()}`") diff --git a/src/ocr_toolkit/posting/result.py b/src/ocr_toolkit/posting/result.py index 4729c71..25886a0 100644 --- a/src/ocr_toolkit/posting/result.py +++ b/src/ocr_toolkit/posting/result.py @@ -122,7 +122,10 @@ def _safe_detail(value: object, reason: str) -> str: def normalize_coverage_diagnostics( - outcome: ReviewOutcome, warnings: Sequence[Any] + outcome: ReviewOutcome, + warnings: Sequence[Any], + *, + legacy_warning_fallback: bool = True, ) -> CoverageDiagnostics: """Normalize manifest failures or legacy warnings once at the posting boundary.""" @@ -136,7 +139,7 @@ def normalize_coverage_diagnostics( ) for item in outcome.failed_items ) - elif outcome.kind == "partial": + elif outcome.kind == "partial" and legacy_warning_fallback: for warning in warnings: path = warning.get("file") or warning.get("path") if isinstance(warning, dict) else None candidates.append((path, _legacy_reason(warning), ocr_warning_text(warning))) diff --git a/src/ocr_toolkit/posting/workflow.py b/src/ocr_toolkit/posting/workflow.py index 805b75a..0bcdeda 100644 --- a/src/ocr_toolkit/posting/workflow.py +++ b/src/ocr_toolkit/posting/workflow.py @@ -17,11 +17,13 @@ from ocr_toolkit.common.markdown import markdown_code_block, neutralize_quick_actions from ocr_toolkit.evidence.artifacts import repository_artifacts from ocr_toolkit.ocr_result import ( + TOOLKIT_ADVISORY_KEY, TOOLKIT_RESULT_KEY, OcrResultMalformed, OcrResultMissing, OcrResultTooLarge, load_ocr_result, + parse_toolkit_advisory, ) from ocr_toolkit.posting import gitlab as gitlab_api from ocr_toolkit.posting.approval import ( @@ -31,6 +33,8 @@ evaluate_approval_policy, provisional_approval_result, publication_dlp_state, + publication_outcome_for_summary, + toolkit_receipt_is_valid, ) from ocr_toolkit.posting.comments import ( clean_text, @@ -41,6 +45,7 @@ format_fallback_comment_chunks, format_inline_comment, format_mcp_usage_summary, + format_ocr_core_advisory, format_omitted_comments_summary, format_publication_dlp_details, format_reviewer_guide, @@ -625,6 +630,19 @@ def post_results(config: GitLabConfig, result: dict[str, Any]) -> int: title="**Open Code Review publication policy error**", ) + advisory = None + if TOOLKIT_ADVISORY_KEY in result: + try: + advisory = parse_toolkit_advisory(result[TOOLKIT_ADVISORY_KEY]) + except OcrResultMalformed as exc: + return invalid_ocr_schema_exit(config, str(exc)) + if not toolkit_receipt_is_valid(toolkit_metadata): + return invalid_ocr_schema_exit( + config, + "OCR toolkit advisory is not bound to a valid receipt v5", + ) + ocr_core_advisory_summary = format_ocr_core_advisory(advisory) + comments_value = result.get("comments", []) warnings_value = result.get("warnings", []) tool_calls_summary = format_tool_calls_summary(result.get("tool_calls")) @@ -651,16 +669,25 @@ def post_results(config: GitLabConfig, result: dict[str, Any]) -> int: approval_comments = list(comments) warnings = warnings_value - coverage_diagnostics = normalize_coverage_diagnostics(outcome, warnings) - if outcome.kind == "failed": + try: + summary_outcome = publication_outcome_for_summary(outcome, publication) + except OcrResultContractError as exc: + return invalid_ocr_schema_exit(config, str(exc)) + coverage_diagnostics = normalize_coverage_diagnostics( + summary_outcome, + warnings, + legacy_warning_fallback=publication_state != "publication-filtered", + ) + if summary_outcome.kind == "failed": return post_manifest_failure( config, - outcome, + summary_outcome, outcome_message, warnings, tool_calls_summary=tool_calls_summary, mcp_usage_summary=mcp_usage_summary, token_usage_summary=token_usage_summary, + ocr_core_advisory_summary=ocr_core_advisory_summary, ) billing_reason = llm_billing_failure_reason(warnings) @@ -733,11 +760,19 @@ def post_results(config: GitLabConfig, result: dict[str, Any]) -> int: summary_run_id = secrets.token_hex(16) receipt_sha, reviewed_author_id = approval_receipt_identity(result.get(TOOLKIT_RESULT_KEY)) reviewed_commit = receipt_sha or reviewed_sha() + summary_status = ( + "publication-filtered" + if publication_state == "publication-filtered" + and summary_outcome.kind in {"clean", "warning"} + else "budget_exceeded" + if summary_outcome.budget_exceeded + else summary_outcome.kind + ) reviewer_guide = format_reviewer_guide( comments, omitted_count, - outcome_status="budget_exceeded" if outcome.budget_exceeded else outcome.kind, - coverage_summary=outcome.coverage_summary, + outcome_status=summary_status, + coverage_summary=summary_outcome.coverage_summary, ) if publishable_comment_count == 0: @@ -754,13 +789,14 @@ def render_no_comments_summary(approval_result: ApprovalResult) -> str: tool_calls_summary=tool_calls_summary, mcp_usage_summary=mcp_usage_summary, token_usage_summary=token_usage_summary, + ocr_core_advisory_summary=ocr_core_advisory_summary, publication_dlp_details=dlp_details, reviewer_guide=reviewer_guide, reviewed_sha=reviewed_commit, mr_head_sha=mr_head_sha(), - outcome_status=("budget_exceeded" if outcome.budget_exceeded else outcome.kind), + outcome_status=summary_status, outcome_message=outcome_message, - coverage_summary=outcome.coverage_summary, + coverage_summary=summary_outcome.coverage_summary, coverage_diagnostics=coverage_diagnostics, warnings=warnings, suppressed_count=suppressed_count, @@ -940,14 +976,15 @@ def render_findings_summary(approval_result: ApprovalResult) -> str: tool_calls_summary=tool_calls_summary, mcp_usage_summary=mcp_usage_summary, token_usage_summary=token_usage_summary, + ocr_core_advisory_summary=ocr_core_advisory_summary, publication_dlp_details=dlp_details, reviewer_guide=reviewer_guide, fallback_reasons=fallback_reasons, reviewed_sha=reviewed_commit, mr_head_sha=mr_head_sha(), - outcome_status="budget_exceeded" if outcome.budget_exceeded else outcome.kind, + outcome_status=summary_status, outcome_message=outcome_message, - coverage_summary=outcome.coverage_summary, + coverage_summary=summary_outcome.coverage_summary, coverage_diagnostics=coverage_diagnostics, warnings=warnings, suppressed_count=suppressed_count, @@ -1060,6 +1097,7 @@ def post_manifest_failure( tool_calls_summary: str = "", mcp_usage_summary: str = "", token_usage_summary: str = "", + ocr_core_advisory_summary: str = "", ) -> int: """Post a manifest-declared run failure while preserving prior review notes.""" @@ -1072,6 +1110,7 @@ def post_manifest_failure( tool_calls_summary=tool_calls_summary, mcp_usage_summary=mcp_usage_summary, token_usage_summary=token_usage_summary, + ocr_core_advisory_summary=ocr_core_advisory_summary, outcome_status="failed", outcome_message=message, coverage_summary=outcome.coverage_summary, diff --git a/src/ocr_toolkit/preflight.py b/src/ocr_toolkit/preflight.py index 0a4e7b2..e62204c 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.10.0" +EXPECTED_OCR_VERSION = "1.10.1" class PreflightError(Exception): diff --git a/src/ocr_toolkit/review_runner.py b/src/ocr_toolkit/review_runner.py index 542e05b..9725582 100644 --- a/src/ocr_toolkit/review_runner.py +++ b/src/ocr_toolkit/review_runner.py @@ -76,13 +76,17 @@ from ocr_toolkit.ocr_result import ( MAX_TOOLKIT_MCP_USAGE_COUNT, PUBLIC_REVIEW_TOOL_CALL_NAMES, + TOOLKIT_ADVISORY_KEY, TOOLKIT_RESULT_KEY, TOOLKIT_RESULT_SCHEMA_VERSION, OcrResultMalformed, OcrResultMissing, OcrResultTooLarge, + OcrToolkitAdvisory, + background_recommended_advisory, inspect_ocr_result, load_ocr_result, + toolkit_advisory_payload, transform_ocr_result, ) from ocr_toolkit.posting.result import ocr_warning_text @@ -156,7 +160,7 @@ class ReviewRunnerError(Exception): class BackgroundQualification: """Carry closed toolkit-authored projections of installed OCR diagnostics.""" - warning: str | None = None + advisory: OcrToolkitAdvisory | None = None operator_notices: tuple[str, ...] = () @@ -441,7 +445,13 @@ def _review_receipt( } -def _dlp_reasons(value: object, *, budgets: TextBudgets, matcher: ForbiddenMatcher) -> Counter[str]: +def _dlp_reasons( + value: object, + *, + budgets: TextBudgets, + matcher: ForbiddenMatcher, + allow_horizontal_tabs: bool = False, +) -> Counter[str]: """Count closed DLP failures without retaining hostile strings or locations.""" reasons: Counter[str] = Counter() @@ -459,12 +469,24 @@ def _dlp_reasons(value: object, *, budgets: TextBudgets, matcher: ForbiddenMatch budgets=budgets, publication=True, forbidden_matcher=matcher, + allow_horizontal_tabs=allow_horizontal_tabs, ) if not checked.admitted: reasons[checked.reason] += 1 return reasons +def _code_field_allows_horizontal_tabs(path: tuple[object, ...]) -> bool: + """Allow HTAB only in the two closed code-bearing finding fields.""" + + return bool( + len(path) == 3 + and path[0] == "comments" + and isinstance(path[1], int) + and path[2] in {"existing_code", "suggestion_code"} + ) + + def _private_dlp_decisions( payload: dict[str, object], *, forbidden: tuple[str, ...] ) -> dict[str, object]: @@ -527,6 +549,7 @@ def add_decision(decision: dict[str, object]) -> None: budgets=budgets, publication=True, forbidden_matcher=matcher, + allow_horizontal_tabs=_code_field_allows_horizontal_tabs(path), ) if checked.admitted: continue @@ -582,32 +605,36 @@ def _write_private_dlp_decisions( raise ReviewRunnerError("OCR private DLP diagnostics could not be written") from exc -def _publication_sinks(payload: dict[str, object]) -> list[object]: +def _publication_sinks(payload: dict[str, object]) -> list[tuple[object, bool]]: """Select only OCR-controlled values that the posting owner can render.""" - sinks: list[object] = [] + sinks: list[tuple[object, bool]] = [] message = payload.get("message") if message is not None: - sinks.append(message) + sinks.append((message, False)) comments = payload.get("comments") if isinstance(comments, list): - sinks.extend( - {key: value for key, value in item.items() if key in QUARANTINE_COMMENT_FIELDS} - for item in comments - if isinstance(item, dict) - ) + for item in comments: + if not isinstance(item, dict): + continue + sinks.extend( + ( + value, + isinstance(value, str) and key in {"existing_code", "suggestion_code"}, + ) + for key, value in item.items() + if key in QUARANTINE_COMMENT_FIELDS + ) warnings = payload.get("warnings") if warnings is not None: - sinks.append(warnings) + sinks.append((warnings, False)) manifest = payload.get("manifest") coverage = manifest.get("coverage") if isinstance(manifest, dict) else None failed = coverage.get("failed") if isinstance(coverage, dict) else None if isinstance(failed, list): - sinks.extend( - {key: item.get(key) for key in ("path", "reason") if key in item} - for item in failed - if isinstance(item, dict) - ) + for item in failed: + if isinstance(item, dict): + sinks.extend((item[key], False) for key in ("path", "reason") if key in item) return sinks @@ -729,7 +756,14 @@ def _safe_publication_comments( for key, field_value in item.items(): if not isinstance(key, str) or key not in QUARANTINE_COMMENT_FIELDS: continue - if _dlp_reasons(field_value, budgets=budgets, matcher=matcher): + if _dlp_reasons( + field_value, + budgets=budgets, + matcher=matcher, + allow_horizontal_tabs=( + isinstance(field_value, str) and key in {"existing_code", "suggestion_code"} + ), + ): omitted_fields += 1 content_unsafe = content_unsafe or key == "content" continue @@ -844,8 +878,15 @@ def _publication_projection( budgets = TextBudgets(max_chars=2_000_000, max_bytes=8_000_000, max_lines=100_000) matcher = ForbiddenMatcher.compile(forbidden) sink_reasons: Counter[str] = Counter() - for sink in _publication_sinks(payload): - sink_reasons.update(_dlp_reasons(sink, budgets=budgets, matcher=matcher)) + for sink, allow_horizontal_tabs in _publication_sinks(payload): + sink_reasons.update( + _dlp_reasons( + sink, + budgets=budgets, + matcher=matcher, + allow_horizontal_tabs=allow_horizontal_tabs, + ) + ) sanitized, private_reasons, redacted_fields = _sanitize_nonpublication_fields( payload, budgets=budgets, matcher=matcher ) @@ -871,7 +912,7 @@ def _publication_projection( payload.get("warnings"), budgets=budgets, matcher=matcher ) projected: dict[str, object] = { - "status": "completed_with_errors", + "status": "budget_exceeded" if outcome.budget_exceeded else "completed_with_errors", "message": ( "Publication policy produced a safe partial OCR result. Independently safe " "findings may be published, but the result must not be treated as a complete " @@ -883,6 +924,8 @@ def _publication_projection( payload.get("tool_calls"), allowed_tools=allowed_tools ), } + if outcome.budget_exceeded: + projected["summary"] = {"budget_exceeded": True} else: reasons = sink_reasons + private_reasons return ( @@ -924,7 +967,7 @@ def _finalize_ocr_result( evidence_action_counts: dict[str, int] | None = None, *, forbidden: tuple[str, ...], - toolkit_warnings: tuple[str, ...] = (), + toolkit_advisory: OcrToolkitAdvisory | None = None, ) -> tuple[dict[str, int], bool, dict[str, object]]: """Validate, DLP-project, and receipt-bind one result in one atomic read/replace.""" @@ -935,25 +978,12 @@ def _finalize_ocr_result( def finalize(payload: dict[str, object]) -> dict[str, object]: nonlocal filtered, publication, usage - if TOOLKIT_RESULT_KEY in payload: - raise OcrResultMalformed(f"OCR result contains reserved field {TOOLKIT_RESULT_KEY!r}") + for reserved in (TOOLKIT_RESULT_KEY, TOOLKIT_ADVISORY_KEY): + if reserved in payload: + raise OcrResultMalformed(f"OCR result contains reserved field {reserved!r}") warnings = payload.get("warnings", []) if not isinstance(warnings, list): raise OcrResultMalformed("OCR result warnings must be a list") - warning_texts = {ocr_warning_text(warning) for warning in warnings} - appended_warnings: list[str] = [] - for warning in toolkit_warnings: - if warning in warning_texts: - continue - warning_texts.add(warning) - appended_warnings.append(warning) - payload = { - **payload, - "warnings": [ - *warnings, - *appended_warnings, - ], - } metadata = _review_receipt( payload, composition, @@ -969,7 +999,10 @@ def finalize(payload: dict[str, object]) -> dict[str, object]: mcp = metadata.get("mcp") raw_usage = mcp.get("usage") if isinstance(mcp, dict) else None usage = dict(raw_usage) if isinstance(raw_usage, dict) else {} - return {**projected, TOOLKIT_RESULT_KEY: metadata} + finalized = {**projected, TOOLKIT_RESULT_KEY: metadata} + if toolkit_advisory is not None: + finalized[TOOLKIT_ADVISORY_KEY] = toolkit_advisory_payload(toolkit_advisory) + return finalized try: transform_ocr_result(result_path, finalize) @@ -1158,14 +1191,14 @@ def _parse_background_preview(*, returncode: int, stderr: bytes) -> BackgroundQu or len(max_tools_matches) > 1 ): raise ReviewRunnerError("OCR background preview returned ambiguous diagnostics") - warning: str | None = None + advisory: OcrToolkitAdvisory | None = None if soft_matches: actual, limit = (int(value) for value in soft_matches[0].groups()) if not 0 < limit < actual: raise ReviewRunnerError("OCR background preview returned invalid thresholds") - warning = ( - f"Installed OCR reported a {actual}-character review background above its " - f"recommended {limit}-character threshold; review continued." + advisory = background_recommended_advisory( + actual=actual, + recommended=limit, ) operator_notices: tuple[str, ...] = () if max_tools_matches: @@ -1179,7 +1212,7 @@ def _parse_background_preview(*, returncode: int, stderr: bytes) -> BackgroundQu "effective tool-call limit.", ) return BackgroundQualification( - warning=warning, + advisory=advisory, operator_notices=operator_notices, ) if hard_character is not None: @@ -1295,9 +1328,11 @@ def _run_background_qualified_review( file=sys.stderr, ) return 2, BackgroundQualification() - if qualification.warning is not None: + if qualification.advisory is not None: print( - f"OCR background qualification warning: {qualification.warning}", + "OCR core advisory: " + f"background {qualification.advisory.actual} characters; recommended " + f"{qualification.advisory.recommended} characters; accepted by OCR core", file=sys.stderr, ) for notice in qualification.operator_notices: @@ -1933,11 +1968,7 @@ def run_evidence_review( enrichment, evidence_action_counts, forbidden=forbidden, - toolkit_warnings=( - (background_qualification.warning,) - if background_qualification.warning is not None - else () - ), + toolkit_advisory=background_qualification.advisory, ) except ReviewRunnerError: try: diff --git a/tests/test_environment_contract.py b/tests/test_environment_contract.py index 09dd73d..0ba1936 100644 --- a/tests/test_environment_contract.py +++ b/tests/test_environment_contract.py @@ -69,8 +69,8 @@ } EXAMPLE_DEFAULTS = { - "OCR_VERSION": "v1.10.0", - "OCR_SHA256": "f8f99ea071bed77dbcaa15fdd2083287bb8ae408d5928b3943ebe0788d191b6b", + "OCR_VERSION": "v1.10.1", + "OCR_SHA256": "8b806c221d409727a21611b4a7952d8e15edadbbc25f5affccaeb8f677e4055c", "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", @@ -299,7 +299,7 @@ def test_example_local_defaults_match_the_pipeline() -> None: encoding="utf-8" ) for name, value in { - "OCR_VERSION": "v1.10.0", + "OCR_VERSION": "v1.10.1", "OCR_SHA256": EXAMPLE_DEFAULTS["OCR_SHA256"], "OCR_TOOLKIT_VERSION": STABLE_TOOLKIT_VERSION, "OCR_MAX_TOOLS": "0", diff --git a/tests/test_ocr_compat.py b/tests/test_ocr_compat.py index 8c866f2..0f3dd86 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.10.0" - assert manifest["monitoring_floor"] == "1.10.0" + assert manifest["recommended_version"] == "1.10.1" + assert manifest["monitoring_floor"] == "1.10.1" assert [(item["version"], item["status"]) for item in manifest["releases"]] == [ ("1.7.17", "tested"), ("1.8.0", "tested"), @@ -70,6 +70,7 @@ def test_committed_manifest_is_valid_and_has_recommended_tested_baseline() -> No ("1.9.9", "tested"), ("1.9.10", "tested"), ("1.10.0", "tested"), + ("1.10.1", "tested"), ] @@ -154,9 +155,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.10.1")] + first_page = [release("1.10.2")] first_page.extend({"draft": True} for _ in range(module.MAX_RELEASES_PER_PAGE - 1)) - second_page = [release("1.10.0")] + second_page = [release("1.10.1")] requested: list[str] = [] def fake_request(url: str) -> list[dict[str, Any]]: @@ -166,14 +167,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.10.1"] + assert [item["tag_name"] for item in unseen] == ["v1.10.2"] 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.10.1")] + page = [release("1.10.2")] page.extend({"draft": True} for _ in range(module.MAX_RELEASES_PER_PAGE - 1)) with patched_attr(module, "_request_json", lambda _url: page): @@ -218,14 +219,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.10.1")]) + matrix = module.qualification_matrix(manifest, [release("1.10.2")]) assert matrix == { "include": [ { - "comparison_version": "1.10.0", - "tag": "v1.10.1", - "tested_baseline_version": "1.10.0", + "comparison_version": "1.10.1", + "tag": "v1.10.2", + "tested_baseline_version": "1.10.1", } ] } @@ -1427,11 +1428,11 @@ def test_prepare_update_rejects_human_review_candidate(tmp_path: Path) -> None: module = load_script() evidence = { "schema_version": 2, - "version": "1.10.1", + "version": "1.10.2", "result": "compatible", "classification": "human-review-required", - "comparison_version": "1.10.0", - "tested_baseline_version": "1.10.0", + "comparison_version": "1.10.1", + "tested_baseline_version": "1.10.1", } with pytest.raises(module.CompatibilityError, match="bounded conclusion"): @@ -1450,8 +1451,8 @@ def test_prepare_update_requires_human_review_for_minor_transition() -> None: "version": "1.11.0", "result": "compatible", "classification": "automatic-safe", - "comparison_version": "1.10.0", - "tested_baseline_version": "1.10.0", + "comparison_version": "1.10.1", + "tested_baseline_version": "1.10.1", } with pytest.raises(module.CompatibilityError, match="explicit human review"): @@ -1492,8 +1493,8 @@ def test_prepare_update_rejects_nonadjacent_minor_transition() -> None: "version": "1.12.0", "result": "compatible", "classification": "human-review-required", - "comparison_version": "1.10.0", - "tested_baseline_version": "1.10.0", + "comparison_version": "1.10.1", + "tested_baseline_version": "1.10.1", } with pytest.raises(module.CompatibilityError, match="contiguous release sequence"): @@ -1510,11 +1511,11 @@ def test_prepare_update_rejects_conclusion_outside_evidence_chain() -> None: module = load_script() evidence = { "schema_version": 2, - "version": "1.10.1", + "version": "1.10.2", "result": "compatible", "classification": "automatic-safe", - "comparison_version": "1.10.0", - "tested_baseline_version": "1.10.0", + "comparison_version": "1.10.1", + "tested_baseline_version": "1.10.1", } with pytest.raises(module.CompatibilityError, match="only evidence versions"): @@ -1522,7 +1523,7 @@ def test_prepare_update_rejects_conclusion_outside_evidence_chain() -> None: manifest_path=MANIFEST, evidence=evidence, fragment_number=72, - human_conclusions={"1.10.2": "Synthetic unrelated conclusion."}, + human_conclusions={"1.10.3": "Synthetic unrelated conclusion."}, root=PROJECT_ROOT, ) @@ -1534,11 +1535,11 @@ def test_prepare_update_rejects_invalid_optional_reviewed_conclusion( module = load_script() evidence = { "schema_version": 2, - "version": "1.10.1", + "version": "1.10.2", "result": "compatible", "classification": "automatic-safe", - "comparison_version": "1.10.0", - "tested_baseline_version": "1.10.0", + "comparison_version": "1.10.1", + "tested_baseline_version": "1.10.1", } with pytest.raises(module.CompatibilityError, match="bounded plain text"): @@ -1546,7 +1547,7 @@ def test_prepare_update_rejects_invalid_optional_reviewed_conclusion( manifest_path=MANIFEST, evidence=evidence, fragment_number=72, - human_conclusions={"1.10.1": conclusion}, + human_conclusions={"1.10.2": conclusion}, root=PROJECT_ROOT, ) diff --git a/tests/test_operations_docs.py b/tests/test_operations_docs.py index 2d77606..fbd9a72 100644 --- a/tests/test_operations_docs.py +++ b/tests/test_operations_docs.py @@ -227,6 +227,30 @@ def test_completion_cap_and_provider_failure_boundaries_are_public() -> None: assert "receipt, DLP, telemetry, severity, finding, or approval signal" in security +def test_summary_and_code_tab_boundaries_are_public() -> None: + """Keep coverage, filtering, advisory, and field-specific HTAB semantics distinct.""" + + operations = OPERATIONS.read_text(encoding="utf-8") + configuration = CONFIGURATION.read_text(encoding="utf-8") + gitlab = GITLAB_GUIDE.read_text(encoding="utf-8") + security = (PROJECT_ROOT / "docs" / "security.md").read_text(encoding="utf-8") + + for phrase in ( + "Review complete with publication filtering", + "it is not called incomplete OCR coverage", + "at least two findings", + "Tool and token lines remain independent", + ): + assert phrase in gitlab + for document in (operations, configuration, security): + assert "existing_code" in document + assert "suggestion_code" in document + assert "horizontal tab" in document.casefold() + assert "filtered warnings into legacy failed-item inference" in configuration + assert "public projection is incomplete" in operations + assert "all other control/format characters remain blocking" in operations + + def test_finding_badge_contract_is_opt_in_and_privacy_explicit() -> None: operations = OPERATIONS.read_text(encoding="utf-8") configuration = CONFIGURATION.read_text(encoding="utf-8") @@ -474,9 +498,12 @@ def test_ocr_compatibility_workflow_is_bounded_and_protected() -> None: "OCR 1.9.9 — inherited predecessor", "OCR 1.9.10 — toolkit 0.8.0 target and 0.8.2 predecessor", "OCR 1.10.0 — toolkit 0.8.2 and 0.8.3 target", + "OCR 1.10.1 — toolkit 0.8.4 target", + "ocr.toolkit-advisory/v1", "ocr.llm-retry-report/v1", "not toolkit telemetry", "Deploy toolkit 0.8.2 or 0.8.3 directly with OCR 1.10.0", + "Deploy toolkit 0.8.4 directly with OCR 1.10.1", "max_completion_tokens=16384", "do not install OCR 1.9.10 as an intermediate step", ): diff --git a/tests/test_posting_approval.py b/tests/test_posting_approval.py index d2b034c..8d0463f 100644 --- a/tests/test_posting_approval.py +++ b/tests/test_posting_approval.py @@ -7,6 +7,7 @@ from contextlib import redirect_stderr from typing import Any +from ocr_toolkit import ocr_result from ocr_toolkit.posting import ( approval, formatting, @@ -260,6 +261,54 @@ def test_publication_dlp_filtered_receipt_is_valid_but_never_eligible(self) -> N "publication DLP filtered the complete review result", ) + def test_filtered_receipt_rejects_outcomes_that_contradict_coverage(self) -> None: + """Accept run-level failure but reject impossible coverage/outcome combinations.""" + + publication = { + "state": "publication-filtered", + "reason_counts": { + "forbidden": 1, + "invalid_text": 0, + "laundering": 0, + "limit": 0, + "pii": 0, + "secret": 0, + }, + "retained": {"comments": 0, "warnings": 0}, + "omitted": {"comments": 1, "warnings": 0, "fields": 0}, + "original": { + "outcome": "failed", + "selected": 2, + "completed": 2, + "reused": 0, + "failed": 0, + "waived": 0, + }, + } + self.assertEqual(approval.publication_dlp_state(publication), "publication-filtered") + + invalid_originals = ( + {"outcome": "partial", "selected": 0, "completed": 0, "failed": 0}, + {"outcome": "clean", "selected": 0, "completed": 0, "failed": 0}, + {"outcome": "warning", "selected": 0, "completed": 0, "failed": 0}, + {"outcome": "skipped", "selected": 1, "completed": 1, "failed": 0}, + {"outcome": "partial", "selected": 2, "completed": 2, "failed": 0}, + {"outcome": "partial", "selected": 2, "completed": 0, "failed": 2}, + {"outcome": "clean", "selected": 2, "completed": 0, "failed": 2}, + {"outcome": "warning", "selected": 2, "completed": 0, "failed": 2}, + ) + for original in invalid_originals: + candidate = { + **publication, + "original": { + "reused": 0, + "waived": 0, + **original, + }, + } + with self.subTest(original=original): + self.assertIsNone(approval.publication_dlp_state(candidate)) + def test_private_only_sanitization_keeps_existing_approval_gates(self) -> None: receipt = receipt_v5() receipt["publication"] = { @@ -863,6 +912,210 @@ def test_receipt_identity_is_atomic_for_summary_and_approval(self) -> None: with self.subTest(candidate=candidate): self.assertEqual(workflow.approval_receipt_identity(candidate), ("", None)) + def test_valid_receipt_binds_advisory_without_changing_summary_or_approval_inputs(self) -> None: + """Publish one closed advisory in Technical details with ordinary clean status.""" + + receipt = receipt_v5(author_id=41) + self.assertTrue(approval.toolkit_receipt_is_valid(receipt)) + notes: list[str] = [] + + def capture_note(_config: Any, _title: str, body: str, *_args: Any) -> dict[str, int]: + notes.append(body) + return {"id": 1} + + with ( + patched_attr( + workflow, "collect_previous_bot_comment_refs", lambda _config: BotCommentRefs() + ), + patched_attr(workflow, "post_review_note_bounded", capture_note), + patched_attr(workflow, "finalize_review_approval", lambda *_args, **_kwargs: 0), + ): + exit_code = workflow.post_results( + gitlab_config(), + { + "status": "complete", + "comments": [], + "warnings": [], + "manifest": { + "schema_version": "ocr.run-manifest/v1", + "operation": "review", + "terminal_state": "complete", + "coverage": { + "selected": [{"item_id": "synthetic-item"}], + "completed": [{"item_id": "synthetic-item"}], + "reused": [], + "failed": [], + "waived": [], + }, + }, + "_ocr_toolkit": receipt, + ocr_result.TOOLKIT_ADVISORY_KEY: ocr_result.toolkit_advisory_payload( + ocr_result.background_recommended_advisory( + actual=2_100, + recommended=2_000, + ) + ), + }, + ) + + self.assertEqual(exit_code, 0) + self.assertEqual(len(notes), 1) + visible, technical = notes[0].split("
", 1) + self.assertIn("Review complete — no findings", visible) + self.assertNotIn("warnings", visible.casefold()) + self.assertNotIn("OCR core advisory", visible) + self.assertIn( + "OCR core advisory: background 2100 characters; recommended 2000 characters; " + "accepted by OCR core", + technical, + ) + + def test_complete_filtered_review_keeps_coverage_and_activity_dimensions_separate( + self, + ) -> None: + """Render scenario B without inventing partial coverage or a failed item.""" + + receipt = receipt_v5(author_id=41) + receipt["publication"] = { + "state": "publication-filtered", + "reason_counts": { + "forbidden": 0, + "invalid_text": 2, + "laundering": 0, + "limit": 0, + "pii": 0, + "secret": 0, + }, + "retained": {"comments": 1, "warnings": 0}, + "omitted": {"comments": 0, "warnings": 0, "fields": 2}, + "original": { + "outcome": "clean", + "selected": 5, + "completed": 5, + "reused": 0, + "failed": 0, + "waived": 0, + }, + } + notes: list[tuple[str, str]] = [] + + def capture_note( + _config: Any, + title: str, + body: str, + *_args: Any, + ) -> dict[str, int]: + notes.append((title, body)) + return {"id": len(notes)} + + with ( + patched_attr( + workflow, "collect_previous_bot_comment_refs", lambda _config: BotCommentRefs() + ), + patched_attr(workflow, "get_diff_refs", lambda _config: None), + patched_attr(workflow, "post_review_note_bounded", capture_note), + patched_attr(workflow, "finalize_review_approval", lambda *_args, **_kwargs: 0), + ): + exit_code = workflow.post_results( + gitlab_config(), + { + "status": "completed_with_errors", + "message": "Publication policy produced a safe partial OCR result.", + "comments": [ + { + "path": "src/example.py", + "line": 7, + "content": "Keep the validated branch.", + "severity": "low", + "category": "maintainability", + } + ], + "warnings": [], + "tool_calls": {"total": 2, "by_tool": {"file_read": 2}}, + "usage": {"input_tokens": 40, "output_tokens": 8}, + "_ocr_toolkit": receipt, + ocr_result.TOOLKIT_ADVISORY_KEY: ocr_result.toolkit_advisory_payload( + ocr_result.background_recommended_advisory( + actual=2_248, + recommended=2_000, + ) + ), + }, + ) + + self.assertEqual(exit_code, 0) + summary = next(body for title, body in notes if not title and "## Open Code Review" in body) + self.assertIn("Review complete with publication filtering — 1 finding published", summary) + self.assertNotIn("Review incomplete", summary) + self.assertNotIn("OCR reported partial coverage", summary) + self.assertNotIn("failed item(s) had no safe", summary) + self.assertIn("Coverage: selected 5; completed 5; reused 0; failed 0; waived 0.", summary) + self.assertIn("The public projection changed", summary) + self.assertNotIn("### Recommended focus areas", summary) + self.assertIn("- all OCR tool calls: 2 total (`file_read`: 2)", summary) + self.assertIn("- token usage: 48 total (input: 40, output: 8)", summary) + self.assertIn("OCR core advisory: background 2248 characters", summary) + self.assertIn("Automatic approval: `not eligible`", summary) + + def test_private_sanitized_review_keeps_tool_and_token_activity_visible(self) -> None: + """Keep independent numeric activity lines after private-only sanitization.""" + + receipt = receipt_v5(author_id=41) + receipt["publication"] = { + "state": "private-sanitized", + "reason_counts": { + "forbidden": 0, + "invalid_text": 0, + "laundering": 0, + "limit": 0, + "pii": 1, + "secret": 0, + }, + "sanitized_fields": 1, + } + notes: list[str] = [] + + def capture_note(_config: Any, _title: str, body: str, *_args: Any) -> dict[str, int]: + notes.append(body) + return {"id": 1} + + with ( + patched_attr( + workflow, "collect_previous_bot_comment_refs", lambda _config: BotCommentRefs() + ), + patched_attr(workflow, "post_review_note_bounded", capture_note), + patched_attr(workflow, "finalize_review_approval", lambda *_args, **_kwargs: 0), + ): + exit_code = workflow.post_results( + gitlab_config(), + { + "status": "complete", + "comments": [], + "warnings": [], + "manifest": { + "schema_version": "ocr.run-manifest/v1", + "operation": "review", + "terminal_state": "complete", + "coverage": { + "selected": [{"item_id": "safe-item"}], + "completed": [{"item_id": "safe-item"}], + "reused": [], + "failed": [], + "waived": [], + }, + }, + "tool_calls": {"total": 3, "by_tool": {"file_read": 3}}, + "usage": {"input_tokens": 20, "output_tokens": 4}, + "_ocr_toolkit": receipt, + }, + ) + + self.assertEqual(exit_code, 0) + self.assertEqual(len(notes), 1) + self.assertIn("- all OCR tool calls: 3 total (`file_read`: 3)", notes[0]) + self.assertIn("- token usage: 24 total (input: 20, output: 4)", notes[0]) + self.assertIn("Private result sanitization signal", notes[0]) + def test_current_summary_readback_requires_one_owned_run_marker(self) -> None: body = build_marked_note_body(markers.build_summary_run_marker(self.RUN_ID) + "\nsummary") notes = [ diff --git a/tests/test_posting_helpers.py b/tests/test_posting_helpers.py index c5cc16e..e8a4d2d 100644 --- a/tests/test_posting_helpers.py +++ b/tests/test_posting_helpers.py @@ -18,9 +18,8 @@ from ocr_toolkit import ocr_result from ocr_toolkit.common.git import isolated_git_environment, read_only_git_prefix -from ocr_toolkit.posting import comments as posting_comments -from ocr_toolkit.posting import formatting as posting_formatting from ocr_toolkit.posting import ( + approval, gitlab, markers, payloads, @@ -30,6 +29,8 @@ snapshot, workflow, ) +from ocr_toolkit.posting import comments as posting_comments +from ocr_toolkit.posting import formatting as posting_formatting from ocr_toolkit.posting.markers import FINGERPRINT_LEN, build_marker from ocr_toolkit.posting.suggestions import SuggestionDecision, SuggestionState from ocr_toolkit.posting.transaction import PostingTransaction @@ -42,7 +43,7 @@ write_pre_execution_status, ) from ocr_toolkit.provider_failure import ProviderFailureReason -from ocr_toolkit.result_contract import CoverageFailure, ReviewOutcome +from ocr_toolkit.result_contract import CoverageFailure, OcrResultContractError, ReviewOutcome from tests.support import ( gitlab_config, patched_attr, @@ -91,6 +92,77 @@ def test_invalid_v5_publication_state_never_reaches_normal_result_flow(self) -> self.assertEqual(exit_code, 0) self.assertEqual(notes, ["**Open Code Review publication policy error**"]) + def test_advisory_without_exact_receipt_v5_never_reaches_normal_result_flow(self) -> None: + """Treat a correctly shaped but unbound advisory as an invalid result.""" + + notes: list[str] = [] + advisory = ocr_result.toolkit_advisory_payload( + ocr_result.background_recommended_advisory(actual=2_100, recommended=2_000) + ) + with ( + patched_attr( + workflow, + "post_review_note_bounded", + lambda _config, title, *_args: notes.append(title) or {"id": 1}, + ), + patched_attr(workflow, "finalize_posting", lambda *_args: True), + redirect_stderr(io.StringIO()), + ): + exit_code = workflow.post_results( + gitlab_config(), + { + "status": "complete", + "comments": [], + "warnings": [], + ocr_result.TOOLKIT_ADVISORY_KEY: advisory, + }, + ) + + self.assertEqual(exit_code, 0) + self.assertEqual(notes, ["**Open Code Review result schema error**"]) + + def test_advisory_parser_rejects_unknown_extended_and_malformed_values(self) -> None: + """Accept no provider text, schema extension, boolean, or impossible threshold.""" + + valid = ocr_result.toolkit_advisory_payload( + ocr_result.background_recommended_advisory(actual=2_100, recommended=2_000) + ) + cases = ( + {}, + {**valid, "provider": "untrusted"}, + {**valid, "kind": "unknown"}, + {**valid, "actual": True}, + {**valid, "actual": 2_000}, + ) + for value in cases: + with self.subTest(value=value): + with self.assertRaises(ocr_result.OcrResultMalformed): + ocr_result.parse_toolkit_advisory(value) + + def test_advisory_renders_only_inside_technical_details(self) -> None: + """Keep the complete status ordinary and render only fixed labels plus numbers.""" + + advisory = ocr_result.background_recommended_advisory(actual=2_100, recommended=2_000) + line = posting_formatting.format_ocr_core_advisory(advisory) + summary = posting_formatting.summarize_result( + total=0, + inline_count=0, + fallback_count=0, + warning_count=0, + outcome_status="success", + ocr_core_advisory_summary=line, + emoji=True, + ) + visible, technical = summary.split("
", 1) + + self.assertIn("✅ **Review complete — no findings**", visible) + self.assertNotIn("OCR core advisory", visible) + self.assertIn( + "- OCR core advisory: background 2100 characters; recommended 2000 characters; " + "accepted by OCR core", + technical, + ) + def test_line_number_rejects_bool_and_non_decimal_values(self) -> None: self.assertEqual(posting_comments.line_number(True), 0) self.assertEqual(posting_comments.line_number("1.5"), 0) @@ -1281,6 +1353,46 @@ def capture_note( "waived": [], }, }, + "_ocr_toolkit": { + "schema_version": 5, + "review": { + "source_sha": "a" * 40, + "policy_sha": "b" * 40, + "mr_author_id": 41, + }, + "context": { + "mode": "off", + "state": "disabled", + "classes": [], + "policy_digest": None, + "per_source": {}, + "degradation_counts": {"invalid": 0, "limit": 0, "unavailable": 0}, + "required_degraded": False, + "mutable_admitted": False, + "tool_usage": {"context_get": 0, "context_list": 0}, + }, + "mcp": { + "capabilities": [ + { + "server": "ocr_toolkit_evidence", + "transport": "builtin", + "tools": ["ocr_toolkit_evidence"], + } + ], + "usage": {}, + }, + "evidence": { + "mandatory": False, + "used": False, + "calls": 0, + "actions": {"state": "unavailable"}, + }, + "publication": {"state": "passed"}, + "cleanup": {"result": "passed"}, + }, + ocr_result.TOOLKIT_ADVISORY_KEY: ocr_result.toolkit_advisory_payload( + ocr_result.background_recommended_advisory(actual=2_100, recommended=2_000) + ), } with ( patched_attr( @@ -1298,6 +1410,13 @@ def capture_note( self.assertEqual(collect_calls, []) self.assertIn("Previous OCR review comments were preserved", notes[0]) self.assertIn("Coverage: selected 1", notes[0]) + visible, technical = notes[0].split("
", 1) + self.assertNotIn("OCR core advisory", visible) + self.assertIn( + "OCR core advisory: background 2100 characters; recommended 2000 characters; " + "accepted by OCR core", + technical, + ) def test_no_comments_note_uses_bounded_publishing(self) -> None: called: list[str] = [] @@ -2710,6 +2829,96 @@ def test_publication_filtering_signal_is_visible_and_machine_readable(self) -> N self.assertIn("Private result sanitization signal", private_details) self.assertIn("approval-relevant review is unchanged", private_details) + def test_filtered_receipt_recovers_only_validated_original_coverage(self) -> None: + """Use original counts for display while suppressing legacy warning fallback.""" + + publication = { + "state": "publication-filtered", + "reason_counts": { + "forbidden": 1, + "invalid_text": 0, + "laundering": 0, + "limit": 0, + "pii": 0, + "secret": 0, + }, + "retained": {"comments": 1, "warnings": 1}, + "omitted": {"comments": 0, "warnings": 0, "fields": 1}, + "original": { + "outcome": "partial", + "selected": 3, + "completed": 2, + "reused": 0, + "failed": 1, + "waived": 0, + }, + } + projected = ReviewOutcome("completed_with_errors", "partial", False) + original = approval.publication_outcome_for_summary(projected, publication) + diagnostics = result.normalize_coverage_diagnostics( + original, + ["provider warning without a path"], + legacy_warning_fallback=False, + ) + + self.assertTrue(original.manifest_present) + self.assertEqual(original.kind, "partial") + self.assertEqual( + original.coverage_summary, + "Coverage: selected 3; completed 2; reused 0; failed 1; waived 0.", + ) + self.assertEqual(diagnostics.failed_total, 0) + self.assertEqual(diagnostics.invalid, 0) + + impossible = { + **publication, + "original": {**publication["original"], "completed": 3}, + } + self.assertIsNone(approval.publication_dlp_state(impossible)) + self.assertIs(approval.publication_outcome_for_summary(projected, impossible), projected) + + with self.assertRaises(OcrResultContractError): + approval.publication_outcome_for_summary( + ReviewOutcome("success", "clean", False), + publication, + ) + + budget_projection = ReviewOutcome("budget_exceeded", "partial", True) + clean_original = { + **publication, + "original": { + "outcome": "clean", + "selected": 3, + "completed": 3, + "reused": 0, + "failed": 0, + "waived": 0, + }, + } + with self.assertRaises(OcrResultContractError): + approval.publication_outcome_for_summary(budget_projection, clean_original) + + def test_one_finding_omits_focus_ranking_while_two_keep_it(self) -> None: + """Rank only when at least two published findings create a useful choice.""" + + one = posting_formatting.format_reviewer_guide( + [{"path": "one.py", "line": 1, "content": "Single finding."}], + 0, + ) + two = posting_formatting.format_reviewer_guide( + [ + {"path": "one.py", "line": 1, "content": "First finding."}, + {"path": "two.py", "line": 2, "content": "Second finding."}, + ], + 0, + ) + + self.assertNotIn("### Recommended focus areas", one) + self.assertNotIn("Single finding.", one) + self.assertIn("### Recommended focus areas", two) + self.assertIn(r"First finding\.", two) + self.assertIn(r"Second finding\.", two) + def test_summary_omits_zero_counts_and_can_disable_emoji(self) -> None: summary = posting_formatting.summarize_result( total=0, @@ -3312,7 +3521,8 @@ def test_reviewer_guide_snippet_is_markdown_neutral(self) -> None: "path": "file.py\n/close", "line": 1, "content": "**bold**\n/merge @all", - } + }, + {"path": "z.py", "line": 2, "content": "Second finding."}, ], 0, ) @@ -3329,7 +3539,8 @@ def test_reviewer_guide_snippet_escapes_html_and_autolinks(self) -> None: "path": "file.py", "line": 1, "content": "See & x", - } + }, + {"path": "z.py", "line": 2, "content": "Second finding."}, ], 0, ) diff --git a/tests/test_review_runner.py b/tests/test_review_runner.py index 36cc897..1ba2141 100644 --- a/tests/test_review_runner.py +++ b/tests/test_review_runner.py @@ -17,7 +17,7 @@ import pytest -from ocr_toolkit import review_runner +from ocr_toolkit import ocr_result, review_runner from ocr_toolkit.context.broker import BrokerResult from ocr_toolkit.context.contracts import RecognizerPolicy from ocr_toolkit.context.policy import parse_policy @@ -726,6 +726,96 @@ def test_publication_projection_blocks_context_copy_secret_pii_and_laundering() assert blocked is True +def test_publication_dlp_allows_tabs_only_in_code_fields_without_skipping_controls( + monkeypatch: pytest.MonkeyPatch, +) -> None: + """Preserve code indentation while every other DLP detector remains authoritative.""" + + safe_comment: dict[str, object] = { + "path": "src/example.py", + "content": "Keep the bounded branch.", + "existing_code": "if ready:\n\treturn current", + "suggestion_code": "if ready:\n\treturn updated", + } + safe_payload: dict[str, object] = { + "status": "success", + "comments": [safe_comment], + "warnings": [], + } + projected, publication, blocked = review_runner._publication_projection( + safe_payload, + forbidden=(), + allowed_tools=frozenset(), + ) + + assert projected is safe_payload + assert publication == {"state": "passed"} + assert blocked is False + decisions = review_runner._private_dlp_decisions(safe_payload, forbidden=()) + assert decisions["decisions"] == [] + + tab_secret = "synthetic\ttab-bearing\tsecret" + monkeypatch.setenv("SYNTHETIC_SECRET", tab_secret) + for value, forbidden in ( + (tab_secret, ()), + ("private\ttab-bearing\tcode", ("private\ttab-bearing\tcode",)), + ): + payload = { + **safe_payload, + "comments": [{**safe_comment, "existing_code": value}], + } + projected, publication, blocked = review_runner._publication_projection( + payload, + forbidden=forbidden, + allowed_tools=frozenset(), + ) + assert projected["comments"][0].get("existing_code") is None + assert publication["state"] == "publication-filtered" + assert blocked is True + + hostile_values = ( + "line\vhidden", + "line\fhidden", + "line\x00hidden", + "line\u202ehidden", + "owner = 'synthetic@example.invalid'", + "Authorization: Bearer synthetic-tab-field-token", + "hidden", + "private code sentence", + ) + for value in hostile_values: + payload = { + **safe_payload, + "comments": [ + { + **safe_comment, + "existing_code": value, + } + ], + } + projected, publication, blocked = review_runner._publication_projection( + payload, + forbidden=("private code sentence",), + allowed_tools=frozenset(), + ) + assert publication["state"] == "publication-filtered" + assert blocked is True + assert value not in json.dumps(projected) + + content_with_tab = { + **safe_payload, + "comments": [{"content": "Finding\twith hidden layout"}], + } + projected, publication, blocked = review_runner._publication_projection( + content_with_tab, + forbidden=(), + allowed_tools=frozenset(), + ) + assert projected["comments"] == [] + assert publication["reason_counts"]["invalid_text"] == 1 + assert blocked is True + + def test_publication_dlp_retains_only_safe_local_findings_and_closed_receipt( tmp_path: Path, ) -> None: @@ -818,10 +908,10 @@ def test_publication_dlp_retains_only_safe_local_findings_and_closed_receipt( assert publication["omitted"] == {"comments": 1, "warnings": 1, "fields": 2} -def test_background_preview_warning_is_atomically_finalized_and_blocks_approval( +def test_background_preview_advisory_is_atomically_finalized_without_blocking_approval( tmp_path: Path, ) -> None: - """Make an installed-OCR soft warning visible to summary and approval owners.""" + """Keep the accepted numeric advisory outside warnings, DLP, and approval inputs.""" result = tmp_path / "result.json" result.write_text( @@ -853,10 +943,7 @@ def test_background_preview_warning_is_atomically_finalized_and_blocks_approval( external_servers=(), secret_values=(), ) - warning = ( - "Installed OCR reported a 2100-character review background above its recommended " - "2000-character threshold; review continued." - ) + advisory = ocr_result.background_recommended_advisory(actual=2_100, recommended=2_000) _usage, blocked, publication = review_runner._finalize_ocr_result( result, @@ -864,11 +951,18 @@ def test_background_preview_warning_is_atomically_finalized_and_blocks_approval( replace(DEFAULT_IDENTITY, mr_author_id=41), None, forbidden=(), - toolkit_warnings=(warning, warning), + toolkit_advisory=advisory, ) persisted = json.loads(result.read_text(encoding="utf-8")) - assert persisted["warnings"] == [warning] + assert persisted["warnings"] == [] + assert persisted[ocr_result.TOOLKIT_ADVISORY_KEY] == { + "schema_version": "ocr.toolkit-advisory/v1", + "kind": "background_recommended_limit", + "actual": 2_100, + "recommended": 2_000, + "unit": "characters", + } assert publication == {"state": "passed"} assert blocked is False outcome = parse_result_outcome(persisted) @@ -880,13 +974,74 @@ def test_background_preview_warning_is_atomically_finalized_and_blocks_approval( 0, persisted["_ocr_toolkit"], ) - assert eligibility.eligible is False - assert eligibility.result.reason == "the OCR review reported warnings" + assert eligibility.eligible is True serialized = result.read_text(encoding="utf-8") + assert "Installed OCR reported" not in serialized assert "private discussion sentence" not in serialized assert "synthetic@example.invalid" not in serialized +def test_raw_ocr_result_cannot_supply_the_reserved_toolkit_advisory(tmp_path: Path) -> None: + """Reject even a well-shaped advisory before receipt and publication policy run.""" + + result = tmp_path / "result.json" + result.write_text( + json.dumps( + { + ocr_result.TOOLKIT_ADVISORY_KEY: ocr_result.toolkit_advisory_payload( + ocr_result.background_recommended_advisory( + actual=2_100, + recommended=2_000, + ) + ) + } + ), + encoding="utf-8", + ) + composition = MCPComposition( + payload={}, + capabilities=(MCPCapability("ocr_toolkit_evidence", ("ocr_toolkit_evidence",), True),), + external_servers=(), + secret_values=(), + ) + + with pytest.raises(review_runner.ReviewRunnerError, match="not valid bounded JSON"): + review_runner._finalize_ocr_result( + result, + composition, + replace(DEFAULT_IDENTITY, mr_author_id=41), + None, + forbidden=(), + ) + + assert ocr_result.TOOLKIT_ADVISORY_KEY in result.read_text(encoding="utf-8") + + +def test_raw_ocr_result_rejects_duplicate_reserved_advisory_keys(tmp_path: Path) -> None: + """Fail closed before JSON duplicate-key collapse can select an advisory value.""" + + result = tmp_path / "result.json" + result.write_text( + '{"_ocr_toolkit_advisory":{},"_ocr_toolkit_advisory":{}}', + encoding="utf-8", + ) + composition = MCPComposition( + payload={}, + capabilities=(MCPCapability("ocr_toolkit_evidence", ("ocr_toolkit_evidence",), True),), + external_servers=(), + secret_values=(), + ) + + with pytest.raises(review_runner.ReviewRunnerError, match="not valid bounded JSON"): + review_runner._finalize_ocr_result( + result, + composition, + replace(DEFAULT_IDENTITY, mr_author_id=41), + None, + forbidden=(), + ) + + def test_publication_dlp_ignores_private_non_rendered_identifiers_but_filters_sinks() -> None: payload: dict[str, object] = { "status": "complete", @@ -1336,6 +1491,56 @@ def test_unsafe_manifest_failure_detail_is_partial_and_destroyed() -> None: assert "private failure detail" not in json.dumps(projected) +def test_filtered_budget_projection_preserves_the_original_stop_state() -> None: + """Keep a real token-budget stop stronger than independent publication filtering.""" + + payload: dict[str, object] = { + "status": "partial", + "summary": {"budget_exceeded": True}, + "comments": [{"content": "Safe finding."}], + "warnings": [], + "manifest": { + "schema_version": "ocr.run-manifest/v1", + "operation": "review", + "terminal_state": "partial", + "coverage": { + "selected": [{"item_id": "safe-a"}, {"item_id": "safe-b"}], + "completed": [{"item_id": "safe-a"}], + "reused": [], + "failed": [ + { + "item_id": "safe-b", + "path": "src/private.py", + "classification": "budget", + "reason": "private failure detail", + } + ], + "waived": [], + }, + }, + } + projected, publication, blocked = review_runner._publication_projection( + payload, + forbidden=("private failure detail",), + allowed_tools=frozenset(), + ) + projected_outcome = parse_result_outcome(projected) + summary_outcome = approval.publication_outcome_for_summary( + projected_outcome, + publication, + ) + + assert blocked is True + assert projected["status"] == "budget_exceeded" + assert projected["summary"] == {"budget_exceeded": True} + assert projected_outcome.budget_exceeded is True + assert summary_outcome.kind == "partial" + assert summary_outcome.budget_exceeded is True + assert summary_outcome.coverage_summary == ( + "Coverage: selected 2; completed 1; reused 0; failed 1; waived 0." + ) + + def test_publication_dlp_atomically_sanitizes_private_fields_without_losing_manifest( tmp_path: Path, ) -> None: @@ -2181,14 +2386,14 @@ def test_review_rejects_caller_owned_output(arguments: list[str]) -> None: @pytest.mark.parametrize( - ("returncode", "stderr", "warning", "notices", "rejection"), + ("returncode", "stderr", "advisory", "notices", "rejection"), [ (0, b"", None, (), None), ( 0, b"[ocr] --background-file content is 2001 characters, exceeding the recommended " b"2000 (continuing but review quality might be impacted)\n", - "2001-character review background above its recommended 2000-character threshold", + (2_001, 2_000), (), None, ), @@ -2204,7 +2409,7 @@ def test_review_rejects_caller_owned_output(arguments: list[str]) -> None: b"[ocr] --max-tools 30 is below minimum 50, using 50\n" b"[ocr] --background-file content is 2001 characters, exceeding the recommended " b"2000 (continuing but review quality might be impacted)\n", - "2001-character review background above its recommended 2000-character threshold", + (2_001, 2_000), ("--max-tools 30", "normalization target of 50", "template remains authoritative"), None, ), @@ -2229,7 +2434,7 @@ def test_review_rejects_caller_owned_output(arguments: list[str]) -> None: def test_background_preview_parser_accepts_only_closed_installed_ocr_forms( returncode: int, stderr: bytes, - warning: str | None, + advisory: tuple[int, int] | None, notices: tuple[str, ...], rejection: tuple[str, int, int, str] | None, ) -> None: @@ -2237,9 +2442,11 @@ def test_background_preview_parser_accepts_only_closed_installed_ocr_forms( if rejection is None: result = review_runner._parse_background_preview(returncode=returncode, stderr=stderr) - assert warning is None or warning in result.warning # type: ignore[operator] - if warning is None: - assert result.warning is None + if advisory is None: + assert result.advisory is None + else: + assert result.advisory is not None + assert (result.advisory.actual, result.advisory.recommended) == advisory for expected in notices: assert expected in result.operator_notices[0] if not notices: @@ -2322,7 +2529,10 @@ def fake_run(argv: list[str], **kwargs: object) -> subprocess.CompletedProcess[b ) assert argv_seen == ["/private/ocr", "review", *production_args, "--preview"] - assert result.warning is not None + assert result.advisory == ocr_result.OcrToolkitAdvisory( + actual=2_100, + recommended=2_000, + ) assert list(session_home.iterdir()) == [] @@ -2370,7 +2580,7 @@ def fail_after_cleanup(path: Path) -> None: ) assert exit_code == 2 - assert qualification.warning is None + assert qualification.advisory is None assert model_calls == [] persisted = json.loads(artifacts.pre_execution_status.read_text(encoding="utf-8")) assert (persisted["actual"], persisted["limit"], persisted["unit"]) == ( @@ -2446,7 +2656,7 @@ def reject(*_args: object, **_kwargs: object) -> review_runner.BackgroundQualifi ) assert exit_code == 2 - assert qualification.warning is None + assert qualification.advisory is None assert model_calls == [] persisted = json.loads(artifacts.pre_execution_status.read_text(encoding="utf-8")) assert persisted == { @@ -2676,7 +2886,7 @@ def write_bootstrap(path: Path, content: str) -> None: else: if ocr_exit_code == 0: assert events[9] == "ocr-usage" - assert finalized[0]["toolkit_warnings"] == () + assert finalized[0]["toolkit_advisory"] is None else: assert "ocr-usage" not in events assert not artifacts.store.exists() diff --git a/tests/test_runtime_helpers.py b/tests/test_runtime_helpers.py index 8949138..ce7e254 100644 --- a/tests/test_runtime_helpers.py +++ b/tests/test_runtime_helpers.py @@ -1476,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.10.0\n", stderr="" + args=["ocr", "--version"], returncode=0, stdout="ocr 1.10.1\n", stderr="" ) with ( patched_attr(preflight.shutil, "which", lambda _name: "/usr/bin/ocr"),