Expose Agent Actions And Workspace Changes - #103
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (22)
🚧 Files skipped from review as they are similar to previous changes (22)
📝 WalkthroughWalkthroughThis change adds versioned, correlated action records with typed details, outcomes, affected paths, replay persistence, and fail-closed unknown states. It adds bounded read-only workspace inspection for trees, files, changes, and diffs. REST, CLI, Textual, notebook, and browser interfaces consume shared gateway responses. The browser adds Files and Changes tabs with read-only code and diff viewers. Tests and smoke workflows cover security, limits, replay, transport, rendering, accessibility, and integration behavior. Sequence Diagram(s)sequenceDiagram
participant Client
participant SessionGateway
participant WorkspaceInspector
participant Projection
Client->>SessionGateway: request workspace tree or changes
SessionGateway->>WorkspaceInspector: validate path and limits
WorkspaceInspector->>Projection: read session-scoped action evidence
WorkspaceInspector-->>SessionGateway: return bounded response
SessionGateway-->>Client: return typed workspace data
Possibly related issues
Possibly related PRs
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![github-code-quality[bot]](https://avatars.githubusercontent.com/u/9919?s=60&v=4){kind=small}
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 14
🧹 Nitpick comments (18)
packages/webui/src/App.test.tsx (2)
1-1: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDuplicate workspace limits fixture across two test files.
App.test.tsx'sworkspaceLimits()andclient.test.ts's inlinelimitsobject define the same bounded-response limits with equivalent numeric values, with no shared source of truth inpackages/webui/src/test/fixtures.ts.
packages/webui/src/App.test.tsx#L187-195: MoveworkspaceLimits()intopackages/webui/src/test/fixtures.tsas a shared export, and import it here.packages/webui/src/client.test.ts#L227-234: Replace the inlinelimitsobject literal with the same sharedworkspaceLimits()export frompackages/webui/src/test/fixtures.ts.🤖 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/App.test.tsx` at line 1, Move the App.test.tsx workspaceLimits() fixture into the shared test/fixtures.ts module and export it. Update App.test.tsx and client.test.ts to import and reuse workspaceLimits(), replacing client.test.ts’s inline limits object while preserving the existing bounded-response values.
187-195: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDeduplicate the workspace limits fixture.
workspaceLimits()here and the inlinelimitsobject inpackages/webui/src/client.test.ts(lines 227-234) define the same bounded-response limits with equivalent numeric values. Move this intopackages/webui/src/test/fixtures.tsas a shared export, alongsidesyntheticActionandemptyProjection, so both test files use one source of truth.🤖 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/App.test.tsx` around lines 187 - 195, Move the workspaceLimits fixture from App.test.tsx into the shared test fixtures module alongside syntheticAction and emptyProjection, exporting it for reuse. Update App.test.tsx and client.test.ts to import and use this shared fixture instead of defining local or inline limits objects, preserving the existing limit values.packages/webui/src/components/ProjectWorkspace.tsx (1)
71-100: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winRemove the response casts by branching the request per mode.
Lines 83 and 87 cast the resolved value with
as WorkspaceTreeandas WorkspaceChanges. The ternary at Lines 73-76 widens the promise type to the union, so TypeScript no longer verifies that the tree branch received a tree. If a client method ever returns the other shape, the cast hides the mismatch and the component renders undefined fields.♻️ Proposed refactor
useEffect(() => { let active = true; - const request = - mode === "files" ? - client.getWorkspaceTree(sessionId) - : client.getWorkspaceChanges(sessionId); - void request - .then((response) => { - if (!active) return; - if (mode === "files") { - setOverview({ - key: overviewKey, - tree: response as WorkspaceTree, - }); - } else { - setOverview({ - changes: response as WorkspaceChanges, - key: overviewKey, - }); - } - }) + const request: Promise<OverviewSnapshot> = + mode === "files" ? + client + .getWorkspaceTree(sessionId) + .then((tree) => ({ key: overviewKey, tree })) + : client + .getWorkspaceChanges(sessionId) + .then((changes) => ({ changes, key: overviewKey })); + void request + .then((snapshot) => { + if (active) setOverview(snapshot); + }) .catch((caught: unknown) => { if (active) { setOverview({ error: errorMessage(caught), key: overviewKey }); } });🤖 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/ProjectWorkspace.tsx` around lines 71 - 100, Refactor the useEffect request flow to branch explicitly on mode before awaiting the client call, so each branch receives the correctly typed response from getWorkspaceTree or getWorkspaceChanges. Remove the WorkspaceTree and WorkspaceChanges casts while preserving the active guard, overviewKey updates, and shared error handling.packages/webui/src/components/ConversationWorkspace.tsx (1)
446-450: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winMove action state labels into the shared presentation contract.
actionStateLabel(String)derives user-facing text from raw action states in the browser, whileActionPresentationResponseonly exposes tool and risk labels. Add mapped state labels and possible unknown labels to the gateway-owned action presentation schema/response, then let the web, CLI, and notebook adapters consume them instead of formattingaction.statewith local helpers.🤖 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 446 - 450, Remove the browser-local actionStateLabel formatting and extend the gateway-owned ActionPresentationResponse contract with mapped action-state labels plus possible unknown-state labels. Populate these fields centrally, then update the web, CLI, and notebook adapters to consume the shared presentation values instead of deriving display text from action.state.Source: Coding guidelines
packages/webui/src/components/ProjectWorkspace.test.tsx (1)
487-575: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAlign the selection state model with the App rendering.
App.tsxrenders two separateProjectWorkspaceinstances keyed by${sessionId}-filesand${sessionId}-changes, with fixedmodeprops. This test switchesmodeon a single unkeyed instance, so it exercises state that production cannot hold. Either make mode switching a supported app contract, or replace the per-modeselectedPathsrecord with a singleselectedPath: string | nulland update this test to cover the two-instance behavior.🤖 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/ProjectWorkspace.test.tsx` around lines 487 - 575, This test verifies independent selection state across modes by rerendering a single ProjectWorkspace instance with different mode props, but production renders two separately keyed instances (keyed by ${sessionId}-files and ${sessionId}-changes) that never undergo mode switching. Either simplify the selection state model to use a single selectedPath property instead of a per-mode selectedPaths record and update the test to verify that two separate component instances maintain independent selections without rerendering, or explicitly make mode switching a supported contract in the ProjectWorkspace component. Choose the approach that aligns the test behavior with the App rendering pattern and update the test accordingly to cover the actual production scenario.packages/core-adapter/src/heartwood/core_adapter/_service.py (2)
754-767: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueReuse one intent-resolution helper for both recovery paths.
_has_failed_approval_recovery_lockedrepeats the receipt lookup, receipt validation, and_approval_intentderivation from_recover_approval_commands_locked. Extract a shared iterator over(command_id, record, intent)so both paths stay consistent when receipt validation changes.🤖 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/core-adapter/src/heartwood/core_adapter/_service.py` around lines 754 - 767, Extract a shared iterator that yields each unresolved command’s command_id, validated record, and derived approval intent, including the existing missing-receipt error. Update both _has_failed_approval_recovery_locked and _recover_approval_commands_locked to consume this iterator, removing their duplicated lookup and _approval_intent derivation while preserving current behavior.
779-806: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueSimplify the unreachable
elifguard.The enclosing condition at Line 779 already proves that
_approval_intent_failed(events, intent)isFalse. Theelif not _approval_intent_failed(events, intent)test at Line 796 is therefore always true. Replace it with a plainelseto make the fail-closed path explicit.♻️ Proposed simplification
- elif not _approval_intent_failed(events, intent): + else: recovered.append(self._record_unknown_approval_outcome(intent))🤖 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/core-adapter/src/heartwood/core_adapter/_service.py` around lines 779 - 806, In the approval recovery flow around _approval_intent_resolved and _approval_intent_failed, replace the redundant “elif not _approval_intent_failed(events, intent)” branch with a plain else. Preserve the existing _record_unknown_approval_outcome(intent) behavior and surrounding event replay logic.packages/gateway/src/heartwood/gateway/_session_projection.py (1)
1171-1186: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueExport the discriminated union and the command-outcome model.
__all__lists each concrete detail model but omits theProjectionActionDetailsalias andProjectionCommandOutcome, whichpackages/gateway/src/heartwood/gateway/__init__.pyimports at Line 148. Downstream Python consumers must then rebuild the union themselves;packages/notebook/tests/test_notebook.pyalready declares a local union of the three detail types. Export both names so consumers annotate against one contract.🤖 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/_session_projection.py` around lines 1171 - 1186, Update __all__ in the session projection module to include the ProjectionActionDetails discriminated-union alias and ProjectionCommandOutcome model alongside the existing projection exports. Preserve the current concrete detail-model exports so downstream consumers can import the shared action-details contract and command outcome directly.packages/gateway/src/heartwood/gateway/_gateway.py (1)
629-641: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winInitialize the project before bounded workspace reads.
workspace_treeandworkspace_filedo not callself.project.initialize(). Every other read path in this class, for examplereplay_eventsat Line 665 andpersisted_session_projectionat Line 682, initializes the project first.workspace_changesandworkspace_diffinitialize it indirectly throughsession_projection. Add the same call so the two path-only methods behave consistently.♻️ Proposed change
def workspace_tree( self, *, path: str = ".", depth: int | None = None, ) -> WorkspaceTreeResponse: """Return the bounded project tree shared by every interface.""" + self.project.initialize() return self.workspace_inspector.tree(path, depth=depth) def workspace_file(self, *, path: str) -> WorkspaceFileResponse: """Return one bounded read-only project file.""" + self.project.initialize() return self.workspace_inspector.file(path)🤖 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/_gateway.py` around lines 629 - 641, Update workspace_tree and workspace_file to call self.project.initialize() before invoking self.workspace_inspector.tree or self.workspace_inspector.file, matching the initialization behavior of the other read paths while preserving their existing arguments and return values.packages/gateway/tests/test_openhands_sdk.py (1)
2579-2580: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueAssert against the exported limit constants.
These assertions repeat
16_000and240. If_MAX_TOOL_RESULT_CHARSor_MAX_TOOL_RESULT_LINESchanges, the test keeps asserting the old bounds. Import the constants and use them here.♻️ Proposed change
- assert len(translated.tool_execution.result) <= 16_000 - assert len(translated.tool_execution.result.splitlines()) <= 240 + assert len(translated.tool_execution.result) <= _MAX_TOOL_RESULT_CHARS + assert len(translated.tool_execution.result.splitlines()) <= _MAX_TOOL_RESULT_LINES🤖 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/tests/test_openhands_sdk.py` around lines 2579 - 2580, Update the test assertions for translated.tool_execution.result to import and use _MAX_TOOL_RESULT_CHARS and _MAX_TOOL_RESULT_LINES instead of hardcoded 16_000 and 240 values, keeping the existing length and line-count checks unchanged.packages/gateway/src/heartwood/gateway/_openhands_sdk.py (2)
1456-1464: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winCompute the project path once per action.
_affected_pathscalls_project_path, and_tool_callcalls_project_pathagain for the same event. Each call runsPath.resolve(), which touches the filesystem. Pass the resolved value into_affected_pathsinstead.♻️ Proposed change
-def _affected_paths(event: ActionEvent, *, workspace: Path | None) -> tuple[str, ...]: +def _affected_paths(event: ActionEvent, *, project_path: str | None) -> tuple[str, ...]: """Return only paths proven to be modified by a typed file-editor action.""" action = event.action - project_path = _project_path(event, workspace=workspace) if not isinstance(action, FileEditorAction) or action.command == "view": return () if project_path is None: return () return (project_path,)Then update the caller in
_tool_call:project_path = _project_path(event, workspace=workspace) return ProposedToolCall( ... affected_paths=_affected_paths(event, project_path=project_path), project_path=project_path, )🤖 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/_openhands_sdk.py` around lines 1456 - 1464, Update _affected_paths to accept the already-resolved project_path as a keyword argument instead of calling _project_path internally. In _tool_call, compute _project_path once and pass that value to both project_path and _affected_paths, preserving the existing action and None checks.
1031-1047: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winExtract the shared interrupted-outcome rule.
_interrupted_outcome_errorand_state_events(Lines 1321-1338) now computeintentional_pauseand the "inactive with unmatched actions" condition with duplicated expressions. The two copies must stay in agreement, otherwisesubmit_turnand state reporting can disagree about a fatal outcome. Extract one helper that returns both conditions and call it from both sites.🤖 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/_openhands_sdk.py` around lines 1031 - 1047, The interrupted-outcome logic is duplicated between _interrupted_outcome_error and _state_events, allowing submit_turn and state reporting to diverge. Extract a shared helper returning intentional_pause and the inactive-with-unmatched-actions result, then update both methods to use that helper while preserving their existing outcome handling.packages/gateway/tests/test_gateway_contract.py (1)
224-236: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueReuse one
FailingResolutionBackenddefinition.This class is identical to the
FailingResolutionBackenddefined at Lines 144-156 in the previous test. Move it to module scope and reference it from both tests so the fatal-resolution behavior stays defined in one place.🤖 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/tests/test_gateway_contract.py` around lines 224 - 236, Move the duplicate FailingResolutionBackend class definition to module scope, preserving its resolve_confirmation behavior and return value, then remove the local definitions and reuse the shared class in both tests.packages/gateway/src/heartwood/gateway/_workspace.py (5)
1010-1025: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueTruncated directories do not produce a deterministic entry set.
_bounded_directory_entriesstops afterlimitnon-reserved entries and sorts afterwards. The retained subset therefore depends on the filesystem iteration order, and only the presentation is sorted. The test at Line 176 ofpackages/gateway/tests/test_workspace.pyshows this: the input order["z.txt", ".git", "a.txt", ...]yields["a.txt", "z.txt"].The docstring of
treeat Line 119 states "deterministic bounded tree". Two clients inspecting the same truncated directory on different filesystems can receive different entries. Either sort before applying the limit, which costs a full enumeration, or adjust the docstring to state that a truncated directory returns an arbitrary bounded subset.🤖 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/_workspace.py` around lines 1010 - 1025, Update _bounded_directory_entries so non-reserved entries are sorted by name before applying limit, ensuring truncated results are deterministic; preserve the truncated flag and bounded output behavior.
363-368: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winEach diff request repeats the full change enumeration.
diffcallsself.changes(projection)to confirm that the path is in the bounded changed-file list.changesinvokes the OpenHands Git API again, so opening one file diff in the browser or terminal costs a second full repository scan. The test at Line 539 records this asrequested_change_paths == [".", "."].Consider accepting an already-computed
WorkspaceChangesResponseas an optional argument, so an interface that has just rendered the changed-file list can pass it. That keeps the fail-closed membership check and removes the duplicate scan.🤖 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/_workspace.py` around lines 363 - 368, Update Workspace.diff to accept an optional precomputed WorkspaceChangesResponse and use it for the bounded-path membership check when provided; otherwise retain the existing self.changes(projection) fallback. Preserve the fail-closed behavior and update callers that already have the rendered changes response to pass it, avoiding duplicate Git scans.
851-853: 🩺 Stability & Availability | 🔵 TrivialGlobal logger suppression removes Git diagnostics process-wide.
_configure_openhands_git_loggingraises theopenhands.sdk.gitlogger toCRITICALon the first workspace call and never restores it. The typed responses intentionally hide upstream probe noise from users, and the test at Line 1001 ofpackages/gateway/tests/test_workspace.pyasserts that no records are emitted. The side effect is process-wide and permanent, so a maintainer investigating a Git failure loses the upstream detail.Consider attaching a filter or a dedicated handler scoped to the inspection calls, or keeping the suppression but recording the reason in Heartwood's own structured log when a Git path returns
status: "unavailable".🤖 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/_workspace.py` around lines 851 - 853, Replace the process-wide suppression in _configure_openhands_git_logging with call-scoped filtering or handling around the workspace inspection calls, so upstream Git probe noise remains hidden from typed responses without permanently muting openhands.sdk.git diagnostics. Preserve the no-records behavior asserted by the workspace test and restore any temporary logger state after each inspection.
737-787: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winType
statusandsourcewith the schema literals.Both helpers declare
status: strandsource: str. The schemas restrict these fields to Literal unions. A misspelled status therefore passes type checking and fails only at runtime insideapi_response. The rest of this file already annotates such values precisely, for examplekindat Line 183 andstatusat Line 698.♻️ Proposed tightening of the response helper signatures
`@staticmethod` def _file_response( *, path: str, - status: str, + status: Literal["available", "binary", "truncated", "unavailable", "unsupported"], content: str | None = None,`@staticmethod` def _diff_response( *, path: str, - status: str, - source: str = "unavailable", + status: Literal[ + "available", "binary", "truncated", "unavailable", "non-git", "unsupported" + ], + source: Literal["git", "session-action", "unavailable"] = "unavailable", original: str | None = 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/gateway/src/heartwood/gateway/_workspace.py` around lines 737 - 787, Update the _file_response and _diff_response helper signatures to annotate status and source with their corresponding schema Literal union types instead of str, matching the WorkspaceFileResponse and WorkspaceDiffResponse definitions and existing precise annotations elsewhere in the file.
637-638: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueRemove the unused
_safe_existing_filehelper.
_safe_existing_fileis only defined and has no callers.removeit, unless this helper has a planned API use.🤖 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/_workspace.py` around lines 637 - 638, Remove the unused _safe_existing_file method from the gateway workspace implementation, since it has no callers and no planned API use. Leave _safe_path and other path-validation behavior unchanged.
🤖 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 666-728: Apply the generation-counter and exclusive-worker pattern
used by _load_workspace_file and _load_workspace_diff to
_request_workspace_overview and _load_workspace_overview. Schedule only one
overview worker at a time, increment and capture a refresh generation when
requesting an overview, and pass that generation through
_finish_workspace_overview so stale results are discarded before rendering or
triggering follow-up refreshes.
In `@packages/core-adapter/src/heartwood/core_adapter/_facade.py`:
- Around line 767-772: Update _persisted_tool_kind so mypy can verify its
Literal return type when value is object: either add an explicit typing.cast
consistent with _action_kind or replace the set-membership check with explicit
equality branches for "terminal", "file-editor", and "task", while retaining
"other" as the fallback.
In `@packages/gateway/src/heartwood/gateway/_gateway.py`:
- Around line 499-527: Update handle() and _reconciled_session_events() to reuse
the persisted replay and project_session result already computed for projected
commands, passing them through reconciliation instead of replaying
FileSessionStore and projecting the same events again. Preserve the existing
fatal_unavailable_reason behavior while ensuring each command uses a single
shared persisted replay.
In `@packages/gateway/src/heartwood/gateway/_openhands_sdk.py`:
- Around line 1467-1481: Update _project_path to reuse the shared
project-relative path logic from _workspace_paths.py, including
RESERVED_PROJECT_COMPONENTS and its case-insensitive exclusion behavior. Replace
the local exact-match checks for ".heartwood" and ".git" while preserving
workspace containment validation and returning the normalized relative path for
allowed files.
In `@packages/gateway/src/heartwood/gateway/_rest.py`:
- Around line 241-275: Update the workspace route handling around the visible
session/workspace dispatch: parse the optional depth value before the shared try
block and map malformed integers to status 400, while preserving valid depth
behavior. When the four-part workspace path matches but the request method is
not GET, return status 405 instead of allowing it to fall through to the
unknown-route response; keep existing session and workspace error mappings
unchanged.
- Around line 250-268: Update the query parsing in the workspace route handler
to retain a missing path as None instead of defaulting it to ".". Use the
resulting None check for the required-path validation in the file and diff
branches, while continuing to pass "." through when it was explicitly supplied.
In `@packages/gateway/src/heartwood/gateway/_session_projection.py`:
- Around line 393-410: Update the identity check in the action execution
projection to treat a missing action_id from legacy TOOL_CALL_PROPOSED events as
unknown rather than conflicting with a newer execution action_id. Keep strict
comparison when the proposal contains an action_id, while preserving the
existing tool-name and terminal-outcome validations and integrity-failure
behavior for genuine mismatches.
In `@packages/gateway/src/heartwood/gateway/_workspace.py`:
- Around line 210-221: Guard the _directory_has_public_entry call in the
non-descend branch against OSError, marking entry["kind"] as "unsupported" when
the probe fails and continuing traversal. Preserve the existing
truncated-directory handling and ensure raw probe errors do not escape tree() as
untyped exceptions.
- Around line 705-718: Update the _session_changes logic that builds
WorkspaceChangeResponse.action_ids to append action.action_id instead of
action.tool_call_id, while preserving deduplication and existing-path
aggregation. Update the related fixture or assertion to expect the action_id
value rather than the tool-call identifier.
- Around line 565-573: Update _git_baseline() to normalize decoded Git content
by removing only the trailing newline, matching _openhands_current_text’s
splitlines()-compatible behavior. Replace content.strip() while preserving
leading indentation and all other whitespace, so Git comparison remains accurate
for indented files.
- Around line 885-899: Update _sanitized_git_environment to avoid mutating
process-global os.environ during OpenHands git inspection, preferably by
wrapping the calls in an isolated subprocess or equivalent environment-scoped
mechanism that passes _SAFE_GIT_ENVIRONMENT explicitly. If the SDK prevents
this, document the unavoidable global-environment constraint directly beside
_sanitized_git_environment and ensure the existing lock and restoration behavior
remain intact.
In `@packages/notebook/src/heartwood/notebook/_widgets.py`:
- Around line 104-148: Sanitize all user-controlled action text in _action_items
before it reaches _section_html, reusing the existing terminal_safe_text
behavior for Unicode control, bidi-override, and surrogate characters, then
retain HTML escaping afterward. Cover command labels, paths, arguments,
decisions, and outcome output through the notebook rendering path, and
add/update tests for _action_items/_section_html matching the CLI
control-character coverage.
In `@packages/webui/src/App.tsx`:
- Around line 902-937: Update the Files and Changes tab rendering around
ProjectWorkspace so each workspace view is not mounted while inactive or before
first activation. Track whether each tab has been visited, render its
ProjectWorkspace only after activation, and preserve the mounted instance
thereafter while retaining the existing sessionId, revision, mode, and key
behavior.
In `@packages/webui/src/styles.css`:
- Line 606: In the font-family declaration at the affected styles.css rule,
remove the quotes from the SFMono-Regular family name while leaving the
remaining fallback fonts unchanged.
---
Nitpick comments:
In `@packages/core-adapter/src/heartwood/core_adapter/_service.py`:
- Around line 754-767: Extract a shared iterator that yields each unresolved
command’s command_id, validated record, and derived approval intent, including
the existing missing-receipt error. Update both
_has_failed_approval_recovery_locked and _recover_approval_commands_locked to
consume this iterator, removing their duplicated lookup and _approval_intent
derivation while preserving current behavior.
- Around line 779-806: In the approval recovery flow around
_approval_intent_resolved and _approval_intent_failed, replace the redundant
“elif not _approval_intent_failed(events, intent)” branch with a plain else.
Preserve the existing _record_unknown_approval_outcome(intent) behavior and
surrounding event replay logic.
In `@packages/gateway/src/heartwood/gateway/_gateway.py`:
- Around line 629-641: Update workspace_tree and workspace_file to call
self.project.initialize() before invoking self.workspace_inspector.tree or
self.workspace_inspector.file, matching the initialization behavior of the other
read paths while preserving their existing arguments and return values.
In `@packages/gateway/src/heartwood/gateway/_openhands_sdk.py`:
- Around line 1456-1464: Update _affected_paths to accept the already-resolved
project_path as a keyword argument instead of calling _project_path internally.
In _tool_call, compute _project_path once and pass that value to both
project_path and _affected_paths, preserving the existing action and None
checks.
- Around line 1031-1047: The interrupted-outcome logic is duplicated between
_interrupted_outcome_error and _state_events, allowing submit_turn and state
reporting to diverge. Extract a shared helper returning intentional_pause and
the inactive-with-unmatched-actions result, then update both methods to use that
helper while preserving their existing outcome handling.
In `@packages/gateway/src/heartwood/gateway/_session_projection.py`:
- Around line 1171-1186: Update __all__ in the session projection module to
include the ProjectionActionDetails discriminated-union alias and
ProjectionCommandOutcome model alongside the existing projection exports.
Preserve the current concrete detail-model exports so downstream consumers can
import the shared action-details contract and command outcome directly.
In `@packages/gateway/src/heartwood/gateway/_workspace.py`:
- Around line 1010-1025: Update _bounded_directory_entries so non-reserved
entries are sorted by name before applying limit, ensuring truncated results are
deterministic; preserve the truncated flag and bounded output behavior.
- Around line 363-368: Update Workspace.diff to accept an optional precomputed
WorkspaceChangesResponse and use it for the bounded-path membership check when
provided; otherwise retain the existing self.changes(projection) fallback.
Preserve the fail-closed behavior and update callers that already have the
rendered changes response to pass it, avoiding duplicate Git scans.
- Around line 851-853: Replace the process-wide suppression in
_configure_openhands_git_logging with call-scoped filtering or handling around
the workspace inspection calls, so upstream Git probe noise remains hidden from
typed responses without permanently muting openhands.sdk.git diagnostics.
Preserve the no-records behavior asserted by the workspace test and restore any
temporary logger state after each inspection.
- Around line 737-787: Update the _file_response and _diff_response helper
signatures to annotate status and source with their corresponding schema Literal
union types instead of str, matching the WorkspaceFileResponse and
WorkspaceDiffResponse definitions and existing precise annotations elsewhere in
the file.
- Around line 637-638: Remove the unused _safe_existing_file method from the
gateway workspace implementation, since it has no callers and no planned API
use. Leave _safe_path and other path-validation behavior unchanged.
In `@packages/gateway/tests/test_gateway_contract.py`:
- Around line 224-236: Move the duplicate FailingResolutionBackend class
definition to module scope, preserving its resolve_confirmation behavior and
return value, then remove the local definitions and reuse the shared class in
both tests.
In `@packages/gateway/tests/test_openhands_sdk.py`:
- Around line 2579-2580: Update the test assertions for
translated.tool_execution.result to import and use _MAX_TOOL_RESULT_CHARS and
_MAX_TOOL_RESULT_LINES instead of hardcoded 16_000 and 240 values, keeping the
existing length and line-count checks unchanged.
In `@packages/webui/src/App.test.tsx`:
- Line 1: Move the App.test.tsx workspaceLimits() fixture into the shared
test/fixtures.ts module and export it. Update App.test.tsx and client.test.ts to
import and reuse workspaceLimits(), replacing client.test.ts’s inline limits
object while preserving the existing bounded-response values.
- Around line 187-195: Move the workspaceLimits fixture from App.test.tsx into
the shared test fixtures module alongside syntheticAction and emptyProjection,
exporting it for reuse. Update App.test.tsx and client.test.ts to import and use
this shared fixture instead of defining local or inline limits objects,
preserving the existing limit values.
In `@packages/webui/src/components/ConversationWorkspace.tsx`:
- Around line 446-450: Remove the browser-local actionStateLabel formatting and
extend the gateway-owned ActionPresentationResponse contract with mapped
action-state labels plus possible unknown-state labels. Populate these fields
centrally, then update the web, CLI, and notebook adapters to consume the shared
presentation values instead of deriving display text from action.state.
In `@packages/webui/src/components/ProjectWorkspace.test.tsx`:
- Around line 487-575: This test verifies independent selection state across
modes by rerendering a single ProjectWorkspace instance with different mode
props, but production renders two separately keyed instances (keyed by
${sessionId}-files and ${sessionId}-changes) that never undergo mode switching.
Either simplify the selection state model to use a single selectedPath property
instead of a per-mode selectedPaths record and update the test to verify that
two separate component instances maintain independent selections without
rerendering, or explicitly make mode switching a supported contract in the
ProjectWorkspace component. Choose the approach that aligns the test behavior
with the App rendering pattern and update the test accordingly to cover the
actual production scenario.
In `@packages/webui/src/components/ProjectWorkspace.tsx`:
- Around line 71-100: Refactor the useEffect request flow to branch explicitly
on mode before awaiting the client call, so each branch receives the correctly
typed response from getWorkspaceTree or getWorkspaceChanges. Remove the
WorkspaceTree and WorkspaceChanges casts while preserving the active guard,
overviewKey updates, and shared error handling.
🪄 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: cbfade9f-681b-4ccd-8deb-d0227c7c83c4
⛔ Files ignored due to path filters (8)
documentation/assets/screenshots/browser-action-review.pngis excluded by!**/*.pngdocumentation/assets/screenshots/browser-action-settings.pngis excluded by!**/*.pngdocumentation/assets/screenshots/browser-changes.pngis excluded by!**/*.pngdocumentation/assets/screenshots/browser-conversation.pngis excluded by!**/*.pngdocumentation/assets/screenshots/browser-files.pngis excluded by!**/*.pngpackages/webui/package-lock.jsonis excluded by!**/package-lock.jsonpackages/webui/src/apiTypes.generated.tsis excluded by!**/*.generated.*packages/webui/src/sessionProjection.generated.tsis excluded by!**/*.generated.*
📒 Files selected for processing (62)
README.mddocumentation/architecture/sessions-audit.mddocumentation/architecture/system.mddocumentation/architecture/testing.mddocumentation/assets/screenshots/browser-changes.png.licensedocumentation/assets/screenshots/browser-files.png.licensedocumentation/reference/cli.mddocumentation/reference/troubleshooting.mddocumentation/use/actions-audit.mddocumentation/use/browser.mddocumentation/use/notebooks.mddocumentation/use/terminal.mdimages/generic/scripts/offline_stack_smoke.shimages/generic/scripts/verify_coding_agent_e2e.pypackages/cli/src/heartwood/cli/__init__.pypackages/cli/src/heartwood/cli/_interactive.pypackages/cli/src/heartwood/cli/_terminal_text.pypackages/cli/src/heartwood/cli/_tui.pypackages/cli/src/heartwood/cli/_workspace_presentation.pypackages/cli/tests/test_cli.pypackages/cli/tests/test_interactive.pypackages/cli/tests/test_workspace_presentation.pypackages/compliance/tests/test_coding_agent_qualification.pypackages/compliance/tests/test_container_assets.pypackages/core-adapter/src/heartwood/core_adapter/__init__.pypackages/core-adapter/src/heartwood/core_adapter/_facade.pypackages/core-adapter/src/heartwood/core_adapter/_service.pypackages/core-adapter/tests/test_deterministic_backend.pypackages/core-adapter/tests/test_session_service.pypackages/gateway/src/heartwood/gateway/__init__.pypackages/gateway/src/heartwood/gateway/_gateway.pypackages/gateway/src/heartwood/gateway/_openhands_sdk.pypackages/gateway/src/heartwood/gateway/_rest.pypackages/gateway/src/heartwood/gateway/_session_projection.pypackages/gateway/src/heartwood/gateway/_workspace.pypackages/gateway/src/heartwood/gateway/_workspace_paths.pypackages/gateway/tests/test_gateway_contract.pypackages/gateway/tests/test_openhands_sdk.pypackages/gateway/tests/test_session_projection.pypackages/gateway/tests/test_workspace.pypackages/notebook/src/heartwood/notebook/_view_model.pypackages/notebook/src/heartwood/notebook/_widgets.pypackages/notebook/tests/test_notebook.pypackages/schemas/src/heartwood/schemas/__init__.pypackages/schemas/src/heartwood/schemas/_api.pypackages/webui/package.jsonpackages/webui/scripts/smoke-jupyter-proxy.cjspackages/webui/scripts/smoke-reference-analysis.cjspackages/webui/src/App.test.tsxpackages/webui/src/App.tsxpackages/webui/src/client.test.tspackages/webui/src/client.tspackages/webui/src/components/CodeViewer.tsxpackages/webui/src/components/ConversationWorkspace.tsxpackages/webui/src/components/ProjectWorkspace.test.tsxpackages/webui/src/components/ProjectWorkspace.tsxpackages/webui/src/e2e/app.spec.tspackages/webui/src/projectionSchema.tspackages/webui/src/styles.csspackages/webui/src/test/fixtures.tspackages/webui/src/test/setup.tspackages/webui/src/types.ts
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
packages/gateway/src/heartwood/gateway/_action_presentation.py (1)
70-87: 🚀 Performance & Scalability | 🔵 Trivial | 💤 Low valueConsider a fast path for text that needs no escaping.
display_safe_textcallsunicodedata.categoryfor every character. The bounded workspace limits allow 512 KiB files and 1 MiB diffs, and_section_htmlinpackages/notebook/src/heartwood/notebook/_widgets.py(lines 204-216) calls this helper per item. The per-character loop therefore runs on the largest supported payloads.A cheap pre-check keeps the current behavior and skips the loop for ordinary text.
♻️ Proposed fast path
def display_safe_text(value: object, *, preserve_newlines: bool = False) -> str: """Render control and formatting characters visibly in presentation adapters.""" - rendered: list[str] = [] - for character in str(value): + text = str(value) + if text.isprintable() or (preserve_newlines and text.replace("\n", "").isprintable()): + return text + rendered: list[str] = [] + for character in text:🤖 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/_action_presentation.py` around lines 70 - 87, Update display_safe_text to add a cheap pre-check that returns the original string when it contains no characters requiring escaping, while preserving newline handling when preserve_newlines is enabled. Keep the existing per-character rendering path unchanged for text that may contain control, formatting, surrogate, or otherwise disallowed characters.packages/gateway/src/heartwood/gateway/_gateway.py (1)
637-666: 🚀 Performance & Scalability | 🔵 TrivialConsider the reconciliation cost on the workspace read path.
workspace_changesandworkspace_diffcallsession_projection, which reconciles durable state through_reconciled_session_events.packages/gateway/tests/test_workspace.py(line 1482) pins this behavior, so it is intended: non-Git change evidence must reflect current actions.The browser and terminal Changes views poll these endpoints. Each poll therefore triggers one backend reconciliation. If polling intervals are short, consider a projection revision check or a short-lived cache keyed by
session_idand projectionrevision, so repeated reads within one revision reuse the derived changes.
workspace_treeandworkspace_filecorrectly avoid the projection and the state lock, which keeps plain file reads cheap.🤖 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/_gateway.py` around lines 637 - 666, Optimize workspace_changes and workspace_diff by reusing derived change results for the same session_id and projection revision, using a short-lived cache or revision check around session_projection. Preserve reconciliation when the projection revision changes so non-Git evidence remains current, while repeated polls within one revision avoid redundant backend work.
🤖 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/gateway/src/heartwood/gateway/_action_presentation.py`:
- Around line 65-67: Update action_state_label to ensure unknown-state fallback
labels are sanitized before they reach terminal output, using the existing
terminal-safe text mechanism before format_action_record_lines interpolates the
heading; alternatively reject unknown states explicitly instead of returning an
unsanitized fallback. Preserve the existing labels for known ACTION_STATE_LABELS
entries.
---
Nitpick comments:
In `@packages/gateway/src/heartwood/gateway/_action_presentation.py`:
- Around line 70-87: Update display_safe_text to add a cheap pre-check that
returns the original string when it contains no characters requiring escaping,
while preserving newline handling when preserve_newlines is enabled. Keep the
existing per-character rendering path unchanged for text that may contain
control, formatting, surrogate, or otherwise disallowed characters.
In `@packages/gateway/src/heartwood/gateway/_gateway.py`:
- Around line 637-666: Optimize workspace_changes and workspace_diff by reusing
derived change results for the same session_id and projection revision, using a
short-lived cache or revision check around session_projection. Preserve
reconciliation when the projection revision changes so non-Git evidence remains
current, while repeated polls within one revision avoid redundant backend work.
🪄 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: ebb0d9b9-98e6-4b03-b436-b0c69f934d3f
⛔ Files ignored due to path filters (1)
packages/webui/src/apiTypes.generated.tsis excluded by!**/*.generated.*
📒 Files selected for processing (38)
images/generic/scripts/offline_stack_smoke.shimages/generic/scripts/verify_coding_agent_e2e.pypackages/cli/src/heartwood/cli/__init__.pypackages/cli/src/heartwood/cli/_interactive.pypackages/cli/src/heartwood/cli/_tui.pypackages/cli/src/heartwood/cli/_workspace_presentation.pypackages/cli/tests/test_cli.pypackages/cli/tests/test_interactive.pypackages/compliance/tests/test_coding_agent_qualification.pypackages/core-adapter/src/heartwood/core_adapter/_facade.pypackages/core-adapter/src/heartwood/core_adapter/_service.pypackages/core-adapter/tests/test_deterministic_backend.pypackages/core-adapter/tests/test_session_service.pypackages/gateway/src/heartwood/gateway/__init__.pypackages/gateway/src/heartwood/gateway/_action_presentation.pypackages/gateway/src/heartwood/gateway/_gateway.pypackages/gateway/src/heartwood/gateway/_openhands_sdk.pypackages/gateway/src/heartwood/gateway/_rest.pypackages/gateway/src/heartwood/gateway/_session_projection.pypackages/gateway/src/heartwood/gateway/_workspace.pypackages/gateway/tests/test_action_settings.pypackages/gateway/tests/test_gateway_contract.pypackages/gateway/tests/test_openhands_sdk.pypackages/gateway/tests/test_session_projection.pypackages/gateway/tests/test_workspace.pypackages/notebook/src/heartwood/notebook/_widgets.pypackages/notebook/tests/test_notebook.pypackages/schemas/src/heartwood/schemas/_api.pypackages/webui/src/App.test.tsxpackages/webui/src/App.tsxpackages/webui/src/actionPresentation.test.tspackages/webui/src/actionPresentation.tspackages/webui/src/client.test.tspackages/webui/src/components/ConversationWorkspace.tsxpackages/webui/src/components/ProjectWorkspace.test.tsxpackages/webui/src/components/ProjectWorkspace.tsxpackages/webui/src/e2e/app.spec.tspackages/webui/src/styles.css
🚧 Files skipped from review as they are similar to previous changes (29)
- images/generic/scripts/verify_coding_agent_e2e.py
- packages/webui/src/components/ProjectWorkspace.test.tsx
- packages/webui/src/client.test.ts
- images/generic/scripts/offline_stack_smoke.sh
- packages/gateway/src/heartwood/gateway/init.py
- packages/webui/src/styles.css
- packages/gateway/src/heartwood/gateway/_rest.py
- packages/webui/src/App.tsx
- packages/cli/src/heartwood/cli/_workspace_presentation.py
- packages/notebook/src/heartwood/notebook/_widgets.py
- packages/webui/src/components/ConversationWorkspace.tsx
- packages/core-adapter/tests/test_deterministic_backend.py
- packages/webui/src/e2e/app.spec.ts
- packages/core-adapter/src/heartwood/core_adapter/_facade.py
- packages/core-adapter/tests/test_session_service.py
- packages/webui/src/App.test.tsx
- packages/webui/src/components/ProjectWorkspace.tsx
- packages/schemas/src/heartwood/schemas/_api.py
- packages/gateway/tests/test_session_projection.py
- packages/compliance/tests/test_coding_agent_qualification.py
- packages/cli/src/heartwood/cli/_tui.py
- packages/gateway/src/heartwood/gateway/_openhands_sdk.py
- packages/notebook/tests/test_notebook.py
- packages/cli/src/heartwood/cli/init.py
- packages/core-adapter/src/heartwood/core_adapter/_service.py
- packages/cli/src/heartwood/cli/_interactive.py
- packages/gateway/tests/test_gateway_contract.py
- packages/gateway/src/heartwood/gateway/_session_projection.py
- packages/gateway/src/heartwood/gateway/_workspace.py
♻️ Current Situation & Problem
Closes #26.
Agent actions and workspace changes lacked one correlated, interface-neutral projection. Terminal, browser, and notebook clients therefore had incomplete execution evidence and no shared bounded project inspection.
⚙️ Release Notes
📚 Documentation
Updated the terminal, browser, notebook, action and audit, architecture, testing, CLI, and troubleshooting guidance. Added current browser screenshots for Files and Changes.
✅ Testing
Code of Conduct & Contributing Guidelines
By creating and submitting this pull request, you agree to follow our Code of Conduct and Contributing Guidelines: