Skip to content

fix(lock): distinguish held from cannot-be-created in acquireLock (#999) - #1036

Closed
Jay0130-a wants to merge 1 commit into
Tencent:mainfrom
Jay0130-a:fix/lock-error-999
Closed

Jay0130-a wants to merge 1 commit into
Tencent:mainfrom
Jay0130-a:fix/lock-error-999

Conversation

@Jay0130-a

Copy link
Copy Markdown
Contributor

Summary

acquireLock returned false both when another process held the lock and when the lock could not be created at all (a parent path in the way, an unwritable directory, a full disk). Every caller read the second case as "busy": pull printed sync in progress elsewhere and exited 0 while nothing had synced, retry loops burned their whole budget, and the real cause was never named.

This splits the verdict into acquired | busy | notWritable (LockResult / acquireLockResult, with acquireLock kept as the boolean wrapper) and migrates every caller. A notWritable verdict names the lock path and reason (the closest existing path when a parent is in the way, otherwise the errno), stops retry loops immediately, and surfaces as an error on pull/push (exit code 1) instead of a silent skip.

Type of Change

  • Bug fix (non-breaking change that fixes an issue)

Test Plan

  • npx tsc --noEmit passes
  • npm run lint — oxlint on all changed files is clean. The --type-aware pass cannot run in this checkout (it reports Failed to find tsgolint executable, i.e. the oxlint-tsgolint dev dependency is missing), not a finding.
  • npx vitest run — the full suite does not complete green on this Windows machine for six pre-existing reasons below; CI on Linux is the real gate.
  • Added/updated tests for the change

Related Issues

Fixes #999

Notes for Reviewers

  • The notWritable shape reuses NotWritableError and existingAncestor from git-exclude.ts (the pattern fix: delivery and local-state bugs (#993) #995 established), so the message style is already familiar and tested.
  • pull: a scope whose sync lock is not-writable is dropped from every clone-consuming stage exactly like a contended scope, reported with the path and reason, and fails the command (process.exitCode = 1).
  • push (both self-mode and git-mode) already fails on contention; it now reports the not-writable reason instead of the generic "another process" line.
  • Polling callers (votes, models/switch, models/profile, pending-learnings, dashboard-collector, jsonl-store, session-collector, secret-store, codex-team-hooks, local-agent, branch-worktree, uninstall) stop immediately on notWritable instead of waiting out their retry budget; bootstrap, migrate, source, and learnings-publish keep their existing return semantics but name the real cause.
  • The EEXIST from the exclusive-create path still means "the lock exists" and stays busy; only the ensureDir-time EEXIST (a file where the lock's directory should be) is classified not-writable.
  • Verification on Windows 10 / Node 24, branch @ 4c1ab4d: tsc --noEmit clean; oxlint clean; vitest run over update (64), lock-atomic (22), migrate (62), votes, model-switch (71), model-profile (30), jsonl-store, session-collector, save-session, import-lock, dashboard-collector (155), push-pending-pr (29), codex-hook-rules, self-worktree-rebind, reports-reader-lock — all pass.
  • Pre-existing Windows-environment failures, byte-for-byte the same on unmodified main @ 2cbf556 in this environment: git-exclude / mcp-git-exclude (8.3 short-path vs long-path assertions), pending-learnings (POSIX path separators), secret-values / mcp-secrets / source (case or physical-target path shape), local-agent token 0o600. None are touched by this change.

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.

[bug] A lock teamai cannot create is reported as held by another process

2 participants