Repository navigation
arch(ws-17): prove product live planned-runtime cutover - #3653
Conversation
There was a problem hiding this comment.
Code Review
This pull request implements the 'Product-Live Readiness Evidence' workstream (WS-17), focusing on ensuring the Reborn agent loop can be safely composed with the product path. Key changes include the introduction of a RunProfileResolver in the host runtime wiring, the addition of a ProductLiveCancellationProbe to verify that cancellation factories are externally controllable, and the implementation of build_product_live_planned_runtime with fail-closed readiness checks for critical adapters. Additionally, ThreadCheckpointLoopExitEvidencePort was updated to support scoped verification and product-path cancellation observation. Extensive contract tests were added to validate that no-profile turns correctly exercise the planned runtime and support cancellation. Documentation and CLI tools were also updated to reflect these readiness snapshots. I have no feedback to provide as there were no review comments to assess.
Review notes — WS17 product live cutover evidenceTo answer the obvious question first: this is not the flip. Reading this as a "live cutover" PR would be wrong; reading it as "everything that needs to be true before the cutover" is correct. Fail-closed builder is the right shape
This closes the gate that WS16 left open (issue #3602). The remaining work is purely calling this from a product entry point. Caller-level tests are the cutover evidenceThree tests in The new owner-scoping fix on Items to track
Minor
Rollback posture is solid: text-only path preserved, no schema/state changes, reverting the cutover is "stop calling the new builder at the composition root." |
SummaryReviewed WS17 PR #3653 only. Readiness evidence improved, but current tests still do not prove production default-path live cutover. Validation:
Findings
Security/data-flow notes
Correctness/invariant notes
Missing tests
|
Squash of #3653 (8 commits) onto reborn-integration stack. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Deferred from WS-16 review (PR #3652)The WS-16 review surfaced production-readiness items that were explicitly out of scope for WS-16 and tagged for this PR.
Suggested follow-up: extend |
Squash of #3653 (8 commits) onto reborn-integration stack. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…osed gates) Resolve reviewer feedback on #3653: - zmanian: add `debug!` on the default `is_product_cancellation_observed` Ok(false) so on-call has a breadcrumb when a factory is not product-live. - zmanian: replace remaining `.unwrap()` with typed `.expect("planned default profile resolver")` in WS-14/WS-16/WS-17 reborn tests. - serrrfirat #1: remove the manual `request_product_cancellation` backdoor from the product-live cancellation proof. Wire a `CompositeTurnRunWakeNotifier` in `build_default_planned_runtime` so `coordinator.cancel_run` fans out to both the worker wake channel and `RunCancellationFactory::notify_run_wake`. The cancellation contract test now drives observation purely from `cancel_run` and polls until the retained run handle flips. - henrypark133 #1-3: extend the product-live readiness gate with fail-closed checks for `ModelPolicyGuard`, `ModelBudgetAccountant`, and `SafetyContext`. Adds matching `DefaultPlannedRuntimeParts` fields, three new `ProductLiveRuntimeReadinessComponent` variants, builder wiring on `RebornLoopDriverHostFactory`, and three new regression tests asserting each missing component is rejected. Item #4 (`production_readiness` gate invocation from a startup entry point) and serrrfirat #2 (tool-use canary) remain deferred per the PR description — both are part of the composition-root flip, which zmanian's review tagged for a separate PR. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
… test helpers)
- runtime: re-type ProductLiveRuntimeBuildError::Probe.reason
String -> source: AgentLoopHostError. Strongly-typed per
.claude/rules/types.md, carries kind + diagnostic_ref, and Error::source
now returns the underlying probe failure for chain inspectors.
- cancellation_port: doc-comment RunCancellationFactory::notify_run_wake
with the sync/non-blocking contract that CompositeTurnRunWakeNotifier
relies on; doc-comment ProductLiveCancellationProbe with the
ephemeral-handle contract.
- inbound_turn_contract / loop_driver_host tests: make the test
ReadyRunCancellationProbe own its RunCancellationHandle directly
(was leaking one entry into the factory handles map on every
readiness verify); add local turn_state_store_dyn() and
test_safety_context() helpers and route the duplicated cast +
InstructionSafetyContext::new("policy:test", ...) call sites
through them.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b26455e8d5
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| let Some(run_profile_resolver) = self.run_profile_resolver.as_ref() else { | ||
| return Err(production_wiring_report( | ||
| ProductionWiringComponent::RunProfileResolver, | ||
| ProductionWiringIssueKind::Missing, | ||
| None, | ||
| )); |
There was a problem hiding this comment.
Wire a production run-profile resolver before coordinator build
turn_coordinator_for_production now hard-fails when run_profile_resolver is unset, but the production service assembly paths still never set one (crates/ironclaw_reborn_composition/src/factory.rs libsql path at lines 265-287 and postgres path at lines 327-349 only call .with_turn_run_wake_notifier(...) before turn_coordinator_for_production()). In production/migration profiles this now returns a missing-component wiring report and prevents coordinator construction, so bootstrapping the Reborn production facades fails even with valid DB/runtime wiring.
Useful? React with 👍 / 👎.
* arch(ws-17): prove product live planned-runtime cutover Squash of nearai#3653 (8 commits) onto reborn-integration stack. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * arch(ws-17): address PR review (debug log, cancel-path proof, fail-closed gates) Resolve reviewer feedback on nearai#3653: - zmanian: add `debug!` on the default `is_product_cancellation_observed` Ok(false) so on-call has a breadcrumb when a factory is not product-live. - zmanian: replace remaining `.unwrap()` with typed `.expect("planned default profile resolver")` in WS-14/WS-16/WS-17 reborn tests. - serrrfirat #1: remove the manual `request_product_cancellation` backdoor from the product-live cancellation proof. Wire a `CompositeTurnRunWakeNotifier` in `build_default_planned_runtime` so `coordinator.cancel_run` fans out to both the worker wake channel and `RunCancellationFactory::notify_run_wake`. The cancellation contract test now drives observation purely from `cancel_run` and polls until the retained run handle flips. - henrypark133 #1-3: extend the product-live readiness gate with fail-closed checks for `ModelPolicyGuard`, `ModelBudgetAccountant`, and `SafetyContext`. Adds matching `DefaultPlannedRuntimeParts` fields, three new `ProductLiveRuntimeReadinessComponent` variants, builder wiring on `RebornLoopDriverHostFactory`, and three new regression tests asserting each missing component is rejected. Item #4 (`production_readiness` gate invocation from a startup entry point) and serrrfirat #2 (tool-use canary) remain deferred per the PR description — both are part of the composition-root flip, which zmanian's review tagged for a separate PR. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * arch(ws-17): tidy review nits (typed probe error, probe lifetime doc, test helpers) - runtime: re-type ProductLiveRuntimeBuildError::Probe.reason String -> source: AgentLoopHostError. Strongly-typed per .claude/rules/types.md, carries kind + diagnostic_ref, and Error::source now returns the underlying probe failure for chain inspectors. - cancellation_port: doc-comment RunCancellationFactory::notify_run_wake with the sync/non-blocking contract that CompositeTurnRunWakeNotifier relies on; doc-comment ProductLiveCancellationProbe with the ephemeral-handle contract. - inbound_turn_contract / loop_driver_host tests: make the test ReadyRunCancellationProbe own its RunCancellationHandle directly (was leaking one entry into the factory handles map on every readiness verify); add local turn_state_store_dyn() and test_safety_context() helpers and route the duplicated cast + InstructionSafetyContext::new("policy:test", ...) call sites through them. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
Context
Product live cutover/readiness branch. It proves the product inbound path can select the planned runtime for a normal no-profile user message and persist the final assistant reply in product-visible thread history.
Master spec:
docs/reborn/agent-loop-skeleton.mdWorkstream brief:
docs/reborn/agent-loop-briefs/product-live-cutover.mdStack base:
arch/ws-16Latest stack maintenance on 2026-05-14:
origin/reborn-integrationthrough WS17 before publishing the PR descriptions.What landed
RunProfileResolver; it no longer silently relies onDefaultTurnCoordinator's text-only resolver.reborn-planned-defaultwith the planned resolver.DefaultInboundTurnService, runs the Reborn worker, and verifies the final assistant reply is persisted.ThreadCheckpointLoopExitEvidencePortcan verify completion evidence against the configured product/userThreadScope, fixing owner-scoped thread verification.Arc<C>implementsTurnCoordinatorfor shared live coordinators.build_product_live_planned_runtime(...)fails closed unless model-route resolver, input queue, cancellation factory, and identity context source are present.Reviewer focus
Non-goals / deferred work
Validation
git diff --checkcargo fmt --checkcargo test -p ironclaw_reborn --test loop_driver_host product_live_runtimecargo test -p ironclaw_product_workflow --test inbound_turn_contract user_message_no_profile_can_cancel_product_live_run_from_product_pathcargo test -p ironclaw_reborn loop_exit_applier::tests::cancelled_exit_requires_observed_cancel_inputcargo test -p ironclaw_reborn loop_exit_applier::tests::observed_host_cancellation_still_requires_final_checkpoint_when_configuredcargo test -p ironclaw_host_runtime --test host_runtime_services_contract production_wiring_validation_rejects_noop_turn_wake_notifiercargo test -p ironclaw_reborn_cli --test smoke run_reports_runtime_readiness_snapshot_without_touching_v1_stateStack position