Repository navigation
test(reborn): PR-E2 seam constructors — skill/durable/gateway (E-SKILL, E-DURABLE, E-GATEWAY) - #5514
Merged
Merged
Conversation
railway-app
Bot
temporarily deployed
to
ironclaw-ci-preview / ironclaw-pr-5514
July 1, 2026 22:45
Destroyed
github-actions
Bot
added
size: L
📝 WalkthroughSummary by CodeRabbit
WalkthroughThe PR splits Reborn test-support helpers into dedicated modules, adds durable extension-store reopen access, wires synthetic skill activation through the runtime harness, adds mock model gateways, and introduces parked-model cancellation support with integration tests. ChangesTest-support seams and integration tests
Estimated code review effort: 4 (Complex) | ~60 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 |
Contributor
There was a problem hiding this comment.
Pull request overview
Adds three Reborn integration-test “seam constructors” (E-SKILL, E-DURABLE, E-GATEWAY) that let the int-tier harness drive otherwise hard-to-reach production wiring paths (skill activation/context injection, durable extension install persistence across reopen, and mid-turn cancellation while the model call is parked).
Changes:
- Introduces a parking LLM provider + gate to deterministically pause the first model call and enable mid-turn cancellation assertions.
- Adds test-support wiring for local-dev
skill_activate(capability wrap + runtimeskill_context_source) and a durable reopen helper for the extension installation store. - Adds three new integration tests that drive each seam end-to-end.
Reviewed changes
Copilot reviewed 12 out of 12 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/support/reborn/scripted_provider.rs | Adds ParkingModelGate/ParkingLlm to park the first model call for cancel-path testing. |
| tests/support/reborn/harness.rs | Wires optional skill context source + synthetic skill_activate wrapping into the host-runtime harness; adds helpers for storage root + seeding a system skill. |
| tests/support/reborn/group.rs | Adds skill_activation_tools() group builder and integrates skill context source + optional parking provider into thread construction. |
| tests/support/reborn/builder.rs | Adds durable reopen assertion for extension installs; adds cancel_run and model-request inspection support; threads optional park gate through the harness builder. |
| tests/reborn_integration_skill_activate.rs | New integration test proving skill_activate dispatch + prompt injection via skill_context_source. |
| tests/reborn_integration_durable.rs | New integration test proving extension install persists across an independent store reopen. |
| tests/reborn_integration_cancel.rs | New integration test proving a parked run can be cancelled mid-turn and reaches TurnStatus::Cancelled. |
| crates/ironclaw_reborn_composition/src/test_support.rs | Adds test-support entrypoints/constants/types for E-DURABLE and E-SKILL seams. |
| crates/ironclaw_reborn_composition/src/runtime/local_dev/skill_activation.rs | Adds test-only wrapper to inject skill_activate synthetic capability onto a port. |
| crates/ironclaw_reborn_composition/src/runtime/local_dev.rs | Exposes SKILL_ACTIVATE_CAPABILITY_ID under test-support and re-exports the new test wrapper. |
| crates/ironclaw_reborn_composition/src/runtime.rs | Adds test-support forwarders for skill context source + skill activation wrapper; adjusts internal struct visibility to support test wiring. |
| crates/ironclaw_reborn_composition/src/factory.rs | Adds test-support factory to reopen a fresh extension-installation store at a given local-dev storage root. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
![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 test-support seams and integration tests to verify mid-turn cancellation, cross-reopen capability durability, and synthetic skill activation. It adds helper functions to expose private local-dev configurations, updates the test harness to support model parking and skill context injection, and adds three new integration test suites. The feedback suggests refactoring the ParkingModelGate synchronization helper to use tokio::sync::Notify instead of Mutex<Option<oneshot::Sender/Receiver>> to prevent potential race conditions and simplify the implementation.
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.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 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 `@tests/reborn_integration_cancel.rs`:
- Around line 6-7: The top-of-test comment in reborn_integration_cancel.rs
overstates the wiring by claiming a wired cancellation_factory and coordinator
fan-out; update the comment to match the actual setup used by the test, where
group.rs leaves cancellation_factory as None and cancellation behavior comes
from the loop driver’s default factory. Keep the comment focused on the intent
of the test and avoid any cross-layer guarantee language that is not enforced by
this test’s setup.
- Around line 31-35: The integration test can hang because
gate.wait_until_parked() has no timeout if the run fails before reaching the
model gateway. Update the cancel test in reborn_integration_cancel to wrap the
parking wait with a bounded timeout, using the existing gate.wait_until_parked()
call path and the harness.submit_turn_async flow, so the test fails fast instead
of waiting indefinitely.
In `@tests/support/reborn/group.rs`:
- Around line 315-319: The doc comment for the skill-activation test is out of
sync with the actual setup in the builder. Update the comment near the `greet`
skill setup to reflect that the skill is seeded at the system scope, or soften
the wording so it only describes the intended behavior without promising
subject-user scope. Keep the description aligned with the existing
`builtin.skill_activate`/`skill_context_source` flow and the seeded skill in
this `reborn::group` test fixture.
🪄 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: 14a31c39-283c-483b-bf20-0536881ce9e0
📒 Files selected for processing (12)
crates/ironclaw_reborn_composition/src/factory.rscrates/ironclaw_reborn_composition/src/runtime.rscrates/ironclaw_reborn_composition/src/runtime/local_dev.rscrates/ironclaw_reborn_composition/src/runtime/local_dev/skill_activation.rscrates/ironclaw_reborn_composition/src/test_support.rstests/reborn_integration_cancel.rstests/reborn_integration_durable.rstests/reborn_integration_skill_activate.rstests/support/reborn/builder.rstests/support/reborn/group.rstests/support/reborn/harness.rstests/support/reborn/scripted_provider.rs
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 0ec102aa1c
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
henrypark133
force-pushed
the
reborn-cov-PR-E2
branch
from
July 1, 2026 22:54
0ec102a to
0aa8bd7
Compare
railway-app
Bot
temporarily deployed
to
ironclaw-ci-preview / ironclaw-pr-5514
July 1, 2026 22:54
Destroyed
Contributor
Reborn integration-tier coverageLine coverage (Reborn crates): 15.06% — 9509 / 63132 lines Per-crate breakdown (12 crates, lowest-covered first)
This signal is informational: coverage never gates the PR — not the percentage, not the per-crate holes, not the 0-coverage callout. |
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 `@tests/support/reborn/scripted_provider.rs`:
- Around line 84-88: `wait_until_parked` in `scripted_provider.rs` is swallowing
a broken parking handshake by ignoring the `oneshot` receive result. Update
`wait_until_parked` (and the related parked-wait path around the second
occurrence) to fail loudly when the sender is dropped by using `expect` with a
clear message or by returning a `Result` to the caller and propagating the
error. Keep the fix localized to the `wait_until_parked` logic and the
`parked_rx`/`rx.await` handling so the cancel seam cannot proceed on a silent
failure.
🪄 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: 2dce11c6-4628-4264-8b31-47274e86fdfe
📒 Files selected for processing (12)
crates/ironclaw_reborn_composition/src/factory.rscrates/ironclaw_reborn_composition/src/runtime.rscrates/ironclaw_reborn_composition/src/runtime/local_dev.rscrates/ironclaw_reborn_composition/src/runtime/local_dev/skill_activation.rscrates/ironclaw_reborn_composition/src/test_support.rstests/reborn_integration_cancel.rstests/reborn_integration_durable.rstests/reborn_integration_skill_activate.rstests/support/reborn/builder.rstests/support/reborn/group.rstests/support/reborn/harness.rstests/support/reborn/scripted_provider.rs
|
🚅 Deployed to the ironclaw-pr-5514 environment in ironclaw-ci-preview
|
henrypark133
commented
Jul 2, 2026
henrypark133
left a comment
henrypark133
left a comment
Collaborator
Author
There was a problem hiding this comment.
Code Review (multi-agent)
Intent: Add Reborn integration-test seam constructors for skill activation, durable storage, and gateway cancellation with minimal driving tests.
Stats: 1 net-new finding (from 6 raw, 5 after overlap dedup, 1 after live-thread dedupe) across 1 file. Reviewers run: security, bugs, performance, tests, conventions, local-patterns, maintainability, approach. Reviewers failed: none. Body-only: 0.
Existing unresolved threads already cover the unused import, cancellation doc wording, parking wait timeout, and parking handshake failure behavior, so I am not reposting those here.
Maintainability
- Medium Keep the trace handle through parking mode (
tests/support/reborn/group.rs:847-854, confidence 75) - anchor:tests/support/reborn/group.rs:847
Parking mode is only a wrapper around the same scripted provider, but this branch hides theTraceLlminsideParkingLlmand storesNoneon the harness. That forcesRebornIntegrationHarness.scripted_llmto become optional and adds parked-thread special cases incaptured_system_promptsandassert_model_request_contains, even though the underlying trace still exists.
railway-app
Bot
temporarily deployed
to
ironclaw-ci-preview / ironclaw-pr-5514
July 2, 2026 00:13
Destroyed
Collaborator
Author
|
Thanks for the reviews — addressed in 5dc14f2: Fixed
Kept, with rationale (gemini, High —
Not a defect (Copilot — |
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 `@tests/support/reborn/group.rs`:
- Around line 317-320: The doc comment for the seeded `greet` skill is
misleading about the activation path: it refers to `($greet)` even though this
seam is exercised by an explicit `builtin.skill_activate` dispatch and the user
message intentionally omits `greet`. Update the comment near the `greet` setup
in `group.rs` to describe the actual activation mechanism and soften any
cross-layer guarantee language so it reflects intent rather than implying the
turn text itself triggers activation.
🪄 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: 39657c52-0bf0-4a15-952b-77b3855fd410
📒 Files selected for processing (3)
tests/reborn_integration_cancel.rstests/support/reborn/group.rstests/support/reborn/scripted_provider.rs
…L, E-DURABLE, E-GATEWAY) Add three integration-test seam constructors to the Reborn int tier, each shipped with a minimal test that actually drives it: - E-SKILL: `skill_activation_tools()` group wires the local-dev `skill_activate` synthetic capability (via the real `wrap_skill_activation_capability_for_test` production wrap) and the runtime `skill_context_source`. `reborn_integration_skill_activate` seeds a `greet` system skill, activates it through the capability, and asserts both that the capability dispatched (`count: 1`) and that the skill's instructions injected into a later model request. Mutation- verified on both halves. - E-DURABLE: `open_local_dev_extension_installation_store_for_test` reopens a fresh, independent `ExtensionInstallationStore` at the harness's on-disk storage root, reusing the production mounts + `default_state_path`. `reborn_integration_durable` installs an extension through a real turn and asserts it survives an independent reopen (real on-disk persistence, not in-memory state). - E-GATEWAY: `ParkingModelGate`/`ParkingLlm` park the model call at the vendor-SDK seam so a mid-turn `cancel_run` can be exercised. `reborn_integration_cancel` parks, cancels, releases, and asserts the run reaches `Cancelled` (not `Completed`). The optional `cancellation_factory` is intentionally left `None` — the loop-driver host builds its own default cancellation factory, so wiring one here would be dead, untested code. All new test-support crate seams are `#[cfg(feature = "test-support")]` (zero prod bytes) with doc-comments naming the production call site each mirrors. Every new test mutation-verified RED-for-the-right-reason. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…ng-gate rationale - reborn_integration_cancel: wrap `wait_until_parked()` in a 10s timeout so the test fails fast instead of hanging on CI if the run dies before reaching the model gateway (CodeRabbit). - reborn_integration_cancel + group.rs: correct doc comments to match the actual wiring — cancellation is observed by the loop-driver host's own default `TurnStateRunCancellationFactory` (the optional `cancellation_factory` is `None`), and the `greet` skill is system-scoped, not subject-user-scoped (Copilot / CodeRabbit). - scripted_provider: document why `ParkingModelGate` keeps its `oneshot` single-shot design rather than `tokio::sync::Notify` — the `take()`-based channels are idempotent under a repeat `park()` from the decorator chain's retry/failover, whereas a single-permit `Notify` pair would deadlock the second waiter (gemini). Copilot's "unused ExtensionInstallationStore import" is a false positive: the trait must be in scope for the `store.list_installations()` call in `assert_extension_install_persists_after_reopen`, and CI Clippy (-D warnings) passes. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…visibility narrowing
- builder.rs/group.rs/scripted_provider.rs: collapse `scripted_llm` back to a
plain `Arc<TraceLlm>` (was `Option<Arc<TraceLlm>>`, `None` when parked).
Parking mode is only a wrapper around the same scripted provider — build one
`Arc<TraceLlm>` up front and have `ParkingLlm` hold/clone it, so
`captured_system_prompts`/`assert_model_request_contains` no longer need
Option special-casing for a parked thread (reviewer: repo owner).
- scripted_provider.rs: `ParkingModelGate::wait_until_parked`/`park` now
`.expect()` instead of silently dropping a broken parking handshake via
`let _ = rx.await` (CodeRabbit).
- runtime.rs/test_support.rs: `LocalDevSkillContextSource` reverts to fully
private (no more unconditional `pub(crate)` widening in production builds);
`local_dev_filesystem_skill_context_source_for_test` (already
`#[cfg(feature = "test-support")]`) now returns just the two fields the
caller needs as a tuple instead of the struct, matching the sibling
`wrap_project_create_capability_for_test` cfg-gated-accessor pattern instead
of widening a struct's field visibility (Copilot).
Verified: cargo build (plain + --features test-support + --features libsql)
clean; reborn_integration_{cancel,durable,skill_activate}, reborn_group_approvals,
reborn_integration_{greeting,secret_injection} all green (6/6 binaries,
0 failures); clippy --all-features clean on both the affected test binaries and
the ironclaw_reborn_composition crate.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
henrypark133
force-pushed
the
reborn-cov-PR-E2
branch
from
July 2, 2026 01:22
5dc14f2 to
9de94cc
Compare
railway-app
Bot
temporarily deployed
to
ironclaw-ci-preview / ironclaw-pr-5514
July 2, 2026 01:22
Destroyed
3 tasks done
Collaborator
Author
|
Rebased onto the latest `main` (2 more coverage PRs landed: #5434, #5482 — conflicts in `group.rs`/`harness.rs` resolved) and addressed the remaining review comments in 9de94cc: Fixed
Verified: build clean (plain / `--features test-support` / `--features libsql`); all 6 affected test binaries green (`reborn_integration_{cancel,durable,skill_activate}`, `reborn_group_approvals`, `reborn_integration_{greeting,secret_injection}`, 0 failures); clippy `--all-features` clean. Still not a defect: Copilot/codex's repeated "unused `ExtensionInstallationStore` import" flag — the trait must be in scope for `store.list_installations()`; CI Clippy (`-D warnings`) passes. |
henrypark133
commented
Jul 2, 2026
henrypark133
left a comment
henrypark133
left a comment
Collaborator
Author
There was a problem hiding this comment.
Code Review (multi-agent)
Intent: Add mutation-verified Reborn integration-test seam constructors for skill activation, durable extension storage, and gateway cancellation.
Stats: 2 findings posted (from 5 raw reviewer findings, 2 after dedup/filter) across 2 files. Reviewers run: security, bugs, performance, tests, conventions, local-patterns, maintainability, approach. Reviewers failed: none. Body-only: 0.
Maintainability
-
Medium Split new seam families out of the catch-all test_support file (
crates/ironclaw_reborn_composition/src/test_support.rs:853-981, confidence 75) — anchor:crates/ironclaw_reborn_composition/src/test_support.rs:853This patch adds the E-DURABLE and E-SKILL facade code to the central
test_support.rsfile, pushing it to 1002 lines. The new entries are independent seam families, so keeping all of them inline makes the facade harder to scan and turns future seam additions into more catch-all growth.
Conventions
-
Low E-SKILL doc comment names the wrong seeding path (
tests/support/reborn/harness.rs:1965-1971, confidence 75) — anchor:.claude/rules/review-discipline.md:69The new doc comment says the skill is seeded by
RebornIntegrationGroup::skill_toolsand depends on tenant + user, but the PR actually seeds a system skill throughRebornIntegrationGroup::skill_activation_tools. Also flagged by local-patterns/Low.
Addresses review: the file crossed 1000 lines with this PR's E-DURABLE/E-SKILL
additions, and kept growing as a flat catch-all of independent seam families
(E-PROFILE, E-PROJ, OAuth/product-auth, secrets/local-dev-boot, budget
gateway, E-DURABLE, E-SKILL). Converted to a directory module
(`test_support/`) mirroring the existing `runtime/` split pattern in this
same crate: one file per seam family, `mod.rs` is a thin re-export layer
listing the full public surface at a glance. Every doc comment preserved
verbatim; module-file boundaries only, no behavior change.
Also: fixed a stale E-SKILL doc comment in harness.rs that still named a
nonexistent `RebornIntegrationGroup::skill_tools` ctor with tenant/user
scoping — the real ctor is `skill_activation_tools`, seeding a system-scoped
skill.
Verified: build clean (plain + --features test-support + --features libsql +
full workspace --features libsql check); all 6 affected test binaries green
(reborn_integration_{cancel,durable,skill_activate}, reborn_group_approvals,
reborn_integration_{greeting,secret_injection}); clippy --all-features clean.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
railway-app
Bot
temporarily deployed
to
ironclaw-ci-preview / ironclaw-pr-5514
July 2, 2026 03:58
Destroyed
Collaborator
Author
|
Addressed in 8b1005c: Fixed
Verified: build clean (plain / `--features test-support` / `--features libsql` / full workspace `--features libsql` check); all 6 affected test binaries green; clippy `--all-features` clean. |
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 `@crates/ironclaw_reborn_composition/src/test_support/budget_gateway.rs`:
- Around line 104-130: The poison-recovery lock handling is duplicated across
`push`, `call_count`, `next_reply`, and the
`stream_model`/`stream_model_with_capabilities` paths in `budget_gateway.rs`.
Extract the repeated
`.lock().unwrap_or_else(std::sync::PoisonError::into_inner)` logic into a small
private helper on the same type, then update `record_call`/`push` and
`call_count`/`next_reply` to use it so both `stream_model` impls and related
methods share one recovery path.
🪄 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: fcbba37e-7427-402a-8ac0-e9fe205c869d
📒 Files selected for processing (9)
crates/ironclaw_reborn_composition/src/test_support/budget_gateway.rscrates/ironclaw_reborn_composition/src/test_support/durable.rscrates/ironclaw_reborn_composition/src/test_support/local_dev_boot.rscrates/ironclaw_reborn_composition/src/test_support/mod.rscrates/ironclaw_reborn_composition/src/test_support/oauth_product_auth.rscrates/ironclaw_reborn_composition/src/test_support/project_create.rscrates/ironclaw_reborn_composition/src/test_support/skill_activation.rscrates/ironclaw_reborn_composition/src/test_support/user_profile.rstests/support/reborn/harness.rs
henrypark133
commented
Jul 2, 2026
henrypark133
left a comment
henrypark133
left a comment
Collaborator
Author
There was a problem hiding this comment.
Code Review (multi-agent)
Intent: Add Reborn integration-test seam constructors for skill activation, durable local-dev extension storage, and gateway cancellation, each backed by minimal driving tests.
Stats: 3 findings (from 5 raw, 3 after dedup/filter) across 3 files. Reviewers run: security, bugs, performance, tests, conventions, local-patterns, maintainability, approach. Reviewers failed: none. Body-only: 0
Findings
- Medium Parking gate guarantee lacks direct coverage (
tests/support/reborn/scripted_provider.rs:55-67, confidence 75) — anchor: tests/support/reborn/scripted_provider.rs:55
The new parking gate documents two concurrency guarantees that the committed integration test does not exercise:release()may run before the provider awaits, and a second model call returns immediately instead of blocking. Because this helper is what makes the cancellation seam safe from deadlocks, those guarantees should be enforced by a focused test rather than only described in the comment. - Low Skill activation tenant is hardcoded in two owners (
tests/support/reborn/harness.rs:1618-1622, confidence 75) — anchor: tests/support/reborn/harness.rs:1622; tests/support/reborn/group.rs:440
SKILL_ACTIVATION_RUN_TENANTmust match the group run tenant fromRebornIntegrationGroupBuilder::build_base, but both sides hand-code"tenant-itest". The new comment carries the sync contract, so changing the group scope without this separate constant would silently build the skill context source for a different tenant than the turn uses.
Also flagged by: conventions/Low - Low Model-request assertion belongs with assertion helpers (
tests/support/reborn/builder.rs:898-917, confidence 75) — anchor: tests/support/reborn/CLAUDE.md:75; tests/support/reborn/CLAUDE.md:101
The support-tree spec keeps richer egress, tool-result, and model-prompt assertions inassertions.rs, whilebuilder.rsowns the harness builder, core assertions, and capture accessors. Addingassert_model_request_containsbeside the builder splits the model-prompt assertion surface fromassert_system_prompt_contains, making future prompt assertions harder to find.
…relocation, coverage, tenant drift - skill_activation.rs: `.expect(...)` now includes the underlying build-error value in the panic message instead of dropping it (Copilot). - budget_gateway.rs: extracted the 5x-duplicated `.lock().unwrap_or_else(PoisonError::into_inner)` into a private `lock<T>` helper (mirrors the same pattern already in scripted_provider.rs) (CodeRabbit). - builder.rs/assertions.rs: relocated `assert_model_request_contains` next to its sibling `assert_system_prompt_contains` in assertions.rs — builder.rs keeps only the harness builder, core assertions, and capture accessors per the support-tree's file-ownership spec (repo owner). - scripted_provider.rs: added `parking_llm_release_before_await_and_second_call_do_not_block`, a focused unit test enforcing the two concurrency guarantees `ParkingModelGate`'s doc comment already claimed (release-before-await ordering; second call doesn't block) but the committed integration test never exercised directly. Mutation-verified. Also fixed `clippy::items_after_test_module` by moving the `mod tests` block to the true end of the file (repo owner). - harness.rs/group.rs: deleted `SKILL_ACTIVATION_RUN_TENANT`, the second independent hardcode of `"tenant-itest"` that risked drifting from the group's actual run-scope tenant. `HostRuntimeHarnessOptions` gains an `Option<TenantId>` field set only via a new `with_skill_activation_tenant(tenant)` builder method that ONLY `skill_activation_tools()` calls — `new_with_options` itself, shared by every other harness variant, is untouched. `RebornIntegrationGroupBuilder` now passes `base.canonical_binding.tenant_id`, the same tenant `build_base` already resolved, so the two can never diverge (repo owner). Verified: build clean (composition crate plain/test-support/libsql, tests/ tree libsql); 8 test binaries green (0 failures); clippy --all-features clean on both the composition crate and all affected test binaries. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
railway-app
Bot
temporarily deployed
to
ironclaw-ci-preview / ironclaw-pr-5514
July 2, 2026 05:00
Destroyed
Collaborator
Author
|
Addressed in 4bf0654: Fixed
Verified: build clean (composition crate plain/test-support/libsql, tests/ tree libsql); 8 test binaries green; clippy `--all-features` clean on both the composition crate and all affected test binaries. Not a defect (Copilot — `builder.rs:35` "unused `ExtensionInstallationStore` import"): same false positive flagged 4 times now across this PR's diff churn. `store.list_installations()` (line 571) requires the trait in scope; CI Clippy (`-D warnings`) passes. |
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 `@tests/support/reborn/scripted_provider.rs`:
- Around line 198-217: The test is exercising ParkingModelGate::park() directly
instead of the real call sites that enforce the parking behavior. Update the
test to drive the parked provider through ParkingLlm::{complete,
complete_with_tools} under a timeout so the assertion covers the caller-facing
seam, not the private gate. Keep the existing lost-wakeup and second-call
guarantees, but make them observable via the public/model-facing API rather than
calling park() directly.
🪄 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: d5aa1fae-d613-474c-8e81-d5ec1bcafa53
📒 Files selected for processing (7)
crates/ironclaw_reborn_composition/src/test_support/budget_gateway.rscrates/ironclaw_reborn_composition/src/test_support/skill_activation.rstests/support/reborn/assertions.rstests/support/reborn/builder.rstests/support/reborn/group.rstests/support/reborn/harness.rstests/support/reborn/scripted_provider.rs
💤 Files with no reviewable changes (1)
- tests/support/reborn/builder.rs
Comment on lines
+198
to
+217
| #[tokio::test] | ||
| async fn parking_llm_release_before_await_and_second_call_do_not_block() { | ||
| let gate = ParkingModelGate::new(); | ||
|
|
||
| // Guarantee 1: release fires before any `park()` call exists to | ||
| // receive it. | ||
| gate.release(); | ||
| tokio::time::timeout(Duration::from_secs(5), gate.park()) | ||
| .await | ||
| .expect( | ||
| "park() must resolve promptly when release() ran first \ | ||
| (oneshot send is lost-wakeup free)", | ||
| ); | ||
|
|
||
| // Guarantee 2: a second park() call, after the first full | ||
| // park+release cycle already consumed both channels, must not | ||
| // block. | ||
| tokio::time::timeout(Duration::from_secs(5), gate.park()) | ||
| .await | ||
| .expect("second park() call must return immediately, not block"); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Drive the parking guarantee through ParkingLlm, not private park().
This test validates ParkingModelGate::park() directly, but the seam that gates the model side effect is ParkingLlm::{complete, complete_with_tools}. A wrapper regression could still pass. Add a caller-level assertion that runs the parked provider under timeout. As per path instructions, “Test through the caller: when a helper gates a side effect, require a test driving the real call 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 `@tests/support/reborn/scripted_provider.rs` around lines 198 - 217, The test
is exercising ParkingModelGate::park() directly instead of the real call sites
that enforce the parking behavior. Update the test to drive the parked provider
through ParkingLlm::{complete, complete_with_tools} under a timeout so the
assertion covers the caller-facing seam, not the private gate. Keep the existing
lost-wakeup and second-call guarantees, but make them observable via the
public/model-facing API rather than calling park() directly.
Source: Path instructions
henrypark133
added a commit
that referenced
this pull request
Jul 2, 2026
Resolves conflicts with PR-E2 (#5514, E-SKILL/E-DURABLE/E-GATEWAY seam constructors) and E-TRIGGERED-SUBMIT (#5516), both landed on main after this branch was cut and both touching the same shared Reborn integration- test harness scaffolding (tests/support/reborn/{builder,group,harness, mod,assertions}.rs). Only builder.rs had textual conflict markers (kept both with_safety_context() and park_model()); the other four files auto-merged via 3-way merge and were manually verified to retain both sides' additions: - ours: safety_context (C-SAFETY), push_response_body/web_access_tools/ capability_backend.rs extraction/assert_egress_body_contains_any (C-WEBACCESS) - theirs: park_model, skill_activation_tools, triggered_submit module, E-SKILL/E-GATEWAY/E-DURABLE plumbing Two real compile breaks surfaced only by `cargo test --no-run` (workspace `cargo check` doesn't build test targets) and fixed: - builder.rs referenced GroupCapability (from main's E-DURABLE test) without it being imported - web_access_tools() was missing the skill_activation_source field main added to HostRuntimeCapabilityHarness Also fixed 4 stale doc-comment references to WebAccessTestHandler/ web_access_test_error, which were deleted by an earlier commit on this branch (production register_bundled_web_access_first_party_handlers is used directly instead) but the doc comments describing web_access_tools()/ WebAccessTools/the error-mapping test/WEB_ACCESS_PROVIDER_ID weren't all updated at the time — two were already flagged by PR review (Copilot, CodeRabbit), two more found during this merge. Verified: cargo check --workspace --all-features clean; all 9 relevant reborn_integration_* test binaries pass (web_access, safety, greeting, http_matcher, secret_injection, cancel, durable, skill_activate, triggered_submit — 218 total tests, 0 failed); cargo clippy -p ironclaw --tests --all-features zero warnings. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
henrypark133
added a commit
that referenced
this pull request
Jul 2, 2026
…L, C-DURABLE, C-ERRORS) Tier-2 coverage on the three seams PR-E2 (#5514) landed, extending their smoke drivers to real behavior rather than duplicating them. C-SKILL (extends E-SKILL): - reborn_integration_skill_activate.rs: new skill_auto_activates_via_criteria_without_explicit_capability_call, checked in #[ignore]'d. It surfaced a real product gap, not a test bug: SkillActivationMode::ActivationCriteria (keyword/regex auto-activation) is unreachable from the modern Reborn TurnCoordinator/agent-loop path today. SelectableSkillContextSource::load_skill_context_candidates only runs fresh criteria selection when take_message_for_run returns Some, which is populated exclusively by record_user_message/record_message (activation.rs). That function's only production caller is the legacy RebornRuntime::submit_user_turn (runtime.rs ~1924) — a different, older runtime, not the TurnCoordinator stack accept_inbound drives. Production skill_context_source wiring (build_reborn_runtime -> local_dev_filesystem_skill_context_source) hands the raw SelectableSkillContextSource to the loop driver with no message-recording decorator, same shape as this test harness. Escalated per the fold-in directive (architectural, touches the shared loop-ingress path) rather than fixed here; full root cause documented inline as a TODO. - New reborn_group_skills/ binary: skill_list/skill_install/skill_remove at int tier (previously QA/trace-tier only), reusing the same HostRuntimeCapabilityHarness::skill_management_tools() preset the trace-tier harness already wires (now pub(crate)) via a new RebornIntegrationGroup::skill_management_tools() constructor. C-DURABLE (extends E-DURABLE): approval-request and trigger state now provably survive an independent reopen of the on-disk local-dev capability store, paralleling assert_reply_persists_after_reopen and the existing extension-install durability test. New factory.rs test-support functions open_local_dev_approval_request_store_for_test / open_local_dev_trigger_repository_for_test compose only already-public production pieces (mount_default_local_dev_database_roots, wrap_scoped, FilesystemApprovalRequestStore::new, LibSqlTriggerRepository::new) — zero new business logic, #[cfg(test-support, libsql)] only. Extracted open_local_dev_libsql_database as the single owner of the libsql::Builder::new_local sequence (was about to become a third inline copy); production build_default_local_dev_database_roots now calls it too. New scenario_approval_request_persists_after_reopen.rs / scenario_trigger_persists_after_reopen.rs in the existing reborn_group_approvals / reborn_group_triggers binaries. C-ERRORS (extends E-GATEWAY): three new tests in reborn_integration_cancel.rs. - cancelled_run_does_not_block_a_second_turn_on_the_same_thread: regression guard (PR #5206 precedent) that cancelling a parked run releases whatever admission slot it held — no leak found. - busy_reject_when_thread_already_has_an_active_run: a second submit on a thread with an active run returns ProductInboundAck::RejectedBusy, not Accepted. New submit_turn_ack() in builder.rs (extracted from submit_turn_async, which now narrows it to Accepted) returns the raw ack. - mid_turn_provider_error_reaches_failed_with_model_error_category: a raw provider Err the decorator chain classifies non-retryable (LlmError::ContextLengthExceeded) reaches TurnStatus::Failed categorized "model_error". New ErrLlm provider in scripted_provider.rs (deliberately not the retryable RequestFailed a naturally-exhausted TraceLlm would return, to keep the test fast/deterministic) wired via a new fail_model knob mirroring park_gate's existing shape on both RebornThreadBuilder and RebornIntegrationHarnessBuilder. Structural: tests/support/reborn/group.rs was already over the 1000-line ceiling (1007) before this PR. Split first, per the file-size guardrail: extracted every per-capability preset constructor (live_approvals, builtin_tools, extension_lifecycle, live_auth_gate, project_lifecycle, profile_tools, triggers, skill_activation_tools, and the new skill_management_tools) into a new sibling group_constructors.rs — mechanics (build_base/into_group, now pub(crate)) stay in group.rs, "which capability" selection moves out. Mirrors the harness_mcp.rs split precedent. Every new test mutation-verified RED for the right reason (production mutation where applicable — dispatch_install output, both new factory.rs reopen paths pointed at a wrong subdir — or a self-check assertion flip otherwise), then reverted to a clean diff. clippy --all-features clean (zero warnings); all touched/new binaries green (reborn_group_{approvals,extensions,memory,triggers,skills}, reborn_integration_{cancel,durable,skill_activate,backend_matrix}). A pre-existing, unrelated parallel-execution flake in ironclaw_reborn_composition's own lib test suite (two `default_system_prompt` tests) was confirmed to reproduce identically on the pre-PR base commit (e798422) — not introduced by this change. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
henrypark133
added a commit
that referenced
this pull request
Jul 2, 2026
…L, C-DURABLE, C-ERRORS) Tier-2 coverage on the three seams PR-E2 (#5514) landed, extending their smoke drivers to real behavior rather than duplicating them. C-SKILL (extends E-SKILL): - reborn_integration_skill_activate.rs: new skill_auto_activates_via_criteria_without_explicit_capability_call, checked in #[ignore]'d. It surfaced a real product gap, not a test bug: SkillActivationMode::ActivationCriteria (keyword/regex auto-activation) is unreachable from the modern Reborn TurnCoordinator/agent-loop path today. SelectableSkillContextSource::load_skill_context_candidates only runs fresh criteria selection when take_message_for_run returns Some, which is populated exclusively by record_user_message/record_message (activation.rs). That function's only production caller is the legacy RebornRuntime::submit_user_turn (runtime.rs ~1924) — a different, older runtime, not the TurnCoordinator stack accept_inbound drives. Production skill_context_source wiring (build_reborn_runtime -> local_dev_filesystem_skill_context_source) hands the raw SelectableSkillContextSource to the loop driver with no message-recording decorator, same shape as this test harness. Escalated per the fold-in directive (architectural, touches the shared loop-ingress path) rather than fixed here; full root cause documented inline as a TODO. - New reborn_group_skills/ binary: skill_list/skill_install/skill_remove at int tier (previously QA/trace-tier only), reusing the same HostRuntimeCapabilityHarness::skill_management_tools() preset the trace-tier harness already wires (now pub(crate)) via a new RebornIntegrationGroup::skill_management_tools() constructor. C-DURABLE (extends E-DURABLE): approval-request and trigger state now provably survive an independent reopen of the on-disk local-dev capability store, paralleling assert_reply_persists_after_reopen and the existing extension-install durability test. New factory.rs test-support functions open_local_dev_approval_request_store_for_test / open_local_dev_trigger_repository_for_test compose only already-public production pieces (mount_default_local_dev_database_roots, wrap_scoped, FilesystemApprovalRequestStore::new, LibSqlTriggerRepository::new) — zero new business logic, #[cfg(test-support, libsql)] only. Extracted open_local_dev_libsql_database as the single owner of the libsql::Builder::new_local sequence (was about to become a third inline copy); production build_default_local_dev_database_roots now calls it too. New scenario_approval_request_persists_after_reopen.rs / scenario_trigger_persists_after_reopen.rs in the existing reborn_group_approvals / reborn_group_triggers binaries. C-ERRORS (extends E-GATEWAY): three new tests in reborn_integration_cancel.rs. - cancelled_run_does_not_block_a_second_turn_on_the_same_thread: regression guard (PR #5206 precedent) that cancelling a parked run releases whatever admission slot it held — no leak found. - busy_reject_when_thread_already_has_an_active_run: a second submit on a thread with an active run returns ProductInboundAck::RejectedBusy, not Accepted. New submit_turn_ack() in builder.rs (extracted from submit_turn_async, which now narrows it to Accepted) returns the raw ack. - mid_turn_provider_error_reaches_failed_with_model_error_category: a raw provider Err the decorator chain classifies non-retryable (LlmError::ContextLengthExceeded) reaches TurnStatus::Failed categorized "model_error". New ErrLlm provider in scripted_provider.rs (deliberately not the retryable RequestFailed a naturally-exhausted TraceLlm would return, to keep the test fast/deterministic) wired via a new fail_model knob mirroring park_gate's existing shape on both RebornThreadBuilder and RebornIntegrationHarnessBuilder. Structural: tests/support/reborn/group.rs was already over the 1000-line ceiling (1007) before this PR. Split first, per the file-size guardrail: extracted every per-capability preset constructor (live_approvals, builtin_tools, extension_lifecycle, live_auth_gate, project_lifecycle, profile_tools, triggers, skill_activation_tools, and the new skill_management_tools) into a new sibling group_constructors.rs — mechanics (build_base/into_group, now pub(crate)) stay in group.rs, "which capability" selection moves out. Mirrors the harness_mcp.rs split precedent. Every new test mutation-verified RED for the right reason (production mutation where applicable — dispatch_install output, both new factory.rs reopen paths pointed at a wrong subdir — or a self-check assertion flip otherwise), then reverted to a clean diff. clippy --all-features clean (zero warnings); all touched/new binaries green (reborn_group_{approvals,extensions,memory,triggers,skills}, reborn_integration_{cancel,durable,skill_activate,backend_matrix}). A pre-existing, unrelated parallel-execution flake in ironclaw_reborn_composition's own lib test suite (two `default_system_prompt` tests) was confirmed to reproduce identically on the pre-PR base commit (e798422) — not introduced by this change. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
henrypark133
added a commit
that referenced
this pull request
Jul 2, 2026
…C-DURABLE, C-ERRORS) (#5547) * test(reborn): PR-C2 int-tier coverage — skill/durable/gateway (C-SKILL, C-DURABLE, C-ERRORS) Tier-2 coverage on the three seams PR-E2 (#5514) landed, extending their smoke drivers to real behavior rather than duplicating them. C-SKILL (extends E-SKILL): - reborn_integration_skill_activate.rs: new skill_auto_activates_via_criteria_without_explicit_capability_call, checked in #[ignore]'d. It surfaced a real product gap, not a test bug: SkillActivationMode::ActivationCriteria (keyword/regex auto-activation) is unreachable from the modern Reborn TurnCoordinator/agent-loop path today. SelectableSkillContextSource::load_skill_context_candidates only runs fresh criteria selection when take_message_for_run returns Some, which is populated exclusively by record_user_message/record_message (activation.rs). That function's only production caller is the legacy RebornRuntime::submit_user_turn (runtime.rs ~1924) — a different, older runtime, not the TurnCoordinator stack accept_inbound drives. Production skill_context_source wiring (build_reborn_runtime -> local_dev_filesystem_skill_context_source) hands the raw SelectableSkillContextSource to the loop driver with no message-recording decorator, same shape as this test harness. Escalated per the fold-in directive (architectural, touches the shared loop-ingress path) rather than fixed here; full root cause documented inline as a TODO. - New reborn_group_skills/ binary: skill_list/skill_install/skill_remove at int tier (previously QA/trace-tier only), reusing the same HostRuntimeCapabilityHarness::skill_management_tools() preset the trace-tier harness already wires (now pub(crate)) via a new RebornIntegrationGroup::skill_management_tools() constructor. C-DURABLE (extends E-DURABLE): approval-request and trigger state now provably survive an independent reopen of the on-disk local-dev capability store, paralleling assert_reply_persists_after_reopen and the existing extension-install durability test. New factory.rs test-support functions open_local_dev_approval_request_store_for_test / open_local_dev_trigger_repository_for_test compose only already-public production pieces (mount_default_local_dev_database_roots, wrap_scoped, FilesystemApprovalRequestStore::new, LibSqlTriggerRepository::new) — zero new business logic, #[cfg(test-support, libsql)] only. Extracted open_local_dev_libsql_database as the single owner of the libsql::Builder::new_local sequence (was about to become a third inline copy); production build_default_local_dev_database_roots now calls it too. New scenario_approval_request_persists_after_reopen.rs / scenario_trigger_persists_after_reopen.rs in the existing reborn_group_approvals / reborn_group_triggers binaries. C-ERRORS (extends E-GATEWAY): three new tests in reborn_integration_cancel.rs. - cancelled_run_does_not_block_a_second_turn_on_the_same_thread: regression guard (PR #5206 precedent) that cancelling a parked run releases whatever admission slot it held — no leak found. - busy_reject_when_thread_already_has_an_active_run: a second submit on a thread with an active run returns ProductInboundAck::RejectedBusy, not Accepted. New submit_turn_ack() in builder.rs (extracted from submit_turn_async, which now narrows it to Accepted) returns the raw ack. - mid_turn_provider_error_reaches_failed_with_model_error_category: a raw provider Err the decorator chain classifies non-retryable (LlmError::ContextLengthExceeded) reaches TurnStatus::Failed categorized "model_error". New ErrLlm provider in scripted_provider.rs (deliberately not the retryable RequestFailed a naturally-exhausted TraceLlm would return, to keep the test fast/deterministic) wired via a new fail_model knob mirroring park_gate's existing shape on both RebornThreadBuilder and RebornIntegrationHarnessBuilder. Structural: tests/support/reborn/group.rs was already over the 1000-line ceiling (1007) before this PR. Split first, per the file-size guardrail: extracted every per-capability preset constructor (live_approvals, builtin_tools, extension_lifecycle, live_auth_gate, project_lifecycle, profile_tools, triggers, skill_activation_tools, and the new skill_management_tools) into a new sibling group_constructors.rs — mechanics (build_base/into_group, now pub(crate)) stay in group.rs, "which capability" selection moves out. Mirrors the harness_mcp.rs split precedent. Every new test mutation-verified RED for the right reason (production mutation where applicable — dispatch_install output, both new factory.rs reopen paths pointed at a wrong subdir — or a self-check assertion flip otherwise), then reverted to a clean diff. clippy --all-features clean (zero warnings); all touched/new binaries green (reborn_group_{approvals,extensions,memory,triggers,skills}, reborn_integration_{cancel,durable,skill_activate,backend_matrix}). A pre-existing, unrelated parallel-execution flake in ironclaw_reborn_composition's own lib test suite (two `default_system_prompt` tests) was confirmed to reproduce identically on the pre-PR base commit (e798422) — not introduced by this change. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * test(reborn): pin criteria auto-activation intentionally OFF on coordinator path Auto-activation via SkillActivationMode::ActivationCriteria is disabled on purpose for the modern TurnCoordinator path (product decision, closed #5530). Replace the #[ignore]'d gap-pinning test with a positive pin of the OFF state: a keyword-matching message alone must not inject the skill prompt. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * test(reborn): address review feedback on PR #5547 - Tighten the criteria-auto-activation-OFF assertion to check the specific "not found" error message instead of generic is_err(), since assert_model_request_contains has a second (JSON-serialize) Err path that could otherwise mask a false pass. - Drop the independently-reopened ApprovalRequestStore before the live harness resumes writing through the gate, avoiding two open libsql connections spanning a write. - Route live_auth_gate/project_lifecycle/profile_tools through the shared build_with_capability tail instead of hand-rolling build_base + into_group. - Replace RebornThreadBuilder's park_gate/fail_model pair with a single ThreadModelMode enum so the three per-thread model-call modes are mutually exclusive by construction instead of an unenforced priority rule (mirrors the existing ShellMode precedent in builder.rs). - Fix doc/code drift: the skill-activation module doc no longer contradicts the criteria-auto-activation-OFF test, the approvals group module doc lists all six scenarios in order, and the durable-store test-support summary labels approval/trigger reopen helpers C-DURABLE (matching the per-fn docs) instead of E-DURABLE. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * address review feedback round 2 - factory.rs: delegate open_local_dev_trigger_repository_for_test to the production local_dev_trigger_repository helper instead of hand-mirroring its construction+migration sequence. - group_constructors.rs: make it a private child module of group.rs (not a pub sibling from mod.rs) so GroupBaseData, canonical_binding, build_base, and into_group can go back to module-private visibility instead of pub(crate) across the whole test-support crate. - CLAUDE.md: document skill_management_tools() in the Available constructors table and add a group_constructors.rs entry to the Files map. - reborn_integration_cancel.rs: fix C-ERRORS module doc mismatch (three tests described as "other two rows"). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
36 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
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
Adds three integration-test seam constructors to the Reborn int tier, each shipped with a minimal test that actually drives it (no dead seams — PR-E1 lesson applied).
skill_activation_tools()+wrap_skill_activation_capability_for_test+skill_context_sourcewiringreborn_integration_skill_activateopen_local_dev_extension_installation_store_for_test(reopens fresh store at on-disk root)reborn_integration_durableParkingModelGate/ParkingLlm+cancel_runreborn_integration_cancelcancel_run(run reachesCompletednotCancelled)Notes
greetsystem skill so resolution is independent of run-scope owner (user-scoped filesystem resolution is already covered by theruntime.rssuite); the seam here only needs the skill to exist so the capability + context wiring can be driven. The test uses a user message without thegreetkeyword, so the injected sentinel can only originate from the explicitskill_activatecall.cancellation_factoryis intentionallyNone.RebornLoopDriverHostFactoryalways builds its own defaultTurnStateRunCancellationFactory(25ms cancel poll), which drives the parked run toCancelledon resume. Wiring an explicit factory here would only add the coordinator'sCompositeTurnRunWakeNotifierfan-out (product-live retained-run-handle observation), which this test does not exercise — so it would be dead, untested code.#[cfg(feature = "test-support")](zero prod bytes) with doc-comments naming the production call site each mirrors.Verification
cargo clippy --all --benches --tests --examples --all-features— zero code warnings.reborn_group_approvals+reborn_integration_profileall green with--features libsql.🤖 Generated with Claude Code