Repository navigation
docs(reborn): propose architecture simplification — fewer DTOs, less dyn, no local-specific structs - #6175
Merged
Conversation
…dyn, no local-specific structs Design note proposing a fundamental simplification of the Reborn host/runtime internals, grounded in a code audit and a cross-reference against the last ~30 days of PRs/issues. Thesis: DTO proliferation (~14 mirror structs per capability call), dyn proliferation (~6 hot-path trait objects, most single-impl), and local-specific store structs are three symptoms of one decision — treating every crate boundary as a trust boundary when Reborn has exactly one (loop <-> host). Proposes: one canonical payload type in ironclaw_host_api (Invocation/Authority/ Outcome), authority as a single fold, a closed RuntimeLane enum instead of dyn RuntimeAdapter, backend-generic stores (RowBackend) to delete the InMemory*/ Filesystem* tree, and DeploymentConfig-as-data instead of composition-mode struct families. Preserves all security invariants; incremental migration with a first-party-lane proof-of-concept slice. Complements #6168. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Contributor
🔎 IronLoop Review StatusHead: Current reviewers:
Reviewer summaries
Recent activity
Available commands
Run metadataAdmission: webhook accepted the request and IronLoop persisted reviewer state before this projection. |
railway-app
Bot
temporarily deployed
to
ironclaw-ci-preview / ironclaw-pr-6175
July 17, 2026 04:33
Destroyed
github-actions
Bot
added
scope: docs
📝 WalkthroughSummary by CodeRabbit
WalkthroughThe PR adds a design proposal for consolidating capability execution, authorization, storage, deployment configuration, and trust boundaries, plus repository rules requiring fail-closed tenant process isolation. ChangesArchitecture simplification
Estimated code review effort: 2 (Simple) | ~10 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
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. Comment |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Code Review
This pull request introduces a design proposal to simplify the Reborn host/runtime architecture by reducing DTO proliferation, minimizing dynamic dispatch (dyn), and consolidating local-specific storage structs into backend-generic stores. The reviewer feedback focuses on architectural refinements: clarifying dynamic backend selection for TurnStore<B>, addressing the complexities of abstracting database transactions under a single RowBackend trait (recommending a Repository or Unit of Work pattern instead), and preserving sealed construction guarantees for the Authority struct to prevent security risks at the trust boundary.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
Comment on lines
+212
to
+217
| - Make each durable store generic over a single storage backend: | ||
|
|
||
| ```rust | ||
| struct TurnStore<B: RowBackend> { backend: B } // domain logic written ONCE | ||
| enum Backend { Memory(MemBackend), Sqlite(..), Postgres(..) } // 3 backends shared by ALL domains | ||
| ``` |
Contributor
There was a problem hiding this comment.
Using a compile-time generic parameter B: RowBackend for TurnStore is excellent for static dispatch, but it requires deciding how dynamic backend selection (e.g., based on DeploymentConfig at runtime) will be handled.
If TurnStore is statically typed as TurnStore<B>, then the code assembling the runtime must either:
- Monomorphize the entire runtime pipeline for each backend (leading to code bloat and complex composition wiring), or
- Implement
RowBackendfor theBackendenum itself (i.e.,impl RowBackend for Backend), allowingTurnStore<Backend>to act as a single concrete type that dispatches dynamically internally.
Recommendation:
Clarify in the design note that enum Backend will implement RowBackend to enable runtime dynamic dispatch. Ensure that the documentation precisely reflects the current implementation of these traits and methods, and explicitly mark any features that are not yet implemented or are renamed as 'planned' to maintain accuracy.
References
- Ensure documentation precisely reflects the current implementation of traits and methods; remove or explicitly mark as 'planned' any references to features that were renamed or not implemented during development.
Comment on lines
+299
to
+302
| - **`RowBackend` is the hard part.** In-memory vs durable stores differ in | ||
| locking and overlay semantics (e.g. the runner-lease in-memory overlay in | ||
| `ironclaw_turns`), so the backend trait must model transactions and locks, not | ||
| just get/put. If that abstraction gets leaky, the collapse is not worth it. |
Contributor
There was a problem hiding this comment.
Abstracting database transactions and locking mechanisms under a single RowBackend trait is notoriously difficult because of the fundamental differences in concurrency models between in-memory maps, SQLite (which locks at the database level), and PostgreSQL (which supports fine-grained row-level locking like SELECT ... FOR UPDATE).
If the RowBackend trait exposes low-level transaction or locking primitives, SQL-specific details or in-memory synchronization details will inevitably leak into the domain logic, defeating the purpose of the abstraction.
Recommendation:
Instead of a low-level RowBackend that abstracts rows, consider a domain-specific Repository or Unit of Work pattern where the transaction boundaries and locking strategies are implemented entirely within the concrete database-specific repository implementations. This keeps the domain logic completely decoupled from the underlying storage engine's concurrency and transaction semantics.
Comment on lines
+317
to
+320
| 2. Does `Authority` accrete as one value, or should trust/approval/reservation | ||
| remain separately-typed witnesses passed as a tuple to preserve sealed | ||
| construction guarantees (see the trust-boundary stack note, | ||
| `docs/reborn/2026-05-11-trust-boundary-stack-note.md`)? |
Contributor
There was a problem hiding this comment.
Regarding the second open question, accreting trust, approvals, and reservations into a single public Authority struct poses a significant risk to the sealed construction guarantees of the trust boundary. If any host code can construct or modify an Authority struct, it becomes much harder to statically guarantee that a dispatch request was actually authorized by the correct policy engines.
Recommendation:
If exposing these full internal types in public interfaces is temporarily required for development velocity, explicitly document this as a temporary state (e.g., using a 'NOTE' block) and plan for narrowed projections in future iterations to maintain clean crate boundaries. Otherwise, to preserve sealed construction:
- Keep the individual witness types (e.g.,
TrustClass,ApprovalLease,Reservation) as private fields withinAuthorityor use a sealed trait pattern. - Ensure that
Authoritycan only be constructed by a centralizedauthorizefunction. - Alternatively, pass them as a tuple of strongly-typed, non-forgeable witness tokens.
References
- When exposing full internal types in public interfaces for development velocity, explicitly document this as a temporary state (e.g., using a 'NOTE' block) and plan for narrowed projections in future iterations to maintain clean crate boundaries.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 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 `@docs/reborn/2026-07-17-architecture-simplification-dto-dyn-local.md`:
- Around line 30-54: Add reproducible evidence to the architecture proposal for
the “~14 DTOs” and “~8 request shapes/~6 result shapes/6 crate boundaries/6+ dyn
seams” claims, including exact file:line references or symbols for definitions
and conversion sites. Document the search boundary used to derive the count and
cite the dated query or commit range supporting the “last ~30 days” audit.
Extend the request-type table with these citations while preserving its existing
entries and conclusions.
- Around line 156-166: Make Authority an unforgeable authorization witness: hide
its fields and expose construction only through host-kernel authorization,
following the LoopExitValidationPolicy private-field and host-only constructor
pattern. Update authorize and dispatch so callers cannot construct or retain an
Authority without passing authorize, or restrict dispatch to the authorization
kernel while preserving the existing invocation and outcome flow.
- Around line 210-224: Update the backend migration plan around Backend and the
Filesystem* stores to preserve filesystem persistence compatibility before
removal. Either add a filesystem backend to the shared RowBackend design or
explicitly define deprecation, data migration, rollback, and compatibility-test
requirements; ensure the no-behavior-change objective remains supported.
- Around line 165-180: The architecture document’s pseudocode and prose use
inconsistent result contracts for authorize and dispatch. Update the authorize
and dispatch definitions to use one exact Result<Outcome, Blocked> seam,
explicitly mapping blocked authorization, recoverable runtime failures, and
loop-terminal errors into the appropriate result variants without retaining
ambiguous Ok(Failed) versus Err(terminate) behavior.
- Around line 158-165: The authorize function currently accepts a separate scope
that can diverge from Invocation.scope. Update authorize and its callers to
derive authorization from the invocation’s scope, or use a single typed scope
witness shared by both, while preserving the existing policy and Blocked
behavior.
🪄 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: 08d2e475-cd5e-40fa-9e18-a9be046a1508
📒 Files selected for processing (1)
docs/reborn/2026-07-17-architecture-simplification-dto-dyn-local.md
![ironloopai[bot]](https://avatars.githubusercontent.com/in/4116490?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
❌ IronLoop Review: reviewer
Review at a glance
| Verdict | Blocking | Notes | Inline | Head |
|---|---|---|---|---|
| ❌ Changes requested | 4 | 2 | 6 | 9c9670ba8422 |
Head: 9c9670ba8422f1a0f3b7bbf3aac0281f2286f90b
Next: Fix the blocking findings, push the PR branch, then re-run this reviewer.
Run details
Status: Current
Needs human: no
Needs validation: no
Summary
The docs-only change is reviewable, but its central simplification model conflicts with current security and storage contracts. It would expose raw capability input across a refs-only membrane, replace sealed authority witnesses with constructible vocabulary, and erase required post-dispatch settlement and terminal-error paths. The storage and trait inventories also need correction.
Findings
Blocking: 4 / Notes: 2
Blocking findings
1. ❌ [HIGH] Keep raw input behind the ref-only loop membrane
Location: docs/reborn/2026-07-17-architecture-simplification-dto-dyn-local.md:158
Invocation carries raw Json, but the current loop-facing CapabilityInvocation carries a CapabilityInputRef plus activity, surface-version, and typed approval/auth-resume evidence. The proposal says the new type replaces that DTO while later claiming the membrane remains refs-only. If this type crosses LoopCapabilityPort, it bypasses the host-owned input registration/resolution boundary; if it is created only after that boundary, it cannot replace the loop DTO. Retain a distinct loop-facing ref request or make the canonical type preserve the validated refs and resume identities.
2. ❌ [HIGH] Resolve sealed authority witnesses before defining Authority
Location: docs/reborn/2026-07-17-architecture-simplification-dto-dyn-local.md:159-160
TrustClass is neutral vocabulary whose privileged variants are constructible in-process; authorization consumes the sealed ironclaw_trust::EffectiveTrustClass/TrustDecision instead. Approval leases are owned by authorization, outside leaf-level host_api. Defining this cross-crate aggregate in host_api therefore either creates forbidden upward dependencies or demotes host-minted proofs to forgeable field data. Resolve the witness question before the proposed first migration slice and use separately sealed witnesses or a host-private opaque authority object.
3. ❌ [HIGH] Preserve terminal errors and post-dispatch settlement
Location: docs/reborn/2026-07-17-architecture-simplification-dto-dyn-local.md:165-180
The proposed Result<Outcome, Blocked> has no channel for host/storage/runtime failures and models authority only before dispatch. Current obligations require prepare/complete/abort phases around dispatch; reservations must reconcile or release exactly once; approval leases transition through claim/dispatch/consume or abort; run-state failures must be persisted. Collapsing these into pre-dispatch Authority can omit cleanup or turn terminal infrastructure faults into recoverable outcomes. Specify an asynchronous multi-phase lifecycle with an explicit terminal-error channel and post-dispatch settlement before claiming these stages can collapse.
4. ❌ [HIGH] Account for untrusted runtime and egress boundaries
Location: docs/reborn/2026-07-17-architecture-simplification-dto-dyn-local.md:19-24
The loop is not the only untrusted boundary. WASM guests, scripts/containers, MCP/plugin input, runtime-provided HTTP data, and external services remain untrusted and are mediated by filesystem, network, secret, sandbox, and redaction boundaries. Revise the thesis to distinguish ordinary Rust crate seams from these security boundaries and enumerate which mediation points remain; otherwise the later claim that all security invariants survive is unsupported.
Non-blocking notes (2)
1. 💬 [MEDIUM] Use the existing RootFilesystem storage fabric
Location: docs/reborn/2026-07-17-architecture-simplification-dto-dyn-local.md:212-224
The accepted 2026-05-14-universal-fs-dispatch.md ADR already makes RootFilesystem the single Memory/libSQL/Postgres backend abstraction and explicitly forbids a parallel backend trait. Domain stores take ScopedFilesystem; backend selection occurs below them. Introducing RowBackend recreates the dispatch layer that ADR removed, while the referenced 2026-04-25 storage note is marked superseded. Express the proposed domain-logic consolidation over ScopedFilesystem/RootFilesystem and reference the accepted ADR.
2. 💬 [MEDIUM] Correct the trait-object inventory
Location: docs/reborn/2026-07-17-architecture-simplification-dto-dyn-local.md:204-206
CapabilityHost is already a concrete generic CapabilityHost<'a, D>, not a trait. CapabilityDispatcher is the neutral lower-level port that prevents ironclaw_capabilities from depending on the concrete dispatcher, and RuntimeAdapterRequest<F, G> is generic over filesystem/governor services rather than closures. Re-count actual trait objects separately from generic ports and explain how dependency inversion and test substitution remain if a port is removed.
Developer follow-up
After fixing this feedback:
- Push the fix to this PR branch.
- Re-run this reviewer with
@ironloopai review --agent reviewerif you only changed this reviewer's findings. - Re-run all reviewers with
@ironloopai reviewwhen the fix may affect multiple areas.
|
|
||
| ```rust | ||
| // ── ironclaw_host_api (the bottom crate everyone already depends on) ── | ||
| struct Invocation { capability: CapabilityId, input: Json, scope: Scope, estimate: Estimate } // the ONE payload |
Contributor
There was a problem hiding this comment.
This raw Json cannot replace the loop-facing CapabilityInvocation while preserving the refs-only membrane. The current type carries an input ref plus activity/surface/resume evidence. Keep a separate loop request or preserve those validated references in the canonical type.
| ```rust | ||
| // ── ironclaw_host_api (the bottom crate everyone already depends on) ── | ||
| struct Invocation { capability: CapabilityId, input: Json, scope: Scope, estimate: Estimate } // the ONE payload | ||
| struct Authority { trust: TrustClass, approval: Option<ApprovalLease>, |
Contributor
There was a problem hiding this comment.
A raw TrustClass is not a policy-validated witness; privileged variants are sealed by EffectiveTrustClass/TrustDecision, and approval leases live outside leaf-level host_api. Resolve the sealed-witness design before making this aggregate the first migration slice.
| - Authorization becomes a single `authorize()` body — the four scattered policy | ||
| checks collapse into one reviewable function with visible ordering. This also | ||
| removes the `Ok(Failed)` vs `Err(terminate)` ambiguity, because the single seam | ||
| returns one `Result<Outcome, Blocked>` shape. |
Contributor
There was a problem hiding this comment.
Result<Outcome, Blocked> loses terminal host/storage/runtime errors and does not model post-dispatch obligation completion/abort, reservation settlement, lease consumption, or run-state transitions. Preserve an explicit error channel and multi-phase cleanup lifecycle.
| shortcuts leaking into production. | ||
|
|
||
| The thesis: these are three symptoms of **one** decision — *treating every crate | ||
| boundary as if it were a trust boundary.* Reborn has exactly **one** trust |
Contributor
There was a problem hiding this comment.
The loop is not the only untrusted boundary: WASM/script/MCP execution, runtime-supplied egress data, containers, and external services remain untrusted behind mediated filesystem/network/secrets/sandbox surfaces. Please distinguish crate seams from those security boundaries.
| - Make each durable store generic over a single storage backend: | ||
|
|
||
| ```rust | ||
| struct TurnStore<B: RowBackend> { backend: B } // domain logic written ONCE |
Contributor
There was a problem hiding this comment.
The accepted universal-filesystem ADR already designates RootFilesystem as the one backend trait and explicitly forbids a parallel backend abstraction. Consolidate domain logic over ScopedFilesystem/RootFilesystem; the referenced 2026-04-25 storage note is superseded.
| until every `match` handles it. WASM extensions stay open — they are *data* | ||
| behind the `Wasm` lane, not new lanes — so a closed lane set costs no real | ||
| extensibility. | ||
| - **Delete** the `HostRuntime`, `CapabilityDispatcher`, and `CapabilityHost` |
Contributor
There was a problem hiding this comment.
CapabilityHost is already a concrete generic struct, not a trait, and CapabilityDispatcher is a neutral dependency-inversion port. Please correct the dyn inventory and show how the existing crate boundary remains without that port.
|
🚅 Deployed to the ironclaw-pr-6175 environment in ironclaw-ci-preview
|
Fold the mechanistic root cause into §1.1: a hop-by-hop field diff of the five request types, showing only three are genuinely distinct states and the other two are duplication forced by the crate DAG plus dead transitional fields (trust_decision is ignored by DefaultHostRuntime; idempotency_key is unimplemented). Names the four mechanisms and quantifies the ~40% that is pure duplication. Add §3.1 mapping the five request types onto the three real states (Invocation -> +Authority -> +resolved handles), showing how each mechanism is eliminated or made explicit. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
railway-app
Bot
temporarily deployed
to
ironclaw-ci-preview / ironclaw-pr-6175
July 17, 2026 04:57
Destroyed
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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 `@docs/reborn/2026-07-17-architecture-simplification-dto-dyn-local.md`:
- Around line 94-99: Clarify the denominator behind the “~40%” claim in the
paragraph discussing “~14 re-wraps” and five request types: explicitly state
that it represents 2 of 5 request types, or recalculate and label the percentage
using the ~14-transition denominator.
- Around line 234-247: Add authenticated_actor_user_id to the canonical
Invocation or Authority state replacing CapabilityDispatchRequest, then preserve
and forward it through authorization and RuntimeDispatcher lane dispatch into
RuntimeAdapterRequest. Update the relevant dispatch construction and forwarding
paths without dropping the authenticated actor identity.
🪄 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: 604151ab-6dff-4fb5-8f8b-15af4ba92cee
📒 Files selected for processing (1)
docs/reborn/2026-07-17-architecture-simplification-dto-dyn-local.md
Comment on lines
+94
to
+99
| **Net:** of the five request types, hops 2 and 3 carry no new information (they | ||
| exist for Mechanism 1 and are padded by Mechanism 3); the other three are genuine | ||
| states that should be explicit and named. "~14 re-wraps" is really **~3 real | ||
| transitions + ~2 pure duplications + dead fields copied at every hop** — so ~40% | ||
| is pure duplication that a shared bottom-crate vocabulary removes outright, and | ||
| the rest becomes legible once the three states are named. |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
State the denominator for the “~40%” claim.
The text shifts from “~14 re-wraps” to five request types before reporting 40%. Clarify that this is 2/5, or recalculate against the stated ~14 denominator.
🤖 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 `@docs/reborn/2026-07-17-architecture-simplification-dto-dyn-local.md` around
lines 94 - 99, Clarify the denominator behind the “~40%” claim in the paragraph
discussing “~14 re-wraps” and five request types: explicitly state that it
represents 2 of 5 request types, or recalculate and label the percentage using
the ~14-transition denominator.
Source: Coding guidelines
…ions §1.3: replace the flat dyn table with prod-vs-test-double counts and storage (Arc<dyn>), and name the three mechanisms — trait-as-test-seam (HostRuntime: 1 prod + 6 doubles; CapabilityDispatcher 1 + 2), speculative replaceability, and generic-and-dyn double indirection. Correct a mislabel: CapabilityHost is a concrete generic struct, not a trait; the dyn on that path is the dispatcher it holds. RuntimeAdapter = 5 impls (4 lanes + resolver), a closed set. §1.4: quantify the per-backend, per-domain store duplication with LOC (turns ~4,260 in-memory vs ~1,710 filesystem) and a domain table (turns/processes/ approvals/authorization/run_state), and name the two mechanisms — logic welded to storage, multiplied by the TurnRun/processes lifecycle split. §4.2: drop CapabilityHost from the "delete trait" list (already concrete) and route the test-seam need through generics/one boundary fake. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
railway-app
Bot
temporarily deployed
to
ironclaw-ci-preview / ironclaw-pr-6175
July 17, 2026 05:02
Destroyed
There was a problem hiding this comment.
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)
docs/reborn/2026-07-17-architecture-simplification-dto-dyn-local.md (1)
342-346: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winKeep this conditional until the mode matrix is spelled out. The doc still asks whether
DeploymentConfigcan encode everyLocalDev/HostedDev/EnterpriseDevdifference (lines 437-438). Per.claude/rules/discovery-claims.md, this is load-bearing and needs a concrete behavior map, not a summary. Enumerate the preserved differences and the single enforcement point, or mark this migration step as contingent.🤖 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 `@docs/reborn/2026-07-17-architecture-simplification-dto-dyn-local.md` around lines 342 - 346, Update the migration step describing removal of composition-mode types to remain explicitly contingent until the mode matrix is defined. In the surrounding architecture discussion, enumerate the behavior differences preserved across LocalDev, HostedDev, and EnterpriseDev and identify the single enforcement point, or state that the `DeploymentConfig` migration is conditional on completing that mapping.
🤖 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 `@docs/reborn/2026-07-17-architecture-simplification-dto-dyn-local.md`:
- Around line 156-159: Update the RuntimeAdapter simplification discussion to
explicitly account for the fifth production implementation, ServiceResolved,
before describing the set as closed. State where its wrapper-resolution behavior
moves in the proposed design and how its existing semantics are preserved,
ensuring the four RuntimeLane variants do not omit this production path.
---
Outside diff comments:
In `@docs/reborn/2026-07-17-architecture-simplification-dto-dyn-local.md`:
- Around line 342-346: Update the migration step describing removal of
composition-mode types to remain explicitly contingent until the mode matrix is
defined. In the surrounding architecture discussion, enumerate the behavior
differences preserved across LocalDev, HostedDev, and EnterpriseDev and identify
the single enforcement point, or state that the `DeploymentConfig` migration is
conditional on completing that mapping.
🪄 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: 03e4a71a-686b-4f43-b489-b6452f47cd22
📒 Files selected for processing (1)
docs/reborn/2026-07-17-architecture-simplification-dto-dyn-local.md
… Local* Fold in five directives: - §2.1: the operating-system lens — kernel = mechanism (small, frozen, feature- agnostic vocabulary + a few real seams); everything that varies by feature or deployment is policy resolved to data at the edge. Adding a feature must not change the kernel. - §4.3 (rewritten): the storage seam already exists — RootFilesystem, with a first-class InMemoryBackend. Delete every hand-written InMemory*Store; tests use FilesystemXStore<InMemoryBackend>. No RowBackend to invent. - §4.4 (new): local-dev is a policy config (a DeploymentConfig value), NOT an implementation. The ~66-identifier LocalDev* shadow runtime across 42 composition files collapses to one config literal selecting shared substrates. Rename the two genuine resource types (LocalFilesystem->DiskFilesystem, LocalHostProcessPort-> HostProcessPort). Enforce with a no-"Local*"-type-names boundary test. - §4.5 (new): enumerate and freeze the kernel boundary — host_api's ~124 types + the ~13 AgentLoopDriverHost ports + the mediators. Move runtime_policy (mode enums) out of the vocabulary; freeze the neutral authority survivors by test. - §5/§7/refs updated: before/after rows for in-memory stores and Local*; migration resequenced by risk (delete-in-memory and Local*->config are the low-risk first slices); new evidence pointers. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
railway-app
Bot
temporarily deployed
to
ironclaw-ci-preview / ironclaw-pr-6175
July 17, 2026 05:10
Destroyed
There was a problem hiding this comment.
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 `@docs/reborn/2026-07-17-architecture-simplification-dto-dyn-local.md`:
- Around line 453-455: Update the ironclaw_architecture public-name guard to use
token-aware matching rather than substring bans, allowing names such as Locale,
HookLocalId, and LocalTraceSubmissionRecord. Reject only deployment-mode type
families and explicitly misnamed resource types, then align the documented rule
and associated tests with the implemented behavior.
🪄 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: 9bd4dcc2-5f18-4ecd-8c7b-0ef58dd229ed
📒 Files selected for processing (1)
docs/reborn/2026-07-17-architecture-simplification-dto-dyn-local.md
Four-cluster audit of the ~40 LocalDev* types (policies, stores, capability wiring, trust/evidence) against "config not code". Adds §4.4.1 with the verified verdict: DeploymentConfig expresses every local-dev *selection*, but the LocalDev* family is three things, and zero-LocalDev is three moves not one — 1. Already config: the capability policy is literally a TOML file; stores are the same shared types prod uses (production_turn_state_store<F> called by both), backend-selected; LocalDevOverride trust seam is inert. 2. Mis-prefixed shared substrate (gate-evidence readers, lease-terms provider, auth read-model, capability IO): genuine code but not local — de-prefix and share, not configify. 3. Genuine local-only mechanism (capability-port decorator stack: synthetic tools, surface disclosure, mid-run refresh): behavior stays code, but config-GATED shared middleware, not a LocalDev* factory. Synthetic product-ops (project_create/skill_activate/result_read/outbound_delivery) should become first-party capabilities on the normal lane. Security note: no trust/approval bypass found — override inert, provider trust only user_trusted, gate-evidence readers fail closed. §8 Q3 answered. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
railway-app
Bot
temporarily deployed
to
ironclaw-ci-preview / ironclaw-pr-6175
July 17, 2026 05:23
Destroyed
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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 `@docs/reborn/2026-07-17-architecture-simplification-dto-dyn-local.md`:
- Around line 417-421: Add reproducible definition/write-site citations to the
audit sections covering policy deserialization, production_turn_state_store call
sites, LocalDevOverride::evaluate, trust minting, fail-closed gate readers, and
the search boundary supporting the “~40” count. Use exact file:line and symbol
references, preserve the stated crates/openwiki/markdown search scope, and
update the “verified” and “no trust or approval bypass” conclusions to point to
that evidence.
- Around line 494-502: Define explicit per-deployment capability-surface
visibility and fail-closed defaults before claiming synthetic operations are
globally available. Update the affected architecture statements to make
availability conditional on that policy, and specify boundary-test coverage for
hosted and enterprise lanes before asserting unchanged behavior.
🪄 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: f5fdbb08-aa03-46ef-bae9-b046b699326a
📒 Files selected for processing (1)
docs/reborn/2026-07-17-architecture-simplification-dto-dyn-local.md
…case study §5 (new) — Target structure: the minimal kernel and clean interfaces. Component table (kernel = authority/recovery; substrates = mechanism behind ports; loops and products = replaceable userland). Interface sketches: the one generic ProductSurface (open/submit_turn/events/reply/resolve_gate/cancel — feature- agnostic), the kernel authorize/dispatch, the AgentLoopHost trust membrane, the substrate ports (RootFilesystem, ProcessSandbox with scope-only SandboxMount, SecretBroker, NetworkPolicy), and DeploymentConfig-as-data. Structure diagram. §6 (new) — Case study: the shell cross-tenant escape (#6170). Verified root cause (shell is a real OS subprocess the virtual FS doesn't bound; unsafe host port is the default; HostedSingleTenant -> LocalSingleUser -> LocalHost) and how the §5 structure makes it structurally impossible (ProcessSandbox as the only path, unconstructible host port, mode-from-fact, fail-closed, two-user containment test). Renumbered subsequent sections (7 before/after, 8 invariants, 9 migration, 10 open questions); added #6170 to references. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
railway-app
Bot
temporarily deployed
to
ironclaw-ci-preview / ironclaw-pr-6175
July 17, 2026 05:46
Destroyed
There was a problem hiding this comment.
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 `@docs/reborn/2026-07-17-architecture-simplification-dto-dyn-local.md`:
- Around line 649-659: Make DeploymentConfig data-only by defining concrete,
serializable enums for NetworkPolicy, Backend, and ProcessPolicy (and the
existing ApprovalPolicy if needed), rather than storing trait objects or
unspecified policy types. Update build_runtime to match on these enum values and
construct the corresponding runtime implementations, preserving the single
configuration-driven entry point and LocalDev/Hosted/Enterprise constants.
🪄 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: 21e44a2b-fe43-4121-b960-5c58e42ff9af
📒 Files selected for processing (1)
docs/reborn/2026-07-17-architecture-simplification-dto-dyn-local.md
…itecture rule at the plan safety-and-sandbox.md: new "Process and shell execution: real OS isolation, per tenant" section — the standing invariant issue #6170 violated. Codifies that the virtual ScopedFilesystem does not contain a subprocess; multi-user/served deployments must route process spawns through TenantSandboxProcessPort with a scope-derived mount (never LocalHostProcessPort); deployment mode must reflect multi-user serving; fail closed (no sandbox => no shell, never host shell); and requires a two-user cross-tenant escape test for changes to process ports, planner backend rules, or the profile->mode mapping. architecture.md: add a "Direction" pointer to the simplification design doc as the owning plan for the DTO/dyn/InMemory*/LocalDev* debt the smells describe, and cross-reference type-placement.md. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
railway-app
Bot
temporarily deployed
to
ironclaw-ci-preview / ironclaw-pr-6175
July 17, 2026 05:53
Destroyed
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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 @.claude/rules/architecture.md:
- Around line 211-214: Update the arch-exempt guidance in the architecture
documentation to define an actionable plan citation format: replace the
unresolved “plan `#NNNN`” placeholder with the actual plan identifier, or
explicitly specify how annotations must reference a valid plan and section.
Preserve the requirement that exemptions name an existing plan rather than
create a new one.
In @.claude/rules/safety-and-sandbox.md:
- Around line 108-116: Revise the safety guidance around
ScopedFilesystem/MountView and TenantSandboxProcessPort to clarify that virtual
mounts alone do not contain spawned processes, while the sandbox’s
kernel-enforced mount namespace does. Remove the broad claim that processes
always ignore those mounts, and preserve the requirement that multi-user
deployments route spawns through the sandbox whose mount derives from the turn
scope.
🪄 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: a44be9fd-f2df-4ebb-a3a8-389aae68833b
📒 Files selected for processing (2)
.claude/rules/architecture.md.claude/rules/safety-and-sandbox.md
Comment on lines
+211
to
+214
| smell here traces to one of those, cite that doc's section as the "plan #NNNN" | ||
| an `arch-exempt` must name, rather than opening a new one. The doc's §5 is the | ||
| minimal-kernel/clean-interface destination; new code should move toward it, not | ||
| add to the debt. |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Define the plan #NNNN`` citation format.
This rule requires arch-exempt annotations to cite plan #NNNN``, but neither the plan identifier nor NNNN resolution is defined here. Replace the placeholder with the actual plan ID or specify the required annotation format so exemptions are actionable.
🤖 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 @.claude/rules/architecture.md around lines 211 - 214, Update the arch-exempt
guidance in the architecture documentation to define an actionable plan citation
format: replace the unresolved “plan `#NNNN`” placeholder with the actual plan
identifier, or explicitly specify how annotations must reference a valid plan
and section. Preserve the requirement that exemptions name an existing plan
rather than create a new one.
Comment on lines
+108
to
+116
| - **The virtual filesystem does not contain a subprocess.** `ScopedFilesystem` / | ||
| `MountView` bound the *filesystem capability* (`filesystem.read`), a virtual-path | ||
| abstraction. A spawned OS process (`builtin.shell`, script lanes) sees the **real | ||
| kernel filesystem** and ignores those mounts. Never treat the scoped/virtual | ||
| filesystem as containment for a subprocess. | ||
| - **The only real containment for an OS process is the sandbox it runs in.** Any | ||
| deployment that authenticates more than one user MUST route process spawns through | ||
| the sandboxed port (`TenantSandboxProcessPort`, backed by `ironclaw_process_sandbox`) | ||
| whose mount is derived from the turn scope — never through the unsandboxed |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
Distinguish virtual scoping from sandbox mount containment.
ScopedFilesystem/MountView is not subprocess containment, but “a spawned OS process ... ignores those mounts” is too broad and conflicts with the following requirement that TenantSandboxProcessPort derive its OS mount from turn scope. State that virtual mounts alone do not contain processes; only the sandbox’s kernel-enforced mount namespace does.
🤖 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 @.claude/rules/safety-and-sandbox.md around lines 108 - 116, Revise the
safety guidance around ScopedFilesystem/MountView and TenantSandboxProcessPort
to clarify that virtual mounts alone do not contain spawned processes, while the
sandbox’s kernel-enforced mount namespace does. Remove the broad claim that
processes always ignore those mounts, and preserve the requirement that
multi-user deployments route spawns through the sandbox whose mount derives from
the turn scope.
… latency, event fan-out) Adds a performance section grounded in the actual hotspots: consolidating stores onto RootFilesystem (§4.3) makes the backend latency profile the kernel's, and it can be remote (libSQL/Postgres). Ranked critical-path table (per-turn ~11-store fan-out; heartbeat vs store-lock with lease-TTL coupling; libSQL BEGIN IMMEDIATE single-writer + #5751/#6089 contention; authorize() per-tool-call reads; active- thread lock; event append/projection fan-out; recovery poll). Plus the no-lock-across-remote-I/O rule (turn_scheduler.rs:787/881), what the refactor helps vs risks (centralizing latency/writer contention onto one seam), and design guidance (batched per-transition write, isolated heartbeat, cached read-mostly authority, async coalesced events, writer sharding). Renumbered Open questions to §13. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
railway-app
Bot
temporarily deployed
to
ironclaw-ci-preview / ironclaw-pr-6175
July 17, 2026 06:38
Destroyed
1. TurnRun vs ironclaw_processes: converge the mechanism (shared LeasedWorkUnit — §4.3 already collapses the store layer), keep the policy distinct (turn resume- from-checkpoint vs process terminal+re-spawn); bonus, the shared lease-recovery gives processes the reconciler they lack. 2. Authority: one sealed value (host-only construction via authorize()); narrowed read projection if an adapter needs a field. 3. DeploymentConfig: surface-disclosure derived from process:HostUnsandboxed; mid-run refresh gated by session:LongLived|PerRun; synthetic tools promoted to first-party capabilities (default hidden in hosted) — a security improvement (they gain authorize/scope-binding). Remaining items are tuning knobs, not architecture. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
railway-app
Bot
temporarily deployed
to
ironclaw-ci-preview / ironclaw-pr-6175
July 17, 2026 06:42
Destroyed
…me (BenKurrek gist) Adds §5.9 mapping this doc against the URT extension/adapter/auth design: strong, independent convergence (no-product-code-in-composition; config-not-code as recipe+engine auth; runtime-kind=closed lane set; built-ins on the identical pipeline; the trust boundaries; ProductSurface above the host pipelines). Complementary scope: URT is the deep extension/adapter/auth axis, this doc the broader kernel refactor; they compose (URT's dispatcher pipeline = authorize+ dispatch; adapter invoke/deliver = RuntimeLane execution). Adopts two URT refinements: (a) product_auth collapses to recipe data + one host AuthEngine, not per-adapter code (§5.8); (b) the Deletion/Addition/retired- taxonomy tests are the products-in-composition ratchet (§10). Clarifies WebUI consumes ProductSurface directly and is not a ChannelAdapter. Adds a reference. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
railway-app
Bot
temporarily deployed
to
ironclaw-ci-preview / ironclaw-pr-6175
July 17, 2026 06:50
Destroyed
… integration points
Name the two seams where this doc and the Unified Extension Runtime must agree:
1. One closed execution enum RuntimeLane = {FirstParty|Wasm|Mcp|Process}; the URT's
extension-declarable runtime *kinds* (first_party/wasm/mcp) are a strict subset.
Process (OS-subprocess/script sandbox) is host-only — no manifest can select it;
only host built-ins (shell/script) dispatch to it via ProcessSandbox. Load kind
(URT) vs execution lane (this doc) are different axes; don't merge them.
2. ToolPorts is derived from Authority, never independent: dispatch() materializes
egress (NetworkPolicy + host-side SecretBroker lease), state (ScopedFilesystem =
Authority.mounts), logging from (&Invocation, &Authority, descriptor). ToolPorts
can't be wider than Authority grants; the adapter never sees Authority itself. So
URT's ToolAdapter::invoke(call, ports) IS the body of dispatch(inv, auth, lane).
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
railway-app
Bot
temporarily deployed
to
ironclaw-ci-preview / ironclaw-pr-6175
July 17, 2026 06:52
Destroyed
There was a problem hiding this comment.
Actionable comments posted: 4
♻️ Duplicate comments (1)
docs/reborn/2026-07-17-architecture-simplification-dto-dyn-local.md (1)
352-355: 🗄️ Data Integrity & Integration | 🟠 MajorRetain a stable invocation idempotency identity.
Invocationhas no idempotency key, and this text permits deleting it despite the replay/crash-recovery requirements. Without a durable operation ID and atomic claim/outcome record, a retry can rerun a side effect. Add the key to the canonical state and define deduplication recovery.🤖 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 `@docs/reborn/2026-07-17-architecture-simplification-dto-dyn-local.md` around lines 352 - 355, Revise the architecture text around Mechanism 3 and the canonical Invocation state to retain a durable operation/idempotency key. Define the atomic claim and outcome record used to deduplicate retries, including replay and crash-recovery behavior, and remove the option to delete idempotency_key.
🤖 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 `@docs/reborn/2026-07-17-architecture-simplification-dto-dyn-local.md`:
- Around line 1129-1134: The architecture table’s Event append + projection
fan-out guidance must keep durable event commits atomic with transition state.
Update the event persistence design to append events within the same transaction
or outbox boundary as the state commit, while continuing to decouple only
subscriber delivery and fan-out through bounded buffers.
- Around line 303-313: Update Authority and the authorize/dispatch flow so
authorized state is cryptographically or structurally bound to the specific
Invocation, including capability, scope, and authenticated actor provenance from
authenticated_actor_user_id. Have authorize return this invocation-bound state
and require dispatch to consume that bound state rather than independently
supplied inv and auth values, preventing mismatched invocation/authorization
pairs.
- Around line 303-307: Update the canonical model around Invocation and
Authority so the loop membrane preserves validated input-reference, activity,
surface-version, and resume-token state before crossing the trust boundary.
Either retain a separate loop request type or extend Invocation with the
corresponding validated fields; do not leave Invocation as raw Json while
claiming it carries reference and resume data.
- Around line 324-333: Clarify the executable result contract around the
documented invoke/authorize/dispatch flow so it explicitly maps blocked,
recoverable, terminal, host/storage, and obligation/lease cleanup failures.
Reconcile the bare Outcome return described near line 313 with the
Result<Outcome, Blocked> contract, specifying how terminal and post-dispatch
failures propagate without reintroducing the removed ambiguity.
---
Duplicate comments:
In `@docs/reborn/2026-07-17-architecture-simplification-dto-dyn-local.md`:
- Around line 352-355: Revise the architecture text around Mechanism 3 and the
canonical Invocation state to retain a durable operation/idempotency key. Define
the atomic claim and outcome record used to deduplicate retries, including
replay and crash-recovery behavior, and remove the option to delete
idempotency_key.
🪄 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: c48bd9ea-9b9b-4eff-87b0-06c1d77b4a48
📒 Files selected for processing (1)
docs/reborn/2026-07-17-architecture-simplification-dto-dyn-local.md
There was a problem hiding this comment.
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 `@docs/reborn/2026-07-17-architecture-simplification-dto-dyn-local.md`:
- Around line 821-850: Clarify the Process-lane contract in the RuntimeLane and
ToolPorts sections: explicitly state whether first-party/WASM requests through a
host capability are allowed, and require them to pass an explicit Authority and
deployment-policy check that fails closed. Define ToolPorts.process as a bounded
ProcessSandbox derived by dispatch(), and add a multi-user isolation test
covering this authorization and sandbox boundary.
🪄 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: f12f7683-28ef-4c65-a42d-32a68b5e3bb8
📒 Files selected for processing (1)
docs/reborn/2026-07-17-architecture-simplification-dto-dyn-local.md
Nine substantive design-contract fixes: - §3 core model: LoopRequest (loop pre-trust, input-by-ref) resolved to Invocation at the membrane; Authorized = sealed AND invocation-bound (actor/scope/activity_id provenance) so dispatch can't be handed a mismatched (inv,auth); activity_id IS the invocation idempotency identity (idempotency_key unified with it, not deleted, satisfying §11.3); three distinct outcome channels Blocked | HostFailure | Outcome (no Ok(Failed)/Err ambiguity). Type count 3→4. §3.1/§5.3/§5.4/§11.1 aligned; Authority→Authorized throughout. - §11.2/§6: scope cross-tenant isolation to multi-user/served deployments (matrix test), not "any deployment state" (single-user local legitimately allows host proc). - §9/§10: quarantine the known-red two-user test (#[ignore]/expected-fail until the fix merges); ratchets freeze checked-in symbol allowlists (set membership), not aggregate counts (a swap evades a count). - §12: durable event append is atomic with the state transition (same tx/outbox); only subscriber fan-out is decoupled. - §5.8: adapters resolved via a product-neutral ExtensionId-keyed factory registry passed to composition as input; config lists ids, not types. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
railway-app
Bot
temporarily deployed
to
ironclaw-ci-preview / ironclaw-pr-6175
July 17, 2026 07:05
Destroyed
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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 `@docs/reborn/2026-07-17-architecture-simplification-dto-dyn-local.md`:
- Around line 315-317: Update the resolve function declaration to return
Result<Invocation, ...> instead of Invocation, with an explicit error type for
missing references or invalid/stale evidence. Propagate resolution failures
before authorize so invalid inputs fail closed, and add tests covering missing
and stale input_ref cases.
- Around line 656-663: Update the authorization flow around Authorized,
authorize(), and dispatch() so the execution lane is sealed into authorization
or represented by a kernel-issued lane witness, rather than independently
supplied by callers. Ensure dispatch validates and uses that bound lane,
preserving rejection of hosted or extension executions targeting Process, and
add or update tests covering this restriction so the documented guarantee
matches behavior.
🪄 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: 1b5b6049-1486-4473-a437-b084109dfb0a
📒 Files selected for processing (1)
docs/reborn/2026-07-17-architecture-simplification-dto-dyn-local.md
Comment on lines
+315
to
+317
| fn resolve (req: LoopRequest, scope: &Scope) -> Invocation; // membrane: deref input, bind actor+scope | ||
| fn authorize(inv: Invocation) -> Result<Authorized, Blocked>; // ALL policy, one place; consumes inv | ||
| fn dispatch (auth: &Authorized, lane: &RuntimeLane) -> Result<Outcome, HostFailure>; // inv is inside auth — can't mismatch |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Give membrane resolution a failure channel.
resolve() dereferences input_ref and binds actor/scope, but is declared infallible. Missing references or invalid evidence therefore have no fail-closed representation before authorization. Return an explicit error (Result<Invocation, ...>) and test invalid/stale inputs.
🤖 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 `@docs/reborn/2026-07-17-architecture-simplification-dto-dyn-local.md` around
lines 315 - 317, Update the resolve function declaration to return
Result<Invocation, ...> instead of Invocation, with an explicit error type for
missing references or invalid/stale evidence. Propagate resolution failures
before authorize so invalid inputs fail closed, and add tests covering missing
and stale input_ref cases.
Source: Coding guidelines
Comment on lines
+656
to
+663
| struct Invocation { activity_id: ActivityId, capability: CapabilityId, input: Json, scope: Scope, actor: ActorId, estimate: Estimate } | ||
| struct Authorized { /* the Invocation bound to trust/approval/reservation/mounts — SEALED: private, built only by authorize() */ } | ||
| enum Blocked { Approval(GateRef), Auth(GateRef), Resource(GateRef) } | ||
| enum HostFailure { Transient(ErrRef), Permanent(ErrRef) } | ||
| struct Outcome { refs: OutcomeRefs, summary: SafeSummary } // tool success OR recoverable tool failure | ||
|
|
||
| fn authorize(inv: Invocation) -> Result<Authorized, Blocked>; // consumes inv; binds actor/scope/activity_id | ||
| fn dispatch (auth: &Authorized, lane: &RuntimeLane) -> Result<Outcome, HostFailure>; // inv is inside auth |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | 🏗️ Heavy lift
Bind the execution lane to Authorized.
dispatch(auth, lane) accepts an independently selected RuntimeLane, while Authorized contains no lane or descriptor binding. The host-only Process restriction is therefore only a call-site convention. Store the resolved lane in the sealed authorization state, or require a sealed kernel-issued lane witness and test hosted/extension-to-Process rejection.
As per coding guidelines, documentation guarantees must match behavior implemented in code and covered by tests.
Also applies to: 849-864
🤖 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 `@docs/reborn/2026-07-17-architecture-simplification-dto-dyn-local.md` around
lines 656 - 663, Update the authorization flow around Authorized, authorize(),
and dispatch() so the execution lane is sealed into authorization or represented
by a kernel-issued lane witness, rather than independently supplied by callers.
Ensure dispatch validates and uses that bound lane, preserving rejection of
hosted or extension executions targeting Process, and add or update tests
covering this restriction so the documented guarantee matches behavior.
Source: Coding guidelines
7 tasks done
theredspoon
added a commit
to theredspoon/ironclaw
that referenced
this pull request
Jul 18, 2026
* chore(ci): dev metrics + composition mass ratchet gate (#6167)
* chore(ci): dev metrics + composition mass ratchet gate
Adds a three-tier development-metrics tool and a guardrail that stops the
ironclaw_reborn_composition crate from accreting more of the codebase.
scripts/dev_metrics.py — three tiers from git + GitHub + working tree:
- Tier 1 flow/speed: PR lead time, size distribution, merge cadence
- Tier 2 quality/stability: change-failure proxy, rework, test share
- Tier 3 codebase health: composition mass, v1 src burndown, file sprawl,
abstraction density, boundary-test coverage
Composition mass ratchet — the dependency-boundary tests police edges
*between* crates but are blind to mass piling up *inside* one crate.
ironclaw_reborn_composition is charter-bound to assembly-only wiring yet is
now ~26.7% of all production crate code. This gate is that missing guard:
- scripts/ci/composition-budget.toml — committed ceiling (enforce +
tolerance), modeled on the existing coverage-floor ratchet
- scripts/ci/check-composition-budget.sh — pure-bash gate; one-directional
(fails only on growth past the ceiling), emits a down-ratchet nudge as
carve-outs free up slack
- scripts/ci/test-check-composition-budget.sh — 22 assertions / 10 fixture
cases incl. a guard that the real tree passes the committed budget
Wiring:
- CI: new composition-budget job in code_style.yml (runs the gate + self-
tests it, registered in the aggregating code-style gate)
- Local: pre-commit-safety.sh runs the gate when composition or the gate
itself is staged; dev-setup.sh install message updated
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* fix(ci): address review — production-only metric, script hardening, dev-metrics tests
Review feedback on #6167 (gemini, ironloopai, coderabbit):
Blocking — gate counted test-only code despite its documented "tests
excluded" contract. Exclude test-only FILES (tests.rs/test_*.rs/*_tests.rs
and /tests/ dirs) from both numerator and denominator; rebaseline the
ceiling 2670 -> 2398 bp (26.70% -> 23.98%). Inline #[cfg(test)] modules
remain a documented, symmetric residual (a line-counter can't parse them).
Added a regression case proving test files are excluded.
check-composition-budget.sh: toml_get no longer aborts under set -e +
pipefail when a key is missing (|| true) so schema validation is reached;
added a missing-key regression case.
test-check-composition-budget.sh: set -euo pipefail (repo invariant);
SIGPIPE-safe capture + fixture generation; pure-bash asserts (no pipes).
dev_metrics.py: bound `gh` with a 30s timeout and treat non-JSON output as
unavailable; fix the trait-impl density regex to count `impl<T> ... for`
generics; harden find/grep/wc probes with pipefail + rc checks (no more
false-zero metrics); UTF-8 file writes; surface the gate-aligned production
share as the ratchet metric and relabel the byte-based trend as a distinct,
coarser measurement; extract a pure classify_commit helper.
New scripts/test_dev_metrics.py — caller-level unit tests for
classification, percentiles, change-failure bucketing, rendering, and the
test-file/impl regexes; wired into the composition-budget CI job.
pre-commit hook: trigger on any staged crates/**.rs change (the metric is a
ratio) and document the working-tree/CI-authoritative limitation.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* fix(ci): harden PR classifier against transient GitHub API flakes
The classify job (#6167 CI) failed with `invalid character '<' looking
for beginning of value`: a transient API error returned an HTML page,
`gh --jq` aborted, and under `set -e` the whole labels-only job failed
and blocked the PR.
pr-labeler.sh now:
- routes every gh call through a `gh_retry` wrapper (retry + linear
backoff), and
- treats each classifier as best-effort — a step that still can't fetch
after retries only emits a `::warning::` and the script exits 0, so
labeling never gates a merge.
Two bash traps fixed along the way, both caught by the new test:
- a bare `if cmd; then …; fi` resets `$?` to 0 after `fi`, so gh_retry's
give-up looked like success — capture rc in the `else`;
- `set -e` is suppressed inside a function on the left of `||`, so the
classifiers check their own fetches explicitly instead of relying on
errexit.
Regression test: .github/scripts/test-pr-labeler.sh (retry/backoff,
give-up, and end-to-end non-fatal + happy-path via a fake `gh`), wired
into the code_style "Static-check self-tests" step and the has_code
path filter so it runs when the labeler or its test changes.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* feat(ci): add dispatch (Arc<dyn>) ratchet + dev-metrics dispatch signals
Companion to the mass ratchet for the "reduce traits & dispatch" goal
(#6168 / runtime-decomposition plan #4471).
check-composition-budget.sh now enforces TWO metrics: composition's share of
production crate code (existing) AND its Arc<dyn> dispatch count. The dispatch
count is scoped to composition production files EXCLUDING src/slack and
src/extension_host — those are owned by the separate channel/extension
refactor, so this gate must not govern or trip on their work. One-directional
like the mass ratchet: only trips on growth; nudges when slack accrues.
composition-budget.toml: arc_dyn_ceiling = 1093 (current governed count),
tolerance 15.
test harness: +6 dispatch cases (within / breach / dry-run / slack+extension
exclusion / missing-key schema error); budget() helper carries the dispatch
keys; count_arc_dyn tolerates no-match under set -e + pipefail. 36 cases pass.
dev_metrics.py: Tier-3 reports governed Arc<dyn> count and distinct dyn-trait
count (the dispatch-breadth trend), matching the ratchet scope.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* feat(reborn-cli): background service install (launchd/systemd) + service restart (#6172)
* feat(reborn-cli): background service install (launchd/systemd) for ironclaw-reborn
Extracted from #6157 (service half only; TUI stays parked there). Adds
`service install/uninstall/start/stop/status`, the serve-invocation
plist/unit contract (IRONCLAW_REBORN_HOME only, no secrets), and the
`full` feature bundle with libsql as default storage.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* feat(reborn-cli): add `service restart` verb
Composes the existing stop+start through the shared ServiceCommandRunner
dispatch. Stopped service starts cleanly; uninstalled service errors with
guidance; a failed start after a successful stop reports the service as
stopped rather than half-restarted.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* test(reborn-cli): pin service-PR surface invariants
Extend help_mentions_reborn_commands to assert `service` is listed
under webui-v2-beta and that no `tui` subcommand exists; add
service_help_lists_all_verbs pinning the six service verbs
(install/start/stop/status/restart/uninstall).
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* refactor(reborn-cli): dedupe service restart, normalize status output
Extracts the shared restart decision tree into restart_generic (fn-pointer
seam; platforms keep only their own detection), normalizes `service status`
to one running/stopped/not-installed vocabulary on both platforms, hoists
write_atomic so launchd plist writes are crash-safe, and fixes a stale
verb-count doc.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* fix(reborn-cli): preserve raw systemd ActiveState as a status detail line
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* fix(reborn-cli): harden service module per PR review (PID parse, systemctl parsing, perms, reload, orphan status, rollback, preflight)
Addresses 9 verified findings from PR #6172 review:
1. launchd service_running misread `-` (loaded-but-stopped) PID as
running; mirrors operator_service_lifecycle's launchd_status_from_line
shape, split into service_running (has PID) vs service_loaded (any
status) since uninstall/install genuinely need the latter.
2. systemd query_unit_state now parses Key=Value lines (order-independent,
no --value) and errors on a missing required key instead of
unwrap_or_default(), which silently read as enabled=false.
3. write_atomic sets 0600 on unix before create_new, matching
operator_service_lifecycle's write_service_file.
4. launchd install now unloads/reloads a currently-loaded job after
rewriting the plist, so a reinstall actually picks up the new
ProgramArguments/EnvironmentVariables.
5. status now queries the manager unconditionally on both platforms so
an orphaned unit (file removed out-of-band, still loaded/enabled)
reports installed; two tests that pinned the old skip-when-absent
behavior were pinning the orphan-hiding bug and are updated/renamed.
6. systemd uninstall's remove_file failure now routes through the same
rollback path (restore file + reload + re-enable) as a daemon-reload
failure, via an extracted rollback_uninstall helper.
7. preflight_warnings gained a webui_token_file_is_valid check; doc
comment corrected to describe what's actually checked.
8. Documented (not built) that launchd's StandardOutPath/StandardErrorPath
logs are unrotated, in both a code comment and `service install --help`.
9. Cargo.toml `full` feature now includes root-llm-provider so
`--no-default-features --features full` stays self-contained.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* feat(reborn-cli): adopt the canonical service identity (com.ironclaw.reborn)
The CLI service surface and the WebUI operator facade
(RebornLocalServiceLifecycle) now deliberately share one unit name and
launchd label. CLI installs atomically replace a facade-installed unit —
a security improvement, since the facade bakes the WebUI token into the
unit file while the CLI unit is secret-free. Consolidating the two
implementations is a documented follow-up.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* fix(reborn-cli): CI mock fidelity, token-file hygiene, review fixes
1. mod.rs: add the "systemctl show unit state" arm to the shared
SuccessfulServiceCommandRunner mock (install_with_runner now queries
unit state pre-write). Production code and the strict parser are
correct; only the mock modeled reality incompletely.
2. webui_token.rs: propagate real I/O errors instead of treating them
as "absent" (was silently overwriting unreadable tokens); reject
symlinked/oversized token files; repair (not reject) a wrongly
permissioned but valid token on accept; serve.rs no longer collapses
VarError::NotUnicode into "unset".
3. launchd.rs/mod.rs: suppress the "keeps the OLD definition" advisory
when install already reloaded a loaded job in place (the definition
is live immediately in that case); systemd's advisory is unaffected.
4. mod.rs: preflight-warning coverage now drives service install
(runner-injectable, warnings returned) instead of only unit-testing
the preflight_warnings helper directly.
5. systemd.rs: uninstall's remove_file step is now injectable so its
rollback test forces a deterministic failure, replacing the
chmod-0o555 approach that silently no-ops under a root test runner.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* fix(reborn-cli): preserve error source chain in service rollback failures
combined_failure flattened the primary error and rollback outcomes into
one anyhow!() string, losing the source chain. Use .context() so the
primary stays inspectable via source()/{:#} beneath the rollback text.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* fix(reborn-cli): single-handle token read closes TOCTOU window
read_token_file_checked previously stat'd then read the token file as
two separate syscalls, letting a symlink/FIFO/oversized file be swapped
in between them; a FIFO also passed the length check and could block
serve startup indefinitely. Now opens once with O_NOFOLLOW|O_NONBLOCK,
checks type/size via fstat on that handle, and bounds the read to
MAX_BYTES+1 from the same fd. Non-unix keeps the prior best-effort path.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* fix(reborn-cli): uninstall disable rollback, hermetic verb dispatch, dry-run coverage, docs
Four verified findings from PR #6172 review round: (1) systemd uninstall's
disable failure now rolls back like every sibling failure branch instead of
propagating with a bare `?`; (2) a smoke test pins that a directory at the
webui-token path fails `onboard --dry-run` non-zero without mutating home;
(3) start/stop/restart/status/uninstall get the same runner-injectable split
`install` already had (`ServicePlatform::*_with_runner`), with one
consolidated clap-dispatch test instead of duplicating per-verb coverage;
(4) FEATURE_PARITY.md and CHANGELOG.md reflect the shipped service-install
feature.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* fix(reborn-cli): single status line per service restart
restart_generic's stop/start fn pointers called the loud
start_with_runner/stop_with_runner, which each print their own
"Service started"/"Service stopped" line in addition to
restart_generic's own summary line, so `service restart` printed two
lines. Add quiet variants (start_with_runner_quiet/
stop_with_runner_quiet) that skip the println, used only by
restart_with_runner; the public start/stop commands keep printing as
before.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* fix(reborn-cli): stop/restart honor manager-loaded state like status/uninstall
launchd `stop` gated on service_running alone, leaving a loaded-but-not-
running KeepAlive job (`-` PID) registered for respawn; `restart` derived
`was_running` the same way, so it bare-loaded an already-loaded label
(which launchd errors on) instead of reloading. systemd `stop` gated only
on unit-file existence, silently no-opping on a unit removed out-of-band
while still loaded/enabled. All three now check manager state the way
status/uninstall already do.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* fix(reborn-cli): honor XDG_CONFIG_HOME for systemd units, guard launchd start on loaded labels
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* test(reborn-cli): make service tests hermetic over XDG_CONFIG_HOME
commit 5b3f39eb9 made config_home() honor $XDG_CONFIG_HOME, which
unit_path() now reads. Service tests that fake $HOME into a tempdir
never cleared XDG_CONFIG_HOME, so on hosts where it's set (CI runners
observed setting it to $HOME/.config), unit_path() resolved to the
real path instead of the tempdir — causing
systemd::tests::restart_not_installed_errors_with_install_guidance
and
tests::install_then_uninstall_linux_writes_and_removes_unit_file to
fail. Production config_home() behavior is unchanged and correct.
Extends the TempHomeGuard helpers in mod.rs and systemd.rs (new,
mirroring mod.rs's) to also clear/restore XDG_CONFIG_HOME, and
switches all HOME-faking tests in systemd.rs onto the guard instead of
manual set/restore blocks.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
* fix(ci): stop safety ReDoS timing guards flaking under coverage [skip-regression-check] (#6181)
The `*_100kb_near_miss` adversarial tests in ironclaw_safety are ReDoS
guards: they feed a ~100 KB near-miss payload to a regex scan and assert
it finishes fast enough to rule out catastrophic backtracking (which
would take seconds or hang). They used a hard 100 ms bound.
Under `cargo llvm-cov` instrumentation on shared CI runners the linear
scan is ~100x slower, so the Coverage (default) job flaked with
"anthropic_api_key pattern took 101ms on 100KB near-miss" — 1 ms over
the threshold. Only the instrumented coverage job is affected.
Replace the per-test hard thresholds (100 ms in leak_detector/validator/
sanitizer, 500 ms already in policy) with one documented shared constant
REDOS_SCAN_BUDGET_MS = 2000 in the crate root. 2 s keeps a large margin
below any real ReDoS while tolerating instrumentation overhead, and the
guards still fail loudly on genuine catastrophic backtracking.
Test-only change; no production behavior touched — hence the
regression-check skip.
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* test(e2e): black-box smoke for ironclaw-reborn serve — restart + kill-9 durability (#5523)
The in-process Reborn integration harness cannot prove real process
startup, real HTTP end-to-end, or process-death durability — its
new_at_path() reopen approximates a restart but never actually kills a
process. Add a thin, permanent black-box smoke suite that boots the
real ironclaw-reborn binary and drives it purely over HTTP:
- boot -> /api/health -> scripted chat round-trip
- tool-call turn executes and finalizes a reply
- graceful restart (SIGINT) preserves thread history
- kill -9 durability: on-disk libsql state survives an unclean death,
server comes back healthy, no leaked child processes
- bearer-auth boundary (401 without token, 200 with)
Fixture design: reuses the existing reborn_v2_restartable_server
fixture (already restart-capable against a persistent home dir) rather
than porting the legacy ManagedIronclawServer class. Extends its
stop() closure with a `hard: bool = False` flag for SIGKILL, so the
fixture's tuple shape and existing consumer are untouched. Promotes
the capability-preview polling helpers out of
test_reborn_webui_v2_legacy_tool_execution.py into the shared harness
(now used by both files) instead of adding a second copy for the new
suite.
Mutation-verified the durability scenario: temporarily pointed
restart() at a fresh home dir per call, confirmed the kill-9 test goes
red for the right reason (persisted thread missing after "restart"),
then reverted to a clean diff.
Wires a `blackbox-smoke` CI job into the existing reborn-e2e.yml
job-per-file pattern (mirrors webui-v2-smoke's build step, no
Playwright/Node needed since this suite is HTTP-only).
Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
* fix(webui-v2): improve toast lifecycle and accessibility (#6151)
* fix(webui): improve toast lifecycle and accessibility
* test(e2e): cover toast lifecycle and stacking
* fix(webui): type toast presentation mappings
* test(e2e): fast-forward toast hover timing
* test(e2e): align toast clock requirements
* fix(webui-v2): add theme selection controls to Appearance settings (#6148)
* fix(webui): add theme controls to appearance settings
* test(e2e): cover appearance theme persistence
* fix(webui): address appearance accessibility review
* fix(webui): type appearance theme controls
* fix(webui): use native theme radios
* feat(reborn): serve webui at root path instead of `v2` (#6152)
* feat(reborn): serve the WebUI from root paths
* test(e2e): cover root-mounted Reborn WebUI
* fix(webui): reject noncanonical SPA paths
* fix(webui): address root-mount review feedback
* refactor(webui): derive static router config errors
* fix(webui-v2): surface workspace download failures (#6150)
* fix(webui-v2): surface workspace download failures
* test(e2e): cover workspace download failure feedback
* test(e2e): centralize workspace download selectors
* test(e2e): navigate workspace downloads through UI
* docs(reborn): propose architecture simplification — fewer DTOs, less dyn, no local-specific structs (#6175)
* docs(reborn): propose architecture simplification — fewer DTOs, less dyn, no local-specific structs
Design note proposing a fundamental simplification of the Reborn host/runtime
internals, grounded in a code audit and a cross-reference against the last ~30
days of PRs/issues.
Thesis: DTO proliferation (~14 mirror structs per capability call), dyn
proliferation (~6 hot-path trait objects, most single-impl), and local-specific
store structs are three symptoms of one decision — treating every crate boundary
as a trust boundary when Reborn has exactly one (loop <-> host).
Proposes: one canonical payload type in ironclaw_host_api (Invocation/Authority/
Outcome), authority as a single fold, a closed RuntimeLane enum instead of dyn
RuntimeAdapter, backend-generic stores (RowBackend) to delete the InMemory*/
Filesystem* tree, and DeploymentConfig-as-data instead of composition-mode
struct families. Preserves all security invariants; incremental migration with
a first-party-lane proof-of-concept slice. Complements #6168.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* docs(reborn): show the field-level "why" behind the ~14 re-wraps
Fold the mechanistic root cause into §1.1: a hop-by-hop field diff of the five
request types, showing only three are genuinely distinct states and the other
two are duplication forced by the crate DAG plus dead transitional fields
(trust_decision is ignored by DefaultHostRuntime; idempotency_key is
unimplemented). Names the four mechanisms and quantifies the ~40% that is pure
duplication.
Add §3.1 mapping the five request types onto the three real states
(Invocation -> +Authority -> +resolved handles), showing how each mechanism is
eliminated or made explicit.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* docs(reborn): show impl/store-level "why" for the dyn and stores sections
§1.3: replace the flat dyn table with prod-vs-test-double counts and storage
(Arc<dyn>), and name the three mechanisms — trait-as-test-seam (HostRuntime: 1
prod + 6 doubles; CapabilityDispatcher 1 + 2), speculative replaceability, and
generic-and-dyn double indirection. Correct a mislabel: CapabilityHost is a
concrete generic struct, not a trait; the dyn on that path is the dispatcher it
holds. RuntimeAdapter = 5 impls (4 lanes + resolver), a closed set.
§1.4: quantify the per-backend, per-domain store duplication with LOC (turns
~4,260 in-memory vs ~1,710 filesystem) and a domain table (turns/processes/
approvals/authorization/run_state), and name the two mechanisms — logic welded
to storage, multiplied by the TurnRun/processes lifecycle split.
§4.2: drop CapabilityHost from the "delete trait" list (already concrete) and
route the test-seam need through generics/one boundary fake.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* docs(reborn): OS mechanism/policy framing — kill in-memory stores and Local*
Fold in five directives:
- §2.1: the operating-system lens — kernel = mechanism (small, frozen, feature-
agnostic vocabulary + a few real seams); everything that varies by feature or
deployment is policy resolved to data at the edge. Adding a feature must not
change the kernel.
- §4.3 (rewritten): the storage seam already exists — RootFilesystem, with a
first-class InMemoryBackend. Delete every hand-written InMemory*Store; tests use
FilesystemXStore<InMemoryBackend>. No RowBackend to invent.
- §4.4 (new): local-dev is a policy config (a DeploymentConfig value), NOT an
implementation. The ~66-identifier LocalDev* shadow runtime across 42 composition
files collapses to one config literal selecting shared substrates. Rename the two
genuine resource types (LocalFilesystem->DiskFilesystem, LocalHostProcessPort->
HostProcessPort). Enforce with a no-"Local*"-type-names boundary test.
- §4.5 (new): enumerate and freeze the kernel boundary — host_api's ~124 types +
the ~13 AgentLoopDriverHost ports + the mediators. Move runtime_policy (mode
enums) out of the vocabulary; freeze the neutral authority survivors by test.
- §5/§7/refs updated: before/after rows for in-memory stores and Local*; migration
resequenced by risk (delete-in-memory and Local*->config are the low-risk first
slices); new evidence pointers.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* docs(reborn): audit result — does DeploymentConfig express everything?
Four-cluster audit of the ~40 LocalDev* types (policies, stores, capability
wiring, trust/evidence) against "config not code". Adds §4.4.1 with the verified
verdict: DeploymentConfig expresses every local-dev *selection*, but the LocalDev*
family is three things, and zero-LocalDev is three moves not one —
1. Already config: the capability policy is literally a TOML file; stores are the
same shared types prod uses (production_turn_state_store<F> called by both),
backend-selected; LocalDevOverride trust seam is inert.
2. Mis-prefixed shared substrate (gate-evidence readers, lease-terms provider,
auth read-model, capability IO): genuine code but not local — de-prefix and
share, not configify.
3. Genuine local-only mechanism (capability-port decorator stack: synthetic tools,
surface disclosure, mid-run refresh): behavior stays code, but config-GATED
shared middleware, not a LocalDev* factory. Synthetic product-ops
(project_create/skill_activate/result_read/outbound_delivery) should become
first-party capabilities on the normal lane.
Security note: no trust/approval bypass found — override inert, provider trust
only user_trusted, gate-evidence readers fail closed. §8 Q3 answered.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* docs(reborn): add target-structure/interfaces section + shell-escape case study
§5 (new) — Target structure: the minimal kernel and clean interfaces. Component
table (kernel = authority/recovery; substrates = mechanism behind ports; loops
and products = replaceable userland). Interface sketches: the one generic
ProductSurface (open/submit_turn/events/reply/resolve_gate/cancel — feature-
agnostic), the kernel authorize/dispatch, the AgentLoopHost trust membrane, the
substrate ports (RootFilesystem, ProcessSandbox with scope-only SandboxMount,
SecretBroker, NetworkPolicy), and DeploymentConfig-as-data. Structure diagram.
§6 (new) — Case study: the shell cross-tenant escape (#6170). Verified root cause
(shell is a real OS subprocess the virtual FS doesn't bound; unsafe host port is
the default; HostedSingleTenant -> LocalSingleUser -> LocalHost) and how the §5
structure makes it structurally impossible (ProcessSandbox as the only path,
unconstructible host port, mode-from-fact, fail-closed, two-user containment test).
Renumbered subsequent sections (7 before/after, 8 invariants, 9 migration,
10 open questions); added #6170 to references.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* docs(rules): add process/shell tenant-isolation invariant; point architecture rule at the plan
safety-and-sandbox.md: new "Process and shell execution: real OS isolation, per
tenant" section — the standing invariant issue #6170 violated. Codifies that the
virtual ScopedFilesystem does not contain a subprocess; multi-user/served
deployments must route process spawns through TenantSandboxProcessPort with a
scope-derived mount (never LocalHostProcessPort); deployment mode must reflect
multi-user serving; fail closed (no sandbox => no shell, never host shell); and
requires a two-user cross-tenant escape test for changes to process ports,
planner backend rules, or the profile->mode mapping.
architecture.md: add a "Direction" pointer to the simplification design doc as
the owning plan for the DTO/dyn/InMemory*/LocalDev* debt the smells describe, and
cross-reference type-placement.md.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* docs(reborn): §5.8 — products are adapters over ProductSurface, collapse composition-split surfaces
Every product owns its whole host side (protocol + transport + identity) as one
adapter consuming the kernel ProductSurface; composition holds no product/transport
code. Quantifies the split: WebUI across 4 places (webui_v2 + webui_ingress +
static + composition/webui), and ~108K LOC of product code in the god-crate
(slack 40.6K, product_auth 32.7K, runtime 14.5K, llm_admin 8.5K, automation 6.1K,
webui 4.6K, outbound 1.8K). Telegram is the closest-to-clean reference shape.
Invariant enforceable by an ironclaw_architecture test banning slack/webui/
telegram/openai/transport identifiers from composition. Ties to §4.4 and #6168.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* docs(reborn): §10 — enforcement / anti-slippage ratchets pulling together all checks
Consolidates the per-axis static checks into one table: process isolation (#6170
two-user escape test), mirror DTOs (check-type-duplicates.py), dyn mediators,
InMemory*Store, Local* types, host_api freeze, and products-in-composition — each
with its home, the addable-now ratchet (freeze current count/allowlist, fail on
new), and the hard ban that is also its definition-of-done when the axis lands.
Notes the guardrail self-test + two-hook-path requirement and that Local*/
InMemory*/composition bans must start as frozen allowlists (can't hard-ban today).
Renumbered Open questions to §11.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* docs(reborn): add §11 testing (state-machine invariants, idempotency from any state); distinguish crate seams from trust boundaries
§11 Testing (new): specifies the behavioral suite — the state machines to pin
(turn/run, capability invoke, lease, gate/resume), the invariants that must hold
from ANY reachable state, the idempotency contract, and how to reach arbitrary
states (model-based stateful property tests, exhaustive state×op enumeration,
fault injection), cross-backend parity, fail-closed/adversarial, interface
conformance harnesses, and integration-first tiering. Design only.
Trust-boundary correction (per review): the doc overstated "exactly one trust
boundary." Reborn has several — the untrusted loop, untrusted runtime-lane
execution (WASM/script/MCP/containers/external services), and untrusted
runtime-supplied data (egress/worker output) — all mediated and all preserved.
What collapses is the trusted mediation-chain CRATE SEAMS, not a trust boundary.
Fixed intro, §2 (retitled + enumerates the boundaries), §5.4, §8 invariants
(added lane + data boundaries; fixed stale RowBackend/TurnStore refs to
RootFilesystem), and §11.6 (lane/worker/egress adversarial). Renumbered Open
questions to §12.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* docs(reborn): fix last 'the one trust membrane' → the loop's (one of several)
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* docs(reborn): consistency pass + address PR review feedback
End-to-end review fixes (also addressing gemini/coderabbit comments; note some
reviewed a superseded head that still proposed RowBackend/TurnStore<B>):
- authorize() no longer takes a separate `scope` that can diverge from
Invocation.scope — derive from inv.scope (§3, §5.3). [coderabbit]
- Clarify the result contract: Outcome carries success OR recoverable failure;
the seam is Result<Outcome, Blocked>; no separate Err(terminate) (§3). [coderabbit]
- Mark `Authority` SEALED — private fields, host-only construction via authorize()
(§3, §5.3); resolve open-question 2 accordingly. [gemini + coderabbit]
- Align §4.4 LOCAL_DEV process field with §5.6/§6: HostUnsandboxed(LocalOnly),
gated by a local-only token a served boot can't mint.
- Retire the RowBackend framing (superseded by RootFilesystem) and note deleting
InMemory*Store has zero persistence-compat impact; durable backends untouched
(§4.3). [gemini/coderabbit]
- Add file:line evidence for the five request types + audit date/window (§1.1).
[coderabbit]
- Intro: "most with exactly one prod impl" -> "several" (only 2 of 5).
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* docs(reborn): §12 — performance-critical paths (locking, remote-store latency, event fan-out)
Adds a performance section grounded in the actual hotspots: consolidating stores
onto RootFilesystem (§4.3) makes the backend latency profile the kernel's, and it
can be remote (libSQL/Postgres). Ranked critical-path table (per-turn ~11-store
fan-out; heartbeat vs store-lock with lease-TTL coupling; libSQL BEGIN IMMEDIATE
single-writer + #5751/#6089 contention; authorize() per-tool-call reads; active-
thread lock; event append/projection fan-out; recovery poll). Plus the
no-lock-across-remote-I/O rule (turn_scheduler.rs:787/881), what the refactor
helps vs risks (centralizing latency/writer contention onto one seam), and design
guidance (batched per-transition write, isolated heartbeat, cached read-mostly
authority, async coalesced events, writer sharding). Renumbered Open questions
to §13.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* docs(reborn): §13 — answer the open questions directly (now Decisions)
1. TurnRun vs ironclaw_processes: converge the mechanism (shared LeasedWorkUnit —
§4.3 already collapses the store layer), keep the policy distinct (turn resume-
from-checkpoint vs process terminal+re-spawn); bonus, the shared lease-recovery
gives processes the reconciler they lack.
2. Authority: one sealed value (host-only construction via authorize()); narrowed
read projection if an adapter needs a field.
3. DeploymentConfig: surface-disclosure derived from process:HostUnsandboxed;
mid-run refresh gated by session:LongLived|PerRun; synthetic tools promoted to
first-party capabilities (default hidden in hosted) — a security improvement
(they gain authorize/scope-binding). Remaining items are tuning knobs, not
architecture.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* docs(reborn): cross-reference the in-progress Unified Extension Runtime (BenKurrek gist)
Adds §5.9 mapping this doc against the URT extension/adapter/auth design: strong,
independent convergence (no-product-code-in-composition; config-not-code as
recipe+engine auth; runtime-kind=closed lane set; built-ins on the identical
pipeline; the trust boundaries; ProductSurface above the host pipelines).
Complementary scope: URT is the deep extension/adapter/auth axis, this doc the
broader kernel refactor; they compose (URT's dispatcher pipeline = authorize+
dispatch; adapter invoke/deliver = RuntimeLane execution).
Adopts two URT refinements: (a) product_auth collapses to recipe data + one host
AuthEngine, not per-adapter code (§5.8); (b) the Deletion/Addition/retired-
taxonomy tests are the products-in-composition ratchet (§10). Clarifies WebUI
consumes ProductSurface directly and is not a ChannelAdapter. Adds a reference.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* docs(reborn): §5.9 — RuntimeLane reconciliation + ToolPorts↔Authority integration points
Name the two seams where this doc and the Unified Extension Runtime must agree:
1. One closed execution enum RuntimeLane = {FirstParty|Wasm|Mcp|Process}; the URT's
extension-declarable runtime *kinds* (first_party/wasm/mcp) are a strict subset.
Process (OS-subprocess/script sandbox) is host-only — no manifest can select it;
only host built-ins (shell/script) dispatch to it via ProcessSandbox. Load kind
(URT) vs execution lane (this doc) are different axes; don't merge them.
2. ToolPorts is derived from Authority, never independent: dispatch() materializes
egress (NetworkPolicy + host-side SecretBroker lease), state (ScopedFilesystem =
Authority.mounts), logging from (&Invocation, &Authority, descriptor). ToolPorts
can't be wider than Authority grants; the adapter never sees Authority itself. So
URT's ToolAdapter::invoke(call, ports) IS the body of dispatch(inv, auth, lane).
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* docs(reborn): address CodeRabbit review findings on the latest head
Nine substantive design-contract fixes:
- §3 core model: LoopRequest (loop pre-trust, input-by-ref) resolved to Invocation
at the membrane; Authorized = sealed AND invocation-bound (actor/scope/activity_id
provenance) so dispatch can't be handed a mismatched (inv,auth); activity_id IS
the invocation idempotency identity (idempotency_key unified with it, not deleted,
satisfying §11.3); three distinct outcome channels Blocked | HostFailure | Outcome
(no Ok(Failed)/Err ambiguity). Type count 3→4. §3.1/§5.3/§5.4/§11.1 aligned;
Authority→Authorized throughout.
- §11.2/§6: scope cross-tenant isolation to multi-user/served deployments (matrix
test), not "any deployment state" (single-user local legitimately allows host proc).
- §9/§10: quarantine the known-red two-user test (#[ignore]/expected-fail until the
fix merges); ratchets freeze checked-in symbol allowlists (set membership), not
aggregate counts (a swap evades a count).
- §12: durable event append is atomic with the state transition (same tx/outbox);
only subscriber fan-out is decoupled.
- §5.8: adapters resolved via a product-neutral ExtensionId-keyed factory registry
passed to composition as input; config lists ids, not types.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* refactor(reborn): approval stores over RootFilesystem, delete InMemory*Store (§4.3) (#6195)
* refactor(reborn): approval stores over RootFilesystem, delete InMemory*Store (§4.3)
First slice of the architecture-simplification note
(docs/reborn/2026-07-17-architecture-simplification-dto-dyn-local.md §4.3):
"in-memory" stops being a bespoke store and becomes a filesystem backend, so
each approval domain has one production Filesystem*Store<F> exercised over the
in-memory backend in tests and libSQL/Postgres in production — no parallel
hand-written implementation to keep in lock-step.
Deletes the three hand-written approval stores in ironclaw_approvals
(InMemoryAutoApproveSettingStore, InMemoryPersistentApprovalPolicyStore,
InMemoryCapabilityPermissionOverrideStore, plus the InMemoryToolPermissionOverrideStore
alias). Everything now runs the existing Filesystem*Store<F>:
- ironclaw_approvals: adds a `test-support`-gated helper module with
in_memory_backed_* constructors (the production store over a fresh
InMemoryBackend mounted at /approvals). The stores' own unit tests move onto
Filesystem*Store<InMemoryBackend>, proving it covers the deleted stores' cases.
- composition factory.rs: the LocalDev* approval-store aliases collapse to one
unconditional Filesystem*Store<LocalDevRootFilesystem>; the no-durable-features
local-dev builder wires them over the composite root filesystem (in-memory
backed) via the existing scoped-filesystem path instead of the deleted
InMemory* stores. wrap_scoped / invocation_mount_view and the /approvals mount
machinery are un-gated so both builders share one path.
- host_runtime production-wiring guard: the fail-closed LocalOnly classification
now keys on FilesystemPersistentApprovalPolicyStore<InMemoryBackend> instead of
the deleted InMemory type. Production (<LibSql>/<Postgres>) and durable-local-dev
(<Composite>) classifications are unchanged; the guard contract test is
repointed and still asserts LocalOnly.
- downstream test suites (host_runtime, composition, product_workflow) repoint to
the test-support helpers; the affected crates enable ironclaw_approvals/test-support
in [dev-dependencies].
Net subtractive (−136 LOC). No trust boundary or persistence-compatibility change:
the in-memory approval stores were volatile/local; the durable libSQL/Postgres
backends are untouched. Boundary tests (ironclaw_architecture) stay green.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* refactor(reborn): address review — migrate root harness, honest volatile approval-store type
Two review findings on the approvals-store consolidation:
1. Root integration harness left uncompilable. `tests/integration/support/
harness/mod.rs` still constructed the deleted `InMemory*Store`s (as
`Arc<dyn …>` defaults). Repoint to the `in_memory_backed_*` helpers and enable
`ironclaw_approvals/test-support` in the root `[dev-dependencies]`.
2. Guard weakened for the no-durable composition. The composite-unified alias made
the no-durable-features build wire
`FilesystemPersistentApprovalPolicyStore<CompositeRootFilesystem>`, whose
TypeId misses the guard's `<InMemoryBackend>` branch, so the volatile store was
classified `ProductionCandidate`. Fix by making the store type honestly reflect
its volatility: the no-durable build now backs the three approval stores with a
dedicated `InMemoryBackend` directly (via `wrap_scoped`), so the concrete type
is `Filesystem*Store<InMemoryBackend>` — which the production-wiring guard
classifies `LocalOnly`, exactly as the sibling `InMemoryRunStateStore` /
`InMemoryCapabilityLeaseStore` are. Durable builds keep the composite-backed
type (distinct, correctly a production candidate). The `LocalDev*` approval
aliases go back to cfg-split (InMemoryBackend vs composite); the guard contract
test now documents that it exercises the exact type the no-durable composition
wires. `local_dev_scoped_filesystem` is re-gated to durable-only (the no-durable
builder no longer uses it); `wrap_scoped`/`invocation_mount_view` stay ungated
since the no-durable builder now calls `wrap_scoped` directly.
Verified: composition compiles + clippy clean on default (no-durable) and libSQL;
guard contract test green; local_dev_authorization tests green; root
reborn_integration_* targets compile.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* refactor(reborn): consolidate WebUI host stack into a single ironclaw_webui crate (+ Slack/OpenAI-compat wiring) (#6194)
* refactor(reborn): merge WebUI host stack into ironclaw_reborn_webui_ingress
Fold `ironclaw_webui_v2` (route surface + SPA bundle) and composition's WebUI
middleware/assembly into `ironclaw_reborn_webui_ingress` so the whole WebUI host
stack is one crate above composition, and composition shrinks.
Move-only for behavior; the composed `webui_v2_app` Router, middleware order,
descriptors, and security invariants are unchanged (locked by the moved contract
tests + the composition/ingress router tests, all green under default features).
Structure:
- `ironclaw_webui_v2/src/*` -> ingress `src/webui_v2/` (public module,
unconditional); `build.rs` + `frontend/` moved to ingress; crate deleted and
removed from workspace members (68 -> 67).
- Composition WebUI middleware (`webui_body_limit`, `webui_operator_auth`,
`webui_rate_limit`, `webui_route_match`, `webui_ws_origin`) + `webui_serve.rs`
-> ingress `src/`.
- `webui_serve.rs` split: `WebuiServeConfig`/`webui_v2_app`/`WebuiV2App`/
`Webui{Serve,Config}Error`/`WebuiAuthenticator`/`WebuiAuthentication` move to
ingress; the mount vocabulary (`PublicRouteMount`/`ProtectedRouteMount`/
`PublicRouteDrain(s)`) stays in composition (`webui/route_mounts.rs`) because
nearai/openai/runtime construct it — moving it up would cycle.
- Product-auth decoupled: `ProductAuthRouteState`, `product_auth_route_mount`,
`ProductAuthRouteMount` exposed `pub` + re-exported from composition root;
ingress imports them (+ `RebornWebuiBundle`, `GoogleOAuthRouteConfig`) via the
composition facade. Composition no longer depends on `ironclaw_webui_v2`.
- Callers repointed: composition tests, ingress tests, reborn_cli
(serve/webui_auth), root v1 int-tier tests + dev-dep, Dockerfile.reborn +
smoke test frontend path, and the ironclaw_architecture boundary spec.
Deferred (out of scope, feature-gated off by default): the
`slack-v2-host-beta` / `openai-compat-beta` blocks in `webui_serve.rs` still
reference composition-internal surfaces and compile out under default features
(declared as known cfgs). Wiring the Slack/OpenAI-compat host surface through
ingress is a follow-up; composition's slack feature will not build until then.
[skip-regression-check] move-only refactor; behavior covered by relocated
contract tests and existing composition/ingress router suites.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* fix(reborn): make Slack + OpenAI-compat host-beta build and wire after WebUI merge
The WebUI host-stack merge (parent commit) hoisted `webui_v2_app` + its config,
authenticator, and middleware surface from `ironclaw_reborn_composition` up into
`ironclaw_reborn_webui_ingress`, but left the `slack-v2-host-beta` /
`openai-compat-beta` blocks in the moved `webui_serve.rs` pointing at
`crate::slack::*` / composition internals that don't exist in ingress. Those
features were declared only as known-cfgs and compiled out, so:
- composition failed to build under `slack-v2-host-beta` (two mount-vocabulary
imports still on the old `webui::webui_serve` path);
- the ingress serve blocks were permanently dead, so the CLI's slack/openai
features forwarded to composition but never mounted the Slack routes —
a functional parity break, not just a compile break;
- composition's slack-gated tests still imported the moved `webui_v2_app`.
Wiring (behavior-preserving; restores pre-merge parity):
- ingress now defines real `slack-v2-host-beta` / `openai-compat-beta` features
that forward to composition (+ optional `ironclaw_reborn_openai_compat`); the
moved `webui_serve.rs` reaches Slack setup/route types and the
OpenAI-compat bearer-evidence helper through composition's public facade
(`ironclaw_reborn_composition::{SlackPersonalSetupServiceSlot,
SlackChannelRouteAdminRouteConfig, slack_channel_route_admin_route_mount,
SlackPersonalOAuthBindingConfig, mark_bearer_token_verified_for_tenant}`).
Ingress does NOT depend on `ironclaw_product_adapters` directly — the
architecture boundary (`reborn_dependency_boundaries.rs`) forbids it, so the
evidence helper is re-exported from composition instead.
- composition promotes `slack_channel_route_admin_route_mount` + its
`SlackChannelRouteAdminRouteMount` return type to `pub` (its sole caller,
`webui_v2_app`, moved up), mirroring the already-public `ProtectedRouteMount`.
- CLI forwards `slack-v2-host-beta` / `openai-compat-beta` to the ingress crate
as well as composition, so the serve blocks compile in and the routes mount.
Tests:
- The 7 composition slack unit tests that drove the now-relocated `webui_v2_app`
move to `ironclaw_reborn_webui_ingress/tests/slack_host_beta_webui_v2.rs`.
They use only composition's public builders, so ingress (which normal-deps
composition — single crate copy, no dev-dep cycle) is their correct home; a
composition lib-test cannot call the ingress `webui_v2_app` without cargo
building two incompatible copies of composition. composition's ingress
dev-dep gains `slack-v2-host-beta` so its own `webui_v2_product_auth*` tests'
`with_slack_*` blocks compile.
Verified (clean env): composition/ingress/cli build + clippy `-D warnings` under
both beta features; ingress `--all-features` suite green incl. the 7 relocated
tests; composition lib (1589) + router (webui_v2_serve 44 / product_auth 51) +
cli (440 incl. Dockerfile smoke) green; `ironclaw_architecture` boundaries hold.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* refactor(reborn): rename crate ironclaw_reborn_webui_ingress -> ironclaw_webui + doc pass
Now that the crate owns the whole WebUI host stack (route surface + SPA +
gateway assembly/middleware + serve loop + host auth), "reborn_webui_ingress"
undersells it. Rename the crate to `ironclaw_webui` and refresh its docs to
describe the composed subsystems.
Rename (pure identifier swap, no behavior change):
- `git mv crates/ironclaw_reborn_webui_ingress crates/ironclaw_webui`; package
`name` + workspace members + root dep alias updated.
- Every `ironclaw_reborn_webui_ingress` reference repointed across Rust, Cargo
manifests, Cargo.lock, Dockerfile.reborn, CI scripts (.sh/.py), the
`ironclaw_architecture` boundary spec (crate_name / forbidden lists / layer
exception / source-path prefixes), root + crate CLAUDE/AGENTS docs, .claude
rules & skills, and the security-parity docs. `openwiki/` (auto-generated) and
`docs/plans/` (historical) intentionally left for their own regen/record.
Docs (README.md new; AGENTS.md + CLAUDE.md restructured):
- README.md: human-facing overview with the three-piece fold-in map
(route surface + SPA from the former `ironclaw_webui_v2`; gateway assembly +
middleware from `ironclaw_reborn_composition::webui`; serve loop + host auth
from this crate's original scope), layering/boundaries, feature flags, build/test.
- AGENTS.md: replaced the stale "deliberately small" framing with an accurate
agent map — composed subsystems, do-not-move-in, allowed deps, how to add a
route / authenticator / OAuth provider.
- CLAUDE.md: reframed opening (it no longer is a "counterpart to webui_v2_app" —
that fn lives here now); Surface table gains the route/gateway symbols
(`webui_v2_router`, `webui_v2_routes`, `WebUiV2State`, `WebUiV2HttpError`,
`webui_v2_app`, `WebuiServeConfig`); folded in the WebChat v2 route table +
streaming/SSE model + SPA build detail; test layout now lists the
route-surface/gateway suites. OAuth login security contract retained verbatim.
Verified: `cargo metadata` resolves; `cargo build -p ironclaw_webui` and
`-p ironclaw_reborn_cli --features slack-v2-host-beta,openai-compat-beta` green;
`cargo test -p ironclaw_architecture reborn` (boundaries, new name) green;
`cargo test -p ironclaw_webui --features slack-v2-host-beta --test
slack_host_beta_webui_v2` green; 0 stale `ironclaw_reborn_webui_ingress` refs
outside openwiki/docs-plans.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* fix(reborn): address PR review findings + stale ironclaw_webui_v2 refs post-merge
Review findings on PR #6194 (gemini-code-assist + ironloopai) and the leftover
references the `ironclaw_webui_v2` → `ironclaw_webui` fold-in left behind.
CI / build path migration (ironloopai "path migration incomplete"):
- Repointed the deleted `crates/ironclaw_webui_v2/frontend` build path to
`crates/ironclaw_webui/frontend` across all workflows (code_style, coverage,
ironclaw-stress, platform-and-compat, reborn-e2e, reborn-playwright),
`.dockerignore`, `scripts/run-reborn-webui.sh`, `scripts/ci/quality_gate_strict.sh`,
and the `regression-test-check.yml` frontend-test detector.
- Test bucketing: dropped the dead `ironclaw_webui_v2` entries from
`reborn-crate-test-buckets.sh` + `package-feature-flags.sh` (the renamed
`ironclaw_webui` entries already exist), repointed `classify-test-scope.sh`,
and widened the `reborn-tests.yml` jq filter to `startswith("ironclaw_webui")`
so the folded crate's tests still land in the webui bucket.
- QA inventory (`scripts/reborn_qa_matrix/audit_surface_inventory.py`) now reads
`crates/ironclaw_webui/src/webui_v2/descriptors.rs`.
- Regenerated `harness/latency/runner/Cargo.lock` (transitively referenced the
deleted crate via composition's `webui-v2-beta`).
Broken doc links / stale comments (gemini):
- `nearai_login_serve.rs` + `runtime.rs`: the broken intra-doc link
`crate::webui::route_mounts::WebuiServeConfig` (type moved out of composition)
is now a plain code span `ironclaw_webui::WebuiServeConfig::with_public_route_mount`
— composition cannot link into `ironclaw_webui` (not a dependency).
- `webui/facade.rs`: comment now says routing/auth/static/SSE live in
`ironclaw_webui`; only the route-mount vocabulary stays in `route_mounts`.
build.rs frontend opt-out (gemini):
- `SKIP_FRONTEND_BUILD=1` skips the Node/pnpm frontend build for backend-only
dev / docs.rs / minimal CI images (`webui_enabled = env::var_os(...).is_none()`).
Guidance docs (ironloopai "update AGENTS/CLAUDE + crates/AGENTS.md"):
- `crates/AGENTS.md`: rewrote the `ironclaw_webui` row to the whole WebUI host
stack, removed the deleted `ironclaw_webui_v2` row, repointed cross-refs.
- Refreshed `ironclaw_webui_v2` → `ironclaw_webui` across living guidance
(`.claude/` rules/skills/commands, `crates/README.md`, `crates/Architecture.md`,
`crates/ironclaw_projects/CLAUDE.md`, `ironclaw_reborn_composition/CLAUDE.md`,
product_workflow comments, security-parity docs) and the `-p ironclaw_webui_v2
--features webui-v2-beta` commands. `ironclaw_webui_v2_static` (a distinct,
still-live v1 crate) left untouched.
Already addressed earlier in this PR, confirmed still green post-merge:
- Slack / OpenAI-compat host-beta compile + wiring (the `slack-v2-host-beta` /
`openai-compat-beta` findings) — commit `a2ed602`.
- The obsolete `/v2` SPA mount — replaced by main's root-serving
`static_router_with_config` in the merge (`c000a16`).
Verified: clippy `-D warnings` on composition (`webui-v2-beta`) and
`ironclaw_webui` (`--all-features`); root int-tier webui tests compile;
QA-inventory path resolves; harness lock clean.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* fix(ci): rustfmt import ordering after crate rename + refresh composition pub-use snapshot
Two CI failures on PR #6194:
- **Formatting / Code Style (fmt+clippy)**: the `ironclaw_reborn_webui_ingress`
→ `ironclaw_webui` rename shifted where the crate sorts in `use` blocks, so
rustfmt wanted to reorder imports across ~26 files. I had wrongly reverted
those fmt-only files during the rename commit (assuming rustfmt-version
drift); the reordering is deterministic and CI's gate caught it. Ran
`cargo fmt --all`.
- **Test Reborn crate bucket (adapters-misc)** →
`composition_public_pub_use_surface_matches_snapshot`: this PR intentionally
changed composition's public facade — `webui_serve`/`Webui*`/`webui_v2_app`
moved out to `ironclaw_webui` (so composition's `webui` re-export is now just
`route_mounts::*`), the product-auth mount builders were exposed, and the
Slack channel-route mount + `mark_bearer_token_verified_for_tenant` were
promoted. Regenerated `docs/plans/composition-pubuse.snapshot` by replaying
the test's own `extract_pub_use_surface` extraction; the diff is exactly those
intended facade changes.
Verified: `cargo fmt --all -- --check` clean; `cargo test -p
ironclaw_architecture --test reborn_composition_boundaries` green (8 passed).
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* refactor(composition): extract runtime.rs inline test module (Phase 0) (#6173)
* refactor(composition): extract runtime.rs inline test module to sibling file
Moves runtime.rs's trailing `#[cfg(test)] mod tests { … }` (~6.9k lines) into
`runtime/tests/core.rs` via the crate's existing `#[path = "runtime/tests/…"]`
convention. Pure move — the module keeps its identity (`crate::runtime::tests`),
so all `super::`/`crate::` refs resolve unchanged; cargo fmt de-indented the
relocated items. runtime.rs: 11,673 -> 4,709 lines.
Phase 0 of the composition decomposition (parent #4471, plan #6168): single-
crate, zero cross-crate coupling, does not touch slack/ or extension_host/.
Fixes the crate's worst file-size violation and — because the inline test block
no longer counts as production LOC (it's now a test-only file, excluded) —
ticks the composition mass ratchet down (~23.98% -> ~23.2%).
Verified: `cargo test -p ironclaw_reborn_composition --all-features --no-run`
compiles all relocated tests unchanged.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* fix(composition): rustfmt runtime test module + merge main
Remove stray leading blank line in runtime/tests/core.rs flagged by the
Formatting CI check, and merge origin/main to bring the branch current.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* Require read-before-edit and reject stale edits in reborn coding tools (#5978)
* Ride out provider outages and drop the 32-call turn cap in the reborn loop
Two failure modes discovered via claw-swe-bench-lite run 9ca133e5 (30% vs
hermes 65% on the same model) discarded hours of agent work:
- A transient provider 5xx storm aborted the whole run after 2 quick
retries (max_attempts_per_class=2, backoff capped at 5s). Availability-
class model errors (transient/unavailable/internal) now retry on their
own deeper budget: max_model_availability_attempts=12 with a 1s..60s
exponential backoff, riding out ~7 minutes of sustained provider
failure. MAX_MODEL_RETRIES raised 8 -> 16 to let the strategy govern.
- DefaultBudgetStrategy's iteration_limit=32 failed closed mid-task with
no synthesis (llm_calls in failed bench tasks clustered at exactly
63/64/127/128). The default is now DEFAULT_ITERATION_BACKSTOP=1024
(subagent 16 -> 256), documented as a runaway backstop: operational
bounds are the resource budget system and stop-condition strategy.
New seam mirroring IRONCLAW_REBORN_PLANNED_DEFAULT_ITERATION_LIMIT:
IRONCLAW_REBORN_MODEL_AVAILABILITY_RETRY_ATTEMPTS ->
DefaultPlannedRuntimeConfig.planned_model_availability_retry_attempts ->
families::default_with_overrides. The integration group harness pins
attempts=1 so scenarios that deliberately script provider failures
(failure_category_demasked) reach Failed in seconds, not minutes; the
availability-retry tests run under paused tokio time.
Family fingerprint digests regenerated for the new strategy parameters.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* Surface tool-failure reasons to the model for shell and coding tools
Benchmark traces showed the model retrying identical failing calls blind:
builtin.shell parameter errors and coding-tool path rejections reached it
as a bare category ("the tool input could not be encoded") because the
handlers built FirstPartyCapabilityError/CodingCapabilityError with no
safe_summary — the model-visible Diagnostic detail channel downstream was
already wired but starved (one agent burned 13 apply_patch calls against
an out-of-scope /testbed path with empty errors).
- shell.rs: shell_error/process_error now carry the concrete reason
("missing 'command' parameter", timeout duration, spawn failure),
bounded to 512 chars. The strict safe-summary validator still falls
back to the fixed category string; the reason always survives on the
secret-scrubbed diagnostic channel.
- coding/paths.rs: scoped-path rejections name the offending path and
the available scoped roots; permission rejections say the operation is
not permitted on that mount.
Covered at the dispatch tier (coding state dispatch, host-runtime
invoke_capability) per test-through-the-caller.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* Run agent_loop scenario test targets under paused tokio time
The deep availability-retry backoff added for provider-outage ride-out
made outage-scripting scenario tests sleep for real: safety_nets alone
took ~423s (the exact cumulative backoff schedule) because scripted or
script-exhausted model errors now retry for minutes. Pause the clock on
all executor scenario targets — they drive the in-process mock host
exclusively, so timers auto-advance and the suites return to seconds.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* Fail fast when no LLM provider is configured instead of riding availability retries
The placeholder unconfigured provider's RequestFailed was mapped through the
catch-all Unavailable arm, so the new deep availability retry budget rode a
permanent configuration fault through ~7 minutes of exponential backoff.
Users with no LLM configured waited minutes for an error that retrying can
never fix, and the Reborn CLI smoke tests that pin fast nonzero exits timed
out (the 4 failures on CI run 29136954176).
Map errors carrying the shared UNCONFIGURED_PROVIDER_ID to
CredentialUnavailable, which is unclassified in loop recovery and therefore
terminal on first sight; the Settings → Inference hint travels on the
scrubbed detail channel. The provider id moves to a shared constant in
ironclaw_llm so the composition placeholder and the runner mapping cannot
drift.
Regression tests: unconfigured_provider_error_maps_to_credential_unavailable_
not_availability and unconfigured_provider_detection_requires_the_placeholder_
provider_id in model_gateway.rs; the existing smoke tests
(*_exits_nonzero_when_runtime_does_not_produce_reply) pin the fast-fail at
the caller tier.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* Derive override-built default-family replay identity from resolved config
families::default_with_overrides swapped budget/recovery strategies but
reused the planner's static version digest, so an overridden composition
carried the pure-default replay identity — violating the component-identity
contract (family.rs: the digest identifies replay-relevant configuration).
- Turn the cfg(test) fingerprint const into a runtime
default_family_fingerprint(iteration_limit, model_availability_attempts)
builder; override-built families hash it with their resolved values at
composition time (BLAKE3, same path as the pinned const). The pure-default
composition keeps the static DEFAULT_FAMILY_DIGEST, and overrides spelling
out the production defaults hash to that same digest.
- Collapse the two Option args into a FamilyOverrides struct and drop the
now-dead (None, None) branch in the runner's registry factory.
- Tests: digest differs per override knob and is deterministic; explicit
production defaults reproduce the static digest; an attempts=1 override
reaches the composed recovery strategy (one retry then abort).
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* Let the recovery strategy own the model retry guard; wake backoff on cancel
Two model-stage fixes from the PR 5959 review:
MAX_MODEL_RETRIES=16 silently capped any configured availability budget of
16+: the retry loop fell through to a generic ModelError exit with
FailedExitDetails::default() — no failure category, no diagnostic ref —
before the strategy could reach its own Abort. The executor now derives the
loop bound from the composed strategy via
RecoveryStrategy::max_total_model_attempts() (DefaultRecoveryStrategy
computes it from its per-class + availability budgets with margin), so
every accepted override reaches the strategy's abort boundary. The
contract-bug fall-through now carries the last observed model error's
category and diagnostic ref instead of empty details.
The availability backoff sleep (up to 60s per attempt) was not
cancellation-aware: a cancel request could wait out the full delay. The
sleep now selects over the host's cancellation_requested() future (same
pattern as the prompt-compaction and failure-explanation waits), and a
boundary cancel check right after the alteration turns the wake into a
checkpointed Cancelled exit without issuing another model call.
Tests (paused tokio time): an availability budget of 20 — past the old
executor cap — fails with the strategy's model_unavailable category and
diagnostic ref after exactly 21 model calls; cancellation requested during
the first 1s backoff exits Cancelled without riding out the sleep.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* Clarify DEFAULT_ITERATION_BACKSTOP doc: resource budgets are not yet enforced
The doc claimed operational bounds come from the resource budget system,
but ResourceBudgetPolicy.max_model_calls and the wall-clock cap are defined
and not applied; until they are, this backstop and the stop-condition
strategy are the only live ceilings.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* Carry tool-failure reasons to the model past the strict summary validator
PR 5959's headline feature (model-visible tool-failure reasons) never
reached the model for path-bearing reasons: LoopSafeSummary rejects
path/payload delimiters and newlines, and dispatch_failure_message
silently degraded every such reason to the generic category sentence
before it could reach the diagnostic channel.
- production.rs (failure_from): a host-authored safe_summary that fails
LoopSafeSummary validation is preserved as the new
DispatchFailureDetail::Diagnostic instead of being dropped; the
message keeps the fixed category sentence (host-authored, Invariant 2).
- capability_port.rs: maps the Diagnostic detail into the model-visible
CapabilityFailureDetail::Diagnostic, scrubbing secret values and
normalizing control characters the observation validator rejects (so
one stray escape byte cannot drop the whole observation); newlines
are preserved. The RetrySameCall arm now forwards structured detail
too.
- coding/paths.rs: scoped-path rejection summaries render the path and
available roots delimiter-free ("path testbed replacer.go",
"available roots: workspace") so they pass the strict validator —
FilesystemDenied surfaces as a Denied loop outcome whose only
model-visible channel is the summary itself.
- shell.rs: bounded_failure_reason documents the (now real) diagnostic
flow; truncation remains char-based (no byte-boundary panics).
Regre…
ilblackdragon
added a commit
that referenced
this pull request
Jul 21, 2026
…spatch-through-witness (#6396) (#6432) * refactor(reborn): thread real InvocationOrigin + Actor::System into the seal — witness always-present (§5.2.1/§9, S1) First slice of the "witness always-present + dispatch-routes-through-witness" arc (#6396). Retires the two fakes in `CapabilityHost::seal_authorization` so the sealed `Authorized` witness carries authoritative facts for every dispatchable invocation, WITHOUT changing any authorization decision: - Origin: deleted the `Product(ProductKind::new("provisional"))` placeholder. Added an ingress-stamped `ExecutionContext.origin: Option<InvocationOrigin>`; the seal resolves `context.origin` → else `LoopRun(run_id)` (transitional compat) → else fail-closed (Option `?`, never a placeholder). The loop host (`invocation_context_from_visible`) now stamps `LoopRun` explicitly. - Actor: an actor-less/host-internal context now seals `Actor::System` as its own class instead of early-returning `None`, so the witness is minted for it too. The witness is `None` now only for the `System` runtime-kind (no lane). Behavior-neutral: the witness is still a forward-looking artifact not consumed by dispatch (that lands in a later slice), so origin/actor become accurate recorded facts on the sealed `Invocation` without influencing allow/deny/gate. Verified there is no production `Product`/`Automation` capability ingress today (invocation is loop-initiated; the former `"provisional"` fallback only ever fired for test contexts), so those origin variants are wired for future ingresses with no live producer yet. Tests: extended the seal allow-path test to assert the real origin; added `authorize_seals_system_actor_and_real_origin_across_ingresses` (System actor + Product/LoopRun/Automation origins driven through `authorize`); extended the loop-host context test to assert the stamped `LoopRun` origin. No test weakened. Gates: cargo fmt; cargo test -p ironclaw_capabilities -p ironclaw_host_api -p ironclaw_host_runtime -p ironclaw_loop_host (1796 pass); clippy -p ironclaw_capabilities -p ironclaw_host_api --all-targets --all-features -D warnings; cargo test -p ironclaw_architecture. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * refactor(reborn): add OriginGatePolicy matrix as declarative capability data (§5.2.1, S2) Second slice of #6396. Introduces the §5.2.1 origin→gate matrix as declarative data threaded from the manifest to the kernel descriptor, with NO authorization logic reading it yet (behavior-neutral): - host_api: `OriginGatePolicy` (Forbidden/AskAlways/GatedUnlessGranted/ ConsentSufficient/Ungated; wire-stable snake_case; default Forbidden) and `OriginGateMatrix` (per-origin loop_run/product/automation, each defaulting to Forbidden) with `policy_for(&InvocationOrigin)`. `CapabilityDescriptor` gains `origin_gate_matrix: Option<OriginGateMatrix>` — `None` = undeclared, treated as all-Forbidden (fail-closed) and targeted by the §5 ratchet (S5). - manifest: `CapabilityDeclV2` and `RawCapabilityV2` gain the same optional field (`#[serde(default)]`, so existing TOML without the key parses to `None`; a partial matrix defaults omitted origins to Forbidden). `from_raw` passes it through. Threaded into both descriptor-mint sites (`capability_descriptors_from_manifest`, hosted-MCP discovery). Absence = Forbidden (deny-by-default). No real matrices are populated here — that is the next slice; every fan-out literal (22 sites, mostly tests) is `None`. Tests: OriginGatePolicy snake_case round-trip; OriginGateMatrix per-origin default-Forbidden; policy_for variant mapping; v2 manifest parse (absent key → None, partial matrix → omitted origin Forbidden). No authz test touched. Gates: cargo fmt; cargo build --workspace; cargo test -p ironclaw_host_api -p ironclaw_extensions; clippy --all-targets --all-features -D warnings; cargo test -p ironclaw_architecture. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * refactor(reborn): populate origin_gate_matrix for every capability + seed Ungated allowlist (§5.2.1, S3) Third slice of #6396. Declares the per-origin gate matrix on every production capability, behavior-preservingly (mirrors today's effect-based LoopRun gate). Still behavior-neutral: nothing reads the matrix yet (the fold is S4). - `loop_run` mirrors current gating under the canonical LocalDev/AskDestructive profile (the only live LoopRun producer). Since `ask_writes` ≡ `ask_destructive`, GATED-today ⇔ effects reach beyond {read_filesystem, dispatch_capability}; nothing is hard-floored, so no AskAlways. Result: 17 builtin caps `Ungated`, the other 160 (19 builtin + all 141 extension — every extension carries `network`) `GatedUnlessGranted`. - `product`/`automation` = `Forbidden` everywhere: deny-by-default, no live producer today; a later reviewed ingress slice fills them. Behavior-neutral now. - Checked-in `UNGATED_LOOP_RUN_CAPABILITIES` allowlist (host_api) pins the 17 ungated-for-model builtins (§5.2.1/§10 seed; the S5 ratchet enforces it). Builtin `loop_run` derives from this allowlist via `builtin_loop_run_seed` — fail-safe: an unlisted builtin defaults to GatedUnlessGranted. - Extension matrices are hand-authored into the 13 first-party TOML assets (the production descriptor source; gsuite Rust builder is test-only), so the ratchet does real work there. Discovered hosted-MCP tools inherit from their template. Load-bearing invariant (allowlist ⇔ builtin loop_run==Ungated) verified by construction + tests; every in-scope descriptor now declares Some(matrix); test fixtures stay None; SyntheticCapabilityDescriptor untouched. Gates: cargo fmt; cargo build --workspace; cargo test across host_api/extensions/ composition/host_runtime (one unrelated pre-existing env-sensitive failure, detect_env_llm_*, caused by LLM_BACKEND set in the shell); clippy --all-targets --all-features -D warnings clean; cargo test -p ironclaw_architecture 10/0. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * refactor(reborn): fold origin→gate matrix into authorize() as a two-tier gate (§5.2.1/§5.2.7, S4) Fourth slice of #6396. Authorization now CONSULTS the origin→gate matrix, keyed on the invocation's resolved InvocationOrigin (context.origin, else LoopRun from run_id — same resolution as seal_authorization) and the descriptor's origin_gate_matrix. Composed inside require_approval_for_profile_policy at the class-A intrinsic-gate layer, so per-scope class-B state (leases, auto-approve, always-allow) stays above it unchanged. Provably behavior-neutral in production. Two gate tiers, each mapped onto the existing gate machinery so semantics are correct — not just present: - `Forbidden` → hard Deny (sanitized DenyReason::PolicyDenied; internal reason in debug log), short-circuited ahead of every class-B step. - `AskAlways` → hard-floor gate composed at step 3 with effects_force_approval: beats class-B auto-approve/always-allow and is NOT suppressed by the Minimal (yolo) bypass — "every invocation gates; persistent grants never honored" (§5.2.7). Only a genuine one-shot approval lease satisfies it. - `GatedUnlessGranted` → soft gate OR'd at step 9: satisfied by the same scoped grant/always-allow machinery as the effect gate, and suppressed under the Minimal-bypass guard so yolo stays "no prompts". - `ConsentSufficient` / `Ungated` → no gate contribution. Behavior-neutral: no production capability declares loop_run == AskAlways or Forbidden (S3 seeds only Ungated / GatedUnlessGranted), so the hard-floor and deny paths are unreachable in production today; GatedUnlessGranted is neutral by the invariant matrix_gate ⟹ effect_gate under every non-Minimal profile, and is yolo-suppressed. The mechanism is fully wired and fail-closed for future Product/Automation ingresses. Tests (driven through the real RuntimeProfileApprovalGatePolicy + profile_approval_authorizer): neutrality across 5 caps × 4 profiles × 2 permission postures (LoopRun decision identical with vs without the matrix — zero flips); ask_always_matrix_gates_even_when_class_b_would_allow (the §5.2.7 proof: AskAlways gates despite global_auto_approve + always_allow); AskAlways gates under Minimal-yolo; GatedUnlessGranted suppressed under yolo; Forbidden→Deny; Ungated/ConsentSufficient add no gate. 27 prior profile-approval tests unchanged. Gates: cargo fmt; test -p ironclaw_reborn_composition -p ironclaw_capabilities -p ironclaw_host_api -p ironclaw_approvals (1678 pass; only the pre-existing env-sensitive detect_env_llm_* fails in this shell); clippy --all-targets --all-features -D warnings clean; cargo test -p ironclaw_architecture. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * test(reborn): ratchet the origin→gate matrix invariants + per-descriptor well-formedness (§5.2.1/§10, S5) Fifth slice of #6396. Locks in the S2–S4 origin→gate matrix invariants as guardrails; test-only, no production behavior change. New `crates/ironclaw_architecture/tests/reborn_origin_gate_matrix_ratchet.rs` (mirrors the sibling reborn_*_ratchet.rs pattern) enforces the invariants a leaf test crate can own soundly: - The `UNGATED_LOOP_RUN_CAPABILITIES` allowlist (imported from its owner, so the pin checks the REAL constant) is frozen to the reviewed 17-id seed — any addition/removal must update the checked-in EXPECTED_UNGATED_SEED in the same PR, so ungating a capability for the model is a review-visible diff. (This is the behavior-preserving grandfathered seed, deliberately not the doc's "starts empty" §10 ideal — documented in the file header; a future tightening removes entries with review.) - The 13 hand-authored first-party extension TOML assets each declare an origin_gate_matrix with a loop_run, never `consent_sufficient` on loop_run/automation (Product-only per §5.2.1), and no `ungated` loop_run outside the allowlist — the source where a typo could silently ungate a networked capability. "Every production descriptor declares Some(matrix)" (invariant 1) is enforced at the owning-crate build sites (ironclaw_architecture cannot depend on the composition/host_runtime crates that assemble descriptors): the builtin enumeration test in ironclaw_host_runtime (extended here to assert every builtin declares Some, loop_run matches the allowlist, product/automation Forbidden, no Product-only policy misused) plus the existing S3 extension-lifecycle and bundled-extension tests. The ratchet header cross-references all three. Proportionate §11.7 conformance: per-descriptor well-formedness/self-consistency checks added; a full golden-file origin→gate conformance harness (none exists today) is left as a follow-up. Dev-only deps on ironclaw_architecture (ironclaw_host_api, toml), documented as non-production edges. No STOP-item surfaced: no TOML uses ungated/consent_sufficient, no production descriptor is None. Gates: cargo fmt; cargo test -p ironclaw_architecture (10 pass, new ratchet 3/3); cargo test -p ironclaw_host_api -p ironclaw_extensions; the extended builtin enumeration test passes; clippy -p ironclaw_architecture --all-targets --all-features -D warnings clean. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * refactor(reborn): dispatch consumes the Authorized witness — single-use, expiry-fail-closed, byte-neutral (§5.3.2/§9, S6) Sixth/final slice of #6396 (its "Slice 2"). `invoke_json`/`spawn_json` now take the sealed `Authorized` out of the authorize fold and route dispatch through it instead of re-deriving from loose request fields: - The witness is consumed single-use via `Authorized::into_parts(now)` (moves it — a second dispatch is a compile error). Its sealed mounts/reservation drive dispatch; the lane is consumed as proof (the dispatcher owns closed-lane routing and re-derives the identical descriptor lane). - Expired witness at dispatch fails closed: aborts the prepared obligations (releasing the reservation through the obligation lifecycle), consumes the witness via `Authorized::abort` (never `Drop`), fails the run, returns a terminal denial. Unreachable in a synchronous authorize→dispatch today; new-behavior-by-design for the async/held-witness future. Byte-neutral: the witness carries the fold's `obligation_outcome.{mounts, resource_reservation}` verbatim, so dispatch inputs are identical to the pre-S6 path for every capability that seals a witness. Findings from the security-critical review (issue warned these paths hide masked behaviors): - Mounts kept as `Option<MountView>` end-to-end (witness `mounts()` → `Option<&MountView>`; seal no longer collapses `None` to default): a downstream filesystem-plan consumer distinguishes `None` (fail-closed) from `Some(empty)`, so preserving the Option keeps dispatch byte-identical rather than behaviorally-equivalent. - The issue's "remove the dispatcher's reserve-when-None fallback" premise was stale: `ironclaw_dispatcher` has no such fallback (it only holds/releases a passed reservation); the real reserve-when-None lives in the runtime adapters and is load-bearing. Removed nothing. - `None`-witness path is NOT failed closed: `system.process_sandbox` (RuntimeKind::System → no lane → no witness) is legitimately spawned through `spawn_json`, so a `None` witness falls back to the obligation-derived dispatch inputs (exactly today's values). Failing closed would regress production. This leaves a documented dual path (witness for untrusted lanes, obligation fallback for host-internal System spawns); collapsing it fully requires representing host-internal spawns in the lane model — a follow-up beyond S6's neutrality mandate. Tests (ironclaw_capabilities + host_api): witness-none-mounts dispatched verbatim (not a collapsed default); spawn uses context-mounts fallback via the witness Option; expired witness fails closed and releases the reservation via abort (invoke + spawn); single-use is structural. Pre-existing byte-neutral dispatch tests remain green unchanged. Gates: cargo fmt; workspace clippy --all-targets --all-features -D warnings clean; targeted Reborn crate suites green (host_api/capabilities/dispatcher/host_runtime/ loop_host/extensions/reborn_composition/approvals/architecture). Two pre-existing facade_factory failures inherited from the #6392 base (its runtime-policy message sanitization vs stale test expectations) are unrelated to this stack. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * refactor(reborn): code-quality pass on the #6396 stack (dedup witness consumption; centralize origin resolution) Addresses findings from a strict code-quality review of the six-slice stack. No behavior change — pure structural cleanup; all touched-crate tests green (4164 passed / 0 failed), clippy -D warnings clean, fmt clean. - Dedup (S6): `invoke_json` and `spawn_json` carried an identical ~35-line witness-consumption block (single-use `into_parts`, expiry fail-closed + `abort`, None→obligation fallback) differing only in the obligation phase. Extracted `CapabilityHost::dispatch_inputs_from_witness`, consolidating the logic and its (security-sensitive) rationale in one place and shrinking the oversized host.rs. The helper carries the `abort_obligations` arg set plus the witness; invoke's and spawn's request types differ so no shared bundle unifies them (same justification as the adjacent `seal_authorization` exemption) — annotated arch-exempt, folds into a prepared-invocation bundle with plan #6175. - Centralize origin resolution: the `context.origin` else `LoopRun(run_id)` rule was duplicated in `seal_authorization` (capabilities) and `require_approval_for_profile_policy` (composition). Moved to `ExecutionContext::resolved_origin()` in host_api — the single definition of the "run_id implies LoopRun" rule, resolved through the type that owns the fields. Dropped the now-unused `InvocationOrigin` import from host.rs. Also fixes a rebase-induced test breakage: main replaced the `UnusedDispatcher` test double with `dispatch_test_support::TestDispatcher`; updated the S1 seal test accordingly. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(reborn): address origin gate review feedback --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This was referenced Jul 22, 2026
11 of 21 tasks
17 of 29 tasks
This branch was successfully deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
A design note (not a contract) proposing a fundamental simplification of the Reborn host/runtime internals:
docs/reborn/2026-07-17-architecture-simplification-dto-dyn-local.md.It targets three recurring costs and shows they share one root cause:
dynproliferation — ~6 hot-path trait objects, most with exactly one production impl (HostRuntime→DefaultHostRuntime,CapabilityDispatcher→RuntimeDispatcher).InMemory*/Filesystem*store tree per domain, plus deployment-mode struct families.Root cause: every crate boundary is treated as a trust boundary, but Reborn has exactly one (the untrusted loop ↔ the host). Everything below it is trusted host code that was split into ~6 crates, each paying full DTO +
dyn+ backend-struct tax.Proposal (summary)
ironclaw_host_api:Invocation/Authority/Outcome.RuntimeCapabilityRequest/CapabilityInvocationRequest/CapabilityDispatchRequest/RuntimeAdapterRequestcollapse away (they wereInvocation+ a field now inAuthority). ~14 types → 3.authorize()fold — the four scattered policy sites (loop_host/host_runtime/capabilities/dispatcher) become one reviewable function; also removes the silentOk(Failed)vsErr(terminate)footgun.enum RuntimeLaneinstead ofdyn RuntimeAdapter(WASM extensions stay open — they're data behind theWasmlane). Delete the single-impl mediator traits. 6+dyn→ ~2 + one enum.TurnStore<B: RowBackend>, sharedMemory/Sqlite/Postgres) delete theInMemory*/Filesystem*tree;DeploymentConfig-as-data replaces composition-mode struct families.Invariants preserved
Touches only trusted internals: the one loop↔host membrane stays; authorize-before-dispatch is strengthened (one function, not four layers);
LoopExitrefs-only evidence and the durable lease/recovery model are untouched;host_apistays vocabulary-only (Golden Boundary #1).Migration
Incremental, boundary-tests-green at each step. First slice: the first-party capability down-path only — define the three types, write
authorize(), thread(&Invocation, &Authority)without merging crates, then measure type/dyncounts before rolling further.Relationship to open issues
Complements #6168 (shed product code out of the composition god-crate) on an orthogonal axis (shed mirror DTOs/traits in). Also addresses the capability-path pain behind #6137 / #6138.
Notes
🤖 Generated with Claude Code