Sync Claude task tools to workspace todos - #8965
Open
austinywang wants to merge 118 commits into
Open
austinywang wants to merge 118 commits into
austinywang wants to merge 118 commits into
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:
📝 WalkthroughWalkthroughClaude task tools now trigger asynchronous synchronization. The implementation loads bounded task snapshots, resolves personal and team ownership, publishes Feed data, and reconciles workspace todos with durable ownership and cleanup state. ChangesClaude task synchronization
Estimated code review effort: 5 (Critical) | ~120 minutes Sequence Diagram(s)sequenceDiagram
participant Claude
participant ClaudeWrapper
participant CMUXCLI
participant TaskStore
participant Feed
participant WorkspaceTodo
Claude->>ClaudeWrapper: emit TaskCreate, TaskUpdate, TaskGet, TaskList, or TeamDelete
ClaudeWrapper->>CMUXCLI: invoke hooks claude task-sync asynchronously
CMUXCLI->>TaskStore: resolve and load task snapshot under deadline and lock
TaskStore-->>CMUXCLI: return tasks and ownership state
CMUXCLI->>Feed: publish TodoWrite task snapshot
CMUXCLI->>WorkspaceTodo: reconcile owner-scoped checklist
WorkspaceTodo-->>CMUXCLI: return per-workspace results
CMUXCLI-->>ClaudeWrapper: acknowledge task-sync hook
Suggested reviewers: Important Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional. ❌ Failed checks (2 errors, 1 warning)
✅ Passed checks (22 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 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 |
Contributor
Greptile SummaryAdds Claude task-store synchronization into Feed and owner-scoped workspace todos.
Confidence Score: 4/5The PR does not appear safe to merge until task synchronization prevents an accepted Feed update from diverging from a rejected workspace reconciliation. The current delivery path acknowledges the new Feed snapshot before attempting atomic workspace reconciliation; when the combined checklist exceeds its cap or another reconciliation error occurs, the workspace preserves its previous rows and the hook provides no rollback for the already-committed Feed state. Files Needing Attention: CLI/CMUXCLI+ClaudeTaskSync.swift Important Files Changed
Reviews (13): Last reviewed commit: "Merge remote-tracking branch 'origin/mai..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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 `@CLI/CMUXCLI`+ClaudeTaskSync.swift:
- Around line 74-91: Align the Feed telemetry payload with the workspace
checklist limit by applying the existing 50-item bound when building
feedSnapshot in the task-sync flow. Update the todos mapping used by
sendFeedTelemetry while preserving the existing claudeTaskFeedDictionary
transformation and checklist behavior.
In `@cmuxTests/ClaudeHookLiveDeliveryTargetTestSupport.swift`:
- Around line 151-177: Update the test support around mutationSeen and the
mock-server handler to track feed.push and workspace.todo.set deliveries
separately. Wait for both method-specific events before asserting completion,
ensuring the test cannot finish after only the Feed destination has been
delivered.
🪄 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: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: ee37fe9f-e011-43de-8a6a-05d49b3daf7d
📒 Files selected for processing (15)
.github/workflows/ci.ymlCLI/CMUXCLI+ClaudeTaskSync.swiftCLI/FeedEventClassifier.swiftCLI/cmux.swiftPackages/macOS/CMUXAgentLaunch/Sources/CMUXAgentLaunch/ClaudeTaskSnapshotLoader.swiftPackages/macOS/CMUXAgentLaunch/Sources/CMUXAgentLaunch/Workstream/WorkstreamPayload.swiftPackages/macOS/CMUXAgentLaunch/Sources/CMUXAgentLaunch/Workstream/WorkstreamStore.swiftPackages/macOS/CMUXAgentLaunch/Tests/CMUXAgentLaunchTests/ClaudeTaskSnapshotLoaderTests.swiftPackages/macOS/CMUXAgentLaunch/Tests/CMUXAgentLaunchTests/Workstream/WorkstreamStoreTests.swiftResources/bin/cmux-claude-wrappercmux.xcodeproj/project.pbxprojcmuxTests/ClaudeHookLiveDeliveryTargetTestSupport.swiftcmuxTests/ClaudeTaskSyncHookTests.swiftcmuxTests/FeedEventClassificationTests.swifttests/test_claude_wrapper_hooks.py
Contributor
Author
|
Review disposition for Greptile’s summary-only >50 concern: consciously rejected for this PR. The 50-item ceiling is the existing product-wide |
This was referenced Aug 3, 2026
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@Packages/macOS/CmuxControlSocket/Sources/CmuxControlSocket/Coordinator/WorkspaceTodo/ControlCommandCoordinator`+WorkspaceTodoSetOpen.swift:
- Around line 121-126: Update the owner_id validation in the WorkspaceTodo
control command coordinator to obtain both caller-visible error messages from
the relevant control-context string structure instead of English literals. Add
the new strings to that context and resolve them at the app boundary with
String(localized:defaultValue:), while ensuring this package does not call
String(localized:) directly.
In
`@Packages/macOS/CmuxWorkspaces/Tests/CmuxWorkspacesTests/Values/WorkspaceChecklistReconciliationTests.swift`:
- Around line 6-57: Add a focused test alongside
reconcileReplacesOnlyTheMatchingOwner that directly calls
replaceChecklist(with:) on a checklist containing an existing owned item, then
asserts the replacement preserves that item’s ownerID. Avoid using
reconcileChecklist in this test so owner preservation is verified by
replaceChecklist itself.
🪄 Autofix
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: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: a87ba8b8-c106-4410-bdeb-268351a9a28c
📒 Files selected for processing (27)
.github/workflows/ci.ymlCLI/CMUXCLI+ClaudeTaskSync.swiftPackages/macOS/CMUXAgentLaunch/Sources/CMUXAgentLaunch/ClaudeTaskRecord.swiftPackages/macOS/CMUXAgentLaunch/Sources/CMUXAgentLaunch/ClaudeTaskRootResolver.swiftPackages/macOS/CMUXAgentLaunch/Sources/CMUXAgentLaunch/ClaudeTaskSnapshotLoader.swiftPackages/macOS/CMUXAgentLaunch/Sources/CMUXAgentLaunch/ClaudeTaskSnapshotLoaderError.swiftPackages/macOS/CMUXAgentLaunch/Sources/CMUXAgentLaunch/Workstream/WorkstreamPayload.swiftPackages/macOS/CMUXAgentLaunch/Tests/CMUXAgentLaunchTests/ClaudeTaskSnapshotLoaderTests.swiftPackages/macOS/CmuxControlSocket/Sources/CmuxControlSocket/Coordinator/WorkspaceTodo/ControlCommandCoordinator+WorkspaceTodo.swiftPackages/macOS/CmuxControlSocket/Sources/CmuxControlSocket/Coordinator/WorkspaceTodo/ControlCommandCoordinator+WorkspaceTodoSetOpen.swiftPackages/macOS/CmuxControlSocket/Sources/CmuxControlSocket/Coordinator/WorkspaceTodo/ControlWorkspaceTodoContext.swiftPackages/macOS/CmuxControlSocket/Sources/CmuxControlSocket/Coordinator/WorkspaceTodo/ControlWorkspaceTodoSetResolution.swiftPackages/macOS/CmuxControlSocket/Tests/CmuxControlSocketTests/ControlCommandCoordinatorWorkspaceTodoSetOpenTests.swiftPackages/macOS/CmuxControlSocket/Tests/CmuxControlSocketTests/ControlCommandCoordinatorWorkspaceTodoTests.swiftPackages/macOS/CmuxControlSocket/Tests/CmuxControlSocketTests/ControlWorkspaceTodoContextTestStubs.swiftPackages/macOS/CmuxWorkspaces/Sources/CmuxWorkspaces/Values/WorkspaceChecklistItem.swiftPackages/macOS/CmuxWorkspaces/Sources/CmuxWorkspaces/Values/WorkspaceChecklistReconciliation.swiftPackages/macOS/CmuxWorkspaces/Sources/CmuxWorkspaces/Values/WorkspaceChecklistReplacement.swiftPackages/macOS/CmuxWorkspaces/Tests/CmuxWorkspacesTests/Values/WorkspaceChecklistReconciliationTests.swiftSources/SessionPersistence+Todos.swiftSources/TerminalController+ControlWorkspaceTodoBatchMutation.swiftSources/TerminalController+ControlWorkspaceTodoContext.swiftSources/Workspace+Todos.swiftcmux.xcodeproj/project.pbxprojcmuxTests/ClaudeHookLiveDeliveryTargetTestSupport.swiftcmuxTests/ClaudeTaskSyncHookTests.swiftcmuxTests/WorkspaceTodoSnapshotTests.swift
|
Note GitHub couldn't provide a complete incremental comparison for this pull request, so CodeRabbit is performing a full review instead. This review may take a little longer. |
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
Packages/macOS/CmuxWorkspaces/Sources/CmuxWorkspaces/Values/WorkspaceChecklistReplacement.swift (1)
75-75: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winReject duplicate existing checklist IDs before building the dictionary.
Dictionary(uniqueKeysWithValues:)traps for duplicate keys.restoredChecklistpreserves stored item IDs without uniqueness validation, so a malformed restored checklist can crash before returning the atomic rejection already covered for replacement IDs.Add existing-ID uniqueness validation before this initializer and add a regression test for a persisted duplicate owned 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 `@Packages/macOS/CmuxWorkspaces/Sources/CmuxWorkspaces/Values/WorkspaceChecklistReplacement.swift` at line 75, Validate that the existing checklist items have unique IDs before constructing existingById with Dictionary(uniqueKeysWithValues:). If duplicate owned IDs are found, reject the replacement atomically using the existing failure path rather than allowing the initializer to trap; add a regression test covering a persisted checklist with duplicate owned IDs.
🤖 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 `@CLI/CMUXCLI`+ClaudeTaskSync.swift:
- Around line 202-204: Update claudeTaskChecklistOwnerID(sessionID:) to ensure
the generated “claude:” owner ID is at most 500 characters, enforcing the
493-character sessionID limit before checklist reconciliation. Preserve valid
identifiers and ensure overlong values are rejected or bounded before Feed
telemetry and workspace.todo.reconcile are invoked.
In
`@Packages/macOS/CMUXAgentLaunch/Sources/CMUXAgentLaunch/ClaudeTaskSnapshotLoader.swift`:
- Around line 124-132: Validate the filename-to-task-ID binding in the
snapshot-loading loop before appending a todo: require record.id to be a valid
path component and require the JSON filename stem to equal record.id. Reuse the
same identity-validation behavior as taskFile(in:matches:decoder:), then
continue for invalid or mismatched files while preserving the existing
canonicalState and nonEmptyTaskText checks.
In `@Resources/Localizable.xcstrings`:
- Around line 234342-234375: The new localization keys
socket.workspace.todo.reconcile.invalidOwnerIDLength and
socket.workspace.todo.reconcile.missingOwnerID currently include only en and ja;
add translated stringUnit entries for every other locale already present in
Resources/Localizable.xcstrings, preserving the existing messages and translated
state.
---
Outside diff comments:
In
`@Packages/macOS/CmuxWorkspaces/Sources/CmuxWorkspaces/Values/WorkspaceChecklistReplacement.swift`:
- Line 75: Validate that the existing checklist items have unique IDs before
constructing existingById with Dictionary(uniqueKeysWithValues:). If duplicate
owned IDs are found, reject the replacement atomically using the existing
failure path rather than allowing the initializer to trap; add a regression test
covering a persisted checklist with duplicate owned IDs.
🪄 Autofix
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: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 501e949c-d968-4ad0-aad0-abb322356007
📒 Files selected for processing (37)
.github/workflows/ci.ymlCLI/CMUXCLI+ClaudeTaskSync.swiftCLI/cmux.swiftPackages/macOS/CMUXAgentLaunch/Sources/CMUXAgentLaunch/ClaudeTaskIdentity.swiftPackages/macOS/CMUXAgentLaunch/Sources/CMUXAgentLaunch/ClaudeTaskRecord.swiftPackages/macOS/CMUXAgentLaunch/Sources/CMUXAgentLaunch/ClaudeTaskRootResolver.swiftPackages/macOS/CMUXAgentLaunch/Sources/CMUXAgentLaunch/ClaudeTaskSnapshot.swiftPackages/macOS/CMUXAgentLaunch/Sources/CMUXAgentLaunch/ClaudeTaskSnapshotLoader.swiftPackages/macOS/CMUXAgentLaunch/Sources/CMUXAgentLaunch/ClaudeTaskSnapshotLoaderError.swiftPackages/macOS/CMUXAgentLaunch/Sources/CMUXAgentLaunch/Workstream/WorkstreamPayload.swiftPackages/macOS/CMUXAgentLaunch/Sources/CMUXAgentLaunch/Workstream/WorkstreamStore.swiftPackages/macOS/CMUXAgentLaunch/Tests/CMUXAgentLaunchTests/ClaudeTaskSnapshotLoaderTests.swiftPackages/macOS/CMUXAgentLaunch/Tests/CMUXAgentLaunchTests/Workstream/WorkstreamStoreTests.swiftPackages/macOS/CmuxControlSocket/Sources/CmuxControlSocket/Coordinator/WorkspaceTodo/ControlCommandCoordinator+WorkspaceTodo.swiftPackages/macOS/CmuxControlSocket/Sources/CmuxControlSocket/Coordinator/WorkspaceTodo/ControlCommandCoordinator+WorkspaceTodoSetOpen.swiftPackages/macOS/CmuxControlSocket/Sources/CmuxControlSocket/Coordinator/WorkspaceTodo/ControlWorkspaceTodoContext.swiftPackages/macOS/CmuxControlSocket/Sources/CmuxControlSocket/Coordinator/WorkspaceTodo/ControlWorkspaceTodoSetResolution.swiftPackages/macOS/CmuxControlSocket/Sources/CmuxControlSocket/Coordinator/WorkspaceTodo/ControlWorkspaceTodoStrings.swiftPackages/macOS/CmuxControlSocket/Tests/CmuxControlSocketTests/ControlCommandCoordinatorWorkspaceTodoSetOpenTests.swiftPackages/macOS/CmuxControlSocket/Tests/CmuxControlSocketTests/ControlCommandCoordinatorWorkspaceTodoTests.swiftPackages/macOS/CmuxControlSocket/Tests/CmuxControlSocketTests/ControlWorkspaceTodoContextTestStubs.swiftPackages/macOS/CmuxWorkspaces/Sources/CmuxWorkspaces/Values/WorkspaceChecklistItem.swiftPackages/macOS/CmuxWorkspaces/Sources/CmuxWorkspaces/Values/WorkspaceChecklistReconciliation.swiftPackages/macOS/CmuxWorkspaces/Sources/CmuxWorkspaces/Values/WorkspaceChecklistReplacement.swiftPackages/macOS/CmuxWorkspaces/Tests/CmuxWorkspacesTests/Session/RestoredPanelTitleBoundaryTests.swiftPackages/macOS/CmuxWorkspaces/Tests/CmuxWorkspacesTests/Values/WorkspaceChecklistReconciliationTests.swiftResources/Localizable.xcstringsResources/bin/cmux-claude-wrapperSources/SessionPersistence+Todos.swiftSources/TerminalController+ControlWorkspaceTodoBatchMutation.swiftSources/TerminalController+ControlWorkspaceTodoContext.swiftSources/Workspace+Todos.swiftcmux.xcodeproj/project.pbxprojcmuxTests/ClaudeHookLiveDeliveryTargetTestSupport.swiftcmuxTests/ClaudeTaskSyncHookTests.swiftcmuxTests/WorkspaceTodoSnapshotTests.swifttests/test_claude_wrapper_hooks.py
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
Contributor
Author
|
Review follow-up: 1e23136 also fixes CodeRabbit’s outside-diff duplicate-persisted-ID finding. On the test-only HEAD, WorkspaceChecklistReconciliationTests crashed with “Fatal error: Duplicate values for key”; replaceChecklist now rejects the corrupted persisted identity set atomically with duplicateId, and the focused reconciliation suite passes 7/7. |
There was a problem hiding this comment.
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 `@Resources/Localizable.xcstrings`:
- Around line 234578-234588: Update the zh-Hans and zh-Hant stringUnit values
for the invalid owner_id message to use the natural wording “owner_id 缺失或无效” and
“owner_id 缺失或無效”, respectively, while leaving their translated states unchanged.
🪄 Autofix
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: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: c680074d-d2d4-46ff-b81c-fd5d14b2c800
📒 Files selected for processing (7)
CLI/CMUXCLI+ClaudeTaskSync.swiftPackages/macOS/CMUXAgentLaunch/Sources/CMUXAgentLaunch/ClaudeTaskSnapshotLoader.swiftPackages/macOS/CMUXAgentLaunch/Tests/CMUXAgentLaunchTests/ClaudeTaskSnapshotLoaderTests.swiftPackages/macOS/CmuxWorkspaces/Sources/CmuxWorkspaces/Values/WorkspaceChecklistReplacement.swiftPackages/macOS/CmuxWorkspaces/Tests/CmuxWorkspacesTests/Values/WorkspaceChecklistReconciliationTests.swiftResources/Localizable.xcstringscmuxTests/ClaudeTaskSyncHookTests.swift
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using default effort and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit d368660. Configure here.
Contributor
Author
|
Review-thread audit re-checked at HEAD All 55 review threads have an explicit Austin reply; unresolved: 0; unanswered: 0.
|
This branch was successfully deployed
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
2 participants
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
Testing
/usr/bin/arch -arm64 swift testinPackages/macOS/CMUXAgentLaunch(253 tests)python3 tests/test_claude_wrapper_hooks.py./scripts/lint-pbxproj-test-wiring.sh./scripts/check-pbxproj.shCloses #8960
Cross-process synchronization rationale
Claude Code starts each asynchronous task hook as an independent CLI process. A Swift actor can serialize state only inside one process, so it cannot prevent an older hook process from publishing after a newer one. The task-sync path therefore uses a portable macOS
lockf(1)file-command lease beside the durable hook-state store (/usr/bin/lockf -k -s -t ...) to cover exactly one authoritative snapshot read plus its Feed and workspace deliveries. The scan is bounded (512 root entries, 512 task entries, and 64 KiB per task file), there is no sleep or polling, and closing the lease pipe releases the lock if a hook process exits. This is cross-process transaction ordering, not ongoing in-process mutable state.Need help on this PR? Tag
@codesmith-botwith what you need. Autofix is disabled.Summary by cubic
For #8960, Claude task hooks now reconcile authoritative task snapshots into Feed and workspace todos instead of leaving workspace checklists empty. Task updates stay live-only in Feed (no longer appended to disk history), and reconciliation replaces only items owned by that task list.
Details
PostToolUsetask events andTaskCompletedfor session, configured, and automatic-team lists across Claude profiles and relay scopes.activeForm, and bounds filesystem scans, file sizes, and snapshot text.workspace.todo.reconcileto replace one owner's items while preserving unrelated todos, IDs, and attachments.CMUXAgentLaunch, and bundled-wrapper coverage to CI.Migration
workspace.todo.reconcilemust provide a non-emptyowner_idbetween 1 and 500 characters.Written for commit ec37648. Summary will update on new commits.
Summary by CodeRabbit
New Features
Bug Fixes
Documentation
Note
Medium Risk
Large new cross-process hook and durable-state machinery mutates workspace todos and Feed; mitigated by deadlines, coalescing, and fail-closed rollback, but edge cases around team delete and owner replacement remain sensitive.
Overview
Adds a
claude-hook task-syncpipeline that reads Claude’s on-disk task store and keeps Feed and workspace checklists aligned viaworkspace.todo.reconcile, with a validate-only pass before Feed so capacity rejections cannot diverge the two views.The work is split into orchestrated paths for configured lists, automatic team bindings, personal/session lists, and
TeamDelete, backed by new session-store state: scoped coalescing claims, cross-process task-sync locks, destination/team proofs, list retirement, and bounded legacy-owner cleanup with rollback when delivery fails.Hook routing now probes live surface/workspace identity when a pane may have moved workspaces, instead of failing closed on a stale recorded surface alone.
CI adds focused regressions for automatic team sync, task-sync hooks, and workspace todo snapshots, and includes
CmuxWorkspacesin the app-host build list.Reviewed by Cursor Bugbot for commit ec37648. Bugbot is set up for automated code reviews on this repo. Configure here.