Repository navigation
fix(OMN-17871): bind primary artifact metadata evidence - #3217
Conversation
There was a problem hiding this comment.
Hostile Reviewer — adversarial findings (OMN-17492)
Models succeeded: glm-review
Models failed: codex
New finding threads: 7
Deduped (already posted on this PR): 0
Nit-level findings suppressed: 0
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.
Findings demoted from threads (anchor rejected)
-
[CRITICAL] hostile-reviewer (glm-review)
Schema hardcodes document-specific hashes as Literal types | ModelRsdArtifactSourceRelation pins every field to exact Literal values, including two SHA-256 digests and a model ID. The class no longer validates a schema; it validates exactly one document. Any provenance record for a different artifact revision, a re-uploaded model card, or a different base model requires a source code change and redeploy. This couples data to code in a way that defeats the purpose of a validation model and will fail on the f
Resolve this thread when addressed — the
Hostile Review Thread Gateblocks while hostile-reviewer threads are unresolved (OMN-17492). -
[MAJOR] hostile-reviewer (glm-review)
v1 to v2 migration rejects all previously valid records without a compatibility story | schema_version and signature_domain Literals were narrowed from v1 to v2 and AUTHORITY_DOMAIN was bumped. Any persisted or in-flight v1 provenance document, and any signature produced under the v1 domain, now fails validation with no documented migration, dual-read window, or rejection-reason distinction. The plan contains no evidence the v1 schema is dead or that no v1 records exist in storage. | Evidence: - schema
Resolve this thread when addressed — the
Hostile Review Thread Gateblocks while hostile-reviewer threads are unresolved (OMN-17492). -
[MAJOR] hostile-reviewer (glm-review)
No test asserts v1 documents and signatures are rejected | The negative test matrix was extended for artifact_source_relation but no test feeds a v1 schema_version, a v1 signature_domain, or a signature produced over the v1 preimage and asserts rejection. Given the domain-separator bump silently invalidates all v1 signatures, this is the single most important negative case in the diff and it is absent. | Evidence: The diff modifies _AUTHORITY_DOMAIN and both Literal fields but adds tests only for artifact_s
Resolve this thread when addressed — the
Hostile Review Thread Gateblocks while hostile-reviewer threads are unresolved (OMN-17492). -
[MAJOR] hostile-reviewer (glm-review)
Sentinel signature not recomputed for the new signing domain | signature_domain changed to v2, which changes the signing preimage, but signature_base64 is byte-identical to the prior example. The comment says an operator must issue a real signature before approval, yet the shipped example now pairs a v2 domain with a signature that could not have been produced over the v2 preimage by any key. If any verification path short-circuits on the sentinel key fingerprint, the document ships an internally inconsiste
Resolve this thread when addressed — the
Hostile Review Thread Gateblocks while hostile-reviewer threads are unresolved (OMN-17492). -
[MINOR] hostile-reviewer (glm-review)
Domain separator duplicated between model Literal and signing constant | The string omninode-rsd.model-artifact-provenance.v2 now exists in two places: the signature_domain Literal in the model and the _AUTHORITY_DOMAIN bytes in the runtime module. Nothing ties them together; a future version bump that updates one but not the other produces documents whose declared domain differs from the domain actually signed, which is exactly the class of bug this diff is trying to close. | Evidence: signature_domain: Li
Resolve this thread when addressed — the
Hostile Review Thread Gateblocks while hostile-reviewer threads are unresolved (OMN-17492). -
[MINOR] hostile-reviewer (glm-review)
Tamper matrix omits source_config_sha256 and artifact_metadata_authority | The new negative test covers four relation fields but not source_config_sha256, artifact_config absence aside, artifact_metadata_authority, or artifact_embedded_source_model_card_sha256. The latter is particularly relevant since its security meaning (embedded card matches upstream card) is the invariant the diff comments emphasize. | Evidence: ("relation_status", "verified"), ("approval_status", "approved"), ("artifact_declared_base_
Resolve this thread when addressed — the
Hostile Review Thread Gateblocks while hostile-reviewer threads are unresolved (OMN-17492). -
[MINOR] hostile-reviewer (glm-review)
relation_status vocabulary defined only by a single Literal | relation_status: Literal["publisher-declared-unverified"] admits exactly one state. A provenance relation enum with no 'verified' or 'revoked' members cannot express state transitions without a schema bump, which this diff demonstrates is a breaking, signature-invalidating operation. The type system is doing the work a versioned enum should. | Evidence: relation_status: Literal["publisher-declared-unverified"] | Fix: Define the full closed vocabu
Resolve this thread when addressed — the
Hostile Review Thread Gateblocks while hostile-reviewer threads are unresolved (OMN-17492).
|
| 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)
#8303) * evidence: OCC companion pass 1 for OmniNode-ai/omnibase_infra#3217 * evidence: OCC companion self-bind for #8303 --------- 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: 6
Deduped (already posted on this PR): 0
Nit-level findings suppressed: 0
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.
Findings demoted from threads (anchor rejected)
-
[MAJOR] hostile-reviewer (glm-review)
Single-artifact literals baked into a reusable-looking model | ModelRsdArtifactSourceRelation pins every field, including SHA-256 digests and the base model id, as Literal types. This makes the class a frozen snapshot of one fixture rather than a schema. Any second artifact, or a re-upload that changes a HuggingFace file, requires editing Python source. The same defect exists in the parent model, but this diff deepens it by adding nine more hardcoded constants. | Evidence: source_model_card_sha256: Literal[
Resolve this thread when addressed — the
Hostile Review Thread Gateblocks while hostile-reviewer threads are unresolved (OMN-17492). -
[MAJOR] hostile-reviewer (glm-review)
Schema version bump has no compatibility story for v1 documents | Both the pydantic Literal and _AUTHORITY_DOMAIN were bumped from v1 to v2 in the same commit. Any persisted or in-flight v1 provenance document will now fail validation with no migration path, dual-read window, or documented rejection rationale. If the intent is fail-closed rejection of v1, that decision is undocumented in the diff. | Evidence: schema_version: Literal["rsd.model-artifact-provenance.v2"] and _AUTHORITY_DOMAIN = b"omninode-rsd.
Resolve this thread when addressed — the
Hostile Review Thread Gateblocks while hostile-reviewer threads are unresolved (OMN-17492). -
[MAJOR] hostile-reviewer (glm-review)
Tamper-coverage matrix omits half the new nested fields | The new parametrized rejection test covers relation_status, approval_status, artifact_declared_base_model_id, and source_model_card_sha256, but not source_config_sha256, source_metadata_authority, artifact_metadata_authority, artifact_model_card_sha256, or artifact_embedded_source_model_card_sha256. Combined with the Literal-typed model this is largely defense in depth, but the existing matrix pattern in test_exact_candidate_bindings_reject_substitut
Resolve this thread when addressed — the
Hostile Review Thread Gateblocks while hostile-reviewer threads are unresolved (OMN-17492). -
[MINOR] hostile-reviewer (glm-review)
Roundtrip coverage claim is asserted, not verified | The serialization-roundtrip exemption registry claims ModelRsdArtifactSourceRelation has 'Strict primary-metadata and non-approval coverage' in test_rsd_model_artifact_provenance.py. The diff adds field-value assertions and tamper tests, but no canonical-JSON serialization roundtrip of the nested model. A strict nested pydantic model can serialize fields (e.g., ordering or aliasing) in a way that diverges from canonical form; nothing in the diff tests thi
Resolve this thread when addressed — the
Hostile Review Thread Gateblocks while hostile-reviewer threads are unresolved (OMN-17492). -
[MINOR] hostile-reviewer (glm-review)
Signature domain version and schema version decoupled without cross-check | signature_domain and schema_version are bumped in lockstep but nothing enforces their coupling. A v2 schema document carrying signature_domain v1 would fail only via the Literal on signature_domain; conversely, the signing preimage uses _AUTHORITY_DOMAIN from the module constant, not from the document. If a v1 document were ever re-validated by an older reader against v2 domain bytes, mismatched domain constants across deployments p
Resolve this thread when addressed — the
Hostile Review Thread Gateblocks while hostile-reviewer threads are unresolved (OMN-17492). -
[MINOR] hostile-reviewer (glm-review)
Nested approval_status semantics unverified against parent approval workflow | The nested relation carries approval_status: not-approved while the parent carries approval_status: unapproved. Two approval vocabularies in one document invites divergence; nothing in the diff defines whether an approval flow must transition both fields consistently, or whether the nested status is informational. | Evidence: relation_status: publisher-declared-unverified / approval_status: not-approved (nested) vs approval_statu
Resolve this thread when addressed — the
Hostile Review Thread Gateblocks while hostile-reviewer threads are unresolved (OMN-17492).
Follow-up to merged #3213.
Provenance state (non-authorizing)
The relation is explicitly publisher-declared-unverified and the approval state is explicitly not-approved. execute_enabled=false. This record cannot authorize artifact use, deployment, runtime activation, or a provider call.
Exact scope
This PR holds immutable source/artifact metadata evidence for the Qwen3.8 artifact binding and substitution/absence tests. It does not assert a verified source-weight derivation or approved conversion: official metadata available for the exact revisions does not provide such an attestation.
Required future dependency
A separately owned C1/pre-provider activation authority must consume an independently verified and approved source-to-artifact conversion attestation before it can authorize execution. No current in-scope consumer exists; this PR intentionally does not invent adapter, C1, provider, canary, deployment, runtime, or database behavior.
Validation
Evidence-Ticket: OMN-17871
Evidence-Source: OCC#8303