Repository navigation
feat(OMN-18900): create dod_verify_runs, the durable definition-of-done verdict table - #3883
Merged
jonahgabriel merged 11 commits intoSep 20, 2026
Merged
Conversation
…ne verdict table Vendors the two node-owned migrations for omnimarket's new `node_projection_dod_verdict` and declares the grant they deliver. This lands FIRST, before the omnimarket side, because omnimarket's node-migration-vendor-parity-gate fails a pull request touching `migrations/*.sql` unless the vendored copy already exists at this repo's dev tip. Vendor first, declare second (the OMN-18768 ordering trap). WHY THE TABLE EXISTS `node_dod_verify` produces a per-ticket, machine-readable, check-level verdict and throws it away. On 2026-09-20 the entire durable population of that verdict on this fleet was nine ad-hoc JSON files in one developer's scratch directory; every other verdict ever produced is gone. The table named in the only comment that mentions one did not exist: nothing created it and nothing wrote it. One row per RUN, keyed on ticket, correlation id and completion time. The metric it serves is work-caused attempts until the definition of done verifies true, which is a question about a ticket's history: a table keyed on the ticket alone holds only the last answer. WHAT IS IN THE MIGRATIONS `0000` creates `omninode_internal.dod_verify_runs` with the per-check class counts, including the behaviour-proving count the eval metric's done predicate requires, plus the derived outcome and its typed refusal reason. It carries the OMN-15376 shape-reconciliation block, so a pre-existing drifted table of that name converges rather than silently no-opping. `0001` delivers the grants and ASSERTS every one of them, the BIGSERIAL cursor's own sequence included. Asserting only the table INSERT is exactly what let OMN-17379 ship: `pr_merged_events` sat 24 days behind its topic at consumer lag zero, the consumer reading fine and every write failing on the sequence, while the assertion that would have caught it was true throughout. THE TOPOLOGY DECLARATION IS PART OF THE SAME CHANGE `check_topology_grant_delivery` caught this change delivering a grant the topology did not declare, which would have pushed the delivered-but- undeclared residual from 3 to 4. The remedy is the declaration, never the ratchet bound: the instances were regenerated with `generate_application_database_table_grants.py --write`, the only sanctioned writer for those lists, and the rendered catalogs with it. The diff is one line per instance. LAB READBACK (rule 24), `.201` dev lane, compose project `omnibase-infra`, container `omnibase-infra-postgres`, database `omninode_cloud`, throwaway schema dropped afterwards and verified absent: - `0000` applied clean, including the shape-reconciliation block and all three indexes; - the writer's actual upsert, read out of the module constant rather than retyped, was prepared and executed four times and produced three rows: the redelivery converged on its key, the re-verification at a later completion time became a second row and flipped to `done`, and the failing run stored as `refused` / `checks_failed`; - the columns read back with real types, `correlation_id` as `uuid` and `completed_at` as `timestamp with time zone`. That is the one question a database double cannot answer. Local gates: `tests/ci/test_topology_grant_delivery.py`, `test_node_migration_shape_reconciliation.py` and `test_flat_node_migration_shape_parity.py`, 353 passed. `sync-node-migrations.sh --check` reports in sync.
jonahgabriel
enabled auto-merge (squash)
September 20, 2026 15:06
Contributor
|
OCC autobind did not mint a companion for this PR: no changed-file candidate could be proven RED against the merge base, and emitting a PR-existence probe instead would be non-falsifiable evidence (OMN-15247). Hand-authored evidence is required. |
jonahgabriel
pushed a commit
to OmniNode-ai/onex_change_control
that referenced
this pull request
Sep 20, 2026
#10564) * evidence: OCC companion pass 1 for OmniNode-ai/omnibase_infra#3883 * evidence: OCC companion self-bind for #10564 --------- Co-authored-by: node-occ-companion-effect <occ-companion-effect@omninode.ai>
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
Contributor
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): 8
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: 8 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]
docker/migrations/forward/nodes/node_projection_dod_verdict/0000_create_dod_verify_runs.sql(glm-review) — Deduplication key embeds TIMESTAMPTZ round-trip fidelity assumption | Idempotency for a redelivered event depends on completed_at being byte-identical across deliveries. If the producer serializes tim - [MAJOR]
docker/migrations/forward/nodes/node_projection_dod_verdict/0000_create_dod_verify_runs.sql(glm-review) — Drift reconciliation omits primary key, so ON CONFLICT fails silently-shaped tables | The ALTER ... ADD COLUMN IF NOT EXISTS block claims 'shape reconciliation, not merely existence', but reconciles c - [MINOR]
docker/migrations/forward/nodes/node_projection_dod_verdict/0000_create_dod_verify_runs.sql(glm-review) — Type drift not reconciled despite 'shape reconciliation' claim | ADD COLUMN IF NOT EXISTS no-ops when the column exists with any type. A drifted ticket_id INTEGER or completed_at TIMESTAMP column woul - [MINOR]
docker/migrations/forward/nodes/node_projection_dod_verdict/0000_create_dod_verify_runs.sql(glm-review) — Enumerated text columns lack CHECK constraints | The comments enumerate closed value sets (status: pending|verified|failed|skipped|unresolved; outcome: done|refused; outcome_refusal: four values) but - [MINOR]
docker/migrations/forward/nodes/node_projection_dod_verdict/0000_create_dod_verify_runs.sql(glm-review) — BIGSERIAL drift path yields NULL cursor rows under a UNIQUE index | On a drifted table, added projection_cursor is nullable and existing rows are NULL. The unique index on projection_cursor tolerates - [MINOR]
docker/migrations/forward/nodes/node_projection_dod_verdict/0000_create_dod_verify_runs.sql(glm-review) — No test coverage for the drifted-table path the diff explicitly anticipates | The migration contains roughly 40 lines defending against a pre-existing drifted table, including nullable-column behavior - [MINOR]
docker/migrations/forward/nodes/node_projection_dod_verdict/0001_grant_omninode_runtime_dod_verify_runs.sql(glm-review) — UPDATE grant broader than the documented write pattern requires | The stated write is INSERT ... ON CONFLICT DO UPDATE touching a single row, which requires UPDATE but not necessarily a free-standing - [MINOR]
docker/migrations/forward/nodes/node_projection_dod_verdict/0001_grant_omninode_runtime_dod_verify_runs.sql(glm-review) — Grant assertion depends on information_schema visibility semantics | information_schema.role_table_grants filters to grants where the current user is grantor or grantee or an enabled role of the curre
Contributor
|
| 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)
…reason The manifest declaration count is a pinned roster, not a derived number, so a change adding node-owned migrations moves it by hand and says why beside the existing entries. 200 to 202: 0000_create_dod_verify_runs.sql and 0001_grant_omninode_runtime_dod_verify_runs.sql. The grant is a SECOND declaration rather than an edit to the first because it rides in the owning node's own lineage rather than a shared cross-node file, which is the same shape as the node_projection_runner_fleet and node_projection_lab_lane_health pairs already counted above it.
Contributor
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): 7
Nit-level findings suppressed: 2
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: 7 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.
- [CRITICAL]
docker/migrations/forward/nodes/node_projection_dod_verdict/0000_create_dod_verify_runs.sql(glm-review) — Drifted-table path leaves primary key absent, breaking ON CONFLICT writer | The reconciliation block repairs column shape only. It explicitly does not re-assert the primary key on a database where a d - [MAJOR]
docker/migrations/forward/nodes/node_projection_dod_verdict/0000_create_dod_verify_runs.sql(glm-review) — Shape reconciliation cannot restore NOT NULL or data integrity on drifted tables | The guarded ADD COLUMN path deliberately omits NOT NULL for columns without defaults, producing a nullable, unconstra - [MAJOR]
docker/migrations/forward/nodes/node_projection_dod_verdict/0000_create_dod_verify_runs.sql(glm-review) — ADD COLUMN IF NOT EXISTS BIGSERIAL attaches a new sequence to a drifted table | ALTER TABLE ... ADD COLUMN IF NOT EXISTS projection_cursor BIGSERIAL on a drifted table creates a fresh sequence and bac - [MINOR]
docker/migrations/forward/nodes/node_projection_dod_verdict/0000_create_dod_verify_runs.sql(glm-review) — No CHECK constraints on enumerated TEXT columns | status, outcome, and outcome_refusal are documented as closed enumerations in comments but declared as unconstrained TEXT. The comment block argues th - [MINOR]
docker/migrations/forward/nodes/node_projection_dod_verdict/0000_create_dod_verify_runs.sql(glm-review) — Unresolved_cause pairing invariant unenforced in schema | The migration documents that unresolved_cause is set exactly when status = 'unresolved', but enforces nothing. The 0001 grant migration assert - [MINOR]
tests/unit/scripts/validation/test_application_migration_manifest.py(glm-review) — Test updates only a magic count; no coverage of drift reconciliation | The only test change asserts len(result.declarations) == 202. None of the behaviors this diff introduces are covered: the drifted - [MINOR]
docker/migrations/forward/nodes/node_projection_dod_verdict/0000_create_dod_verify_runs.sql(glm-review) — Unique index on projection_cursor on drifted path may fail or tolerate NULLs | idx_dod_verify_runs_cursor is created UNIQUE on a column that is nullable on the drifted reconciliation path. Postgres to
…lf-expiring This pull request vendors the create and grant migrations (step 2 of the forced three-part order) BEFORE omnimarket lands the node package that declares the relation in its contract (step 3). So the shipped topology instances declare a relation the PINNED omnimarket contracts cannot derive, and the OMN-18863 derivation check goes red saying so. Regenerating is the wrong remedy and the failure text says why: it would DELETE the declaration while the vendored migration still grants it, tripping the OMN-18768 reverse ratchet and refusing the projection binding at boot, which takes the whole runtime process down. The right remedy, which that same text names, is a supplemental declaration for the window. So this adds one ContractTableDeclaration and registers it in _INTERIM_ENTRIES in the expiry module. Both halves in one change, which is what that module asks for in its own docstring: the entry is SELF-EXPIRING, and the expiry assertion goes red on the pin advance that makes it redundant and names the deletion it wants. Its predecessors for runtime_error_fingerprints and lab_lane_health were both retired exactly that way, neither by anyone remembering. Retired by: omnimarket#2722 merging and the pin advancing past it. PROOF THE BRIDGE DERIVES WHAT THE CONTRACT WILL Regenerating the instances against a contracts root WITHOUT the node package, which is what the pin sees, writes nothing: "instances updated: (none), catalogs rendered: (none)", with dod_verify_runs still declared. That is the same byte-identical result the two retired predecessors produced, and it is the evidence that the bridge and the future contract derive the same thing. Local: the topology grant delivery suite, the expiry module and the migration manifest suite, 102 passed. The SQL ownership gate reports PASS against omnimarket#2724, now merged.
…-dod-verdict-migrations
Contributor
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): 7
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: 7 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.
- [CRITICAL]
docker/migrations/forward/nodes/node_projection_dod_verdict/0001_grant_omninode_runtime_dod_verify_runs.sql, assertions 1 and 5(glm-review) — Assertions without FROM clauses cannot fail; they return zero rows instead | The fail-loud pattern used is 1/count(*). This only raises a division-by-zero when the aggregate is evaluated and returns z - [MAJOR]
docker/migrations/forward/nodes/node_projection_dod_verdict/0001_grant_omninode_runtime_dod_verify_runs.sql, section 3(glm-review) — Sequence USAGE grant targets a statically named sequence that may not exist on a drifted database | The migration grants USAGE on omninode_internal.dod_verify_runs_projection_cursor_seq by literal nam - [MAJOR]
docker/migrations/forward/nodes/node_projection_dod_verdict/0000_create_dod_verify_runs.sql, drift-reconciliation block(glm-review) — Drift reconciliation path omits the primary key, breaking the documented ON CONFLICT writer | The grant migration states the writer issues INSERT ... ON CONFLICT DO UPDATE. On the drift path, the PK ( - [MINOR]
docker/migrations/forward/nodes/node_projection_dod_verdict/0000_create_dod_verify_runs.sql, final statement(glm-review) — Redundant unique index on BIGSERIAL projection_cursor | projection_cursor is BIGSERIAL, backed by a sequence that never reissues values, making uniqueness structural. idx_dod_verify_runs_cursor adds a - [MINOR]
src/omnibase_infra/topology/table_grant_derivation.py, OMN-18900 entry(glm-review) — Declared role dod_verdict is not the role the grant migration provisions | The bridge declaration sets role='dod_verdict', while the delivering migration grants to omninode_runtime only. If the grant- - [MINOR]
docker/migrations/forward/nodes/node_projection_dod_verdict/0001_grant_omninode_runtime_dod_verify_runs.sql, assertion 1(glm-review) — pg_get_serial_sequence result compared to literal may differ in quoting | pg_get_serial_sequence returns the sequence name in a canonical quoted form ('omninode_internal.dod_verify_runs_projection_cur - [MINOR]
tests/ci/test_supplemental_declaration_expiry_omn18863.py, _INTERIM_ENTRIES(glm-review) — Expiry guard keyed to an unmerged upstream PR | The interim entry is registered against omnimarket#2722 before that PR merges, per the code comment 'Retired by: omnimarket#2722 merging'. If the refere
…s pair The Integration Test Coverage gate refused this pull request, correctly: it is titled a feature and added nothing under tests/integration/. Eight checks now cover the contract of the vendored migration pair, in the shape of their neighbours in tests/integration/migrations/. WHAT THEY ASSERT The three-column run key, which is what makes the attempts metric answerable at all. Every class count the metric reads, including the behaviour-proving one that was zero in the only verify payload the inventory could find. A guarded ADD COLUMN per declared column, without which a drifted table of this name keeps its own shape and the next column-dependent statement takes the whole migration run down. The three indexes. Every granted privilege ASSERTED and not merely issued, the BIGSERIAL sequence included, which is the half whose absence stayed green through the 24-day OMN-17379 outage. No dynamic SQL and no schema-creating statement. And byte-identity with the omnimarket source, because vendored copies drifting is a six-occurrence defect and the parity gate only watches the direction where the source moves. WHAT THEY DO NOT CLAIM They are file-contract checks. They need no database, so they never skip on the merge-gating job and cannot contribute a false green to it. They are NOT a live-database proof and say so in their own docstring; that proof was taken against real PostgreSQL on the lab dev lane and is quoted in the pull request body, and the omnimarket side carries a real-Postgres write-path test over this same file. ONE THING WORTH RECORDING Two of the eight failed on first run, both because the migrations explain IN A COMMENT why they do not use the construct being forbidden. A raw substring search fires on the documentation about the rule and reports the file as violating the thing it is explaining -- a check satisfiable only by deleting the explanation. Both absence assertions now read the file with its comment lines stripped. The byte-identity check skips, and only skips, when no omnimarket tree is resolvable at all. That is a missing optional cross-repo checkout rather than a provisioned service, so it is not a silent-skip false green; verified by running the module with the environment cleared.
Contributor
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): 7
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: 7 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.
- [CRITICAL]
docker/migrations/forward/nodes/node_projection_dod_verdict/0001_grant_omninode_runtime_dod_verify_runs.sql(glm-review) — Division-by-zero grant assertions are vacuous, not fail-loud | The migration's assertion queries are of the form SELECT 1 / count(*) ... WHERE . When the fact is false, the WHERE clause filters - [MAJOR]
docker/migrations/forward/nodes/node_projection_dod_verdict/0000_create_dod_verify_runs.sql(glm-review) — Shape reconciliation never restores the primary key on a drifted table | The comments state the create is idempotent "in SHAPE and not merely in existence" and that "the primary key likewise belongs t - [MAJOR]
docker/migrations/forward/nodes/node_projection_dod_verdict/0000_create_dod_verify_runs.sql(glm-review) — Drift-path columns have wrong types with no detection | ADD COLUMN IF NOT EXISTS no-ops if a column of the same name already exists with any type. A hand-made dod_verify_runs with correlation_id TEXT - [MINOR]
docker/migrations/forward/nodes/node_projection_dod_verdict/0000_create_dod_verify_runs.sql(glm-review) — Redelivery convergence depends on producer-controlled completed_at | The three-column key uses completed_at to disambiguate colliding correlation_ids, and the comment asserts that 'a genuine redeliver - [MINOR]
tests/integration/migrations/test_dod_verify_runs_omn18900.py(glm-review) — Byte-identity gate skips unconditionally until omnimarket lands | test_the_vendored_pair_is_byte_identical_to_its_omnimarket_source skips when no omnimarket tree is resolvable, and per the diff narrat - [MINOR]
docker/migrations/forward/nodes/node_projection_dod_verdict/0000_create_dod_verify_runs.sql(glm-review) — No constraints enforce the documented status/outcome invariants | The comments document invariants the migration does not enforce: status is one of five enumerated values, outcome is done|refused, unr - [MINOR]
docker/migrations/forward/nodes/node_projection_dod_verdict/0001_grant_omninode_runtime_dod_verify_runs.sql(glm-review) — UPDATE grant broader than the stated write pattern requires | The rationale grants SELECT, INSERT, UPDATE because of RETURNING and ON CONFLICT, which is defensible, but the runtime role can now rewrit
The skip-count ratchet caught the byte-identity check by name: it compared the vendored files against an omnimarket checkout, and no ordinary test split has one, so it was collected and never executed. A test that never runs proves nothing, and the ratchet is right to refuse the set growing by it. Recording it in the baseline was the other option the gate offers and it is the worse one here, because the question is answerable locally. The forward-migration ledger already carries a sha256 per vendored file, and bootstrap resolves a historical migration against exactly that checksum. So the check now asserts each vendored file against its own ledger row: a file hand-edited here with the ledger left alone is both a real defect and one this repository can see by itself. It also stopped needing the cross-repository checkout, which is what made it skippable in the first place. Verified by running the module with the environment cleared: eight executed, none skipped. The direction the omnimarket-side parity gate watches is unchanged and still covers the other half, where the source moves.
Contributor
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): 8
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: 8 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]
tests/integration/migrations/test_dod_verify_runs_omn18900.py(glm-review) — File-contract tests marked integration may be excluded from gating splits | The new tests are pure filesystem checks that require no database, yet each carries @pytest.mark.integration. The module doc - [MAJOR]
docker/migrations/forward/nodes/node_projection_dod_verdict/0000_create_dod_verify_runs.sql(glm-review) — Key convergence depends on byte-identical completed_at from the producer | The migration comment states 'A genuine redelivery carries all three [key columns] identically and converges on the row it al - [MAJOR]
docker/migrations/forward/nodes/node_projection_dod_verdict/0000_create_dod_verify_runs.sql(glm-review) — Drifted-table path yields rows violating declared NOT NULL constraints | The shape-reconciliation block deliberately adds columns without NOT NULL so Postgres accepts the ALTER on a pre-populated drif - [MINOR]
src/omnibase_infra/topology/table_grant_derivation.py(glm-review) — ContractTableDeclaration role mismatch: role dod_verdict, grant to omninode_runtime | The bridged declaration sets role='dod_verdict' while the grant migration delivers privileges to omninode_runtime - [MINOR]
docker/migrations/forward/nodes/node_projection_dod_verdict/0000_create_dod_verify_runs.sql(glm-review) — No CHECK constraints on status, outcome, or outcome_refusal enumerations | The comments enumerate closed value sets (pending|verified|failed|skipped|unresolved; done|refused; four refusal causes) and - [MINOR]
tests/integration/migrations/test_dod_verify_runs_omn18900.py(glm-review) — Substring assertions are brittle against formatting, not semantics | The column-declaration tests match ' {column} ' and 'ADD COLUMN IF NOT EXISTS {column}' as raw substrings. They pass on a column - [MINOR]
docker/migrations/forward/nodes/node_projection_dod_verdict/0000_create_dod_verify_runs.sql(glm-review) — Unique cursor index creation can fail on drifted pre-populated tables | On a drifted table, ADD COLUMN IF NOT EXISTS projection_cursor BIGSERIAL backfills each existing row from the new sequence, so u - [MINOR]
tests/integration/migrations/test_dod_verify_runs_omn18900.py(glm-review) — Executable-SQL stripper is line-granular and misses inline comments | _executable_sql only drops lines whose first non-space characters are '--'. A forbidden construct appended after a trailing commen
…-dod-verdict-migrations
The release-identity gate refused 0.38.36: dev published it while this branch was in flight. Open pull requests were scanned before choosing, as the colliding-version trap asks: 3874 takes .36 and 3887 takes .37, so this takes .38 rather than the obvious next number. Also merges dev, which is where .36 was published.
Contributor
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): 7
Nit-level findings suppressed: 2
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: 7 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.
- [CRITICAL]
docker/migrations/forward/nodes/node_projection_dod_verdict/0000_create_dod_verify_runs.sql(glm-review) — Drifted-table path leaves no primary key, breaking ON CONFLICT and sequence wiring | On a database where dod_verify_runs pre-exists with a different shape, the CREATE no-ops and the guarded ADD COLUMN - [MAJOR]
docker/migrations/forward/nodes/node_projection_dod_verdict/0000_create_dod_verify_runs.sql(glm-review) — Twenty-statement reconciliation block is not atomic | 0000 issues one CREATE plus 19 ALTER TABLE statements and two/three index creations. If the migration runner does not wrap a node's file in a sing - [MAJOR]
docker/migrations/forward/nodes/node_projection_dod_verdict/0000_create_dod_verify_runs.sql(glm-review) — Timestamp key component makes redelivery convergence clock-dependent | The primary key uses completed_at to disambiguate two runs sharing a caller-supplied correlation_id. But completed_at is event ti - [MAJOR]
docker/migrations/forward/nodes/node_projection_dod_verdict/0000_create_dod_verify_runs.sql(glm-review) — Enumerated status and outcome domains unenforced at the database | status, outcome, and outcome_refusal are unconstrained TEXT despite the migration enumerating their allowed values in comments, and t - [MINOR]
docker/migrations/forward/nodes/node_projection_dod_verdict/0000_create_dod_verify_runs.sql(glm-review) — Count invariants declared in prose but not asserted | The schema implies verified_count + failed_count + skipped_count + superseded_count relates to total_checks, and non_probative_count is described - [MINOR]
tests/integration/migrations/test_dod_verify_runs_omn18900.py(glm-review) — Checksum parity test validates self-referential data | test_the_vendored_pair_matches_the_checksum_the_ledger_records compares the vendored files against digests recorded in the same repository's ledg - [MINOR]
src/omnibase_infra/topology/table_grant_derivation.py(glm-review) — Self-expiring interim entry depends on an external PR number with no verification | The bridge entry in table_grant_derivation.py and its _INTERIM_ENTRIES registration both hardcode 'omnimarket#2722'
…bump The runner-image smoke gate refused a stale shared_env_digest. The cause is this branch: bumping the project version rewrote uv.lock, and the shared environment digest is derived from it, so the recorded identity no longer matched the recomputed one. Checked before regenerating that this is not pre-existing dev staleness being swept into an unrelated pull request: origin/dev records the old digest and its uv.lock differs from this branch, so the drift arrived with the bump. Regenerated with the tool the error names rather than hand-edited, and it now verifies clean.
Contributor
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): 7
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: 7 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]
docker/migrations/forward/nodes/node_projection_dod_verdict/0000_create_dod_verify_runs.sql(glm-review) — Drifted-table path leaves no primary key, breaking upsert convergence | The shape-reconciliation block deliberately omits NOT NULL from the guarded ADD COLUMNs and explicitly does not re-assert the pr - [MAJOR]
tests/integration/migrations/test_dod_verify_runs_omn18900.py(glm-review) — Tests marked integration may be excluded from gating despite never-needs-database claim | The new test module asserts its checks 'never skip and never contribute a false green to the merge-gating job' - [MINOR]
docker/migrations/forward/nodes/node_projection_dod_verdict/0000_create_dod_verify_runs.sql(glm-review) — No CHECK constraints on enumerated text columns | status and outcome carry enumerated value sets ('pending | verified | failed | skipped | unresolved' and 'done | refused') documented only in comments - [MINOR]
docker/migrations/forward/nodes/node_projection_dod_verdict/0001_grant_omninode_runtime_dod_verify_runs.sql(glm-review) — Sequence identity assertion brittle to pg_get_serial_sequence output formatting | The assertion compares pg_get_serial_sequence(...) to the literal 'omninode_internal.dod_verify_runs_projection_cursor - [MINOR]
docker/catalog/database-topology/*.yaml(glm-review) — Topology sha256 self-certification is unverifiable in the diff | Nine topology YAMLs update a source sha256 covering the profile catalog, and the profile_catalog_sha256 itself is unchanged while the o - [MINOR]
docker/migrations/forward/nodes/node_projection_dod_verdict/0001_grant_omninode_runtime_dod_verify_runs.sql(glm-review) — No retention or growth policy for an append-only table | The migration declares a per-run append-only table with no DELETE grant ('a verification run is history that is never retracted'), yet the metr - [MINOR]
src/omnibase_infra/topology/table_grant_derivation.py(glm-review) — Interim bridge declaration grants to a role the migration does not touch | The bridged ContractTableDeclaration sets role="dod_verdict" and access="write" against a node that does not exist in any pin
…-dod-verdict-migrations
Contributor
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): 9
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: 9 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]
docker/migrations/forward/nodes/node_projection_dod_verdict/0000_create_dod_verify_runs.sql(glm-review) — Drift path yields a table that cannot serve the writer's upsert | On a drifted pre-existing table, the guarded ADD COLUMN block deliberately omits NOT NULL and the PRIMARY KEY is not re-asserted. The - [MAJOR]
docker/migrations/forward/nodes/node_projection_dod_verdict/0001_grant_omninode_runtime_dod_verify_runs.sql(glm-review) — Identity assertion fails permanently, not loudly-then-recoverably, on drifted cursor column | If a hand-made table already has projection_cursor as plain BIGINT, ADD COLUMN IF NOT EXISTS no-ops, pg_ge - [MINOR]
docker/migrations/forward/nodes/node_projection_dod_verdict/0001_grant_omninode_runtime_dod_verify_runs.sql(glm-review) — Schema USAGE granted but never asserted | The grant file's stated discipline is that every granted privilege is asserted, citing OMN-17379. GRANT USAGE ON SCHEMA omninode_internal is issued but has no - [MINOR]
docker/migrations/forward/nodes/node_projection_dod_verdict/0000_create_dod_verify_runs.sql(glm-review) — Idempotency key includes client-supplied event time | completed_at is part of the primary key and, per the migration's own comments, is event time supplied by the run. Redelivery convergence therefore - [MINOR]
docker/migrations/forward/nodes/node_projection_dod_verdict/0000_create_dod_verify_runs.sql(glm-review) — Enumeration columns carry no CHECK constraints | status and outcome are TEXT with enumerated domains documented only in comments, and outcome_refusal pairs conditionally with outcome. The row-alone co - [MINOR]
docker/migrations/forward/nodes/node_projection_dod_verdict/0001_grant_omninode_runtime_dod_verify_runs.sql(glm-review) — Unconditional UPDATE grant permits history rewrite | omninode_runtime receives UPDATE on all columns of all rows. The migration declares runs immutable history ('a verification run is history that is - [MINOR]
src/omnibase_infra/topology/table_grant_derivation.py(glm-review) — Role mismatch between derivation entry and delivered grants | The supplemental ContractTableDeclaration sets role="dod_verdict" while the grant migration delivers to omninode_runtime and the topology - [MINOR]
tests/integration/migrations/test_dod_verify_runs_omn18900.py(glm-review) — Database-free file checks marked integration | The new test module states the checks 'need no database, so they never skip and never contribute a false green to the merge-gating job', yet every test c - [MINOR]
docker/runners/runner-image.lock.json(glm-review) — Runner image lock digests bumped without corresponding change in the diff | identity_digest and shared_env_digest change while base_image_digest, gh_version, install args, and every tool version are u
…-dod-verdict-migrations # Conflicts: # docker/runners/runner-image.lock.json # pyproject.toml # uv.lock
Contributor
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): 7
Nit-level findings suppressed: 2
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: 7 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]
docker/migrations/forward/nodes/node_projection_dod_verdict/0001_grant_omninode_runtime_dod_verify_runs.sql(glm-review) — Grant assertion spurious failure when grants issued by a different role | The role_table_grants assertions in 0001 filter only on grantee, table, and privilege_type, not grantor. information_schema.ro - [MAJOR]
docker/migrations/forward/nodes/node_projection_dod_verdict/0000_create_dod_verify_runs.sql(glm-review) — Drifted database with BIGINT cursor fails the grant migration with no recovery path | On a drifted table where projection_cursor already exists as plain BIGINT, ADD COLUMN IF NOT EXISTS no-ops, no seq - [MINOR]
docker/catalog/database-topology/*.yaml(glm-review) — sha256 self-reference in topology instance files unverifiable and unchanged profile hash | Eight dogfood-style YAMLs under docker/catalog/database-topology/ and three files under src/omnibase_infra/to - [MINOR]
docker/runners/runner-image.lock.json(glm-review) — runner-image.lock.json digest changes are unrelated to the migration | identity_digest and shared_env_digest change with no corresponding change to shared_env_install_args, dependency files, or any ru - [MINOR]
docker/migrations/forward/nodes/node_projection_dod_verdict/0000_create_dod_verify_runs.sql(glm-review) — Enumerated columns stored as unconstrained TEXT | status and outcome carry five- and two-value vocabularies respectively, documented only in comments. The writer model enforces the pairing invariant, - [MINOR]
tests/integration/migrations/test_dod_verify_runs_omn18900.py(glm-review) — Ledger parser silently drops malformed rows | _ledger_digests skips any line whose split does not yield exactly six columns. A ledger row corrupted to five or seven columns would be silently excluded, - [MINOR]
tests/integration/migrations/test_dod_verify_runs_omn18900.py(glm-review) — Checksum test self-referentially passes on a coordinated hand edit | The anti-drift test compares the vendored file against the digest recorded in the same repository's ledger. A hand edit that update
jonahgabriel
deleted the
jonah/omn-18900-vendor-dod-verdict-migrations
branch
September 20, 2026 20:20
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes OMN-18900
Vendors the two node-owned migrations for omnimarket's new
node_projection_dod_verdictand declares the grant they deliver.Why this lands first
omnimarket's node-migration-vendor-parity-gate fails a pull request touching
migrations/*.sqlunless the vendored copy already exists at this repo's dev tip. Vendor first, declare second — the OMN-18768 ordering trap. The omnimarket side (the fold, the writer, the tests) is the sibling pull request and opens once this one is green.Why the table exists
node_dod_verifyproduces a per-ticket, machine-readable, check-level verdict and throws it away. On 2026-09-20 the entire durable population of that verdict on this fleet was nine ad-hoc JSON files in one developer's scratch directory; every other verdict ever produced is gone. The table named in the only comment that mentions one did not exist: nothing created it and nothing wrote it.This is decision 3 of the operator-approved plan
beta/plans/2026-09-20-jev-decision-workflow-eval-plan.mdinknowledge-base-internal(pull request 587, squash895797a5), which the operator ordered started first. The durable authorization record is the RULING row indocs/tracking/ROLLING_WORK_LEDGER.mdat 2026-09-20T14:17:16Z, with a NOTE row beside it recording that the plan document was revised the same day.One row per RUN, keyed on ticket, correlation identifier and completion time. The metric it serves is work-caused attempts until the definition of done verifies true, which is a question about a ticket's history: a table keyed on the ticket alone carries only the last answer and can never be asked how many attempts preceded it.
What is in the migrations
0000createsomninode_internal.dod_verify_runswith the per-check class counts — the behaviour-proving count the eval metric's done predicate requires among them — plus the derived outcome and its typed refusal reason. It carries the OMN-15376 shape-reconciliation block, so a pre-existing drifted table of that name converges rather than silently no-opping and taking the next column-dependent statement down with it.0001delivers the grants and asserts every one of them, the BIGSERIAL cursor's own standalone sequence included. Asserting only the table INSERT is exactly what let OMN-17379 ship:pr_merged_eventssat 24 days behind its topic at consumer lag zero, the consumer reading fine and every write failing on the sequence, while the assertion that would have caught it was true throughout.The topology declaration is part of the same change
check_topology_grant_deliverycaught this change delivering a grant the topology did not declare, which would have moved the delivered-but-undeclared residual from 3 to 4. The remedy is the declaration, never the ratchet bound. The instances were regenerated withgenerate_application_database_table_grants.py --write, the only sanctioned writer for those lists, and the rendered catalogs with it. The diff is one line per instance.Lab readback (Operating Rule 24)
.201dev lane, compose projectomnibase-infra, containeromnibase-infra-postgres, databaseomninode_cloud, in a throwaway schema that was dropped afterwards and verified absent.0000applied clean, including the shape-reconciliation block and all three indexes.done, and the failing run stored asrefused/checks_failed.The last two lines are the point of taking this on real Postgres rather than a double: a double accepts an ISO string bound to a
TIMESTAMPTZand a string bound to aUUIDexactly as readily as the real types, which is the gap that took a crash-looping runtime to production with every mock green under OMN-15905.Local verification
tests/ci/test_topology_grant_delivery.py,test_node_migration_shape_reconciliation.py,test_flat_node_migration_shape_parity.py— 353 passed. The first of those was RED before the topology regeneration, naming this exact table.scripts/sync-node-migrations.sh --checkreports in sync.ruff format --checkclean; the governed pre-push hook ran on push and passed.diff -q).Evidence-Ticket: OMN-18900
Evidence-Source: OCC#10564