Improve Action Review Across Heartwood Interfaces - #95
Merged
Conversation
📝 WalkthroughWalkthroughThis change standardizes action-review terminology and metadata across Heartwood. The gateway now exposes policy-aware mode and presentation data, validates persisted modes, synchronizes configuration access, and refreshes cached services. The terminal adds Sequence Diagram(s)sequenceDiagram
participant Researcher
participant WebUI
participant SessionGateway
participant ActionService
Researcher->>WebUI: open action-review settings
WebUI->>SessionGateway: select allowed mode
SessionGateway->>ActionService: apply configuration
ActionService-->>SessionGateway: return action-set state
SessionGateway-->>WebUI: render grouped actions
Researcher->>WebUI: allow or reject complete set
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (5)
packages/cli/src/heartwood/cli/__init__.py (1)
833-837: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDerive the default-mode label instead of hardcoding it.
"Review Every Action"duplicates the label thatACTION_MODE_OPTIONSowns, while line 860 selects"always-confirm"through the gateway. Usingaction_mode_label("always-confirm")keeps setup output aligned if the label changes.♻️ Proposed refactor
print( " Action review: Existing project setting" if resume_existing - else " Action review: Review Every Action" + else f" Action review: {action_mode_label('always-confirm')}" )🤖 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 `@packages/cli/src/heartwood/cli/__init__.py` around lines 833 - 837, Update the action review output near the resume_existing conditional to derive the default-mode text through action_mode_label("always-confirm") instead of hardcoding "Review Every Action". Preserve the existing "Existing project setting" branch and reuse the ACTION_MODE_OPTIONS-backed helper so the setup label stays synchronized with the selected gateway mode.packages/webui/src/components/SessionRail.tsx (1)
21-22: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value
UtilityPanelis now declared identically in bothSessionRail.tsxandUtilitySheet.tsx.Adding a member requires editing both. Consider exporting it once (e.g. from
../types) and re-exporting.🤖 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 `@packages/webui/src/components/SessionRail.tsx` around lines 21 - 22, Consolidate the duplicated UtilityPanel type used by SessionRail and UtilitySheet into a single shared declaration, preferably in the existing types module, and import or re-export it from both components. Preserve the current union members and update references so adding future members requires changing only the shared type.packages/webui/src/components/ConversationWorkspace.tsx (1)
288-294: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winKey the focus effect on the representative's id, not object identity.
setRepresentativeis a fresh element reference wheneverbuildViewModelre-runs (new events arrive), so the heading can steal focus repeatedly while the same action set is pending — e.g. while the researcher is reading the expanded arguments.♻️ Suggested change
- useEffect(() => { - if (setRepresentative) headingRef.current?.focus(); - }, [setRepresentative]); + const representativeId = setRepresentative?.targetId ?? null; + useEffect(() => { + if (representativeId !== null) headingRef.current?.focus(); + }, [representativeId]);🤖 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 `@packages/webui/src/components/ConversationWorkspace.tsx` around lines 288 - 294, Update the focus effect for setRepresentative in ConversationWorkspace so its dependency tracks the representative’s stable id rather than the object reference. Preserve the existing focus behavior when the action set changes, while preventing refocus when buildViewModel recreates the same representative object.packages/webui/src/styles.css (1)
700-702: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valuePrefer
:focus-visiblefor suppressing the heading outline.The heading is
tabIndex={-1}and focused programmatically, so hiding the ring is reasonable, but:focus-visiblekeeps an indicator if the element ever becomes keyboard-reachable.♻️ Suggested change
-.approval-introduction h2:focus { +.approval-introduction h2:focus:not(:focus-visible) { outline: none; }🤖 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 `@packages/webui/src/styles.css` around lines 700 - 702, Update the .approval-introduction h2 focus styling to use :focus-visible instead of :focus when suppressing the outline, preserving the current programmatic-focus behavior while allowing an indicator for keyboard-reachable focus.packages/gateway/src/heartwood/gateway/_project_config.py (1)
363-394: 🩺 Stability & Availability | 🔵 Trivial | 💤 Low valueClarify
locked()’s non-reentrantFileLockbehavior
locked()creates a newFileLockon each call. If a nested call happens in the same thread,filelockwill catch the self-deadlock and raise at runtime rather than hang indefinitely, but the current helper contract/docs should make this explicit. Use a singleis_singletonlock or re-shield the helper with a clear non-reentrancy requirement/error.🤖 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 `@packages/gateway/src/heartwood/gateway/_project_config.py` around lines 363 - 394, The locked() helper currently creates a new FileLock per invocation without documenting or enforcing its non-reentrant behavior. Update locked() and its callers’ contract to clearly require non-nested use, or configure a shared singleton FileLock so nested acquisition is handled explicitly; ensure any unsupported nested call raises a clear error rather than relying on an opaque self-deadlock failure.
🤖 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 `@packages/cli/src/heartwood/cli/_tui.py`:
- Around line 428-445: Guard the self.session.action_settings() call in
action_show_permissions with the existing ActionSettingsError handling pattern,
and return without pushing ActionModeScreen when loading settings fails.
Preserve the current locked_reason calculation and _action_mode_selected
callback behavior for successful reads.
In `@packages/compliance/src/heartwood/compliance/_reviewer_packet.py`:
- Around line 148-150: Update the action-policy sentence in the reviewer packet
text to explicitly state that the deployment-policy opt-in `confirm-risky`
automatically allows only action sets composed entirely of low-risk actions,
replacing the incomplete “continues only sets” wording while preserving the
surrounding policy details.
---
Nitpick comments:
In `@packages/cli/src/heartwood/cli/__init__.py`:
- Around line 833-837: Update the action review output near the resume_existing
conditional to derive the default-mode text through
action_mode_label("always-confirm") instead of hardcoding "Review Every Action".
Preserve the existing "Existing project setting" branch and reuse the
ACTION_MODE_OPTIONS-backed helper so the setup label stays synchronized with the
selected gateway mode.
In `@packages/gateway/src/heartwood/gateway/_project_config.py`:
- Around line 363-394: The locked() helper currently creates a new FileLock per
invocation without documenting or enforcing its non-reentrant behavior. Update
locked() and its callers’ contract to clearly require non-nested use, or
configure a shared singleton FileLock so nested acquisition is handled
explicitly; ensure any unsupported nested call raises a clear error rather than
relying on an opaque self-deadlock failure.
In `@packages/webui/src/components/ConversationWorkspace.tsx`:
- Around line 288-294: Update the focus effect for setRepresentative in
ConversationWorkspace so its dependency tracks the representative’s stable id
rather than the object reference. Preserve the existing focus behavior when the
action set changes, while preventing refocus when buildViewModel recreates the
same representative object.
In `@packages/webui/src/components/SessionRail.tsx`:
- Around line 21-22: Consolidate the duplicated UtilityPanel type used by
SessionRail and UtilitySheet into a single shared declaration, preferably in the
existing types module, and import or re-export it from both components. Preserve
the current union members and update references so adding future members
requires changing only the shared type.
In `@packages/webui/src/styles.css`:
- Around line 700-702: Update the .approval-introduction h2 focus styling to use
:focus-visible instead of :focus when suppressing the outline, preserving the
current programmatic-focus behavior while allowing an indicator for
keyboard-reachable focus.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: a153eb3b-43c9-4abe-86d2-23fbaeda54f2
⛔ Files ignored due to path filters (3)
documentation/assets/screenshots/browser-action-review.pngis excluded by!**/*.pngdocumentation/assets/screenshots/browser-action-settings.pngis excluded by!**/*.pngdocumentation/assets/screenshots/browser-conversation.pngis excluded by!**/*.png
📒 Files selected for processing (44)
documentation/architecture/index.mddocumentation/architecture/sessions-audit.mddocumentation/index.mddocumentation/models/connections.mddocumentation/operate/security.mddocumentation/reference/cli.mddocumentation/start/index.mddocumentation/use/actions-audit.mddocumentation/use/browser.mddocumentation/use/terminal.mdpackages/adapters/src/heartwood/adapters/platform/carina.pypackages/cli/README.mdpackages/cli/src/heartwood/cli/__init__.pypackages/cli/src/heartwood/cli/_interactive.pypackages/cli/src/heartwood/cli/_tui.pypackages/cli/tests/test_cli.pypackages/cli/tests/test_interactive.pypackages/compliance/src/heartwood/compliance/_reviewer_packet.pypackages/compliance/tests/test_reviewer_packet.pypackages/gateway/src/heartwood/gateway/__init__.pypackages/gateway/src/heartwood/gateway/_action_presentation.pypackages/gateway/src/heartwood/gateway/_action_settings.pypackages/gateway/src/heartwood/gateway/_gateway.pypackages/gateway/src/heartwood/gateway/_project_config.pypackages/gateway/src/heartwood/gateway/_readiness.pypackages/gateway/tests/test_action_settings.pypackages/gateway/tests/test_gateway_contract.pypackages/gateway/tests/test_project_config.pypackages/notebook/src/heartwood/notebook/_widgets.pypackages/notebook/tests/test_notebook.pypackages/webui/scripts/smoke-reference-analysis.cjspackages/webui/src/App.test.tsxpackages/webui/src/App.tsxpackages/webui/src/actionPresentation.tspackages/webui/src/client.test.tspackages/webui/src/components/ConversationWorkspace.tsxpackages/webui/src/components/SessionRail.tsxpackages/webui/src/components/UtilitySheet.tsxpackages/webui/src/components/WorkspaceHeader.tsxpackages/webui/src/e2e/app.spec.tspackages/webui/src/styles.csspackages/webui/src/types.tspackages/webui/src/viewModel.test.tspackages/webui/src/viewModel.ts
1 task
This was referenced Jul 26, 2026
This was referenced Jul 31, 2026
1 task
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
♻️ Current situation & Problem
Action review used different terminology and presentation across interfaces, obscured grouped-decision scope, and could leave a cached session service on stale project settings. This advances #26 and #41 while remaining independent of the runtime work in #24.
⚙️ Release Notes
📚 Documentation
Updated the terminal, browser, action-review, security, and command guidance with current terminology and regenerated browser screenshots.
✅ Testing
Code of Conduct & Contributing Guidelines
By creating and submitting this pull request, you agree to follow our Code of Conduct and Contributing Guidelines: