fix(reborn): persist Slack host conversation bindings#5252
Conversation
railway-app
Bot
temporarily deployed
to
ironclaw-ci-preview / ironclaw-pr-5252
📝 WalkthroughSummary by CodeRabbit
WalkthroughDurable conversation initialization is split out, Slack host beta now threads conversation ports through mount and resolver construction, chat send disabling changes for active runs, and the affected mount builders and tests become async. ChangesSlack conversation port threading
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 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 updates the Slack host-beta implementation to support durable conversation services when the libsql or postgres features are enabled, rather than always relying on in-memory bindings. This is accomplished by introducing a SlackConversationPorts struct, making route mount building functions asynchronous, and passing the resolved conversation ports to the installation resolver. Additionally, tests have been updated and a new test has been added to verify that conversation bindings persist across runtime restarts. There are no review comments to address.
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: 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/slack_host_beta.rs`:
- Around line 1516-1582: This restart-regression test currently assumes
conversation bindings survive a runtime reopen even when the build uses
InMemoryConversationServices, which is only true for durable backends. Update
build_slack_events_route_mount_reuses_conversation_binding_after_runtime_reopen
to run only when libsql or postgres is enabled, or otherwise skip/assert the
in-memory fallback behavior, using the
runtime_with_root_and_host_egress_override and build_slack_events_route_mount
setup as the gate.
🪄 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: 80eed385-d333-46f6-a3da-1cf882d9fa3c
📒 Files selected for processing (3)
crates/ironclaw_reborn_composition/src/factory.rscrates/ironclaw_reborn_composition/src/slack_host_beta.rscrates/ironclaw_reborn_composition/src/slack_host_beta/runtime_setup.rs
railway-app
Bot
temporarily deployed
to
ironclaw-ci-preview / ironclaw-pr-5252
railway-app
Bot
temporarily deployed
to
ironclaw-ci-preview / ironclaw-pr-5252
github-actions
Bot
added
size: XL
|
🚅 Deployed to the ironclaw-pr-5252 environment in ironclaw-ci-preview
|
railway-app
Bot
temporarily deployed
to
ironclaw-ci-preview / ironclaw-pr-5252
There was a problem hiding this comment.
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_v2_static/static/js/pages/chat/chat.js (1)
92-105: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winKeep the thread row in
RUNNINGwhile this new active-run gate is blocking sends.This adds a current-thread busy state (
activeRun.statusrunning/queued withisProcessing === false), but the effect at Lines 201-216 still clears sidebar state wheneverisProcessingis false. The newchat.test.mjscase covers exactly that rehydrated state, so the composer stays blocked while the thread row flips back to idle after 1.5s.Suggested fix
const activeRunBlocksSubmit = Boolean( activeThreadId && activeRun?.runId && activeRun.threadId === activeThreadId && !pendingGate && (!activeRun.status || activeRun.status === "queued" || activeRun.status === "running") ); @@ - if (isProcessing) { + if (isProcessing || activeRunBlocksSubmit) { setThreadState(activeThreadId, THREAD_STATE.RUNNING); return undefined; } @@ - }, [activeThreadId, pendingGate, isProcessing]); + }, [activeThreadId, pendingGate, isProcessing, activeRunBlocksSubmit]);🤖 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_v2_static/static/js/pages/chat/chat.js` around lines 92 - 105, The current-thread busy state is being cleared too early in the sidebar sync logic, causing the thread row to flip back to idle while `activeRunBlocksSubmit` is still blocking sends. Update the state reset/effect in `chat.js` around the `isProcessing` handling so it preserves `RUNNING`/busy status for the active thread whenever `activeRun.status` is `queued` or `running` and `activeRun.threadId` matches `activeThreadId`, instead of clearing sidebar state solely because `isProcessing` is false. Keep the composer gating in sync with `activeRunBlocksSubmit` and the existing `composerSendDisabled` logic so the rehydrated active-run case remains blocked.
🤖 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.
Outside diff comments:
In `@crates/ironclaw_webui_v2_static/static/js/pages/chat/chat.js`:
- Around line 92-105: The current-thread busy state is being cleared too early
in the sidebar sync logic, causing the thread row to flip back to idle while
`activeRunBlocksSubmit` is still blocking sends. Update the state reset/effect
in `chat.js` around the `isProcessing` handling so it preserves `RUNNING`/busy
status for the active thread whenever `activeRun.status` is `queued` or
`running` and `activeRun.threadId` matches `activeThreadId`, instead of clearing
sidebar state solely because `isProcessing` is false. Keep the composer gating
in sync with `activeRunBlocksSubmit` and the existing `composerSendDisabled`
logic so the rehydrated active-run case remains blocked.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 8a24ddd6-3415-41b0-8d5e-7bb95c4d8f55
⛔ Files ignored due to path filters (1)
crates/ironclaw_webui_v2_static/static/dist/app.jsis excluded by!**/dist/**
📒 Files selected for processing (2)
crates/ironclaw_webui_v2_static/static/js/pages/chat/chat.jscrates/ironclaw_webui_v2_static/static/js/pages/chat/lib/chat.test.mjs
1 participant
Summary
Change Type
Linked Issue
None. This fixes the Slack host-beta restart binding loss reported from runtime logs.
Validation
cargo fmt --all -- --checkcargo clippy --all --benches --tests --examples --all-features -- -D warningscargo buildCARGO_INCREMENTAL=0 CARGO_PROFILE_DEV_DEBUG=0 CARGO_PROFILE_TEST_DEBUG=0 cargo test -p ironclaw_reborn_composition --features test-support,webui-v2-beta,slack-v2-host-beta,libsql slack_host_beta::tests:: -- --nocaptureCARGO_INCREMENTAL=0 CARGO_PROFILE_DEV_DEBUG=0 CARGO_PROFILE_TEST_DEBUG=0 cargo clippy -p ironclaw_reborn_composition --features test-support,webui-v2-beta,slack-v2-host-beta,libsql --tests -- -D warningscargo check -p ironclaw_reborn_composition --features test-support,webui-v2-beta,slack-v2-host-beta,postgrescargo check -p ironclaw_reborn_composition --features test-support,webui-v2-beta,slack-v2-host-betanode --test crates/ironclaw_webui_v2_static/static/js/pages/chat/lib/chat.test.mjscargo test -p ironclaw_webui_v2_staticgit diff --check/Users/firatsertgoz/.local/bin/python3.11 scripts/check_no_panics.py --base origin/main --head HEADcargo test --features integrationif database-backed or integration behavior changedplaywright; CI runs the WebUI smoke test.review-prorpr-shepherd --fixwas run before requesting review: CodeRabbit feedback was fetched and addressed manually; CI is green on the pushed head.Security Impact
This touches host-beta Slack event routing and conversation binding persistence. It does not weaken webhook verification, bearer auth, CORS/origin checks, outbound HTTP policy, or secret handling. The change replaces process-local conversation binding storage with existing host filesystem-backed conversation services for DB-backed profiles.
Reborn Trust-Boundary Checklist
rg "ConversationServicesUnavailable|SlackHostBetaBuildError" crates/ironclaw_reborn_composition/src.serde(default)fields fail closed or have migration tests. No new serde fields are added.Transient,Permanent,Misconfigured,PolicyDeniedor equivalent). Slack route construction now fails loudly with a conversation-services-unavailable build error if durable services cannot initialize.SlackConversationPortstype describes runtime-provided conversation service ports.Database Impact
No database migration is needed. This uses the existing conversation filesystem abstraction and its configured backend; it does not add a Slack-specific table or schema. The durable restart regression is gated to
libsqlorpostgresbuilds because no-DB builds intentionally use in-memory services.Blast Radius
Touches Slack host-beta runtime composition, Slack route mount construction, dynamic Slack installation resolver wiring, and WebUI v2 chat submit gating. Primary risk is Slack route construction failing if durable conversation services are unavailable in a DB-backed hosted profile; that failure is intentional and operator-visible instead of silently losing restart state.
Rollback Plan
Revert this PR to restore the previous Slack host-beta in-memory conversation binding behavior. Rollback risk is recurrence of the original restart bug: Slack conversation-to-thread bindings are lost after process restart and follow-up delivery can fail or create the wrong conversation linkage.
Review Follow-Through
CodeRabbit left one actionable inline comment asking that the restart regression be durable-backend gated; this has been addressed with a
#[cfg(any(feature = "libsql", feature = "postgres"))]gate on that test. Reviewer judgment is still needed on whether the Slack host-beta mount API async conversion is acceptable for callers.Review track: C (security/runtime/DB/CI)