Repository navigation
[codex] Use runtime local tasks in Wework - #1502
Merged
Merged
Conversation
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughThis PR replaces local-Codex-specific flows with runtime-work APIs, executor RPC handling, and runtime task UI/state. It adds backend and executor runtime task orchestration, runtime IM continuation, runtime-focused Wework navigation and workbench behavior, and removes legacy conversation search, archived-chat UI, and local-Codex integration. ChangesRuntime local work migration
Sequence Diagram(s)sequenceDiagram
participant Wework as Wework UI
participant Backend as backend/app/api/endpoints/runtime_work.py
participant Service as backend/app/services/runtime_work_service.py
participant RPC as backend/app/services/device/runtime_rpc_service.py
participant Executor as executor/runtime_work/rpc_handler.py
Wework->>Backend: POST /api/runtime-work/send
Backend->>Service: RuntimeSendRequest
Service->>RPC: runtime.tasks.send
RPC->>Executor: runtime:rpc
Executor-->>RPC: accepted / error / messages
RPC-->>Service: runtime response
Service-->>Backend: RuntimeSendResponse
Backend-->>Wework: JSON response
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
qdaxb
marked this pull request as ready for review
June 20, 2026 20:16
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 9
Note
Due to the large number of review comments, Critical, Major severity comments were prioritized as inline comments.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
wework/src/features/workbench/WorkbenchProvider.tsx (1)
830-2735: 🛠️ Refactor suggestion | 🟠 Major | 🏗️ Heavy liftExtract runtime-work orchestration out of this oversized provider.
The new runtime polling/open/send flow keeps growing a 2,700+ line file and a provider function far beyond the project limits; move runtime-work state/effects/actions into focused hooks/modules before this becomes harder to test safely.
As per coding guidelines, “File size MUST NOT exceed 1000 lines” and “Function length MUST NOT exceed 50 lines (preferred maximum).”
🤖 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 `@wework/src/features/workbench/WorkbenchProvider.tsx` around lines 830 - 2735, The WorkbenchProvider component exceeds the 1000-line file limit and violates function length guidelines. Extract all runtime-work related state management, effects, and callbacks into separate custom hooks or modules. Specifically, isolate the runtime-work logic including state.currentRuntimeTask, openRuntimeLocalTask, refreshRuntimeTranscript, sendPreparedRuntimeMessage, the runtime polling effect, and all related handlers that reference resolvedServices.runtimeWorkApi. Create focused hooks like useRuntimeWorkOrchestration or useRuntimeTaskManagement that encapsulate these concerns and return the necessary state and callbacks to be consumed by WorkbenchProvider, then update WorkbenchProvider to use these new hooks instead of managing the runtime logic directly.Source: Coding guidelines
wework/src/components/layout/DesktopSidebar.tsx (1)
238-324: 🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick winExtract duplicated runtime task helpers into a shared utility.
getRuntimeTaskTime,sortRuntimeTasks,getRuntimeTaskRuntimeLabel, andisRuntimeTaskSelectedare duplicated withwework/src/components/layout/MobileDrawer.tsx, which is already increasing drift risk.As per coding guidelines, “Extract common logic from ALL code immediately when changes are made and logic should be shared” and “never copy-paste code.”
🤖 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 `@wework/src/components/layout/DesktopSidebar.tsx` around lines 238 - 324, The utility functions getRuntimeTaskTime, sortRuntimeTasks, getRuntimeTaskRuntimeLabel, and isRuntimeTaskSelected are duplicated between DesktopSidebar.tsx and MobileDrawer.tsx, creating maintenance drift. Extract these four functions into a new shared utility file (such as runtimeTaskHelpers.ts or similar) in a common utilities directory, then import and use them in both DesktopSidebar.tsx and MobileDrawer.tsx instead of keeping duplicate copies. Ensure all type dependencies required by these functions are also exported from the shared utility.Source: Coding guidelines
wework/src/pages/PluginManagementPage.tsx (1)
130-144:⚠️ Potential issue | 🟠 Major | ⚡ Quick winPass
devicesintoMobileDrawerto keep mobile behavior parity.The mobile drawer here is missing
devices={state.devices}. That makes device-dependent drawer actions (including project-creation readiness) behave as if no devices exist.Suggested fix
<MobileDrawer open={drawerOpen} user={state.user} + devices={state.devices} projects={state.projects} runtimeWork={state.runtimeWork}🤖 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 `@wework/src/pages/PluginManagementPage.tsx` around lines 130 - 144, The MobileDrawer component is missing the devices prop which causes device-dependent drawer actions and project-creation readiness to fail on mobile. Add the devices prop to the MobileDrawer component by passing state.devices to it, ensuring parity with how the drawer is used elsewhere and that all device-dependent functionality works correctly.
🟡 Minor comments (17)
wework/src/types/api.ts-327-330 (1)
327-330:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winInclude the optional runtime message source in the send contract.
The backend forwards
request.sourcewhen present, but this TS type prevents callers from preserving IM/manual attribution on runtime sends.Proposed contract fix
export interface RuntimeSendRequest { address: RuntimeTaskAddress message: string + source?: RuntimeMessageSource | null }🤖 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 `@wework/src/types/api.ts` around lines 327 - 330, The RuntimeSendRequest interface is missing an optional source field that the backend supports when forwarding requests. Add an optional source property to the RuntimeSendRequest interface to allow callers to preserve IM/manual attribution information on runtime sends, ensuring the TypeScript type definition matches the backend contract.wework/src/features/workbench/workbenchReducer.ts-66-70 (1)
66-70:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winMake runtime-task project assignment explicit to avoid stale project context.
runtime_task_openedkeeps the previouscurrentProjectwhenprojectisundefined. The runtime open path can dispatchundefinedfor unmapped workspaces, so opening such a task can leave an unrelated project selected.Suggested fix
| { type: 'runtime_task_opened' address: RuntimeTaskAddress - project?: ProjectWithTasks | null + project: ProjectWithTasks | null } ... case 'runtime_task_opened': return { ...state, - currentProject: action.project === undefined ? state.currentProject : action.project, + currentProject: action.project, currentTask: null, currentRuntimeTask: action.address, }Also update dispatch sites to pass
null(notundefined) when no project mapping exists.Also applies to: 282-287
🤖 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 `@wework/src/features/workbench/workbenchReducer.ts` around lines 66 - 70, The runtime_task_opened action type allows undefined for the project field, which causes the reducer to retain the previous currentProject value when no project mapping exists for unmapped workspaces, leaving stale project context. Update all dispatch sites that send the runtime_task_opened action (including those around lines 282-287) to explicitly pass null instead of undefined for the project field when no project mapping exists. This will ensure the reducer can distinguish between "no change needed" (undefined) and "clear the project" (null), preventing unrelated projects from remaining selected.wework/src/components/layout/DesktopSidebar.tsx-402-403 (1)
402-403:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winRuntime task row selector is not unique enough.
The DOM id/query key is based only on
localTaskId, but task addressing uses{ deviceId, workspacePath, localTaskId }. If two workspaces reuse the same local id, scroll targeting can jump to the wrong row.Also applies to: 775-777
🤖 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 `@wework/src/components/layout/DesktopSidebar.tsx` around lines 402 - 403, The data-testid attribute for the runtime task row uses only localTaskId which is insufficient for uniqueness since task addressing requires the combination of deviceId, workspacePath, and localTaskId. Update the data-testid value to include all three identifiers (for example, combining them with hyphens or another delimiter) to ensure the selector is globally unique across all workspaces and devices. Apply this same fix to all occurrences of the runtime-local-task-row selector, including the ones mentioned at lines 775-777.wework/src/components/layout/MobileDrawer.tsx-95-98 (1)
95-98:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winGuard runtime workspace label against missing device name.
RuntimeDeviceWorkspace.deviceNameis optional, so current formatting can renderundefined · <path>. Add a fallback todeviceId(or localized unknown label).Suggested fix
function getRuntimeWorkspaceLabel(workspace: RuntimeDeviceWorkspace) { const pathLabel = workspace.label || workspace.workspacePath - return `${workspace.deviceName} · ${pathLabel}` + const deviceLabel = workspace.deviceName || workspace.deviceId + return `${deviceLabel} · ${pathLabel}` }🤖 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 `@wework/src/components/layout/MobileDrawer.tsx` around lines 95 - 98, The getRuntimeWorkspaceLabel function uses the optional workspace.deviceName property directly in the string template, which can result in "undefined · <path>" being rendered if deviceName is not provided. Modify the function to add a fallback value for the device name portion of the label, using workspace.deviceId as a fallback or a localized string representing an unknown device name when deviceName is missing or falsy.executor/runtime_work/local_task_store.py-51-55 (1)
51-55:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winAdd return type annotation on
__init__.
LocalTaskStore.__init__should be annotated with-> None.As per coding guidelines, Python files must use type hints for all functions and classes.
🤖 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 `@executor/runtime_work/local_task_store.py` around lines 51 - 55, The `LocalTaskStore.__init__` method is missing a return type annotation. Add `-> None` to the method signature after the closing parenthesis of the parameters and before the colon to comply with the coding guidelines requiring type hints for all functions and classes.Source: Coding guidelines
executor/runtime_work/rpc_handler.py-23-28 (1)
23-28:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winAdd return type annotation on
__init__.
RuntimeWorkRpcHandler.__init__is missing-> None.As per coding guidelines, Python files must use type hints for all functions and classes.
🤖 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 `@executor/runtime_work/rpc_handler.py` around lines 23 - 28, The `__init__` method of the `RuntimeWorkRpcHandler` class is missing a return type annotation. Add `-> None` after the closing parenthesis of the method signature to indicate that this constructor returns nothing, following the required Python type hinting conventions.Source: Coding guidelines
executor/runtime_work/codex_discovery.py-26-30 (1)
26-30:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winAdd an explicit return annotation on
__init__.
CodexSessionDiscovery.__init__is missing-> None.As per coding guidelines, Python files must use type hints for all functions and classes.
🤖 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 `@executor/runtime_work/codex_discovery.py` around lines 26 - 30, The __init__ method of CodexSessionDiscovery is missing an explicit return type annotation as required by the coding guidelines. Add -> None between the closing parenthesis of the parameter list and the colon in the __init__ method signature to specify that the method returns None.Source: Coding guidelines
executor/tests/runtime_work/test_local_task_store.py-402-403 (1)
402-403:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winReplace
asyncio.sleep(0)synchronization with deterministic completion checks.Using event-loop yields as readiness gates makes these tests timing-sensitive; they can intermittently fail under slower scheduling.
Also applies to: 473-486
🤖 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 `@executor/tests/runtime_work/test_local_task_store.py` around lines 402 - 403, Replace all instances of `asyncio.sleep(0)` with deterministic completion checks that verify the intended asynchronous operation has actually finished. Instead of relying on event-loop yields as synchronization points, wait for specific conditions, events, or state changes that indicate the operation is complete (such as checking for task completion, waiting on events, or verifying expected state transitions). This applies to both occurrences in the test file and will eliminate the timing-sensitive behavior that causes intermittent test failures.executor/runtime_work/local_task_store.py-103-107 (1)
103-107:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winSort by parsed datetimes, not raw timestamp strings.
Current ordering is lexical and can misorder mixed ISO formats/offsets.
Suggested fix
return sorted( filtered, - key=lambda record: (record.updated_at, record.created_at), + key=lambda record: ( + datetime.fromisoformat(record.updated_at.replace("Z", "+00:00")), + datetime.fromisoformat(record.created_at.replace("Z", "+00:00")), + ), reverse=True, )🤖 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 `@executor/runtime_work/local_task_store.py` around lines 103 - 107, The sorted function at line 103-107 is sorting by raw timestamp strings (record.updated_at and record.created_at) which causes incorrect lexical ordering, especially with mixed ISO formats or timezone offsets. Parse these timestamp strings into actual datetime objects before using them as sort keys in the lambda function. Modify the key lambda to convert record.updated_at and record.created_at to datetime objects (e.g., using dateutil.parser or datetime.fromisoformat) so that the sorting is performed on parsed datetime values rather than string comparisons.backend/app/services/runtime_work_service.py-967-968 (1)
967-968:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winUse UTC timestamp for consistency with database server time.
datetime.now()returns a naive datetime in the local timezone, which is inconsistent withfunc.now()used forcreated_at/updated_at(database server time, typically UTC). This could cause ordering issues if the application server timezone differs from the database.🔧 Suggested fix
-from datetime import datetime +from datetime import datetime, timezone- row.last_seen_at = datetime.now() + row.last_seen_at = datetime.now(timezone.utc)🤖 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 `@backend/app/services/runtime_work_service.py` around lines 967 - 968, Replace the `datetime.now()` call in the `row.last_seen_at` assignment with a UTC-aware datetime to ensure consistency with how other timestamp fields like `created_at` and `updated_at` are set using `func.now()` (database server time in UTC). Use either `datetime.utcnow()` or `datetime.now(timezone.utc)` depending on your Python version to ensure the timestamp is stored in UTC and avoids timezone-related ordering issues.executor/tests/runtime_work/test_runtime_work_runner_registration.py-6-35 (1)
6-35:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winAdd type hints to the new test and helper callables.
The new test function and helper callable definitions are untyped, which breaks the repository’s Python typing requirement.
As per coding guidelines,
**/*.py: Python files MUST use type hints for all functions and classes.🤖 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 `@executor/tests/runtime_work/test_runtime_work_runner_registration.py` around lines 6 - 35, The test function test_local_runner_registers_runtime_rpc_handler and its helper callables lack type hints, violating the repository's typing requirement. Add a return type hint of None to the test function, add type hints to the Handler class including the __getattr__ method (returning Callable) and the handle inner function, and add appropriate type hints to all lambda functions used throughout the test including the ones assigned to websocket_client, runtime_work_handler, and _register_extension_handlers to ensure full type compliance.Source: Coding guidelines
backend/tests/schemas/test_user_schema.py-11-11 (1)
11-11:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winAnnotate the test function return type.
test_user_preferences_accept_runtime_model_selectionshould declare-> Noneto satisfy the project’s Python typing requirement.Suggested patch
-def test_user_preferences_accept_runtime_model_selection(): +def test_user_preferences_accept_runtime_model_selection() -> None:As per coding guidelines,
**/*.py: Python files MUST use type hints for all functions and classes.🤖 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 `@backend/tests/schemas/test_user_schema.py` at line 11, The test function `test_user_preferences_accept_runtime_model_selection` is missing a return type annotation. Add `-> None` to the function signature after the closing parenthesis to comply with the project's Python typing requirements that mandate type hints for all functions and classes.Source: Coding guidelines
backend/tests/api/test_user_preferences_api.py-34-38 (1)
34-38:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winAdd an explicit return type to the new test function.
test_read_current_user_accepts_runtime_model_selectionis missing-> None, which violates the repository typing rule for Python functions.Suggested patch
`@pytest.mark.api` def test_read_current_user_accepts_runtime_model_selection( user_preferences_client: TestClient, test_db: Session, test_user: User, -): +) -> None:As per coding guidelines,
**/*.py: Python files MUST use type hints for all functions and classes.🤖 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 `@backend/tests/api/test_user_preferences_api.py` around lines 34 - 38, The function test_read_current_user_accepts_runtime_model_selection is missing an explicit return type annotation. Add `-> None` after the closing parenthesis of the function parameters to comply with the repository's type hints requirement for all Python functions. This should be placed before the colon that precedes the function body.Source: Coding guidelines
backend/tests/services/test_runtime_rpc_service.py-11-11 (1)
11-11:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winAdd type hints to the async test signature.
Please annotate fixture params and add
-> Nonefor this test to satisfy repository Python typing requirements.As per coding guidelines,
Python files MUST use type hints for all functions and classes.🤖 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 `@backend/tests/services/test_runtime_rpc_service.py` at line 11, The test function test_runtime_rpc_service_returns_runtime_failure_ack is missing type hints required by the repository's Python typing standards. Add a type annotation for the monkeypatch parameter (which is a pytest fixture of type MonkeyPatch) and add the return type annotation -> None to the function signature to comply with the typing requirements.Source: Coding guidelines
executor/runtime_work/agent_adapter.py-33-37 (1)
33-37:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winAdd explicit
-> Noneannotations on constructor methods.The new class constructors are missing explicit return annotations.
As per coding guidelines,
Python files MUST use type hints for all functions and classes.Also applies to: 110-123
🤖 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 `@executor/runtime_work/agent_adapter.py` around lines 33 - 37, The __init__ constructor method in the AgentAdapter class is missing an explicit return type annotation. Add `-> None` after the parameter list in the __init__ method signature to comply with type hinting requirements. Apply the same fix to any other constructor methods mentioned at lines 110-123 that also lack the explicit return type annotation.Source: Coding guidelines
backend/tests/api/endpoints/test_runtime_work_api.py-8-14 (1)
8-14:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winAdd explicit type hints to test function signatures.
These new Python test functions currently omit required type annotations. Add parameter and
-> Noneannotations for each test/helper function.As per coding guidelines,
Python files MUST use type hints for all functions and classes.Also applies to: 38-43, 80-85
🤖 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 `@backend/tests/api/endpoints/test_runtime_work_api.py` around lines 8 - 14, The test functions in this file are missing required type annotations for their parameters and return types. Add explicit type hints to the function signatures for _auth_headers (which already has proper annotations for reference), test_list_runtime_work_endpoint_uses_current_user, and the other test functions mentioned in the also applies to sections. For each test function, add type annotations for all parameters (such as test_client, test_token, monkeypatch, etc.) and append -> None as the return type annotation to indicate these functions return nothing.Source: Coding guidelines
executor/runtime_work/agent_adapter.py-209-224 (1)
209-224:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winHandle
asyncio.CancelledErrorto keep task state consistent.If the background coroutine is cancelled,
_mark_not_running()is skipped becauseCancelledErroris not caught byexcept Exception. This can leave tasks stuck asrunning=True.💡 Proposed fix
@@ try: await self.execute_agent(request, emitter) self._mark_not_running(local_task_id) + except asyncio.CancelledError: + self._mark_not_running(local_task_id) + raise except Exception as exc: logger.exception("Runtime agent execution failed: %s", exc) await emitter.error(str(exc), "execution_error") self._mark_not_running(local_task_id)🤖 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 `@executor/runtime_work/agent_adapter.py` around lines 209 - 224, The `_run_agent` method does not handle `asyncio.CancelledError` separately, which means if the background coroutine is cancelled, the `except Exception` block is skipped since `CancelledError` inherits from `BaseException` rather than `Exception`. This leaves the task stuck in a running state. Add a separate exception handler for `asyncio.CancelledError` before or after the general exception handler that also calls `_mark_not_running(local_task_id)` to ensure the task state is properly cleaned up when the coroutine is cancelled, while maintaining the same error emission behavior for regular exceptions.
🧹 Nitpick comments (9)
wework/src/api/projects.ts (1)
22-84: ⚡ Quick winSplit
createProjectApiinto smaller builders to satisfy the function-length rule
createProjectApicurrently spans beyond the 50-line limit. Please extract related method groups (e.g., project CRUD vs session starters) into small helpers and compose the return object.As per coding guidelines, "Function length MUST NOT exceed 50 lines (preferred maximum)."
Refactor sketch
export function createProjectApi(client: HttpClient) { + const projectCrud = createProjectCrudApi(client) + const sessionApi = createProjectSessionApi(client) + return { - listProjects(): Promise<ProjectListResponse> { - return client.get(withClientOrigin('/projects')) - }, - ... - startCodeServerSession( - projectId: number, - options?: { taskId?: number } - ): Promise<ProjectDeviceSessionResponse> { - return startProjectSession(withClientOrigin(`/projects/${projectId}/code-server`), options) - }, + ...projectCrud, + ...sessionApi, } }🤖 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 `@wework/src/api/projects.ts` around lines 22 - 84, The createProjectApi function exceeds the 50-line limit. Extract the methods into logical groupings: create separate builder functions for project CRUD operations (listProjects, getProject, createProject, createGitWorkspaceProject, updateProject, deleteProject), worktree operations (listWorktrees, deleteWorktree), and session operations (startTerminalSession, startCodeServerSession). Each builder function should accept the client parameter and the helper functions (projectSessionPayload, startProjectSession), return an object with its related methods, and then compose all builders together in the main createProjectApi return statement by spreading or merging their results.Source: Coding guidelines
executor/tests/runtime_work/test_local_task_store.py (2)
345-422: 🏗️ Heavy liftSplit oversized test functions into fixtures/helpers.
These tests exceed the 50-line function limit and are doing setup, execution, and assertions for multiple behaviors in a single body. Extracting builders/fixtures will keep each test focused and easier to maintain.
As per coding guidelines, function length must not exceed 50 lines.
Also applies to: 425-509
🤖 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 `@executor/tests/runtime_work/test_local_task_store.py` around lines 345 - 422, The test function test_runtime_work_handler_creates_runtime_task_with_local_transcript exceeds the 50-line function limit by combining setup, execution, and assertions for multiple behaviors. Refactor this test by extracting the EmptyDiscovery class and the execute_agent function into reusable fixtures or helper functions, extracting the RuntimeWorkRpcHandler initialization into a fixture that creates the handler with all required dependencies, and creating a builder or fixture for the complex RPC request payloads to simplify the test body. This will reduce the function length to under 50 lines while keeping setup logic separate and reusable across other tests that apply to the same comment.Source: Coding guidelines
11-562: 🏗️ Heavy liftAdd type hints to test and helper function signatures.
Multiple functions in this file are unannotated (
tmp_path, helper methods, async test defs).As per coding guidelines, Python files must use type hints for all functions and classes.
🤖 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 `@executor/tests/runtime_work/test_local_task_store.py` around lines 11 - 562, Add type hints to all function signatures and method definitions in this test file. For test functions taking `tmp_path` parameter, annotate it as `pathlib.Path`. For all test functions and async test functions, add return type annotations (typically `-> None`). Also add type hints to helper classes and their methods, such as `EmptyDiscovery.discover()`, and to internal async functions like `execute_agent(request, emitter)` to ensure full type coverage throughout the file per coding guidelines.Source: Coding guidelines
executor/runtime_work/codex_discovery.py (1)
294-370: 🏗️ Heavy liftSplit
_read_session_transcriptinto smaller handlers.This parser is doing event decode, state transitions, and message shaping in one large block, which also exceeds the function-length rule. Please extract per-event handlers (
user_message,agent_message,task_complete,turn_aborted) and keep the loop focused on dispatch.As per coding guidelines, function length must not exceed 50 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 `@executor/runtime_work/codex_discovery.py` around lines 294 - 370, The function _read_session_transcript exceeds the 50-line limit by handling multiple event type cases (user_message, agent_message, task_complete, turn_aborted) with full message construction logic inline. Extract separate handler functions for each event type (one for user_message, one for agent_message, one for task_complete, one for turn_aborted), each returning the constructed message or messages, and refactor the main loop in _read_session_transcript to dispatch to these handlers based on the event_type value. This keeps the main function focused on file reading and dispatch while reducing its length to comply with the coding guidelines.Source: Coding guidelines
executor/runtime_work/local_task_store.py (1)
48-49: ⚡ Quick winMark mutable lock caches as class variables explicitly.
Annotating these as
ClassVarclarifies shared state intent and avoids mutable-class-attribute lint churn.Suggested fix
-from typing import Any, Callable, Optional +from typing import Any, Callable, ClassVar, Optional @@ - _locks: dict[Path, threading.RLock] = {} - _locks_guard = threading.Lock() + _locks: ClassVar[dict[Path, threading.RLock]] = {} + _locks_guard: ClassVar[threading.Lock] = threading.Lock()🤖 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 `@executor/runtime_work/local_task_store.py` around lines 48 - 49, Mark the mutable class-level attributes _locks and _locks_guard with explicit ClassVar type annotations to clarify their shared state intent and avoid lint warnings. Import ClassVar from the typing module, then annotate _locks as ClassVar[dict[Path, threading.RLock]] and _locks_guard as ClassVar[threading.Lock] to make it explicit that these are class variables rather than instance variables.backend/app/services/runtime_work_service.py (1)
239-239: 💤 Low valueConsider using
raise ... from excfor exception chaining.The HTTPException raises within except blocks lose the original exception context. While the error message is preserved in the
detail, usingfrom excpreserves the full traceback for debugging.♻️ Suggested improvement
except RuntimeRpcError as exc: - raise HTTPException(status_code=status.HTTP_502_BAD_GATEWAY, detail=str(exc)) + raise HTTPException(status_code=status.HTTP_502_BAD_GATEWAY, detail=str(exc)) from excApply similar changes at lines 270 and 306.
Also applies to: 270-270, 306-306
🤖 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 `@backend/app/services/runtime_work_service.py` at line 239, The HTTPException raises within the except blocks are not using exception chaining, which loses the original exception traceback for debugging purposes. In the except blocks at the three locations where HTTPException is raised with detail=str(exc) (around lines 239, 270, and 306), change each raise statement to use `raise ... from exc` syntax instead of just `raise`. This will preserve the full exception chain and traceback while still providing the error message in the HTTPException detail field.backend/tests/services/test_runtime_work_service.py (1)
383-457: 💤 Low valueTest relies on team_id=3 without creating the team fixture.
The test passes
teamId=3toRuntimeTaskCreateRequestbut mocks_build_runtime_execution_requestto bypass the actual team lookup. This is fine for this unit test, but consider adding an integration test that exercises the full_build_runtime_execution_requestpath including team validation.🤖 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 `@backend/tests/services/test_runtime_work_service.py` around lines 383 - 457, The test uses teamId=3 in the RuntimeTaskCreateRequest but mocks _build_runtime_execution_request to bypass actual team validation. Add a new integration test alongside this existing unit test that creates the necessary team fixture (with id=3), removes the mock for _build_runtime_execution_request to exercise the actual implementation, and verifies that team validation and the full runtime task creation flow work correctly with real data rather than mocked behavior. This ensures the team lookup logic is properly tested end-to-end.executor/tests/runtime_work/test_runtime_work_runner_registration.py (1)
39-39: ⚡ Quick winAssert the RPC handler binding, not only event presence.
The current check only verifies the event key exists. Add an assertion that
RuntimeEvents.RPCis wired torunner.runtime_work_handler.handle_runtime_rpcso the test fails on wrong callback registration.🤖 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 `@executor/tests/runtime_work/test_runtime_work_runner_registration.py` at line 39, The assertion on RuntimeEvents.RPC only checks that the event key exists in the registered dictionary, but does not verify that it is bound to the correct handler. Replace the current assertion with one that checks the value at registered[RuntimeEvents.RPC] equals runner.runtime_work_handler.handle_runtime_rpc to ensure the proper callback function is registered for the RPC event.backend/tests/api/endpoints/test_runtime_work_api.py (1)
12-36: ⚡ Quick winAssert authenticated user context is forwarded to service calls.
These tests validate response and payload mapping, but they don’t consistently assert that endpoint handlers pass the current user identity (
user_id) into service-layer calls. Adding that assertion will better lock down auth-scoped behavior for runtime-work APIs.Also applies to: 38-77, 80-112
🤖 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 `@backend/tests/api/endpoints/test_runtime_work_api.py` around lines 12 - 36, The test test_list_runtime_work_endpoint_uses_current_user validates that client_origin is passed to the service layer, but it does not assert that the current authenticated user's user_id is also forwarded to the service call. Add an assertion after the existing assertion for client_origin to verify that service_mock.await_args.kwargs contains the user_id parameter extracted from the authenticated test_token context, ensuring that auth-scoped behavior is properly enforced at the service layer.
🤖 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 `@docs/superpowers/specs/2026-06-20-runtime-session-workitems-design.md`:
- Around line 124-128: The LocalTask storage description in the markdown file is
documenting outdated SQLite-based storage which conflicts with the JSON index
storage model introduced in this PR. Update the storage path reference to
reflect the actual runtime-work contract by replacing the SQLite path reference
with the JSON index path that uses the environment variable WEGENT_EXECUTOR_HOME
instead of the home directory shorthand, ensuring the documentation accurately
describes the current storage implementation.
In `@executor/runtime_work/agent_adapter.py`:
- Around line 124-127: The create() method and _execution_request() method are
currently accepting missing or blank workspacePath values and normalizing them
silently, which causes ambiguous workspace identity. Add validation logic at the
beginning of both methods to explicitly check if the workspacePath from the
payload is missing or blank, and raise an appropriate error (such as ValueError
or similar) before calling normalize_workspace_path(). This ensures invalid
workspace paths are rejected early rather than silently normalized.
In `@executor/runtime_work/local_task_store.py`:
- Around line 147-163: In the update_task method, enforce that the local_task_id
primary key cannot be changed by the updater. Before creating the normalized
LocalTaskRecord, capture the original local_task_id from the current object.
Then when constructing the normalized record, explicitly set local_task_id to
the original value from current instead of using updated.local_task_id,
preventing the updater from modifying the primary key. This ensures the task
record is updated at its original key in the index without leaving orphaned
entries.
In `@executor/runtime_work/rpc_handler.py`:
- Around line 149-160: The code accepts both "content" and "message" keys from
the payload but forwards the original unmodified payload to adapter.send(),
which expects a "message" key. If only "content" is provided, validation passes
but the adapter rejects it. After extracting the content value (where you check
isinstance(content, str)), normalize the payload by ensuring the "message" key
is set to this content value before passing it to adapter.send() in the await
statement. This ensures consistency between what the validation accepts and what
the adapter receives.
In `@wework/src/components/layout/DesktopSidebar.tsx`:
- Around line 489-511: The ProjectItem function is missing the onSelectProject
callback parameter which is needed to select the project when clicked. Add
onSelectProject as a parameter to the ProjectItem function signature
destructuring (similar to how onToggleProject, onRemoveProject, and
onRenameProject are defined), and then call onSelectProject(project.id) in
addition to onToggleProject when handling the project item click around line
530. This will ensure that clicking a project both selects it and toggles
expansion, maintaining consistent behavior between desktop and mobile
implementations.
In `@wework/src/features/workbench/WorkbenchProvider.tsx`:
- Around line 805-818: The findRuntimeWorkspaceForDevice function currently
returns the first matching workspace found using .find(), but when a device has
multiple available runtime workspaces, this can incorrectly route standalone
sends to the wrong repository. Modify the function to return a workspace only
when the choice is unambiguous: instead of using .find() and returning the first
match, filter the workspaces array to find all matching items and only return
the workspacePath if exactly one workspace matches the criteria (deviceId,
available, and has a workspacePath); otherwise return null to require explicit
workspace selection.
- Around line 1075-1094: In the refreshWorkLists function, the
runtimeWorkApi?.listRuntimeWork() call has a catch handler that returns
EMPTY_RUNTIME_WORK on failure, which clears the sidebar even on transient
errors. Change the catch handler to return undefined instead of
EMPTY_RUNTIME_WORK so that the reducer receives undefined on runtime-work fetch
failures, allowing its existing preserve-previous snapshot logic to apply and
keep the last known state in the sidebar.
- Around line 1631-1668: The refreshRuntimeTranscript callback can complete with
stale data if the user switches tasks while a poll is in-flight. In the
useEffect where pollTranscript calls refreshRuntimeTranscript, add a check of
the cancelled flag after the refreshRuntimeTranscript promise settles but before
the dispatch happens. Specifically, after the void
refreshRuntimeTranscript(address) call in pollTranscript, verify that cancelled
is still false before allowing the dispatchMessages to occur, ensuring that only
the currently active task's transcript updates are processed.
In `@wework/src/types/api.ts`:
- Around line 354-360: The RuntimeTaskCreateResponse interface is missing the
deviceId field needed for the UI to construct a valid RuntimeTaskAddress after
task creation. Add a deviceId property to the RuntimeTaskCreateResponse
interface to return the resolved device identifier along with the other response
fields like accepted, localTaskId, workspacePath, runtime, and error. This will
prevent the UI from having to fall back to an empty device id.
---
Outside diff comments:
In `@wework/src/components/layout/DesktopSidebar.tsx`:
- Around line 238-324: The utility functions getRuntimeTaskTime,
sortRuntimeTasks, getRuntimeTaskRuntimeLabel, and isRuntimeTaskSelected are
duplicated between DesktopSidebar.tsx and MobileDrawer.tsx, creating maintenance
drift. Extract these four functions into a new shared utility file (such as
runtimeTaskHelpers.ts or similar) in a common utilities directory, then import
and use them in both DesktopSidebar.tsx and MobileDrawer.tsx instead of keeping
duplicate copies. Ensure all type dependencies required by these functions are
also exported from the shared utility.
In `@wework/src/features/workbench/WorkbenchProvider.tsx`:
- Around line 830-2735: The WorkbenchProvider component exceeds the 1000-line
file limit and violates function length guidelines. Extract all runtime-work
related state management, effects, and callbacks into separate custom hooks or
modules. Specifically, isolate the runtime-work logic including
state.currentRuntimeTask, openRuntimeLocalTask, refreshRuntimeTranscript,
sendPreparedRuntimeMessage, the runtime polling effect, and all related handlers
that reference resolvedServices.runtimeWorkApi. Create focused hooks like
useRuntimeWorkOrchestration or useRuntimeTaskManagement that encapsulate these
concerns and return the necessary state and callbacks to be consumed by
WorkbenchProvider, then update WorkbenchProvider to use these new hooks instead
of managing the runtime logic directly.
In `@wework/src/pages/PluginManagementPage.tsx`:
- Around line 130-144: The MobileDrawer component is missing the devices prop
which causes device-dependent drawer actions and project-creation readiness to
fail on mobile. Add the devices prop to the MobileDrawer component by passing
state.devices to it, ensuring parity with how the drawer is used elsewhere and
that all device-dependent functionality works correctly.
---
Minor comments:
In `@backend/app/services/runtime_work_service.py`:
- Around line 967-968: Replace the `datetime.now()` call in the
`row.last_seen_at` assignment with a UTC-aware datetime to ensure consistency
with how other timestamp fields like `created_at` and `updated_at` are set using
`func.now()` (database server time in UTC). Use either `datetime.utcnow()` or
`datetime.now(timezone.utc)` depending on your Python version to ensure the
timestamp is stored in UTC and avoids timezone-related ordering issues.
In `@backend/tests/api/endpoints/test_runtime_work_api.py`:
- Around line 8-14: The test functions in this file are missing required type
annotations for their parameters and return types. Add explicit type hints to
the function signatures for _auth_headers (which already has proper annotations
for reference), test_list_runtime_work_endpoint_uses_current_user, and the other
test functions mentioned in the also applies to sections. For each test
function, add type annotations for all parameters (such as test_client,
test_token, monkeypatch, etc.) and append -> None as the return type annotation
to indicate these functions return nothing.
In `@backend/tests/api/test_user_preferences_api.py`:
- Around line 34-38: The function
test_read_current_user_accepts_runtime_model_selection is missing an explicit
return type annotation. Add `-> None` after the closing parenthesis of the
function parameters to comply with the repository's type hints requirement for
all Python functions. This should be placed before the colon that precedes the
function body.
In `@backend/tests/schemas/test_user_schema.py`:
- Line 11: The test function
`test_user_preferences_accept_runtime_model_selection` is missing a return type
annotation. Add `-> None` to the function signature after the closing
parenthesis to comply with the project's Python typing requirements that mandate
type hints for all functions and classes.
In `@backend/tests/services/test_runtime_rpc_service.py`:
- Line 11: The test function
test_runtime_rpc_service_returns_runtime_failure_ack is missing type hints
required by the repository's Python typing standards. Add a type annotation for
the monkeypatch parameter (which is a pytest fixture of type MonkeyPatch) and
add the return type annotation -> None to the function signature to comply with
the typing requirements.
In `@executor/runtime_work/agent_adapter.py`:
- Around line 33-37: The __init__ constructor method in the AgentAdapter class
is missing an explicit return type annotation. Add `-> None` after the parameter
list in the __init__ method signature to comply with type hinting requirements.
Apply the same fix to any other constructor methods mentioned at lines 110-123
that also lack the explicit return type annotation.
- Around line 209-224: The `_run_agent` method does not handle
`asyncio.CancelledError` separately, which means if the background coroutine is
cancelled, the `except Exception` block is skipped since `CancelledError`
inherits from `BaseException` rather than `Exception`. This leaves the task
stuck in a running state. Add a separate exception handler for
`asyncio.CancelledError` before or after the general exception handler that also
calls `_mark_not_running(local_task_id)` to ensure the task state is properly
cleaned up when the coroutine is cancelled, while maintaining the same error
emission behavior for regular exceptions.
In `@executor/runtime_work/codex_discovery.py`:
- Around line 26-30: The __init__ method of CodexSessionDiscovery is missing an
explicit return type annotation as required by the coding guidelines. Add ->
None between the closing parenthesis of the parameter list and the colon in the
__init__ method signature to specify that the method returns None.
In `@executor/runtime_work/local_task_store.py`:
- Around line 51-55: The `LocalTaskStore.__init__` method is missing a return
type annotation. Add `-> None` to the method signature after the closing
parenthesis of the parameters and before the colon to comply with the coding
guidelines requiring type hints for all functions and classes.
- Around line 103-107: The sorted function at line 103-107 is sorting by raw
timestamp strings (record.updated_at and record.created_at) which causes
incorrect lexical ordering, especially with mixed ISO formats or timezone
offsets. Parse these timestamp strings into actual datetime objects before using
them as sort keys in the lambda function. Modify the key lambda to convert
record.updated_at and record.created_at to datetime objects (e.g., using
dateutil.parser or datetime.fromisoformat) so that the sorting is performed on
parsed datetime values rather than string comparisons.
In `@executor/runtime_work/rpc_handler.py`:
- Around line 23-28: The `__init__` method of the `RuntimeWorkRpcHandler` class
is missing a return type annotation. Add `-> None` after the closing parenthesis
of the method signature to indicate that this constructor returns nothing,
following the required Python type hinting conventions.
In `@executor/tests/runtime_work/test_local_task_store.py`:
- Around line 402-403: Replace all instances of `asyncio.sleep(0)` with
deterministic completion checks that verify the intended asynchronous operation
has actually finished. Instead of relying on event-loop yields as
synchronization points, wait for specific conditions, events, or state changes
that indicate the operation is complete (such as checking for task completion,
waiting on events, or verifying expected state transitions). This applies to
both occurrences in the test file and will eliminate the timing-sensitive
behavior that causes intermittent test failures.
In `@executor/tests/runtime_work/test_runtime_work_runner_registration.py`:
- Around line 6-35: The test function
test_local_runner_registers_runtime_rpc_handler and its helper callables lack
type hints, violating the repository's typing requirement. Add a return type
hint of None to the test function, add type hints to the Handler class including
the __getattr__ method (returning Callable) and the handle inner function, and
add appropriate type hints to all lambda functions used throughout the test
including the ones assigned to websocket_client, runtime_work_handler, and
_register_extension_handlers to ensure full type compliance.
In `@wework/src/components/layout/DesktopSidebar.tsx`:
- Around line 402-403: The data-testid attribute for the runtime task row uses
only localTaskId which is insufficient for uniqueness since task addressing
requires the combination of deviceId, workspacePath, and localTaskId. Update the
data-testid value to include all three identifiers (for example, combining them
with hyphens or another delimiter) to ensure the selector is globally unique
across all workspaces and devices. Apply this same fix to all occurrences of the
runtime-local-task-row selector, including the ones mentioned at lines 775-777.
In `@wework/src/components/layout/MobileDrawer.tsx`:
- Around line 95-98: The getRuntimeWorkspaceLabel function uses the optional
workspace.deviceName property directly in the string template, which can result
in "undefined · <path>" being rendered if deviceName is not provided. Modify the
function to add a fallback value for the device name portion of the label, using
workspace.deviceId as a fallback or a localized string representing an unknown
device name when deviceName is missing or falsy.
In `@wework/src/features/workbench/workbenchReducer.ts`:
- Around line 66-70: The runtime_task_opened action type allows undefined for
the project field, which causes the reducer to retain the previous
currentProject value when no project mapping exists for unmapped workspaces,
leaving stale project context. Update all dispatch sites that send the
runtime_task_opened action (including those around lines 282-287) to explicitly
pass null instead of undefined for the project field when no project mapping
exists. This will ensure the reducer can distinguish between "no change needed"
(undefined) and "clear the project" (null), preventing unrelated projects from
remaining selected.
In `@wework/src/types/api.ts`:
- Around line 327-330: The RuntimeSendRequest interface is missing an optional
source field that the backend supports when forwarding requests. Add an optional
source property to the RuntimeSendRequest interface to allow callers to preserve
IM/manual attribution information on runtime sends, ensuring the TypeScript type
definition matches the backend contract.
---
Nitpick comments:
In `@backend/app/services/runtime_work_service.py`:
- Line 239: The HTTPException raises within the except blocks are not using
exception chaining, which loses the original exception traceback for debugging
purposes. In the except blocks at the three locations where HTTPException is
raised with detail=str(exc) (around lines 239, 270, and 306), change each raise
statement to use `raise ... from exc` syntax instead of just `raise`. This will
preserve the full exception chain and traceback while still providing the error
message in the HTTPException detail field.
In `@backend/tests/api/endpoints/test_runtime_work_api.py`:
- Around line 12-36: The test test_list_runtime_work_endpoint_uses_current_user
validates that client_origin is passed to the service layer, but it does not
assert that the current authenticated user's user_id is also forwarded to the
service call. Add an assertion after the existing assertion for client_origin to
verify that service_mock.await_args.kwargs contains the user_id parameter
extracted from the authenticated test_token context, ensuring that auth-scoped
behavior is properly enforced at the service layer.
In `@backend/tests/services/test_runtime_work_service.py`:
- Around line 383-457: The test uses teamId=3 in the RuntimeTaskCreateRequest
but mocks _build_runtime_execution_request to bypass actual team validation. Add
a new integration test alongside this existing unit test that creates the
necessary team fixture (with id=3), removes the mock for
_build_runtime_execution_request to exercise the actual implementation, and
verifies that team validation and the full runtime task creation flow work
correctly with real data rather than mocked behavior. This ensures the team
lookup logic is properly tested end-to-end.
In `@executor/runtime_work/codex_discovery.py`:
- Around line 294-370: The function _read_session_transcript exceeds the 50-line
limit by handling multiple event type cases (user_message, agent_message,
task_complete, turn_aborted) with full message construction logic inline.
Extract separate handler functions for each event type (one for user_message,
one for agent_message, one for task_complete, one for turn_aborted), each
returning the constructed message or messages, and refactor the main loop in
_read_session_transcript to dispatch to these handlers based on the event_type
value. This keeps the main function focused on file reading and dispatch while
reducing its length to comply with the coding guidelines.
In `@executor/runtime_work/local_task_store.py`:
- Around line 48-49: Mark the mutable class-level attributes _locks and
_locks_guard with explicit ClassVar type annotations to clarify their shared
state intent and avoid lint warnings. Import ClassVar from the typing module,
then annotate _locks as ClassVar[dict[Path, threading.RLock]] and _locks_guard
as ClassVar[threading.Lock] to make it explicit that these are class variables
rather than instance variables.
In `@executor/tests/runtime_work/test_local_task_store.py`:
- Around line 345-422: The test function
test_runtime_work_handler_creates_runtime_task_with_local_transcript exceeds the
50-line function limit by combining setup, execution, and assertions for
multiple behaviors. Refactor this test by extracting the EmptyDiscovery class
and the execute_agent function into reusable fixtures or helper functions,
extracting the RuntimeWorkRpcHandler initialization into a fixture that creates
the handler with all required dependencies, and creating a builder or fixture
for the complex RPC request payloads to simplify the test body. This will reduce
the function length to under 50 lines while keeping setup logic separate and
reusable across other tests that apply to the same comment.
- Around line 11-562: Add type hints to all function signatures and method
definitions in this test file. For test functions taking `tmp_path` parameter,
annotate it as `pathlib.Path`. For all test functions and async test functions,
add return type annotations (typically `-> None`). Also add type hints to helper
classes and their methods, such as `EmptyDiscovery.discover()`, and to internal
async functions like `execute_agent(request, emitter)` to ensure full type
coverage throughout the file per coding guidelines.
In `@executor/tests/runtime_work/test_runtime_work_runner_registration.py`:
- Line 39: The assertion on RuntimeEvents.RPC only checks that the event key
exists in the registered dictionary, but does not verify that it is bound to the
correct handler. Replace the current assertion with one that checks the value at
registered[RuntimeEvents.RPC] equals
runner.runtime_work_handler.handle_runtime_rpc to ensure the proper callback
function is registered for the RPC event.
In `@wework/src/api/projects.ts`:
- Around line 22-84: The createProjectApi function exceeds the 50-line limit.
Extract the methods into logical groupings: create separate builder functions
for project CRUD operations (listProjects, getProject, createProject,
createGitWorkspaceProject, updateProject, deleteProject), worktree operations
(listWorktrees, deleteWorktree), and session operations (startTerminalSession,
startCodeServerSession). Each builder function should accept the client
parameter and the helper functions (projectSessionPayload, startProjectSession),
return an object with its related methods, and then compose all builders
together in the main createProjectApi return statement by spreading or merging
their results.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
wework/src/features/workbench/workbenchReducer.test.ts (1)
276-278: ⚡ Quick winAssert that DB task context is cleared when opening runtime task.
This test should also verify
currentTaskis null to lock in the reducer’s mutual exclusivity betweencurrentTaskandcurrentRuntimeTask.Suggested assertion
expect(opened.currentProject).toBeNull() + expect(opened.currentTask).toBeNull() expect(opened.currentRuntimeTask?.localTaskId).toBe('runtime-1')🤖 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 `@wework/src/features/workbench/workbenchReducer.test.ts` around lines 276 - 278, The test currently verifies that currentProject is null and currentRuntimeTask is set when opening a runtime task, but it is missing an assertion to verify mutual exclusivity between currentTask and currentRuntimeTask. Add an expect statement to assert that opened.currentTask is null alongside the existing expectations for currentProject and currentRuntimeTask to ensure the reducer properly clears the regular task when a runtime task is opened.wework/src/components/layout/DesktopWorkbenchLayout.test.tsx (1)
2009-2016: ⚡ Quick winStrengthen callback argument assertions for both toggle clicks.
You already verify the first call payload and total calls, but this can still pass if the second call uses a wrong project id. Assert both calls explicitly.
Suggested test tightening
- expect(baseProps.onSelectProject).toHaveBeenCalledWith(1) + expect(baseProps.onSelectProject).toHaveBeenNthCalledWith(1, 1) @@ - expect(baseProps.onSelectProject).toHaveBeenCalledTimes(2) + expect(baseProps.onSelectProject).toHaveBeenNthCalledWith(2, 1) + expect(baseProps.onSelectProject).toHaveBeenCalledTimes(2)🤖 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 `@wework/src/components/layout/DesktopWorkbenchLayout.test.tsx` around lines 2009 - 2016, The test verifies the first call to baseProps.onSelectProject with the payload 1 and checks the total call count is 2, but it does not verify what arguments the second call received. Add an assertion after the userEvent.click on project-item-button to explicitly check the arguments passed to the second call of baseProps.onSelectProject (using a method that checks the Nth call parameters) to ensure the callback was invoked with the correct project id on the second click, making the test more robust against incorrect payload values.
🤖 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 `@wework/src/components/layout/runtimeTaskSidebarHelpers.ts`:
- Around line 9-11: The sort comparator in the file is vulnerable to NaN
comparisons when timestamps are malformed. In the comparator function, after
calling getTime() on the leftTime and rightTime variables, add guards to check
if the result is NaN and normalize it to 0. This ensures that invalid timestamps
(which result in NaN from getTime()) are treated as epoch time zero before
performing the subtraction in the return statement, preventing unpredictable
sort behavior caused by NaN arithmetic.
---
Nitpick comments:
In `@wework/src/components/layout/DesktopWorkbenchLayout.test.tsx`:
- Around line 2009-2016: The test verifies the first call to
baseProps.onSelectProject with the payload 1 and checks the total call count is
2, but it does not verify what arguments the second call received. Add an
assertion after the userEvent.click on project-item-button to explicitly check
the arguments passed to the second call of baseProps.onSelectProject (using a
method that checks the Nth call parameters) to ensure the callback was invoked
with the correct project id on the second click, making the test more robust
against incorrect payload values.
In `@wework/src/features/workbench/workbenchReducer.test.ts`:
- Around line 276-278: The test currently verifies that currentProject is null
and currentRuntimeTask is set when opening a runtime task, but it is missing an
assertion to verify mutual exclusivity between currentTask and
currentRuntimeTask. Add an expect statement to assert that opened.currentTask is
null alongside the existing expectations for currentProject and
currentRuntimeTask to ensure the reducer properly clears the regular task when a
runtime task is opened.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: 7f31bbdb-bd4d-44ed-a368-fc81ddd3539f
📒 Files selected for processing (20)
backend/app/schemas/runtime_work.pybackend/app/services/runtime_work_service.pybackend/tests/services/test_runtime_work_service.pydocs/superpowers/specs/2026-06-20-runtime-session-workitems-design.mdexecutor/runtime_work/agent_adapter.pyexecutor/runtime_work/local_task_store.pyexecutor/runtime_work/rpc_handler.pyexecutor/tests/runtime_work/test_local_task_store.pywework/src/components/layout/DesktopSidebar.test.tsxwework/src/components/layout/DesktopSidebar.tsxwework/src/components/layout/DesktopWorkbenchLayout.test.tsxwework/src/components/layout/MobileDrawer.tsxwework/src/components/layout/runtimeTaskSidebarHelpers.tswework/src/features/workbench/WorkbenchProvider.test.tsxwework/src/features/workbench/WorkbenchProvider.tsxwework/src/features/workbench/workbenchReducer.test.tswework/src/features/workbench/workbenchReducer.tswework/src/pages/PluginManagementPage.tsxwework/src/pages/PluginsPage.tsxwework/src/types/api.ts
✅ Files skipped from review due to trivial changes (1)
- docs/superpowers/specs/2026-06-20-runtime-session-workitems-design.md
🚧 Files skipped from review as they are similar to previous changes (15)
- wework/src/types/api.ts
- backend/tests/services/test_runtime_work_service.py
- backend/app/schemas/runtime_work.py
- wework/src/components/layout/DesktopSidebar.test.tsx
- wework/src/pages/PluginsPage.tsx
- wework/src/pages/PluginManagementPage.tsx
- executor/tests/runtime_work/test_local_task_store.py
- executor/runtime_work/agent_adapter.py
- executor/runtime_work/local_task_store.py
- wework/src/components/layout/MobileDrawer.tsx
- executor/runtime_work/rpc_handler.py
- backend/app/services/runtime_work_service.py
- wework/src/features/workbench/workbenchReducer.ts
- wework/src/features/workbench/WorkbenchProvider.tsx
- wework/src/components/layout/DesktopSidebar.tsx
Contributor
There was a problem hiding this comment.
Actionable comments posted: 15
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
executor/runtime_work/rpc_handler.py (1)
264-274:⚠️ Potential issue | 🟠 Major | ⚡ Quick winReject sends while the local task is already running.
_send()can start another adapter/SDK turn for the same task even whentask.runningis already true. That can overlap Codex thread resumes or local agent runs and corrupt the transcript/event stream.Suggested change
content = payload.get("content") or payload.get("message") if not isinstance(content, str) or not content.strip(): raise ValueError("message is required") + if task.running: + return self._error( + "Runtime task is still running", + code="conflict", + ) source = ( payload.get("source") if isinstance(payload.get("source"), dict) else 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 `@executor/runtime_work/rpc_handler.py` around lines 264 - 274, The _send() method does not validate whether the task is already running before initiating a send operation, which can cause overlapping adapter/SDK turns and corrupt the transcript/event stream. After calling _load_payload_task() to get the task object, add a validation check to ensure task.running is false, and raise an appropriate exception (such as ValueError or RuntimeError) if the task is already in a running state before proceeding with the send operation.executor/tests/runtime_work/test_local_task_store.py (1)
969-1006:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winAvoid relying on one event-loop tick for background completion.
await asyncio.sleep(0)can race with the adapter background task. Userun_background=Falsein this test sohandle_runtime_rpc()returns after the stubbed agent has written the transcript.Suggested change
"claude_code": RuntimeAgentAdapter( runtime="claude_code", store=store, execute_agent=execute_agent, + run_background=False, ) }, codex_discovery=EmptyDiscovery(), @@ assert result["success"] is True assert result["accepted"] is True local_task_id = result["localTaskId"] - await asyncio.sleep(0) transcript = await handler.handle_runtime_rpc(🤖 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 `@executor/tests/runtime_work/test_local_task_store.py` around lines 969 - 1006, The test is using `await asyncio.sleep(0)` which only yields to the event loop once and can race with the adapter's background task execution. Modify the `handle_runtime_rpc()` call to add `run_background=False` parameter so that the method waits for the stubbed agent to complete writing the transcript before returning, eliminating the race condition and making the test more reliable without relying on a single event-loop tick.
🧹 Nitpick comments (7)
executor/runtime_work/codex_discovery.py (2)
353-429: 🏗️ Heavy liftExtract transcript event handling out of
_read_session_transcript.This parser is now over the preferred 50-line function limit and mixes file I/O, event dispatch, turn state, and message construction. Split event-specific handlers or a small parser class before this grows further. As per coding guidelines, “Function length: Max 50 lines per function (preferred).” Based on learnings, “Manage complexity through extraction.”
🤖 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 `@executor/runtime_work/codex_discovery.py` around lines 353 - 429, The _read_session_transcript function exceeds the 50-line limit and handles multiple concerns including file I/O, event dispatching, turn state management, and message construction. Extract the event-specific handling logic by creating separate handler functions for each event type (user_message, agent_message, task_complete, turn_aborted) or a small parser class to encapsulate the payload processing logic. Move the conditional branches and message construction logic out of the main loop in _read_session_transcript so the function focuses only on file I/O iteration and pending message finalization, keeping the code clean and maintainable.Sources: Coding guidelines, Learnings
42-49: ⚡ Quick winAdd tracing to the runtime-work entrypoints.
discover(),read_transcript(),archive_thread(), and asyncstream_message()are the main SDK/runtime boundaries but currently emit no spans or attributes. As per coding guidelines, “Use OpenTelemetry tracing decorators:@trace_async(span_name, tracer_name, extract_attributes)for async functions,@trace_sync()for sync functions.”Also applies to: 92-100, 116-149
🤖 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 `@executor/runtime_work/codex_discovery.py` around lines 42 - 49, Add OpenTelemetry tracing decorators to the runtime-work entrypoint methods to comply with coding guidelines. Apply the `@trace_sync`() decorator to the synchronous methods discover(), read_transcript(), and archive_thread() to enable tracing for these SDK/runtime boundaries. For the async stream_message() method, apply the `@trace_async`() decorator instead, providing appropriate span names and tracer names as needed. Ensure the decorators are imported from the appropriate tracing module and placed directly above each method definition.Source: Coding guidelines
executor/tests/runtime_work/test_local_task_store.py (1)
949-1027: 🏗️ Heavy liftSplit this oversized test module.
This file now exceeds the 1000-line limit, with change details extending to line 1167. Move Codex discovery, RPC handler, and agent adapter scenarios into focused test modules. As per coding guidelines, “File size limit: If a file exceeds 1000 lines, split it into multiple sub-modules.” Based on learnings, “Manage complexity through extraction.”
🤖 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 `@executor/tests/runtime_work/test_local_task_store.py` around lines 949 - 1027, The test file has exceeded the 1000-line limit and needs to be refactored into multiple focused test modules. Extract test scenarios that involve RuntimeWorkRpcHandler, RuntimeAgentAdapter, and LocalTaskStore into separate dedicated test modules (such as test_rpc_handler.py, test_agent_adapter.py, etc.). Move test helper classes like EmptyDiscovery and helper functions like execute_agent to appropriate test utility modules or fixture files. Update imports in the original test file to reference the newly extracted test modules while maintaining test organization and clarity.Sources: Coding guidelines, Learnings
executor/runtime_work/rpc_handler.py (1)
90-118: ⚡ Quick winTrace the runtime RPC dispatch boundary.
handle_runtime_rpc()is the central async entrypoint for list/transcript/send/archive/status, but it has no tracing decorator or attributes for method/task/workspace. As per coding guidelines, “Use OpenTelemetry tracing decorators:@trace_async(span_name, tracer_name, extract_attributes)for async functions.”🤖 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 `@executor/runtime_work/rpc_handler.py` around lines 90 - 118, The handle_runtime_rpc async method lacks distributed tracing instrumentation as required by the coding guidelines. Add the `@trace_async` decorator to the handle_runtime_rpc method with an appropriate span name like "handle_runtime_rpc", specify the tracer_name parameter, and configure extract_attributes to capture the method name from the incoming data dict so that runtime RPC dispatch operations are properly traced for observability and debugging.Source: Coding guidelines
backend/app/services/im/session_service.py (1)
183-196: ⚡ Quick winTrace the new runtime-task binding mutation.
bind_active_runtime_taskis a new async state mutation that controls IM → runtime continuation, but it has no tracing decorator. Add the project@trace_async(...)wrapper and set non-sensitive attributes such assession_key,device_id, andlocal_task_id.As per coding guidelines,
Use OpenTelemetry tracing decorators:@trace_async(span_name, tracer_name, extract_attributes) for async functions,@trace_sync() for sync functions. Add events with add_span_event(), set attributes with set_span_attribute().🤖 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 `@backend/app/services/im/session_service.py` around lines 183 - 196, The bind_active_runtime_task async method is missing OpenTelemetry tracing instrumentation. Add the `@trace_async` decorator to this method with an appropriate span name, tracer name, and an extract_attributes function that captures non-sensitive session attributes such as session_key, device_id, and local_task_id from the session parameter. Follow the project's coding guidelines for async function tracing by including the trace decorator above the async function definition.Source: Coding guidelines
backend/app/services/runtime_work_kind_store.py (1)
28-129: ⚡ Quick winAdd tracing to the new runtime-work store operations.
upsert_device_workspace_kind,list_device_workspace_kinds, andtouch_device_workspace_kindare new synchronous persistence paths. Add@trace_sync()and set attributes/events for the Kind identity and workspace mapping lifecycle.As per coding guidelines,
Use OpenTelemetry tracing decorators:@trace_async(span_name, tracer_name, extract_attributes) for async functions,@trace_sync() for sync functions. Add events with add_span_event(), set attributes with set_span_attribute().🤖 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 `@backend/app/services/runtime_work_kind_store.py` around lines 28 - 129, Add OpenTelemetry tracing to three synchronous persistence functions that manage DeviceWorkspace Kind entities. First, decorate upsert_device_workspace_kind, list_device_workspace_kinds, and touch_device_workspace_kind with the `@trace_sync`() decorator. Then, within each function body, use set_span_attribute() to record Kind identity information (such as user_id, device_id, workspace_path_hash, and kind type) and use add_span_event() to track the lifecycle events of the workspace mapping operations (creation, updates, retrieval, etc.) to enable observability of these persistence operations.Source: Coding guidelines
backend/app/services/channels/handler.py (1)
976-1150: 🏗️ Heavy liftExtract and trace the runtime continuation flow.
This adds a large runtime IM orchestration block to an already oversized handler. Move request construction/callback lifecycle/web-emission into a dedicated runtime IM continuation helper/service, and decorate the new async/sync entry points with the project tracing decorators.
As per coding guidelines,
File size limit: If a file exceeds 1000 lines, split it into multiple sub-modules,Function length: Max 50 lines per function (preferred), andUse OpenTelemetry tracing decorators:@trace_async(span_name, tracer_name, extract_attributes) for async functions,@trace_sync() for sync functions. Based on learnings,Manage complexity through extraction: If a module is already complex, prioritize extracting common logic into utilities or creating new modules rather than adding more complexity to the existing module.🤖 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 `@backend/app/services/channels/handler.py` around lines 976 - 1150, The _execute_private_im_continue_runtime_task method and its related helper methods (_runtime_task_callback_key, _register_private_im_runtime_callback, _delete_private_im_runtime_callback, _emit_private_im_runtime_user_message_to_web) contain complex runtime orchestration logic that should be extracted from this handler into a dedicated service module. Create a new runtime IM continuation service that encapsulates the request construction, callback lifecycle management, and web-emission logic, then refactor the handler to delegate to this service. Apply the appropriate OpenTelemetry tracing decorators (`@trace_async` for async functions and `@trace_sync` for sync functions) to all new service methods following the project guidelines. Ensure each extracted function stays under the 50-line limit and the new module maintains proper separation of concerns.Sources: Coding guidelines, Learnings
🤖 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 `@backend/app/api/endpoints/runtime_work.py`:
- Around line 134-170: Both the bind_runtime_task_im_sessions_endpoint and
archive_runtime_task_endpoint async functions are missing OpenTelemetry tracing
instrumentation required by the repository standards. Add the `@trace_async`
decorator to each function with appropriate span_name, tracer_name, and
extract_attributes parameters. Additionally, within the function bodies before
returning the service call results, add span metadata by calling
add_span_event() to record significant events and set_span_attribute() to attach
relevant attributes such as user_id or request details to provide full
traceability consistent with other endpoints in the codebase.
In `@backend/app/api/ws/device_namespace.py`:
- Around line 1526-1892: The DeviceNamespace class is too large and contains
excessive event-translation logic that should be extracted. Create a new helper
module or class to handle local-task event processing, and move the following
methods out of DeviceNamespace: _local_task_execution_event,
_emit_local_task_execution_event, _local_task_chat_payload,
_local_task_tool_block, _local_task_tool_update_payload,
_forward_local_task_event_to_channel_callbacks, and _local_task_terminal_status.
Keep _handle_local_task_responses_api_event in DeviceNamespace as the entry
point that delegates to this new helper class for all event translation and
emission logic. This will reduce DeviceNamespace's responsibility to
authentication and routing while keeping event-handling logic focused and
organized in a dedicated module.
In `@backend/app/services/channels/handler.py`:
- Around line 1000-1037: The callback is being registered before validating the
runtime_task address, and only HTTPException is caught during cleanup. If
RuntimeTaskAddress.model_validate() raises a ValidationError, the callback
remains registered with a stale key. Move the
RuntimeTaskAddress.model_validate(runtime_task) call to the start of this
handler before calling _register_private_im_runtime_callback, derive the
callback_key from the validated address result instead of calling
_runtime_task_callback_key(runtime_task), and update the exception handler to
catch both ValidationError and HTTPException, ensuring
_delete_private_im_runtime_callback(callback_key) is called for all validation
and runtime failures.
In `@backend/app/services/channels/telegram/emitter.py`:
- Around line 395-459: The _finished flag check needs to be moved inside the
_update_lock acquisition block in the emit_done method to prevent race
conditions where concurrent terminal events (emit_error, emit_cancelled) can
overwrite a finalized message. Additionally, the pending content flushing logic
(which concatenates _pending_content to final_content) currently only exists in
emit_done but should also be applied in the emit_error and emit_cancelled
methods to avoid dropping recently throttled chunks before showing terminal
markers. Move the _finished check inside the async with self._update_lock block
after acquiring the lock, and ensure all three terminal event handlers
(emit_done, emit_error, emit_cancelled) consistently handle _pending_content by
flushing it into the final content before creating the display message.
In `@backend/app/services/device/command_registry.py`:
- Around line 832-856: The broad except Exception block in the thread discovery
code silently converts all failures (import errors, binary resolution, API
failures, normalization errors) into an empty records list, making it impossible
for callers to distinguish between legitimate empty results and actual discovery
failures. Instead of catching the exception and setting records = [], either log
the error and include it in a structured response with an error field alongside
threads, or re-raise the exception and let the caller handle it with a non-zero
exit code. Reference the except Exception block that currently executes records
= [] and either add error information to the response structure or remove the
silent catch to allow proper error propagation.
In `@backend/app/services/runtime_work_kind_store.py`:
- Around line 41-46: The `_get_device_workspace_kind` helper function filters by
`is_active` status, which causes soft-deleted rows to be ignored during upsert
operations. When a row with the same Kind identity (identified by the three
fields: namespace, name, and user_id) exists but is inactive, the code treats it
as missing and attempts to insert a duplicate, causing conflicts. Replace the
call to `_get_device_workspace_kind` in the `upsert_device_workspace_kind`
function with a direct database query that locates the row by its three identity
fields regardless of the `is_active` status, then reactivate it by setting
`row.is_active = True` instead of adding a new row. Apply the same fix to the
second occurrence mentioned in lines 154-170.
- Line 90: In the runtime_work_kind_store.py file at line 90, the expression
Kind.is_active == True uses explicit boolean comparison which violates the E712
Ruff rule. Replace this comparison with the boolean predicate directly by
changing Kind.is_active == True to simply Kind.is_active, following PEP 8 and
standard SQLAlchemy patterns for boolean field predicates.
In `@backend/app/services/runtime_work_service.py`:
- Around line 369-370: In the except block that catches RuntimeRpcError and
raises HTTPException with the status code HTTP_502_BAD_GATEWAY, modify the raise
statement to include "from exc" at the end to preserve the exception chain. This
ensures that the original RuntimeRpcError is maintained in the exception
context, which helps with debugging by showing the full exception traceback.
In `@backend/tests/api/ws/test_device_capabilities_state.py`:
- Around line 467-490: The await first_event_started.wait() call lacks a timeout
bound, which can cause the test to hang indefinitely if the event is never set.
Wrap the first_event_started.wait() call with asyncio.wait_for() and provide a
reasonable timeout value (such as a few seconds) to ensure the test fails with a
clear timeout error instead of hanging if the event is never triggered.
In `@backend/tests/services/test_local_device_command_service.py`:
- Around line 494-497: The environment override in the limit test does not set
CODEX_HOME, which can cause the test to accidentally depend on host defaults or
home directory state if the command script reads config paths. Add CODEX_HOME to
the env dictionary (around line 494-497) alongside PYTHONPATH and
WEGENT_CODEX_THREADS_LIMIT, using a test-specific path such as fake_sdk_root or
a similar temporary directory to ensure the test remains fully isolated from the
host system state.
- Around line 450-451: Replace all hardcoded "python3" strings with
sys.executable in all subprocess.run calls throughout the test module to ensure
the test uses the same Python interpreter as the active runtime. First, add
import sys at the top of the module if not already present. Then locate all
instances where subprocess.run is called with ["python3" as the first argument
(found in 13 locations including lines 83, 451, 493, 1093, 1148, 1192, 1234,
1264, 1309, 1335, 1372, 1401, 1488) and replace each ["python3" with
[sys.executable to ensure test consistency across different Python environments.
In `@executor/runtime_work/codex_discovery.py`:
- Around line 271-278: The _codex_enum_value function uses a broad Exception
handler that catches all exceptions, which can mask real SDK or import errors.
Replace the generic except Exception clause with specific exception types that
represent the expected failure modes: ImportError (for import failures),
AttributeError (for missing enum names in v2_all), and TypeError or ValueError
(for invalid enum values passed to the enum constructor). This way, unexpected
errors will propagate and alert you to real issues rather than silently falling
back to the raw string value.
In `@executor/runtime_work/rpc_handler.py`:
- Around line 358-367: In the exception handler for the stream_message call in
the try-except block, after logging the exception with logger.exception, emit a
terminal error event to notify clients listening to responses_event_emitter. Use
the emitter object created by self._create_local_task_emitter to emit a
response.error message so that IM/web streams properly close instead of
remaining open until timeout. This ensures error information is propagated to
all listening clients rather than only being logged server-side.
- Around line 171-182: The current filtering logic at line 171-182 removes all
stored tasks with runtime="codex", which incorrectly hides adapter-created Codex
tasks with executionRequest and archived Codex tasks. Instead of filtering out
every "codex" runtime task from store_tasks, modify the logic to only exclude
stored Codex tasks that are duplicates of SDK-discovered tasks (i.e., keep
stored Codex tasks that are not found in discovered_tasks). This way,
adapter-created Codex tasks and archived Codex tasks will remain visible while
still avoiding duplicates of active SDK-discovered tasks.
In `@executor/tests/runtime_work/test_local_task_store.py`:
- Around line 163-171: Replace the hard-coded `/tmp` path in the SimpleNamespace
metadata fixture with a path derived from the `tmp_path` fixture. In the
_codex_discovery_for_threads call around the path parameter and all other
occurrences mentioned at lines 208-216 and 580-639, construct the temporary file
paths using `tmp_path` instead of hard-coding `/tmp/thread.jsonl`. For example,
change `path="/tmp/thread.jsonl"` to `path=str(tmp_path / "thread.jsonl")` to
satisfy static analysis checks while maintaining the same test behavior.
---
Outside diff comments:
In `@executor/runtime_work/rpc_handler.py`:
- Around line 264-274: The _send() method does not validate whether the task is
already running before initiating a send operation, which can cause overlapping
adapter/SDK turns and corrupt the transcript/event stream. After calling
_load_payload_task() to get the task object, add a validation check to ensure
task.running is false, and raise an appropriate exception (such as ValueError or
RuntimeError) if the task is already in a running state before proceeding with
the send operation.
In `@executor/tests/runtime_work/test_local_task_store.py`:
- Around line 969-1006: The test is using `await asyncio.sleep(0)` which only
yields to the event loop once and can race with the adapter's background task
execution. Modify the `handle_runtime_rpc()` call to add `run_background=False`
parameter so that the method waits for the stubbed agent to complete writing the
transcript before returning, eliminating the race condition and making the test
more reliable without relying on a single event-loop tick.
---
Nitpick comments:
In `@backend/app/services/channels/handler.py`:
- Around line 976-1150: The _execute_private_im_continue_runtime_task method and
its related helper methods (_runtime_task_callback_key,
_register_private_im_runtime_callback, _delete_private_im_runtime_callback,
_emit_private_im_runtime_user_message_to_web) contain complex runtime
orchestration logic that should be extracted from this handler into a dedicated
service module. Create a new runtime IM continuation service that encapsulates
the request construction, callback lifecycle management, and web-emission logic,
then refactor the handler to delegate to this service. Apply the appropriate
OpenTelemetry tracing decorators (`@trace_async` for async functions and
`@trace_sync` for sync functions) to all new service methods following the project
guidelines. Ensure each extracted function stays under the 50-line limit and the
new module maintains proper separation of concerns.
In `@backend/app/services/im/session_service.py`:
- Around line 183-196: The bind_active_runtime_task async method is missing
OpenTelemetry tracing instrumentation. Add the `@trace_async` decorator to this
method with an appropriate span name, tracer name, and an extract_attributes
function that captures non-sensitive session attributes such as session_key,
device_id, and local_task_id from the session parameter. Follow the project's
coding guidelines for async function tracing by including the trace decorator
above the async function definition.
In `@backend/app/services/runtime_work_kind_store.py`:
- Around line 28-129: Add OpenTelemetry tracing to three synchronous persistence
functions that manage DeviceWorkspace Kind entities. First, decorate
upsert_device_workspace_kind, list_device_workspace_kinds, and
touch_device_workspace_kind with the `@trace_sync`() decorator. Then, within each
function body, use set_span_attribute() to record Kind identity information
(such as user_id, device_id, workspace_path_hash, and kind type) and use
add_span_event() to track the lifecycle events of the workspace mapping
operations (creation, updates, retrieval, etc.) to enable observability of these
persistence operations.
In `@executor/runtime_work/codex_discovery.py`:
- Around line 353-429: The _read_session_transcript function exceeds the 50-line
limit and handles multiple concerns including file I/O, event dispatching, turn
state management, and message construction. Extract the event-specific handling
logic by creating separate handler functions for each event type (user_message,
agent_message, task_complete, turn_aborted) or a small parser class to
encapsulate the payload processing logic. Move the conditional branches and
message construction logic out of the main loop in _read_session_transcript so
the function focuses only on file I/O iteration and pending message
finalization, keeping the code clean and maintainable.
- Around line 42-49: Add OpenTelemetry tracing decorators to the runtime-work
entrypoint methods to comply with coding guidelines. Apply the `@trace_sync`()
decorator to the synchronous methods discover(), read_transcript(), and
archive_thread() to enable tracing for these SDK/runtime boundaries. For the
async stream_message() method, apply the `@trace_async`() decorator instead,
providing appropriate span names and tracer names as needed. Ensure the
decorators are imported from the appropriate tracing module and placed directly
above each method definition.
In `@executor/runtime_work/rpc_handler.py`:
- Around line 90-118: The handle_runtime_rpc async method lacks distributed
tracing instrumentation as required by the coding guidelines. Add the
`@trace_async` decorator to the handle_runtime_rpc method with an appropriate span
name like "handle_runtime_rpc", specify the tracer_name parameter, and configure
extract_attributes to capture the method name from the incoming data dict so
that runtime RPC dispatch operations are properly traced for observability and
debugging.
In `@executor/tests/runtime_work/test_local_task_store.py`:
- Around line 949-1027: The test file has exceeded the 1000-line limit and needs
to be refactored into multiple focused test modules. Extract test scenarios that
involve RuntimeWorkRpcHandler, RuntimeAgentAdapter, and LocalTaskStore into
separate dedicated test modules (such as test_rpc_handler.py,
test_agent_adapter.py, etc.). Move test helper classes like EmptyDiscovery and
helper functions like execute_agent to appropriate test utility modules or
fixture files. Update imports in the original test file to reference the newly
extracted test modules while maintaining test organization and clarity.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: 8ee011cf-62ab-4aff-9a02-1f5675c7c250
📒 Files selected for processing (53)
backend/app/api/endpoints/runtime_work.pybackend/app/api/ws/device_namespace.pybackend/app/models/im_session.pybackend/app/schemas/runtime_work.pybackend/app/services/channels/callback.pybackend/app/services/channels/handler.pybackend/app/services/channels/telegram/emitter.pybackend/app/services/device/command_registry.pybackend/app/services/im/command_router.pybackend/app/services/im/session_service.pybackend/app/services/runtime_work_kind_store.pybackend/app/services/runtime_work_service.pybackend/tests/api/endpoints/test_runtime_work_api.pybackend/tests/api/ws/test_device_capabilities_state.pybackend/tests/services/channels/telegram_channel/test_emitter.pybackend/tests/services/channels/test_private_im_session_integration.pybackend/tests/services/test_local_device_command_service.pybackend/tests/services/test_runtime_work_service.pydocs/superpowers/specs/2026-06-20-runtime-session-workitems-design.mdexecutor/modes/local/runner.pyexecutor/runtime_work/agent_adapter.pyexecutor/runtime_work/codex_discovery.pyexecutor/runtime_work/rpc_handler.pyexecutor/tests/runtime_work/test_local_task_store.pywework/src/api/imSessions.test.tswework/src/api/imSessions.tswework/src/api/runtimeWork.test.tswework/src/api/runtimeWork.tswework/src/components/chat/ContinueInImDialog.test.tsxwework/src/components/chat/ContinueInImDialog.tsxwework/src/components/chat/MessageList.test.tsxwework/src/components/chat/MessageList.tsxwework/src/components/layout/DesktopSidebar.test.tsxwework/src/components/layout/DesktopSidebar.tsxwework/src/components/layout/DesktopWorkbenchLayout.test.tsxwework/src/components/layout/DesktopWorkbenchLayout.tsxwework/src/components/layout/DesktopWorkbenchMain.tsxwework/src/components/layout/MobileDrawer.tsxwework/src/components/layout/MobileWorkbenchLayout.test.tsxwework/src/components/layout/MobileWorkbenchLayout.tsxwework/src/components/layout/runtimeTaskSidebarHelpers.tswework/src/components/layout/useResizableSidebar.tswework/src/features/workbench/WorkbenchProvider.test.tsxwework/src/features/workbench/WorkbenchProvider.tsxwework/src/i18n/locales/en/common.jsonwework/src/i18n/locales/zh-CN/common.jsonwework/src/lib/navigation.test.tswework/src/lib/navigation.tswework/src/pages/PluginManagementPage.tsxwework/src/pages/PluginsPage.tsxwework/src/pages/WorkbenchPage.tsxwework/src/stream/chatStream.tswework/src/types/api.ts
💤 Files with no reviewable changes (1)
- wework/src/api/imSessions.test.ts
✅ Files skipped from review due to trivial changes (2)
- wework/src/api/runtimeWork.test.ts
- docs/superpowers/specs/2026-06-20-runtime-session-workitems-design.md
🚧 Files skipped from review as they are similar to previous changes (6)
- executor/modes/local/runner.py
- backend/tests/api/endpoints/test_runtime_work_api.py
- wework/src/api/runtimeWork.ts
- executor/runtime_work/agent_adapter.py
- wework/src/components/layout/MobileDrawer.tsx
- wework/src/components/layout/DesktopSidebar.tsx
* docs(wework): design task fork execution target * feat(backend): add task fork schemas * feat(backend): resolve forked task history * feat(backend): create forked tasks * feat(backend): expose task fork endpoint * feat(backend): include fork history in task views * feat(backend): continue forked task message history * feat(wework): add task fork api client * feat(wework): add task fork workbench action * feat(wework): add task fork target dialog * feat(task): support fork runtime migration * feat(runtime): support wework task transfer
Contributor
There was a problem hiding this comment.
Actionable comments posted: 11
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (6)
backend/app/services/channels/handler.py (1)
977-1169: 🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick winInstrument the new runtime-task continuation path with tracing.
Line 977 and Line 1115 add new async runtime-flow entry points, but they are missing
@trace_asyncand span-level events/attributes, which leaves this path under-instrumented in production debugging.As per coding guidelines,
Use OpenTelemetry tracing decorators from shared/telemetry/decorators.pyandAdd add_span_event() and set_span_attribute() calls to add events and attributes to current span for OpenTelemetry tracing.🤖 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 `@backend/app/services/channels/handler.py` around lines 977 - 1169, The async methods _execute_private_im_continue_runtime_task and _emit_private_im_runtime_user_message_to_web lack OpenTelemetry tracing instrumentation. Add the `@trace_async` decorator from shared/telemetry/decorators.py to both methods. Within _execute_private_im_continue_runtime_task, add span events and attributes at key checkpoints: when validating the runtime task address, registering the callback, sending the runtime message, handling errors, and when the response is processed. Similarly, in _emit_private_im_runtime_user_message_to_web, add span events and attributes when extracting task identifiers and when emitting to the socket. Use add_span_event() for notable occurrences and set_span_attribute() for contextual information like user_id, device_id, and task_id to enable production debugging and tracing visibility.Source: Coding guidelines
backend/app/services/runtime_work_kind_store.py (1)
28-173: 🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick winAdd tracing decorators/events to the new Kind-store service functions.
The new sync service functions in this module are not instrumented, so write/list/touch operations currently lack consistent trace visibility and span attributes (for user, project, device/workspace identity).
As per coding guidelines,
Use OpenTelemetry tracing decorators from shared/telemetry/decorators.py:@trace_syncfor sync functionsandAdd add_span_event() and set_span_attribute() calls to add events and attributes to current span for OpenTelemetry tracing.🤖 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 `@backend/app/services/runtime_work_kind_store.py` around lines 28 - 173, Add OpenTelemetry tracing instrumentation to the three main sync functions in this module to provide trace visibility for Kind-store operations. Apply the `@trace_sync` decorator from shared/telemetry/decorators.py to upsert_device_workspace_kind, list_device_workspace_kinds, and touch_device_workspace_kind functions. Within each function, use set_span_attribute() calls to record relevant span attributes such as user_id, project_id, device_id, and workspace_path_hash for context tracking, and use add_span_event() calls to capture key operation events like "device_workspace_created", "device_workspace_updated", "device_workspace_listed", and "device_workspace_touched" to provide visibility into the tracing spans for write/list/touch operations.Source: Coding guidelines
docs/superpowers/specs/2026-06-19-wework-task-fork-execution-target-design.md (1)
1-322:⚠️ Potential issue | 🟡 MinorMove spec file to zh-first documentation structure and create English version.
Per coding guidelines, documentation files must follow the "Write Chinese docs first (docs/zh/), then create English versions (docs/en/)" pattern. This spec is currently in
docs/superpowers/specs/(flat structure) but should be:
- Moved to
docs/zh/superpowers/specs/2026-06-19-wework-task-fork-execution-target-design.md- English version created at
docs/en/superpowers/specs/2026-06-19-wework-task-fork-execution-target-design.mdThe frontmatter and content structure are otherwise compliant.
🤖 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 `@docs/superpowers/specs/2026-06-19-wework-task-fork-execution-target-design.md` around lines 1 - 322, The documentation file `2026-06-19-wework-task-fork-execution-target-design.md` is currently located in a flat structure under `docs/superpowers/specs/` but needs to follow the zh-first documentation pattern. Move the entire file content to `docs/zh/superpowers/specs/2026-06-19-wework-task-fork-execution-target-design.md` to establish it as the Chinese version, then create an English translation of the same file (including all sections: background, goals, non-goals, user experience, API design, data model, history parsing, message ID rules, task details, execution context, workspace handling, frontend integration, error handling, testing plan, and implementation boundaries) at `docs/en/superpowers/specs/2026-06-19-wework-task-fork-execution-target-design.md`. Keep the frontmatter structure identical in both versions.Source: Coding guidelines
executor/agents/codex/codex_agent.py (1)
117-124:⚠️ Potential issue | 🟠 Major | ⚡ Quick winPrepare standalone workspace metadata before restoring the fork archive.
For standalone forked Codex tasks,
restore_fork_workspace_archive_if_needed()can setself.project_pathto the fallback task directory before_prepare_standalone_chat_workspace()setstask_data.project_workspace_path. The restored workspace and completion field can then point at different directories.🐛 Proposed reorder
async def pre_execute(self) -> Tuple[TaskStatus, Optional[str]]: try: - self.prepare_project_workspace_path() - await self.restore_fork_workspace_archive_if_needed() - await self.download_code() self._prepare_standalone_chat_workspace() + self.prepare_project_workspace_path() + await self.restore_fork_workspace_archive_if_needed() + await self.download_code() if self.project_path is None: - self.prepare_project_workspace_path() - if self.project_path is None: self.project_path = self._default_workspace_path() Path(self.project_path).mkdir(parents=True, exist_ok=True)🤖 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 `@executor/agents/codex/codex_agent.py` around lines 117 - 124, The initialization sequence has the methods in the wrong order, causing `restore_fork_workspace_archive_if_needed()` to set `self.project_path` before `_prepare_standalone_chat_workspace()` has a chance to prepare the standalone workspace metadata. Move the `_prepare_standalone_chat_workspace()` call to occur before `restore_fork_workspace_archive_if_needed()` so that the standalone workspace metadata is properly initialized before the fork archive restoration attempts to set paths, ensuring the restored workspace and completion fields point to the correct directory.executor/agents/claude_code/claude_code_agent.py (1)
1103-1113:⚠️ Potential issue | 🟠 Major | ⚡ Quick winKeep
self.session_idin sync after seeding an inherited session.When
_seed_inherited_session()creates the saved session file,self.session_idwas already resolved in__init__. The client resumessaved_session_id, but later queries still pass the staleself.session_id, so forked Claude turns may not continue the inherited session correctly.🐛 Proposed fix
if saved_session_id: + self.session_id = saved_session_id logger.info( f"Resuming Claude session for task {self.task_id} " f"(bot_id={self._bot_id}): {saved_session_id}" ) @@ # Remove resume option and retry del self.options["resume"] saved_session_id = None + self.session_id = self._internal_session_key # Recreate client without resume optionAlso applies to: 1186-1197
🤖 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 `@executor/agents/claude_code/claude_code_agent.py` around lines 1103 - 1113, After `_seed_inherited_session()` creates the saved session file and `saved_session_id` is loaded via `SessionManager.load_saved_session_id()`, you must synchronize `self.session_id` with the loaded `saved_session_id` to ensure consistency. When `self.options["resume"]` is set to `saved_session_id`, also update `self.session_id` to the same value so that subsequent queries and forked Claude turns use the correct inherited session ID instead of the stale `self.session_id` that was resolved during `__init__`. Apply this same fix at the other location mentioned in the comment (around lines 1186-1197).backend/app/services/adapters/task_kinds/helpers.py (1)
93-117:⚠️ Potential issue | 🟠 Major | ⚡ Quick winAvoid loading the full fork history just to check existence.
existing_subtasksmaterializes every inherited/local subtask, but the downstream pipeline logic only needs a truthy “has prior messages” signal. On long fork histories this turns each new message into an unbounded DB/read path; uselimit=1or a dedicated existence helper instead.⚡ Proposed refactor
- existing_subtasks = [ - item.subtask - for item in task_fork_history_resolver.resolve_for_task( + has_existing_subtasks = bool( + task_fork_history_resolver.resolve_for_task( db, task_id=task.id, user_id=user_id, + limit=1, ) - ] + ) next_message_id = task_fork_history_resolver.get_next_message_id( db, task_id=task.id, user_id=user_id, @@ team, team_crd, task_crd, - existing_subtasks, + has_existing_subtasks, )Also update the helper parameter and truthiness check:
- existing_subtasks: List[Subtask], + has_existing_subtasks: bool, ) -> tuple[str, List[int]]: @@ - elif existing_subtasks: + elif has_existing_subtasks:🤖 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 `@backend/app/services/adapters/task_kinds/helpers.py` around lines 93 - 117, The code materializes the entire fork history into the existing_subtasks list, but the downstream _build_pipeline_assistant_subtask_plan function only needs to know if subtasks exist, not what they are. Replace the list comprehension that loads all subtasks by calling task_fork_history_resolver.resolve_for_task with a limit parameter set to 1 to check for existence only, then convert the result to a boolean check (e.g., checking if any items exist). Update the variable from existing_subtasks to a more appropriate name like has_prior_subtasks and modify the _build_pipeline_assistant_subtask_plan call to pass this boolean flag instead of the full list, along with updating the function signature if needed to accept a boolean parameter.
♻️ Duplicate comments (1)
backend/app/api/endpoints/runtime_work.py (1)
224-240:⚠️ Potential issue | 🟠 Major | ⚡ Quick winAdd required OpenTelemetry instrumentation to the new
/forkendpoint.This async endpoint is missing
@trace_async(...),set_span_attribute(...), andadd_span_event(...), so it breaks tracing consistency for runtime-work APIs.🔧 Suggested patch
`@router.post`( "/fork", response_model=RuntimeTaskForkResponse, response_model_by_alias=True, ) +@trace_async("runtime_work.fork_task", "runtime_work.api") async def fork_runtime_task_endpoint( request: RuntimeTaskForkRequest, db: Session = Depends(get_db), current_user: User = Depends(get_current_user), ): """Fork a native runtime LocalTask to another device workspace.""" + set_span_attribute("user.id", current_user.id) + set_span_attribute("runtime.source_device_id", request.source.device_id) + set_span_attribute("runtime.source_local_task_id", request.source.local_task_id) + set_span_attribute("runtime.target_device_id", request.target.device_id) + add_span_event( + "runtime_work.task.fork", + { + "user.id": current_user.id, + "runtime.source_device_id": request.source.device_id, + "runtime.source_local_task_id": request.source.local_task_id, + "runtime.target_device_id": request.target.device_id, + }, + ) return await runtime_work_service.fork_runtime_task( db=db, user_id=current_user.id, request=request, )As per coding guidelines,
**/*.py: “Use OpenTelemetry tracing decorators from shared/telemetry/decorators.py:@trace_asyncfor async functions... Add add_span_event() and set_span_attribute() calls...”.🤖 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 `@backend/app/api/endpoints/runtime_work.py` around lines 224 - 240, The fork_runtime_task_endpoint async function is missing OpenTelemetry instrumentation required by coding guidelines. Add the `@trace_async` decorator from shared/telemetry/decorators.py to the function, and within the function body add set_span_attribute calls to record relevant attributes like user_id and request details, and add_span_event calls to mark important execution points such as task fork initiation and completion. Ensure these imports and instrumentation calls are consistent with other runtime-work endpoints in the same file.Source: Coding guidelines
🧹 Nitpick comments (10)
backend/app/services/channels/handler.py (1)
977-1084: 🏗️ Heavy liftSplit
_execute_private_im_continue_runtime_taskinto smaller helpers.This method now handles address validation, callback registration/cleanup, runtime RPC dispatch, and accepted/rejected response branching in one block. Extracting those responsibilities will reduce regression risk and make tests simpler.
Based on learnings,
Function length: Max 50 lines per function (preferred)andManage complexity through extraction: prioritize extracting common logic into utilities or creating new modules rather than adding more complexity to existing modules.🤖 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 `@backend/app/services/channels/handler.py` around lines 977 - 1084, The method `_execute_private_im_continue_runtime_task` is too long and handles multiple responsibilities including address validation, callback registration, runtime RPC dispatch, and response handling. Extract this into smaller helper methods: create one helper to validate and retrieve the RuntimeTaskAddress, create another to send the runtime message and handle send errors (the try-except block calling runtime_work_service.send_runtime_message), and create a third to handle the response branching logic (checking response.accepted and sending appropriate replies). Keep the main method focused on orchestration by calling these helpers in sequence, reducing complexity and improving testability.Source: Learnings
shared/tests/test_openai_converter.py (1)
98-125: ⚡ Quick winAdd return type annotation to test function.
Test function should include
-> Nonereturn type annotation per coding guidelines.📝 Proposed fix
-def test_round_trip_preserves_fork_runtime_and_inherited_sessions(): +def test_round_trip_preserves_fork_runtime_and_inherited_sessions() -> 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 `@shared/tests/test_openai_converter.py` around lines 98 - 125, The test function test_round_trip_preserves_fork_runtime_and_inherited_sessions is missing the return type annotation. Add `-> None` return type annotation to the function signature after the closing parenthesis of the function parameters to comply with the coding guidelines.Source: Coding guidelines
backend/tests/schemas/test_task_fork_schema.py (1)
13-21: ⚡ Quick winAdd type hints to test functions.
Test functions should include return type annotations (
-> None) for consistency with coding guidelines requiring type hints.📝 Proposed fix
-def test_task_fork_spec_round_trips_camel_case_fields(): +def test_task_fork_spec_round_trips_camel_case_fields() -> None:Apply the same pattern to all test functions in this file.
🤖 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 `@backend/tests/schemas/test_task_fork_schema.py` around lines 13 - 21, The test function `test_task_fork_spec_round_trips_camel_case_fields` is missing a return type annotation. Add `-> None` to the function signature after the closing parenthesis of the parameters to include the return type hint, which should be applied to all test functions in this file for consistency with coding guidelines.Source: Coding guidelines
executor/tests/services/test_workspace_archive_restore.py (2)
31-45: ⚡ Quick winAdd return type annotation to test function.
Test function should include
-> Nonereturn type annotation.📝 Proposed fix
-def test_restore_archive_content_splits_workspace_and_allowlisted_home(tmp_path): +def test_restore_archive_content_splits_workspace_and_allowlisted_home(tmp_path: Path) -> None:Note: Also added type hint for
tmp_pathparameter.🤖 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 `@executor/tests/services/test_workspace_archive_restore.py` around lines 31 - 45, Add a return type annotation `-> None` to the test function `test_restore_archive_content_splits_workspace_and_allowlisted_home`. Additionally, add a type hint for the `tmp_path` parameter to specify its type instead of leaving it untyped.Source: Coding guidelines
11-28: ⚡ Quick winAdd type hints to helper function.
The helper function should include return type annotation for consistency with coding guidelines.
📝 Proposed fix
-def _archive_bytes() -> bytes: +def _archive_bytes() -> bytes:Wait, the function already has
-> bytesannotation. Let me re-check... Looking at line 11:def _archive_bytes() -> bytes:Actually, it DOES have the type hint. My mistake. This is correct.
🤖 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 `@executor/tests/services/test_workspace_archive_restore.py` around lines 11 - 28, No changes are needed. The reviewer initially flagged the _archive_bytes() function for adding type hints but upon inspection recognized that the function already includes the proper return type annotation (-> bytes). The code is compliant with coding guidelines as written.Source: Coding guidelines
executor/tests/agents/test_fork_session_inheritance.py (1)
17-24: ⚡ Quick winAdd type hints to helper and test functions.
All functions should include type annotations per coding guidelines.
📝 Proposed fix
-def _emitter() -> MagicMock: +def _emitter() -> MagicMock:Also apply to all test functions:
-def test_claude_code_seeds_forked_task_session_from_inherited_session(tmp_path): +def test_claude_code_seeds_forked_task_session_from_inherited_session(tmp_path: Path) -> None:And the async test functions:
-async def test_codex_resumes_inherited_thread_for_forked_task(tmp_path): +async def test_codex_resumes_inherited_thread_for_forked_task(tmp_path: Path) -> 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 `@executor/tests/agents/test_fork_session_inheritance.py` around lines 17 - 24, Add type hints to all helper and test functions in the test file to comply with coding guidelines. The _emitter helper function should have explicit return type annotation for MagicMock, and all test functions (both regular and async functions marked with the `@pytest.mark.asyncio` or similar decorators) should include proper return type hints. For functions with no return value, use None as the return type. Ensure parameter types are also annotated where applicable.Source: Coding guidelines
backend/app/services/adapters/task_kinds/queries.py (1)
369-374: ⚡ Quick winExtract the fork-history limit into a named constant.
Line 373 hardcodes
100; this is a policy/value that should be centralized to avoid drift.As per coding guidelines, "Extract magic numbers to constants in Python code."
Proposed refactor
+TASK_DETAIL_FORK_HISTORY_LIMIT = 100 + ... subtasks = task_fork_history_resolver.resolve_for_task( db=db, task_id=task_id, user_id=task_dict.get("user_id") or user_id, - limit=100, + limit=TASK_DETAIL_FORK_HISTORY_LIMIT, )🤖 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 `@backend/app/services/adapters/task_kinds/queries.py` around lines 369 - 374, Extract the hardcoded value 100 used in the limit parameter of the task_fork_history_resolver.resolve_for_task() method call into a named constant. Define this constant at the module or class level with a descriptive name that reflects its purpose (e.g., FORK_HISTORY_LIMIT or similar), then replace the hardcoded 100 with the new constant reference to centralize this policy value and prevent drift.Source: Coding guidelines
backend/tests/services/test_task_fork_history.py (1)
93-131: ⚡ Quick winAdd a mixed-owner fork lineage test case.
Current fork tests only cover same-owner chains, so owner-mismatch regressions in lineage resolution can slip through.
🤖 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 `@backend/tests/services/test_task_fork_history.py` around lines 93 - 131, Add a new test function following a similar pattern to test_resolve_for_fork_uses_parent_cutoff that validates fork lineage resolution when tasks have different owners. Create a test scenario where the root task and/or fork task are owned by different user IDs (not all owned by the same user_id=7), then verify that task_fork_history_resolver.resolve_for_task correctly handles owner-mismatch cases and returns the expected lineage with proper inheritance flags. This ensures that the fork history resolution logic properly enforces owner boundaries and prevents cross-owner lineage issues.backend/app/services/runtime_work_service.py (1)
429-604: ⚖️ Poor tradeoffConsider decomposing the fork transfer flow.
At ~175 lines, this function significantly exceeds the preferred 50-line limit. The three fallback strategies (direct import → push transfer → storage fallback) could be extracted into separate helper functions to improve readability and testability.
🤖 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 `@backend/app/services/runtime_work_service.py` around lines 429 - 604, The fork_runtime_task function exceeds the 50-line preferred limit at approximately 175 lines and contains three distinct fallback strategies that should be extracted into separate helper functions. Extract the push transfer fallback logic (the nested block that calls prepare_fork_receiver and push_fork_transfer when the initial import fails) into a dedicated helper function, extract the storage fallback logic (the final block that generates presigned URLs and calls upload_fork_transfer) into another helper function, and refactor fork_runtime_task to orchestrate these three strategies sequentially by calling the extracted helper functions with appropriate parameters and chaining them based on success conditions. Each helper function should handle its strategy independently and return whether it succeeded, allowing the main function to delegate to the next fallback strategy when needed.Source: Learnings
backend/tests/services/test_local_device_command_service.py (1)
445-483: ⚡ Quick winExtract shared fake-SDK scaffolding to avoid fixture drift.
_write_failing_openai_codex_sdkduplicates package/bootstrap code from_write_fake_openai_codex_sdk. Pull the shared file-generation into one helper and vary onlythread_listbehavior so both fixtures stay structurally identical.Suggested refactor
+def _write_openai_codex_sdk(root, thread_list_impl): + package_root = root / "openai_codex" + generated_root = package_root / "generated" + generated_root.mkdir(parents=True) + (package_root / "__init__.py").write_text( + f""" +class CodexConfig: + def __init__(self, **kwargs): + self.kwargs = kwargs + + +class Codex: + def __init__(self, config): + self.config = config + + def __enter__(self): + return self + + def __exit__(self, _exc_type, _exc, _tb): + return None + + def thread_list(self, **kwargs): +{thread_list_impl} +""".strip(), + encoding="utf-8", + ) + (generated_root / "__init__.py").write_text("", encoding="utf-8") + (generated_root / "v2_all.py").write_text( + """ +class SortDirection(str): + pass + + +class ThreadSortKey(str): + pass +""".strip(), + encoding="utf-8", + ) + return rootBased on learnings: "Reference, extract, then reuse" and "Extract common logic from ALL code".
🤖 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 `@backend/tests/services/test_local_device_command_service.py` around lines 445 - 483, Extract the shared package scaffolding code from both _write_failing_openai_codex_sdk and _write_fake_openai_codex_sdk into a single helper function that creates the common directory structure and shared classes like CodexConfig and Codex context manager. Then refactor both fixtures to call this shared helper and only override the thread_list method behavior to maintain structural consistency and avoid duplication.Source: Learnings
🤖 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 `@backend/app/api/ws/local_task_responses.py`:
- Around line 46-47: The subtask_id calculation using runtime_subtask_id at line
46 does not include device_id in its fallback hash, causing collisions when
identical local_task_ids appear across different devices. Modify the
runtime_subtask_id function (defined around lines 384-388) to accept and include
device_id in the fallback hash computation. Then update the call to
runtime_subtask_id at line 46 to extract device_id from the data parameter and
pass it to the function so that the resulting subtask_id is scoped by both
local_task_id and device_id, preventing cross-device event collisions in the
shared lock map.
- Around line 42-43: The race condition exists because cleanup_lock removes the
registry entry while coroutines may still hold references to or be waiting on
the lock object. After releasing the lock in the try-finally blocks where
cleanup_lock is called, ensure the cleanup operation is delayed or protected by
a synchronization mechanism so that get_lock cannot allocate a new lock object
for the same subtask_id before all coroutines have finished with the old lock.
Consider implementing reference counting within the lock-wrapper object or using
an async-safe locking mechanism around the registry to prevent new lock
allocations during the cleanup window, ensuring only one lock instance is active
per subtask at any time.
In `@backend/app/services/runtime_work_service.py`:
- Around line 457-458: The fork_runtime_task function has five exception
handlers (RuntimeRpcError and others at the specified line ranges) that are
missing the exception chain preservation pattern used consistently throughout
the file. In each of these exception handlers where HTTPException is raised, add
`from exc` at the end of the raise statement to preserve the exception chain for
better error tracking and debugging, making the pattern consistent with other
functions like get_runtime_transcript, send_runtime_message, and
archive_runtime_task.
In `@backend/app/services/task_fork_history.py`:
- Around line 118-122: The lineage lookup in the task fork history traversal
incorrectly restricts all ancestor task queries to the current user_id. When
traversing fork chains where parent tasks may have different owners (such as
shared-task forks), this causes ancestor lookups to fail. Remove or
conditionally exclude the owner_user_id parameter from the task_store.get_by_id
call when retrieving parent/ancestor tasks in the lineage chain to allow lookups
of tasks owned by different users. Ensure the fix applies to the ancestor
traversal logic while maintaining necessary access control for initial task
retrieval.
In `@backend/tests/services/channels/test_private_im_session_integration.py`:
- Around line 541-547: The test currently verifies that "callback" and "send"
are not in the calls tracking variable, but does not verify that
delete_callback_info is also not called on the guard path. Add an explicit
assertion after the existing checks to ensure "delete_callback_info" is also not
in calls, matching the pattern of the existing assertions for "callback" and
"send".
- Line 544: The assertion string in the handler.replies comparison contains a
fullwidth comma (Chinese punctuation character) instead of a regular ASCII
comma, which triggers the Ruff RUF001 lint rule for confusable Unicode
characters. Replace the fullwidth comma between "不可用" and "请" in the expected
reply string "当前本地任务不可用，请回到 Wework 重新选择。" with a standard ASCII comma to pass
lint checks.
In `@executor/runtime_work/fork_transfer.py`:
- Around line 133-134: Replace the cleartext HTTP URLs with HTTPS or refactor to
use a secure relay. In the URL construction where
f"http://{host}:{port}{TRANSFER_UPLOAD_PATH_PREFIX}{transfer_id}?token={token}"
is built, change the protocol from http:// to https://, or better yet, replace
the direct URL generation with an HTTPS/mTLS relay endpoint or an object-storage
upload/download path. Apply this change to all locations where transfer URLs are
constructed (also at lines 198-199 and 264-264 as noted in the comment) to
ensure runtime session archives are not exposed over cleartext on shared
networks.
- Around line 207-214: The current implementation reads the entire archive file
into memory using handle.read(), which causes memory spikes for large
workspaces. Replace this synchronous read with an async generator that properly
handles streaming for HTTPX 0.28.1. In the client.put() call within the async
with httpx.AsyncClient block, implement an async generator that uses
run_in_executor() to read file chunks asynchronously from the archive_path
handle, ensuring file I/O operations don't block the event loop, then pass this
async generator as the content parameter to client.put().
In `@executor/runtime_work/rpc_handler.py`:
- Around line 280-283: The `_send` method has a race condition where checking
`task.running` at the start is not atomic with setting `running=True` later in
the method. Concurrent calls to `_send` can both pass the initial check before
either one sets `running=True`, allowing multiple SDK streams to start for the
same task. Fix this by making the running transition atomic, either by acquiring
a lock before the check in `_send` and only releasing it after `running=True` is
set around line 331, or by using an atomic compare-and-set operation that
prevents concurrent callers from passing the check simultaneously.
In `@executor/services/workspace_archive_restore.py`:
- Around line 27-30: The EXECUTOR_HOME_ARCHIVE_ALLOWLIST set is missing the
Codex session paths that are archived by fork_transfer.HOME_SESSION_ALLOWLIST.
Add the missing paths `.codex/sessions`, `.codex/archived_sessions`, and
`.codex/state` to the EXECUTOR_HOME_ARCHIVE_ALLOWLIST set to ensure session
state files are properly restored when unpacking Codex fork packages. Apply the
same change to the other allowlist definitions mentioned at lines 108-111 and
168-183 to keep them in sync.
- Around line 199-202: The fallback in the except TypeError block that handles
Python 3.10/3.11 compatibility is unsafe because it extracts all tar member
types including symlinks, hardlinks, and special files. Replace the unsafe
archive.extractall(path=str(path), members=members) call in the except TypeError
block with manual safe extraction logic that iterates through the members list
and only extracts tar members that are regular files, skipping any symlinks,
hardlinks, directories, or special file types to prevent potential security
vulnerabilities.
---
Outside diff comments:
In `@backend/app/services/adapters/task_kinds/helpers.py`:
- Around line 93-117: The code materializes the entire fork history into the
existing_subtasks list, but the downstream
_build_pipeline_assistant_subtask_plan function only needs to know if subtasks
exist, not what they are. Replace the list comprehension that loads all subtasks
by calling task_fork_history_resolver.resolve_for_task with a limit parameter
set to 1 to check for existence only, then convert the result to a boolean check
(e.g., checking if any items exist). Update the variable from existing_subtasks
to a more appropriate name like has_prior_subtasks and modify the
_build_pipeline_assistant_subtask_plan call to pass this boolean flag instead of
the full list, along with updating the function signature if needed to accept a
boolean parameter.
In `@backend/app/services/channels/handler.py`:
- Around line 977-1169: The async methods
_execute_private_im_continue_runtime_task and
_emit_private_im_runtime_user_message_to_web lack OpenTelemetry tracing
instrumentation. Add the `@trace_async` decorator from
shared/telemetry/decorators.py to both methods. Within
_execute_private_im_continue_runtime_task, add span events and attributes at key
checkpoints: when validating the runtime task address, registering the callback,
sending the runtime message, handling errors, and when the response is
processed. Similarly, in _emit_private_im_runtime_user_message_to_web, add span
events and attributes when extracting task identifiers and when emitting to the
socket. Use add_span_event() for notable occurrences and set_span_attribute()
for contextual information like user_id, device_id, and task_id to enable
production debugging and tracing visibility.
In `@backend/app/services/runtime_work_kind_store.py`:
- Around line 28-173: Add OpenTelemetry tracing instrumentation to the three
main sync functions in this module to provide trace visibility for Kind-store
operations. Apply the `@trace_sync` decorator from shared/telemetry/decorators.py
to upsert_device_workspace_kind, list_device_workspace_kinds, and
touch_device_workspace_kind functions. Within each function, use
set_span_attribute() calls to record relevant span attributes such as user_id,
project_id, device_id, and workspace_path_hash for context tracking, and use
add_span_event() calls to capture key operation events like
"device_workspace_created", "device_workspace_updated",
"device_workspace_listed", and "device_workspace_touched" to provide visibility
into the tracing spans for write/list/touch operations.
In
`@docs/superpowers/specs/2026-06-19-wework-task-fork-execution-target-design.md`:
- Around line 1-322: The documentation file
`2026-06-19-wework-task-fork-execution-target-design.md` is currently located in
a flat structure under `docs/superpowers/specs/` but needs to follow the
zh-first documentation pattern. Move the entire file content to
`docs/zh/superpowers/specs/2026-06-19-wework-task-fork-execution-target-design.md`
to establish it as the Chinese version, then create an English translation of
the same file (including all sections: background, goals, non-goals, user
experience, API design, data model, history parsing, message ID rules, task
details, execution context, workspace handling, frontend integration, error
handling, testing plan, and implementation boundaries) at
`docs/en/superpowers/specs/2026-06-19-wework-task-fork-execution-target-design.md`.
Keep the frontmatter structure identical in both versions.
In `@executor/agents/claude_code/claude_code_agent.py`:
- Around line 1103-1113: After `_seed_inherited_session()` creates the saved
session file and `saved_session_id` is loaded via
`SessionManager.load_saved_session_id()`, you must synchronize `self.session_id`
with the loaded `saved_session_id` to ensure consistency. When
`self.options["resume"]` is set to `saved_session_id`, also update
`self.session_id` to the same value so that subsequent queries and forked Claude
turns use the correct inherited session ID instead of the stale
`self.session_id` that was resolved during `__init__`. Apply this same fix at
the other location mentioned in the comment (around lines 1186-1197).
In `@executor/agents/codex/codex_agent.py`:
- Around line 117-124: The initialization sequence has the methods in the wrong
order, causing `restore_fork_workspace_archive_if_needed()` to set
`self.project_path` before `_prepare_standalone_chat_workspace()` has a chance
to prepare the standalone workspace metadata. Move the
`_prepare_standalone_chat_workspace()` call to occur before
`restore_fork_workspace_archive_if_needed()` so that the standalone workspace
metadata is properly initialized before the fork archive restoration attempts to
set paths, ensuring the restored workspace and completion fields point to the
correct directory.
---
Duplicate comments:
In `@backend/app/api/endpoints/runtime_work.py`:
- Around line 224-240: The fork_runtime_task_endpoint async function is missing
OpenTelemetry instrumentation required by coding guidelines. Add the
`@trace_async` decorator from shared/telemetry/decorators.py to the function, and
within the function body add set_span_attribute calls to record relevant
attributes like user_id and request details, and add_span_event calls to mark
important execution points such as task fork initiation and completion. Ensure
these imports and instrumentation calls are consistent with other runtime-work
endpoints in the same file.
---
Nitpick comments:
In `@backend/app/services/adapters/task_kinds/queries.py`:
- Around line 369-374: Extract the hardcoded value 100 used in the limit
parameter of the task_fork_history_resolver.resolve_for_task() method call into
a named constant. Define this constant at the module or class level with a
descriptive name that reflects its purpose (e.g., FORK_HISTORY_LIMIT or
similar), then replace the hardcoded 100 with the new constant reference to
centralize this policy value and prevent drift.
In `@backend/app/services/channels/handler.py`:
- Around line 977-1084: The method `_execute_private_im_continue_runtime_task`
is too long and handles multiple responsibilities including address validation,
callback registration, runtime RPC dispatch, and response handling. Extract this
into smaller helper methods: create one helper to validate and retrieve the
RuntimeTaskAddress, create another to send the runtime message and handle send
errors (the try-except block calling runtime_work_service.send_runtime_message),
and create a third to handle the response branching logic (checking
response.accepted and sending appropriate replies). Keep the main method focused
on orchestration by calling these helpers in sequence, reducing complexity and
improving testability.
In `@backend/app/services/runtime_work_service.py`:
- Around line 429-604: The fork_runtime_task function exceeds the 50-line
preferred limit at approximately 175 lines and contains three distinct fallback
strategies that should be extracted into separate helper functions. Extract the
push transfer fallback logic (the nested block that calls prepare_fork_receiver
and push_fork_transfer when the initial import fails) into a dedicated helper
function, extract the storage fallback logic (the final block that generates
presigned URLs and calls upload_fork_transfer) into another helper function, and
refactor fork_runtime_task to orchestrate these three strategies sequentially by
calling the extracted helper functions with appropriate parameters and chaining
them based on success conditions. Each helper function should handle its
strategy independently and return whether it succeeded, allowing the main
function to delegate to the next fallback strategy when needed.
In `@backend/tests/schemas/test_task_fork_schema.py`:
- Around line 13-21: The test function
`test_task_fork_spec_round_trips_camel_case_fields` is missing a return type
annotation. Add `-> None` to the function signature after the closing
parenthesis of the parameters to include the return type hint, which should be
applied to all test functions in this file for consistency with coding
guidelines.
In `@backend/tests/services/test_local_device_command_service.py`:
- Around line 445-483: Extract the shared package scaffolding code from both
_write_failing_openai_codex_sdk and _write_fake_openai_codex_sdk into a single
helper function that creates the common directory structure and shared classes
like CodexConfig and Codex context manager. Then refactor both fixtures to call
this shared helper and only override the thread_list method behavior to maintain
structural consistency and avoid duplication.
In `@backend/tests/services/test_task_fork_history.py`:
- Around line 93-131: Add a new test function following a similar pattern to
test_resolve_for_fork_uses_parent_cutoff that validates fork lineage resolution
when tasks have different owners. Create a test scenario where the root task
and/or fork task are owned by different user IDs (not all owned by the same
user_id=7), then verify that task_fork_history_resolver.resolve_for_task
correctly handles owner-mismatch cases and returns the expected lineage with
proper inheritance flags. This ensures that the fork history resolution logic
properly enforces owner boundaries and prevents cross-owner lineage issues.
In `@executor/tests/agents/test_fork_session_inheritance.py`:
- Around line 17-24: Add type hints to all helper and test functions in the test
file to comply with coding guidelines. The _emitter helper function should have
explicit return type annotation for MagicMock, and all test functions (both
regular and async functions marked with the `@pytest.mark.asyncio` or similar
decorators) should include proper return type hints. For functions with no
return value, use None as the return type. Ensure parameter types are also
annotated where applicable.
In `@executor/tests/services/test_workspace_archive_restore.py`:
- Around line 31-45: Add a return type annotation `-> None` to the test function
`test_restore_archive_content_splits_workspace_and_allowlisted_home`.
Additionally, add a type hint for the `tmp_path` parameter to specify its type
instead of leaving it untyped.
- Around line 11-28: No changes are needed. The reviewer initially flagged the
_archive_bytes() function for adding type hints but upon inspection recognized
that the function already includes the proper return type annotation (-> bytes).
The code is compliant with coding guidelines as written.
In `@shared/tests/test_openai_converter.py`:
- Around line 98-125: The test function
test_round_trip_preserves_fork_runtime_and_inherited_sessions is missing the
return type annotation. Add `-> None` return type annotation to the function
signature after the closing parenthesis of the function parameters to comply
with the coding guidelines.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: 13437a18-1175-4098-806f-d2c8e1f759e7
📒 Files selected for processing (77)
backend/app/api/endpoints/adapter/tasks.pybackend/app/api/endpoints/internal/chat_storage.pybackend/app/api/endpoints/internal/workspace_archives.pybackend/app/api/endpoints/runtime_work.pybackend/app/api/ws/chat_namespace.pybackend/app/api/ws/device_namespace.pybackend/app/api/ws/local_task_responses.pybackend/app/schemas/kind.pybackend/app/schemas/runtime_work.pybackend/app/schemas/task_fork.pybackend/app/services/adapters/task_kinds/helpers.pybackend/app/services/adapters/task_kinds/queries.pybackend/app/services/adapters/task_kinds/task_detail_helpers.pybackend/app/services/channels/handler.pybackend/app/services/channels/telegram/emitter.pybackend/app/services/chat/trigger/lifecycle.pybackend/app/services/device/command_registry.pybackend/app/services/execution/dispatcher.pybackend/app/services/execution/request_builder.pybackend/app/services/runtime_work_kind_store.pybackend/app/services/runtime_work_service.pybackend/app/services/task_fork.pybackend/app/services/task_fork_history.pybackend/tests/api/internal/test_workspace_archive_download_url.pybackend/tests/api/test_internal_chat_storage_fork_history.pybackend/tests/api/test_task_fork_endpoint.pybackend/tests/api/ws/test_chat_namespace_fork_history.pybackend/tests/api/ws/test_device_capabilities_state.pybackend/tests/schemas/test_task_fork_schema.pybackend/tests/services/adapters/test_task_kinds_fork_detail.pybackend/tests/services/adapters/test_task_kinds_fork_message_ids.pybackend/tests/services/channels/telegram_channel/test_emitter.pybackend/tests/services/channels/test_private_im_session_integration.pybackend/tests/services/chat/storage/test_task_manager.pybackend/tests/services/execution/test_dispatcher_recovery.pybackend/tests/services/execution/test_extract_completed_result.pybackend/tests/services/execution/test_request_builder_fork_runtime.pybackend/tests/services/test_local_device_command_service.pybackend/tests/services/test_runtime_work_service.pybackend/tests/services/test_task_fork_history.pybackend/tests/services/test_task_fork_service.pydocs/superpowers/specs/2026-06-19-wework-task-fork-execution-target-design.mdexecutor/agents/base.pyexecutor/agents/claude_code/claude_code_agent.pyexecutor/agents/claude_code/response_processor.pyexecutor/agents/codex/codex_agent.pyexecutor/agents/codex/event_mapper.pyexecutor/runtime_work/agent_adapter.pyexecutor/runtime_work/codex_discovery.pyexecutor/runtime_work/fork_transfer.pyexecutor/runtime_work/rpc_handler.pyexecutor/services/workspace_archive_restore.pyexecutor/tests/agents/test_claude_response_processor.pyexecutor/tests/agents/test_codex_event_mapper.pyexecutor/tests/agents/test_fork_session_inheritance.pyexecutor/tests/runtime_work/test_local_task_store.pyexecutor/tests/services/test_workspace_archive_restore.pyshared/models/execution.pyshared/models/openai_converter.pyshared/tests/test_openai_converter.pywework/src/api/runtimeWork.test.tswework/src/api/runtimeWork.tswework/src/api/tasks.test.tswework/src/components/layout/DesktopSidebar.tsxwework/src/components/layout/DesktopWorkbenchLayout.test.tsxwework/src/components/layout/DesktopWorkbenchLayout.tsxwework/src/components/layout/DesktopWorkbenchMain.tsxwework/src/components/layout/MobileWorkbenchLayout.test.tsxwework/src/components/layout/MobileWorkbenchLayout.tsxwework/src/components/layout/TaskForkDialog.test.tsxwework/src/components/layout/TaskForkDialog.tsxwework/src/features/workbench/WorkbenchProvider.test.tsxwework/src/features/workbench/WorkbenchProvider.tsxwework/src/i18n/locales/en/common.jsonwework/src/i18n/locales/zh-CN/common.jsonwework/src/pages/WorkbenchPage.tsxwework/src/types/api.ts
✅ Files skipped from review due to trivial changes (2)
- backend/tests/services/adapters/test_task_kinds_fork_message_ids.py
- backend/app/schemas/task_fork.py
🚧 Files skipped from review as they are similar to previous changes (8)
- wework/src/api/runtimeWork.ts
- backend/app/schemas/runtime_work.py
- backend/tests/api/ws/test_device_capabilities_state.py
- executor/runtime_work/codex_discovery.py
- backend/app/services/channels/telegram/emitter.py
- backend/app/services/device/command_registry.py
- executor/runtime_work/agent_adapter.py
- wework/src/components/layout/DesktopSidebar.tsx
qdaxb
force-pushed
the
codex/runtime-session-workitems
branch
from
June 21, 2026 16:55
516b01e to
769bf14
Compare
This was referenced Jun 22, 2026
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
1 participant
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
Validation
cd wework && pnpm exec tsc -b --pretty falsecd wework && pnpm run lintcd wework && pnpm exec vitest runcd wework && pnpm run buildcd backend && uv run pytest tests/services/test_runtime_work_service.py tests/api/endpoints/test_runtime_work_api.py tests/services/test_runtime_rpc_service.py tests/schemas/test_user_schema.py tests/api/test_user_preferences_api.py tests/services/adapters/test_task_archive_operations.py -qcd executor && uv run pytest tests/runtime_work/test_local_task_store.py -qcd shared && uv run pytest -qSummary by CodeRabbit
Release Notes
New Features
UI & API Changes
Documentation