Skip to content

Fix main Reborn Playwright failures - #6546

Closed
ilblackdragon wants to merge 1 commit into
mainfrom
fix-main-reborn-playwright
Closed

ilblackdragon wants to merge 1 commit into
mainfrom
fix-main-reborn-playwright

Conversation

@ilblackdragon

@ilblackdragon ilblackdragon commented Jul 23, 2026 •

Copy link
Copy Markdown
Member

Summary

  • Fix WebUI ProductSurface extension lifecycle gestures by granting the lifecycle filesystem mount and using a fresh activity id for non-idempotent user-gesture capability calls.
  • Fix extension removal after hosted MCP discovery by unpublishing the actual active package, including after an idempotent absent-remove cleanup path.
  • Update Reborn Playwright expectations for retired channel discovery routes, extension channel surfaces, approval decline payloads, GitHub casing, attachment limits, and current extension UI copy.

Change Type

  • Bug fix
  • New feature
  • Refactor
  • Documentation
  • CI/Infrastructure
  • Security
  • Dependencies

Linked Issue

None.

Validation

  • cargo fmt --all -- --check
  • cargo clippy --all --benches --tests --examples --all-features -- -D warnings
  • cargo build
  • Relevant tests pass: see commands below
  • cargo test --features integration if database-backed or integration behavior changed: Not applicable: this fixes WebUI/product capability and browser CI behavior, not DB integration behavior.
  • Manual testing: ran the full Reborn Playwright workflow matrix locally.
  • If a coding agent was used and supports it, review-pr or pr-shepherd --fix was run before requesting review

Test Strategy

User behavior:
WebUI users can install, activate, and remove extensions through ProductSurface-backed gestures without stale ProductResult collisions or missing lifecycle mounts. Browser-visible copy and route expectations now match current Reborn WebUI behavior.

Risk areas:

  • Model behavior
  • Browser
  • Side effect
  • Persistence
  • Security or permissions
  • External provider
  • Cross-component behavior

Tests added or updated:

  • Unit or contract: product_invocation_mounts_*, extension lifecycle remove regressions, WebUI facade ProductSurface lifecycle regression, WebUI handler contract tests.
  • Reborn integration: Not applicable: this is ProductSurface/WebUI ingress and extension lifecycle behavior, covered at crate/facade/browser seams.
  • Recorded fixture: Not applicable: no model request-shape or tool-choice behavior changed.
  • Browser E2E: updated Reborn Playwright shards for served API routes, legacy auth/input behavior, settings/extensions behavior, and ran the full matrix locally.
  • Backend or runtime: targeted ironclaw_reborn_composition and ironclaw_webui cargo tests plus clippy.
  • Live canary: Not applicable: no live provider behavior changed.

What the tests prove:
ProductSurface extension lifecycle calls have the mount authority they need, repeated same-input user gestures do not collide on ProductResult activity ids, hosted MCP removal unpublishes discovered active packages, and all Reborn Playwright shards pass against current UI/API contracts.

Commands run:

  • cargo fmt --all -- --check
  • git diff --check
  • cargo build -p ironclaw --bin ironclaw
  • cargo clippy -p ironclaw_reborn_composition -p ironclaw_webui --all-targets --all-features -- -D warnings
  • cargo test -p ironclaw_architecture --test reborn_extension_specificity reborn_generic_code_names_no_concrete_extension
  • corepack pnpm test -- pages/chat/components/auth-oauth-card.test.ts pages/chat/lib/attachments.test.ts
  • cargo test -p ironclaw_reborn_composition product_invocation_mounts --lib
  • cargo test -p ironclaw_reborn_composition product_surface_extension_lifecycle_remove_succeeds_after_activation --lib
  • cargo test -p ironclaw_reborn_composition hosted_mcp_remove_unpublishes_discovered_active_package_after_absent_cleanup --lib
  • cargo test -p ironclaw_reborn_composition first_party_extension_remove_succeeds_after_absent_cleanup_reinstall_and_activate --lib
  • cargo test -p ironclaw_reborn_composition local_dev_extension_lifecycle_tools_manage_visible_extension_surface --lib
  • cargo test -p ironclaw_webui --test webui_v2_handlers_contract activate_and_remove_extension_decode_path_package_id_to_lifecycle_paths -- --nocapture
  • cargo test -p ironclaw_webui --test webui_v2_handlers_contract settings_tool -- --nocapture
  • cargo test -p ironclaw_webui --test webui_v2_handlers_contract set_outbound_preferences_dispatches_body_through_invoke -- --nocapture
  • pytest tests/e2e/scenarios/test_reborn_webui_v2_smoke.py tests/e2e/scenarios/test_reborn_v2_file_download.py -v --timeout=120 --durations=25
  • pytest tests/e2e/scenarios/test_reborn_webui_v2_automation_trace_outbound_api.py tests/e2e/scenarios/test_reborn_webui_v2_extensions_api.py tests/e2e/scenarios/test_reborn_webui_v2_filesystem_api.py tests/e2e/scenarios/test_reborn_webui_v2_operator_api.py tests/e2e/scenarios/test_reborn_webui_v2_product_auth_api.py tests/e2e/scenarios/test_reborn_webui_v2_session_api.py tests/e2e/scenarios/test_reborn_webui_v2_skills_api.py tests/e2e/scenarios/test_reborn_webui_v2_streaming_run_control_api.py -v --timeout=120 --durations=25
  • pytest tests/e2e/scenarios/test_reborn_webui_v2_legacy_core.py tests/e2e/scenarios/test_reborn_webui_v2_legacy_rendering.py tests/e2e/scenarios/test_reborn_webui_v2_legacy_chat_actions.py tests/e2e/scenarios/test_reborn_webui_v2_legacy_dom_resource_limits.py -v --timeout=120 --durations=25
  • pytest tests/e2e/scenarios/test_reborn_webui_v2_legacy_approval.py tests/e2e/scenarios/test_reborn_webui_v2_legacy_auth_flows.py tests/e2e/scenarios/test_reborn_webui_v2_legacy_attachments.py -v --timeout=120 --durations=25
  • pytest tests/e2e/scenarios/test_reborn_webui_v2_legacy_extensions.py tests/e2e/scenarios/test_reborn_webui_v2_legacy_settings_search.py tests/e2e/scenarios/test_reborn_webui_v2_legacy_skills.py tests/e2e/scenarios/test_reborn_webui_v2_legacy_tool_permissions.py tests/e2e/scenarios/test_reborn_webui_v2_legacy_projects.py -k 'not test_reborn_legacy_always_approve_survives_reborn_restart' -v --timeout=120 --durations=25
  • pytest tests/e2e/scenarios/test_reborn_webui_v2_legacy_message_persistence.py tests/e2e/scenarios/test_reborn_webui_v2_legacy_pending_messages.py tests/e2e/scenarios/test_reborn_webui_v2_legacy_sse_history.py tests/e2e/scenarios/test_reborn_webui_v2_legacy_tool_execution.py -v --timeout=120 --durations=25

Security Impact

Changes ProductSurface mount grants for extension install/activate/remove so those lifecycle capabilities receive /system/extensions authority through the existing lifecycle mount helper. No new network calls, secrets, auth bypasses, or sandbox policy changes.

Reborn Trust-Boundary Checklist

  • Public policy/evidence/trust-bearing types: who can construct them? Not changed; this only routes existing ProductSurface lifecycle capabilities through existing mount authority.
  • Untrusted content enters prompts only through an envelope/escaping primitive. Not applicable: no prompt path changes.
  • Hashes declare purpose; trust/binding/authenticity uses SHA-256/BLAKE3 or separate authenticity check. Not applicable: removed deterministic activity-id hashing for user gestures.
  • New/changed status, exit, policy, runtime, or error variants: downstream match sites audited. Command/output: no variants added or changed.
  • Security/durability serde(default) fields fail closed or have migration tests. Not applicable: no serde schema changes.
  • Queues/maps/buffers/counters have bounds and overflow-safe arithmetic. Not applicable: no queue/map/buffer/counter bounds changed.
  • Driver/operator-visible errors have stable class semantics (Transient, Permanent, Misconfigured, PolicyDenied or equivalent). Not applicable: no new error classes.
  • Sandbox/native/host names accurately describe trust boundary. Existing ProductSurface and lifecycle naming retained.

Database Impact

None.

Blast Radius

Touches WebUI ProductSurface capability invocation, extension lifecycle removal, Reborn Playwright browser/API expectations, and small frontend fallback copy/limit behavior. A regression would most likely show up as extension install/activate/remove failure, repeated ProductSurface gesture result reuse, or stale WebUI test expectations.

Rollback Plan

Revert this PR. That restores the previous deterministic WebUI ProductSurface activity ids, previous extension lifecycle unpublish behavior, and prior Playwright expectations.

Review Follow-Through

Known follow-up: the full workspace clippy command was not run locally; targeted touched-crate clippy and the full Reborn Playwright matrix were run. Main branch currently has issue-comment-triggered nearai-bench failures unrelated to push CI; the scheduled Reborn Playwright failure being fixed was on pre-current-main SHA 62c5ad08.


Review track: C (runtime/permissions/browser CI)

@gemini-code-assist

Copy link
Copy Markdown
Contributor

Caution

The consumer version of Gemini Code Assist on GitHub has been sunset. All code review activity has officially ceased.

@ironloopai

ironloopai Bot commented Jul 23, 2026 •

Copy link
Copy Markdown
Contributor

🔎 IronLoop Review Status

Head: 755534eacf13cb0f34941b9b2262fadd4cc73866
Result: One or more review results were superseded by a newer PR head.
Next: Run @ironloopai review on the latest PR head.
Updated: 2026-07-23T07:07:38.448Z

Current reviewers:

Reviewer State Verdict Findings Last update
ironloop/common-reviewer (reviewer) Superseded N/A N/A 2026-07-23T07:07:38.436Z
Reviewer summaries
Reviewer Detail
ironloop/common-reviewer (reviewer) Superseded by a newer PR head. New head: 755534e. Previous verdict: Changes requested.
Recent activity
Time Reviewer State Detail
2026-07-23T06:06:26.983Z ironloop/common-reviewer (reviewer) Superseded A newer PR head replaced this review (55828ad).
2026-07-23T06:31:21.395Z ironloop/common-reviewer (reviewer) Queued Accepted review request for head d907d9a.
2026-07-23T06:31:21.395Z ironloop/common-reviewer (reviewer) Queued Waiting for this reviewer lane to become available.
2026-07-23T06:31:21.487Z ironloop/common-reviewer (reviewer) Started Reviewer worker started.
2026-07-23T06:31:24.816Z ironloop/common-reviewer (reviewer) Workspace ready Prepared isolated checkout (merge_ref) at 266db98.
2026-07-23T06:38:15.814Z ironloop/common-reviewer (reviewer) Result captured Changes requested; 1 blocking finding.
2026-07-23T06:38:15.814Z ironloop/common-reviewer (reviewer) Completed Review completed and terminal status was persisted.
2026-07-23T07:07:38.436Z ironloop/common-reviewer (reviewer) Superseded A newer PR head replaced this review (755534e).
Available commands
  • @ironloopai help
  • @ironloopai agents
  • @ironloopai review
  • @ironloopai review --agent <agent>
Run metadata

Admission: webhook accepted the request and IronLoop persisted reviewer state before this projection.

@railway-app
railway-app Bot temporarily deployed to ironclaw-ci-preview / ironclaw-pr-6546 July 23, 2026 06:00 Destroyed
@coderabbitai

coderabbitai Bot commented Jul 23, 2026 •

Copy link
Copy Markdown

Review Change Stack

📝 Walkthrough

Summary by CodeRabbit

  • New Features

    • Extension lifecycle actions now reliably support repeated removal, including after activation, and improve action tracking across retries.
    • Increased the temporary per-file attachment size limit from 5 MB to 10 MB.
    • Provider names now include standardized display for GitHub (e.g., “GitHub”).
  • Bug Fixes

    • Improved cleanup and state restoration for active extension publications during removal.
    • Updated legacy approval/auth outcomes to report cancel/deny as “declined.”
    • Refined extension catalog messaging and Telegram pairing flow expectations.
  • Tests

    • Expanded WebUI and E2E coverage for removal idempotency, lifecycle behavior, attachment limits, and updated UI/request contracts.

Walkthrough

Extension lifecycle removal now uses the active registry package, WebUI lifecycle requests propagate client action IDs into explicit activity IDs, and frontend/E2E contracts update for lifecycle payloads, labels, limits, resolutions, and extension surfaces.

Changes

Extension lifecycle and idempotency

Layer / File(s) Summary
Active-package removal and cleanup
crates/ironclaw_reborn_composition/src/extension_host/extension_lifecycle.rs, crates/ironclaw_reborn_composition/src/extension_host/extension_lifecycle_capabilities.rs
Removal snapshots the active package for unpublishing and compensation, with coverage for already-absent removal and hosted-MCP cleanup.
Product lifecycle capability surface
crates/ironclaw_reborn_composition/src/webui/product_capability.rs, crates/ironclaw_reborn_composition/src/webui/facade/tests.rs
Extension lifecycle capability IDs receive lifecycle mounts and are tested through remove, install, activate, and remove operations.

WebUI activity and request contracts

Layer / File(s) Summary
Client-action request contracts
crates/ironclaw_product_workflow/src/webui_inbound.rs, crates/ironclaw_webui/src/webui_v2/handlers.rs, crates/ironclaw_product_workflow/tests/...
Setup and lifecycle request DTOs accept optional client action IDs, with shared parsing and updated request fixtures.
Deterministic activity-ID routing
crates/ironclaw_webui/src/webui_v2/handlers.rs
Lifecycle, outbound-preferences, and LLM-provider handlers use explicit activity-ID derivation; generic invocation uses a fresh ID.
Frontend lifecycle payloads
crates/ironclaw_webui/frontend/src/lib/..., crates/ironclaw_webui/frontend/src/pages/extensions/lib/..., crates/ironclaw_webui/tests/..., tests/e2e/scenarios/test_reborn_*extensions*.py
Extension mutation and setup requests include client action IDs, with unit, contract, and E2E coverage.

Outbound type wiring

Layer / File(s) Summary
Outbound registration outcome imports
crates/ironclaw_reborn_composition/src/outbound/mod.rs, crates/ironclaw_reborn_composition/src/runtime.rs, crates/ironclaw_reborn_composition/src/runtime/tests/outbound_delivery.rs
OutboundDeliveryTargetRegistrationOutcome is sourced directly from ironclaw_outbound.

Frontend and E2E contract updates

Layer / File(s) Summary
Frontend display and attachment limits
crates/ironclaw_webui/frontend/src/pages/chat/..., tests/e2e/scenarios/test_reborn_webui_v2_legacy_attachments.py
Provider display names include GitHub, and the fallback attachment limit and matching fixture expectation are 10 MB.
Approval and served endpoint expectations
tests/e2e/scenarios/test_reborn_webui_v2_automation_trace_outbound_api.py, tests/e2e/scenarios/test_reborn_webui_v2_legacy_approval.py, tests/e2e/scenarios/test_reborn_webui_v2_legacy_auth_flows.py
Resolution expectations use declined, and connectable-channel endpoint assertions are removed.
Extension E2E fixtures and assertions
tests/e2e/scenarios/test_reborn_webui_v2_legacy_extensions.py, tests/e2e/scenarios/test_reborn_webui_v2_legacy_settings_search.py
Extension request assertions, catalog and pairing expectations, and mocked channel/MCP surfaces are updated.

Estimated code review effort: 4 (Complex) | ~60 minutes

Possibly related issues

Possibly related PRs

Suggested reviewers: serrrfirat

🚥 Pre-merge checks | ✅ 4
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title is clearly related to the PR’s main fix: Reborn Playwright failures in WebUI extension lifecycle behavior.
Description check ✅ Passed The description matches the template closely and fills the required sections with concrete summary, validation, impact, and rollback details.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@github-actions github-actions Bot added size: L 200-499 changed lines risk: low Changes to docs, tests, or low-risk modules contributor: core 20+ merged PRs labels Jul 23, 2026

@ironloopai ironloopai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ IronLoop Review: reviewer

Review at a glance

Verdict Blocking Notes Inline Head
✅ Approved 0 0 0 31f99cdf3717

Head: 31f99cdf3717aab003805bb1cd64b5332461ab47
Next: No reviewer action needed.

Run details

Status: Current
Needs human: no
Needs validation: no

Summary

Reviewed the complete 16-file (+365/-82) PR diff across extension lifecycle, ProductSurface, WebUI handlers, frontend, and E2E expectations. No actionable regression found.

Findings

None.

Developer follow-up

After fixing this feedback:

  1. Push the fix to this PR branch.
  2. Re-run this reviewer with @ironloopai review --agent reviewer if you only changed this reviewer's findings.
  3. Re-run all reviewers with @ironloopai review when the fix may affect multiple areas.

@ilblackdragon
ilblackdragon force-pushed the fix-main-reborn-playwright branch from 31f99cd to 55828ad Compare July 23, 2026 06:06
@railway-app
railway-app Bot temporarily deployed to ironclaw-ci-preview / ironclaw-pr-6546 July 23, 2026 06:06 Destroyed
@railway-app

railway-app Bot commented Jul 23, 2026 •

Copy link
Copy Markdown

🚅 Deployed to the ironclaw-pr-6546 environment in ironclaw-ci-preview

Service Status Web Updated (UTC)
ironclaw ✅ Success (View Logs) Web Jul 23, 2026 at 6:25 am

@ilblackdragon
ilblackdragon force-pushed the fix-main-reborn-playwright branch from 55828ad to d907d9a Compare July 23, 2026 06:15
@railway-app
railway-app Bot temporarily deployed to ironclaw-ci-preview / ironclaw-pr-6546 July 23, 2026 06:15 Destroyed
@github-actions

Copy link
Copy Markdown
Contributor

Coverage ratchet

Ratchet mode: ENFORCING

RATCHET PASS: global
  observed: 86.33% (309775 / 358834 lines)
  floor:    86.27% (tolerance 0.5pp -> effective floor 85.77%)
  denominator: 358834 lines now vs 354049 at floor capture (+4785 lines, +1.35%) — not a material change

⚠️ 2 Reborn crate(s) have 0 int-tier coverage (target: 0) — ironclaw_prompt_envelope, ironclaw_scripts

Reborn integration-tier coverage

Line coverage (Reborn crates): 86.33% — 309775 / 358834 lines

Per-crate breakdown (62 crates, lowest-covered first)
Crate Line % Covered / Total
ironclaw_prompt_envelope 0% 0 / 88
ironclaw_scripts 0% 0 / 345
ironclaw_telegram_extension 34.88% 60 / 172
ironclaw_event_projections 43.31% 673 / 1554
ironclaw_product_context 57.78% 26 / 45
ironclaw_dispatcher 60% 72 / 120
ironclaw_observability 61.54% 16 / 26
ironclaw_authorization 62.98% 609 / 967
ironclaw_memory 69.2% 773 / 1117
ironclaw_trust 72.88% 661 / 907
ironclaw_filesystem 73.5% 4543 / 6181
ironclaw_capabilities 74.07% 2717 / 3668
ironclaw_wasm_limiter 74.6% 47 / 63
ironclaw_extractors 74.72% 538 / 720
ironclaw_projects 76.48% 400 / 523
ironclaw_triggers 77.33% 2531 / 3273
ironclaw_mcp 77.56% 736 / 949
ironclaw_reborn_cli 78.25% 10379 / 13264
ironclaw_llm 78.63% 20824 / 26485
ironclaw_wasm 79.72% 735 / 922
ironclaw_process_sandbox 80.46% 671 / 834
ironclaw_memory_native 81.17% 3195 / 3936
ironclaw_events 81.95% 1594 / 1945
ironclaw_first_party_extensions 82.34% 6620 / 8040
ironclaw_reborn_event_store 83.03% 1169 / 1408
ironclaw_telegram_v2_adapter 83.07% 2017 / 2428
ironclaw_product_adapter_registry 83.43% 574 / 688
ironclaw_processes 83.78% 940 / 1122
ironclaw_reborn_identity 83.8% 450 / 537
ironclaw_secrets 83.8% 2550 / 3043
ironclaw_reborn_config 84.17% 1962 / 2331
ironclaw_common 84.48% 1769 / 2094
ironclaw_product_adapters 84.65% 3308 / 3908
ironclaw_auth 85.01% 4011 / 4718
ironclaw_run_state 85.61% 458 / 535
ironclaw_network 85.97% 913 / 1062
ironclaw_product_workflow 86.51% 16271 / 18809
ironclaw_hooks 86.6% 9931 / 11468
ironclaw_extensions 87.02% 3795 / 4361
ironclaw_threads 87.2% 4851 / 5563
ironclaw_host_api 87.55% 5407 / 6176
ironclaw_skills 87.58% 4470 / 5104
ironclaw_reborn_traces 88.11% 11972 / 13587
ironclaw_reborn_composition 88.21% 57482 / 65168
ironclaw_turns 88.63% 14581 / 16452
ironclaw_host_runtime 88.66% 18459 / 20819
ironclaw_reborn_openai_compat 88.92% 3580 / 4026
ironclaw_slack_extension 89.18% 2439 / 2735
ironclaw_extension_host 89.59% 2856 / 3188
ironclaw_approvals 90.18% 1598 / 1772
ironclaw_conversations 90.39% 3123 / 3455
ironclaw_webui 90.43% 9111 / 10075
ironclaw_resources 90.85% 4477 / 4928
ironclaw_event_streams 91.24% 1063 / 1165
ironclaw_runner 91.27% 17073 / 18707
ironclaw_loop_host 92.26% 16260 / 17624
ironclaw_attachments 93.06% 630 / 677
ironclaw_outbound 93.98% 3733 / 3972
ironclaw_agent_loop 94.93% 9840 / 10365
ironclaw_safety 95.15% 3749 / 3940
ironclaw_first_party_extension_ports 95.62% 3672 / 3840
ironclaw_runtime_policy 96.55% 811 / 840

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)
Module / Crate Reason Issue
crate: ironclaw_embeddings v1-only: consumed only by root ironclaw (src/app.rs, src/tools/builtin/memory.rs, src/workspace/mod.rs, src/config/{mod,embeddings}.rs); no crates/* dependents. Covered by "Tests (Legacy)". #5657
crate: ironclaw_gateway v1-only: consumed only by root ironclaw (src/channels/web/platform/static_files.rs, src/channels/web/handlers/frontend.rs); no crates/* dependents. Covered by "Tests (Legacy)". #5657
crate: ironclaw_tui v1-only: consumed only by root ironclaw (src/main.rs, src/channels/tui.rs); no crates/* dependents. Crate's own doc comment confirms it bridges INTO v1, not Reborn. Covered by "Tests (Legacy)". #5657

@ilblackdragon
ilblackdragon marked this pull request as ready for review July 23, 2026 06:31
@gemini-code-assist

Copy link
Copy Markdown
Contributor

Caution

The consumer version of Gemini Code Assist on GitHub has been sunset. All code review activity has officially ceased.

@ironloopai ironloopai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

❌ IronLoop Review: reviewer

Review at a glance

Verdict Blocking Notes Inline Head
❌ Changes requested 1 0 1 d907d9aa866b

Head: d907d9aa866b3c70688be2f494dfe6ab959fd722
Next: Fix the blocking findings, push the PR branch, then re-run this reviewer.

Run details

Status: Current
Needs human: no
Needs validation: no

Summary

One blocking idempotency regression: generic WebUI capability mutations now generate a new activity ID on every HTTP attempt.

Findings

Blocking: 1 / Notes: 0

Blocking findings

1. ❌ [MEDIUM] Preserve activity IDs across retries

Location: crates/ironclaw_webui/src/webui_v2/handlers.rs:2705
ActivityId is the ProductSurface mutation idempotency identity and must be preserved across retries. Generating it server-side for every request means a response-lost retry of a generic mutation (including install/activate/remove) is indistinguishable from a new gesture and receives a new identity. The extension frontend posts no client action key, so it cannot preserve one across retries. Generate a per-gesture key in the client and forward/reuse it for retries, while generating a distinct key for a later user gesture.

Developer follow-up

After fixing this feedback:

  1. Push the fix to this PR branch.
  2. Re-run this reviewer with @ironloopai review --agent reviewer if you only changed this reviewer's findings.
  3. Re-run all reviewers with @ironloopai review when the fix may affect multiple areas.

caller,
capability,
input,
ActivityId::new(),

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ActivityId is the ProductSurface mutation idempotency identity and must survive retries. A fresh server-side ID makes a response-lost retry of any generic mutation a new operation; the extension client sends no action key that it can reuse. Please carry a per-gesture client key through this path, reusing it only for retries.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 3

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (2)
crates/ironclaw_reborn_composition/src/webui/product_capability.rs (1)

244-266: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick win

Production extension-lifecycle web product calls use the local-dev lifecycle mount path.

ProductCapabilityMounts::Production branches only for skill-management in product_invocation_mounts; EXTENSION_INSTALL/ACTIVATE/REMOVE still call crate::local_dev_mounts::system_extensions_lifecycle_mount_view(), while production production-runnables get a production ProductResultFilesystem through RuntimeProductCapabilityInvoker::from_services. The current tests cover only LocalDev, so the path is not regression-tested for production invocations. Violates the composition/product boundary invariant: production capability wiring should not reuse local-dev-only product invocation mounts.

🤖 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 `@crates/ironclaw_reborn_composition/src/webui/product_capability.rs` around
lines 244 - 266, The product_invocation_mounts function routes
extension-lifecycle capabilities through the local-dev mount path regardless of
ProductCapabilityMounts. Update this branch to select the production lifecycle
mount implementation for ProductCapabilityMounts::Production while preserving
the existing local-dev behavior, and add coverage for production
extension-lifecycle invocations.

Source: Path instructions

crates/ironclaw_webui/src/webui_v2/handlers.rs (1)

2725-2797: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Duplicate caller/capability seed-prefix logic across the two new functions.

llm_provider_upsert_activity_id and outbound_preferences_activity_id repeat an identical 6-segment prefix (context label, tenant/user/agent/project, capability_id). Extract a shared helper so the two seed derivations can't silently diverge on caller-scope isolation.

♻️ Proposed extraction
+fn seed_caller_prefix(
+    context: &'static str,
+    caller: &WebUiAuthenticatedCaller,
+    capability_id: &CapabilityId,
+) -> Vec<u8> {
+    let mut seed = Vec::new();
+    for segment in [
+        context,
+        caller.tenant_id.as_str(),
+        caller.user_id.as_str(),
+        caller.agent_id.as_ref().map(|id| id.as_str()).unwrap_or(""),
+        caller
+            .project_id
+            .as_ref()
+            .map(|id| id.as_str())
+            .unwrap_or(""),
+        capability_id.as_str(),
+    ] {
+        seed.extend_from_slice(&(segment.len() as u64).to_be_bytes());
+        seed.extend_from_slice(segment.as_bytes());
+    }
+    seed
+}

Then each function calls seed_caller_prefix(...) and appends only its own request-specific fields.

🤖 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 `@crates/ironclaw_webui/src/webui_v2/handlers.rs` around lines 2725 - 2797,
Extract the duplicated caller/capability seed-prefix construction from
llm_provider_upsert_activity_id and outbound_preferences_activity_id into a
shared seed_caller_prefix helper. Have both functions reuse it, passing their
distinct context label and capability_id, then append only their
request-specific fields while preserving the existing segment ordering and
length-prefix encoding.
🤖 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_webui/frontend/src/pages/chat/components/auth-oauth-card.tsx`:
- Around line 45-54: Update providerDisplayName to verify that providerId is an
own key of PROVIDER_DISPLAY_NAMES before returning its value, preventing
inherited properties such as toString or constructor from entering the label
path; preserve the existing nearai and GitHub fallback behavior.

In `@crates/ironclaw_webui/src/webui_v2/handlers.rs`:
- Around line 2742-2759: Update the deterministic seed construction in the
UpsertLlmProviderRequest activity-ID path to encode presence separately from
string contents for name, base_url, default_model, model, and api_key. Preserve
distinct seed bytes for None and Some(""), following the presence-flag approach
used by outbound_preferences_activity_id while keeping existing values
deterministic.
- Around line 2753-2764: Update the ActivityId generation logic around
request.api_key so raw API-key bytes are never included in the seed passed to
Uuid::new_v5; use ActivityId::new() for a fresh identifier, unless an existing
server-side keyed HMAC mechanism is required for idempotency. Remove the
api_key-derived seed handling while preserving the function’s successful
ActivityId return behavior.

---

Outside diff comments:
In `@crates/ironclaw_reborn_composition/src/webui/product_capability.rs`:
- Around line 244-266: The product_invocation_mounts function routes
extension-lifecycle capabilities through the local-dev mount path regardless of
ProductCapabilityMounts. Update this branch to select the production lifecycle
mount implementation for ProductCapabilityMounts::Production while preserving
the existing local-dev behavior, and add coverage for production
extension-lifecycle invocations.

In `@crates/ironclaw_webui/src/webui_v2/handlers.rs`:
- Around line 2725-2797: Extract the duplicated caller/capability seed-prefix
construction from llm_provider_upsert_activity_id and
outbound_preferences_activity_id into a shared seed_caller_prefix helper. Have
both functions reuse it, passing their distinct context label and capability_id,
then append only their request-specific fields while preserving the existing
segment ordering and length-prefix encoding.
🪄 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: 888310a7-03c7-4c09-b218-b668f8115f56

📥 Commits

Reviewing files that changed from the base of the PR and between 342bb85 and d907d9a.

📒 Files selected for processing (16)
  • crates/ironclaw_reborn_composition/src/extension_host/extension_lifecycle.rs
  • crates/ironclaw_reborn_composition/src/extension_host/extension_lifecycle_capabilities.rs
  • crates/ironclaw_reborn_composition/src/outbound/mod.rs
  • crates/ironclaw_reborn_composition/src/runtime.rs
  • crates/ironclaw_reborn_composition/src/runtime/tests/outbound_delivery.rs
  • crates/ironclaw_reborn_composition/src/webui/facade/tests.rs
  • crates/ironclaw_reborn_composition/src/webui/product_capability.rs
  • crates/ironclaw_webui/frontend/src/pages/chat/components/auth-oauth-card.tsx
  • crates/ironclaw_webui/frontend/src/pages/chat/lib/attachments.ts
  • crates/ironclaw_webui/src/webui_v2/handlers.rs
  • tests/e2e/scenarios/test_reborn_webui_v2_automation_trace_outbound_api.py
  • tests/e2e/scenarios/test_reborn_webui_v2_legacy_approval.py
  • tests/e2e/scenarios/test_reborn_webui_v2_legacy_attachments.py
  • tests/e2e/scenarios/test_reborn_webui_v2_legacy_auth_flows.py
  • tests/e2e/scenarios/test_reborn_webui_v2_legacy_extensions.py
  • tests/e2e/scenarios/test_reborn_webui_v2_legacy_settings_search.py
💤 Files with no reviewable changes (1)
  • tests/e2e/scenarios/test_reborn_webui_v2_automation_trace_outbound_api.py

Comment thread crates/ironclaw_webui/frontend/src/pages/chat/components/auth-oauth-card.tsx Outdated
Comment thread crates/ironclaw_webui/src/webui_v2/handlers.rs
Comment thread crates/ironclaw_webui/src/webui_v2/handlers.rs Outdated
@ilblackdragon
ilblackdragon force-pushed the fix-main-reborn-playwright branch from d907d9a to 755534e Compare July 23, 2026 07:07
@railway-app
railway-app Bot temporarily deployed to ironclaw-ci-preview / ironclaw-pr-6546 July 23, 2026 07:07 Destroyed
@github-actions github-actions Bot added size: XL 500+ changed lines and removed size: L 200-499 changed lines labels Jul 23, 2026

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
crates/ironclaw_webui/tests/webui_v2_handlers_contract.rs (1)

5226-5289: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick win

Extend the retry-idempotency assertion to activate/remove/setup.

install_extension's test (just above, lines 5017-5070) now proves a retried request with the same client_action_id yields the same ProductSurface activity id — the exact regression the prior review flagged. activate_and_remove_extension_decode_path_package_id_to_lifecycle_paths and setup_extension_invokes_product_surface_capability exercise the same shared extension_lifecycle_activity_id helper but don't assert this for their capabilities. Consider mirroring the install test's second-request + invoke_calls[i].2 == invoke_calls[j].2 pattern for activate, remove, and setup so a regression in the shared helper is caught through all four production callers, not just one.

Also applies to: 5327-5380

🤖 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 `@crates/ironclaw_webui/tests/webui_v2_handlers_contract.rs` around lines 5226
- 5289, The lifecycle tests only verify capability payloads, not retry
idempotency of the shared extension_lifecycle_activity_id behavior. Extend
activate_and_remove_extension_decode_path_package_id_to_lifecycle_paths and
setup_extension_invokes_product_surface_capability with repeated requests using
the same client_action_id, then assert each retry produces the same activity ID
in the corresponding invoke_calls entries, mirroring the install_extension test
pattern for activate, remove, and setup.
🤖 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/e2e/scenarios/test_reborn_webui_v2_legacy_extensions.py`:
- Around line 20-41: Update _assert_install_requests and
_assert_setup_submit_requests to collect each request’s client_action_id while
validating it, then assert the batch contains only unique IDs using len(ids) ==
len(set(ids)). Preserve the existing request-count and body/package assertions.

---

Outside diff comments:
In `@crates/ironclaw_webui/tests/webui_v2_handlers_contract.rs`:
- Around line 5226-5289: The lifecycle tests only verify capability payloads,
not retry idempotency of the shared extension_lifecycle_activity_id behavior.
Extend activate_and_remove_extension_decode_path_package_id_to_lifecycle_paths
and setup_extension_invokes_product_surface_capability with repeated requests
using the same client_action_id, then assert each retry produces the same
activity ID in the corresponding invoke_calls entries, mirroring the
install_extension test pattern for activate, remove, and setup.
🪄 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: a4091f92-06f2-4517-9138-cad3df38d041

📥 Commits

Reviewing files that changed from the base of the PR and between d907d9a and 755534e.

📒 Files selected for processing (29)
  • crates/ironclaw_product_workflow/src/lib.rs
  • crates/ironclaw_product_workflow/src/reborn_services/extension_setup_credentials.rs
  • crates/ironclaw_product_workflow/src/webui_inbound.rs
  • crates/ironclaw_product_workflow/tests/reborn_services_contract.rs
  • crates/ironclaw_reborn_composition/src/extension_host/extension_lifecycle.rs
  • crates/ironclaw_reborn_composition/src/extension_host/extension_lifecycle_capabilities.rs
  • crates/ironclaw_reborn_composition/src/outbound/mod.rs
  • crates/ironclaw_reborn_composition/src/runtime.rs
  • crates/ironclaw_reborn_composition/src/runtime/tests/core.rs
  • crates/ironclaw_reborn_composition/src/runtime/tests/outbound_delivery.rs
  • crates/ironclaw_reborn_composition/src/webui/facade/tests.rs
  • crates/ironclaw_reborn_composition/src/webui/product_capability.rs
  • crates/ironclaw_webui/frontend/src/lib/api.test.ts
  • crates/ironclaw_webui/frontend/src/lib/api.ts
  • crates/ironclaw_webui/frontend/src/pages/chat/components/auth-oauth-card.tsx
  • crates/ironclaw_webui/frontend/src/pages/chat/lib/attachments.ts
  • crates/ironclaw_webui/frontend/src/pages/extensions/lib/extensions-api.test.ts
  • crates/ironclaw_webui/frontend/src/pages/extensions/lib/extensions-api.ts
  • crates/ironclaw_webui/src/webui_v2/handlers.rs
  • crates/ironclaw_webui/tests/webui_v2_handlers_contract.rs
  • tests/e2e/scenarios/test_reborn_qa_trace_full_path.py
  • tests/e2e/scenarios/test_reborn_slack_channel_e2e.py
  • tests/e2e/scenarios/test_reborn_webui_v2_automation_trace_outbound_api.py
  • tests/e2e/scenarios/test_reborn_webui_v2_extensions_api.py
  • tests/e2e/scenarios/test_reborn_webui_v2_legacy_approval.py
  • tests/e2e/scenarios/test_reborn_webui_v2_legacy_attachments.py
  • tests/e2e/scenarios/test_reborn_webui_v2_legacy_auth_flows.py
  • tests/e2e/scenarios/test_reborn_webui_v2_legacy_extensions.py
  • tests/e2e/scenarios/test_reborn_webui_v2_legacy_settings_search.py
💤 Files with no reviewable changes (1)
  • tests/e2e/scenarios/test_reborn_webui_v2_automation_trace_outbound_api.py

Comment on lines +20 to +41
def _assert_client_action_id(body: dict) -> None:
assert isinstance(body.get("client_action_id"), str)
assert body["client_action_id"]


def _assert_install_requests(requests: list[dict], *package_ids: str) -> None:
assert len(requests) == len(package_ids)
for request, package_id in zip(requests, package_ids, strict=True):
assert request.get("package_ref") == _package_ref(package_id)
_assert_client_action_id(request)


def _assert_setup_submit_requests(
requests: list[dict], expected: list[dict]
) -> None:
assert len(requests) == len(expected)
for request, expected_request in zip(requests, expected, strict=True):
assert request["package_id"] == expected_request["package_id"]
body = dict(request["body"])
_assert_client_action_id(body)
body.pop("client_action_id")
assert body == expected_request["body"]

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Assert client-action ID uniqueness.

Lines [20-41] verify only that each ID is non-empty. A regression that reuses one ID across repeated non-idempotent install/setup gestures would still pass these helpers, despite the PR objective requiring fresh activity IDs. Collect the IDs per request batch and assert len(ids) == len(set(ids)).

Proposed test fix
 def _assert_install_requests(requests: list[dict], *package_ids: str) -> None:
     assert len(requests) == len(package_ids)
+    client_action_ids = []
     for request, package_id in zip(requests, package_ids, strict=True):
         assert request.get("package_ref") == _package_ref(package_id)
         _assert_client_action_id(request)
+        client_action_ids.append(request["client_action_id"])
+    assert len(client_action_ids) == len(set(client_action_ids))
🤖 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/e2e/scenarios/test_reborn_webui_v2_legacy_extensions.py` around lines
20 - 41, Update _assert_install_requests and _assert_setup_submit_requests to
collect each request’s client_action_id while validating it, then assert the
batch contains only unique IDs using len(ids) == len(set(ids)). Preserve the
existing request-count and body/package assertions.

@ilblackdragon

Copy link
Copy Markdown
Member Author

Temporarily closing/reopening to refresh a stale pull ref; branch refs/heads/fix-main-reborn-playwright is at 0af5783 but refs/pull/6546/head remained at 755534e.

@ilblackdragon ilblackdragon mentioned this pull request Jul 23, 2026
22 of 29 tasks
@ilblackdragon

Copy link
Copy Markdown
Member Author

Replacement PR opened at #6553 from a fresh branch because GitHub refused to reopen this PR after its pull ref stopped tracking the branch.

@ilblackdragon ilblackdragon mentioned this pull request Jul 23, 2026
22 of 29 tasks
@ilblackdragon

Copy link
Copy Markdown
Member Author

Final replacement PR is #6554. The first replacement (#6553) also hit stale pull-ref behavior; #6554 uses a fresh branch whose pull ref is tracking correctly.

This branch was successfully deployed

No deployments
ironclaw-ci-preview / ironclaw-pr-6546 — 755534ea Deployed Jul 23, 2026 by railway-app[bot]
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

contributor: core 20+ merged PRs risk: low Changes to docs, tests, or low-risk modules size: XL 500+ changed lines

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant