Skip to content

feat(OMN-14058): add optional tenant_id to delegation wire DTOs [OPERATOR-ACCEPTED INTERIM] - #1404

Merged
jonahgabriel merged 1 commit into
devfrom
jonah/omn-14058-interim-tenant-id-stamp
Jul 8, 2026
Merged

jonahgabriel merged 1 commit into
devfrom
jonah/omn-14058-interim-tenant-id-stamp

Conversation

@jonahgabriel

@jonahgabriel jonahgabriel commented Jul 8, 2026 •

Copy link
Copy Markdown
Contributor

Summary

OMN-14058 (beta-blocking): delegation/savings projection rows land under tenant_id='omninode' (the omnidash-side column default, omnidash/db/migrations/0001_tenant_rls.sql) because no tenant identity exists anywhere in the delegation chain today. OPERATOR-ACCEPTED INTERIM FIX — the operator explicitly accepted this pragmatic path for beta; the durable per-tenant identity design is tracked separately as OMN-14107 and is out of scope here.

This PR is the omnibase_core half: it adds an OPTIONAL tenant_id field to the two canonical delegation wire DTOs so the omnimarket delegation FSM can thread a real tenant identity end-to-end. Companion omnimarket PR threads ONEX_TENANT_ID (read once at request-acceptance) through the FSM and stamps it onto the delegation/savings projection rows.

Changes

  • ModelDelegationRequest (models/delegation/wire/model_delegation_wire_request.py): tenant_id: str | None = Field(default=None, ...).
  • ModelDelegationResult (models/delegation/wire/model_delegation_result.py): same.
  • Both models are frozen=True, extra="forbid" — the field is additive/optional so no existing serialization or validation behavior changes.
  • Uses Field(default=None) (not a bare = None) so the substrate-identity-field-optionality pre-commit gate (which only flags a bare-constant None default on IDENTITY_FIELD_NAMES, and tenant_id is in that set) does not fire — verified locally by running the gate script directly against both files.

Golden-chain impact

None. canonical_request_hash (runtime/golden_chain/recorded_replay_transport.py) only projects model, messages, max_tokens, temperature — tenant_id is not part of that projection, so no fixture re-record is needed (omnimarket/tests/fixtures/golden_chain/*).

Verification

  • uv run pytest tests/unit/models/delegation/ -q — 181 passed.
  • uv run ruff format --check / uv run ruff check on both changed files — clean.
  • uv run mypy on both changed files — Success: no issues found in 2 source files.
  • pre-commit run --files <both files> — all hooks passed (substrate gates, ai-slop, pydantic-pattern, etc.).
  • Full local pytest tests/ in progress at push time (background); will report final tail if requested.

Dependency note

This is a cross-repo interim fix. The companion omnimarket PR imports omnibase_core.models.delegation.wire.ModelDelegationRequest/ModelDelegationResult and depends on this field existing — omnimarket's CI will not go fully green until this PR merges, releases, and the omnibase-core pin in omnimarket's pyproject.toml/uv.lock is bumped (the standard repin omnibase-core X -> Y follow-up, same pattern as recent PR history in that repo).

OCC / DoD

Per lane instructions: NOT self-authoring an OCC Evidence-Source companion (a parallel lane is fixing OCC-companion autogen; see memory feedback_no_self_authored_evidence). This PR needs an independent OCC companion + dod_evidence on OMN-14058 before it can merge — that dependency is flagged, not resolved, here.

Closes OMN-14058 (partially — omnibase_core half only; see companion omnimarket PR for the FSM + projection threading).

No merge, no deploy — this PR is queued for independent adversarial review per the dispatching lane's instructions.

Summary by CodeRabbit

  • New Features
    • Added optional tenant identifiers to delegation requests and results.
    • Supports multi-tenant handling with a fallback tenant value when none is provided.
    • Clarifies that tenant identifiers are named values rather than UUIDs.

Evidence-Source: OCC#3726
Evidence-Ticket: OMN-14058

…ATOR-ACCEPTED INTERIM]

Adds an OPTIONAL tenant_id field to ModelDelegationRequest and
ModelDelegationResult so the delegation FSM (omnimarket) can thread a real
tenant identity through the request/terminal wire boundary instead of
delegation/savings projection rows silently landing under the shared
'omninode' column default.

OPERATOR-ACCEPTED INTERIM per OMN-14058 (beta-blocking tenant attribution).
The durable per-tenant identity design is tracked separately as OMN-14107;
this is the pragmatic beta fix, not the final design.

Both models are frozen + extra="forbid"; tenant_id is added as
Field(default=None) (not a bare `= None`) so no existing serialization or
substrate-gate behavior changes. No golden-chain fixture impact: request_hash
only projects model/messages/max_tokens/temperature.
@coderabbitai

coderabbitai Bot commented Jul 8, 2026 •

Copy link
Copy Markdown
Contributor

Review Change Stack

📝 Walkthrough

Walkthrough

Two delegation wire model files receive a new optional tenant_id: str | None field with default=None, each documented with descriptions explaining multi-tenant isolation semantics, fallback behavior (via ONEX_TENANT_ID env var or shared 'omninode' tenant), and a clarifying comment that the identifier is not a UUID.

Changes

Tenant ID Field Addition

Layer / File(s) Summary
Add tenant_id field to wire models
src/omnibase_core/models/delegation/wire/model_delegation_wire_request.py, src/omnibase_core/models/delegation/wire/model_delegation_result.py
Both models add an optional `tenant_id: str

Estimated code review effort: 1 (Trivial) | ~5 minutes

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and accurately summarizes the main change: adding an optional tenant_id to delegation wire DTOs.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch jonah/omn-14058-interim-tenant-id-stamp

Warning

Review ran into problems

🔥 Problems

Git: Failed to clone repository. Please run the @coderabbitai full review command to re-trigger a full review. If the issue persists, set path_filters to include or exclude specific files.


Comment @coderabbitai help to get the list of available commands.

@jonahgabriel

Copy link
Copy Markdown
Contributor Author

Cross-repo heads-up (independent adversarial verification of the companion omnimarket PR #1648):

The wire-DTO tenant_id field added here is correct in isolation, but the downstream projection write path has a HIGH-severity latent defect: delegation_events and savings_estimates (the two tables the omnimarket delegation projection handlers write tenant_id into) have no tenant_id column in the omnibase_infra-owned schema — git grep tenant_id -- '*.sql' in omnibase_infra finds it only on an unrelated table (0019_delegation_budget_state.sql). The repeated claim that "the NOT NULL DEFAULT 'omninode' column applies cleanly on INSERT" is false for both tables; the only place that default exists is a manually-applied, cloud-only omnidash migration that never touches the runtime Postgres these handlers write to.

Once ONEX_TENANT_ID is set, PostgresSyncProjectionAdapter.upsert() will raise column "tenant_id" does not exist on real Postgres — masked today only because unit tests use an in-memory dict adapter with no schema to violate.

A migration + real-Postgres regression test is in flight under OMN-14058 (source of truth in omnimarket's node migration dirs, vendored into omnibase_infra). Not a blocker for this PR's DTO change specifically, but the omnimarket companion (#1648) should not merge/deploy until that migration lands — flagging here since this PR is the dependency root of that chain.

@jonahgabriel
jonahgabriel merged commit aae8787 into dev Jul 8, 2026
236 of 284 checks passed
@jonahgabriel
jonahgabriel deleted the jonah/omn-14058-interim-tenant-id-stamp branch July 8, 2026 22:23
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