Skip to content

feat(reborn): route caller-requested model on OpenAI-compatible API (Phase 2) - #5985

Merged
ilblackdragon merged 7 commits into
mainfrom
feat/reborn-responses-model-select
Jul 14, 2026
Merged

ilblackdragon merged 7 commits into
mainfrom
feat/reborn-responses-model-select

Conversation

@ilblackdragon

Copy link
Copy Markdown
Member

What

Makes the OpenAI-compatible Responses/Chat API's model field actually route the turn's LLM call, instead of being validated-and-echoed but ignored. Stacked on top of #5976 (per-run usage + cost); merge that first.

Semantics (as agreed): route if the provider can serve it, else silently fall back to the deployment's active model — no hard 400.

Why

Before this, a client's model was dropped at submit; every turn ran on the deployment's globally-active provider model. The routing seam (resolved_model_route → LlmProviderModelGateway) already threaded a per-run model to the gateway, but the gateway ignored it and product turns always resolved to provider.active_model_name().

How

requested_model is threaded from the request to the model call as an advisory route, honored by the gateway at the provider boundary:

  • ironclaw_product_adapters — UserMessagePayload gains an optional requested_model (wire-defaulted; with_requested_model filters empty). Surfaces that don't pick a model leave it None.
  • ironclaw_reborn_openai_compat — the chat + responses workflows set requested_model from request.model on the submitted payload (previously dropped).
  • ironclaw_product_workflow — AcceptedProductInboundTurn::submit carries it onto SubmitTurnRequest.requested_model (new field).
  • ironclaw_turns — the store's submit_turn records it as an advisory LoopModelRouteSnapshot on the new run (LoopModelRouteSnapshot::advisory(model): only model_id is meaningful, provider/config/auth are "requested" placeholders; is_advisory() distinguishes it; returns None for empty/invalid model strings → fallback). This reuses the existing resolved_model_route field that already flows run → loop context → gateway, so no new run-state field and no fixture churn beyond setting it.
  • ironclaw_runner —
    • LlmProviderModelGateway::request_model_override now prefers request.resolved_model_route.model_id over the profile default, falling back to the active model when absent. Providers that honor per-request overrides (e.g. NEAR AI) serve the requested model; providers that bake the model at construction ignore it and fall back — the "route-if-serveable-else-fallback" decision happens at the provider boundary.
    • attach_model_route_snapshot passes an advisory snapshot through unvalidated when no route resolver is wired (the default product runtime that serves the OpenAI-compat surface). Routed hosts (resolver present, fail-closed RoutedLlmProviderModelGateway) are unchanged and still validate — and never receive an advisory snapshot, since only the OpenAI-compat surface (on the default runtime) sets one.

Child/subagent runs and idempotent replays carry no requested model (fall back to the active model); the model is not persisted in the message store, so replays intentionally don't recover it.

Tests

  • ironclaw_runner (llm_gateway, recording-provider integration seam): the per-run requested model overrides the profile default on the captured CompletionRequest.model; absent a route it falls back to the profile default.
  • ironclaw_turns: submit_turn records an advisory route from requested_model (and none when absent); advisory() carries the model / marks itself advisory / trims and rejects empty or invalid model strings; an operator-resolved route is not advisory.
  • ironclaw_product_adapters: UserMessagePayload round-trips requested_model over the wire, omits it when None, and filters empty.

Known limitation

Per-request model routing only takes effect for providers that honor CompletionRequest.model (NEAR AI today); RigAdapter-backed providers (OpenAI/Anthropic/etc.) bake the model at construction and ignore the override, falling back to their configured model. The response still echoes the requested model. Enforcing "operator-configured only" with a strict catalog check (vs. the provider-boundary fallback here) would require wiring the model catalog/resolver into the default runtime — a follow-up.

🤖 Generated with Claude Code

ilblackdragon and others added 2 commits July 11, 2026 03:33
…ponses API

The OpenAI-compatible Responses (and Chat) surface hard-coded `usage: None`
and Reborn captured no per-run token totals anywhere — token counts existed
only per-LLM-call, were spent transiently on budget/stop heuristics, and were
never aggregated, persisted, or projected. Callers had no view of tokens or
cost.

This lands the shared per-run usage backbone and surfaces it as `usage` +
an IronClaw `cost` extension.

- ironclaw_turns: LoopModelUsage gains cache token fields (previously dropped
  at the gateway) + add_assign/total_tokens. New additive
  `model_usage: Option<LoopModelUsage>` on TurnRunState/TurnRunRecord/RunRecord
  rides the JSON-blob snapshot like resolved_model_route — no table, column,
  migration, or per-backend change. The dead usage_summary_ref/LoopUsageSummaryRef
  (never set/read; pointed at a store that never existed) is replaced by an
  inline model_usage value on LoopCompleted/LoopFailed, extracted in
  LoopExitApplier::apply and accumulated onto the run record at the terminal
  transition (block/resume legs sum).
- ironclaw_runner: reply paths preserve provider cache token counts.
- ironclaw_agent_loop: LoopExecutionState accumulates cumulative usage at both
  assistant-reply finalize paths; completed/failed exits carry it.
- ironclaw_reborn_openai_compat: OpenAiResponseUsage/OpenAiUsage gain
  input_tokens_details.cached_tokens (OpenAI-standard) + a namespaced cost
  object (input/cached-input/output/total USD, decimal strings). No new
  rust_decimal/ironclaw_llm dependency on the route crate.
- ironclaw_reborn_composition: the projection reader reads persisted model_usage
  via get_run_state, prices it through ironclaw_llm::costs (cache-read at the
  provider discount, unknown models fall back to default rate not zero), and
  fills usage. Cost gated behind root-llm-provider; tokens always reported.

Follow-up (Phase 2): route the requested model through the turn so model
selection actually takes effect (and cost prices the model that ran).

Tests: token-breakout, cost pricing incl. cache discount, and unknown-model
default-rate fallback; existing responses/chat/DTO/streaming contract suites
updated for the new optional usage fields.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…Phase 2)

Stacked on the usage/cost PR. Makes the OpenAI-compatible Responses/Chat
`model` field actually route the turn's LLM call instead of being
validated-and-echoed but ignored. Semantics: route if the provider can
serve it, else silently fall back to the deployment's active model.

The requested model is threaded from the request to the model call as an
advisory route, honored at the provider boundary:

- product_adapters: UserMessagePayload gains optional requested_model
  (wire-defaulted; with_requested_model filters empty).
- openai_compat: chat + responses workflows set requested_model from
  request.model on the submitted payload (previously dropped).
- product_workflow: AcceptedProductInboundTurn::submit carries it onto the
  new SubmitTurnRequest.requested_model.
- ironclaw_turns: submit_turn records it as an advisory LoopModelRouteSnapshot
  (LoopModelRouteSnapshot::advisory — only model_id meaningful, is_advisory()
  distinguishes it, None for empty/invalid). Reuses the existing
  resolved_model_route field that already flows run -> loop context -> gateway,
  so no new run-state field.
- ironclaw_runner: LlmProviderModelGateway::request_model_override prefers the
  request's route model_id over the profile default, falling back to the active
  model when absent (providers that honor per-request overrides serve it, others
  fall back). attach_model_route_snapshot passes an advisory snapshot through
  unvalidated when no route resolver is wired (default runtime); routed
  fail-closed hosts are unchanged.

Child/subagent runs and idempotent replays carry no requested model.

Known limitation: per-request routing only takes effect for providers that
honor CompletionRequest.model (NEAR AI); RigAdapter-backed providers ignore it
and fall back. Strict operator-configured-only validation would need the model
catalog wired into the default runtime (follow-up).

Tests: gateway honors requested route over profile default + falls back when
absent (recording-provider seam); submit records advisory route from
requested_model (and none when absent); advisory() construction/validation;
UserMessagePayload requested_model serde round-trip + empty filtering.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@gemini-code-assist

Copy link
Copy Markdown
Contributor

Warning

You have reached your daily quota limit. Please wait up to 24 hours and I will start processing your requests again!

@coderabbitai

coderabbitai Bot commented Jul 11, 2026 •

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: 3e42e880-92ee-4177-9312-92f0fa5fd432

📥 Commits

Reviewing files that changed from the base of the PR and between 2e36e17 and f5aff5d.

📒 Files selected for processing (3)
  • crates/ironclaw_product_adapters/src/inbound.rs
  • crates/ironclaw_reborn_openai_compat/src/chat_workflow.rs
  • crates/ironclaw_reborn_openai_compat/src/responses_workflow.rs

📝 Walkthrough

Summary by CodeRabbit

  • New Features

    • OpenAI-compatible requests can now include a preferred model hint, carried into run creation when provided.
    • Inbound user messages support an optional requested-model hint, with validation and automatic handling of empty/default values.
  • Bug Fixes

    • Model routing now honors per-request hints with correct precedence and validation.
    • Advisory model-route snapshots can pass through without a resolver, while operator routes remain fail-safe.
  • Tests

    • Updated and expanded tests to cover the new requested-model behavior and routing expectations.

Walkthrough

Adds an optional requested-model hint to inbound payloads, propagates it into turn state, represents it as an advisory route, and applies it during gateway model selection. Existing submission call sites explicitly set the field to None.

Changes

Requested model routing

Layer / File(s) Summary
Model hint and advisory route contracts
crates/ironclaw_product_adapters/src/inbound.rs, crates/ironclaw_turns/src/request.rs, crates/ironclaw_turns/src/run_profile/host.rs
Adds optional bounded payload support, serde handling, advisory route construction, and validation tests.
Inbound model hint propagation
crates/ironclaw_product_workflow/src/inbound_turn.rs, crates/ironclaw_reborn_openai_compat/src/*
Carries requested models from compatible inbound requests into SubmitTurnRequest; replay and WebUI paths use None.
Run route recording and gateway selection
crates/ironclaw_turns/src/memory/mod.rs, crates/ironclaw_runner/src/{loop_driver_host.rs,model_gateway.rs}, crates/ironclaw_runner/tests/*
Records advisory routes, preserves resolver-less advisory snapshots, and prioritizes per-request models over profile defaults.
Submit request call-site updates
crates/ironclaw_*/**, tests/integration/*, tools/ironclaw_stress/src/user_turn.rs
Initializes requested_model explicitly as None across production helpers, composition paths, stress tooling, and existing tests.

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

Sequence Diagram(s)

sequenceDiagram
  participant OpenAIClient
  participant UserMessagePayload
  participant TurnCoordinator
  participant RunState
  participant LlmProviderModelGateway
  OpenAIClient->>UserMessagePayload: provide model hint
  UserMessagePayload->>TurnCoordinator: submit requested_model
  TurnCoordinator->>RunState: record advisory model route
  RunState->>LlmProviderModelGateway: provide resolved route
  LlmProviderModelGateway->>LlmProviderModelGateway: select requested model or fallback
Loading

Possibly related issues

Possibly related PRs

  • nearai/ironclaw#4836: Modifies the same inbound submission construction site in crates/ironclaw_conversations/src/inbound.rs.

Suggested reviewers: henrypark133, think-in-universe

🚥 Pre-merge checks | ✅ 3 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Description check ⚠️ Warning It covers the feature well, but it misses most required template sections such as Change Type, Linked Issue, Validation, and the trust-boundary checklist. Rewrite it in the repo template and fill in Summary, Change Type, Linked Issue, Validation, Security Impact, Blast Radius, Rollback Plan, and Review Track.
✅ Passed checks (3 passed)
Check name Status Explanation
Title check ✅ Passed Conventional Commits style and accurately reflects advisory model-routing work across OpenAI-compatible request handling.
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.

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

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

@railway-app
railway-app Bot temporarily deployed to ironclaw-ci-preview / ironclaw-pr-5985 July 11, 2026 06:35 Destroyed
@github-actions github-actions Bot added size: L 200-499 changed lines risk: low Changes to docs, tests, or low-risk modules contributor: core 20+ merged PRs labels Jul 11, 2026
@railway-app

railway-app Bot commented Jul 11, 2026 •

Copy link
Copy Markdown

🚅 Deployed to the ironclaw-pr-5985 environment in ironclaw-ci-preview

Service Status Web Updated (UTC)
ironclaw ✅ Success (View Logs) Web Jul 14, 2026 at 5:57 am

ilblackdragon added a commit that referenced this pull request Jul 13, 2026
Addresses PR #5976 review findings (Copilot, ironloopai, CodeRabbit):

- Stop double-counting cache tokens in the OpenAI-compatible usage/cost
  projection. `cache_read_input_tokens` is a subset of `input_tokens`
  (per the type contract + how nearai/OpenAI populate it), so it is no
  longer added on top of the wire `input_tokens`, and the full-rate
  billable input is now `input_tokens - cache_read` (cache_read stays
  discounted). `cache_creation` remains additive. (Copilot, ironloopai)
- Accumulate model-response usage for EVERY model turn in the canonical
  executor, before branching on the output, so tool-using
  (`CapabilityCalls`) turns no longer drop their usage/cost. Removed the
  now-duplicate accumulation in `AssistantReplyStage`. (ironloopai)
- Clear `cumulative_model_usage` when `rebase_for_run` rebases onto a
  different run, so a retry does not re-report the failed run's tokens.
  Preserved for same-run gate resume. (CodeRabbit)
- Replace (not accumulate) run usage in the validated loop-exit
  transition: the loop reports its per-run cumulative at every exit, so a
  block→resume→complete sequence no longer double-counts pre-block legs.
  (CodeRabbit)
- Annotate the best-effort `.ok()?` DB/IO reads in `read_run_usage` with
  `// silent-ok:` comments. (CodeRabbit)

Tests: updated the usage/cost projection tests to the corrected cache
semantics and added non-zero `cache_creation` + Claude 10x cache-read
discount coverage; caller-level regression tests for the rebase reset,
the block→resume→complete cumulative usage, and capability-turn usage
accumulation. Also fixed two root integration-test `OpenAiResponseUsage`
literals missing the new `cost`/`input_tokens_details` fields (the CI
compile break).

Deferred: pricing by the resolved model route (ironloopai) lands in the
stacked model-routing PR #5985, where `resolved_model_route` is actually
populated and testable.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
ilblackdragon added a commit that referenced this pull request Jul 14, 2026
…ponses API (#5976)

* feat(reborn): per-run token usage + USD cost on OpenAI-compatible Responses API

The OpenAI-compatible Responses (and Chat) surface hard-coded `usage: None`
and Reborn captured no per-run token totals anywhere — token counts existed
only per-LLM-call, were spent transiently on budget/stop heuristics, and were
never aggregated, persisted, or projected. Callers had no view of tokens or
cost.

This lands the shared per-run usage backbone and surfaces it as `usage` +
an IronClaw `cost` extension.

- ironclaw_turns: LoopModelUsage gains cache token fields (previously dropped
  at the gateway) + add_assign/total_tokens. New additive
  `model_usage: Option<LoopModelUsage>` on TurnRunState/TurnRunRecord/RunRecord
  rides the JSON-blob snapshot like resolved_model_route — no table, column,
  migration, or per-backend change. The dead usage_summary_ref/LoopUsageSummaryRef
  (never set/read; pointed at a store that never existed) is replaced by an
  inline model_usage value on LoopCompleted/LoopFailed, extracted in
  LoopExitApplier::apply and accumulated onto the run record at the terminal
  transition (block/resume legs sum).
- ironclaw_runner: reply paths preserve provider cache token counts.
- ironclaw_agent_loop: LoopExecutionState accumulates cumulative usage at both
  assistant-reply finalize paths; completed/failed exits carry it.
- ironclaw_reborn_openai_compat: OpenAiResponseUsage/OpenAiUsage gain
  input_tokens_details.cached_tokens (OpenAI-standard) + a namespaced cost
  object (input/cached-input/output/total USD, decimal strings). No new
  rust_decimal/ironclaw_llm dependency on the route crate.
- ironclaw_reborn_composition: the projection reader reads persisted model_usage
  via get_run_state, prices it through ironclaw_llm::costs (cache-read at the
  provider discount, unknown models fall back to default rate not zero), and
  fills usage. Cost gated behind root-llm-provider; tokens always reported.

Follow-up (Phase 2): route the requested model through the turn so model
selection actually takes effect (and cost prices the model that ran).

Tests: token-breakout, cost pricing incl. cache discount, and unknown-model
default-rate fallback; existing responses/chat/DTO/streaming contract suites
updated for the new optional usage fields.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* fix(reborn): correct per-run usage/cost accounting from PR #5976 review

Addresses PR #5976 review findings (Copilot, ironloopai, CodeRabbit):

- Stop double-counting cache tokens in the OpenAI-compatible usage/cost
  projection. `cache_read_input_tokens` is a subset of `input_tokens`
  (per the type contract + how nearai/OpenAI populate it), so it is no
  longer added on top of the wire `input_tokens`, and the full-rate
  billable input is now `input_tokens - cache_read` (cache_read stays
  discounted). `cache_creation` remains additive. (Copilot, ironloopai)
- Accumulate model-response usage for EVERY model turn in the canonical
  executor, before branching on the output, so tool-using
  (`CapabilityCalls`) turns no longer drop their usage/cost. Removed the
  now-duplicate accumulation in `AssistantReplyStage`. (ironloopai)
- Clear `cumulative_model_usage` when `rebase_for_run` rebases onto a
  different run, so a retry does not re-report the failed run's tokens.
  Preserved for same-run gate resume. (CodeRabbit)
- Replace (not accumulate) run usage in the validated loop-exit
  transition: the loop reports its per-run cumulative at every exit, so a
  block→resume→complete sequence no longer double-counts pre-block legs.
  (CodeRabbit)
- Annotate the best-effort `.ok()?` DB/IO reads in `read_run_usage` with
  `// silent-ok:` comments. (CodeRabbit)

Tests: updated the usage/cost projection tests to the corrected cache
semantics and added non-zero `cache_creation` + Claude 10x cache-read
discount coverage; caller-level regression tests for the rebase reset,
the block→resume→complete cumulative usage, and capability-turn usage
accumulation. Also fixed two root integration-test `OpenAiResponseUsage`
literals missing the new `cost`/`input_tokens_details` fields (the CI
compile break).

Deferred: pricing by the resolved model route (ironloopai) lands in the
stacked model-routing PR #5985, where `resolved_model_route` is actually
populated and testable.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Base automatically changed from feat/reborn-responses-model-select-and-cost to main July 14, 2026 04:30
ilblackdragon and others added 3 commits July 14, 2026 04:44
…cate

The is_advisory() method had zero production consumers: the model gateway
reads snapshot.model_id uniformly regardless of advisory-ness, and
loop_driver_host gates on route-resolver presence rather than the advisory
flag. The predicate (and the operator_resolved_route_is_not_advisory test
that existed only to exercise it) dressed the three "requested" sentinel
placeholder components up as a first-class concept nothing acts on.

Keep advisory() — the actual reuse seam that stores a caller-requested
model hint in resolved_model_route — and have the remaining tests assert on
model_id, the only component that carries meaning.

Addresses thermo-nuclear code-quality review of PR #5985.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…-model-select

# Conflicts:
#	crates/ironclaw_agent_loop/src/executor/assistant_reply.rs
#	crates/ironclaw_reborn_composition/src/llm_admin/openai_compat_serve.rs
#	crates/ironclaw_reborn_composition/src/llm_admin/openai_compat_serve/tests.rs
#	crates/ironclaw_turns/src/memory/mod.rs
…host

Merging main surfaced a semantic conflict between two independently-correct
changes:

- #5985 made a resolver-less host (the default product runtime) pass a
  persisted model-route snapshot through unvalidated, so an OpenAI-compatible
  caller's requested model is honored by the non-routed gateway.
- main independently hardened the same host to FAIL CLOSED when a model-route
  snapshot is present but no resolver is wired (an operator route we cannot
  validate is a misconfiguration), with tests locking that behavior.

The auto-merge collapsed these into an unconditional pass-through, which broke
`text_only_host_factory_rejects_persisted_model_route_snapshot_without_resolver`.

Both intentions are correct and coexist by discriminating the snapshot kind —
exactly what LoopModelRouteSnapshot::is_advisory() expresses:

- advisory snapshot (caller-requested hint) + no resolver -> pass through
- operator route + no resolver -> fail closed ("resolver required")

This reverses the earlier "drop dead is_advisory()" commit on this branch: the
predicate looked unused in #5985 in isolation, but main's stricter guard makes
it load-bearing. is_advisory() and its unit tests are restored, the guard now
branches on it, and a caller-path regression test
(`text_only_host_factory_passes_advisory_model_route_snapshot_without_resolver`)
pins the pass-through side that main's reject test does not cover.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@railway-app
railway-app Bot temporarily deployed to ironclaw-ci-preview / ironclaw-pr-5985 July 14, 2026 05:19 Destroyed

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 1

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
crates/ironclaw_product_adapters/src/inbound.rs (1)

153-162: 🔒 Security & Privacy | 🔴 Critical | ⚡ Quick win

Ingress length bounds are bypassed during deserialization.

UserMessagePayload::new validates the payload while requested_model is still None. The deserialize implementation then attaches the untrusted wire.requested_model using with_requested_model, but never calls validate() again. As a result, the REQUESTED_MODEL_MAX_BYTES limit is completely bypassed for network ingress payloads.

As per repository invariants, you must validate and bound original ingress payloads before storage or dispatch.

Proposed fix
     fn deserialize<D>(deserializer: D) -> Result<Self, D::Error>
     where
         D: Deserializer<'de>,
     {
         let wire = UserMessagePayloadWire::deserialize(deserializer)?;
-        Self::new(wire.text, wire.attachments, wire.trigger)
-            .map(|payload| payload.with_requested_model(wire.requested_model))
-            .map_err(serde::de::Error::custom)
+        let payload = Self::new(wire.text, wire.attachments, wire.trigger)
+            .map(|payload| payload.with_requested_model(wire.requested_model))
+            .map_err(serde::de::Error::custom)?;
+        payload.validate().map_err(serde::de::Error::custom)?;
+        Ok(payload)
     }
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@crates/ironclaw_product_adapters/src/inbound.rs` around lines 153 - 162,
Update UserMessagePayload::deserialize so the deserialized wire fields,
including wire.requested_model, are validated against ingress bounds before the
payload is stored or dispatched. After applying with_requested_model, invoke the
payload validation path again and convert any validation failure through
serde::de::Error::custom, preserving UserMessagePayload::new validation for the
other fields.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@crates/ironclaw_reborn_openai_compat/src/chat_workflow.rs`:
- Around line 798-799: The payload builders in
crates/ironclaw_reborn_openai_compat/src/chat_workflow.rs lines 798-799 and
crates/ironclaw_reborn_openai_compat/src/responses_workflow.rs lines 1389-1390
must revalidate after applying with_requested_model: bind the completed
UserMessagePayload, call payload.validate()?, then return it. Preserve the
existing builder inputs and error propagation.

---

Outside diff comments:
In `@crates/ironclaw_product_adapters/src/inbound.rs`:
- Around line 153-162: Update UserMessagePayload::deserialize so the
deserialized wire fields, including wire.requested_model, are validated against
ingress bounds before the payload is stored or dispatched. After applying
with_requested_model, invoke the payload validation path again and convert any
validation failure through serde::de::Error::custom, preserving
UserMessagePayload::new validation for the other fields.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: c5c3a9e7-cc22-4ea9-a9aa-852a5f4be769

📥 Commits

Reviewing files that changed from the base of the PR and between 63a306d and 65d02c0.

📒 Files selected for processing (34)
  • crates/ironclaw_conversations/src/inbound.rs
  • crates/ironclaw_host_runtime/tests/support/host_runtime_harness.rs
  • crates/ironclaw_loop_host/tests/turn_event_publisher_contract.rs
  • crates/ironclaw_product_adapters/src/inbound.rs
  • crates/ironclaw_product_workflow/src/auth_continuation.rs
  • crates/ironclaw_product_workflow/src/inbound_turn.rs
  • crates/ironclaw_product_workflow/src/reborn_services.rs
  • crates/ironclaw_product_workflow/tests/product_workflow_contract.rs
  • crates/ironclaw_reborn_composition/src/factory.rs
  • crates/ironclaw_reborn_composition/src/factory/auth_tests.rs
  • crates/ironclaw_reborn_composition/src/runtime.rs
  • crates/ironclaw_reborn_composition/src/runtime/tests/auth_interaction.rs
  • crates/ironclaw_reborn_openai_compat/src/chat_workflow.rs
  • crates/ironclaw_reborn_openai_compat/src/responses_workflow.rs
  • crates/ironclaw_runner/src/loop_driver_host.rs
  • crates/ironclaw_runner/src/model_gateway.rs
  • crates/ironclaw_runner/src/subagent/await_edge/boot_recovery.rs
  • crates/ironclaw_runner/src/subagent/await_edge/resolver.rs
  • crates/ironclaw_runner/tests/concurrent_workers.rs
  • crates/ironclaw_runner/tests/llm_gateway.rs
  • crates/ironclaw_runner/tests/loop_driver_host.rs
  • crates/ironclaw_runner/tests/turn_scheduler_contract.rs
  • crates/ironclaw_turns/src/memory/mod.rs
  • crates/ironclaw_turns/src/request.rs
  • crates/ironclaw_turns/src/run_profile/host.rs
  • crates/ironclaw_turns/tests/active_run_ref_state_contract.rs
  • crates/ironclaw_turns/tests/agent_loop_host_contract.rs
  • crates/ironclaw_turns/tests/filesystem_turn_state_contract.rs
  • crates/ironclaw_turns/tests/per_inbound_type_concurrency_cap.rs
  • crates/ironclaw_turns/tests/per_user_concurrency_cap.rs
  • crates/ironclaw_turns/tests/retry_failed_turn_store_contract.rs
  • crates/ironclaw_turns/tests/turn_coordinator_contract.rs
  • tests/integration/subagent_await_edge.rs
  • tools/ironclaw_stress/src/user_turn.rs

Comment thread crates/ironclaw_reborn_openai_compat/src/chat_workflow.rs Outdated
The Responses/Chat OpenAI-compat surfaces forwarded request.model verbatim as
a per-run requested-model hint. But "default" is the server's alias for "use
the active model" (the models listing advertises it), not a concrete model id.
Forwarding it created an advisory route with model_id "default", which
request_model_override rejects as non-concrete (PolicyDenied) — failing every
run whose client sent model="default", including 6 legacy Responses API E2E
scenarios (status "failed" instead of "completed").

Map the wire model through model_validation::requested_model_hint before
forwarding: the "default" sentinel (and, defensively, empty) yields None so the
run falls back to normal resolution — the active model on the non-routed
gateway, the resolver's default route on routed hosts — while a concrete model
name is still forwarded. Keeps the model_gateway "default"-is-not-concrete
guard intact for genuine route/active-model misconfiguration.

Regression: model_validation unit tests for the sentinel/whitespace/concrete
cases; the legacy Responses API E2E suite exercises the full path.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@railway-app
railway-app Bot temporarily deployed to ironclaw-ci-preview / ironclaw-pr-5985 July 14, 2026 05:32 Destroyed

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@crates/ironclaw_reborn_openai_compat/src/chat_workflow.rs`:
- Around line 798-801: In
crates/ironclaw_reborn_openai_compat/src/chat_workflow.rs lines 798-801, bind
the UserMessagePayload builder result, call payload.validate()? after
with_requested_model, and return the validated payload. Apply the same change in
crates/ironclaw_reborn_openai_compat/src/responses_workflow.rs lines 1389-1392,
ensuring both ingress paths validate the model hint before dispatch.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: b2702106-aa47-4d0e-92e4-c5e625380a37

📥 Commits

Reviewing files that changed from the base of the PR and between 65d02c0 and 2e36e17.

📒 Files selected for processing (3)
  • crates/ironclaw_reborn_openai_compat/src/chat_workflow.rs
  • crates/ironclaw_reborn_openai_compat/src/model_validation.rs
  • crates/ironclaw_reborn_openai_compat/src/responses_workflow.rs

Comment thread crates/ironclaw_reborn_openai_compat/src/chat_workflow.rs
@github-actions

github-actions Bot commented Jul 14, 2026 •

Copy link
Copy Markdown
Contributor

Coverage ratchet

Ratchet mode: ENFORCING

RATCHET PASS: global
  observed: 85.54% (302060 / 353108 lines)
  floor:    85.3% (tolerance 0.5pp -> effective floor 84.8%)
  denominator: 353108 lines now vs 320188 at floor capture (+32920 lines, +10.28%) — material change (>5%)

⚠️ 2 Reborn crate(s) have 0 int-tier coverage (target: 0) — ironclaw_prompt_envelope, ironclaw_scripts

Reborn integration-tier coverage

Line coverage (Reborn crates): 85.54% — 302060 / 353108 lines

Per-crate breakdown (63 crates, lowest-covered first)
Crate Line % Covered / Total
ironclaw_prompt_envelope 0% 0 / 88
ironclaw_scripts 0% 0 / 345
ironclaw_runtime_policy 31.75% 80 / 252
ironclaw_event_projections 43.31% 673 / 1554
ironclaw_run_state 53.07% 225 / 424
ironclaw_authorization 53.89% 464 / 861
ironclaw_triggers 59.89% 1792 / 2992
ironclaw_observability 61.54% 16 / 26
ironclaw_webui_v2 62.93% 2679 / 4257
ironclaw_mcp 63.03% 578 / 917
ironclaw_reborn_cli 66.18% 4488 / 6781
ironclaw_filesystem 67.1% 3833 / 5712
ironclaw_dispatcher 67.15% 92 / 137
ironclaw_memory 69.2% 773 / 1117
ironclaw_reborn_migration 71.57% 1551 / 2167
ironclaw_trust 72.88% 661 / 907
ironclaw_capabilities 74.39% 1685 / 2265
ironclaw_wasm_limiter 74.6% 47 / 63
ironclaw_reborn_event_store 74.67% 958 / 1283
ironclaw_extractors 74.72% 538 / 720
ironclaw_llm 78.36% 20328 / 25941
ironclaw_product_context 78.57% 11 / 14
ironclaw_first_party_extensions 78.81% 5576 / 7075
ironclaw_process_sandbox 80.65% 671 / 832
ironclaw_wasm_product_adapters 80.71% 1448 / 1794
ironclaw_memory_native 81.22% 3205 / 3946
ironclaw_secrets 82.7% 2791 / 3375
ironclaw_events 82.86% 1765 / 2130
ironclaw_reborn_identity 83.59% 433 / 518
ironclaw_wasm 83.97% 1011 / 1204
ironclaw_auth 83.99% 3147 / 3747
ironclaw_reborn_config 84.06% 1814 / 2158
ironclaw_processes 84.44% 993 / 1176
ironclaw_common 84.85% 1490 / 1756
ironclaw_turns 84.99% 13669 / 16084
ironclaw_host_api 85.13% 2663 / 3128
ironclaw_product_workflow 85.57% 10845 / 12674
ironclaw_projects 85.92% 659 / 767
ironclaw_network 86.12% 670 / 778
ironclaw_threads 86.7% 4594 / 5299
ironclaw_slack_v2_adapter 86.79% 1806 / 2081
ironclaw_product_adapters 87.18% 3265 / 3745
ironclaw_skills 87.58% 4470 / 5104
ironclaw_hooks 87.75% 9917 / 11302
ironclaw_product_adapter_registry 88.06% 531 / 603
ironclaw_reborn_traces 88.19% 11946 / 13546
ironclaw_host_runtime 88.59% 17395 / 19635
ironclaw_reborn_composition 89.28% 80125 / 89747
ironclaw_runner 89.3% 16742 / 18749
ironclaw_extensions 89.38% 2971 / 3324
ironclaw_approvals 89.41% 1587 / 1775
ironclaw_reborn_openai_compat 89.55% 3798 / 4241
ironclaw_conversations 90.33% 3121 / 3455
ironclaw_event_streams 90.82% 1009 / 1111
ironclaw_loop_host 92.52% 14811 / 16008
ironclaw_resources 92.83% 4736 / 5102
ironclaw_attachments 93.06% 630 / 677
ironclaw_reborn_webui_ingress 93.19% 2217 / 2379
ironclaw_telegram_v2_adapter 93.62% 2511 / 2682
ironclaw_agent_loop 94.74% 8980 / 9479
ironclaw_safety 95.04% 3677 / 3869
ironclaw_first_party_extension_ports 95.24% 3343 / 3510
ironclaw_outbound 95.59% 3556 / 3720

This table itself is informational and never gates the PR on its own — not the percentage, not the per-crate holes, not the 0-coverage callout. A separate coverage ratchet (dry-run until enforce=true; see tests/integration/coverage-floor.toml) can fail the build on specific configured floors.

Exemptions (3 entry/entries excluded from the accounting above)
Module / Crate Reason Issue
crate: ironclaw_embeddings v1-only: consumed only by root ironclaw (src/app.rs, src/tools/builtin/memory.rs, src/workspace/mod.rs, src/config/{mod,embeddings}.rs); no crates/* dependents. Covered by "Tests (Legacy)". #5657
crate: ironclaw_gateway v1-only: consumed only by root ironclaw (src/channels/web/platform/static_files.rs, src/channels/web/handlers/frontend.rs); no crates/* dependents. Covered by "Tests (Legacy)". #5657
crate: ironclaw_tui v1-only: consumed only by root ironclaw (src/main.rs, src/channels/tui.rs); no crates/* dependents. Crate's own doc comment confirms it bridges INTO v1, not Reborn. Covered by "Tests (Legacy)". #5657

…uction

CodeRabbit review flagged that UserMessagePayload::with_requested_model attaches
the model hint AFTER new() ran validate() (with requested_model still None), so
the 256-byte REQUESTED_MODEL_MAX_BYTES bound was bypassed. The crate contract is
explicit: "Validated DTOs must validate both constructors and serde
deserialization."

- The custom Deserialize (untrusted wire path) now re-validates the assembled
  payload, so a wire-supplied requested_model is bounded like every other
  ingress field. This is the real bypass — an unbounded model string could
  otherwise deserialize and flow to persistence/dispatch.
- The Responses and Chat OpenAI-compat builder call sites now validate the
  assembled payload before submitting (defense in depth; request.model is
  already capped at parse by validate_model_name, but the type invariant must
  hold locally).

Regression: user_message_payload_bounds_requested_model_on_every_path asserts an
over-limit hint is rejected via both the builder+validate path and
deserialization, and that a hint at the cap is accepted.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@railway-app
railway-app Bot temporarily deployed to ironclaw-ci-preview / ironclaw-pr-5985 July 14, 2026 05:46 Destroyed
@ilblackdragon

Copy link
Copy Markdown
Member Author

@coderabbitai Addressed the ingress-validation finding in f5aff5d.

Critical (outside-diff) — inbound.rs UserMessagePayload deserialization bypasses length bounds: confirmed and fixed. The custom Deserialize attached requested_model after new() had validated the payload (with the field still None), so the 256-byte bound was skipped on the wire path. It now re-validates the fully-assembled payload before returning, matching the crate contract ("Validated DTOs must validate both constructors and serde deserialization").

Major (inline) — builder call sites: the Chat and Responses OpenAI-compat workflows now validate()? the assembled payload before submitting (defense in depth; request.model is already capped at parse by validate_model_name).

Regression: user_message_payload_bounds_requested_model_on_every_path pins rejection of an over-limit hint via both the builder+validate and deserialize paths, and acceptance at the cap.

@coderabbitai

coderabbitai Bot commented Jul 14, 2026

Copy link
Copy Markdown

Tip

For best results, initiate chat on the files or code changes.

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail
gh pr diff 5985 --repo nearai/ironclaw | sed -n '1,400p' | grep -n "requested_model\|with_requested_model\|fn deserialize\|validate(" -A3 -B3 crates/ironclaw_product_adapters/src/inbound.rs || true
echo "---direct file fetch---"
gh api repos/nearai/ironclaw/commits/f5aff5dc7 --jq '.files[].filename'

Length of output: 7654


Confirmed against f5aff5d. The Deserialize impl now calls payload.validate() after with_requested_model, so the wire path re-checks the 256-byte bound instead of relying on new()'s pre-attachment validation — the bypass is closed. Regression test user_message_payload_bounds_requested_model_on_every_path correctly exercises builder+validate, deserialize-rejection over cap, and at-cap acceptance. Also verified the Chat/Responses workflow files are in the same commit for the validate()? defense-in-depth call.

Nothing further to add — this matches the crate's validated-DTO contract.

(=^・ω・^=)


If you found this review helpful, would you consider giving us a shout-out on X?

Thank you for using CodeRabbit!

@ilblackdragon
ilblackdragon merged commit a373f29 into main Jul 14, 2026
66 checks passed
@ilblackdragon
ilblackdragon deleted the feat/reborn-responses-model-select branch July 14, 2026 06:10

This branch was successfully deployed

No deployments
ironclaw-ci-preview / ironclaw-pr-5985 — f5aff5dc Deployed Jul 14, 2026 by railway-app[bot]
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

contributor: core 20+ merged PRs risk: low Changes to docs, tests, or low-risk modules size: L 200-499 changed lines

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant