Skip to content

fix(wework): preserve active turn running state - #2398

Merged
qdaxb merged 6 commits into
mainfrom
fix/wework-running-state
Aug 3, 2026
Merged

qdaxb merged 6 commits into
mainfrom
fix/wework-running-state

Conversation

@qdaxb

@qdaxb qdaxb commented Aug 3, 2026 •

Copy link
Copy Markdown
Contributor

Summary

  • key runtime lifecycle start/settled events by turn identity
  • ignore stale settlement events from older turns so a newer streaming turn remains running
  • document the lifecycle invariant and add focused unit coverage
  • extend the real Tauri supervisor lifecycle E2E to replay the stale-event race
  • register supervisor-lifecycle in the CI desktop checkpoint matrix and path classifier

Reproduction

The new desktop E2E fails on the test-only baseline commit b5dee1103: while the supervisor correction assistant message is still streaming, replaying an older turn settlement clears the lifecycle and sidebar running indicator.

Verification

  • pnpm --filter wework test
  • pnpm --filter wework exec eslint ...changed files...
  • pnpm --filter wework exec prettier --check ...changed files...
  • pnpm --filter wework e2e:desktop -- --segment supervisor-lifecycle
  • .github/scripts/test-classify-ci-changes.sh
  • pre-push Wework ESLint, TypeScript, and unit checks

Summary by CodeRabbit

  • Bug Fixes

    • Prevented outdated task completion events from stopping a newer active task.
    • Improved task-running indicators when executor status updates arrive late.
    • Preserved active task state during task renaming and conversation recovery.
    • Improved lifecycle handling across completion, error, and context-compaction flows.
  • Tests

    • Added coverage for stale events, delayed status updates, and supervisor correction workflows.
    • Expanded desktop end-to-end classification for task lifecycle scenarios.

@coderabbitai

coderabbitai Bot commented Aug 3, 2026 •

Copy link
Copy Markdown
Contributor

Review Change Stack

📝 Walkthrough

Walkthrough

Runtime task lifecycle state now tracks turn identity. Stream handlers and providers forward identifiers to lifecycle actions. Reducer logic rejects stale settlements. Desktop E2E tests reproduce the stale-event scenario, and CI classification adds supervisor-lifecycle coverage.

Changes

Runtime lifecycle reconciliation

Layer / File(s) Summary
Turn-aware lifecycle state
wework/src/features/workbench/runtimeTaskLifecycle/*, wework/DESIGN.md
Lifecycle state and events now carry turnId. The reducer preserves newer active turns when it receives stale settlements. Store recovery and rename paths preserve the active turn identifier. Tests cover stale settlements and delayed executor snapshots.
Turn identifier propagation
wework/src/features/workbench/runtimePaneMessages.ts, wework/src/features/workbench/WorkbenchProvider.tsx, wework/src/features/workbench/runtimeTaskLifecycle/RuntimeTaskLifecycleProvider.tsx
Assistant lifecycle callbacks now pass turn identifiers through stream handling, workbench wiring, and E2E lifecycle event handling.
Supervisor lifecycle E2E validation
wework/e2e/desktop/task-flow.e2e.mjs, wework/src/e2e/automation.ts, .github/scripts/*
Supervisor correction responses remain open during streaming so tests can replay stale settlements. Desktop classification and assertions include the new supervisor-lifecycle segment.

Estimated code review effort: 4 (Complex) | ~45 minutes

Possibly related PRs

Suggested labels: ci:all

Sequence Diagram(s)

sequenceDiagram
  participant DesktopE2E
  participant RuntimeTaskLifecycleProvider
  participant RuntimeTaskLifecycleStore
  participant RuntimeTaskLifecycleReducer
  DesktopE2E->>RuntimeTaskLifecycleProvider: dispatch stale turn_settled event
  RuntimeTaskLifecycleProvider->>RuntimeTaskLifecycleStore: call turnSettled with turnId
  RuntimeTaskLifecycleStore->>RuntimeTaskLifecycleReducer: dispatch lifecycle event
  RuntimeTaskLifecycleReducer-->>RuntimeTaskLifecycleStore: retain newer active turn
  RuntimeTaskLifecycleStore-->>DesktopE2E: task remains running
Loading
🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and concisely describes the main change: preserving the active turn running state during stale lifecycle events.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches 💡 1
📝 Generate docstrings 💡
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/wework-running-state

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@qdaxb
qdaxb marked this pull request as ready for review August 3, 2026 17:22
@qdaxb
qdaxb enabled auto-merge August 3, 2026 18:00

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@wework/src/e2e/automation.ts`:
- Around line 762-768: Split the cohesive control-command handling from
executeDesktopControlCommand into a separate module because the automation file
exceeds the 1,000-line guideline. Keep executeDesktopControlCommand as the
dispatcher, move the relevant command cases and their supporting logic together,
and delegate to the extracted module without changing command behavior.
🪄 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 Plus

Run ID: 91f6e554-e575-499f-abef-68ab744bbf05

📥 Commits

Reviewing files that changed from the base of the PR and between 6e82bfb and 84fcf8f.

📒 Files selected for processing (13)
  • .github/scripts/classify-wework-desktop-e2e.sh
  • .github/scripts/test-classify-ci-changes.sh
  • wework/DESIGN.md
  • wework/e2e/desktop/task-flow.e2e.mjs
  • wework/src/e2e/automation.ts
  • wework/src/features/workbench/WorkbenchProvider.tsx
  • wework/src/features/workbench/runtimePaneMessages.ts
  • wework/src/features/workbench/runtimeTaskLifecycle/RuntimeTaskLifecycleProvider.tsx
  • wework/src/features/workbench/runtimeTaskLifecycle/RuntimeTaskLifecycleStore.test.ts
  • wework/src/features/workbench/runtimeTaskLifecycle/RuntimeTaskLifecycleStore.ts
  • wework/src/features/workbench/runtimeTaskLifecycle/RuntimeTaskMachine.ts
  • wework/src/features/workbench/runtimeTaskLifecycle/reducer.ts
  • wework/src/features/workbench/runtimeTaskLifecycle/types.ts

Comment on lines +762 to +768
case 'dispatchRuntimeLifecycleEvent':
window.dispatchEvent(
new CustomEvent('wework:e2e:runtime-task-lifecycle', {
detail: JSON.parse(command.value ?? '{}'),
})
)
return ''

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

📐 Maintainability & Code Quality | 🟠 Major | 🏗️ Heavy lift

Split wework/src/e2e/automation.ts before adding more command cases.

The file has at least 1,116 lines after this change. Extract a cohesive control-command group into a separate module. Keep executeDesktopControlCommand as the dispatcher.

As per coding guidelines, "split files over 1000 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 `@wework/src/e2e/automation.ts` around lines 762 - 768, Split the cohesive
control-command handling from executeDesktopControlCommand into a separate
module because the automation file exceeds the 1,000-line guideline. Keep
executeDesktopControlCommand as the dispatcher, move the relevant command cases
and their supporting logic together, and delegate to the extracted module
without changing command behavior.

Source: Coding guidelines

@qdaxb
qdaxb added this pull request to the merge queue Aug 3, 2026
Merged via the queue into main with commit e6bea4c Aug 3, 2026
54 of 56 checks passed
@qdaxb
qdaxb deleted the fix/wework-running-state branch August 3, 2026 18:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant