Repository navigation
chore(OMN-18993): vendor and declare the delegate-skill command-claims migration - #3918
Conversation
…s migration OMN-18887 adds a node-scoped migration in omnimarket creating the durable claim that stops a redelivered delegate-skill command from re-running and re-billing the inference. omnimarket node migrations are vendored here, and the parity gate enforces the ordering: the vendored copy must exist on dev before that PR can pass. Produced by scripts/sync-node-migrations.sh with OMNIMARKET_SRC pointed at the OMN-18887 branch, which is the script's own documented override, rather than copied by hand. Hand-copying is the drift class that gate exists to catch, and it has caught it seven times. Verified byte-identical against the source, and the script's own check reports in sync. The declaration ledger row is the second half and is not optional: a vendored SQL artifact must be bound to one stream, owner, domain, version and content checksum, or explicitly blocked. Domain is omninode_internal rather than tenant, because this is internal control state for one node rather than tenant-scoped data -- the same classification the dod_verify_runs rows carry, and the table declares no row-level security to enforce a tenant claim with. The checksum is computed from the vendored file, so a later edit on either side fails the gate rather than drifting quietly. No behaviour changes here. The table is read and written only by the delegate-skill claim port in omnimarket; nothing in this repository consumes it. Evidence-Ticket: OMN-18993
|
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. |
#10659) * evidence: OCC companion pass 1 for OmniNode-ai/omnibase_infra#3918 * evidence: OCC companion self-bind for #10659 --------- 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}
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: 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.
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_delegate_skill_orchestrator/0001_delegate_skill_command_claims.sql(glm-review) — Crashed handler permanently blocks redelivery with no recovery path | The migration explicitly refuses any reclaim policy for claims lacking terminal_json, arguing that running and dead-mid-flight sta - [MAJOR]
docker/migrations/forward/nodes/node_delegate_skill_orchestrator/0001_delegate_skill_command_claims.sql(glm-review) — Unbounded table growth with no retention or pruning policy | Every delivered record inserts a row keyed by delivery_id, and the migration defines no retention, pruning, or archival. On an at-least-onc - [MAJOR]
docker/migrations/forward/nodes/node_delegate_skill_orchestrator/0001_delegate_skill_command_claims.sql(glm-review) — claimed_at stored as TEXT with no stated format or ordering guarantee | The migration's core correctness argument depends on claimed_at semantics: the value written on INSERT is the first claimer's an - [MAJOR]
docker/migrations/forward/nodes/node_delegate_skill_orchestrator/0001_delegate_skill_command_claims.sql(glm-review) — Empty-string sentinel for terminal_json conflates 'no terminal' with valid terminal | terminal_json TEXT NOT NULL DEFAULT '' uses an empty string as the not-yet-terminal marker. Any consumer check mus - [MINOR]
docker/migrations/forward/nodes/node_delegate_skill_orchestrator/0001_delegate_skill_command_claims.sql(glm-review) — No tenant isolation on a table storing tenant_id | Sibling ledger migrations (node_canary_score_reducer 0002) apply tenant_id plus RLS. This table stores tenant_id but the migration creates no RLS pol - [MINOR]
docker/migrations/forward/nodes/node_delegate_skill_orchestrator/0001_delegate_skill_command_claims.sql(glm-review) — Migration documents upsert semantics that the schema cannot enforce | The correctness argument rests on 'claimed_at is written on INSERT and never on conflict' and a single-statement atomic claim. No - [MINOR]
docker/migrations/forward/nodes/node_delegate_skill_orchestrator/0001_delegate_skill_command_claims.sql(glm-review) — correlation_id and tenant_id silently defaulted to empty string | DEFAULT '' on correlation_id and tenant_id means any caller that omits them produces rows that are useless for the stated diagnostic p - [MINOR]
docker/migrations/forward/nodes/node_delegate_skill_orchestrator/0001_delegate_skill_command_claims.sql(glm-review) — No test coverage for the concurrency claim | The diff's central assertion is that four concurrent records cannot slip through the insert-or-conflict claim. The migration ships with no accompanying tes - [MINOR]
docker/migrations/forward/nodes/node_delegate_skill_orchestrator/0001_delegate_skill_command_claims.sql(glm-review) — CREATE TABLE IF NOT EXISTS weakens migration integrity checking | A forward migration registered with a content hash should fail loudly if the table already exists; IF NOT EXISTS masks drift from part
|
| 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)
The application-domain gate refused the first vendored copy: an application relation must be schema-qualified, because an unqualified CREATE lands wherever search_path points. Corrected at the omnimarket source and re-vendored, so the two stay byte-identical rather than diverging at the copy. Worth recording, because it is not obvious and it cost a cycle. Once a migration carries a declaration row, the sync script treats its vendored bytes as FROZEN and reports zero files updated rather than rewriting them. That is the legacy-declared exemption working as designed: a migration that has landed is immutable, and an upstream delta must arrive as a new file. It does not apply to one that has never merged anywhere, so the row was removed, the corrected bytes re-vendored, and the row re-added with the new checksum. Editing the vendored copy directly would have produced exactly the drift the checksum exists to catch. Both gates green: the declaration validator passes at 203 active, and the sync script's own check reports in sync. Evidence-Ticket: OMN-18993
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_delegate_skill_orchestrator/0001_delegate_skill_command_claims.sql(glm-review) — Unbounded claim table growth with no retention or eviction path | Every delivered record inserts a row permanently. No terminal-only pruning, no partitioning, no archival job. The comment defers 'recl - [MAJOR]
docker/migrations/forward/nodes/node_delegate_skill_orchestrator/0001_delegate_skill_command_claims.sql(glm-review) — TEXT timestamps defeat any future staleness logic | claimed_at is TEXT. The design explicitly rules out timer-based reclaim, but any future diagnostic, dashboard, or eventual TTL will need interval ar - [MAJOR]
docker/migrations/forward/nodes/node_delegate_skill_orchestrator/0001_delegate_skill_command_claims.sql(glm-review) — terminal_json as TEXT loses integrity guarantees | terminal_json is TEXT with no CHECK constraint. Malformed or empty-trailing JSON will only be discovered at read time, after the suppressed redeliver - [MAJOR]
docker/migrations/forward/nodes/node_delegate_skill_orchestrator/0001_delegate_skill_command_claims.sql(glm-review) — Empty-string defaults for correlation_id and tenant_id enable provenance loss | Defaults of '' allow claims to be inserted with no tenant attribution and no correlation linkage. For a table explicitly - [MAJOR]
docker/migrations/forward/nodes/node_delegate_skill_orchestrator/0001_delegate_skill_command_claims.sql(glm-review) — Cross-table index creation is not transactionally coupled to table creation semantics | CREATE INDEX IF NOT EXISTS on a non-concurrent path is fine, but the name lacks schema qualification precedent a - [MINOR]
docker/migrations/forward/nodes/node_delegate_skill_orchestrator/0001_delegate_skill_command_claims.sql(glm-review) — delivery_id-as-PK depends on unverified broker redelivery semantics | The design's correctness hinges on the assumption that a redelivery carries an identical delivery identifier while a retried-with- - [MINOR]
docker/migrations/forward/nodes/node_delegate_skill_orchestrator/0001_delegate_skill_command_claims.sql(glm-review) — No test coverage for the concurrency claim | The comment asserts atomicity under four concurrent records but the diff contains no test, fixture, or harness evidence. Concurrent INSERT ... ON CONFLICT - [MINOR]
docker/migrations/forward/_ledger/application-migrations.tsv(glm-review) — No rollback path for a new control table | The TSV registry registers only the forward migration. Whether intentional, the diff ships no reverse migration or documented teardown for a table that, per
The OMN-18774 gate refused the table for carrying a tenant_id column while declared OMNINODE_INTERNAL, which receives no tenant stamping and no row-level security. Corrected at the omnimarket source and re-vendored, so the two stay byte-identical rather than diverging at the copy. Same unfreeze sequence as the last correction, for the same documented reason: a declared migration's vendored bytes are frozen, so the row comes out, the corrected bytes go in, and the row goes back with the new checksum. Editing the vendored copy directly would produce exactly the drift the checksum exists to catch. Declaration validator passes at 203 active. Evidence-Ticket: OMN-18993
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.
- [CRITICAL]
docker/migrations/forward/nodes/node_delegate_skill_orchestrator/0001_delegate_skill_command_claims.sql(glm-review) — Idempotency keyed on delivery_id does not survive rebalance redelivery | The design premise is that a rebalance or crash re-delivers the same record with the same delivery_id, so PRIMARY KEY on delive - [MAJOR]
docker/migrations/forward/nodes/node_delegate_skill_orchestrator/0001_delegate_skill_command_claims.sql(glm-review) — No reclamation policy yields unbounded claim table growth | The migration explicitly defers stale-claim reclamation to a follow-up, but there is not even a retention policy for rows that HAVE recorded - [MAJOR]
docker/migrations/forward/nodes/node_delegate_skill_orchestrator/0001_delegate_skill_command_claims.sql(glm-review) — correlation_id default contradicts the stated contract | The header comment states correlation 'defaults to a fresh uuid4 but a caller may supply one'. The column defaults to the empty string, not a u - [MAJOR]
docker/migrations/forward/nodes/node_delegate_skill_orchestrator/0001_delegate_skill_command_claims.sql(glm-review) — terminal_json stored as unvalidated TEXT | The terminal envelope that answers a suppressed redelivery is persisted as free-form TEXT with an empty-string default. Nothing prevents malformed JSON, and - [MAJOR]
docker/migrations/forward/nodes/node_delegate_skill_orchestrator/0001_delegate_skill_command_claims.sql(glm-review) — Claim win/lose protocol asserted in comments but untested in the diff | The header claims atomicity 'by construction' via INSERT ... ON CONFLICT semantics, but the diff contains no migration-side test - [MINOR]
docker/migrations/forward/nodes/node_delegate_skill_orchestrator/0001_delegate_skill_command_claims.sql(glm-review) — claimed_at as TEXT timestamp | claimed_at is TEXT NOT NULL with no stated format or timezone convention. Any future stale-claim logic (the acknowledged follow-up) will require parsing an ad-hoc textua - [MINOR]
docker/migrations/forward/nodes/node_delegate_skill_orchestrator/0001_delegate_skill_command_claims.sql(glm-review) — No RLS or access grant statements for a control table holding terminal payloads | The comment argues tenant_id and RLS are correctly absent because the relation is internal. That argument covers multi - [MINOR]
docker/migrations/forward/_ledger/application-migrations.tsv(glm-review) — No reverse migration | The forward migration is registered in the TSV ledger, but no corresponding reverse migration directory entry is present in the diff. If the rollback path exists elsewhere by co
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]
docker/migrations/forward/nodes/node_delegate_skill_orchestrator/0001_delegate_skill_command_claims.sql(glm-review) — claim-at-suppression protocol is implicit and unverifiable from the diff | The migration's correctness argument rests on an application-side pattern: INSERT ... ON CONFLICT DO NOTHING (or similar), th - [MAJOR]
docker/migrations/forward/nodes/node_delegate_skill_orchestrator/0001_delegate_skill_command_claims.sql(glm-review) — no retention, reclamation, or growth bound on an unbounded control table | The comment explicitly defers stale-claim reclamation, but the table grows by one row per delivered record forever, including - [MAJOR]
docker/migrations/forward/nodes/node_delegate_skill_orchestrator/0001_delegate_skill_command_claims.sql(glm-review) — delivery_id as suppression key assumes redelivery preserves record identity | Suppression keys on the delivering record's identity. The comment's model of redelivery (same record twice) holds for Kafk - [MINOR]
docker/migrations/forward/nodes/node_delegate_skill_orchestrator/0001_delegate_skill_command_claims.sql(glm-review) — empty-string sentinels instead of NULL | correlation_id DEFAULT '' and terminal_json DEFAULT '' use empty strings as absence markers. This permits corrupt states (empty terminal_json treated as 'no te - [MINOR]
docker/migrations/forward/nodes/node_delegate_skill_orchestrator/0001_delegate_skill_command_claims.sql(glm-review) — no evidence of concurrency tests for the atomic-claim invariant | The entire value of this change is behavior under concurrency (four in-flight records, rebalance races). The diff contains only DDL. W - [MINOR]
docker/migrations/forward/nodes/node_delegate_skill_orchestrator/0001_delegate_skill_command_claims.sql(glm-review) — tenantless internal table stores provider-derived terminal payloads without access controls | terminal_json holds terminal outcomes of delegated skill execution, potentially containing provider respon
…assertion The deterministic application-migration manifest pins its declaration count so a vendored artifact cannot appear without a reviewer seeing the number move. This PR adds one declaration row, so the pinned count is now off by one and the assertion fails 203 == 202. Bumped to 203 with the rationale beside the existing per-ticket entries: node_delegate_skill_orchestrator/0001_delegate_skill_command_claims.sql, domain omninode_internal rather than tenant because it is per-node control state carrying no row-level security. Verified at the merge base: the ledger holds 202 rows there and the vendored SQL file does not exist, so both are introduced by this diff rather than pre-existing on dev. Claude-Session: https://claude.ai/code/session_011WaS5jrEdJDstEEtiJuRvW
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.
- [CRITICAL]
docker/migrations/forward/nodes/node_delegate_skill_orchestrator/0001_delegate_skill_command_claims.sql(glm-review) — Crash between claim insert and dispatch permanently suppresses redelivery with empty terminal | The design states that a suppressed redelivery answers via terminal_json, and that a claim with no termi - [MAJOR]
docker/migrations/forward/nodes/node_delegate_skill_orchestrator/0001_delegate_skill_command_claims.sql(glm-review) — Redelivery equivalence via delivery_id is asserted, not established | The suppression key assumes a broker redelivery presents the same delivery_id as the original. The migration never names the trans - [MAJOR]
docker/migrations/forward/nodes/node_delegate_skill_orchestrator/0001_delegate_skill_command_claims.sql(glm-review) — claimed_at stored as TEXT defeats the stated win-condition comparison | The header claims 'claimed_at ... is the FIRST claimer's; comparing it against the value that caller passed is the did I win ans - [MAJOR]
docker/migrations/forward/nodes/node_delegate_skill_orchestrator/0001_delegate_skill_command_claims.sql(glm-review) — Unbounded retention on an at-least-once claim table | Every delegate-skill delivery inserts a row permanently. At-least-once delivery with four concurrent consumers means the table grows at the comman - [MINOR]
docker/migrations/forward/nodes/node_delegate_skill_orchestrator/0001_delegate_skill_command_claims.sql(glm-review) — terminal_json accepts arbitrary text with an empty-string sentinel | terminal_json is unconstrained TEXT defaulting to ''. Nothing at the schema level distinguishes 'no terminal recorded', 'terminal p - [MINOR]
tests/unit/scripts/validation/test_application_migration_manifest.py(glm-review) — Manifest test updated with a hardcoded count and no coverage of the new migration's semantics | The diff's only test change is bumping an equality assertion from 202 to 203 with an explanatory comment
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]
docker/migrations/forward/nodes/node_delegate_skill_orchestrator/0001_delegate_skill_command_claims.sql(glm-review) — Unbounded growth of claims table with no pruning of terminal rows | The migration explicitly defers reclaiming stale in-flight claims, but it also establishes no retention path for rows that DID recor - [MAJOR]
docker/migrations/forward/nodes/node_delegate_skill_orchestrator/0001_delegate_skill_command_claims.sql(glm-review) — Producer-side deduplication gap not addressed by delivery_id keying | Keying on delivery_id suppresses broker redelivery of the same record, but not producer-level duplication. If an upstream producer - [MINOR]
docker/migrations/forward/nodes/node_delegate_skill_orchestrator/0001_delegate_skill_command_claims.sql(glm-review) — claimed_at stored as TEXT defeats any temporal reasoning | A timestamp as unconstrained TEXT cannot be compared, ordered, or ranged correctly across writers with differing formats or timezones. This f - [MINOR]
docker/migrations/forward/nodes/node_delegate_skill_orchestrator/0001_delegate_skill_command_claims.sql(glm-review) — terminal_json stored as opaque TEXT with no validation constraint | terminal_json is written and later replayed to answer suppressed redeliveries. Nothing in the schema constrains it to valid JSON or - [MINOR]
docker/migrations/forward/nodes/node_delegate_skill_orchestrator/0001_delegate_skill_command_claims.sql(glm-review) — No upper bound or CHECK on TEXT primary key and correlation_id | delivery_id and correlation_id are unconstrained TEXT. A malformed or hostile record id of arbitrary length becomes a primary key; corr - [MINOR]
tests/unit/scripts/validation/test_application_migration_manifest.py(glm-review) — Manifest test change asserts only a count, not the new declaration | The test bumps 202 to 203 with an extensive comment, but the visible diff asserts no properties of the new declaration itself: doma
…ime grant OMN-19029 landed two additions onto omnimarket#2744 (head d7edd16efd28, version 0.4.171): an OMN-15376 reconciliation block on the claims migration, one guarded nullable ADD COLUMN IF NOT EXISTS per declared column between the CREATE TABLE and the correlation_id index, and a companion migration issuing schema USAGE plus SELECT, INSERT, UPDATE to omninode_runtime. Re-vendored both files from that head. The claims migration's bytes changed, and the sync script refuses to rewrite a file whose checksum is already pinned, so its ledger row was removed and re-added with the new checksum rather than the vendored copy being edited by hand. The grant file carries its own declaration row, the pair shape node_projection_ lab_lane_health and node_projection_dod_verdict already use. Both vendored files verified byte-identical to the source tree. With the grant now delivered, the topology regeneration and the self-expiring ownership bridge that were withheld from the previous commit are safe to land. The bridge declares the relation against the committed contract pin, which does not yet carry the producing node's db_io entry, so the push to dev derives what this pull request derives rather than going red one push later. It is registered for expiry against omnimarket#2744 so the pin advance that makes it redundant deletes it. Proven locally against the omnimarket tree at d7edd16efd28: the grant derivation reports three instances in sync and 69 pass, 0 fail, 0 residual on all nine profiles; the push-side derivation holds against the committed pin; the schema-qualification gate passes; the vendor tree reports in sync; and the shape-reconciliation case that was red now passes. No test skipped or xfailed, no exemption widened, no ratchet bound raised. Claude-Session: https://claude.ai/code/session_011WaS5jrEdJDstEEtiJuRvW
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): 11
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.
Below quorum: 11 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_delegate_skill_orchestrator/0001_grant_omninode_runtime_delegate_skill_command_claims.sql(glm-review) — information_schema.role_table_grants may not show grants to omninode_runtime | The assertion queries information_schema.role_table_grants, which per the SQL standard filters rows to those where the cu - [MAJOR]
docker/migrations/forward/nodes/node_delegate_skill_orchestrator/0001_delegate_skill_command_claims.sql(glm-review) — Concurrent in-flight redelivery defeats the double-bill guard | The idempotency design only answers 'did I win' via RETURNING claimed_at at claim time. For a suppressed redelivery, the answer is repla - [MAJOR]
docker/migrations/forward/nodes/node_delegate_skill_orchestrator/0001_delegate_skill_command_claims.sql(glm-review) — Shape reconciliation produces a table the writer cannot use | On a drifted pre-existing table lacking the primary key, the reconciliation block adds delivery_id as a plain nullable column with no uniq - [MAJOR]
docker/migrations/forward/nodes/node_delegate_skill_orchestrator/0001_grant_omninode_runtime_delegate_skill_command_claims.sql(glm-review) — Grant migration contradicts its own independence claim | Section 1's comment states 'a migration must not depend on a sibling file having run', yet section 2 issues GRANT ... ON omninode_internal.dele - [MAJOR]
docker/migrations/forward/nodes/node_delegate_skill_orchestrator/0001_delegate_skill_command_claims.sql(glm-review) — Unbounded claim table with no retention or vacuum strategy | Every delivered record inserts a row keyed on delivery_id, which is per-record, not per-correlation; the table therefore grows linearly wit - [MINOR]
docker/migrations/forward/nodes/node_delegate_skill_orchestrator/0001_delegate_skill_command_claims.sql(glm-review) — TEXT timestamps with no format constraint | claimed_at is TEXT NOT NULL with no CHECK constraint mandating a canonical format. The race verdict depends on comparing the caller-passed claimed_at agains - [MINOR]
docker/migrations/forward/nodes/node_delegate_skill_orchestrator/0001_delegate_skill_command_claims.sql(glm-review) — Unbounded TEXT columns and empty-string sentinels | delivery_id, correlation_id, and terminal_json are unconstrained TEXT. terminal_json in particular stores an arbitrary caller-supplied envelope repl - [MINOR]
docker/migrations/forward/nodes/node_delegate_skill_orchestrator/0001_delegate_skill_command_claims.sql(glm-review) — Reconciliation comment contradicts its own DDL | The comment block states 'Every add is NULLABLE deliberately' and later 'the constraint's absence on a repaired table costs nothing', yet two of the fo - [MINOR]
docker/migrations/forward/nodes/node_delegate_skill_orchestrator/0001_grant_omninode_runtime_delegate_skill_command_claims.sql(glm-review) — Schema USAGE grant is not asserted | The migration asserts SELECT, INSERT, and UPDATE table privileges with fail-loud division checks but does not assert the USAGE ON SCHEMA privilege issued in sectio - [MINOR]
docker/migrations/forward/_ledger/application-migrations.tsv(glm-review) — Duplicate version 0001 within one node lineage | Two forward migrations for node_delegate_skill_orchestrator share the version prefix 0001. Whether this is legal depends on the ledger's uniqueness key - [MINOR]
tests/ci/test_supplemental_declaration_expiry_omn18863.py(glm-review) — Interim declaration expiry keyed on table name alone | _INTERIM_ENTRIES maps 'delegate_skill_command_claims' to omnimarket#2744 without a node or schema qualifier. If another node later declares a tab
The Integration Test Coverage gate classifies this pull request as a feature change now that it touches src/, and it was refusing it for carrying no test under tests/integration/. Rather than satisfy that with a placeholder, this pins the properties of the vendored pair that nothing else in this repository would catch locally. Written in the shape of its neighbour test_dod_verify_runs_omn18900.py, which covers the same table-plus-grant pair shape: file-contract checks that need no database, so they never skip and never contribute a false green to the merge-gating job. Ten cases. The primary key is the delivering record rather than the correlation, because correlation is the retry identity and callers reuse it. Every declared column carries a guarded ADD COLUMN, and each of those adds is nullable, which is the OMN-16777 lesson: a guarded add spelled NOT NULL cannot reconcile a table that already holds rows, and the static gate stays green while it fails. The reconciliation region sits between the create and the column-dependent index, because placed after it the block would be correct and useless. Every privilege the grant issues is asserted back, including schema usage, which is the shape whose absence let OMN-17379 ship a 24-day outage. No DELETE and no sequence grant, the key being TEXT. No tenant column, since an omninode_internal relation enforces no row-level security. And both vendored files match the sha256 their ledger rows record, which catches a hand edit here without needing an omnimarket checkout. The live-execution half is not duplicated. The corpus-wide proof in test_node_migration_shape_drift_omn15376.py already derives an unfixed variant by deleting the reconciliation region from the real file and proves RED on a drifted table, GREEN with the region restored, and schema equality between the fresh and drifted paths. This migration carries the region markers that suite keys on, so it is covered without being named. Claude-Session: https://claude.ai/code/session_011WaS5jrEdJDstEEtiJuRvW
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): 13
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: 13 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_delegate_skill_orchestrator/0001_delegate_skill_command_claims.sql(glm-review) — SHA-256 column in ledger is not verified against DB-stored checksum, and TEXT timestamps lose ordering guarantees | claimed_at is TEXT. If the writer formats it as an ISO-8601 string, lexicographic co - [MAJOR]
docker/migrations/forward/nodes/node_delegate_skill_orchestrator/0001_delegate_skill_command_claims.sql(glm-review) — Correlation_id NOT NULL is dropped on the drifted path without a verification step | The CREATE declares correlation_id TEXT NOT NULL DEFAULT '', but the shape-reconciliation adds it as nullable TEXT - [MAJOR]
docker/migrations/forward/nodes/node_delegate_skill_orchestrator/0001_delegate_skill_command_claims.sql(glm-review) — No NOT NULL constraint re-assertion means the two paths differ in logical shape | The comment claims 'both paths end at the same set of columns', but the fresh path has NOT NULL on delivery_id, correl - [MAJOR]
docker/migrations/forward/nodes/node_delegate_skill_orchestrator/0001_delegate_skill_command_claims.sql(glm-review) — ON CONFLICT DO UPDATE race verdict depends on RETURNING, but the UPDATE arm has no guard against terminal overwrite | The migration documents that terminal_json 'is how a suppressed redelivery still A - [MAJOR]
docker/migrations/forward/nodes/node_delegate_skill_orchestrator/0001_grant_omninode_runtime_delegate_skill_command_claims.sql(glm-review) — Grant migration asserts privileges via 1/count(*) but does not assert USAGE | The comment block explicitly says the schema USAGE is asserted too, and the test file asserts the string 'GRANT USAGE ON S - [MAJOR]
docker/migrations/forward/nodes/node_delegate_skill_orchestrator/0001_delegate_skill_command_claims.sql(glm-review) — Unbounded growth of delegate_skill_command_claims with no retention or expiry | The migration explicitly declines to implement stale-claim reclaim and names no retention policy for rows whose terminal - [MAJOR]
docker/migrations/forward/nodes/node_delegate_skill_orchestrator/0001_delegate_skill_command_claims.sql(glm-review) — correlation_id index has no partial predicate, defeating the stated retention posture | The index on correlation_id is unconditional. Since the table's stated purpose is dedup-by-delivery_id (PRIMARY - [MINOR]
tests/integration/migrations/test_delegate_skill_command_claims_omn18993.py(glm-review) — test_the_grant_issues_no_delete_and_no_sequence_usage matches 'DELETE' as substring | _executable_sql strips comment lines, but 'DELETE' will match any occurrence including inside identifiers, strings - [MINOR]
tests/integration/migrations/test_delegate_skill_command_claims_omn18993.py(glm-review) — Checksum test compares raw bytes to ledger digest, but no test guards the ledger itself against re-vendoring without update | test_the_vendored_pair_matches_the_checksum_the_ledger_records catches han - [MINOR]
docker/migrations/forward/nodes/node_delegate_skill_orchestrator/0001_grant_omninode_runtime_delegate_skill_command_claims.sql(glm-review) — No revocation statement accompanies the grant; previously granted privileges on a drifted lane are never tightened | The grant file issues GRANT SELECT, INSERT, UPDATE but never REVOKEs anything. If a - [MINOR]
tests/ci/test_supplemental_declaration_expiry_omn18863.py(glm-review) — supplemental_declaration_expiry test maps table to PR but no test asserts the PR is actually still open | _INTERIM_ENTRIES now carries delegate_skill_command_claims -> omnimarket#2744. The test 'goes - [MINOR]
tests/integration/migrations/test_delegate_skill_command_claims_omn18993.py(glm-review) — test_the_create_declares_every_column_the_claim_protocol_reads uses fragile whitespace-sensitive substring | The assertion f" {column} " in sql requires exactly four leading spaces and a trailing s - [MINOR]
tests/integration/migrations/test_delegate_skill_command_claims_omn18993.py(glm-review) — test_the_guarded_adds_are_nullable uses .index on a substring that appears inside the reconciliation block itself | _executable_sql strips comments, so the ADD COLUMN IF NOT EXISTS text is searched on
Two conflicts, both the same shape: dev landed OMN-19031's vendored consumer-flow index (202 -> 203) while this branch vendors the claims migration and its runtime grant (202 -> 204). Neither touches the other's rows, so the ledger auto-merged and only the manifest count assertion needed a hand resolution. Resolved by keeping BOTH provenance comments in landing order and stating the composed count once: 202 -> 203 for OMN-19031, then 203 -> 205 for OMN-18993. The count is the sum because the three migrations are disjoint; it is not a bound being raised. Claude-Session: https://claude.ai/code/session_01EYUMfw1eY4umCKvhQvS8ZF
…r tree The supplemental-declaration expiry check and the push-side derivation check, both required in Application Database Domain Enforcement (OMN-15361), demanded opposite things on this pull request. The expiry check said to DELETE the delegate_skill_command_claims bridge; deleting it made assert_push_side_derivation.py refuse the tree, and its refusal text is an instruction to add back exactly what was removed. Both were run locally to confirm, not inferred. OMN-18863 gave the enforcement job a SECOND omnimarket checkout, and this module kept reading the first one. On a trailered pull request .proof-dependencies/omnimarket is the trailer-named BRANCH -- the open source pull request, which by construction declares the very relation the bridge covers -- while .proof-dependencies/omnimarket-pin is the committed pin the push to dev derives from. Measured here: pin f5776c4b45b8 declares 73 relations and NEITHER dod_verify_runs nor delegate_skill_command_claims; trailer tree d7edd16efd28 declares 75 including both. So the check was reading "the source PR is ahead of the pin", which is the premise of every bridge, and reporting it as the bridge having expired. It therefore fired on every trailered vendoring pull request -- the only kind of pull request either check exists for -- and named dod_verify_runs, whose bridge predates this branch, alongside the new one. Prefer the mirror when the job checked one out. On dev and on an untrailered pull request no mirror exists and the single tree IS the pin, so that path is unchanged and this is not a relaxation: with the mirror pointed at a tree that genuinely declares both relations, the assertion still fails and still names both. That control was run. Three new cases cover the selection itself, which nothing observed before: a wrong choice reads green on dev, where only one checkout exists, and misfires only where no test was looking. They build both layouts under tmp_path, so they need no foreign tree and never skip. Claude-Session: https://claude.ai/code/session_01EYUMfw1eY4umCKvhQvS8ZF
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_delegate_skill_orchestrator/0001_delegate_skill_command_claims.sql(glm-review) — Drifted-table path loses the primary key, silently defeating idempotency | The shape reconciliation deliberately adds delivery_id as a plain nullable column and never restores the PRIMARY KEY. On a dr - [MAJOR]
docker/migrations/forward/nodes/node_delegate_skill_orchestrator/0001_grant_omninode_runtime_delegate_skill_command_claims.sql(glm-review) — Grant includes unconstrained UPDATE over terminal_json and claimed_at | The table's correctness argument is protocol-by-discipline: claimed_at must be written only on INSERT and terminal_json only by - [MAJOR]
docker/migrations/forward/nodes/node_delegate_skill_orchestrator/0001_delegate_skill_command_claims.sql(glm-review) — Unbounded table growth with no retention or compaction path | The table is append-and-converge with no DELETE, no expiry, and no archival policy; the migration explicitly defers stale-claim handling t - [MINOR]
docker/migrations/forward/nodes/node_delegate_skill_orchestrator/0001_delegate_skill_command_claims.sql(glm-review) — Timestamps stored as TEXT with no format contract | claimed_at is TEXT with no CHECK constraint or documented serialization format. The 'did I win' verdict is a comparison between the caller-supplied - [MINOR]
tests/integration/migrations/test_delegate_skill_command_claims_omn18993.py(glm-review) — Test docstring claims a schema USAGE assertion that the grant migration does not contain | test_the_grant_covers_and_asserts_every_privilege_it_issues states 'the schema USAGE is asserted too', but th - [MINOR]
docker/migrations/forward/nodes/node_delegate_skill_orchestrator/0001_grant_omninode_runtime_delegate_skill_command_claims.sql(glm-review) — Grant assertion relies on information_schema visibility semantics | information_schema.role_table_grants filters by the current user's role membership chain per the SQL standard; a grant that exists b - [MINOR]
docker/migrations/forward/nodes/node_delegate_skill_orchestrator/0001_delegate_skill_command_claims.sql(glm-review) — Redelivery semantics for terminal_json overwrite are undefined in the schema | The design says a suppressed redelivery replays terminal_json so the consumer still answers, but the schema defines no ru
|
Pre-PR proof: surface=105 receipt=/Users/jonah/Code/omni_home/knowledge-base-internal/.recovery-evidence/omn18993-prepr-proof-105-20260921T190557Z/receipt.txt sha256=ab8136aa35d10f6d5c490148a62690ecc865b24642b9dea93b8c42f73dc04fef result=PASS Run post-merge, and stated plainly rather than dressed as the pre-merge gate it is not. The runtime-proof rule (ledger RULING 18:47:16Z) arrived after both pull requests had already merged: omnibase_infra#3918 at 18:18:56Z, omnimarket#2744 at 18:54:45Z, both with auto-merge armed by the operator at 10:40Z and 10:59Z, hours before the rule existed. There was nothing left to disarm, so the proof was run against the merged bytes instead of the candidate. It protects the tag cut that reads this merge sha, which is the next thing downstream. Not a hot-patch. The .105 dogfood stack was serving ten healthy containers and the ruling says a surface serving something that works is not taken. The changed path here is a migration plus its grant, so it was exercised in a scratch database on the live 21 assertions, 0 failures:
One honest note on method: the first run of the proof reported every assertion failing at once because Surface leased and released through the ledger |
…t dev head The changelog entry opened with PR #3927 covered 14 release-relevant commits as of candidate f552eb1. dev has since carried two more runtime-affecting merges that are release-relevant per the train's own classifier -- #3918 (OMN-18993, the migration this cut needed) and #3932 (OMN-18910) -- bringing the count to 16. Regenerated verbatim via scripts/ci/release_train.py changelog against the branch's own now-current-with-dev tree; no entry hand-written. Onex-Lane: omn18595-decide-token-build-0650 Onex-Session: 65175c9abe4b4da2a60bef324104cf51
|
Pre-PR proof countersign: receipt sha256=ab8136aa35d10f6d5c490148a62690ecc865b24642b9dea93b8c42f73dc04fef actor=prepr-countersign verdict=COUNTERSIGNED-WITH-LIMITS I recomputed the receipt digest and it matches the value quoted in the comment above and in the ledger RELEASE row. I independently re-derived both migration digests from the merged tree rather than taking them from the receipt: Two limits, neither of which touches the result. The proof is a scratch-database run and belongs to the non-exclusive class, but it was recorded under the bare exclusive surface id, so the ledger shows the one shared full-runtime surface held for three minutes by work that excluded nobody. The RELEASE row also omits the |
* chore: release omnibase_infra 0.38.51 (OMN-18595) Cut from dev at f552eb1, whose compose-dev lab-pass receipt (artifact 10649755206) records PASS on all eleven checks. Claude-Session: https://claude.ai/code/session_011WaS5jrEdJDstEEtiJuRvW Onex-Lane: omn18595-decide-token-build-0650 Onex-Session: 65175c9abe4b4da2a60bef324104cf51 * chore: refresh release changelog for omnibase_infra 0.38.51 to current dev head The changelog entry opened with PR #3927 covered 14 release-relevant commits as of candidate f552eb1. dev has since carried two more runtime-affecting merges that are release-relevant per the train's own classifier -- #3918 (OMN-18993, the migration this cut needed) and #3932 (OMN-18910) -- bringing the count to 16. Regenerated verbatim via scripts/ci/release_train.py changelog against the branch's own now-current-with-dev tree; no entry hand-written. Onex-Lane: omn18595-decide-token-build-0650 Onex-Session: 65175c9abe4b4da2a60bef324104cf51 --------- Co-authored-by: onexbot-occ-writer[bot] <307849072+onexbot-occ-writer[bot]@users.noreply.github.com>
1 participant
What
One vendored migration plus its declaration row. Nothing else.
OMN-18887 adds a node-scoped migration in omnimarket creating the durable claim that stops a redelivered delegate-skill command from re-running and re-billing the inference. omnimarket node migrations are vendored here, and
node-migration-vendor-parity-gateenforces the ordering in that direction. Verbatim from the failing run onomnimarket#2744:The gate names its own reason: it exists to stop the recurring node-migration-sync drift class, OMN-14975 and six prior tickets. This PR is that first half.
How the copy was produced
By
scripts/sync-node-migrations.sh, withOMNIMARKET_SRCpointed at the OMN-18887 branch — the script's own documented override, since the migration is not yet on omnimarketdev. Not copied by hand, because hand-copying is precisely the drift the gate catches. Verified byte-identical against the source, and the script's own--checkreports in sync.The declaration row is the other half
A vendored SQL artifact must be bound to one stream, owner, domain, version and content checksum, or explicitly blocked by a ticket. Without the row the declaration check fails with
missing=[...], which it did.Domain is
omninode_internal, nottenant. This is internal control state for one node rather than tenant-scoped data — the same classification thedod_verify_runsrows carry. The table declares no row-level security, so claiming thetenantdomain would assert an enforcement that is not there.The checksum is computed from the vendored file, so a later edit on either side fails the gate rather than drifting quietly. That is the property worth having.
Validator after the change:
Blast radius
No behaviour change in this repository. The table is read and written only by the delegate-skill claim port in omnimarket; nothing here consumes it. The diff is two files: the vendored SQL and its ledger row.
Ordering
This must merge before
omnimarket#2744. That is the whole point of the gate, and why this is a blocking child of OMN-18887 rather than parallel work.Evidence-Ticket: OMN-18993
Evidence-Source: OCC#10659
Cross-repo derivation
The application-domain gate reads its ownership authority from a checkout of omnimarket, and the table this PR vendors is declared in
scripts/application-relation-ownership.yamlon the OMN-18887 branch, which has not merged. Resolving omnimarketdevinstead, the gate does not see that declaration and refuses the relation as unowned — while OMN-18887 cannot merge until this PR lands, because of the vendor-parity ordering. That is a genuine cycle.A bare source-ref trailer does not break it, and measurably makes things worse: the resolver proves a declared ref reachable from omnimarket
devand fails closed on anything unmerged, so an earlier bare trailer here turned a previously green vendor-sync check red withdev...jonah/omn-18 = ahead. The mechanism that does apply is the paired-source path: a source ref together with the open omnimarket pull request and its exact current head SHA. The resolver then verifies that pull request is open, non-draft, targetsdev, lives in the omnimarket repository, and that both its head ref and head SHA match what is declared here. A new commit there invalidates the SHA and this PR goes red rather than drifting.With the paired trailers the resolver returns the pull request head SHA, the vendor tree reports in sync against that tree, and the schema-qualification gate passes against the branch ownership manifest. All three are verified locally.
The omnimarket source is now complete
OMN-19029 landed two additions onto omnimarket#2744, head
d7edd16efd28. The guardedCREATE TABLEnow reconciles all four declared columns with nullable guarded adds, and a companion migration issues schemaUSAGEplusSELECT, INSERT, UPDATEto the runtime principal, so the grant the derivation owes is delivered rather than merely declared.Both files are re-vendored from that head and verified byte-identical to it. The claims migration's bytes changed, and the sync script refuses to rewrite a file whose checksum is already pinned, so its ledger row was removed and re-added with the new checksum rather than the vendored copy being hand-edited. The grant file carries its own declaration row.
With the grant delivered, the topology regeneration and the self-expiring ownership bridge are safe to land and are included here. The bridge declares the relation against the committed contract pin, which does not yet carry the producing node's declaration, so a push to
devderives what this pull request derives instead of going red one push later. It is registered for expiry against omnimarket#2744, so the pin advance that makes it redundant deletes it.Verified locally against that tree: grant derivation reports three instances in sync and 69 pass, 0 fail, 0 residual across all nine profiles; the push-side derivation holds against the committed pin; the schema-qualification gate passes; the vendor tree reports in sync; and the shape-reconciliation case that was red now passes.
Omnimarket-Source-Ref: jonah/omn-18887-delegate-skill-idempotency
Node-Migration-Source-PR: omnimarket#2744
Node-Migration-Source-SHA: d7edd16efd28eadc4309c55d74bf353aacf24655