Repository navigation
docs: refresh Reborn ProductSurface routing design - #6444
Conversation
|
Caution The consumer version of Gemini Code Assist on GitHub has been sunset. All code review activity has officially ceased. |
🔎 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. |
📝 WalkthroughSummary by CodeRabbit
WalkthroughThe Reborn architecture design note updates ProductSurface terminology and contracts, introduces kernel-owned causal routing, reframes terminals and channels as adapters, and refreshes enforcement, implementation status, validation instructions, and references. ChangesReborn architecture simplification
Estimated code review effort: 2 (Simple) | ~10 minutes Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 3 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (3 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 |
dc0ce7c to
788513e
Compare
|
🚅 Deployed to the ironclaw-pr-6444 environment in ironclaw-ci-preview
|
788513e to
56b337a
Compare
|
Caution The consumer version of Gemini Code Assist on GitHub has been sunset. All code review activity has officially ceased. |
56b337a to
18b613a
Compare
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 1147-1169: Soften the present-tense guarantees in the architecture
passage around ProductSurface calls and durable events to describe the target
architecture rather than current behavior. Replace assertions that every event
already carries complete correlation and adapters never maintain delivery truth
with “must” or equivalent intent language, consistent with §14, until the event
schema, path contract, and conformance tests enforce these requirements.
- Around line 804-806: Revise the facade description to state actor binding as
the target state rather than an already enforced guarantee. Update the wording
around the “ONLY surface” and sealing claims to avoid asserting that
invoke/query are actor-bound until the caller ingress is removed or backed by
implementation and conformance tests.
🪄 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: 032b58c8-72ef-4c30-89d9-1e5ad9256a0f
📒 Files selected for processing (1)
docs/reborn/2026-07-17-architecture-simplification-dto-dyn-local.md
| This is not observability garnish. It is routing truth. Every `ProductSurface` | ||
| call either starts that path (`open_conversation`, `submit_turn`, `invoke`) or | ||
| continues it (`events`, `reply`, `resolve_gate`, `cancel`, `query`). Every durable | ||
| event and projection emitted for a turn, invocation, gate, outcome, or delivery | ||
| attempt carries enough typed correlation to answer: | ||
|
|
||
| - who or what caused this (`Actor`, `InvocationOrigin`, `RoutineId` / | ||
| `TurnRunId`); | ||
| - under which scope and authority it ran (`ResourceScope`, `Authorized`, | ||
| approval/auth/resource gate records); | ||
| - which product terminal/channel can observe or render it (conversation binding, | ||
| projection cursor, reply target, delivery attempt); | ||
| - whether replay must return a recorded result, re-enter `authorize()`, or report | ||
| `HostFailure::Uncertain` (§11.3). | ||
|
|
||
| The prohibition that falls out is simple: **adapters render from projected path | ||
| state; they do not maintain hidden delivery truth.** Slack timestamps, Telegram | ||
| message ids, WebUI tabs, HTTP request ids, and OpenAI-compat request ids are | ||
| terminal/device coordinates. They may be stored as external refs or delivery | ||
| metadata, but they are never canonical run, gate, invocation, or approval | ||
| authority. If a side effect succeeds, fails, blocks, resumes, or becomes | ||
| uncertain, that fact is recorded on the kernel path first and only then rendered | ||
| back through the terminal/channel. |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift
Soften present-tense causal-routing guarantees until implemented.
This states that every durable event carries complete routing correlation and that adapters never own delivery truth, while §14 explicitly says causal routing is design-only and has no first-class path contract yet. Use “must”/“target architecture” wording until the event schema, path contract, and conformance tests enforce it.
As per coding guidelines, cross-layer guarantees must be enforced by code or tests, or softened to describe intent.
🤖 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 1147 - 1169, Soften the present-tense guarantees in the architecture
passage around ProductSurface calls and durable events to describe the target
architecture rather than current behavior. Replace assertions that every event
already carries complete correlation and adapters never maintain delivery truth
with “must” or equivalent intent language, consistent with §14, until the event
schema, path contract, and conformance tests enforce these requirements.
Source: Coding guidelines
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 2272-2282: Update the completion snapshot table to add evidence
citations for each load-bearing Done, Landed, and Live claim, prioritizing the
product facade, origin→gate matrix, generic extension/channel runtime, and
InMemory*Store deletion rows. Cite exact defining symbols, implementation files,
or ratchet tests from the relevant write sites, while preserving the existing
status descriptions and keeping the citations specific enough to verify each
claim.
- Around line 2432-2435: Update the documentation validation instructions in the
architecture simplification document to require running `mint dev` and `mint
broken-links` from the `docs/` directory before merging, in addition to the
existing checks.
🪄 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: 55632451-26e1-4161-8a47-7f5cc38fed96
📒 Files selected for processing (1)
docs/reborn/2026-07-17-architecture-simplification-dto-dyn-local.md
| | Result DTO collapse | **Done** — `CapabilityOutcome` and result mirrors are deleted; `host_api::Resolution` is canonical. | | ||
| | Request-side DTO collapse | **Done** — retired request mirror DTO names are banned; `LoopRequest`, tuple parts, and private `RuntimeLaneRequest` carry the remaining distinct states. | | ||
| | Request/witness vocabulary | **Done** — `Invocation`, `Actor`, `InvocationOrigin`, `Authorized`, and dispatch-through-witness are live. | | ||
| | Pre-flight authority fold | **Done** — trust, grants, approvals, credentials, resources, and lane resolution are folded through `authorize()`. | | ||
| | Origin→gate matrix | **Live, tightening remains** — matrices are descriptor data and ratcheted; `LoopRun` ungated set is a reviewed 17-id seed to shrink. | | ||
| | `InMemory*Store` mirror deletion | **Done for tracked domain stores** — the ratchet was deleted after the allowlist reached empty. | | ||
| | Generic extension/channel runtime | **Landed** — `ChannelAdapter` + `ironclaw_extension_host`; Slack/Telegram are extension adapters, not bespoke product trees. | | ||
| | Product facade collapse | **Started, mostly remaining** — `invoke`/`query` exist and the facade is frozen at 87 methods; migrations to descriptors/views remain. | | ||
| | Deployment mode as data | **Partial** — renames/ratchets are live, but profile enums and profile-edge branching still exist. | | ||
| | Causal routing / product terminal path | **Design added here, implementation pending** — no first-class duct/path contract yet. | | ||
| | Recoverability endgame | **Partial** — vocabulary and model-visible recovery base are live; conformance/diagnostic observation remain. | |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Add write-site evidence to the completion snapshot.
The Done/Landed/Live claims are load-bearing but lack exact defining symbols, implementation files, or ratchet tests in this table. Add those citations, especially for the facade, origin matrix, extension runtime, and store deletion claims.
As per coding guidelines, load-bearing Markdown claims must cite evidence from the definition or write site.
🤖 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 2272 - 2282, Update the completion snapshot table to add evidence
citations for each load-bearing Done, Landed, and Live claim, prioritizing the
product facade, origin→gate matrix, generic extension/channel runtime, and
InMemory*Store deletion rows. Cite exact defining symbols, implementation files,
or ratchet tests from the relevant write sites, while preserving the existing
status descriptions and keeping the citations specific enough to verify each
claim.
Source: Coding guidelines
| - Urbit Arvo / Gall routing analogy for §5.2.10: official Arvo overview | ||
| (`https://docs.urbit.org/build-on-urbit/app-school/1-arvo`), Gall agent | ||
| overview (`https://docs.urbit.org/build-on-urbit/app-school/2-agent`), and | ||
| kernel/vane docs (`https://docs.urbit.org/urbit-os/kernel/arvo`). |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
cd docs
mint dev
mint broken-linksRepository: nearai/ironclaw
Length of output: 233
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf 'Top-level guidance files:\n'
git ls-files | rg '(^|/)(AGENTS\.md|CLAUDE\.md|\.claude/rules/.*)$' || true
printf '\nSearch for docs validation references:\n'
rg -n "mint dev|mint broken-links|broken-links|docs validation" docs .github .claude AGENTS.md CLAUDE.md -S || trueRepository: nearai/ironclaw
Length of output: 5202
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf 'Docs tree overview:\n'
git ls-files docs | sed -n '1,120p'
printf '\nRelevant excerpt around the changed lines:\n'
sed -n '2426,2440p' docs/reborn/2026-07-17-architecture-simplification-dto-dyn-local.mdRepository: nearai/ironclaw
Length of output: 6824
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf 'Look for mint config or docs package hints:\n'
fd -a 'mint*' docs .github .claude 2>/dev/null || true
fd -a 'package.json' docs .github 2>/dev/null || true
rg -n '"broken-links"|"mint"' docs .github -S || trueRepository: nearai/ironclaw
Length of output: 378
Run the docs checks from docs/. git diff --check doesn’t cover the required Mintlify validation; add mint dev and mint broken-links before merge.
🤖 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 2432 - 2435, Update the documentation validation instructions in the
architecture simplification document to require running `mint dev` and `mint
broken-links` from the `docs/` directory before merging, in addition to the
existing checks.
Source: Coding guidelines
…uting-docs # Conflicts: # docs/reborn/2026-07-17-architecture-simplification-dto-dyn-local.md
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 2492-2499: Update the validation summary for this documentation
change to include running mint dev and mint broken-links from the docs
directory, alongside git diff --check. Ensure all three documented checks are
listed before merge.
- Around line 824-826: Update the 87 frozen trait methods statement to describe
the freeze ratchet’s actual counting method: extracting method names from the
RebornServicesApi trait block and comparing set membership, rather than counting
file-wide async fn occurrences across impls and tests. Add a citation or link to
the relevant freeze-ratchet test as evidence for this load-bearing claim, while
preserving the existing date and reconciliation context.
🪄 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: ebe199e1-fb50-4a77-9cc1-aaf4a29d726e
📒 Files selected for processing (1)
docs/reborn/2026-07-17-architecture-simplification-dto-dyn-local.md
| **87 frozen trait methods** as of 2026-07-22 (the original 2026-07-17 audit | ||
| counted 88 before the first shrink/additive conduit reconciliation; file-wide | ||
| `async fn` counts include impls and tests), and most |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Align the 87-method count with the freeze ratchet.
Lines 824-826 attribute the count to file-wide async fn counting, including impls and tests, but crates/ironclaw_architecture/tests/reborn_facade_method_freeze_ratchet.rs extracts methods from the RebornServicesApi trait block and compares set membership. Document the trait-block extraction instead.
As per coding guidelines, load-bearing Markdown claims must cite evidence from the definition or write site.
🤖 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 824 - 826, Update the 87 frozen trait methods statement to describe the
freeze ratchet’s actual counting method: extracting method names from the
RebornServicesApi trait block and comparing set membership, rather than counting
file-wide async fn occurrences across impls and tests. Add a citation or link to
the relevant freeze-ratchet test as evidence for this load-bearing claim, while
preserving the existing date and reconciliation context.
Source: Coding guidelines
| For a docs-only update to this file, run: | ||
|
|
||
| ```bash | ||
| git diff --check | ||
| cd docs | ||
| mint dev | ||
| mint broken-links | ||
| ``` |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Run the documented Mint checks before merge.
The validation summary reports only git diff --check; this docs/**/* change also requires mint dev and mint broken-links from docs/.
As per coding guidelines, documentation changes must be tested with those commands.
🤖 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 2492 - 2499, Update the validation summary for this documentation change
to include running mint dev and mint broken-links from the docs directory,
alongside git diff --check. Ensure all three documented checks are listed before
merge.
Source: Coding guidelines
What changed
ProductSurface.origin/main, including the merged authorize fold, origin-gate matrix, generic extension runtime, and in-memory-store ratchet cleanup.Why
Channels should start or resume a durable path into the Reborn kernel, then render projected state back out. They should not own canonical run, gate, invocation, approval, or delivery truth.
Validation
git diff --check