Conversation
|
Important Review skippedToo many files! This PR contains 241 files, which is 91 over the limit of 150. To get a review, narrow the scope: Upgrade to a paid plan to raise the limit. ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Plus Run ID: ⛔ Files ignored due to path filters (5)
📒 Files selected for processing (241)
You can disable this status message by setting the Use the checkbox below for a quick retry:
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 |
|
@coderabbitai full review |
✅ Action performedFull review finished. |
|
@coderabbitai full review |
✅ Action performedFull review finished. |
There was a problem hiding this comment.
Code Review
This pull request implements the NEA-25 unified extension model, consolidating Slack bot and user-token identities into a single 'slack' extension. It introduces a robust capability-surface taxonomy, replacing legacy 'kind' strings with surface-based declarations (tool, channel, auth). Key changes include the migration of persisted Slack credential accounts, the introduction of a generic provider-identity actor resolver, and the removal of legacy connectable-channel registry logic in favor of extension-surface discovery. I have filtered out comments that were purely explanatory or non-actionable, while retaining those pointing to security risks, logic errors in migration, and test flakiness.
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.
…#5850) Atomic roll-up of the 8-PR NEA-25 taxonomy stack onto current main. Extension is the only installable product object; tool/channel/auth are derived capability surfaces; runtime kind controls loading only; manifest projection (v2, host_api contracts) is the sole surface-discovery source of truth. The connectable-channels rail and the parallel `kind` taxonomy are removed and pinned by a zero-legacy gate. slack_bot and slack_personal are retired into one `slack` extension with bounded forward migrations. Extensions wire carries runtime + surfaces, not a conflated kind. Supersedes #5833, #5839, #5842, #5845, #5847, #5848, #5849, #5850. Conflicts with main since the train forked were reconciled preserving main's newer behavior (#5851 unified slack cleanup, #6054 get_conversation_info DM resolution, #5499 extension import, #6057 TS source conventions). provider_identity domain duplication removed; the residual is a legitimate up-layer port adapter. See the PR description for the per-PR crosswalk, resolutions, placement audit, and verification. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
🚅 Deployed to the ironclaw-pr-6061 environment in ironclaw-ci-preview
|
…ed taxonomy Main advanced 25f8c12->bc80e54ca after the roll-up was based; #5957 (harden OAuth + per-user extension lifecycles, 81 files, old taxonomy) + #5971 landed. Rebased the roll-up onto the new main and folded #5957 forward: unified its channel_connection_facade_slot -> the train's channel_connection_facade (so #5957's SlackPersonalConnectionCleanupAdapter and the train injector share one facade field — fixes 40 removal/restore tests), applied the train's L03/L04 renames to #5957's parallel code (from_toml_with_contracts->from_toml, slack_actor_identity->provider_identity, LifecycleExtensionSurfaceKind-> CapabilitySurfaceKind, SLACK_PERSONAL_PROVIDER_ID->SLACK_PROVIDER_ID), integrated #5957's cleanup_requirements onto the train structs, re-added the train's activate_for_channel_setup, and converted the extension-ownership migration fixture to manifest v2. #5957's removal-cleanup hardening and the train's taxonomy retirement are both preserved. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
@coderabbitai full review |
✅ Action performedFull review finished. |
- extension_ownership migration fixture: fmt + manifest v2 sections. - host_runtime extension_v2_lifecycle_e2e: the unified manifest v2 validates a capability's required_host_ports at base parse, so an unknown port fails closed with the specific UnknownHostPort (not the older section-level HostApiSectionRejected) — update the inherited assertion. - wiring_parity harness-profile subset: the unified Slack manifest declares ironclaw.product_adapter/v1 (channel) alongside capability_provider (tools), so the test-support bundled-manifest registry must register the product-adapter contract too (adds ironclaw_product_adapter_registry dev-dep), matching the production composition registry. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
… tests github_wasm_runtime_contract.rs's registry_with_slack_user_package parses the unified Slack manifest (which now declares ironclaw.product_adapter/v1 alongside capability_provider) with default_host_api_contract_registry(), which lacks the product-adapter contract -> ManifestV2(UnknownHostApi). Register it (adds the product_adapter_registry dev-dep to host_runtime), matching production. Same fix class as the wiring-parity test-support registry. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Same class as extension_v2_lifecycle_e2e: the unified manifest v2 validates a capability's required_host_ports at base parse, so an unknown port on the telegram fixture fails closed with the specific UnknownHostPort, not the older section-level HostApiSectionRejected. Update the inherited assertion. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Coverage ratchetReborn integration-tier coverageLine coverage (Reborn crates): 85.69% — 305430 / 356429 lines Per-crate breakdown (63 crates, lowest-covered first)
This table itself is informational and never gates the PR on its own — not the percentage, not the per-crate holes, not the 0-coverage callout. A separate coverage ratchet (dry-run until enforce=true; see tests/integration/coverage-floor.toml) can fail the build on specific configured floors. Exemptions (3 entry/entries excluded from the accounting above)
|
Status: CI green, reconciled onto current mainThis roll-up carries the complete intended final state of the 8-PR NEA-25 stack (#5833, #5839, #5842, #5845, #5847, #5848, #5849, #5850) as one atomic PR on current Reconciliation notes for review:
CodeRabbit note: CodeRabbit skips this PR (207 files > its 150 limit). The train's content was already reviewed via the 8 source PRs (each <150 files, still open); only the ~5-commit reconciliation/CI-fix delta here is new. The 8 source PRs are preserved (not closed) as the evidence trail; their head SHAs are in the description. Not for merge by automation — @BenKurrek to review + merge. |
NEA-25: unified extension model — Train A roll-up
This is the single, self-contained Train A roll-up. It supersedes #5833 through
#5850 and is the only Train A prerequisite for the second PR train.
Exact audited head:
8410ea98a2fcf97eabff716ca05b785156a05435Reconciled
main:07f1d65ac5528ca3a7a182c47ad99c28b5efd995This readiness statement includes the current-
mainmerge and its textual andsemantic conflict resolutions. It covers the PR's behavior, architecture,
migrations, security, tests, and suitability as Train B's foundation.
Train A contract
are typed projections of its validated manifest.
runtimeplus typedsurfaces; retired top-levelkindis absent.slack_bot,slack_user, andslack_personalsurvive only in bounded forward-migration,rejection, and test enclaves guarded by the taxonomy test.
model-tool delivery path.
host-bundled unified Slack package/provider combination.
Upgrade and concurrency guarantees
strict parsing, fully validated, and committed with bounded CAS. New public
input still rejects legacy top-level
[[capabilities]].credential bindings, health, timestamps, installation identity, and manifest
authority. Conflicts fail explicitly; feature-disabled or unauthoritative
cleanup preserves the snapshot.
host-bundled predecessor bytes and SHA-256, plus matching manifest reference,
hash, and one of the two known historical cleanup shapes. Copied hashes,
arbitrary host bundles, local/registry sources, malformed state, duplicate
authority, and mismatched cleanup are rejected before mutation.
snapshot and publishes in-memory state only after persistence wins. Fresh
manifest plus installation is one atomic transition.
store-owned compatibility worker, so caller cancellation cannot interrupt a
durable mutation. It is not a hosted multi-writer guarantee.
slack_personalcredential transition runs before composition publishesdurable services, uses bounded versioned CAS, rejects new retired-provider
writes at durable boundaries, and applies aggregate owner/scope/entry budgets.
are decision-owned cancellation-safe transactions.
or deleting authoritative B.
Deployment and rollback
The credential migration is a one-time stop-the-world cutover:
Do not overlap old and new writers. After the data transition, rollback means
restoring the pre-cutover backup or rolling forward; the old binary does not
understand the unified provider identity. CAS-capable state rewrites are
byte-stable after convergence. Legacy non-CAS local startup normalizes in
memory and can repeat the startup compatibility pass.
Explicit Train B exclusions
This PR does not add manifest v3, a resolved-manifest architecture, generic
tool/channel adapters, a generic extension host or loader registry, generic
ingress/delivery coordination, a recipe-driven auth engine, vendor extraction,
or multi-account product behavior. Those are not needed for Train A and are not
smuggled into this foundation.
Audit repairs included
The exhaustive DO/DON'T/code-smell/positive-evidence checklist is committed in
docs/superpowers/specs/2026-07-13-train-a-rollup-hardening-design.md.Verification on the audited worktree
cargo test -p ironclaw_filesystem --all-features— pass, including 9/9in-memory/libSQL/PostgreSQL CAS storm tests.
cargo test -p ironclaw_extensions --all-features— pass.cargo test -p ironclaw_product_workflow --all-features— pass.cargo test -p ironclaw_webui_v2 --all-features— pass.cargo test -p ironclaw_reborn_composition --all-features— allexecutable tests pass after the merge, including 1,784/1,784 unit tests.
cargo test -p ironclaw_architecture— pass: 8 composition-boundary, 34dependency-boundary, and 4 retired-taxonomy tests.
cargo clippy --workspace --all-targets --all-features -- -D warnings— pass.bash scripts/reborn-e2e-rust.sh— pass.pnpm lintandpnpm testunder Node 22 — pass: 737/737 tests in90 files; lint includes TypeScript typecheck.
uv run --project tests/e2e pytest -q tests/e2e/scenarios/test_reborn_webui_v2_extensions_api.py tests/e2e/scenarios/test_reborn_webui_v2_legacy_extensions.py— 42/42 pass,no skips (one harness import warning).
SQLite
ResourceWarnings at unchanged test-helper lines; no test failed.cargo fmt --all -- --check,git diff --check, andscripts/pre-commit-safety.sh— pass.unresolved actionable code finding after accepted fixes and reruns.
pull-request workflow — pass, including all root, fixture, WebUI, integration
coverage, and discovered crate-bucket jobs.
pull-request workflow — pass, including served WebUI, gateway, substrate,
architecture, and runtime jobs.
Live merge status
The current-
mainconflicts are resolved in merge commit1bd5e82cf2a3c43567c1c0f945a74c4f575f76aa; the final exact head is8410ea98a2fcf97eabff716ca05b785156a05435. GitHub reports the PR asMERGEABLE. All exact-head statuses are terminal and non-failing: 59 passed
and 7 intentionally skipped. This includes the pull-request-native Reborn test,
Reborn E2E, code-style/clippy, platform/compatibility, stress, Railway preview,
scope, and regression workflows. There are zero unresolved review threads.
The only remaining merge gate is human review approval (
REVIEW_REQUIRED);GitHub therefore reports the aggregate merge state as
BLOCKEDdespite thebranch being conflict-free and all checks passing.
Superseded source PRs