Skip to content

fix: make review feature locks atomic#13

Merged
steipete merged 3 commits into
openclaw:mainfrom
rohitjavvadi:fix/atomic-review-feature-locks
May 16, 2026
Merged

fix: make review feature locks atomic#13
steipete merged 3 commits into
openclaw:mainfrom
rohitjavvadi:fix/atomic-review-feature-locks

Conversation

@rohitjavvadi
Copy link
Copy Markdown
Contributor

Summary

Fix cross-process review locking so overlapping clawpatch review runs cannot claim and review the same feature at the same time.

Problem

reviewCommand selects features once at the start of a run, then each worker attempts to lock a feature from that in-memory snapshot. Two separate review processes can both read a feature while lock is still null, then both persist a claimed copy and continue into provider review.

That can duplicate provider calls and leave feature state dependent on whichever process writes last. It also conflicts with the documented behavior that overlapping runs should not claim the same feature.

Changes

  • Add an atomic feature claim path using .clawpatch/locks/<featureId>.json with exclusive file creation.
  • Re-read the feature after acquiring the lock file before marking it claimed.
  • Reject stale claims for features that are no longer pending/error unless an explicit feature review requested that behavior.
  • Release lock files after successful review and provider failures.
  • Extend clean-locks to remove lock files as well as embedded feature locks.
  • Make JSON temp file names unique with randomUUID() to avoid same-process timestamp collisions.
  • Add workflow tests for atomic claim behavior, stale claims, provider failure cleanup, and lock cleanup.

Validation

  • corepack pnpm typecheck
  • corepack pnpm lint
  • corepack pnpm format:check
  • corepack pnpm test
  • corepack pnpm build

I also ran a disposable local repro with two concurrent reviewCommand calls against the same temp project. After this change, one review succeeds, the other fails with lock-conflict, and no lock files are left behind.

Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: c43fa7a7ed

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread src/state.ts
@rohitjavvadi rohitjavvadi force-pushed the fix/atomic-review-feature-locks branch from c43fa7a to 06b6950 Compare May 16, 2026 16:51
@steipete steipete merged commit 3b9371c into openclaw:main May 16, 2026
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants