[jonahgabriel]

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 is the omnimarket half — it threads a real tenant identity through the delegation FSM and stamps it onto the projection rows. Companion omnibase_core PR (OmniNode-ai/omnibase_core#1404) adds the tenant_id field to the two canonical wire DTOs this depends on.

Changes

• src/omnimarket/config/settings.py: new onex_tenant_id: str = Field(default="", ...) field — the sanctioned config surface, mapped by pydantic-settings (case_sensitive=False) to the ONEX_TENANT_ID env var. Regenerated .env.example via scripts/generate_env_example.py.
• handler_delegation_workflow.py (bus orchestrator FSM): DelegationWorkflowState.tenant_id resolved exactly ONCE in handle_delegation_request (request-acceptance) — request.tenant_id wins if the caller set it, else falls back to ONEX_TENANT_ID via get_settings(). Carried onto every TerminalEmissionInputs construction site (completed / pre-gate-failed / agent-lifecycle) and through _emit_terminal into the canonical ModelDelegationResult.
• port_local_delegation_dispatch.py (bus-less onex delegate CLI path): same resolve-once-at-dispatch()-acceptance pattern, mirroring the existing source_session_id forwarding into the delegate-skill terminal payload — this is a genuinely separate production entry point (LocalDelegationDispatchPort) that also writes delegation_events evidence rows via HandlerProjectionDelegation, so it needed the same fix to actually close the gap for the CLI path (not just the bus path).
• model_delegate_skill_terminal_projection.py: tenant_id field added to ModelDelegateSkillTerminalProjection (validation_alias tenant_id/tenantId), ModelDelegationEventProjectionRow, ModelDelegateSkillSavingsProjection, and ModelTaskDelegatedSavingsSource (+ from_canonical_payload mapping), so the value survives every projection conversion hop.
• handler_projection_delegation.py: ModelProjectionTaskDelegatedEvent.tenant_id field; project() and project_delegate_skill_terminal() stamp row["tenant_id"] only when the source event carries a non-blank value — the key is omitted (never written as None) so the NOT NULL DEFAULT 'omninode' column applies cleanly on INSERT and an already-known tenant is left untouched on UPDATE (verified against postgres_sync_database.py's upsert(), which builds the INSERT column list directly from row.keys()). _canonical_result_to_task_delegated_payload now carries tenant_id through the canonical-terminal→compat-shape conversion.
• handler_projection_savings.py: same stamp-when-present pattern in project_delegate_skill_savings(). The plain ModelSavingsEstimatedEvent.project() path (a separate upstream event, onex.evt.omnibase-infra.savings-estimated.v1, not part of the delegation chain) is intentionally untouched — no tenant identity is available on that source event.
• Bug fix bundled with the interim stamp: handler_projection_delegation.py's materialize_budget_state call site previously passed tenant_id=event.session_id — a session identifier is not a tenant identifier. Now passes the real event.tenant_id, falling back to ModelDelegationBudgetStateEvent.resolved_tenant()'s existing DEFAULT_TENANT when none is resolved. This was a live latent bug in the same function I was already touching for OMN-14058; flagging it explicitly since it's not literally in scope but is directly adjacent and cheap to fix correctly rather than propagate.
• Two new contract-conformance tests (tests/test_budget_state_surface_omn13235.py, tests/test_golden_chain_projection_savings.py): the state-coverage-gate pre-commit hook promotes pre-existing baselined declared-state gaps to a hard FAIL for any node directly touched by a diff (scripts/validation/state_coverage_baseline.txt — node_projection_delegation's onex.snapshot.projection.delegation.budget-state.v1 and node_projection_savings's two -applied.v1/savings-overview.v1 topics were pre-existing gaps unrelated to this change, verified via git stash — the gate passes on the unmodified tree). Added real, non-vacuous assertions (contract.yaml projection_api.exposures/terminal_event conformance checks, not gaming asserts) rather than bypassing the gate.

Verification

• Focused/targeted pytest runs (all green, not kafka marker): tests/unit/delegation/ (535 passed, 2 skipped), tests/unit/nodes/node_delegate_skill_orchestrator/ (99 passed), the full projection/delegation/golden-chain surface listed below (all green):
  test_golden_chain_projection_delegation.py, test_premium_counterfactual_omn13355.py, test_projection_delegation_tier_distribution_omn13662.py, test_projection_malformed_dlq_omn13548.py, test_savings_estimates_from_source_omn12494.py, test_escalation_metered_cost_reaches_projection_omn13535.py, test_budget_state_surface_omn13235.py, test_delegation_dto_omnibase_compat_imports.py, delegation_golden/test_layer1_contract_unit.py, integration/golden_chain/test_sea_acceptance_chain.py, integration/node_projection_delegation_inference_response/, test_actual_cost_recompute_omn13355.py, test_delegation_converge_one_schema_omn12811.py, test_generation_* (corpus/events proof/failed/semantic), test_golden_chain_projection_savings.py, test_traces_projection_omn13083.py, unit/cost/test_generation_cost_projection_omn13621.py, unit/delegation/test_canonical_failed_terminal_cost_projection_omn13408.py, test_ceiling_quality_gate_wiring_omn13335.py, test_context_pack_hash_plumb_omn13644.py, test_failed_terminal_cost_usd_omn13408.py, test_judge_verdict_events_omn13367.py, test_golden_chain_node_delegate_skill_orchestrator.py, test_roi_overlay_routing_omn14001.py.
• pre-commit run on all changed files — every hook passes, including the previously-failing state-coverage-gate (verified before/after with git stash).
• ruff format --check / ruff check / mypy (non-strict, matching CI) on all changed files — clean.
• Full local pytest tests/ (unconditional, dev-boundary style) in progress in background at push time; will report final tail if requested.
• Note on local verification methodology: omnibase-core resolves from PyPI in this repo's pyproject.toml (no [tool.uv.sources] local override — OMN-13878). To exercise the tenant_id field before the companion omnibase_core PR is merged/released, local test runs used pip install -e <local omnibase_core worktree> --no-deps into this venv (NOT committed to pyproject.toml/uv.lock). CI here will see the old pinned omnibase-core and MUST NOT be expected to go green until the companion PR merges + releases + the pin is repinned.

Dependency / sequencing

Depends on OmniNode-ai/omnibase_core#1404 merging + releasing + a follow-up repin omnibase-core X -> Y commit here (same pattern as recent PR history, e.g. fix(OMN-14138): repin omnibase-core 0.46.4 -> 0.46.5). Until then this PR's CI is expected to fail on the missing tenant_id field.

Non-goals / explicitly out of scope

• The durable per-tenant identity design (org/tenant model, auth-derived tenant resolution, RLS-aware writers) is OMN-14107, not this PR.
• node_delegate_skill_orchestrator's delegate-skill-terminal topic path and the plain savings-estimated.v1 path were scoped in/out as documented above — not a blanket tenant-id sweep across every omnimarket node.

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 (omnimarket half; see companion omnibase_core#1404 for the wire-DTO half).

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

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

Summary by CodeRabbit

• New Features

  • Added optional tenant identity support across delegation and savings flows, helping keep records correctly associated in multi-tenant setups.
  • Local delegation and workflow paths now carry tenant information through to terminal outputs and persisted projections.
  • New configuration support lets the tenant identifier be set via environment or app settings.

• Bug Fixes

  • Preserved tenant values during database updates instead of relying on default behavior.
  • Improved handling so existing tenant assignments are not overwritten when updating records.

• Tests

  • Added coverage for projection contract snapshots and savings contract declarations.

[coderabbitai]

Warning

Review limit reached

You’ve reached a temporary PR review limit under our Fair Usage Limits Policy.

Your recent review volume is higher than typical usage, so adaptive limits are currently applied.

Next review available in: 51 minutes

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 47787da0-e096-4831-9296-56a5f44cd95f

📥 Commits

Reviewing files that changed from the base of the PR and between 63a12b8 and 458eab7.

📒 Files selected for processing (1)

• tests/test_golden_chain_projection_savings.py

📝 Walkthrough

Walkthrough

Adds an ONEX_TENANT_ID configuration variable and onex_tenant_id Settings field, then threads a resolved tenant_id through delegation projection models, the delegation workflow handler, local dispatch port, and projection handlers for delegation/savings, persisting it conditionally to preserve DB defaults. New tests validate contract exposure.

Changes

Tenant ID propagation

Layer / File(s)	Summary
Tenant ID configuration .env.example, src/omnimarket/config/settings.py	Adds documented ONEX_TENANT_ID env var and onex_tenant_id Settings field defaulting to empty string.
Tenant ID fields in delegation projection models src/omnimarket/models/delegation/wire/model_delegate_skill_terminal_projection.py	Adds nullable tenant_id fields (with aliases) to terminal, event, savings, and task-delegated-source projection models, populated from event/canonical payload data.
Workflow handler tenant resolution and terminal emission src/omnimarket/nodes/node_delegation_orchestrator/handlers/handler_delegation_workflow.py	Pins tenant_id at request acceptance (request value or settings fallback), stores it on workflow state, and wires it into TerminalEmissionInputs for failure, quality-gate, and agent-lifecycle terminal paths.
Local dispatch port tenant stamping src/omnimarket/nodes/node_delegate_skill_orchestrator/ports/port_local_delegation_dispatch.py	Resolves resolved_tenant_id once from settings in dispatch and passes it to _project_evidence, stamping the terminal payload across failure/timeout/success paths.
Projection handlers conditional tenant_id persistence src/omnimarket/nodes/node_projection_delegation/handlers/handler_projection_delegation.py, src/omnimarket/nodes/node_projection_savings/handlers/handler_projection_savings.py	ModelProjectionTaskDelegatedEvent gains tenant_id; upserts into delegation_events, budget state, delegate-skill terminal rows, and savings_estimates conditionally include tenant_id to preserve DB defaults when absent.
Contract tests for tenant_id exposure tests/test_budget_state_surface_omn13235.py, tests/test_golden_chain_projection_savings.py	New tests assert contract.yaml exposes tenant_id column for budget-state snapshot and validate savings projection terminal event/table exposure.

Estimated code review effort: 3 (Moderate) | ~25 minutes

Sequence Diagram(s)

sequenceDiagram
  participant Request
  participant HandlerDelegationWorkflow
  participant Settings as get_settings
  participant LocalDispatchPort
  participant HandlerProjectionDelegation
  participant HandlerProjectionSavings

  Request->>HandlerDelegationWorkflow: delegation request accepted
  HandlerDelegationWorkflow->>Settings: fallback onex_tenant_id if request.tenant_id absent
  HandlerDelegationWorkflow->>HandlerDelegationWorkflow: store tenant_id on workflow state
  HandlerDelegationWorkflow->>HandlerProjectionDelegation: terminal event with tenant_id
  HandlerProjectionDelegation->>HandlerProjectionDelegation: conditionally stamp tenant_id in delegation_events/budget-state
  HandlerProjectionDelegation->>HandlerProjectionSavings: task-delegated payload with tenant_id
  HandlerProjectionSavings->>HandlerProjectionSavings: conditionally stamp tenant_id in savings_estimates

  LocalDispatchPort->>Settings: resolve onex_tenant_id
  LocalDispatchPort->>LocalDispatchPort: stamp tenant_id into evidence payload

Loading

Possibly related PRs

• OmniNode-ai/omnimarket#1455: Both PRs thread a pinned request-scoped value from acceptance through terminal emissions into downstream delegation_events in the same workflow handler and converter.
• OmniNode-ai/omnimarket#1580: Both PRs modify _project_evidence(...) in port_local_delegation_dispatch.py, overlapping in the same signature and evidence-projection code path.

🚥 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 summarizes the main change: threading ONEX_TENANT_ID through the delegation workflow and projection paths.

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch jonah/omn-14058-interim-tenant-id-stamp

---

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

[jonahgabriel]

Defect found (independent adversarial verification): missing tenant_id column on both target tables.

handler_projection_delegation.py / handler_projection_savings.py stamp row["tenant_id"] and rely on the comment claim that "the NOT NULL DEFAULT 'omninode' column applies cleanly on INSERT." That column does not exist in the omnibase_infra-owned schema for either table:

• delegation_events (migration 0007_delegation_events.sql, nodes/node_projection_delegation/) — no tenant_id column, no default.
• savings_estimates (migration 074_create_savings_estimates.sql, nodes/node_projection_savings/) — same.

git grep tenant_id -- '*.sql' in omnibase_infra finds the column only in 0019_delegation_budget_state.sql (a different table). There IS a tenant_id ... DEFAULT 'omninode' in omnidash/db/migrations/0001_tenant_rls.sql, but that migration targets omnidash_dev/omnidash_analytics on cloud dev-postgres via a manual, operator-run APPLY PLAN (omnidash/db/README.md) — it is not part of the omnibase_infra migration set that the local .201 runtime lanes (dev/stability-test/prod) apply, and it never touches the Postgres instance PostgresSyncProjectionAdapter/AsyncpgAdapter actually write to.

Failure mode: PostgresSyncProjectionAdapter.upsert() (omnimarket/src/omnimarket/projection/postgres_sync_database.py) builds its INSERT column list directly from row.keys(). The moment ONEX_TENANT_ID is set and a row carries a non-blank tenant_id, the generated INSERT INTO delegation_events (..., tenant_id) ... raises column "tenant_id" does not exist against real Postgres. The in-memory dict adapter used in unit tests has no schema to violate, so it masks this entirely.

A companion fix is in flight under OMN-14058: a forward migration adding tenant_id TEXT NOT NULL DEFAULT 'omninode' to both tables (source of truth in omnimarket/src/omnimarket/nodes/<node>/migrations/, vendored into omnibase_infra), plus a real-Postgres (non-in-memory) regression test. This PR should not merge/deploy until that migration lands — flagging here so it isn't picked up as ready.

[github-actions]

⚠️ Architectural Review — WARNINGS

Errors: 0
Warnings: 1

What this checks

Rule	Description
ARCH-TOPIC-001	No hardcoded Kafka topic strings in handler code
ARCH-DI-001	No event_bus=None bypass
ARCH-DI-002	No direct Handler instantiation outside workflow_runner/adapters
ARCH-DI-003	No reinvented DI containers
ARCH-TOPIC-002	contract.yaml topics follow `onex.{cmd

---

ARCH-DI-002: Direct Handler instantiation (warning)\n\n```\nsrc/omnimarket/nodes/node_delegate_skill_orchestrator/ports/port_local_delegation_dispatch.py:370: self._effect_handler = effect_handler or HandlerLlmDelegationCall()

src/omnimarket/nodes/node_delegate_skill_orchestrator/ports/port_local_delegation_dispatch.py:371: self._projection_handler = projection_handler or HandlerProjectionDelegation()
src/omnimarket/nodes/node_delegate_skill_orchestrator/ports/port_local_delegation_dispatch.py:391: self._judge = judge if judge is not None else HandlerJudgeAdequacy()
src/omnimarket/nodes/node_delegation_orchestrator/handlers/handler_delegation_workflow.py:493: loop = HandlerComplianceLoop()
src/omnimarket/nodes/node_delegation_orchestrator/handlers/handler_delegation_workflow.py:722: HandlerEscalationDecision()\n```\n\nHandler classes should be wired only in workflow_runner.py or adapter modules. Direct instantiation elsewhere bypasses the contract system.\n\n

Powered by node_pr_review_bot

[github-actions]

✅ PR Review Bot — CLEAN

Findings: 0
Run ID: ``

Note: Reviewer models are running as stubs (OMN-7969–OMN-7972 pending). Architectural findings from Phase 1 (arch-lint) are the authoritative signal until model handlers land.

---

Verdict semantics

Verdict	Meaning
clean	No blocking findings
risks_noted	Findings noted; threads passed verification
blocking_issue	MAJOR/CRITICAL findings not resolved — do not merge

Powered by node_pr_review_bot — FSM pipeline with multi-model review fan-out

[github-actions]

❓ Hostile Reviewer — UNKNOWN

Blocking findings (critical/error): 0
Total findings: 0
Models succeeded: none

---

Gate semantics (pilot phase)

Verdict	Meaning	Blocks merge?
passed	No critical/error findings	No
blocked	CRITICAL or ERROR findings found	Yes
degraded	All models unavailable (infra)	No (pilot)

Powered by omniintelligence.review_pairing.cli_review — node-based adversarial review via HandlerLlmCliSubprocess (OMN-8524)