Repository navigation
test(reborn): add extension_activate int-tier scenario (T0-EXTACT) - #5433
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
📝 WalkthroughSummary by CodeRabbit
WalkthroughAdds a new cross-thread E2E scenario that installs ChangesCross-thread activation E2E scenario
Estimated code review effort: 2 (Simple) | ~10 minutes Possibly related PRs
🚥 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 |
github-actions
Bot
added
risk: low
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a new integration test scenario, scenario_activate_then_active_cross_thread, to verify cross-thread extension activation persistence. The test ensures that when an extension (specifically web-access) is installed in one thread and activated in another, a third thread correctly observes the extension as active with its capabilities published. There are no review comments, and I have no feedback to provide.
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.
![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: 94db3bbcd7
ℹ️ 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".
| // is the observable proof that activation published the extension's tool | ||
| // surface (mere install does NOT publish capabilities). | ||
| activator | ||
| .assert_tool_result_contains(r#""web-access.search""#) |
There was a problem hiding this comment.
This assertion only checks the extension_activate result payload for web-access.search; that field is derived from the package's visible_capability_ids, so it can still be present even if activation updates the lifecycle state but fails to publish the tool into the model-visible surface for later turns. In that scenario Thread C would still see installation_phase:"active", so the scenario would pass while the advertised capability is not actually available. Please assert the post-activation surface itself, e.g. by having a later turn verify/invoke web-access.search or otherwise inspect the advertised provider tools after activation.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Not valid for this code path. In commit_activation (extension_lifecycle.rs:552-579), active_extensions.publish(&active_package) runs before the success payload is built; if publish fails it rolls back (disable + restore previous activation state) and returns Err. So activated:true and visible_capability_ids are only ever emitted after publish succeeds — the failure mode described here (lifecycle state advanced but tool not published) short-circuits to an error and never produces this payload. The web-access.search assertion therefore already gates on publish success.
Asserting later-turn invocability of the published tool would test the publish→model-visible-surface wiring end-to-end, which is a distinct invariant from the cross-thread activation persistence this scenario covers (and is out of scope for this coverage PR). Keeping the assertion as-is.
|
🚅 Deployed to the ironclaw-pr-5433 environment in ironclaw-ci-preview
|
Close the `extension_activate` integration-tier coverage gap in `reborn_group_extensions`: install/search/remove already had cross-thread coverage, activation did not. Scenario `activate_then_active_cross_thread` installs the credential-free "web-access" bundled extension in thread A, activates it in thread B (a different conversation over the shared HostRuntimeCapabilityHarness store), and searches in thread C asserting the extension now renders `installation_phase:"active"`. It also asserts the activate payload carries `activated:true` and publishes the `web-access.search` capability surface, with a discriminating guard (absence of `installation_phase:"installed"`) and a non-vacuity guard. "web-access" is used because it is the only bundled extension that activates without raising a credential auth gate, and it is untouched by the existing github/notion scenarios. Mutation-verified: forcing `commit_activation` to leave the installation in the `Installed` state turns the scenario RED with `no recorded capability result containing "installation_phase":"active"`, while the sibling install/remove scenarios stay green. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
No behavior change: rustfmt reordered the mod declarations and wrapped a long builder chain in scenario_activate_then_active_cross_thread.rs. This is a style-only commit on a test-only PR, so no new regression test is added — the touched test scenario itself is the coverage. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
94db3bb to
7e96b50
Compare
There was a problem hiding this comment.
Pull request overview
Adds missing integration-tier coverage for cross-thread builtin.extension_activate persistence in the Reborn extension lifecycle group, ensuring activation is durably visible across distinct conversations sharing the same HostRuntimeCapabilityHarness.
Changes:
- Added Scenario 3 to install
web-accessin one conversation, activate it in a second, and assertinstallation_phase:"active"via search in a third. - Added assertions that activation returns
"activated":trueand publishes at least one expected capability ID (web-access.search). - Registered the new scenario in the
reborn_group_extensionsorchestrator test.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| tests/reborn_group_extensions/scenario_activate_then_active_cross_thread.rs | New cross-thread install→activate→search scenario covering extension_activate at the integration tier. |
| tests/reborn_group_extensions/main.rs | Wires Scenario 3 into the group runner and documents the intended execution order. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
henrypark133
left a comment
henrypark133
left a comment
There was a problem hiding this comment.
Code Review (multi-agent)
Intent: Add integration-tier Reborn extension_activate coverage for cross-thread install, activation, capability publication, and active persistence.
Stats: 0 net-new findings (2 raw reviewer findings, both already covered by unresolved live review threads) across 0 new files. Reviewers run: security, bugs, performance, tests, conventions, local-patterns, maintainability, approach. Reviewers failed: none. Body-only: 0. Inline comments: 0.
Duplicate-Suppressed Findings
- Medium Cross-thread activation does not prove the published tool is callable (
tests/reborn_group_extensions/scenario_activate_then_active_cross_thread.rs:85-90, confidence 75) — already covered by unresolved threadPRRT_kwDORHZ7Z86NLNckat line 90. - Low Module-order comment contradicts the declarations (
tests/reborn_group_extensions/main.rs:20-24, confidence 100) — already covered by unresolved threadPRRT_kwDORHZ7Z86NcAx5at line 24.
No additional blocker or merge-risk finding survived the forced multi-agent pass beyond those existing unresolved threads.
Copilot PR review flagged the module-order comment in tests/reborn_group_extensions/main.rs claiming "install → remove → activate" while the mod declarations were alphabetical (activate first). Reorder the three scenario mod decls to match the stated execution order. Mod declaration order is compile-irrelevant; no behavior change. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…XTACT) Prior commit reordered the scenario `mod` decls to match the comment, but rustfmt enforces alphabetical `mod` ordering (reorder_modules), so `cargo fmt --all --check` failed in CI. Revert to alphabetical order and fix the comment instead: it now states decls are alphabetical (rustfmt) and that execution order (install → remove → activate) is defined by the report.record call sequence in extensions_group_e2e, not decl order. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
henrypark133
left a comment
henrypark133
left a comment
There was a problem hiding this comment.
Code Review (multi-agent)
Intent: Add integration coverage for extension_activate cross-thread lifecycle persistence and capability surface publication.
Stats: 0 findings (from 0 raw, 0 after dedup) across 0 files. Reviewers run: security, bugs, performance, tests, conventions, local-patterns, maintainability, approach. Reviewers failed: none. Body-only: 0.
All eight reviewer lenses returned no actionable findings on the refreshed head c4cd41fbfa480bfff01e5487a9494e4fde35d6fa.
Note: I re-ran the review after the PR head moved from e54f3d48c02355ee8a7e4c7925a3e38bf44bb68f to c4cd41fbfa480bfff01e5487a9494e4fde35d6fa, so this result is attached to the current head. GitHub rejected APPROVE on this self-authored PR, so this is posted as COMMENT instead.
What
Closes the T0-EXTACT coverage gap from the reborn backend coverage roadmap:
reborn_group_extensionscoveredextension_search/install/removeat the integration tier cross-thread, butextension_activatehad no int-tier coverage. The tools were already wired in theextension_lifecycle()group harness; this PR is pure authoring.Scenario added
tests/reborn_group_extensions/scenario_activate_then_active_cross_thread.rs(registered as Scenario 3 inmain.rs), mirroring the two existing cross-thread lifecycle scenarios:web-accessbundled extension → asserts"installed":true.HostRuntimeCapabilityHarness) activates it → asserts"activated":trueand that the activate payload'svisible_capability_idspublishesweb-access.search(the capability surface coming online).web-access→ asserts"installation_phase":"active", proving the activation persisted cross-thread.Guards: a discriminating guard (absence of
installation_phase:"installed") catches a no-op activation, and a non-vacuity guard requires theweb-accesscatalog entry to appear so the active-phase assertion is meaningful.Why
web-accessIt is the only bundled extension that activates without raising a credential auth gate (confirmed by
local_dev_extension_activate_returns_auth_gate_for_missing_extension_credentials), so activation reaches a SUCCESS result rather than blocking on credentials. It is also untouched by the existinggithub/notionscenarios, so the install→activate cycle is a genuine fresh transition over the shared store.Mutation proof
Forcing
commit_activation(extension_lifecycle.rs) to persistExtensionActivationState::Installedinstead ofEnabled(activation no-op) turns only this scenario RED with:The sibling install/remove scenarios stay green. Reverted before commit.
Verification
cargo clippy --all --tests --all-features -- -D warnings— cleanreborn_group_extensionsgreen: 3/3 under--features libsql, 5/5 under--all-featurestests/support/reborn/group.rs:498,driver_protocol_violation, surfaced by T0-CI) — NOT this scenario's assertion (which produces the distinctinstallation_phase:"active"message seen during mutation testing).group.rswas not touched.Notes / deferred
web-access.searchcapability-surface assertion because the task explicitly requires asserting "its capability surfaces"; its comment notes it is a distinct (publication) invariant from cross-thread persistence.🤖 Generated with Claude Code