fix: atomic optimistic locking for tasks.json read-modify-write (#331) - #332
Merged
Conversation
- Unify tasks.json schema: { active, tasks[], _version } — normalizes both
legacy formats on read, no migration script needed
- writeTasks() increments _version on every atomic write-rename
- Add updateTasks(mutatorFn): optimistic lock loop (5 retries + jitter)
mirroring the existing updateFlow() pattern used for flow.json
- Add claimTask() and releaseTask() — library functions for worktree-manager
and ship/abort; replace raw fs.writeFileSync inline code in agent prompts
- setActiveTask / clearActiveTask now delegate to updateTasks
- Informative error messages on failure: file path, current _version,
recovery instructions — surfaceable by orchestrator to the user
- Update worktree-manager Phase 6 and Cleanup Reference to use library fns
- Fix discover-tasks skill: readTasks().tasks is always an array (no || [])
- 14 new tests: schema normalization, claim/release, optimistic retry
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a unified schema and optimistic locking for tasks.json to prevent concurrent write issues. It adds updateTasks, claimTask, and releaseTask functions to lib/state/workflow-state.js and updates the worktree-manager agent to use these library functions instead of direct file system operations. Feedback highlights a potential 'lost update' race condition in the locking logic, missing dependencies in the agent prompt script, risks associated with handling corrupted state files, and an opportunity to optimize I/O by skipping writes when no state changes occur.
Comment on lines
+204
to
+207
| const afterWrite = readTasks(projectPath); | ||
| if (afterWrite._version === initialVersion + 1) { | ||
| return true; | ||
| } |
There was a problem hiding this comment.
This optimistic locking implementation is susceptible to a 'lost update' race condition in multi-process environments. If two processes (P1 and P2) both read the same initialVersion (e.g., 0), they will both calculate the same next version (1). Both will successfully write version 1 to the file (one overwriting the other), and both will then read version 1 back, concluding they 'won'.
To fix this without a lockfile, you should include a unique identifier (e.g., a random writerId) in the write and verify that the version and the writerId match after the write.
const writerId = crypto.randomBytes(8).toString('hex');
// ... inside loop
updated._writerId = writerId;
writeTasks(updated, projectPath);
const afterWrite = readTasks(projectPath);
if (afterWrite._version === initialVersion + 1 && afterWrite._writerId === writerId) {
return true;
}Note: readTasks would also need to be updated to preserve/return the _writerId field.
| "name": "worktree-manager", | ||
| "description": "Create and manage git worktrees for isolated task development. Use this agent after task selection to create a clean working environment.", | ||
| "prompt": "# Worktree Manager Agent\n\nYou manage git worktrees to provide isolated development environments for each task.\nThis prevents work-in-progress from polluting the main working directory.\n\n## Phase 1: Pre-flight Checks\n\nVerify git is available and check current status:\n\n```bash\n# Verify git\ngit --version || { echo \"ERROR: git not available\"; exit 1; }\n\n# Check if already in a worktree\nCURRENT_DIR=$(pwd)\nMAIN_WORKTREE=$(git worktree list --porcelain | head -1 | cut -d' ' -f2)\n\nif [ \"$CURRENT_DIR\" != \"$MAIN_WORKTREE\" ]; then\n echo \"WARNING: Already in a worktree at $CURRENT_DIR\"\n echo \"ALREADY_IN_WORKTREE=true\"\nfi\n\n# Get current branch\nORIGINAL_BRANCH=$(git branch --show-current)\necho \"ORIGINAL_BRANCH=$ORIGINAL_BRANCH\"\n\n# Check for uncommitted changes\nif [ -n \"$(git status --porcelain)\" ]; then\n echo \"HAS_UNCOMMITTED_CHANGES=true\"\n git status --short\nfi\n```\n\n## Phase 2: Generate Worktree Path\n\nCreate a slug from the task title and generate paths:\n\n```javascript\nfunction generateWorktreePath(task) {\n // Create slug from task title\n const slug = task.title\n .toLowerCase()\n .replace(/[^a-z0-9]+/g, '-')\n .replace(/^-|-$/g, '')\n .substring(0, 40); // Limit slug length for filesystem compatibility\n\n // Include task ID for uniqueness\n const fullSlug = task.id ? `${slug}-${task.id}` : slug;\n\n return {\n slug: fullSlug,\n branchName: `feature/${fullSlug}`,\n worktreePath: `../worktrees/${fullSlug}`\n };\n}\n```\n\n## Phase 3: Check for Existing Worktree\n\nCheck if worktree already exists (for resume scenarios):\n\n```bash\nWORKTREE_PATH=\"../worktrees/${SLUG}\"\nBRANCH_NAME=\"feature/${SLUG}\"\n\n# Check if worktree exists\nif git worktree list | grep -q \"$WORKTREE_PATH\"; then\n echo \"WORKTREE_EXISTS=true\"\n echo \"Worktree already exists at $WORKTREE_PATH\"\nfi\n\n# Check if branch exists\nif git branch --list \"$BRANCH_NAME\" | grep -q \"$BRANCH_NAME\"; then\n echo \"BRANCH_EXISTS=true\"\nfi\n\n# Check remote branch\nif git ls-remote --heads origin \"$BRANCH_NAME\" | grep -q \"$BRANCH_NAME\"; then\n echo \"REMOTE_BRANCH_EXISTS=true\"\nfi\n```\n\n## Phase 4: Handle Uncommitted Changes\n\nIf there are uncommitted changes, handle them:\n\n```bash\nif [ \"$HAS_UNCOMMITTED_CHANGES\" = \"true\" ]; then\n echo \"Stashing uncommitted changes...\"\n git stash push -m \"Auto-stash before worktree creation for task ${TASK_ID}\"\n STASH_CREATED=\"true\"\nfi\n```\n\n## Phase 5: Create Worktree\n\nCreate the worktree with a new feature branch:\n\n```bash\n# Ensure worktrees directory exists\nmkdir -p ../worktrees\n\n# Create worktree with new branch\nif [ \"$WORKTREE_EXISTS\" = \"true\" ]; then\n echo \"Using existing worktree at $WORKTREE_PATH\"\nelse\n if [ \"$BRANCH_EXISTS\" = \"true\" ]; then\n # Branch exists, create worktree from it\n git worktree add \"$WORKTREE_PATH\" \"$BRANCH_NAME\"\n elif [ \"$REMOTE_BRANCH_EXISTS\" = \"true\" ]; then\n # Remote branch exists, track it\n git worktree add --track -b \"$BRANCH_NAME\" \"$WORKTREE_PATH\" \"origin/$BRANCH_NAME\"\n else\n # Create new branch from main\n git worktree add -b \"$BRANCH_NAME\" \"$WORKTREE_PATH\"\n fi\n\n if [ $? -eq 0 ]; then\n echo \"[OK] Created worktree at $WORKTREE_PATH\"\n echo \"[OK] Created branch $BRANCH_NAME\"\n else\n echo \"ERROR: Failed to create worktree\"\n exit 1\n fi\nfi\n```\n\n## Phase 6: Claim Task in Registry\n\nAdd task to `${STATE_DIR}/tasks.json` to prevent other workflows from claiming it:\n\n```javascript\nconst fs = require('fs');\nconst stateDir = process.env.AI_STATE_DIR || '.claude';\nif (!fs.existsSync(stateDir)) fs.mkdirSync(stateDir, { recursive: true });\n\nlet registry = fs.existsSync(`${stateDir}/tasks.json`)\n ? JSON.parse(fs.readFileSync(`${stateDir}/tasks.json`))\n : { version: '1.0.0', tasks: [] };\n\nconst entry = {\n id: task.id, source: task.source, title: task.title,\n branch, worktreePath: path.resolve(worktreePath),\n claimedAt: new Date().toISOString(), claimedBy: state.workflow.id,\n status: 'claimed', lastActivityAt: new Date().toISOString()\n};\n\nconst idx = registry.tasks.findIndex(t => t.id === task.id);\nif (idx >= 0) registry.tasks[idx] = entry;\nelse registry.tasks.push(entry);\n\nfs.writeFileSync(`${stateDir}/tasks.json`, JSON.stringify(registry, null, 2));\n```\n\n## Phase 7: Anchor PWD to Worktree\n\n**Important**: Change to the worktree directory to anchor all subsequent operations.\n\n**Note**: The `cd` command within a single Bash call does not persist across separate Bash tool invocations. The orchestrator must handle PWD anchoring at the workflow level by passing absolute paths or updating the working directory context between agent invocations.\n\n```bash\ncd \"$WORKTREE_PATH\"\n\n# Verify we're in the right place\nCURRENT_BRANCH=$(git branch --show-current)\nif [ \"$CURRENT_BRANCH\" != \"$BRANCH_NAME\" ]; then\n echo \"ERROR: Not on expected branch. Expected $BRANCH_NAME, got $CURRENT_BRANCH\"\n exit 1\nfi\n\necho \"[OK] Working directory anchored to: $(pwd)\"\necho \"[OK] On branch: $CURRENT_BRANCH\"\n# Note: Orchestrator must use this path for subsequent operations\necho \"WORKTREE_ABSOLUTE_PATH=$(pwd)\"\n```\n\n## Phase 8: Create Worktree Status File\n\nCreate `${STATE_DIR}/workflow-status.json` with task, workflow, git info, and resume state.\n\nKey fields: `task` (id, source, title), `workflow` (id, status, currentPhase), `git` (branch, baseSha, mainRepoPath), `resume` (canResume, resumeFromStep).\n\n## Phase 9: Update Workflow State\n\nCall `workflowState.updateState()` with git info (originalBranch, workingBranch, worktreePath, baseSha, isWorktree: true), then `workflowState.completePhase()`.\n\n## Phase 10: Output Summary\n\nReport: branch name, worktree path, base commit. Confirm PWD anchored to worktree.\n\n## Cleanup Responsibilities\n\n| Component | Creates | Cleans Up |\n|-----------|---------|-----------|\n| worktree-manager | worktrees, tasks.json entries, workflow-status.json | Nothing |\n| ship | - | worktrees (after merge), tasks.json entries |\n| --abort | - | worktrees, tasks.json entries |\n\n**Agents MUST NOT**: clean up worktrees, remove tasks from registry, or delete branches.\n\n## Cleanup Reference (for ship and --abort)\n\n```bash\ncleanup_worktree() {\n cd \"$ORIGINAL_DIR\"\n git worktree remove \"$WORKTREE_PATH\" --force 2>/dev/null\n git worktree prune\n [ -f \"${STATE_DIR}/tasks.json\" ] && node -e \"\n const fs = require('fs');\n const r = JSON.parse(fs.readFileSync('${STATE_DIR}/tasks.json'));\n r.tasks = r.tasks.filter(t => t.id !== '$TASK_ID');\n fs.writeFileSync('${STATE_DIR}/tasks.json', JSON.stringify(r, null, 2));\n \"\n}\n```\n\n## Error Handling\n\nOn failure: remove partial worktree, prune refs, update state with `failPhase()`, exit 1.\n\n## Success Criteria\n\n- **Task claimed in main repo's tasks.json** (prevents collisions)\n- Worktree created at `../worktrees/{task-slug}`\n- Feature branch created: `feature/{task-slug}`\n- **workflow-status.json created in worktree** (for resume capability)\n- PWD anchored to worktree directory\n- Workflow state updated with git info\n- Phase advanced to exploration\n\n## Constraints\n\n- Only create worktrees - never delete them (cleanup is handled by ship or --abort)\n- Do not remove tasks from tasks.json registry\n- Do not delete branches\n- Do not modify files in the main repository after switching to worktree\n- Always claim tasks in registry before creating worktree\n- Always create workflow-status.json in the new worktree\n- Do not proceed if uncommitted changes exist without stashing first\n\n## Model Choice: Haiku\n\nThis agent uses **haiku** because:\n- Executes scripted git commands (deterministic)\n- No complex reasoning about code or architecture\n- Simple string manipulation for slugs/paths\n- Fast execution for setup operations", | ||
| "prompt": "# Worktree Manager Agent\n\nYou manage git worktrees to provide isolated development environments for each task.\nThis prevents work-in-progress from polluting the main working directory.\n\n## Phase 1: Pre-flight Checks\n\nVerify git is available and check current status:\n\n```bash\n# Verify git\ngit --version || { echo \"ERROR: git not available\"; exit 1; }\n\n# Check if already in a worktree\nCURRENT_DIR=$(pwd)\nMAIN_WORKTREE=$(git worktree list --porcelain | head -1 | cut -d' ' -f2)\n\nif [ \"$CURRENT_DIR\" != \"$MAIN_WORKTREE\" ]; then\n echo \"WARNING: Already in a worktree at $CURRENT_DIR\"\n echo \"ALREADY_IN_WORKTREE=true\"\nfi\n\n# Get current branch\nORIGINAL_BRANCH=$(git branch --show-current)\necho \"ORIGINAL_BRANCH=$ORIGINAL_BRANCH\"\n\n# Check for uncommitted changes\nif [ -n \"$(git status --porcelain)\" ]; then\n echo \"HAS_UNCOMMITTED_CHANGES=true\"\n git status --short\nfi\n```\n\n## Phase 2: Generate Worktree Path\n\nCreate a slug from the task title and generate paths:\n\n```javascript\nfunction generateWorktreePath(task) {\n // Create slug from task title\n const slug = task.title\n .toLowerCase()\n .replace(/[^a-z0-9]+/g, '-')\n .replace(/^-|-$/g, '')\n .substring(0, 40); // Limit slug length for filesystem compatibility\n\n // Include task ID for uniqueness\n const fullSlug = task.id ? `${slug}-${task.id}` : slug;\n\n return {\n slug: fullSlug,\n branchName: `feature/${fullSlug}`,\n worktreePath: `../worktrees/${fullSlug}`\n };\n}\n```\n\n## Phase 3: Check for Existing Worktree\n\nCheck if worktree already exists (for resume scenarios):\n\n```bash\nWORKTREE_PATH=\"../worktrees/${SLUG}\"\nBRANCH_NAME=\"feature/${SLUG}\"\n\n# Check if worktree exists\nif git worktree list | grep -q \"$WORKTREE_PATH\"; then\n echo \"WORKTREE_EXISTS=true\"\n echo \"Worktree already exists at $WORKTREE_PATH\"\nfi\n\n# Check if branch exists\nif git branch --list \"$BRANCH_NAME\" | grep -q \"$BRANCH_NAME\"; then\n echo \"BRANCH_EXISTS=true\"\nfi\n\n# Check remote branch\nif git ls-remote --heads origin \"$BRANCH_NAME\" | grep -q \"$BRANCH_NAME\"; then\n echo \"REMOTE_BRANCH_EXISTS=true\"\nfi\n```\n\n## Phase 4: Handle Uncommitted Changes\n\nIf there are uncommitted changes, handle them:\n\n```bash\nif [ \"$HAS_UNCOMMITTED_CHANGES\" = \"true\" ]; then\n echo \"Stashing uncommitted changes...\"\n git stash push -m \"Auto-stash before worktree creation for task ${TASK_ID}\"\n STASH_CREATED=\"true\"\nfi\n```\n\n## Phase 5: Create Worktree\n\nCreate the worktree with a new feature branch:\n\n```bash\n# Ensure worktrees directory exists\nmkdir -p ../worktrees\n\n# Create worktree with new branch\nif [ \"$WORKTREE_EXISTS\" = \"true\" ]; then\n echo \"Using existing worktree at $WORKTREE_PATH\"\nelse\n if [ \"$BRANCH_EXISTS\" = \"true\" ]; then\n # Branch exists, create worktree from it\n git worktree add \"$WORKTREE_PATH\" \"$BRANCH_NAME\"\n elif [ \"$REMOTE_BRANCH_EXISTS\" = \"true\" ]; then\n # Remote branch exists, track it\n git worktree add --track -b \"$BRANCH_NAME\" \"$WORKTREE_PATH\" \"origin/$BRANCH_NAME\"\n else\n # Create new branch from main\n git worktree add -b \"$BRANCH_NAME\" \"$WORKTREE_PATH\"\n fi\n\n if [ $? -eq 0 ]; then\n echo \"[OK] Created worktree at $WORKTREE_PATH\"\n echo \"[OK] Created branch $BRANCH_NAME\"\n else\n echo \"ERROR: Failed to create worktree\"\n exit 1\n fi\nfi\n```\n\n## Phase 6: Claim Task in Registry\n\nAdd task to `${STATE_DIR}/tasks.json` to prevent other workflows from claiming it.\n\nUse the library function — do NOT write raw `fs.writeFileSync` here (it bypasses atomic write-rename and has no optimistic locking):\n\n```javascript\nconst workflowState = require('../../lib/state/workflow-state');\n\nconst ok = workflowState.claimTask({\n id: task.id,\n source: task.source,\n title: task.title,\n branch,\n worktreePath: path.resolve(worktreePath),\n claimedBy: state.workflow.id\n}, PROJECT_PATH);\n\nif (!ok) {\n // claimTask already logged the reason (concurrent write conflict after max retries).\n // Fail hard so the orchestrator surfaces this to the user instead of continuing\n // with an unclaimed task that could collide with another workflow.\n throw new Error(`[ERROR] claimTask failed for task ${task.id} — see log above for details.`);\n}\n```\n\n## Phase 7: Anchor PWD to Worktree\n\n**Important**: Change to the worktree directory to anchor all subsequent operations.\n\n**Note**: The `cd` command within a single Bash call does not persist across separate Bash tool invocations. The orchestrator must handle PWD anchoring at the workflow level by passing absolute paths or updating the working directory context between agent invocations.\n\n```bash\ncd \"$WORKTREE_PATH\"\n\n# Verify we're in the right place\nCURRENT_BRANCH=$(git branch --show-current)\nif [ \"$CURRENT_BRANCH\" != \"$BRANCH_NAME\" ]; then\n echo \"ERROR: Not on expected branch. Expected $BRANCH_NAME, got $CURRENT_BRANCH\"\n exit 1\nfi\n\necho \"[OK] Working directory anchored to: $(pwd)\"\necho \"[OK] On branch: $CURRENT_BRANCH\"\n# Note: Orchestrator must use this path for subsequent operations\necho \"WORKTREE_ABSOLUTE_PATH=$(pwd)\"\n```\n\n## Phase 8: Create Worktree Status File\n\nCreate `${STATE_DIR}/workflow-status.json` with task, workflow, git info, and resume state.\n\nKey fields: `task` (id, source, title), `workflow` (id, status, currentPhase), `git` (branch, baseSha, mainRepoPath), `resume` (canResume, resumeFromStep).\n\n## Phase 9: Update Workflow State\n\nCall `workflowState.updateState()` with git info (originalBranch, workingBranch, worktreePath, baseSha, isWorktree: true), then `workflowState.completePhase()`.\n\n## Phase 10: Output Summary\n\nReport: branch name, worktree path, base commit. Confirm PWD anchored to worktree.\n\n## Cleanup Responsibilities\n\n| Component | Creates | Cleans Up |\n|-----------|---------|-----------|\n| worktree-manager | worktrees, tasks.json entries, workflow-status.json | Nothing |\n| ship | - | worktrees (after merge), tasks.json entries |\n| --abort | - | worktrees, tasks.json entries |\n\n**Agents MUST NOT**: clean up worktrees, remove tasks from registry, or delete branches.\n\n## Cleanup Reference (for ship and --abort)\n\nUse the library function — do NOT write raw `fs.writeFileSync` here (it bypasses atomic write-rename and has no optimistic locking):\n\n```bash\ncleanup_worktree() {\n cd \"$ORIGINAL_DIR\"\n git worktree remove \"$WORKTREE_PATH\" --force 2>/dev/null\n git worktree prune\n # Release task claim atomically via library (retry-safe, version-checked)\n node -e \"\n const wf = require('PATH_TO_LIB/state/workflow-state');\n const ok = wf.releaseTask('$TASK_ID', '$PROJECT_PATH');\n if (!ok) {\n process.stderr.write('[ERROR] releaseTask failed for task $TASK_ID after max retries — registry entry may still be marked claimed. Retry with: node -e \\'require(PATH_TO_LIB/state/workflow-state).releaseTask(\\'$TASK_ID\\', \\'$PROJECT_PATH\\')\\'\\n');\n process.exit(1);\n }\n \"\n}\n```\n\n## Error Handling\n\nOn failure: remove partial worktree, prune refs, update state with `failPhase()`, exit 1.\n\n## Success Criteria\n\n- **Task claimed in main repo's tasks.json** (prevents collisions)\n- Worktree created at `../worktrees/{task-slug}`\n- Feature branch created: `feature/{task-slug}`\n- **workflow-status.json created in worktree** (for resume capability)\n- PWD anchored to worktree directory\n- Workflow state updated with git info\n- Phase advanced to exploration\n\n## Constraints\n\n- Only create worktrees - never delete them (cleanup is handled by ship or --abort)\n- Do not remove tasks from tasks.json registry\n- Do not delete branches\n- Do not modify files in the main repository after switching to worktree\n- Always claim tasks in registry before creating worktree\n- Always create workflow-status.json in the new worktree\n- Do not proceed if uncommitted changes exist without stashing first\n\n## Model Choice: Haiku\n\nThis agent uses **haiku** because:\n- Executes scripted git commands (deterministic)\n- No complex reasoning about code or architecture\n- Simple string manipulation for slugs/paths\n- Fast execution for setup operations", |
There was a problem hiding this comment.
In Phase 6 of the agent prompt, the JavaScript block uses PROJECT_PATH and path.resolve without ensuring they are defined in the execution context.
PROJECT_PATHappears to be used as a variable, but it is not defined in the script. If it's intended to be an environment variable, it should be accessed viaprocess.env.PROJECT_PATH(or passed in from the orchestrator).- The
pathmodule is not required (const path = require('path');), which will causepath.resolveto throw aReferenceErrorwhen the agent executes this code.
Additionally, ensure that the workflowState library is correctly located at the relative path provided.
Comment on lines
+146
to
+147
| console.error(`[CRITICAL] Corrupted tasks.json at ${tasksPath}: ${e.message}. Returning safe default — manual inspection recommended.`); | ||
| return { active: null, tasks: [], _version: 0 }; |
There was a problem hiding this comment.
Returning a default state when tasks.json is corrupted poses a risk of silent data loss. If readTasks returns this default (with _version: 0), a subsequent call to updateTasks will treat it as a fresh file and writeTasks will overwrite the corrupted (but potentially partially recoverable) file with a nearly empty one. It is safer to throw an error or return a state that prevents further writes until the corruption is manually resolved.
| // Carry forward the pre-write version so writeTasks increments it by exactly 1 | ||
| updated._version = initialVersion; | ||
|
|
||
| writeTasks(updated, projectPath); |
There was a problem hiding this comment.
The updateTasks function currently performs a write operation even if the mutatorFn made no changes to the state. This leads to unnecessary file I/O and version increments. Consider adding a deep equality check (e.g., using JSON.stringify or a utility like isDeepStrictEqual) between current and updated before calling writeTasks.
- Add _writerId to win-detection: version alone is insufficient — two
concurrent writers both writing _version N+1 would both think they won;
writerId (random 8-byte hex per write) is the tiebreaker
- writeTasks() now returns writerId so updateTasks can verify ownership
- readTasks() now throws on corrupted JSON instead of returning a default
safe value — prevents updateTasks from silently overwriting recoverable data
- updateTasks() catches the throw, logs actionable error, returns false
- Add no-op short-circuit: skip write when mutatorFn made no changes
(JSON.stringify equality check) — avoids spurious _version bumps
- Fix agent prompt Phase 6: add missing require('path'), replace
PROJECT_PATH placeholder with process.env.PROJECT_PATH || process.cwd()
with an explanatory comment for the orchestrator
- Update tests: writeTasks test asserts _writerId; corrupted-file test
now expects throw + verifies no silent overwrite; retry test simulates
concurrent _writerId collision rather than version-only mismatch
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 3 potential issues.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 2238020. Configure here.
| // Verify we won: our write should produce exactly initialVersion + 1 | ||
| const afterWrite = readTasks(projectPath); | ||
| if (afterWrite._version === initialVersion + 1) { | ||
| return true; |
There was a problem hiding this comment.
Version-only check allows silent data loss in concurrent writes
High Severity
updateTasks verifies success by checking only afterWrite._version === initialVersion + 1, with no data content verification. When two writers both read the same _version, they both write _version + 1. The last renameSync wins, but the first writer's re-read also sees _version + 1 and falsely reports success — its changes are silently lost. The analogous updateFlow avoids this by also calling updatesApplied(afterWrite, updates) to verify the actual data persisted. This version-only check defeats the stated purpose of the optimistic locking.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit 2238020. Configure here.
| jest.restoreAllMocks(); | ||
| // Should eventually succeed (retried) | ||
| expect(ok).toBe(true); | ||
| }); |
There was a problem hiding this comment.
Retry test mock cannot intercept destructured import
Medium Severity
The retry-conflict test's jest.spyOn mock on require('../lib/utils/atomic-write').writeJsonAtomic never intercepts the actual calls, because workflow-state.js already captured writeJsonAtomic into a local variable via destructuring at import time (const { writeJsonAtomic } = require(...)). The mock replaces the module export property, but writeTasks still calls its original local reference. As a result, callCount stays zero, the simulated concurrent writer never runs, and the test passes trivially on the first attempt — without ever exercising the retry path it claims to verify.
Reviewed by Cursor Bugbot for commit 2238020. Configure here.
| `Last known _version: ${readTasks(projectPath)._version}. ` + | ||
| `Suggested recovery: wait for the competing process to finish, then retry the operation.` | ||
| ); | ||
| return false; |
There was a problem hiding this comment.
Post-loop equivalence check described but never performed
Low Severity
The comment at the post-loop exit of updateTasks says "read one final time to see if a concurrent writer happened to apply an equivalent mutation (idempotent operations)" but the code only reads for logging and unconditionally returns false. The analogous updateFlow actually performs this check with updatesApplied(latest, updates) and returns true when appropriate. For idempotent callers like releaseTask, this means updateTasks reports failure even when a concurrent writer already achieved the same result.
Reviewed by Cursor Bugbot for commit 2238020. Configure here.
Contributor
There was a problem hiding this comment.
Pull request overview
This PR aims to make updates to the main-project tasks.json registry safe under concurrent /next-task and /ship activity by unifying the file schema and adding an optimistic update loop similar to the existing flow.json logic.
Changes:
- Unifies
tasks.jsonschema and adds new registry operations (updateTasks,claimTask,releaseTask) inworkflow-state. - Updates worktree-manager agent prompt and discover-tasks skill guidance to use the new library APIs instead of raw file writes.
- Adds/updates Jest tests for schema normalization and the new tasks registry operations.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| lib/state/workflow-state.js | Adds unified tasks.json schema + new optimistic update loop and task claim/release helpers. |
| tests/workflow-state.test.js | Adds tests for schema normalization and new tasks.json operations. |
| .kiro/skills/discover-tasks/SKILL.md | Updates claimed-task exclusion snippet to rely on tasksRegistry.tasks being an array. |
| .kiro/agents/worktree-manager.json | Replaces raw registry writes with calls to workflowState.claimTask() / releaseTask() in the prompt. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+183
to
+207
| function updateTasks(mutatorFn, projectPath = process.cwd()) { | ||
| const MAX_RETRIES = 5; | ||
|
|
||
| for (let attempt = 0; attempt < MAX_RETRIES; attempt++) { | ||
| const current = readTasks(projectPath); | ||
| const initialVersion = current._version || 0; | ||
|
|
||
| let updated; | ||
| try { | ||
| updated = mutatorFn(structuredClone(current)); | ||
| } catch (e) { | ||
| console.error(`[ERROR] updateTasks: mutatorFn threw on attempt ${attempt + 1}: ${e.message}`); | ||
| return false; | ||
| } | ||
|
|
||
| // Carry forward the pre-write version so writeTasks increments it by exactly 1 | ||
| updated._version = initialVersion; | ||
|
|
||
| writeTasks(updated, projectPath); | ||
|
|
||
| // Verify we won: our write should produce exactly initialVersion + 1 | ||
| const afterWrite = readTasks(projectPath); | ||
| if (afterWrite._version === initialVersion + 1) { | ||
| return true; | ||
| } |
Comment on lines
151
to
+163
| /** | ||
| * Write tasks.json to main project | ||
| * Write tasks.json atomically. | ||
| * Increments _version on every write — this is what optimistic locking uses | ||
| * to detect races: after writeJsonAtomic completes, _version on disk is | ||
| * exactly initialVersion + 1. If a concurrent writer also wrote, it will be | ||
| * initialVersion + 2 or higher, and the losing writer detects the mismatch. | ||
| */ | ||
| function writeTasks(tasks, projectPath = process.cwd()) { | ||
| ensureStateDir(projectPath); | ||
| const copy = structuredClone(tasks); | ||
| copy._version = (copy._version || 0) + 1; | ||
| const tasksPath = getTasksPath(projectPath); | ||
| writeJsonAtomic(tasksPath, tasks); | ||
| writeJsonAtomic(tasksPath, copy); |
Comment on lines
+218
to
+221
| console.error( | ||
| `[ERROR] updateTasks: all ${MAX_RETRIES} attempts failed due to concurrent writers on tasks.json at ${getTasksPath(projectPath)}. ` + | ||
| `Another agent process is modifying the registry simultaneously. ` + | ||
| `Last known _version: ${readTasks(projectPath)._version}. ` + |
| "name": "worktree-manager", | ||
| "description": "Create and manage git worktrees for isolated task development. Use this agent after task selection to create a clean working environment.", | ||
| "prompt": "# Worktree Manager Agent\n\nYou manage git worktrees to provide isolated development environments for each task.\nThis prevents work-in-progress from polluting the main working directory.\n\n## Phase 1: Pre-flight Checks\n\nVerify git is available and check current status:\n\n```bash\n# Verify git\ngit --version || { echo \"ERROR: git not available\"; exit 1; }\n\n# Check if already in a worktree\nCURRENT_DIR=$(pwd)\nMAIN_WORKTREE=$(git worktree list --porcelain | head -1 | cut -d' ' -f2)\n\nif [ \"$CURRENT_DIR\" != \"$MAIN_WORKTREE\" ]; then\n echo \"WARNING: Already in a worktree at $CURRENT_DIR\"\n echo \"ALREADY_IN_WORKTREE=true\"\nfi\n\n# Get current branch\nORIGINAL_BRANCH=$(git branch --show-current)\necho \"ORIGINAL_BRANCH=$ORIGINAL_BRANCH\"\n\n# Check for uncommitted changes\nif [ -n \"$(git status --porcelain)\" ]; then\n echo \"HAS_UNCOMMITTED_CHANGES=true\"\n git status --short\nfi\n```\n\n## Phase 2: Generate Worktree Path\n\nCreate a slug from the task title and generate paths:\n\n```javascript\nfunction generateWorktreePath(task) {\n // Create slug from task title\n const slug = task.title\n .toLowerCase()\n .replace(/[^a-z0-9]+/g, '-')\n .replace(/^-|-$/g, '')\n .substring(0, 40); // Limit slug length for filesystem compatibility\n\n // Include task ID for uniqueness\n const fullSlug = task.id ? `${slug}-${task.id}` : slug;\n\n return {\n slug: fullSlug,\n branchName: `feature/${fullSlug}`,\n worktreePath: `../worktrees/${fullSlug}`\n };\n}\n```\n\n## Phase 3: Check for Existing Worktree\n\nCheck if worktree already exists (for resume scenarios):\n\n```bash\nWORKTREE_PATH=\"../worktrees/${SLUG}\"\nBRANCH_NAME=\"feature/${SLUG}\"\n\n# Check if worktree exists\nif git worktree list | grep -q \"$WORKTREE_PATH\"; then\n echo \"WORKTREE_EXISTS=true\"\n echo \"Worktree already exists at $WORKTREE_PATH\"\nfi\n\n# Check if branch exists\nif git branch --list \"$BRANCH_NAME\" | grep -q \"$BRANCH_NAME\"; then\n echo \"BRANCH_EXISTS=true\"\nfi\n\n# Check remote branch\nif git ls-remote --heads origin \"$BRANCH_NAME\" | grep -q \"$BRANCH_NAME\"; then\n echo \"REMOTE_BRANCH_EXISTS=true\"\nfi\n```\n\n## Phase 4: Handle Uncommitted Changes\n\nIf there are uncommitted changes, handle them:\n\n```bash\nif [ \"$HAS_UNCOMMITTED_CHANGES\" = \"true\" ]; then\n echo \"Stashing uncommitted changes...\"\n git stash push -m \"Auto-stash before worktree creation for task ${TASK_ID}\"\n STASH_CREATED=\"true\"\nfi\n```\n\n## Phase 5: Create Worktree\n\nCreate the worktree with a new feature branch:\n\n```bash\n# Ensure worktrees directory exists\nmkdir -p ../worktrees\n\n# Create worktree with new branch\nif [ \"$WORKTREE_EXISTS\" = \"true\" ]; then\n echo \"Using existing worktree at $WORKTREE_PATH\"\nelse\n if [ \"$BRANCH_EXISTS\" = \"true\" ]; then\n # Branch exists, create worktree from it\n git worktree add \"$WORKTREE_PATH\" \"$BRANCH_NAME\"\n elif [ \"$REMOTE_BRANCH_EXISTS\" = \"true\" ]; then\n # Remote branch exists, track it\n git worktree add --track -b \"$BRANCH_NAME\" \"$WORKTREE_PATH\" \"origin/$BRANCH_NAME\"\n else\n # Create new branch from main\n git worktree add -b \"$BRANCH_NAME\" \"$WORKTREE_PATH\"\n fi\n\n if [ $? -eq 0 ]; then\n echo \"[OK] Created worktree at $WORKTREE_PATH\"\n echo \"[OK] Created branch $BRANCH_NAME\"\n else\n echo \"ERROR: Failed to create worktree\"\n exit 1\n fi\nfi\n```\n\n## Phase 6: Claim Task in Registry\n\nAdd task to `${STATE_DIR}/tasks.json` to prevent other workflows from claiming it:\n\n```javascript\nconst fs = require('fs');\nconst stateDir = process.env.AI_STATE_DIR || '.claude';\nif (!fs.existsSync(stateDir)) fs.mkdirSync(stateDir, { recursive: true });\n\nlet registry = fs.existsSync(`${stateDir}/tasks.json`)\n ? JSON.parse(fs.readFileSync(`${stateDir}/tasks.json`))\n : { version: '1.0.0', tasks: [] };\n\nconst entry = {\n id: task.id, source: task.source, title: task.title,\n branch, worktreePath: path.resolve(worktreePath),\n claimedAt: new Date().toISOString(), claimedBy: state.workflow.id,\n status: 'claimed', lastActivityAt: new Date().toISOString()\n};\n\nconst idx = registry.tasks.findIndex(t => t.id === task.id);\nif (idx >= 0) registry.tasks[idx] = entry;\nelse registry.tasks.push(entry);\n\nfs.writeFileSync(`${stateDir}/tasks.json`, JSON.stringify(registry, null, 2));\n```\n\n## Phase 7: Anchor PWD to Worktree\n\n**Important**: Change to the worktree directory to anchor all subsequent operations.\n\n**Note**: The `cd` command within a single Bash call does not persist across separate Bash tool invocations. The orchestrator must handle PWD anchoring at the workflow level by passing absolute paths or updating the working directory context between agent invocations.\n\n```bash\ncd \"$WORKTREE_PATH\"\n\n# Verify we're in the right place\nCURRENT_BRANCH=$(git branch --show-current)\nif [ \"$CURRENT_BRANCH\" != \"$BRANCH_NAME\" ]; then\n echo \"ERROR: Not on expected branch. Expected $BRANCH_NAME, got $CURRENT_BRANCH\"\n exit 1\nfi\n\necho \"[OK] Working directory anchored to: $(pwd)\"\necho \"[OK] On branch: $CURRENT_BRANCH\"\n# Note: Orchestrator must use this path for subsequent operations\necho \"WORKTREE_ABSOLUTE_PATH=$(pwd)\"\n```\n\n## Phase 8: Create Worktree Status File\n\nCreate `${STATE_DIR}/workflow-status.json` with task, workflow, git info, and resume state.\n\nKey fields: `task` (id, source, title), `workflow` (id, status, currentPhase), `git` (branch, baseSha, mainRepoPath), `resume` (canResume, resumeFromStep).\n\n## Phase 9: Update Workflow State\n\nCall `workflowState.updateState()` with git info (originalBranch, workingBranch, worktreePath, baseSha, isWorktree: true), then `workflowState.completePhase()`.\n\n## Phase 10: Output Summary\n\nReport: branch name, worktree path, base commit. Confirm PWD anchored to worktree.\n\n## Cleanup Responsibilities\n\n| Component | Creates | Cleans Up |\n|-----------|---------|-----------|\n| worktree-manager | worktrees, tasks.json entries, workflow-status.json | Nothing |\n| ship | - | worktrees (after merge), tasks.json entries |\n| --abort | - | worktrees, tasks.json entries |\n\n**Agents MUST NOT**: clean up worktrees, remove tasks from registry, or delete branches.\n\n## Cleanup Reference (for ship and --abort)\n\n```bash\ncleanup_worktree() {\n cd \"$ORIGINAL_DIR\"\n git worktree remove \"$WORKTREE_PATH\" --force 2>/dev/null\n git worktree prune\n [ -f \"${STATE_DIR}/tasks.json\" ] && node -e \"\n const fs = require('fs');\n const r = JSON.parse(fs.readFileSync('${STATE_DIR}/tasks.json'));\n r.tasks = r.tasks.filter(t => t.id !== '$TASK_ID');\n fs.writeFileSync('${STATE_DIR}/tasks.json', JSON.stringify(r, null, 2));\n \"\n}\n```\n\n## Error Handling\n\nOn failure: remove partial worktree, prune refs, update state with `failPhase()`, exit 1.\n\n## Success Criteria\n\n- **Task claimed in main repo's tasks.json** (prevents collisions)\n- Worktree created at `../worktrees/{task-slug}`\n- Feature branch created: `feature/{task-slug}`\n- **workflow-status.json created in worktree** (for resume capability)\n- PWD anchored to worktree directory\n- Workflow state updated with git info\n- Phase advanced to exploration\n\n## Constraints\n\n- Only create worktrees - never delete them (cleanup is handled by ship or --abort)\n- Do not remove tasks from tasks.json registry\n- Do not delete branches\n- Do not modify files in the main repository after switching to worktree\n- Always claim tasks in registry before creating worktree\n- Always create workflow-status.json in the new worktree\n- Do not proceed if uncommitted changes exist without stashing first\n\n## Model Choice: Haiku\n\nThis agent uses **haiku** because:\n- Executes scripted git commands (deterministic)\n- No complex reasoning about code or architecture\n- Simple string manipulation for slugs/paths\n- Fast execution for setup operations", | ||
| "prompt": "# Worktree Manager Agent\n\nYou manage git worktrees to provide isolated development environments for each task.\nThis prevents work-in-progress from polluting the main working directory.\n\n## Phase 1: Pre-flight Checks\n\nVerify git is available and check current status:\n\n```bash\n# Verify git\ngit --version || { echo \"ERROR: git not available\"; exit 1; }\n\n# Check if already in a worktree\nCURRENT_DIR=$(pwd)\nMAIN_WORKTREE=$(git worktree list --porcelain | head -1 | cut -d' ' -f2)\n\nif [ \"$CURRENT_DIR\" != \"$MAIN_WORKTREE\" ]; then\n echo \"WARNING: Already in a worktree at $CURRENT_DIR\"\n echo \"ALREADY_IN_WORKTREE=true\"\nfi\n\n# Get current branch\nORIGINAL_BRANCH=$(git branch --show-current)\necho \"ORIGINAL_BRANCH=$ORIGINAL_BRANCH\"\n\n# Check for uncommitted changes\nif [ -n \"$(git status --porcelain)\" ]; then\n echo \"HAS_UNCOMMITTED_CHANGES=true\"\n git status --short\nfi\n```\n\n## Phase 2: Generate Worktree Path\n\nCreate a slug from the task title and generate paths:\n\n```javascript\nfunction generateWorktreePath(task) {\n // Create slug from task title\n const slug = task.title\n .toLowerCase()\n .replace(/[^a-z0-9]+/g, '-')\n .replace(/^-|-$/g, '')\n .substring(0, 40); // Limit slug length for filesystem compatibility\n\n // Include task ID for uniqueness\n const fullSlug = task.id ? `${slug}-${task.id}` : slug;\n\n return {\n slug: fullSlug,\n branchName: `feature/${fullSlug}`,\n worktreePath: `../worktrees/${fullSlug}`\n };\n}\n```\n\n## Phase 3: Check for Existing Worktree\n\nCheck if worktree already exists (for resume scenarios):\n\n```bash\nWORKTREE_PATH=\"../worktrees/${SLUG}\"\nBRANCH_NAME=\"feature/${SLUG}\"\n\n# Check if worktree exists\nif git worktree list | grep -q \"$WORKTREE_PATH\"; then\n echo \"WORKTREE_EXISTS=true\"\n echo \"Worktree already exists at $WORKTREE_PATH\"\nfi\n\n# Check if branch exists\nif git branch --list \"$BRANCH_NAME\" | grep -q \"$BRANCH_NAME\"; then\n echo \"BRANCH_EXISTS=true\"\nfi\n\n# Check remote branch\nif git ls-remote --heads origin \"$BRANCH_NAME\" | grep -q \"$BRANCH_NAME\"; then\n echo \"REMOTE_BRANCH_EXISTS=true\"\nfi\n```\n\n## Phase 4: Handle Uncommitted Changes\n\nIf there are uncommitted changes, handle them:\n\n```bash\nif [ \"$HAS_UNCOMMITTED_CHANGES\" = \"true\" ]; then\n echo \"Stashing uncommitted changes...\"\n git stash push -m \"Auto-stash before worktree creation for task ${TASK_ID}\"\n STASH_CREATED=\"true\"\nfi\n```\n\n## Phase 5: Create Worktree\n\nCreate the worktree with a new feature branch:\n\n```bash\n# Ensure worktrees directory exists\nmkdir -p ../worktrees\n\n# Create worktree with new branch\nif [ \"$WORKTREE_EXISTS\" = \"true\" ]; then\n echo \"Using existing worktree at $WORKTREE_PATH\"\nelse\n if [ \"$BRANCH_EXISTS\" = \"true\" ]; then\n # Branch exists, create worktree from it\n git worktree add \"$WORKTREE_PATH\" \"$BRANCH_NAME\"\n elif [ \"$REMOTE_BRANCH_EXISTS\" = \"true\" ]; then\n # Remote branch exists, track it\n git worktree add --track -b \"$BRANCH_NAME\" \"$WORKTREE_PATH\" \"origin/$BRANCH_NAME\"\n else\n # Create new branch from main\n git worktree add -b \"$BRANCH_NAME\" \"$WORKTREE_PATH\"\n fi\n\n if [ $? -eq 0 ]; then\n echo \"[OK] Created worktree at $WORKTREE_PATH\"\n echo \"[OK] Created branch $BRANCH_NAME\"\n else\n echo \"ERROR: Failed to create worktree\"\n exit 1\n fi\nfi\n```\n\n## Phase 6: Claim Task in Registry\n\nAdd task to `${STATE_DIR}/tasks.json` to prevent other workflows from claiming it.\n\nUse the library function — do NOT write raw `fs.writeFileSync` here (it bypasses atomic write-rename and has no optimistic locking):\n\n```javascript\nconst workflowState = require('../../lib/state/workflow-state');\n\nconst ok = workflowState.claimTask({\n id: task.id,\n source: task.source,\n title: task.title,\n branch,\n worktreePath: path.resolve(worktreePath),\n claimedBy: state.workflow.id\n}, PROJECT_PATH);\n\nif (!ok) {\n // claimTask already logged the reason (concurrent write conflict after max retries).\n // Fail hard so the orchestrator surfaces this to the user instead of continuing\n // with an unclaimed task that could collide with another workflow.\n throw new Error(`[ERROR] claimTask failed for task ${task.id} — see log above for details.`);\n}\n```\n\n## Phase 7: Anchor PWD to Worktree\n\n**Important**: Change to the worktree directory to anchor all subsequent operations.\n\n**Note**: The `cd` command within a single Bash call does not persist across separate Bash tool invocations. The orchestrator must handle PWD anchoring at the workflow level by passing absolute paths or updating the working directory context between agent invocations.\n\n```bash\ncd \"$WORKTREE_PATH\"\n\n# Verify we're in the right place\nCURRENT_BRANCH=$(git branch --show-current)\nif [ \"$CURRENT_BRANCH\" != \"$BRANCH_NAME\" ]; then\n echo \"ERROR: Not on expected branch. Expected $BRANCH_NAME, got $CURRENT_BRANCH\"\n exit 1\nfi\n\necho \"[OK] Working directory anchored to: $(pwd)\"\necho \"[OK] On branch: $CURRENT_BRANCH\"\n# Note: Orchestrator must use this path for subsequent operations\necho \"WORKTREE_ABSOLUTE_PATH=$(pwd)\"\n```\n\n## Phase 8: Create Worktree Status File\n\nCreate `${STATE_DIR}/workflow-status.json` with task, workflow, git info, and resume state.\n\nKey fields: `task` (id, source, title), `workflow` (id, status, currentPhase), `git` (branch, baseSha, mainRepoPath), `resume` (canResume, resumeFromStep).\n\n## Phase 9: Update Workflow State\n\nCall `workflowState.updateState()` with git info (originalBranch, workingBranch, worktreePath, baseSha, isWorktree: true), then `workflowState.completePhase()`.\n\n## Phase 10: Output Summary\n\nReport: branch name, worktree path, base commit. Confirm PWD anchored to worktree.\n\n## Cleanup Responsibilities\n\n| Component | Creates | Cleans Up |\n|-----------|---------|-----------|\n| worktree-manager | worktrees, tasks.json entries, workflow-status.json | Nothing |\n| ship | - | worktrees (after merge), tasks.json entries |\n| --abort | - | worktrees, tasks.json entries |\n\n**Agents MUST NOT**: clean up worktrees, remove tasks from registry, or delete branches.\n\n## Cleanup Reference (for ship and --abort)\n\nUse the library function — do NOT write raw `fs.writeFileSync` here (it bypasses atomic write-rename and has no optimistic locking):\n\n```bash\ncleanup_worktree() {\n cd \"$ORIGINAL_DIR\"\n git worktree remove \"$WORKTREE_PATH\" --force 2>/dev/null\n git worktree prune\n # Release task claim atomically via library (retry-safe, version-checked)\n node -e \"\n const wf = require('PATH_TO_LIB/state/workflow-state');\n const ok = wf.releaseTask('$TASK_ID', '$PROJECT_PATH');\n if (!ok) {\n process.stderr.write('[ERROR] releaseTask failed for task $TASK_ID after max retries — registry entry may still be marked claimed. Retry with: node -e \\'require(PATH_TO_LIB/state/workflow-state).releaseTask(\\'$TASK_ID\\', \\'$PROJECT_PATH\\')\\'\\n');\n process.exit(1);\n }\n \"\n}\n```\n\n## Error Handling\n\nOn failure: remove partial worktree, prune refs, update state with `failPhase()`, exit 1.\n\n## Success Criteria\n\n- **Task claimed in main repo's tasks.json** (prevents collisions)\n- Worktree created at `../worktrees/{task-slug}`\n- Feature branch created: `feature/{task-slug}`\n- **workflow-status.json created in worktree** (for resume capability)\n- PWD anchored to worktree directory\n- Workflow state updated with git info\n- Phase advanced to exploration\n\n## Constraints\n\n- Only create worktrees - never delete them (cleanup is handled by ship or --abort)\n- Do not remove tasks from tasks.json registry\n- Do not delete branches\n- Do not modify files in the main repository after switching to worktree\n- Always claim tasks in registry before creating worktree\n- Always create workflow-status.json in the new worktree\n- Do not proceed if uncommitted changes exist without stashing first\n\n## Model Choice: Haiku\n\nThis agent uses **haiku** because:\n- Executes scripted git commands (deterministic)\n- No complex reasoning about code or architecture\n- Simple string manipulation for slugs/paths\n- Fast execution for setup operations", |
- Fix writeTasks docstring: clarify that _version alone cannot distinguish concurrent writers (both write N+1); _writerId is the tiebreaker - Implement post-loop idempotency fallback in updateTasks: after exhausting retries, re-run the mutator on the current on-disk state; if the result is already what we wanted (e.g. releaseTask on an already-absent entry), return true instead of a spurious failure — mirrors updateFlow behaviour - Fix retry test: jest.spyOn cannot intercept writeJsonAtomic because workflow-state.js destructures it at import time; use fs.renameSync monkey-patch instead to simulate a concurrent _writerId overwrite after our atomic write completes, exercising the actual retry path - Fix agent prompt Cleanup Reference: replace PATH_TO_LIB placeholder with ./lib/state/workflow-state, replace bare \ with MAIN_REPO_PATH env var pattern with fallback to \, add explanatory comment for the orchestrator
Contributor
There was a problem hiding this comment.
Pull request overview
This PR addresses lost updates and stale entries in the shared tasks.json registry by unifying its schema and adding optimistic-locking retries for all read-modify-write operations, then updates agent/skill guidance and expands test coverage.
Changes:
- Introduces a unified
tasks.jsonschema ({ active, tasks, _version }) with normalization on read and_versionincrements on write. - Adds
updateTasks()optimistic-lock loop plusclaimTask()/releaseTask()helpers and routes active-task setters through the locked update path. - Updates
worktree-manageranddiscover-tasksprompts/docs to use the new APIs, and adds tests for legacy formats and conflict retry behavior.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 6 comments.
| File | Description |
|---|---|
| lib/state/workflow-state.js | Adds tasks registry schema normalization, versioned atomic writes, optimistic-lock update loop, and claim/release helpers. |
| tests/workflow-state.test.js | Adds tests for schema normalization, claim/release behavior, and update retry on conflicts. |
| .kiro/skills/discover-tasks/SKILL.md | Updates claimed-task exclusion snippet to rely on tasks always being an array. |
| .kiro/agents/worktree-manager.json | Replaces raw fs.writeFileSync registry edits with claimTask() / releaseTask() usage in agent prompt. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+143
to
+156
| const raw = fs.readFileSync(tasksPath, 'utf8'); | ||
| let data; | ||
| try { | ||
| const data = JSON.parse(fs.readFileSync(tasksPath, 'utf8')); | ||
| // Normalize legacy format that may not have 'active' field | ||
| if (!Object.prototype.hasOwnProperty.call(data, 'active')) { | ||
| return { active: null }; | ||
| } | ||
| return data; | ||
| data = JSON.parse(raw); | ||
| } catch (e) { | ||
| console.error(`[CRITICAL] Corrupted tasks.json at ${tasksPath}: ${e.message}`); | ||
| return { active: null }; | ||
| throw new Error(`[CRITICAL] Corrupted tasks.json at ${tasksPath}: ${e.message}. File must be repaired or deleted manually before writes are allowed.`); | ||
| } | ||
| // Normalize: ensure every field exists (handles legacy { active } and legacy { version, tasks[] }) | ||
| return { | ||
| active: Object.prototype.hasOwnProperty.call(data, 'active') ? data.active : null, | ||
| tasks: Array.isArray(data.tasks) ? data.tasks : [], | ||
| _version: typeof data._version === 'number' ? data._version : 0, | ||
| _writerId: typeof data._writerId === 'string' ? data._writerId : undefined | ||
| }; |
Comment on lines
+218
to
+220
| // Skip write if mutatorFn made no changes — avoids spurious version bumps | ||
| if (JSON.stringify(updated) === JSON.stringify(current)) { | ||
| return true; |
| tasks.tasks = tasks.tasks.filter(t => t.id !== taskId); | ||
| if (tasks.tasks.length === before) { | ||
| // Not found — that's fine, idempotent | ||
| console.error(`[WARN] releaseTask: task ${taskId} was not found in tasks.json registry. It may have already been released or never claimed.`); |
Comment on lines
+29
to
+30
| const tasksRegistry = workflowState.readTasks(); | ||
| const claimedIds = new Set(tasksRegistry.tasks.map(t => t.id)); |
| "name": "worktree-manager", | ||
| "description": "Create and manage git worktrees for isolated task development. Use this agent after task selection to create a clean working environment.", | ||
| "prompt": "# Worktree Manager Agent\n\nYou manage git worktrees to provide isolated development environments for each task.\nThis prevents work-in-progress from polluting the main working directory.\n\n## Phase 1: Pre-flight Checks\n\nVerify git is available and check current status:\n\n```bash\n# Verify git\ngit --version || { echo \"ERROR: git not available\"; exit 1; }\n\n# Check if already in a worktree\nCURRENT_DIR=$(pwd)\nMAIN_WORKTREE=$(git worktree list --porcelain | head -1 | cut -d' ' -f2)\n\nif [ \"$CURRENT_DIR\" != \"$MAIN_WORKTREE\" ]; then\n echo \"WARNING: Already in a worktree at $CURRENT_DIR\"\n echo \"ALREADY_IN_WORKTREE=true\"\nfi\n\n# Get current branch\nORIGINAL_BRANCH=$(git branch --show-current)\necho \"ORIGINAL_BRANCH=$ORIGINAL_BRANCH\"\n\n# Check for uncommitted changes\nif [ -n \"$(git status --porcelain)\" ]; then\n echo \"HAS_UNCOMMITTED_CHANGES=true\"\n git status --short\nfi\n```\n\n## Phase 2: Generate Worktree Path\n\nCreate a slug from the task title and generate paths:\n\n```javascript\nfunction generateWorktreePath(task) {\n // Create slug from task title\n const slug = task.title\n .toLowerCase()\n .replace(/[^a-z0-9]+/g, '-')\n .replace(/^-|-$/g, '')\n .substring(0, 40); // Limit slug length for filesystem compatibility\n\n // Include task ID for uniqueness\n const fullSlug = task.id ? `${slug}-${task.id}` : slug;\n\n return {\n slug: fullSlug,\n branchName: `feature/${fullSlug}`,\n worktreePath: `../worktrees/${fullSlug}`\n };\n}\n```\n\n## Phase 3: Check for Existing Worktree\n\nCheck if worktree already exists (for resume scenarios):\n\n```bash\nWORKTREE_PATH=\"../worktrees/${SLUG}\"\nBRANCH_NAME=\"feature/${SLUG}\"\n\n# Check if worktree exists\nif git worktree list | grep -q \"$WORKTREE_PATH\"; then\n echo \"WORKTREE_EXISTS=true\"\n echo \"Worktree already exists at $WORKTREE_PATH\"\nfi\n\n# Check if branch exists\nif git branch --list \"$BRANCH_NAME\" | grep -q \"$BRANCH_NAME\"; then\n echo \"BRANCH_EXISTS=true\"\nfi\n\n# Check remote branch\nif git ls-remote --heads origin \"$BRANCH_NAME\" | grep -q \"$BRANCH_NAME\"; then\n echo \"REMOTE_BRANCH_EXISTS=true\"\nfi\n```\n\n## Phase 4: Handle Uncommitted Changes\n\nIf there are uncommitted changes, handle them:\n\n```bash\nif [ \"$HAS_UNCOMMITTED_CHANGES\" = \"true\" ]; then\n echo \"Stashing uncommitted changes...\"\n git stash push -m \"Auto-stash before worktree creation for task ${TASK_ID}\"\n STASH_CREATED=\"true\"\nfi\n```\n\n## Phase 5: Create Worktree\n\nCreate the worktree with a new feature branch:\n\n```bash\n# Ensure worktrees directory exists\nmkdir -p ../worktrees\n\n# Create worktree with new branch\nif [ \"$WORKTREE_EXISTS\" = \"true\" ]; then\n echo \"Using existing worktree at $WORKTREE_PATH\"\nelse\n if [ \"$BRANCH_EXISTS\" = \"true\" ]; then\n # Branch exists, create worktree from it\n git worktree add \"$WORKTREE_PATH\" \"$BRANCH_NAME\"\n elif [ \"$REMOTE_BRANCH_EXISTS\" = \"true\" ]; then\n # Remote branch exists, track it\n git worktree add --track -b \"$BRANCH_NAME\" \"$WORKTREE_PATH\" \"origin/$BRANCH_NAME\"\n else\n # Create new branch from main\n git worktree add -b \"$BRANCH_NAME\" \"$WORKTREE_PATH\"\n fi\n\n if [ $? -eq 0 ]; then\n echo \"[OK] Created worktree at $WORKTREE_PATH\"\n echo \"[OK] Created branch $BRANCH_NAME\"\n else\n echo \"ERROR: Failed to create worktree\"\n exit 1\n fi\nfi\n```\n\n## Phase 6: Claim Task in Registry\n\nAdd task to `${STATE_DIR}/tasks.json` to prevent other workflows from claiming it:\n\n```javascript\nconst fs = require('fs');\nconst stateDir = process.env.AI_STATE_DIR || '.claude';\nif (!fs.existsSync(stateDir)) fs.mkdirSync(stateDir, { recursive: true });\n\nlet registry = fs.existsSync(`${stateDir}/tasks.json`)\n ? JSON.parse(fs.readFileSync(`${stateDir}/tasks.json`))\n : { version: '1.0.0', tasks: [] };\n\nconst entry = {\n id: task.id, source: task.source, title: task.title,\n branch, worktreePath: path.resolve(worktreePath),\n claimedAt: new Date().toISOString(), claimedBy: state.workflow.id,\n status: 'claimed', lastActivityAt: new Date().toISOString()\n};\n\nconst idx = registry.tasks.findIndex(t => t.id === task.id);\nif (idx >= 0) registry.tasks[idx] = entry;\nelse registry.tasks.push(entry);\n\nfs.writeFileSync(`${stateDir}/tasks.json`, JSON.stringify(registry, null, 2));\n```\n\n## Phase 7: Anchor PWD to Worktree\n\n**Important**: Change to the worktree directory to anchor all subsequent operations.\n\n**Note**: The `cd` command within a single Bash call does not persist across separate Bash tool invocations. The orchestrator must handle PWD anchoring at the workflow level by passing absolute paths or updating the working directory context between agent invocations.\n\n```bash\ncd \"$WORKTREE_PATH\"\n\n# Verify we're in the right place\nCURRENT_BRANCH=$(git branch --show-current)\nif [ \"$CURRENT_BRANCH\" != \"$BRANCH_NAME\" ]; then\n echo \"ERROR: Not on expected branch. Expected $BRANCH_NAME, got $CURRENT_BRANCH\"\n exit 1\nfi\n\necho \"[OK] Working directory anchored to: $(pwd)\"\necho \"[OK] On branch: $CURRENT_BRANCH\"\n# Note: Orchestrator must use this path for subsequent operations\necho \"WORKTREE_ABSOLUTE_PATH=$(pwd)\"\n```\n\n## Phase 8: Create Worktree Status File\n\nCreate `${STATE_DIR}/workflow-status.json` with task, workflow, git info, and resume state.\n\nKey fields: `task` (id, source, title), `workflow` (id, status, currentPhase), `git` (branch, baseSha, mainRepoPath), `resume` (canResume, resumeFromStep).\n\n## Phase 9: Update Workflow State\n\nCall `workflowState.updateState()` with git info (originalBranch, workingBranch, worktreePath, baseSha, isWorktree: true), then `workflowState.completePhase()`.\n\n## Phase 10: Output Summary\n\nReport: branch name, worktree path, base commit. Confirm PWD anchored to worktree.\n\n## Cleanup Responsibilities\n\n| Component | Creates | Cleans Up |\n|-----------|---------|-----------|\n| worktree-manager | worktrees, tasks.json entries, workflow-status.json | Nothing |\n| ship | - | worktrees (after merge), tasks.json entries |\n| --abort | - | worktrees, tasks.json entries |\n\n**Agents MUST NOT**: clean up worktrees, remove tasks from registry, or delete branches.\n\n## Cleanup Reference (for ship and --abort)\n\n```bash\ncleanup_worktree() {\n cd \"$ORIGINAL_DIR\"\n git worktree remove \"$WORKTREE_PATH\" --force 2>/dev/null\n git worktree prune\n [ -f \"${STATE_DIR}/tasks.json\" ] && node -e \"\n const fs = require('fs');\n const r = JSON.parse(fs.readFileSync('${STATE_DIR}/tasks.json'));\n r.tasks = r.tasks.filter(t => t.id !== '$TASK_ID');\n fs.writeFileSync('${STATE_DIR}/tasks.json', JSON.stringify(r, null, 2));\n \"\n}\n```\n\n## Error Handling\n\nOn failure: remove partial worktree, prune refs, update state with `failPhase()`, exit 1.\n\n## Success Criteria\n\n- **Task claimed in main repo's tasks.json** (prevents collisions)\n- Worktree created at `../worktrees/{task-slug}`\n- Feature branch created: `feature/{task-slug}`\n- **workflow-status.json created in worktree** (for resume capability)\n- PWD anchored to worktree directory\n- Workflow state updated with git info\n- Phase advanced to exploration\n\n## Constraints\n\n- Only create worktrees - never delete them (cleanup is handled by ship or --abort)\n- Do not remove tasks from tasks.json registry\n- Do not delete branches\n- Do not modify files in the main repository after switching to worktree\n- Always claim tasks in registry before creating worktree\n- Always create workflow-status.json in the new worktree\n- Do not proceed if uncommitted changes exist without stashing first\n\n## Model Choice: Haiku\n\nThis agent uses **haiku** because:\n- Executes scripted git commands (deterministic)\n- No complex reasoning about code or architecture\n- Simple string manipulation for slugs/paths\n- Fast execution for setup operations", | ||
| "prompt": "# Worktree Manager Agent\n\nYou manage git worktrees to provide isolated development environments for each task.\nThis prevents work-in-progress from polluting the main working directory.\n\n## Phase 1: Pre-flight Checks\n\nVerify git is available and check current status:\n\n```bash\n# Verify git\ngit --version || { echo \"ERROR: git not available\"; exit 1; }\n\n# Check if already in a worktree\nCURRENT_DIR=$(pwd)\nMAIN_WORKTREE=$(git worktree list --porcelain | head -1 | cut -d' ' -f2)\n\nif [ \"$CURRENT_DIR\" != \"$MAIN_WORKTREE\" ]; then\n echo \"WARNING: Already in a worktree at $CURRENT_DIR\"\n echo \"ALREADY_IN_WORKTREE=true\"\nfi\n\n# Get current branch\nORIGINAL_BRANCH=$(git branch --show-current)\necho \"ORIGINAL_BRANCH=$ORIGINAL_BRANCH\"\n\n# Check for uncommitted changes\nif [ -n \"$(git status --porcelain)\" ]; then\n echo \"HAS_UNCOMMITTED_CHANGES=true\"\n git status --short\nfi\n```\n\n## Phase 2: Generate Worktree Path\n\nCreate a slug from the task title and generate paths:\n\n```javascript\nfunction generateWorktreePath(task) {\n // Create slug from task title\n const slug = task.title\n .toLowerCase()\n .replace(/[^a-z0-9]+/g, '-')\n .replace(/^-|-$/g, '')\n .substring(0, 40); // Limit slug length for filesystem compatibility\n\n // Include task ID for uniqueness\n const fullSlug = task.id ? `${slug}-${task.id}` : slug;\n\n return {\n slug: fullSlug,\n branchName: `feature/${fullSlug}`,\n worktreePath: `../worktrees/${fullSlug}`\n };\n}\n```\n\n## Phase 3: Check for Existing Worktree\n\nCheck if worktree already exists (for resume scenarios):\n\n```bash\nWORKTREE_PATH=\"../worktrees/${SLUG}\"\nBRANCH_NAME=\"feature/${SLUG}\"\n\n# Check if worktree exists\nif git worktree list | grep -q \"$WORKTREE_PATH\"; then\n echo \"WORKTREE_EXISTS=true\"\n echo \"Worktree already exists at $WORKTREE_PATH\"\nfi\n\n# Check if branch exists\nif git branch --list \"$BRANCH_NAME\" | grep -q \"$BRANCH_NAME\"; then\n echo \"BRANCH_EXISTS=true\"\nfi\n\n# Check remote branch\nif git ls-remote --heads origin \"$BRANCH_NAME\" | grep -q \"$BRANCH_NAME\"; then\n echo \"REMOTE_BRANCH_EXISTS=true\"\nfi\n```\n\n## Phase 4: Handle Uncommitted Changes\n\nIf there are uncommitted changes, handle them:\n\n```bash\nif [ \"$HAS_UNCOMMITTED_CHANGES\" = \"true\" ]; then\n echo \"Stashing uncommitted changes...\"\n git stash push -m \"Auto-stash before worktree creation for task ${TASK_ID}\"\n STASH_CREATED=\"true\"\nfi\n```\n\n## Phase 5: Create Worktree\n\nCreate the worktree with a new feature branch:\n\n```bash\n# Ensure worktrees directory exists\nmkdir -p ../worktrees\n\n# Create worktree with new branch\nif [ \"$WORKTREE_EXISTS\" = \"true\" ]; then\n echo \"Using existing worktree at $WORKTREE_PATH\"\nelse\n if [ \"$BRANCH_EXISTS\" = \"true\" ]; then\n # Branch exists, create worktree from it\n git worktree add \"$WORKTREE_PATH\" \"$BRANCH_NAME\"\n elif [ \"$REMOTE_BRANCH_EXISTS\" = \"true\" ]; then\n # Remote branch exists, track it\n git worktree add --track -b \"$BRANCH_NAME\" \"$WORKTREE_PATH\" \"origin/$BRANCH_NAME\"\n else\n # Create new branch from main\n git worktree add -b \"$BRANCH_NAME\" \"$WORKTREE_PATH\"\n fi\n\n if [ $? -eq 0 ]; then\n echo \"[OK] Created worktree at $WORKTREE_PATH\"\n echo \"[OK] Created branch $BRANCH_NAME\"\n else\n echo \"ERROR: Failed to create worktree\"\n exit 1\n fi\nfi\n```\n\n## Phase 6: Claim Task in Registry\n\nAdd task to `${STATE_DIR}/tasks.json` to prevent other workflows from claiming it.\n\nUse the library function — do NOT write raw `fs.writeFileSync` here (it bypasses atomic write-rename and has no optimistic locking):\n\n```javascript\nconst path = require('path');\nconst workflowState = require('../../lib/state/workflow-state');\n\n// PROJECT_PATH is the main repo root (not the worktree); passed in from the orchestrator\n// or resolved from the worktree git config: git -C worktreePath rev-parse --show-toplevel\nconst projectPath = process.env.PROJECT_PATH || process.cwd();\n\nconst ok = workflowState.claimTask({\n id: task.id,\n source: task.source,\n title: task.title,\n branch,\n worktreePath: path.resolve(worktreePath),\n claimedBy: state.workflow.id\n}, projectPath);\n\nif (!ok) {\n // claimTask already logged the reason (concurrent write conflict after max retries).\n // Fail hard so the orchestrator surfaces this to the user instead of continuing\n // with an unclaimed task that could collide with another workflow.\n throw new Error(`[ERROR] claimTask failed for task ${task.id} in ${projectPath} — see log above for details.`);\n}\n```\n\n## Phase 7: Anchor PWD to Worktree\n\n**Important**: Change to the worktree directory to anchor all subsequent operations.\n\n**Note**: The `cd` command within a single Bash call does not persist across separate Bash tool invocations. The orchestrator must handle PWD anchoring at the workflow level by passing absolute paths or updating the working directory context between agent invocations.\n\n```bash\ncd \"$WORKTREE_PATH\"\n\n# Verify we're in the right place\nCURRENT_BRANCH=$(git branch --show-current)\nif [ \"$CURRENT_BRANCH\" != \"$BRANCH_NAME\" ]; then\n echo \"ERROR: Not on expected branch. Expected $BRANCH_NAME, got $CURRENT_BRANCH\"\n exit 1\nfi\n\necho \"[OK] Working directory anchored to: $(pwd)\"\necho \"[OK] On branch: $CURRENT_BRANCH\"\n# Note: Orchestrator must use this path for subsequent operations\necho \"WORKTREE_ABSOLUTE_PATH=$(pwd)\"\n```\n\n## Phase 8: Create Worktree Status File\n\nCreate `${STATE_DIR}/workflow-status.json` with task, workflow, git info, and resume state.\n\nKey fields: `task` (id, source, title), `workflow` (id, status, currentPhase), `git` (branch, baseSha, mainRepoPath), `resume` (canResume, resumeFromStep).\n\n## Phase 9: Update Workflow State\n\nCall `workflowState.updateState()` with git info (originalBranch, workingBranch, worktreePath, baseSha, isWorktree: true), then `workflowState.completePhase()`.\n\n## Phase 10: Output Summary\n\nReport: branch name, worktree path, base commit. Confirm PWD anchored to worktree.\n\n## Cleanup Responsibilities\n\n| Component | Creates | Cleans Up |\n|-----------|---------|-----------|\n| worktree-manager | worktrees, tasks.json entries, workflow-status.json | Nothing |\n| ship | - | worktrees (after merge), tasks.json entries |\n| --abort | - | worktrees, tasks.json entries |\n\n**Agents MUST NOT**: clean up worktrees, remove tasks from registry, or delete branches.\n\n## Cleanup Reference (for ship and --abort)\n\nUse the library function — do NOT write raw `fs.writeFileSync` here (it bypasses atomic write-rename and has no optimistic locking):\n\n```bash\ncleanup_worktree() {\n cd \"$ORIGINAL_DIR\"\n git worktree remove \"$WORKTREE_PATH\" --force 2>/dev/null\n git worktree prune\n # Release task claim atomically via library (retry-safe, version-checked).\n # PROJECT_PATH = main repo root (not the worktree). Use $ORIGINAL_DIR if available,\n # or resolve from git: git -C \"$ORIGINAL_DIR\" rev-parse --show-toplevel\n MAIN_REPO_PATH=\"${PROJECT_PATH:-$ORIGINAL_DIR}\"\n node -e \"\n const wf = require('./lib/state/workflow-state');\n const ok = wf.releaseTask('$TASK_ID', process.env.MAIN_REPO_PATH || process.cwd());\n if (!ok) {\n process.stderr.write('[ERROR] releaseTask failed for task $TASK_ID after max retries. Registry entry may still be marked claimed. Retry: node -e \\'require(\\\\\"./lib/state/workflow-state\\\\\").releaseTask(\\\\\"\\'$TASK_ID\\'\\\\\")\\' \\n');\n process.exit(1);\n }\n \" MAIN_REPO_PATH=\"$MAIN_REPO_PATH\"\n}\n```\n\n## Error Handling\n\nOn failure: remove partial worktree, prune refs, update state with `failPhase()`, exit 1.\n\n## Success Criteria\n\n- **Task claimed in main repo's tasks.json** (prevents collisions)\n- Worktree created at `../worktrees/{task-slug}`\n- Feature branch created: `feature/{task-slug}`\n- **workflow-status.json created in worktree** (for resume capability)\n- PWD anchored to worktree directory\n- Workflow state updated with git info\n- Phase advanced to exploration\n\n## Constraints\n\n- Only create worktrees - never delete them (cleanup is handled by ship or --abort)\n- Do not remove tasks from tasks.json registry\n- Do not delete branches\n- Do not modify files in the main repository after switching to worktree\n- Always claim tasks in registry before creating worktree\n- Always create workflow-status.json in the new worktree\n- Do not proceed if uncommitted changes exist without stashing first\n\n## Model Choice: Haiku\n\nThis agent uses **haiku** because:\n- Executes scripted git commands (deterministic)\n- No complex reasoning about code or architecture\n- Simple string manipulation for slugs/paths\n- Fast execution for setup operations", |
Comment on lines
+200
to
+220
| const fsActual = require('fs'); | ||
| const originalRename = fsActual.renameSync.bind(fsActual); | ||
| let renameCallCount = 0; | ||
| fsActual.renameSync = function (src, dest) { | ||
| originalRename(src, dest); // complete our write | ||
| renameCallCount++; | ||
| if (renameCallCount === 1) { | ||
| // Concurrent writer wins: stamp a different _writerId | ||
| const current = JSON.parse(fsActual.readFileSync(dest, 'utf8')); | ||
| current._writerId = 'concurrent-winner-foreign-id'; | ||
| fsActual.writeFileSync(dest, JSON.stringify(current, null, 2)); | ||
| } | ||
| }; | ||
|
|
||
| const ok = updateTasks(tasks => { | ||
| tasks.active = { taskId: 'retry-test' }; | ||
| return tasks; | ||
| }, testDir); | ||
|
|
||
| fsActual.renameSync = originalRename; // restore | ||
|
|
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
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
Fixes #331 —
tasks.jsonread-modify-write had no atomicity or locking, allowing concurrent/next-taskand/shipruns to silently lose claims or leave stale registry entries.Root causes fixed
tasks.json— barereadFileSync → mutate → writeFileSyncwith no version check or retryworktree-managerwrote{ version, tasks[] }whileworkflow-state.jsread{ active }, making claim exclusion indiscover-tasksalways return an empty set (claims never filtered)fs.writeFileSyncin agent prompts — bypassed atomic write-rename entirelyApproach: single-file, no lockfile
Keeps
tasks.jsonas one file by applying the same optimistic locking pattern already used forflow.json:Because
fs.renameSyncis atomic at the OS level, only one writer wins per round. The loser detects the version mismatch and retries from a fresh read.Changes
lib/state/workflow-state.js{ active, tasks[], _version }—readTasks()normalizes both legacy formats on read, no migration neededwriteTasks()increments_versionon every writeupdateTasks(mutatorFn)— new optimistic lock loop (5 retries + jitter), mirrorsupdateFlow()claimTask(entry)— atomic upsert intotasks[]for worktree-managerreleaseTask(taskId)— atomic filter fromtasks[]for ship/abort; idempotentsetActiveTask/clearActiveTask— now delegate toupdateTasks_version, copy-pasteable recovery command.kiro/agents/worktree-manager.jsonfs.writeFileSyncclaim block withworkflowState.claimTask()workflowState.releaseTask().kiro/skills/discover-tasks/SKILL.mdreadTasks().tasks || []—tasksis now always an array; claim exclusion now works correctly__tests__/workflow-state.test.jsclaimTask(add, upsert,_versionincrement, missing id),releaseTask(remove, idempotent),updateTasks(mutation + version, conflict retry)Note
Medium Risk
Touches workflow coordination state (
tasks.json) and changes write semantics via versioned optimistic locking; concurrency bugs or schema assumptions could affect/next-taskclaiming and cleanup flows.Overview
Prevents concurrent workflows from clobbering
tasks.jsonby introducing a unified{ active, tasks, _version }schema plus optimistic locking with retries for all read-modify-write operations.Adds
updateTasks()(version check + jittered retry), makeswriteTasks()increment_version, and routessetActiveTask/clearActiveTaskthrough the new locking path; also adds atomicclaimTask()/releaseTask()helpers for worktree claims.Updates the
worktree-manageranddiscover-tasksprompts to use these library APIs (and correctly filter claimed tasks), and expandsworkflow-statetests to cover legacy schema normalization, claim/release behavior, and version-conflict retry.Reviewed by Cursor Bugbot for commit 2238020. Configure here.