fix(webui): link approval card to global auto-approve settings - #5247
Merged
Merged
Conversation
italic-jinxin
added
size: L
railway-app
Bot
temporarily deployed
to
ironclaw-ci-preview / ironclaw-pr-5247
June 25, 2026 14:38
Destroyed
github-actions
Bot
added
size: XL
|
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 (2)
📝 WalkthroughSummary by CodeRabbit
WalkthroughBackend session bootstrap now exposes ChangesGlobal auto-approve shortcut
Chat Input DOM Guard
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 |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
![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_webui_v2_static/static/js/pages/chat/chat.js`:
- Around line 85-93: The global auto-approve flag is being derived from the
settings API response instead of the bootstrap globals, which breaks the
frontend flag-source contract. Update the chat page logic in the
`settingsQuery`/`globalAutoApproveEnabled` path to read the flag from the
existing bootstrap globals source used by the web UI, and remove the dependency
on `settingsQuery.data.settings` for this value.
🪄 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: 4c1ede1e-fc8e-4d24-972f-bc90fc073f43
⛔ Files ignored due to path filters (1)
crates/ironclaw_webui_v2_static/static/dist/app.jsis excluded by!**/dist/**
📒 Files selected for processing (5)
crates/ironclaw_webui_v2_static/static/js/i18n/en.jscrates/ironclaw_webui_v2_static/static/js/pages/chat/chat.jscrates/ironclaw_webui_v2_static/static/js/pages/chat/components/approval-card.jscrates/ironclaw_webui_v2_static/static/js/pages/chat/components/approval-card.test.mjscrates/ironclaw_webui_v2_static/static/js/pages/chat/lib/chat.test.mjs
# Conflicts: # crates/ironclaw_webui_v2_static/static/dist/app.js
railway-app
Bot
temporarily deployed
to
ironclaw-ci-preview / ironclaw-pr-5247
June 25, 2026 15:08
Destroyed
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_webui_v2/src/handlers.rs`:
- Around line 121-133: The `global_auto_approve_enabled` helper is swallowing an
operator-config read error by using `let Ok(config) = ... else { return false
}`, which hides a boundary/IO failure. Keep the fail-closed `false` fallback,
but make it explicit by adding an inline `// silent-ok: ...` justification near
the `get_operator_config_key` call, or otherwise log/propagate the `Err` before
returning. Use `global_auto_approve_enabled`, `get_operator_config_key`, and
`AUTO_APPROVE_CONFIG_KEY` to locate the change.
🪄 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: 27468db3-7a5e-4053-a0b9-5f400658da42
⛔ Files ignored due to path filters (1)
crates/ironclaw_webui_v2_static/static/dist/app.jsis excluded by!**/dist/**
📒 Files selected for processing (9)
crates/ironclaw_webui_v2/src/handlers.rscrates/ironclaw_webui_v2/tests/webui_v2_handlers_contract.rscrates/ironclaw_webui_v2_static/src/assets.rscrates/ironclaw_webui_v2_static/static/js/app/app.jscrates/ironclaw_webui_v2_static/static/js/app/auth.jscrates/ironclaw_webui_v2_static/static/js/layout/gateway-layout.jscrates/ironclaw_webui_v2_static/static/js/pages/chat/chat-page.jscrates/ironclaw_webui_v2_static/static/js/pages/chat/chat.jscrates/ironclaw_webui_v2_static/static/js/pages/chat/lib/chat.test.mjs
|
🚅 Deployed to the ironclaw-pr-5247 environment in ironclaw-ci-preview
|
railway-app
Bot
temporarily deployed
to
ironclaw-ci-preview / ironclaw-pr-5247
June 25, 2026 15:37
Destroyed
Collaborator
|
@claude review |
This comment was marked as resolved.
This comment was marked as resolved.
railway-app
Bot
temporarily deployed
to
ironclaw-ci-preview / ironclaw-pr-5247
June 25, 2026 16:01
Destroyed
# Conflicts: # crates/ironclaw_webui_v2_static/static/dist/app.js
railway-app
Bot
temporarily deployed
to
ironclaw-ci-preview / ironclaw-pr-5247
June 25, 2026 16:12
Destroyed
railway-app
Bot
temporarily deployed
to
ironclaw-ci-preview / ironclaw-pr-5247
June 25, 2026 16:25
Destroyed
Collaborator
|
[BLOCK] Not ready for human final review. Blocking status:
No new inline findings were added in this pass. Guidance for human follow-up:
|
Collaborator
|
@claude review |
Collaborator
|
Mergeability check: is (). Please rebase or merge the current base branch to resolve conflicts before continuing review. |
Collaborator
|
Mergeability check: mergeStateStatus is DIRTY and mergeable is CONFLICTING. Please rebase or merge the current base branch to resolve conflicts before review can proceed. |
# Conflicts: # crates/ironclaw_webui_v2_static/static/js/pages/chat/components/chat-input.js
railway-app
Bot
temporarily deployed
to
ironclaw-ci-preview / ironclaw-pr-5247
June 29, 2026 15:08
Destroyed
# Conflicts: # crates/ironclaw_webui_v2_static/static/js/pages/chat/components/approval-card.test.mjs # crates/ironclaw_webui_v2_static/static/js/pages/chat/components/chat-input.js
railway-app
Bot
temporarily deployed
to
ironclaw-ci-preview / ironclaw-pr-5247
June 30, 2026 06:27
Destroyed
There was a problem hiding this comment.
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 (3)
crates/ironclaw_webui_v2_static/static/js/pages/chat/components/chat-input.js (2)
171-196: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winGuard the staging failure path with the same draft/session snapshot.
Lines 194-195 skip the
activeDraftContextRefcheck that the success path already applies. IfstageFiles()rejects after a thread or auth switch, the old failure is rendered in the newly active composer. That breaks the file’s own “composer can stay mounted across a token/session switch” invariant.Suggested fix
.catch(() => { + const current = activeDraftContextRef.current; + if ( + current.draftKey !== expectedDraftKey || + current.storageScope !== expectedStorageScope || + authScope() !== expectedStorageScope + ) { + return; + } setAttachmentError(t("chat.attachmentStagingFailed")); });🤖 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/components/chat-input.js` around lines 171 - 196, The staging failure path in chat-input.js should use the same draft/session guard as the success path. In the staging promise chain around stageFiles and activeDraftContextRef, capture the current draftKey/storageScope snapshot before awaiting, and in the catch handler only call setAttachmentError if the activeDraftContextRef still matches that snapshot and authScope() is unchanged. Keep the existing success-path check in place so stale staging errors cannot overwrite the newly active composer state.
226-272: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winSnapshot the active composer before awaiting
onSend().
handleSend()now defends the pre-send path, but Lines 245-262 still apply success cleanup to whatever composer is mounted when the promise resolves. If the user switches thread or auth scope while the send is in flight, the stale continuation will clear the next composer’s text/attachments viasetText("")andsetAttachments([]). That is draft loss, not just a visual glitch.Suggested fix
const handleSend = React.useCallback(async () => { + const expectedDraftKey = draftKey; + const expectedStorageScope = storageScope; const trimmed = text.trim(); const hasAttachments = attachments.length > 0; const sendContent = trimmed || (hasAttachments ? ATTACHMENTS_ONLY_CONTENT : ""); @@ try { const response = await onSend(sendContent, { attachments, displayContent: trimmed, }); if (response === null) return; + const current = activeDraftContextRef.current; + if ( + current.draftKey !== expectedDraftKey || + current.storageScope !== expectedStorageScope || + authScope() !== expectedStorageScope + ) { + return; + } setText(""); setAttachments([]); attachmentsRef.current = []; setAttachmentError(""); cancelPendingDraft(); - clearDraft(draftKey); - clearStagedAttachments(draftKey); + clearDraft(expectedDraftKey); + clearStagedAttachments(expectedDraftKey); if (textareaRef.current) textareaRef.current.style.height = "auto"; @@ }, [ text, attachments, disabled, sendDisabled, isSending, onSend, draftKey, + storageScope, cancelPendingDraft, ]);🤖 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/components/chat-input.js` around lines 226 - 272, handleSend currently applies post-send cleanup to whichever composer is mounted when onSend resolves, which can wipe a different thread/scope’s draft after a switch. Snapshot the active composer identity and draft state before awaiting onSend in handleSend, then only run the success cleanup path (setText, setAttachments, attachmentsRef, cancelPendingDraft, clearDraft, clearStagedAttachments, height reset) if the same composer is still active. Use the existing draftKey, textareaRef, and sendBlockedRef flow to guard the continuation.crates/ironclaw_webui_v2_static/static/js/pages/chat/chat.js (1)
127-153: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick win
composerSendDisabledinhandleSenddeps is redundant and defeats the ref pattern.The callback reads
composerSendBlockedRef.currentprecisely to avoid listingcomposerSendDisabledas a dep (keeping the callback stable across processing-state changes). Having it in deps anyway causes the callback to re-create on every processing tick, which re-triggers any downstream memoization depending onhandleSend(e.g.SuggestionChips,ChatInput).♻️ Remove the redundant dep
[ activeThreadId, activeThreadHasGate, approvalSubmitWarning, - composerSendDisabled, onSelectThread, send, ]🤖 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 127 - 153, The handleSend callback in chat.js is still depending on composerSendDisabled even though it already uses composerSendBlockedRef.current to avoid that state in the dependency list. Remove composerSendDisabled from the useCallback dependency array for handleSend so the callback stays stable across processing-state updates and does not force unnecessary re-renders in downstream consumers like SuggestionChips and ChatInput.
🤖 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_v2_static/static/js/pages/chat/components/approval-card.js`:
- Around line 55-59: The resolving reset in approval-card should be keyed to a
stable request identifier instead of the gate object identity, because the
current React.useEffect in approval-card.js clears isResolvingRef and
isResolving whenever gate is re-created. Update the effect to depend on the
gate’s stable request id (or equivalent unique key used by the approve/deny
flow) so the in-flight guard in the approve/deny handlers stays active across
rerenders for the same request and only resets when the actual request changes.
---
Outside diff comments:
In `@crates/ironclaw_webui_v2_static/static/js/pages/chat/chat.js`:
- Around line 127-153: The handleSend callback in chat.js is still depending on
composerSendDisabled even though it already uses composerSendBlockedRef.current
to avoid that state in the dependency list. Remove composerSendDisabled from the
useCallback dependency array for handleSend so the callback stays stable across
processing-state updates and does not force unnecessary re-renders in downstream
consumers like SuggestionChips and ChatInput.
In
`@crates/ironclaw_webui_v2_static/static/js/pages/chat/components/chat-input.js`:
- Around line 171-196: The staging failure path in chat-input.js should use the
same draft/session guard as the success path. In the staging promise chain
around stageFiles and activeDraftContextRef, capture the current
draftKey/storageScope snapshot before awaiting, and in the catch handler only
call setAttachmentError if the activeDraftContextRef still matches that snapshot
and authScope() is unchanged. Keep the existing success-path check in place so
stale staging errors cannot overwrite the newly active composer state.
- Around line 226-272: handleSend currently applies post-send cleanup to
whichever composer is mounted when onSend resolves, which can wipe a different
thread/scope’s draft after a switch. Snapshot the active composer identity and
draft state before awaiting onSend in handleSend, then only run the success
cleanup path (setText, setAttachments, attachmentsRef, cancelPendingDraft,
clearDraft, clearStagedAttachments, height reset) if the same composer is still
active. Use the existing draftKey, textareaRef, and sendBlockedRef flow to guard
the continuation.
🪄 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: 9e7e69da-822c-44ec-8c94-b9caaa6d7e3a
📒 Files selected for processing (16)
crates/ironclaw_webui_v2_static/static/js/i18n/ar.jscrates/ironclaw_webui_v2_static/static/js/i18n/de.jscrates/ironclaw_webui_v2_static/static/js/i18n/en.jscrates/ironclaw_webui_v2_static/static/js/i18n/es.jscrates/ironclaw_webui_v2_static/static/js/i18n/fr.jscrates/ironclaw_webui_v2_static/static/js/i18n/hi.jscrates/ironclaw_webui_v2_static/static/js/i18n/ja.jscrates/ironclaw_webui_v2_static/static/js/i18n/ko.jscrates/ironclaw_webui_v2_static/static/js/i18n/pt-BR.jscrates/ironclaw_webui_v2_static/static/js/i18n/uk.jscrates/ironclaw_webui_v2_static/static/js/i18n/zh-CN.jscrates/ironclaw_webui_v2_static/static/js/pages/chat/chat.jscrates/ironclaw_webui_v2_static/static/js/pages/chat/components/approval-card.jscrates/ironclaw_webui_v2_static/static/js/pages/chat/components/approval-card.test.mjscrates/ironclaw_webui_v2_static/static/js/pages/chat/components/chat-input.jscrates/ironclaw_webui_v2_static/static/js/pages/chat/lib/chat.test.mjs
There was a problem hiding this comment.
Caution
Inline review comments failed to post. This is likely due to GitHub's internal server error or limits when posting large numbers of comments. If you are seeing this consistently it is likely a permissions issue. Please check "Moderation" -> "Code review limits" under your organization settings.
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 (3)
crates/ironclaw_webui_v2_static/static/js/pages/chat/components/chat-input.js (2)
171-196: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winGuard the staging failure path with the same draft/session snapshot.
Lines 194-195 skip the
activeDraftContextRefcheck that the success path already applies. IfstageFiles()rejects after a thread or auth switch, the old failure is rendered in the newly active composer. That breaks the file’s own “composer can stay mounted across a token/session switch” invariant.Suggested fix
.catch(() => { + const current = activeDraftContextRef.current; + if ( + current.draftKey !== expectedDraftKey || + current.storageScope !== expectedStorageScope || + authScope() !== expectedStorageScope + ) { + return; + } setAttachmentError(t("chat.attachmentStagingFailed")); });🤖 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/components/chat-input.js` around lines 171 - 196, The staging failure path in chat-input.js should use the same draft/session guard as the success path. In the staging promise chain around stageFiles and activeDraftContextRef, capture the current draftKey/storageScope snapshot before awaiting, and in the catch handler only call setAttachmentError if the activeDraftContextRef still matches that snapshot and authScope() is unchanged. Keep the existing success-path check in place so stale staging errors cannot overwrite the newly active composer state.
226-272: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winSnapshot the active composer before awaiting
onSend().
handleSend()now defends the pre-send path, but Lines 245-262 still apply success cleanup to whatever composer is mounted when the promise resolves. If the user switches thread or auth scope while the send is in flight, the stale continuation will clear the next composer’s text/attachments viasetText("")andsetAttachments([]). That is draft loss, not just a visual glitch.Suggested fix
const handleSend = React.useCallback(async () => { + const expectedDraftKey = draftKey; + const expectedStorageScope = storageScope; const trimmed = text.trim(); const hasAttachments = attachments.length > 0; const sendContent = trimmed || (hasAttachments ? ATTACHMENTS_ONLY_CONTENT : ""); @@ try { const response = await onSend(sendContent, { attachments, displayContent: trimmed, }); if (response === null) return; + const current = activeDraftContextRef.current; + if ( + current.draftKey !== expectedDraftKey || + current.storageScope !== expectedStorageScope || + authScope() !== expectedStorageScope + ) { + return; + } setText(""); setAttachments([]); attachmentsRef.current = []; setAttachmentError(""); cancelPendingDraft(); - clearDraft(draftKey); - clearStagedAttachments(draftKey); + clearDraft(expectedDraftKey); + clearStagedAttachments(expectedDraftKey); if (textareaRef.current) textareaRef.current.style.height = "auto"; @@ }, [ text, attachments, disabled, sendDisabled, isSending, onSend, draftKey, + storageScope, cancelPendingDraft, ]);🤖 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/components/chat-input.js` around lines 226 - 272, handleSend currently applies post-send cleanup to whichever composer is mounted when onSend resolves, which can wipe a different thread/scope’s draft after a switch. Snapshot the active composer identity and draft state before awaiting onSend in handleSend, then only run the success cleanup path (setText, setAttachments, attachmentsRef, cancelPendingDraft, clearDraft, clearStagedAttachments, height reset) if the same composer is still active. Use the existing draftKey, textareaRef, and sendBlockedRef flow to guard the continuation.crates/ironclaw_webui_v2_static/static/js/pages/chat/chat.js (1)
127-153: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick win
composerSendDisabledinhandleSenddeps is redundant and defeats the ref pattern.The callback reads
composerSendBlockedRef.currentprecisely to avoid listingcomposerSendDisabledas a dep (keeping the callback stable across processing-state changes). Having it in deps anyway causes the callback to re-create on every processing tick, which re-triggers any downstream memoization depending onhandleSend(e.g.SuggestionChips,ChatInput).♻️ Remove the redundant dep
[ activeThreadId, activeThreadHasGate, approvalSubmitWarning, - composerSendDisabled, onSelectThread, send, ]🤖 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 127 - 153, The handleSend callback in chat.js is still depending on composerSendDisabled even though it already uses composerSendBlockedRef.current to avoid that state in the dependency list. Remove composerSendDisabled from the useCallback dependency array for handleSend so the callback stays stable across processing-state updates and does not force unnecessary re-renders in downstream consumers like SuggestionChips and ChatInput.
🤖 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_v2_static/static/js/pages/chat/components/approval-card.js`:
- Around line 55-59: The resolving reset in approval-card should be keyed to a
stable request identifier instead of the gate object identity, because the
current React.useEffect in approval-card.js clears isResolvingRef and
isResolving whenever gate is re-created. Update the effect to depend on the
gate’s stable request id (or equivalent unique key used by the approve/deny
flow) so the in-flight guard in the approve/deny handlers stays active across
rerenders for the same request and only resets when the actual request changes.
---
Outside diff comments:
In `@crates/ironclaw_webui_v2_static/static/js/pages/chat/chat.js`:
- Around line 127-153: The handleSend callback in chat.js is still depending on
composerSendDisabled even though it already uses composerSendBlockedRef.current
to avoid that state in the dependency list. Remove composerSendDisabled from the
useCallback dependency array for handleSend so the callback stays stable across
processing-state updates and does not force unnecessary re-renders in downstream
consumers like SuggestionChips and ChatInput.
In
`@crates/ironclaw_webui_v2_static/static/js/pages/chat/components/chat-input.js`:
- Around line 171-196: The staging failure path in chat-input.js should use the
same draft/session guard as the success path. In the staging promise chain
around stageFiles and activeDraftContextRef, capture the current
draftKey/storageScope snapshot before awaiting, and in the catch handler only
call setAttachmentError if the activeDraftContextRef still matches that snapshot
and authScope() is unchanged. Keep the existing success-path check in place so
stale staging errors cannot overwrite the newly active composer state.
- Around line 226-272: handleSend currently applies post-send cleanup to
whichever composer is mounted when onSend resolves, which can wipe a different
thread/scope’s draft after a switch. Snapshot the active composer identity and
draft state before awaiting onSend in handleSend, then only run the success
cleanup path (setText, setAttachments, attachmentsRef, cancelPendingDraft,
clearDraft, clearStagedAttachments, height reset) if the same composer is still
active. Use the existing draftKey, textareaRef, and sendBlockedRef flow to guard
the continuation.
🪄 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: 9e7e69da-822c-44ec-8c94-b9caaa6d7e3a
📒 Files selected for processing (16)
crates/ironclaw_webui_v2_static/static/js/i18n/ar.jscrates/ironclaw_webui_v2_static/static/js/i18n/de.jscrates/ironclaw_webui_v2_static/static/js/i18n/en.jscrates/ironclaw_webui_v2_static/static/js/i18n/es.jscrates/ironclaw_webui_v2_static/static/js/i18n/fr.jscrates/ironclaw_webui_v2_static/static/js/i18n/hi.jscrates/ironclaw_webui_v2_static/static/js/i18n/ja.jscrates/ironclaw_webui_v2_static/static/js/i18n/ko.jscrates/ironclaw_webui_v2_static/static/js/i18n/pt-BR.jscrates/ironclaw_webui_v2_static/static/js/i18n/uk.jscrates/ironclaw_webui_v2_static/static/js/i18n/zh-CN.jscrates/ironclaw_webui_v2_static/static/js/pages/chat/chat.jscrates/ironclaw_webui_v2_static/static/js/pages/chat/components/approval-card.jscrates/ironclaw_webui_v2_static/static/js/pages/chat/components/approval-card.test.mjscrates/ironclaw_webui_v2_static/static/js/pages/chat/components/chat-input.jscrates/ironclaw_webui_v2_static/static/js/pages/chat/lib/chat.test.mjs
🛑 Comments failed to post (1)
crates/ironclaw_webui_v2_static/static/js/pages/chat/components/approval-card.js (1)
55-59: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Key the resolving reset off a stable request id, not
gateobject identity.Line 55 clears the in-flight guard on any new
gatereference. If the parent re-derives the same pending gate during a rerender, Lines 71-84 stop protecting the approve/deny actions and the same request can be resolved twice.Suggested fix
+ const gateResetKey = gate?.requestId ?? null; + React.useEffect(() => { setExpandedPayload(false); isResolvingRef.current = false; setIsResolving(false); - }, [gate]); + }, [gateResetKey]);Also applies to: 71-84
🤖 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/components/approval-card.js` around lines 55 - 59, The resolving reset in approval-card should be keyed to a stable request identifier instead of the gate object identity, because the current React.useEffect in approval-card.js clears isResolvingRef and isResolving whenever gate is re-created. Update the effect to depend on the gate’s stable request id (or equivalent unique key used by the approve/deny flow) so the in-flight guard in the approve/deny handlers stays active across rerenders for the same request and only resets when the actual request changes.
Collaborator
|
@claude review |
This comment was marked as resolved.
This comment was marked as resolved.
railway-app
Bot
temporarily deployed
to
ironclaw-ci-preview / ironclaw-pr-5247
July 1, 2026 08:42
Destroyed
railway-app
Bot
temporarily deployed
to
ironclaw-ci-preview / ironclaw-pr-5247
July 1, 2026 08:52
Destroyed
# Conflicts: # crates/ironclaw_webui_v2_static/static/js/pages/chat/components/chat-input.js
railway-app
Bot
temporarily deployed
to
ironclaw-ci-preview / ironclaw-pr-5247
July 1, 2026 09:10
Destroyed
Contributor
Author
|
@claude review |
This comment was marked as resolved.
This comment was marked as resolved.
There was a problem hiding this comment.
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_v2_static/static/js/pages/chat/components/chat-input.js (1)
242-253: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winNew DOM-dataset guard duplicates the existing ref-based guard.
domSendDisabled(readingdataset.sendDisabled) is derived from exactly the samedisabled/sendDisabledinputs already folded intosendBlockedRef.current(sendBlocked = disabled || sendDisabled || isSending, set every render at Line 50-53) and into the explicitdisabled/sendDisabled/isSendingchecks right next to it. SincehandleSend'suseCallbackdeps now include all three (Line 305-315), there's no stale-closure scenario this DOM read protects against — refs already solve that per the standard React pattern. The added test (chat-input.test.mjs247-276) only demonstrates the branch by manually forging adatasetvalue that diverges fromsendDisabled/disabled, a state that can't arise from this component's own render output.If there's a concrete external mutator of
data-send-disabledthis is meant to guard against (e.g. non-React DOM manipulation elsewhere), please call it out in a comment; otherwise this is redundant surface area to maintain and reason about across two call sites.♻️ Possible simplification
- const domSendDisabled = - textareaRef.current?.dataset?.sendDisabled === "true"; if ( !sendContent || disabled || sendDisabled || isSending || - domSendDisabled || sendBlockedRef.current ) { return; }if (e.key === "Enter" && !e.shiftKey) { e.preventDefault(); - const domSendDisabled = - e.currentTarget?.dataset?.sendDisabled === "true" || - textareaRef.current?.dataset?.sendDisabled === "true"; - if (domSendDisabled || sendBlockedRef.current) return; + if (sendBlockedRef.current) return; handleSend(); }Also applies to: 341-353
🤖 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/components/chat-input.js` around lines 242 - 253, The new DOM dataset check in chat-input.js is redundant with the existing send-blocking logic already covered by sendBlockedRef.current and the explicit disabled/sendDisabled/isSending guards inside handleSend. Remove the domSendDisabled read and its related branching unless there is a real external mutation case to support, and keep the logic centered on the existing ref-based state in ChatInput/handleSend so behavior is maintained in one place.
🤖 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_v2/src/router.rs`:
- Around line 132-159: The
cached_global_auto_approve_feature/cache_global_auto_approve_feature logic is
memoizing a tenant/user-scoped setting that can change via Settings > Tools,
causing stale values to be replayed across requests. Remove this indefinite
cache for global_auto_approve or add invalidation tied to
AUTO_APPROVE_CONFIG_KEY updates so WebUiAuthenticatedCaller always reflects the
latest setting in GET /session.
---
Outside diff comments:
In
`@crates/ironclaw_webui_v2_static/static/js/pages/chat/components/chat-input.js`:
- Around line 242-253: The new DOM dataset check in chat-input.js is redundant
with the existing send-blocking logic already covered by sendBlockedRef.current
and the explicit disabled/sendDisabled/isSending guards inside handleSend.
Remove the domSendDisabled read and its related branching unless there is a real
external mutation case to support, and keep the logic centered on the existing
ref-based state in ChatInput/handleSend so behavior is maintained in one place.
🪄 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: cf34673f-bb05-4de3-846c-3670d06fe929
📒 Files selected for processing (7)
crates/ironclaw_product_workflow/src/reborn_services.rscrates/ironclaw_product_workflow/tests/reborn_services_contract.rscrates/ironclaw_webui_v2/src/handlers.rscrates/ironclaw_webui_v2/src/router.rscrates/ironclaw_webui_v2/tests/webui_v2_handlers_contract.rscrates/ironclaw_webui_v2_static/static/js/pages/chat/components/chat-input.jscrates/ironclaw_webui_v2_static/static/js/pages/chat/lib/chat-input.test.mjs
railway-app
Bot
temporarily deployed
to
ironclaw-ci-preview / ironclaw-pr-5247
July 1, 2026 09:26
Destroyed
Contributor
Author
|
@claude review |
This comment was marked as resolved.
This comment was marked as resolved.
railway-app
Bot
temporarily deployed
to
ironclaw-ci-preview / ironclaw-pr-5247
July 1, 2026 09:33
Destroyed
think-in-universe
approved these changes
Jul 1, 2026
17 of 30 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
2 participants
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.
Summary
Settings > Toolsso users can quickly find the global setting for automatically approving and executing all actions.agent.auto_approve_toolsonly while an approval gate is visible and keeps existing per-tool approval behavior unchanged.Linked Issue
Closes #5246
Screenshots
Validation
node --test crates/ironclaw_webui_v2_static/static/js/pages/chat/components/approval-card.test.mjsnode --test --test-name-pattern 'Chat deny gate callback routes through approve compatibility path' crates/ironclaw_webui_v2_static/static/js/pages/chat/lib/chat.test.mjsnpm run buildgit diff --checkSecurity Impact
No permission model changes. This only exposes a shortcut to an existing Tools setting and preserves the existing approval flow.
Database Impact
No schema or migration changes.
Blast Radius
Limited to WebUI v2 approval card rendering, settings-state lookup while an approval gate is visible, and the bundled static frontend asset.
Rollback Plan
Revert this PR to remove the approval-card shortcut and return the card to the previous per-tool-only approval UI.