Skip to content

feat(OMN-18893): the pre-PR verify pool bring-up entrypoint and its refusals - #3938

Merged
jonahgabriel merged 6 commits into
devfrom
jonah/omn-18893-prepr-verify-lane-entrypoint
Sep 21, 2026
Merged

jonahgabriel merged 6 commits into
devfrom
jonah/omn-18893-prepr-verify-lane-entrypoint

Conversation

@jonahgabriel

@jonahgabriel jonahgabriel commented Sep 21, 2026 •

Copy link
Copy Markdown
Collaborator

What this is

Task 4 of the ephemeral pre-PR verify lanes epic and its settled plan. Rule 24(e) already sanctions building a branch workspace image through exactly one named entrypoint, scripts/runtime_build/prepr_verify_lane.sh, and records in the same paragraph that it does not exist yet. This is that entrypoint, plus the compose overlay, the slot policy table and the rendered-config gate it needs in order to be safe on a host that is also running the dev lane.

OMN-18893. It builds on the two merged tasks before it, the per-seam topic namespace and the per-slot databases with their own principals, and everything after it in the epic depends on this.

The shape

A slot is an ephemeral compose project running ONE branch's runtime family against the dev lane's shared Postgres, Redpanda, Valkey and Keycloak, isolated on every axis by namespace. It is a premise for nothing: outside GOVERNED_LANES and GRANT_INTERLOCK_LANES, sourcing no stability-proven digest, never part of a promotion's evidence chain.

It accepts no lane argument at all. There is no --lane, --compose-project, --force or --skip option. The compose project, ports, topic namespace, database suffix, consumer-group token and Valkey index are all derived from a slot number by scripts/runtime_build/prepr_slot_policy.py. That table is also the refusal set, so every declared lane is refused by name under its own exit code and anything unrecognised is refused as unknown rather than created. The tests read the table the shell reads, which is what the acceptance criterion asks for: a grep for a lane name passes against a comment as happily as against a branch that refuses it.

A live defect found before this PR opened

This is the reason the rendered-config gate exists, and it is worth reading before the rest.

Compose resolves an interpolation from the OS environment before the env file. The lab host's interactive shell exports the dev lane's own consumer-group environment token. A slot brought up from a terminal would therefore have joined the dev lane's consumer groups on all eleven services while every file on disk said it was isolated. Measured by rendering the three-file merge twice, once with the ambient value and once without.

Two further compose merge behaviours were caught the same way:

  • ports and volumes are appended to the inherited value unless the override entry carries an override tag. An appended port binds the dev lane's 8085; an appended volume mounts the dev lane's data at the same path.
  • The inherited runtime environment's DSNs resolve to the superuser, which reaches every dev database and is exempt from the forced row-level security the tenant tables rely on.

None of the three is visible in a review of the overlay. All three are visible in the rendered output, so the rendered output is what is checked. prepr_verify_rendered_slot.py re-reads the render before anything starts and refuses the bring-up on a mismatch, checking container names, published ports, topic namespace, consumer-group environment, consumer groups, every DSN's database and principal suffix, named volumes and network externality — every expectation derived from the policy table rather than spelled.

What the overlay is responsible for

docker/docker-compose.prepr.yml is a third layer over infra.yml and dev-lane.yml rather than a standalone file. Copying 1,700 lines to change a port and a prefix guarantees the copy drifts, and a slot that verified a branch against a stale copy of the lane definition is a false pass, which is the one failure this epic exists to prevent.

Responsibility Why it is not cosmetic
Fencing — every non-slot service moved onto an unreachable compose profile postgres, redpanda, valkey, keycloak, infisical and the redpanda one-shots carry no profile in the inherited files, so a bare bring-up would create each under its hardcoded dev container name. That is the compose-project collision that left the stability lane broker-less for three days
Volume isolation — all seven named volumes redeclared with the slot in the name Every named volume in infra.yml declares a global Docker name, so an inherited one is the dev lane's volume. A slot would write into the dev lane's data and a teardown with volumes would destroy it
Namespacing — ports, container names, topic prefix, consumer-group environment, per-writer consumer groups and every DSN Each is spelled with no default, so an unset value aborts compose rather than silently starting a slot on the dev lane's namespace

Provenance

The entrypoint stages a slot-private source snapshot and never checks out, fetches into, resets or cleans a canonical clone. It deliberately does not call stage_workspace.sh, which does all four: a slot calling it would test canonical sibling code while naming the branch, and would mutate the shared clones that the dev lane's own builds and a second slot read.

Recorded on the slot descriptor rather than asserted: the target commit, each sibling's commit and where it came from, a content digest of the snapshot itself, and the digest of the image that actually runs. A dirty target worktree is refused rather than built, because the gate keys on a commit and an image built from uncommitted content does not correspond to one.

A pool-wide build lock admits one build at a time and is released before boot, so two slots overlap in every phase except the one that is already serialised.

Attribution

The preflight gains REASON_REQUIRED_LANES. The pool lanes are in it and deliberately in neither GOVERNED_LANES nor GRANT_INTERLOCK_LANES. A slot build mutates a shared host and owes attribution; a slot promotes nothing and sources no stability-proven digest, so no rebuild of one can erode a live prod grant. Extending the interlock would additionally gate an ordinary branch build on resolving the change-control repository, which is a worse gate than none.

POOL_LANES is spelled as a literal there rather than imported from the new module, so the live stability gate gains no import dependency on an ephemeral surface. A test pins the two equal.

Stated limits

  • The slot's gateway is declared, namespaced and not started by default. onex-api is image-referenced rather than lane-built, and a tag predating the topic-namespace change carries no such surface, so starting it would publish unprefixed into the dev lane's topics through the very path the isolation protects. Pinning that image is Task 5 of the epic. The descriptor records whether the gateway started, so a receipt cannot imply a proof that did not run. A running slot therefore shows eleven of the manifest's twelve services until Task 5 lands.
  • No slot has been booted on the lab host yet by this change. Everything below was measured against a real three-file compose render and the entrypoint's own refusal paths. The first live boot is the next step on this ticket and its receipt path will be posted here.
  • This lane must not author the slot-boot evidence for its own ticket. A second actor is needed to verify the first booted slot; the orchestrator will dispatch one.

Rendered against the real infra.yml + dev-lane.yml + prepr.yml merge on this host.

Control Result
Correct slot environment renders clean exit 0, every axis matches policy
Ambient consumer-group token leaks in exit 12, named on all 11 services
Dev port appended rather than replaced exit 12, names the reserved block
Inherited global volume exit 12, names the shared-data hazard
Inherited dev consumer group exit 12, names the partition split
Unsuffixed superuser DSN exit 12, names database and principal
Non-external network exit 12
Bare bring-up with no profile resolves to zero services

Entrypoint refusals, each exercised live against a real worktree:

Refusal Exit
No stated reason 6
Dirty target worktree 5
Slot outside the pool 2
A lane argument 2, rejected as an unknown option
Plan-only on a clean worktree 0, and it correctly preferred the ticket's own sibling worktree over the canonical clone

Unit tests: 45 passed. Lint, format and type checks clean; shellcheck clean at warning level.

Delegation READY probe: correlation 95e16b7f-5e52-40d5-8c0b-cf71c81749c7, run ffab6d1b-2ed7-435e-8a64-ebaf873ac9f6, exit 0 in 15.2 s, quality 1.0 against a 0.8 bar.

Evidence-Ticket: OMN-18893
Evidence-Source: OCC#10722

…efusals

Task 4 of epic OMN-18888. Rule 24(e) already sanctions building a branch
workspace image through one named entrypoint,
scripts/runtime_build/prepr_verify_lane.sh, and records that it does not exist
yet. This is that entrypoint, plus the three things it needs in order to be
safe on a host that is also running the dev lane.

It accepts no lane argument at all. There is no --lane, --compose-project,
--force or --skip option; the compose project, ports, topic namespace,
database suffix, consumer-group token and Valkey index are all DERIVED from a
slot number by scripts/runtime_build/prepr_slot_policy.py. That table is also
the refusal set, so every declared lane is refused by name under its own exit
code and anything unrecognised is refused as unknown rather than created. The
tests read the table the shell reads, which is what the acceptance criterion
asks for: a grep for a lane name passes against a comment as happily as
against a branch that refuses it.

docker/docker-compose.prepr.yml is a third layer over infra.yml and
dev-lane.yml rather than a standalone file. Copying 1,700 lines to change a
port and a prefix guarantees the copy drifts, and a slot that verified a
branch against a stale copy of the lane definition is a false pass, which is
the one failure this epic exists to prevent. It is responsible for exactly
three things.

FENCING. Every service that is not one of the slot's twelve moves onto an
unreachable compose profile. postgres, redpanda, valkey, keycloak, infisical
and the redpanda one-shots carry NO profile in the inherited files, so a bare
bring-up would create each of them under its hardcoded DEV container name,
which is the OMN-13581 failure that left the stability lane broker-less for
three days. Rendered with no profile, the slot project now resolves to zero
services.

VOLUME ISOLATION. Every named volume in infra.yml declares a GLOBAL Docker
name, so an inherited runtime_logs is the dev lane's volume rather than a
project-scoped one. A slot would have written into the dev lane's data, and a
teardown with volumes would have destroyed it. All seven are redeclared with
the slot in the name.

NAMESPACING. Ports, container names, topic prefix, consumer-group environment
token, per-writer consumer groups and every DSN, each spelled with no default,
so an unset value aborts compose rather than silently starting a slot on the
dev lane's namespace.

A LIVE DEFECT, found by rendering the merge rather than reading it, and the
reason a rendered-config gate exists at all. Compose resolves an interpolation
from the OS environment BEFORE the env file, and the lab host's interactive
shell exports the dev lane's own consumer-group environment token. A slot
brought up from a terminal would have joined the DEV LANE's consumer groups on
all eleven services while every file on disk said it was isolated. The
entrypoint now exports the slot's values into the real process environment,
where they outrank the ambient ones, and prepr_verify_rendered_slot.py
re-reads the rendered output rather than trusting that. It checks container
names, published ports, topic namespace, consumer-group environment, consumer
groups, every DSN's database and principal suffix, named volumes and network
externality, all derived from the policy table, and refuses the bring-up
before anything starts.

Two further merge behaviours were caught the same way and are covered by that
same gate: compose APPENDS ports and volumes unless the override entry carries
an override tag, and the inherited runtime environment's DSNs resolve to the
SUPERUSER, which reaches every dev database and is exempt from the forced
row-level security the tenant tables rely on.

The entrypoint stages a slot-private source snapshot and never checks out,
fetches into, resets or cleans a canonical clone. It does not call
stage_workspace.sh, which does all four and would test canonical sibling code
while naming the branch. It records the target commit, each sibling's commit
and origin, a content digest of the snapshot itself, and the digest of the
image that actually runs. A dirty target worktree is refused rather than
built, because the gate keys on a commit and an image built from uncommitted
content does not correspond to one. A pool-wide build lock admits one build at
a time and is released before boot, so two slots overlap in every phase except
the one that is already serialised.

The attribution preflight gains REASON_REQUIRED_LANES. The pool lanes are in
it and deliberately in neither GOVERNED_LANES nor GRANT_INTERLOCK_LANES: a
slot build mutates a shared host and owes attribution, but a slot promotes
nothing, sources no stability-proven digest and is a premise for nothing, so
no rebuild of one can erode a live prod grant. Extending the interlock would
also gate an ordinary branch build on resolving the change-control repository.

The slot's own gateway is declared, namespaced and NOT started by default.
onex-api is image-referenced rather than lane-built, and a tag predating
OMN-18891 carries no topic-namespace surface, so starting it would publish
unprefixed into the dev lane's topics through the very path the isolation
protects. Pinning that image is Task 5 (OMN-18894). The slot descriptor
records whether the gateway started either way, so a receipt cannot imply a
proof that did not run.

Evidence: 45 unit tests, including a positive control on a correct render and
one negative per isolation defect; six controls run against a real three-file
compose render on this host; and every entrypoint refusal exercised live
(no reason, dirty worktree, out-of-pool slot, and a lane argument rejected).

Evidence-Ticket: OMN-18893
…e the slot's own

The entrypoint never resolved the operator environment at all, so every
fail-closed interpolation in the compose overlay -- the shared broker
credentials, the Postgres superuser used to provision the slot, the Keycloak
admin client secret -- would have been unset and the bring-up would have
aborted at the first compose invocation, after the twenty-five minute build
had already been paid for.

It now sources the same two files the deploy path does, in the same order and
under the same `set -a`: the rendered runtime policy, then the operator env
file, resolved from OMNIBASE_OPERATOR_ENV_FILE with the deploy path's default
and refused fail-closed when unreadable. No --env-file is passed and no
snapshot copy is written, matching the reason deploy-runtime.sh stopped
copying one.

The ORDER is the load-bearing part. That file sets KAFKA_ENVIRONMENT to the
dev lane's own consumer-group token on this host, so sourcing it AFTER the
slot's exports would overwrite the slot's namespace with the dev lane's and
put every slot service in the dev lane's consumer groups -- the same defect
the ambient-shell finding already covers, arriving by a second route. The
block therefore sits before the slot exports, and the rendered-config gate
re-reads the result rather than trusting the ordering, because an ordering
argument in a comment is not a control.

Evidence-Ticket: OMN-18893
…r, not after it

The previous commit put the operator env block between the database
provisioning step and the slot's own exports. That fixed one ordering defect
and left another: provision_db_slot.sh opens its superuser connection from
POSTGRES_USER / POSTGRES_PASSWORD / POSTGRES_HOST, which live in the operator
env file and nowhere else, so on a real host it would have failed on an unset
credential after the snapshot had already been staged.

The block now sits immediately after the snapshot, which it needs because it
reads the rendered runtime policy out of that snapshot, and before both
consumers. Its header records that the ordering is load-bearing in both
directions and names the defect each direction produces, so a later reader
moving it has to argue with a specific failure rather than with a preference.

Found by dry-running the entrypoint against the lab host before the live
bring-up.

Evidence-Ticket: OMN-18893
jonahgabriel pushed a commit to OmniNode-ai/onex_change_control that referenced this pull request Sep 21, 2026
…ibase_infra#3938 (#10722)

* evidence(OMN-18893): author OCC companion for OmniNode-ai/omnibase_infra#3938

OCC companion by node_pr_lifecycle_fix_effect (OMN-13317 F1 / OMN-13990 / OMN-14285). Product PR head a252f906271651b1221578be7e856b96dad9d3ec.

* evidence(OMN-18893): self-bind OCC#10722 + rebind contract_sha256

---------

Co-authored-by: omnimarket-bot <bot@omninode.ai>
…from the host

The first live slot provisioning on the lab host died with
`psql: command not found`, after the source snapshot had been staged and the
lane lock taken.

The provisioner invoked a bare `psql`, which assumes a PostgreSQL client on
the host. The lab host has none and never needed one: nothing on PATH,
nothing under /usr/lib/postgresql, no postgresql-client package. Every
migration seam in this repository runs psql INSIDE a postgres:16-alpine
container against the lane's own network, and the dev lane's
forward-migration, cloud-migration, intelligence-migration and migration-gate
services are all that shape. The assumption was simply never exercised,
because until now nothing ran this script anywhere but a developer machine.

psql is now resolved rather than assumed. The host binary is used when one
exists, which is the developer-machine case and what the unit tests exercise;
otherwise a throwaway container on the lane network. No host package is
installed and no image is pulled, since that image is already resident for
the dev lane's own one-shots.

Both paths take the password from the environment and never from argv, which
is why the container form passes a bare `-e PGPASSWORD` rather than an inline
value: a value there would be visible in `docker inspect` and in the host's
process list for the life of the call. Which path answered is printed, so a
run that switched paths between two hosts is visible in the log rather than
silent.

Neither available is a refusal with a named code, not a guess: a
half-provisioned slot on a shared server is worse than an unprovisioned one.

Two tests. One asserts no bare psql call survives outside the resolver, read
over the source rather than by running the script, because the failure is the
ABSENCE of a binary and a test host that happens to have one cannot reproduce
it. The other asserts the password never reaches a command line. The 44
existing provisioner tests still pass unchanged.

Evidence-Ticket: OMN-18893

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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] scripts/runtime_build/prepr_verify_lane.sh / scripts/runtime_build/prepr_verify_rendered_slot.py (glm-review) — TOCTOU gap between rendered-config verification and compose up | The design's stated threat model includes ambient environment variables outranking file values during interpolation (KAFKA_ENVIRONMENT=
  • [MAJOR] docker/docker-compose.prepr.yml; scripts/runtime_build/prepr_verify_lane.sh (glm-review) — Slot database credentials injected as process/container environment | The entrypoint reads the provisioner's env file, remaps <ROLE_UPPER>_PASSWORD variables, and passes them into compose interpolatio
  • [MINOR] scripts/runtime_build/prepr_verify_rendered_slot.py (glm-review) — DSN regex rejects valid DSN forms, risking inconsistent enforcement | _DSN requires an explicit numeric port and a plain postgres/postgresql scheme. DSNs with postgresql+asyncpg:// schemes, no port,
  • [MINOR] scripts/runtime_build/prepr_verify_rendered_slot.py (glm-review) — Verifier assumes mapping-form compose environment | The verifier reads svc.get("environment") or {} and performs key lookups. Compose permits environment as either a mapping or a list of KEY=VALUE s
  • [MINOR] scripts/runtime_build/prepr_verify_lane.sh (glm-review) — Provisioner and scripts run from REPO_ROOT, not the slot snapshot | The entrypoint builds the runtime from a slot-private source snapshot precisely so the branch under test is isolated from canonical
  • [MINOR] docker/docker-compose.prepr.yml; scripts/runtime_build/prepr_verify_rendered_slot.py; tests/unit/scripts/test_prepr_verify_lane_entrypoint_omn18893.py (glm-review) — Service-count inconsistency: twelve vs eleven | The overlay states the slot runs twelve services and the test docstring claims twelve container names are pinned against the census manifest, but the ve
  • [MINOR] tests/unit/scripts/test_prepr_verify_lane_entrypoint_omn18893.py (glm-review) — Slot resolution tested only for slot 3, not the negative boundary | test_a_slot_number_outside_the_pool_is_refused exercises resolve_slot(3) only. Slot 0, negative numbers, and non-integer input are u
  • [MINOR] scripts/runtime_build/prepr_verify_lane.sh (glm-review) — Exit-code duplication across shell and Python is a known drift vector | Twelve exit constants are hand-mirrored between prepr_slot_policy.py and prepr_verify_lane.sh with a test as the only drift dete
  • [MINOR] scripts/runtime_build/prepr_verify_rendered_slot.py (glm-review) — Container-name prefix check uses substring containment | The verifier asserts _container_prefixes(policy) not in container, a substring test. For slot 1, the token -prepr-1- is a substring of any

@github-actions

github-actions Bot commented Sep 21, 2026 •

Copy link
Copy Markdown
Contributor

⚠️ Hostile Reviewer — DEGRADED (informational)

Critical findings: 0
Major findings: 4
Total findings: 8
Models succeeded: glm-review

Note: Fewer than 2 reviewer models succeeded. Degraded results are informational (OMN-8468/OMN-8524) and do not block merge. Error: cli_review exit 2 (fewer than 2 models succeeded — partial/total outage)


Semantics (OMN-17492 — the model finds, thread resolution gates)

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)

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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] scripts/provision_db_slot.sh, psql resolution block (glm-review) — psql container path passes PGPASSWORD through environment and pulls an unpinned image | The container-based psql_run forwards PGPASSWORD from the host environment into the container via `-e PGPASSWORD
  • [MAJOR] scripts/provision_db_slot.sh, psql_run definition (glm-review) — Unconditional -i in container psql_run consumes the caller's stdin | docker run -i attaches the parent's stdin for every invocation, including the -c-only call sites (scalar, psql_admin, `psql
  • [MAJOR] scripts/provision_db_slot.sh, ONEX_PSQL_NETWORK default (glm-review) — Hardcoded default network name in container psql path | ONEX_PSQL_NETWORK defaults to the literal omnibase-infra-network. Compose network names are project-qualified depending on how the base comp
  • [MAJOR] docker/docker-compose.prepr.yml, volume isolation section (glm-review) — Shared overlay plus per-slot volume names requires interpolation the shown patch does not establish | The overlay's stated design is that 'Nothing in docker-compose.prepr.yml names a slot' so one file
  • [MINOR] scripts/provision_db_slot.sh, tooling refusal (glm-review) — EXIT_USAGE used for a missing-tooling environment failure | The refusal when neither psql nor docker exists exits with $EXIT_USAGE. Exit code 2 conventionally and locally (per prepr_slot_policy.py)
  • [MINOR] scripts/runtime_build/prepr_verify_rendered_slot.py, _DSN definition (glm-review) — DSN regex rejects legitimate DSN shapes, weakening the positive suffix check | _DSN requires an explicit user:password@ segment and a numeric port: `^postgres(?:ql)?://(?P[^:/@]+):[^@]*@(?P<ho
  • [MINOR] scripts/runtime_build/prepr_verify_lane.sh, usage block (glm-review) — Attribution reason accepted from ambient environment variable | The entrypoint accepts the attribution reason from ONEX_DEPLOY_REASON in the environment. On the same lab host the design already docume
  • [MINOR] scripts/preflight_lane_deploy_attribution.py, POOL_LANES block (glm-review) — Deliberate literal duplication of POOL_LANES relies solely on a test for consistency | POOL_LANES is redeclared as a literal in the attribution gate rather than imported, justified by import-time frag
  • [MINOR] scripts/provision_db_slot.sh; tests/unit/scripts (psql resolution) (glm-review) — No test coverage shown for stdin/TTY behavior divergence between psql paths | The change introduces two materially different psql execution paths (host binary vs docker run -i) selected by host state,

The first live build reached the Dockerfile and died immediately at
`BUILD_SOURCE=workspace requires OMNI_HOME`, with the arg empty.

Exporting a variable is not the same as passing a build arg. The compose
files declare only the two BUILD_SOURCE selectors under `args:`, and the
Dockerfile's workspace branch refuses without several more, so the canonical
deploy path passes the whole set on the command line. This does the same.

Three of them are not plumbing.

PROMOTION_CLASS=stability-candidate with NON_MAIN_LINEAGE=true is mandatory
on any workspace build, and here it is exactly right rather than merely
tolerated: it stamps the image so the prod-promotion gate refuses it. That is
the same statement as "a slot is a premise for nothing", enforced on the
artifact instead of in prose.

RUNTIME_VERSION is read from the SNAPSHOT's pyproject rather than the clone's,
for the same reason every other provenance fact is: the snapshot is what gets
built. The Dockerfile refuses the placeholder on a workspace build, so an
unreadable version now fails the build rather than labelling the image with a
version it does not have.

NODE_INVENTORY is passed empty, deliberately. The inventory is a property of
the image and cannot be known before the image exists, and an empty value
skips the Dockerfile's verify guard. The two-pass stamping the declared lanes
do is not reproduced: a slot's image is destroyed with the slot and is never
promoted, so nothing downstream reads that stamp.

Evidence-Ticket: OMN-18893

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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): 10
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: 10 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] scripts/provision_db_slot.sh, psql resolution block (glm-review) — Container psql path does not conceal the password as claimed | The comment asserts that passing PGPASSWORD via -e PGPASSWORD rather than an inline value avoids exposure in docker inspect and the h
  • [MAJOR] scripts/provision_db_slot.sh, ONEX_PSQL_NETWORK default (glm-review) — Hardcoded default network name for container psql path | ONEX_PSQL_NETWORK defaults to omnibase-infra-network, a name specific to one lane's compose deployment. If the lane's network is project-scop
  • [MAJOR] scripts/provision_db_slot.sh, psql resolution block (glm-review) — docker CLI present but daemon unreachable is not handled | The psql resolution chooses the container path on command -v docker alone. On a host with the docker binary installed but the daemon stoppe
  • [MAJOR] scripts/runtime_build/prepr_verify_rendered_slot.py, _DSN pattern (glm-review) — DSN regex silently passes DSNs it cannot parse | The verifier's DSN check compiles a regex requiring an explicit user, password, host, numeric port, and path. A rendered DSN that omits the port, uses
  • [MINOR] scripts/provision_db_slot.sh, psql_run container arm (glm-review) — Per-call container spawn for every psql invocation | The container path spawns and destroys a container for each psql_run call, and the script makes many such calls (scalar() probes, psql_admin, datab
  • [MINOR] scripts/runtime_build/prepr_verify_rendered_slot.py, _container_prefixes (glm-review) — Container-name check uses unanchored substring match | The container_name check searches for the substring -prepr-<slot>- anywhere in the rendered name. A dev-lane container whose name happens to em
  • [MINOR] scripts/preflight_lane_deploy_attribution.py, POOL_LANES (glm-review) — Deliberate duplication of POOL_LANES widens the drift window between test runs | POOL_LANES is duplicated as a literal instead of imported, with drift detection delegated to a test that asserts set eq
  • [MINOR] tests/unit/scripts/test_prepr_verify_lane_entrypoint_omn18893.py, module-level _load calls (glm-review) — Test module loads scripts at import time via exec_module | The test module executes prepr_slot_policy.py and prepr_verify_rendered_slot.py at collection time through importlib with sys.modules registr
  • [MINOR] scripts/provision_db_slot.sh, ONEX_PSQL_IMAGE default (glm-review) — ONEX_PSQL_IMAGE environment override enables arbitrary image execution | The image is overridable via ONEX_PSQL_IMAGE with no validation. An environment value pointing at an attacker-influenced regist
  • [MINOR] scripts/provision_db_slot.sh, container psql_run arm (glm-review) — Empty or unset PGPASSWORD not rejected before container path | The container path forwards PGPASSWORD through -e PGPASSWORD, which forwards whatever the variable holds, including unset or empty. The

The first build that got past the build args ran the whole workspace install,
verified all three vendored siblings with matching content parity, and then
failed closed at the provenance step:

  Per-repo VCS provenance has no sibling entries; the staged tree is
  unverifiable (OMN-13030).

That is the in-image verifier doing exactly its job. Dockerfile.runtime COPYs
workspace/sibling-vcs-provenance.json and workspace/sibling-pin-comparison.json
unconditionally, and both are tracked PLACEHOLDERS in the repository that the
staging step is expected to overwrite. A slot-private snapshot carries the
committed placeholder, so the image could not say which commit of each
sibling it had vendored, and the build refused rather than stamping unknown.

This is the honest cost of not calling stage_workspace.sh, and it is worth
paying: that helper reads siblings from the ambient canonical clones and
checks them out, which is the unreliable-narrator build the isolated snapshot
exists to remove. What it also does, and what was missing here, is stamp the
per-repo git identity.

So the facts this script already captured are now written into the snapshot in
the shape the verifier reads: per-sibling commit, branch and dirty flag, plus
the .build-sha marker each staged tree needs because rsync drops .git and
without it the tree has no recoverable commit at all.

They are not a second source of truth. The same three values go onto the slot
descriptor, so the descriptor and the image's own /app/build-provenance.json
cannot disagree about what was vendored -- which is the property that makes
either of them worth reading.

Evidence-Ticket: OMN-18893

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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: 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: 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] scripts/provision_db_slot.sh, new psql_run definition (glm-review) — PGPASSWORD is still exposed in docker inspect despite stated rationale | The new container-based psql path passes the database password via docker run -e PGPASSWORD. The comment claims this avoids e
  • [MAJOR] scripts/provision_db_slot.sh, ONEX_PSQL_IMAGE / ONEX_PSQL_NETWORK assignments (glm-review) — psql container image and network are caller-overridable environment variables | ONEX_PSQL_IMAGE and ONEX_PSQL_NETWORK default sensibly but are read from the ambient environment with no validation. On
  • [MAJOR] scripts/runtime_build/prepr_verify_lane.sh, usage text and gateway handling (glm-review) — Unpinned onex-api image ships in the same diff that documents the hazard | The entrypoint accepts --with-gateway and starts the slot's onex-api today, while the diff itself states that any tag predati
  • [MAJOR] docker/docker-compose.prepr.yml, network configuration (glm-review) — Slot isolation is namespace-only on a fully shared bridge network | Slot services join the dev lane's network and share the dev lane's Postgres, Redpanda, Valkey, and Keycloak by hostname. The DSN suf
  • [MINOR] scripts/provision_db_slot.sh, psql_run function (glm-review) — docker run -i in psql_run can block when stdin is an interactive terminal | psql_run always passes -i so heredocs on stdin reach psql. But most call sites (psql_admin, scalar) use -c and never
  • [MINOR] scripts/runtime_build/prepr_verify_rendered_slot.py, _DSN definition (glm-review) — DSN regex in rendered-slot verifier is narrower than real DSNs | _DSN requires an explicit :\d+ port and a host of [^:/@]+. A DSN rendered without an explicit port (5432 default is legal in postgr
  • [MINOR] scripts/preflight_lane_deploy_attribution.py, POOL_LANES definition (glm-review) — Pool lane set duplicated as literal, pinned only by a cross-module test | POOL_LANES in preflight_lane_deploy_attribution.py duplicates the slot names from prepr_slot_policy.py with an explicit reject
  • [MINOR] scripts/runtime_build/prepr_verify_lane.sh, exit-code block (glm-review) — Exit codes redeclared as literals in shell and pinned by reading source | Fourteen EXIT_* constants are duplicated between Python and the shell script, and the guard against drift is a test module tha

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant