Repository navigation
fix(wework): keep Codex goal state authoritative - #1886
Conversation
|
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:
📝 WalkthroughWalkthroughAdds root Codex turn completion callbacks and runtime goal continuation events, propagates continuation state through workbench streams and session state, updates goal controls, and changes desktop E2E polling, editor filling, compaction handling, cancellation assertions, and diagnostics. ChangesRuntime goal continuation
Desktop E2E automation
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant CodexAppServer
participant CodexNotificationEventMapper
participant ResponseApiStream
participant WorkbenchPaneSession
participant GoalStatusBar
CodexAppServer->>CodexNotificationEventMapper: send root turn lifecycle notification
CodexNotificationEventMapper->>ResponseApiStream: emit runtime.goal.continuation
ResponseApiStream->>WorkbenchPaneSession: dispatch continuation payload
WorkbenchPaneSession->>WorkbenchPaneSession: derive goalContinuing
WorkbenchPaneSession->>GoalStatusBar: pass continuing state
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ 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 |
…nuation-status # Conflicts: # wework/src/components/layout/DesktopWorkbenchMain.tsx
qdaxb
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
wework/src/components/layout/useWorkbenchPaneSession.ts (2)
414-442: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winGoal-load failure handler can clobber a newer, valid goal.
The
.then()branch correctly guards against stale responses withrequestedGoalRevision === goalRevisionRef.current(line 420), but the.catch()branch commitsnullunconditionally once!cancelled. If this effect'sgetRuntimeGoalcall rejects after a different, more recentcommitThreadGoalalready landed (e.g. from the independentonAssistantSettledrefresh at lines 553-579, which runs concurrently against the same task), this catch will incorrectly wipe out the fresher goal state.🔒 Proposed fix to guard the catch branch with the same revision check
.catch(error => { if (!cancelled) { - commitThreadGoal(null) + if (requestedGoalRevision === goalRevisionRef.current) { + commitThreadGoal(null) + } console.error('[Wework] Runtime goal load failed', { address: runtimeAddressDebug(runtimeTaskLoadTarget.address), error, }) } })🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@wework/src/components/layout/useWorkbenchPaneSession.ts` around lines 414 - 442, Guard the getRuntimeGoal rejection handler in the effect using the same requestedGoalRevision === goalRevisionRef.current check as the success branch before calling commitThreadGoal(null) or logging the failure. Update the .catch() condition so stale failures cannot overwrite a newer goal loaded by concurrent refreshes such as onAssistantSettled.
118-118: 📐 Maintainability & Code Quality | 🟠 Major | 🏗️ Heavy liftFile exceeds the 1000-line guideline threshold.
This hook file runs well past 1000 lines (shown ranges extend to ~1836), and this PR adds further state (
goalContinuation,goalRevisionRef,commitThreadGoal) and effect logic to it. Consider extracting goal-state management (threadGoal/goalContinuation/pendingGoalState/commitThreadGoal and their effects) into a dedicated hook.As per coding guidelines, "Split files that exceed 1000 lines and preferably keep functions at 50 lines or fewer."
🤖 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/useWorkbenchPaneSession.ts` at line 118, Extract the goal-state management from useWorkbenchPaneSession into a dedicated hook, including threadGoal, goalContinuation, pendingGoalState, goalRevisionRef, commitThreadGoal, and their related effects. Keep useWorkbenchPaneSession focused on composing the returned state and actions, and update it to consume the new hook without changing behavior.Source: Coding guidelines
executor/src/agents/codex.rs (1)
1264-1276: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winAvoid re-sending
turn/interruptfor the same turn. Oncecancel_requestedis set, every later root notification with a turn id can trigger anotherinterrupt_shared_turnuntil the turn ends. Sinceactive_root_turn_notification_idnow matches more than justturn/started, this can spam duplicate interrupt RPCs during cancellation. Gate the interrupt on a turn-id change or clear the flag after the first send.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@executor/src/agents/codex.rs` around lines 1264 - 1276, The cancellation branch in the root-notification handling can repeatedly call interrupt_shared_turn for the same turn. Track whether the active turn ID changed before updating options.active_turn_id, and only send the interrupt when cancel_requested is true and the notification introduces a new turn ID; alternatively clear the cancellation flag immediately after the first successful interrupt.
🧹 Nitpick comments (1)
wework/src/components/layout/useWorkbenchPaneSession.ts (1)
1641-1679: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueAdd
commitThreadGoalto both dependency arrays. It’s referenced inupdateCurrentGoalStatusandclearCurrentGoal, and including it keeps the hook deps aligned with the rest of 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 `@wework/src/components/layout/useWorkbenchPaneSession.ts` around lines 1641 - 1679, Include commitThreadGoal in the dependency arrays for both updateCurrentGoalStatus and clearCurrentGoal, alongside their existing dependencies, so each callback accurately declares the values it references.
🤖 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 `@executor/src/runtime_work/events.rs`:
- Line 103: Initialize CodexNotificationEventMapper’s goal_status from the
resumed thread’s current goal state during spawn_turn, rather than always
starting at None. Ensure has_active_goal() reflects an already-active goal
before live thread/goal/updated notifications arrive, allowing
runtime.goal.continuation to emit for resumed turns.
In `@wework/src/components/chat/ChatInput.test.tsx`:
- Around line 2195-2222: Extend the status matrix in the “does not offer the
pause action for a %s goal” test to include the complete status alongside
blocked, usageLimited, and budgetLimited, preserving the existing isolated
parameterized test coverage for all non-toggleable goal states.
- Around line 2169-2193: The ChatInput goal-status tests do not cover an active
goal when goalContinuing is false. Add a separate test near the existing “shows
an active goal as continuing” case using the same active goal fixture, arrange
goalContinuing={false}, render ChatInput, and assert the normal active label is
shown rather than “目标继续执行中”; follow Arrange, Act, Assert structure.
In `@wework/src/components/layout/useWorkbenchPaneSession.ts`:
- Around line 1272-1277: Clear the stale goalContinuation state alongside every
client-initiated goal mutation. In the goal creation flow near
commitThreadGoal(response.goal), and in the pause/resume updateCurrentGoalStatus
flow and manual clear flow, reset goalContinuation immediately after the
successful commit so newly created, resumed, or cleared goals cannot inherit a
prior started continuation state.
In `@wework/src/features/workbench/WorkbenchProvider.test.tsx`:
- Around line 5767-5837: Extend the test “resumes a paused runtime goal when
editing and sending its objective” to retain a reference to the mocked
sendRuntimeMessage function and assert it is called after the edit flow with the
edited objective and the runtime task address. Keep the existing setRuntimeGoal
assertion and place the sendRuntimeMessage expectation after the user clicks
“send runtime goal.”
- Around line 5714-5765: Update the test “keeps an active runtime goal active
while the task list is between automatic turns” to drive an actual
automatic-turn boundary rather than only loading a static idle response. Make
the listRuntimeWork fixture transition through running to idle, or trigger the
relevant continuation event, and assert that runtime-goal-status remains active
after the transition; keep the mock and assertions isolated to this test.
---
Outside diff comments:
In `@executor/src/agents/codex.rs`:
- Around line 1264-1276: The cancellation branch in the root-notification
handling can repeatedly call interrupt_shared_turn for the same turn. Track
whether the active turn ID changed before updating options.active_turn_id, and
only send the interrupt when cancel_requested is true and the notification
introduces a new turn ID; alternatively clear the cancellation flag immediately
after the first successful interrupt.
In `@wework/src/components/layout/useWorkbenchPaneSession.ts`:
- Around line 414-442: Guard the getRuntimeGoal rejection handler in the effect
using the same requestedGoalRevision === goalRevisionRef.current check as the
success branch before calling commitThreadGoal(null) or logging the failure.
Update the .catch() condition so stale failures cannot overwrite a newer goal
loaded by concurrent refreshes such as onAssistantSettled.
- Line 118: Extract the goal-state management from useWorkbenchPaneSession into
a dedicated hook, including threadGoal, goalContinuation, pendingGoalState,
goalRevisionRef, commitThreadGoal, and their related effects. Keep
useWorkbenchPaneSession focused on composing the returned state and actions, and
update it to consume the new hook without changing behavior.
---
Nitpick comments:
In `@wework/src/components/layout/useWorkbenchPaneSession.ts`:
- Around line 1641-1679: Include commitThreadGoal in the dependency arrays for
both updateCurrentGoalStatus and clearCurrentGoal, alongside their existing
dependencies, so each callback accurately declares the values it references.
🪄 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: 5badbdb6-e4d7-4714-a8ab-283a1755670b
📒 Files selected for processing (20)
executor/src/agents/codex.rsexecutor/src/agents/mod.rsexecutor/src/runtime_work/codex_notifications.rsexecutor/src/runtime_work/events.rsexecutor/src/runtime_work/handler.rswework/src/components/chat/ChatInput.test.tsxwework/src/components/chat/ChatInput.tsxwework/src/components/chat/composer/GoalStatusBar.tsxwework/src/components/layout/DesktopWorkbenchMain.tsxwework/src/components/layout/useWorkbenchPaneSession.tswework/src/features/workbench/WorkbenchProvider.test.tsxwework/src/features/workbench/runtimePaneMessages.tswework/src/features/workbench/runtimePaneStatus.test.tswework/src/features/workbench/runtimePaneStatus.tswework/src/i18n/locales/en/common.jsonwework/src/i18n/locales/zh-CN/common.jsonwework/src/stream/chatStream.tswework/src/stream/responseApiStream.test.tswework/src/stream/responseApiStream.tswework/src/types/api.ts
| plan_blocks: BTreeMap<String, String>, | ||
| tool_output_deltas: BTreeMap<String, String>, | ||
| completed_user_message_count: usize, | ||
| goal_status: Option<String>, |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Locate the relevant runtime files and map their structure first.
git ls-files executor/src/runtime_work/events.rs executor/src/runtime_work/handler.rs executor/src/runtime_work/codex.rs
echo '--- outline events.rs ---'
ast-grep outline executor/src/runtime_work/events.rs --view expanded || true
echo '--- outline handler.rs ---'
ast-grep outline executor/src/runtime_work/handler.rs --view expanded || true
echo '--- outline codex.rs ---'
ast-grep outline executor/src/runtime_work/codex.rs --view expanded || trueRepository: wecode-ai/Wegent
Length of output: 27749
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Read the relevant sections around the cited lines and search for goal status flow.
sed -n '1,220p' executor/src/runtime_work/events.rs
echo '--- handler.rs matches ---'
rg -n "notification-mapper|CodexNotificationEventMapper|notification_tx|thread/goal/updated|thread/goal/get|goal_status|has_active_goal|runtime.goal.continuation" executor/src/runtime_work/handler.rs executor/src/runtime_work/codex.rs executor/src/runtime_work/events.rsRepository: wecode-ai/Wegent
Length of output: 10772
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the specific code paths that determine goal_status initialization and updates.
printf '\n== events.rs (selected range) ==\n'
nl -ba executor/src/runtime_work/events.rs | sed -n '1,220p'
printf '\n== handler.rs goal/thread references ==\n'
rg -n -C 3 "CodexNotificationEventMapper|notification_tx|thread/goal/updated|thread/goal/get|goal_status|has_active_goal|runtime\.goal\.continuation|spawn_turn" executor/src/runtime_work/handler.rs executor/src/runtime_work/codex.rs executor/src/runtime_work/events.rsRepository: wecode-ai/Wegent
Length of output: 228
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Read only the relevant code slices.
echo '--- executor/src/runtime_work/events.rs ---'
nl -ba executor/src/runtime_work/events.rs | sed -n '1,220p'
echo '--- executor/src/runtime_work/handler.rs ---'
nl -ba executor/src/runtime_work/handler.rs | sed -n '1,260p'
echo '--- executor/src/runtime_work/codex.rs ---'
nl -ba executor/src/runtime_work/codex.rs | sed -n '1,260p'Repository: wecode-ai/Wegent
Length of output: 238
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the exact control flow around goal-status initialization and continuation emission.
echo '--- executor/src/runtime_work/events.rs ---'
nl -ba executor/src/runtime_work/events.rs | sed -n '1,220p'
echo '--- executor/src/runtime_work/handler.rs ---'
nl -ba executor/src/runtime_work/handler.rs | sed -n '1,260p'
echo '--- executor/src/runtime_work/codex.rs ---'
nl -ba executor/src/runtime_work/codex.rs | sed -n '1,260p'
echo '--- references ---'
rg -n -C 2 "thread/goal/updated|thread/goal/get|goal_status|has_active_goal|runtime\.goal\.continuation|CodexNotificationEventMapper|notification_tx|spawn_turn" executor/src/runtime_workRepository: wecode-ai/Wegent
Length of output: 238
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Find where CodexNotificationEventMapper is constructed and how goal_status is initialized.
rg -n -C 4 "CodexNotificationEventMapper\s*\{|goal_status\s*:\s*None|goal_status\s*:\s*Some|new\(" executor/src/runtime_workRepository: wecode-ai/Wegent
Length of output: 50373
🏁 Script executed:
#!/bin/bash
set -euo pipefail
python3 - <<'PY'
from pathlib import Path
def show(path, start, end):
print(f"\n== {path}:{start}-{end} ==")
lines = Path(path).read_text().splitlines()
for i in range(start, min(end, len(lines)) + 1):
print(f"{i:4d}: {lines[i-1]}")
show("executor/src/runtime_work/events.rs", 300, 375)
show("executor/src/runtime_work/events.rs", 930, 960)
show("executor/src/runtime_work/handler.rs", 1260, 1295)
show("executor/src/runtime_work/handler.rs", 2215, 2570)
PYRepository: wecode-ai/Wegent
Length of output: 23631
🏁 Script executed:
#!/bin/bash
set -euo pipefail
python3 - <<'PY'
from pathlib import Path
for path, ranges in [
("executor/src/runtime_work/events.rs", [(300, 375), (930, 960), (2650, 2755)]),
("executor/src/runtime_work/handler.rs", [(1260, 1295), (2215, 2570)]),
]:
lines = Path(path).read_text().splitlines()
for start, end in ranges:
print(f"\n== {path}:{start}-{end} ==")
for i in range(start, min(end, len(lines)) + 1):
print(f"{i:4d}: {lines[i-1]}")
PYRepository: wecode-ai/Wegent
Length of output: 27915
🏁 Script executed:
#!/bin/bash
set -euo pipefail
python3 - <<'PY'
from pathlib import Path
def show(path, start, end):
lines = Path(path).read_text().splitlines()
print(f"\n== {path}:{start}-{end} ==")
for i in range(start, min(end, len(lines)) + 1):
print(f"{i:4d}: {lines[i-1]}")
show("executor/src/runtime_work/handler.rs", 2162, 2315)
show("executor/src/runtime_work/handler.rs", 1848, 1915)
PYRepository: wecode-ai/Wegent
Length of output: 10816
Seed goal_status for resumed turns — CodexNotificationEventMapper starts with goal_status = None on every spawn_turn, and only a live thread/goal/updated notification sets it. If a resumed thread is already active when this mapper is created, has_active_goal() stays false and runtime.goal.continuation won’t emit for that turn.
🤖 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/src/runtime_work/events.rs` at line 103, Initialize
CodexNotificationEventMapper’s goal_status from the resumed thread’s current
goal state during spawn_turn, rather than always starting at None. Ensure
has_active_goal() reflects an already-active goal before live
thread/goal/updated notifications arrive, allowing runtime.goal.continuation to
emit for resumed turns.
| test('shows an active goal as continuing only while a new goal turn is running', () => { | ||
| render( | ||
| <ChatInput | ||
| value="" | ||
| onChange={vi.fn()} | ||
| onSubmit={vi.fn()} | ||
| disabled={false} | ||
| variant="desktop" | ||
| goalContinuing | ||
| goal={{ | ||
| threadId: 'thread-1', | ||
| objective: '继续完成测试', | ||
| status: 'active', | ||
| tokenBudget: null, | ||
| tokensUsed: 0, | ||
| timeUsedSeconds: 0, | ||
| createdAt: 1780000000000, | ||
| updatedAt: 1780000000000, | ||
| }} | ||
| /> | ||
| ) | ||
|
|
||
| expect(screen.getByTestId('goal-status-bar')).toHaveTextContent('目标继续执行中') | ||
| expect(screen.getByTestId('pause-goal-button')).toBeInTheDocument() | ||
| }) |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Cover the non-continuing active state.
This only asserts goalContinuing=true; it never verifies that an active goal uses the normal active label when the flag is false. Add a false-path case so the “only while” behavior cannot regress unnoticed.
As per coding guidelines, tests should follow Arrange, Act, Assert and cover edge cases.
🤖 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/chat/ChatInput.test.tsx` around lines 2169 - 2193, The
ChatInput goal-status tests do not cover an active goal when goalContinuing is
false. Add a separate test near the existing “shows an active goal as
continuing” case using the same active goal fixture, arrange
goalContinuing={false}, render ChatInput, and assert the normal active label is
shown rather than “目标继续执行中”; follow Arrange, Act, Assert structure.
Source: Coding guidelines
| test.each(['blocked', 'usageLimited', 'budgetLimited'] as const)( | ||
| 'does not offer the pause action for a %s goal', | ||
| status => { | ||
| render( | ||
| <ChatInput | ||
| value="" | ||
| onChange={vi.fn()} | ||
| onSubmit={vi.fn()} | ||
| disabled={false} | ||
| variant="desktop" | ||
| goal={{ | ||
| threadId: 'thread-1', | ||
| objective: 'Resolve the issue', | ||
| status, | ||
| tokenBudget: null, | ||
| tokensUsed: 0, | ||
| timeUsedSeconds: 0, | ||
| createdAt: 1780000000000, | ||
| updatedAt: 1780000000000, | ||
| }} | ||
| /> | ||
| ) | ||
|
|
||
| expect(screen.queryByTestId('pause-goal-button')).not.toBeInTheDocument() | ||
| expect(screen.queryByTestId('resume-goal-button')).not.toBeInTheDocument() | ||
| } | ||
| ) | ||
|
|
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Include complete in the disallowed-status matrix.
GoalStatusBar suppresses toggles for complete goals too, but this table omits that status. Add it to cover every non-toggleable goal state.
As per coding guidelines, tests should cover edge cases and remain isolated.
🤖 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/chat/ChatInput.test.tsx` around lines 2195 - 2222,
Extend the status matrix in the “does not offer the pause action for a %s goal”
test to include the complete status alongside blocked, usageLimited, and
budgetLimited, preserving the existing isolated parameterized test coverage for
all non-toggleable goal states.
Source: Coding guidelines
| setError(response.error || i18n.t('workbench.goal_set_failed')) | ||
| return | ||
| } | ||
| setThreadGoal(response.goal) | ||
| commitThreadGoal(response.goal) | ||
| setGoalDraftActive(false) | ||
| const queuedMessage: RuntimePaneQueuedMessage = { |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Stale goalContinuation not cleared on manual goal mutations.
goalContinuation is reset on task-target change, onAssistantStart, and the two server-pushed goal events (onRuntimeGoalUpdated/onRuntimeGoalCleared), but not on the three client-initiated commitThreadGoal calls: goal creation (1275), pause/resume via updateCurrentGoalStatus (1667), and manual clear (1726). Since goalContinuing is derived as goal?.status === 'active' && goalContinuation?.status === 'started', a leftover {status:'started'} from a prior turn can make a freshly created or just-resumed goal appear as "continuing" until the next assistant-start or goal event happens to clear it.
🧹 Proposed fix to reset goalContinuation alongside these commits
commitThreadGoal(response.goal)
+ setGoalContinuation(null)
setGoalDraftActive(false) if (!response.accepted) return false
commitThreadGoal(response.goal)
+ setGoalContinuation(null)
return true const response = await clearRuntimeGoal(currentRuntimeTask)
if (!response.accepted) return false
commitThreadGoal(null)
+ setGoalContinuation(null)
return trueAlso applies to: 1660-1669, 1715-1728
🤖 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/useWorkbenchPaneSession.ts` around lines 1272 -
1277, Clear the stale goalContinuation state alongside every client-initiated
goal mutation. In the goal creation flow near commitThreadGoal(response.goal),
and in the pause/resume updateCurrentGoalStatus flow and manual clear flow,
reset goalContinuation immediately after the successful commit so newly created,
resumed, or cleared goals cannot inherit a prior started continuation state.
| test('keeps an active runtime goal active while the task list is between automatic turns', async () => { | ||
| const runtimeWorkApi = createRuntimeWorkApiMock({ | ||
| listRuntimeWork: vi.fn().mockResolvedValue( | ||
| createRuntimeWork({ | ||
| projects: [ | ||
| { | ||
| project: { id: 7, name: 'Wegent' }, | ||
| deviceWorkspaces: [ | ||
| { | ||
| id: 22, | ||
| projectId: 7, | ||
| deviceId: 'device-1', | ||
| deviceName: 'Project Device', | ||
| deviceStatus: 'online', | ||
| workspacePath: '/workspace/project-alpha', | ||
| mapped: true, | ||
| available: true, | ||
| tasks: [ | ||
| { | ||
| taskId: 'runtime-a', | ||
| workspacePath: '/workspace/project-alpha', | ||
| title: 'Runtime A', | ||
| runtime: 'codex', | ||
| running: false, | ||
| status: 'idle', | ||
| }, | ||
| ], | ||
| }, | ||
| ], | ||
| }, | ||
| ], | ||
| totalTasks: 1, | ||
| }) | ||
| ), | ||
| getRuntimeGoal: vi.fn().mockResolvedValue({ | ||
| accepted: true, | ||
| goal: createRuntimeGoal({ status: 'active' }), | ||
| }), | ||
| }) | ||
| const services = createWorkbenchServices({ | ||
| runtimeWorkApi: runtimeWorkApi as WorkbenchServices['runtimeWorkApi'], | ||
| }) | ||
|
|
||
| renderWorkbench(<RuntimeOpenProbe />, services) | ||
|
|
||
| await userEvent.click(await screen.findByText('open runtime a')) | ||
|
|
||
| await waitFor(() => | ||
| expect(screen.getByTestId('current-runtime-task-running')).toHaveTextContent('idle') | ||
| ) | ||
| expect(screen.getByTestId('runtime-goal-status')).toHaveTextContent('active') | ||
| }) |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Exercise the automatic-turn boundary, not just the initial idle state.
listRuntimeWork returns one static idle response and no subsequent refresh or continuation event is driven. This proves only that an initially active goal loads, not that it remains active across automatic turns. Make the fixture transition through running → idle (or emit the continuation event) and assert the goal status across that transition.
As per coding guidelines, tests should cover edge cases and remain isolated.
🤖 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.test.tsx` around lines 5714 -
5765, Update the test “keeps an active runtime goal active while the task list
is between automatic turns” to drive an actual automatic-turn boundary rather
than only loading a static idle response. Make the listRuntimeWork fixture
transition through running to idle, or trigger the relevant continuation event,
and assert that runtime-goal-status remains active after the transition; keep
the mock and assertions isolated to this test.
Source: Coding guidelines
| test('resumes a paused runtime goal when editing and sending its objective', async () => { | ||
| const setRuntimeGoal = vi.fn().mockResolvedValue({ | ||
| accepted: true, | ||
| goal: createRuntimeGoal({ objective: '更新后的目标', status: 'active' }), | ||
| }) | ||
| const runtimeWorkApi = createRuntimeWorkApiMock({ | ||
| listRuntimeWork: vi.fn().mockResolvedValue( | ||
| createRuntimeWork({ | ||
| projects: [ | ||
| { | ||
| project: { id: 7, name: 'Wegent' }, | ||
| deviceWorkspaces: [ | ||
| { | ||
| id: 22, | ||
| projectId: 7, | ||
| deviceId: 'device-1', | ||
| deviceName: 'Project Device', | ||
| deviceStatus: 'online', | ||
| workspacePath: '/workspace/project-alpha', | ||
| mapped: true, | ||
| available: true, | ||
| tasks: [ | ||
| { | ||
| taskId: 'runtime-a', | ||
| workspacePath: '/workspace/project-alpha', | ||
| title: 'Runtime A', | ||
| runtime: 'codex', | ||
| running: false, | ||
| }, | ||
| ], | ||
| }, | ||
| ], | ||
| }, | ||
| ], | ||
| totalTasks: 1, | ||
| }) | ||
| ), | ||
| getRuntimeGoal: vi.fn().mockResolvedValue({ | ||
| accepted: true, | ||
| goal: createRuntimeGoal({ status: 'paused' }), | ||
| }), | ||
| setRuntimeGoal, | ||
| sendRuntimeMessage: vi.fn().mockResolvedValue({ accepted: true, taskId: 'runtime-a' }), | ||
| }) | ||
| const services = createWorkbenchServices({ | ||
| runtimeWorkApi: runtimeWorkApi as WorkbenchServices['runtimeWorkApi'], | ||
| }) | ||
|
|
||
| renderWorkbench(<RuntimeOpenProbe />, services) | ||
|
|
||
| await userEvent.click(await screen.findByText('open runtime a')) | ||
| await waitFor(() => | ||
| expect(screen.getByTestId('runtime-goal-status')).toHaveTextContent('paused') | ||
| ) | ||
| await userEvent.click(screen.getByText('edit runtime goal')) | ||
| await userEvent.click(screen.getByText('set edited runtime goal')) | ||
| await userEvent.click(screen.getByText('send runtime goal')) | ||
|
|
||
| await waitFor(() => | ||
| expect(setRuntimeGoal).toHaveBeenCalledWith({ | ||
| address: { | ||
| deviceId: 'device-1', | ||
| workspacePath: '/workspace/project-alpha', | ||
| taskId: 'runtime-a', | ||
| }, | ||
| objective: '更新后的目标', | ||
| status: 'active', | ||
| }) | ||
| ) | ||
| expect(screen.getByTestId('runtime-goal-status')).toHaveTextContent('active') | ||
| }) |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Assert that the edited objective is actually sent.
The test verifies setRuntimeGoal, but never checks sendRuntimeMessage. It can therefore pass even if the send step is removed or skipped after the optimistic goal update. Assert that the mocked send API is called after the edit flow.
As per coding guidelines, tests should cover the complete behavior under test.
🤖 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.test.tsx` around lines 5767 -
5837, Extend the test “resumes a paused runtime goal when editing and sending
its objective” to retain a reference to the mocked sendRuntimeMessage function
and assert it is called after the edit flow with the edited objective and the
runtime task address. Keep the existing setRuntimeGoal assertion and place the
sendRuntimeMessage expectation after the user clicks “send runtime goal.”
Source: Coding guidelines
Summary
Keep Wework goal and in-turn guidance state aligned with Codex, including automatic continuation between turns.
Root causes
running: falseinterval as a user-paused goal.turn/started. During automatic continuation, item notifications for the new root turn could arrive while that boundary was unavailable; a template guidance request then sentturn/steerwith the oldexpectedTurnId. Codex correctly rejected it.State-flow coverage
thread/goal/get; revision ordering prevents a delayed response from overwriting a stream event.activegoal authoritative; surface root lifecycle state without inferring pause from task-list idle.turnId; clear it at completion so guidance waits for the next real turn.activeandpausedexpose pause/resume.objectiveand explicitly setstatus: active, resuming a paused goal.paused.Validation
cargo fmt --checkcargo test active_root_turn_notification_uses_item_turn_id_and_ignores_completed_or_child_turnscargo test codex_run_state_ignores_cross_thread_turn_completion --libpnpm exec vitest run src/features/workbench/WorkbenchProvider.test.tsx(89 tests)pnpm typecheckSummary by CodeRabbit
New Features
Bug Fixes
Documentation
Tests