Repository navigation
fix(OMN-18956): arm the evidence refusal from one derivation, not two literals - #3904
Conversation
… literals The first fix moved the validator's two flags off literals and left an identical pair on the receipt CALLBACK, which calls _write_local_run_files and re-runs the same refusal. Both tests passed, the PR merged, and the next delegate on the lane failed with the message it always had: receipt callback failed: completed delegation terminal omits required evidence: budget_evidence, response_contract_evidence Two entry points share one refusal and only one of them was moved. The integration module's own docstring warned that proving the leaf is not proving the entry; it drove the validator and the surviving literal sat on the callback beside it. Both sites now read the same derivation, computed once. Adds a guard that parses the module and fails on any literal True for either flag, with a positive control that the parser finds the flags at all, so a third call site is caught when it is written rather than when a lane run disagrees with its own terminal. Proven both directions: green as written, red with the literals restored.
#10627) * evidence: OCC companion pass 1 for OmniNode-ai/omnibase_infra#3904 * evidence: OCC companion self-bind for #10627 --------- Co-authored-by: node-occ-companion-effect <occ-companion-effect@omninode.ai>
There was a problem hiding this comment.
Hostile Reviewer — adversarial findings (OMN-17492)
Models succeeded: glm-review
Models failed: codex
New finding threads: 0
Deduped (already posted on this PR): 0
Below quorum (one model only, reported not threaded): 6
Nit-level findings suppressed: 1
The model is the FINDER, never the gate: merge is gated only by the
deterministic Hostile Review Thread Gate, which blocks while
hostile-reviewer threads are unresolved. Resolve each thread after
addressing (or rejecting, with a reply) its finding.
Below quorum: 6 finding(s) raised by one model only (OMN-18479)
These are reported and NOT dropped, but they get no thread and do not block: a single model's finding no other model reproduced is not evidence enough to stop a merge. Read them; act on them if they are right.
- [MAJOR]
src/omnibase_infra/cli/cli_delegate.py:1968(glm-review) — receipt_evidence_demanded provenance and mutation contract unverified | The diff swaps literals for receipt_evidence_demanded[0] and [1] but the diff contains no definition or mutation site for this s - [MAJOR]
src/omnibase_infra/cli/cli_delegate.py:1976-1977(glm-review) — Positional indexing obscures flag-to-slot mapping | receipt_evidence_demanded[0] for budget and [1] for contract is a positional contract with no enforcement. A reordering of the underlying derivation - [MAJOR]
tests/unit/cli/test_no_literal_evidence_demand_omn18956.py:52-54(glm-review) — AST lint test enforces style, not the behavior regression it claims to prevent | The test asserts no keyword argument is the literal Constant True. It passes for truthy literals that defeat the fix ju - [MINOR]
tests/unit/cli/test_no_literal_evidence_demand_omn18956.py:37-43(glm-review) — Test reads source at repo-layout-relative path with no existence handling | _MODULE resolves via parents[3] against the test file location. Any relocation of either the test module or cli_delegate.py, - [MINOR]
src/omnibase_infra/cli/cli_delegate.py:1971-1975(glm-review) — Comment documents defect history instead of contract | The inline comment spends eight lines narrating ticket archaeology ('the SECOND site', 'the first fix moved only the validator') and none stating - [MINOR]
src/omnibase_infra/cli/cli_delegate.py:1968(glm-review) — No behavioral test that the callback honors the derived flags | The diff's stated failure was a runtime exit: a run 'exited non-zero after the validator had accepted it'. The new tests verify only tha
|
| Surface | Meaning | Blocks merge? |
|---|---|---|
| Review threads | Per-finding, posted by the reviewer | No (informational) |
Hostile Review Thread Gate |
Deterministic: unresolved hostile-reviewer threads exist | Fails until resolved (not yet a required context) |
degraded verdict |
Fewer than 2 models succeeded (infra) | No |
Powered by omniintelligence.review_pairing.cli_review — multi-model adversarial review: qwen3-review, qwen3-review-b, glm-review (OMN-8468/OMN-8524/OMN-17492)
…d in The integration module drove the receipt VALIDATOR and stayed green while the receipt CALLBACK still refused every completed terminal, which is how the half-applied fix merged. Two entry points share one refusal and only one was covered. Adds both cases on the callback: the lane's own captured terminal is written without raising, and the same terminal is still refused when a contract was requested and its evidence is absent.
There was a problem hiding this comment.
Hostile Reviewer — adversarial findings (OMN-17492)
Models succeeded: glm-review
Models failed: codex
New finding threads: 0
Deduped (already posted on this PR): 0
Below quorum (one model only, reported not threaded): 5
Nit-level findings suppressed: 1
The model is the FINDER, never the gate: merge is gated only by the
deterministic Hostile Review Thread Gate, which blocks while
hostile-reviewer threads are unresolved. Resolve each thread after
addressing (or rejecting, with a reply) its finding.
Below quorum: 5 finding(s) raised by one model only (OMN-18479)
These are reported and NOT dropped, but they get no thread and do not block: a single model's finding no other model reproduced is not evidence enough to stop a merge. Read them; act on them if they are right.
- [MAJOR]
src/omnibase_infra/cli/cli_delegate.py, run_delegate(glm-review) — Single-element list used as mutable shared state without visible provenance | receipt_evidence_demanded[0] and receipt_evidence_demanded[1] are read via index with no indication in this diff of where - [MAJOR]
tests/unit/cli/test_no_literal_evidence_demand_omn18956.py(glm-review) — AST-based literal scan only detects ast.Constant True, not truthy literals | The no-literal scan checks isinstance(value, ast.Constant) and value.value is True. It misses1(truthy int), `ast.UnaryO - [MAJOR]
tests/integration/cli/test_receipt_evidence_envelope_omn18956.py(glm-review) — Positive control on callback path relies on error type, not callback wiring | test_the_receipt_callback_still_refuses_a_requested_contract calls _write_local_run_files directly and asserts a DelegateT - [MINOR]
tests/unit/cli/test_no_literal_evidence_demand_omn18956.py(glm-review) — File-path-based test assumes source layout and module path stability | Path(file).resolve().parents[3] / "src" / ... hard-codes the repository layout. Any restructuring (src-less layout, namespace - [MINOR]
docker/runners/runner-image.lock.json(glm-review) — Diff changes lock digests without accompanying regeneration evidence | identity_digest, shared_env_digest, and the package version bump (0.38.46 -> 0.38.47) appear in the diff, but uv.lock changes onl
My previous fix for this ticket was incomplete and the lane readback caught it. This finishes it.
What was missed
#3896moved the receipt VALIDATOR's two flags off literals and left an identical pair on the receipt CALLBACK, a few lines below it, which calls_write_local_run_filesand re-runs the same refusal. Two entry points share one refusal; one of them was moved.So the merge changed nothing observable. The next delegate on the lane, after the clone was pulled and the dispatch venv reconciled onto the merged version, failed with exactly the message it always had:
Both the unit and integration modules were green throughout. The integration module's own docstring says a fix proven at the leaf can still be wrong at the entry the CLI wires, which was the right instinct and still missed this: it drove the
receipt_validatorand the surviving literal was on thereceipt_callbackbeside it.What changes
Both sites read the same derivation, computed once before either is wired, so they cannot disagree about what the request asked for.
The guard, and why it reads source
A test that parses the module and fails on any literal
Truefor either flag, with a positive control asserting the parser finds the flags at all so an empty scan cannot green it. Proven both directions before pushing: green as written, and red naming both line numbers with the literals restored.Reading source text is usually a poor substitute for asserting behaviour. Here it is the right instrument, because the defect is precisely a call site nobody thought to exercise, and a behavioural test can only cover the paths its author already has in mind. That limitation is what shipped the residual, so the guard is written not to have it.
Proof
752 tests green in
tests/unit/cli/plus the integration module,ruffclean, whole-tree hooks green, runner image identity regenerated because the version bump restales it.Owed
The lane readback that found this: a fresh delegate whose exit status is zero on a completed terminal. It needs this merged and the dispatch venv reconciled, and it is cited on the ticket when taken. I am not calling the ticket proven until that run exits zero, having already called it proven once on a fix that was half applied.
Evidence-Ticket: OMN-18956
Evidence-Source: OCC#10627