Skip to content

feat(loop-worktree): add advisory path locking to prevent multi-loop collisions#274

Open
KhaiTrang1995 wants to merge 1 commit into
cobusgreyling:mainfrom
KhaiTrang1995:feat/loop-worktree-lock
Open

feat(loop-worktree): add advisory path locking to prevent multi-loop collisions#274
KhaiTrang1995 wants to merge 1 commit into
cobusgreyling:mainfrom
KhaiTrang1995:feat/loop-worktree-lock

Conversation

@KhaiTrang1995

Copy link
Copy Markdown
Contributor

Summary

Independently-scheduled loops (e.g. a ci-sweeper loop every 15m and a
dependency-sweeper loop every 6h) can race on the same files with nothing
in code to stop them -- see stories/multi-loop-collision.md and
stories/dependency-vs-ci-sweeper-collision.md. docs/multi-loop.md
already documents an acting_on convention for this, but it's prose-only,
hand-checked by each loop's control script.

What

Adds three subcommands to loop-worktree:

  • lock --paths <glob1,glob2,...> --owner <name> [--ttl <duration>] --
    acquires an advisory lock; fails clearly if another (non-expired) owner
    already holds an overlapping path.
  • unlock --owner <name> -- releases that owner's lock (no-op if it didn't
    hold one).
  • locks [--sweep] [--force] [--json] -- lists active locks; --sweep
    reports expired ones (report-only by default, --force deletes them,
    mirroring gc's existing convention).

Deliberately advisory, not enforced by create itself -- paired by
convention in the control script (lock before create, unlock after
cleanup), the same way loop-context --check and loop-worktree mark --status escalated are already paired without a runtime dependency between
the two tools.

Locking is keyed on path globs rather than run ids, so it also catches
collisions across different patterns (e.g. CI Sweeper and Dependency
Sweeper both touching package.json), not just within one. Path overlap is
compared segment-by-segment (a wildcard segment matches anything at that
position) rather than as a raw string prefix, so docs/api doesn't
false-positive against docs/apidocs.md.

Also updates docs/multi-loop.md to cross-reference this as the mechanical
form of the existing acting_on convention.

Why this shape

  • Matches the repo's existing "prose plus tooling" philosophy rather than
    hard sandboxing -- a loop that skips the lock call isn't physically
    blocked.
  • --owner is restricted to a safe charset (letters/digits/./_/-)
    since it names the lock file directly.
  • Lock acquisition (check-then-write) is serialized through a short-lived
    exclusive-create mutex file so two racing lock calls can't both pass the
    overlap check before either writes -- otherwise the feature would fail at
    exactly the race it exists to prevent.
  • Corrupt/unparseable lock JSON surfaces a clear, specific error naming the
    file, rather than crashing or silently ignoring a lock that might still be
    real.

Tested

tools/loop-worktree: 28/28 passing after a fully clean rebuild (wiped
node_modules/dist, reinstalled, rebuilt). This went through an
independent review pass (a separate agent given only the diff and task
description, not implementation rationale) before pushing, which caught and
led to fixes for: a TOCTOU race between the overlap check and the write
(now serialized via the mutex file), a corrupt-lock-file crash (now a clear
error), a path-segment-boundary bug in the original overlap heuristic (raw
string-prefix instead of segment-aware), and missing --owner validation
(path traversal in the lock filename). All four are covered by new
regression tests, including a concurrent-lock test that fires 3 racing
lockPaths calls on the same path and asserts exactly one succeeds.
19 new tests total (11 unit + 8 CLI integration) plus the 8 pre-existing
loop-worktree tests, all passing unmodified. Version 1.0.0 -> 1.1.0.

…collisions

Independently-scheduled loops can race on the same files with no code to
stop them (see stories/multi-loop-collision.md and
dependency-vs-ci-sweeper-collision.md). Adds `lock`/`unlock`/`locks`
subcommands: an owner locks a set of path globs, another owner's overlapping
lock request fails clearly instead of racing silently. Advisory only, not
enforced by `create` -- paired by convention in the control script, same as
loop-context/loop-worktree already are.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@github-actions

Copy link
Copy Markdown
Contributor

Thanks @KhaiTrang1995 for contributing a production story — visible, reviewable PRs like this grow the reference for everyone.

What happens next

  • Maintainer aims for same-day review on story, adopter, and scoped docs/example PRs (CONTRIBUTING.md).
  • good first issue PRs: comment on the linked issue so we can assign and close on merge.

More ways to help

— loop-engineering maintainers

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant