Skip to content

arch(ws-14): register planned driver default path - #3651

Merged
henrypark133 merged 1 commit into
reborn-integrationfrom
arch/ws-14
May 15, 2026
Merged

henrypark133 merged 1 commit into
reborn-integrationfrom
arch/ws-14

Conversation

@henrypark133

@henrypark133 henrypark133 commented May 14, 2026 •

Copy link
Copy Markdown
Collaborator

Context

Default planned-driver registration branch. It adds the registry/profile helpers that let Reborn select the planned default path while keeping live production cutover deferred.

Master spec: docs/reborn/agent-loop-skeleton.md
Workstream brief: docs/reborn/agent-loop-briefs/planned-driver-registration.md
Stack base: arch/ws-14-parent

Latest stack maintenance on 2026-05-14:

  • Rebased this branch onto its current stack base after the WS0 prompt-authority fix and the follow-up WS8/WS14-parent/WS16/WS17 conflict resolutions.
  • Pushed the updated branch with force-with-lease where the remote already existed, or published it as a new branch where it did not.
  • Verified the final ancestry chain from origin/reborn-integration through WS17 before publishing the PR descriptions.

What landed

  • Planned driver factory and registration helpers.
  • reborn-planned-default run-profile selection support.
  • Explicit planned-default resolver constructor while preserving existing text-only/default resolver behavior.
  • Planned-driver factory tests and E2E planned-driver coverage over the integrated host-port parent.
  • Documentation alignment so WS14 is registration/profile selection, not full product-live cutover.

Reviewer focus

  • The planned default selector should be explicit; existing defaults should not silently change.
  • Driver registration should compose through the registry/profile layer rather than hardcoding one-off paths.
  • The branch should not claim product-live readiness; WS16/WS17 own that evidence.

Non-goals / deferred work

  • Production runtime composition with all live-required services is WS16.
  • Product no-profile inbound cutover and persisted reply evidence is WS17.
  • Text-only rollback profile behavior should remain available.

Validation

  • cargo test -p ironclaw_reborn planned_driver_factory --features ironclaw_agent_loop/test-support
  • cargo test -p ironclaw_reborn --test planned_driver_e2e --features ironclaw_agent_loop/test-support
  • cargo test -p ironclaw_turns --test run_profile_contract
  • cargo check -p ironclaw_turns -p ironclaw_loop_support -p ironclaw_agent_loop -p ironclaw_reborn --features ironclaw_agent_loop/test-support
  • git diff --check

Stack position

[#3550 ws-0] state/checkpoint foundation -> reborn-integration
   |-- #3551 ws-1 strategy alpha -> ws-0
   |-- #3552 ws-2 strategy beta -> ws-0
   |-- #3553 ws-3 strategy gamma -> ws-0
   |-- #3643 ws-3.5 loop family registry -> ws-0
   '-- #3554 level1-merged -> ws-0
         |-- #3555 ws-4 planner facade -> level1
         |-- #3556 ws-5 default strategies -> level1
         '-- #3557 level2-merged -> level1
               '-- #3596 ws-6a canonical executor -> level2
                     '-- #3597 ws-7 PlannedDriver adapter -> ws-6a
                           '-- #3598 ws-8 integration/test support -> ws-7
                                 |-- #3644 ws-9 capability host wiring -> ws-8
                                 |-- #3645 ws-10 checkpoint load/resume -> ws-8
                                 |-- #3646 ws-11 input port -> ws-8
                                 |-- #3647 ws-12 progress port -> ws-8
                                 |-- #3648 ws-13 cancellation accessor -> ws-8
                                 |-- #3649 ws-15 prompt/identity context -> ws-8
                                 '-- #3650 ws-14-parent integrated host ports -> ws-8
                                       '-- #3651 ws-14 planned default registration -> ws-14-parent
                                             '-- #3652 ws-16 live runtime wiring -> ws-14
                                                   '-- #3653 ws-17 product live cutover -> ws-16

@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 May 14, 2026

@gemini-code-assist gemini-code-assist 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.

Code Review

This pull request introduces a factory for the planned driver, centralizing its configuration and registration. It adds a new planned_driver_factory module, updates PlannedDriver to support descriptor-based initialization, and enhances the InMemoryRunProfileResolver to allow for configurable implicit default profiles. New end-to-end tests and unit tests verify the registration and resolution logic. I have no feedback to provide.

@zmanian

zmanian commented May 14, 2026

Copy link
Copy Markdown
Collaborator

Review notes — WS14 planned default registration

Genuine opt-in for the planned path: default InMemoryRunProfileResolver still resolves to interactive_default → lightweight_loop; only new_with_implicit_default or default_planned_run_profile_resolver switches the implicit default. descriptor_for_family now requires a checkpoint schema (good — closes a previously implicit invariant). Registration is order-tolerant via typed DuplicateProfile error. One item before this lands.

Text-only registration tagged Production + all-optional

register_default_text_only_driver uses DriverRequirements::all_optional() but marks DriverKind::Production. The readiness matrix (Production + Production) passes silently. This is the same class of bug as the HostTrustPolicy::empty() regression (issue #3603): a "Production" tag that the readiness check trusts, but with placeholder semantics underneath.

Two options:

  1. Retag text-only as DriverKind::Reference so the Production readiness check requires explicit non-optional ports.
  2. Make per-port requirements explicit instead of all_optional().

Either is fine; the current shape promises Production-ready behavior the type doesn't actually require.

PLANNED_DRIVER_CHECKPOINT_SCHEMA_ID = CHECKPOINT_SCHEMA_ID aliasing

The const aliases the upstream constant. If the upstream schema id ever diverges, this duplication silently binds to the wrong value. A const_assert_eq! or single source of truth (just import and use directly) would be safer.

Minor

  • RunProfileResolutionError::ProfileUnavailable { profile_id } in the None-requested branch reports the resolved implicit profile id, not the original request — minor observability degradation when an operator misconfigures the implicit default.
  • planned_default_profile_definition mints CapabilitySurfaceProfileId::new("interactive_tools") inline; hoist to a const next to other ids.
  • Many helpers return Result<_, String>; a typed error would match the rest of the module.

The planned default itself is correctly marked Production because its ports are all Required — that discipline is the right pattern; just please extend it to text-only.

@serrrfirat

Copy link
Copy Markdown
Collaborator

Summary

Reviewed WS14 PR #3651 only.
Base f46ef664c73fdd2f3982d11db6fcf0492da898f5 → head 8d39d6d0a5f23af947677cd137b621854d002638.

No unintended cutover found, but actual default planned-driver registration is not wired into production/default coordinator path.

Validation:

  • cargo check -p ironclaw_reborn ✅

Findings

# Sev Category File:Line Issue Fix suggestion
1 High Correctness / Composition root crates/ironclaw_reborn/src/planned_driver_factory.rs:181-190, crates/ironclaw_turns/src/coordinator.rs:94-99, crates/ironclaw_reborn/tests/planned_driver_e2e.rs:57-88 PR adds helper default_planned_run_profile_resolver(), but production/default coordinator still constructs InMemoryRunProfileResolver::default(). Tests call the helper directly, not real bootstrap/submit path. Normal requested_run_profile=None / default path still resolves legacy default at runtime. Wire helper into production composition root behind intended gate, or retitle/rescope as groundwork. Add caller-boundary test through actual submit/default coordinator path proving None and default resolve to planned default.

Security/data-flow notes

  • No auth/secret regression found.
  • Safe from accidental cutover today because helper is not wired.

Correctness/invariant notes

  • PR title/intent says planned default registration; executable default path does not change.

Missing tests

  • Actual bootstrap/composition-root/default-submit path uses planned default resolver.
  • Legacy aliases audited (interactive_default, default, reborn-planned-default).

Squash of #3651 (1 commit) onto reborn-integration.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@henrypark133
henrypark133 changed the base branch from arch/ws-14-parent to reborn-integration May 15, 2026 20:36
@henrypark133
henrypark133 marked this pull request as ready for review May 15, 2026 22:06
@henrypark133
henrypark133 merged commit 7647945 into reborn-integration May 15, 2026
2 checks passed
@henrypark133
henrypark133 deleted the arch/ws-14 branch May 15, 2026 22:07
theredspoon pushed a commit to theredspoon/ironclaw that referenced this pull request Jun 21, 2026
Squash of nearai#3651 (1 commit) onto reborn-integration.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
theredspoon pushed a commit to theredspoon/ironclaw that referenced this pull request Jun 21, 2026
Squash of nearai#3651 (1 commit) onto reborn-integration.

Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
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.

3 participants