Repository navigation
fix(OMN-16787): a worktree that could not be materialised is not "no contract" - #2169
Conversation
…contract" `EvidenceCollector._materialize_occ_dev_worktree` has two failure classes. OMN-15454 closed one: a failed `git fetch` refuses. The other — a failed or timed-out `git worktree add` — returned `(None, None, ...)` and the caller silently fell through to `_DEFAULT_CONTRACT_ROOTS`, the `main`-tracking working tree, while the run still stamped `occ_governance_ref: origin/dev`. OCC `dev` runs ~4,159 commits and ~2,184 contracts ahead of `main`, so a dev-only contract is invisible to that fallback and the run reports `CONTRACT_MISSING`. "The clone could not be read at the ref I claim I read" was being laundered into "no contract exists" — a false-negative generator on the only sanctioned Done-flip tool. Measured 2026-08-27: 3 of 22 `CONTRACT_MISSING` results in the beta sweep were this artifact, with the canonical clone 33 commits behind origin/dev. The trigger is ordinary load, not a rare fault. A real `git worktree add --detach origin/dev` of the live OCC repo checks out 32,382 files in ~34.5 s single-threaded, against a 60 s ceiling, under a 5-way-parallel sweep. The residue is visible: 405 worktree registrations against 42 surviving directories on the canonical clone — orphaned admin records from adds that timed out and had their temp dir rmtree'd. So both halves are fixed: * the worktree-materialisation failure now refuses with a distinct `OCC_WORKTREE_UNAVAILABLE` result, honouring the same `DOD_VERIFY_ALLOW_STALE_OCC_REF` override, which (as with FETCH_FAILED) proceeds but marks every result un-attributable plus a standalone disclosure item. Never a silent working-tree read under a dev-ref banner. * the git-op ceiling moves to 300 s and becomes operator-configurable via `DOD_VERIFY_GIT_OP_TIMEOUT_S`, so hitting it means something is genuinely wrong rather than that five verifiers shared a clone. The `refresh_outcome is not None` guard is load-bearing: when no OCC root resolves at all, no fetch is attempted, and that legitimate pre-existing shape keeps its existing reporting rather than being relabelled a fault. Also repairs OMN-15454's own AC3 fixture, which was green for the wrong reason. It built a repo with a LOCAL `dev` branch and no remote, then asked the collector to resolve `origin/dev`; the worktree add failed with `invalid reference` and the assertion only passed because the collector fell back to the working tree, which carried the same contract. It asserted "materialised at the post-fetch tip" while nothing was ever materialised. The fixture now creates a real remote, so the assertion means what it says — and the fail-open it was blind to is exactly what this ticket closes. Refs: OMN-16787
|
Warning Review limit reachedNext included review available in 55 minutes. View limit detailsLimit details: You’ve used the included review currently available. Your 131 included PR review attempts over the past 7 days set your current allowance at 1 review per hour. Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. Review configuration: ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
Comment |
|
| Rule | Description |
|---|---|
| ARCH-TOPIC-001 | No hardcoded Kafka topic strings in handler code |
| ARCH-DI-001 | No event_bus=None bypass |
| ARCH-DI-002 | No direct Handler instantiation outside workflow_runner/adapters |
| ARCH-DI-003 | No reinvented DI containers |
| ARCH-TOPIC-002 | contract.yaml topics follow `onex.{cmd |
ARCH-DI-002: Direct Handler instantiation (warning)\n\n```\nsrc/omnimarket/nodes/node_dod_verify/services/evidence_collector.py:2635: output = HandlerDodEvidenceGithubEffect().handle(command)
src/omnimarket/nodes/node_dod_verify/services/evidence_collector.py:2671: output = HandlerDodEvidenceGithubEffect().handle(command)
src/omnimarket/nodes/node_dod_verify/services/evidence_collector.py:3245: output = HandlerDodEvidenceGithubEffect().handle(command)
src/omnimarket/nodes/node_dod_verify/services/evidence_collector.py:3276: output = HandlerDodEvidenceGithubEffect().handle(command)\n```\n\nHandler classes should be wired only in workflow_runner.py or adapter modules. Direct instantiation elsewhere bypasses the contract system.\n\n
Static architectural lint — no model inference (OMN-14176).
✅ Hostile Reviewer — PASSEDBlocking findings (critical/error): 0 Gate semantics
Powered by omniintelligence.review_pairing.cli_review — node-based adversarial review via HandlerLlmCliSubprocess (OMN-8524) |
…7332) * evidence: OCC companion pass 1 for OmniNode-ai/omnimarket#2169 * evidence: OCC companion self-bind for #7332 --------- Co-authored-by: node-occ-companion-effect <occ-companion-effect@omninode.ai>
1 participant
The defect
EvidenceCollector._materialize_occ_dev_worktreehas two independent failure classes. OMN-15454 closed one of them.git fetchfailed(dev_root, wt, FETCH_FAILED, sha)OCC_REF_REFRESH_FAILED, override viaDOD_VERIFY_ALLOW_STALE_OCC_REFgit worktree addfailed / timed out(None, None, refresh_outcome, None)_DEFAULT_CONTRACT_ROOTS, themain-tracking working tree, while the run still stampsocc_governance_ref: origin/devOCC
devis ~4,198 commits and ~2,189 contracts ahead ofmain, so a dev-only contract is invisible to that fallback and the run returns a single skippedcontractcheck, whichhandler_dod_verifyreports asCONTRACT_MISSING."The clone could not be read at the ref I claim I read" was being laundered into "no contract exists" — a false-negative generator on the only sanctioned Done-flip tool.
Measured, not inferred (2026-08-27)
CONTRACT_MISSINGresults in the beta DoD-verification sweep were this artifact. OMN-16690, OMN-16759 and OMN-16761 all have contracts onorigin/devand produced real verdicts when re-run against a detached worktree ata48d3c8b9.$OMNI_HOME/onex_change_controlwas[behind 33]at the time.git worktree add --detach --force <tmp> origin/devchecks out 32,382 files in 34.5 s single-threaded._GIT_OP_TIMEOUT_Swas 60 s, and the sweep runsdod_verify5-way parallel against the same clone. The ceiling was tripping on ordinary load.git worktree liston the canonical OCC clone shows 405 registrations against 42 surviving directories — 363 orphaned admin records, the exact signature of an add that timed out and had its temp dirrmtree'd withoutgit worktree remove.The fix
OCC_WORKTREE_UNAVAILABLEFAILED result naming the ref and the override, honouring the sameDOD_VERIFY_ALLOW_STALE_OCC_REFescape hatch — which proceeds but marks every result un-attributable and appends a standalone disclosure item.DOD_VERIFY_GIT_OP_TIMEOUT_S, so hitting the new refusal means something is genuinely wrong rather than that five verifiers shared a clone.The
refresh_outcome is not Noneguard is load-bearing: when no OCC root resolves at all no fetch is attempted, and that legitimate pre-existing shape (the CI shape) keeps its existing reporting rather than being relabelled a fault.A pre-existing test that was green for the wrong reason
test_omn_15454_occ_ref_refresh_fail_closed.py::test_ref_lock_race_is_retried_once_ac3built a repo with a localdevbranch and no remote, then asked the collector to resolveorigin/dev. The worktree add failed withinvalid reference, and the assertion only passed because the collector fell back to the working tree — which carried the same contract, sincemainwas branched offdev. It asserted "materialised at the post-fetch tip" while nothing was ever materialised.The fixture now creates a real remote so
origin/devgenuinely exists and the assertion means what it says. The fail-open that test was blind to is exactly what this PR closes.Tests (TDD, red first)
New
test_omn_16787_occ_worktree_unavailable_fail_closed.py, driving the realcollect()path with real git subprocesses:CONTRACT_MISSING.UNATTRIBUTABLE, and appendsocc_worktree_unavailable_override.CONTRACT_MISSING, never the new refusal.DOD_VERIFY_GIT_OP_TIMEOUT_S=0a realsubprocess.TimeoutExpiredfrom a realgit worktree addconverges on the same refusal, against a fixture whose genuineorigin/devcarries the contract (so a silent fallback would have produced a fully VERIFIED table).origin/devworktree still resolves dev-first and records a real 40-char SHA.Verified red before the implementation (ImportError on the new symbols), green after: 12 passed across the new file and the OMN-15454 suite.
ruff format,ruff check,mypy --strictclean; the governed pre-push selector ran 7,974 tests.Refs: OMN-16787
Evidence-Ticket: OMN-16787
Evidence-Source: OCC#7332