diff --git a/clients/web/src/test/integration/auth/node/file-lock.test.ts b/clients/web/src/test/integration/auth/node/file-lock.test.ts index 6d045328..bd4d5929 100644 --- a/clients/web/src/test/integration/auth/node/file-lock.test.ts +++ b/clients/web/src/test/integration/auth/node/file-lock.test.ts @@ -475,8 +475,17 @@ describe("withSecretFileLock reports what it cannot clean up", () => { // not be turned into a failure by its own teardown. expect(result).toBe("saved"); expect(warnings()).toContain("Could not release the lock"); - expect(warnings()).toContain("by hand"); - expect(warnings()).not.toContain("expires on its own"); + // Conditional guidance, not an instruction. `proper-lockfile` forwards + // whatever the filesystem returned and nothing here can tell a permanent + // refusal from a transient one — and if it was transient, this path may + // by then hold a *different, live* Inspector's lock, so an unconditional + // "remove it" would destroy the exclusion of a process that did nothing + // wrong. Both conditions must be stated. + expect(warnings()).toContain("If saves keep failing"); + expect(warnings()).toContain("no other Inspector is running"); + // …and it must not promise the lock clears by itself either, which is + // false whenever the same `rmdir` also blocks stale takeover. + expect(warnings()).not.toMatch(/It expires on its own/); }); it("warns rather than crashing the process when the library detects the takeover", async () => { diff --git a/core/auth/node/file-lock.ts b/core/auth/node/file-lock.ts index 73772031..aaf566d6 100644 --- a/core/auth/node/file-lock.ts +++ b/core/auth/node/file-lock.ts @@ -494,13 +494,20 @@ export async function openSecretFileLock( // happened, so there is nothing to add. if ((err as NodeJS.ErrnoException).code === "ERELEASED") return; warnOnce( - // No branch on the *removal* error code, deliberately. Our own guard - // `stat`s before removing, so anything else reaching here is an - // `rmdir` that was refused — and **stale takeover reclaims through - // that same `rmdir`**, so whatever blocked ours blocks that too. - // `ENOTEMPTY` is the reachable one; `EACCES`, `EPERM` and `EROFS` - // behave identically. Promising expiry was wrong for all of them. - `Could not release the lock on the secrets file at ${target} (${describeError(err)}). Stale takeover reclaims a lock through the same directory removal, so whatever blocked this blocks that too: saves will keep failing until you remove ${lockPathOf(target)} by hand.`, + // **Conditional, not an instruction.** Two blanket versions of this + // have now been wrong in opposite directions: "it expires on its own" + // (false whenever the same `rmdir` that blocked us also blocks stale + // takeover — `ENOTEMPTY`, `EACCES`, `EPERM`, `EROFS`), and "remove it + // by hand" (false whenever the failure was transient, since the path + // may by then hold a *different, live* Inspector's lock, and deleting + // that destroys the exclusion of a process that did nothing wrong). + // + // `proper-lockfile` forwards whatever the filesystem returned, and + // nothing here can tell a permanent refusal from a passing one. So + // this reports what happened and states the two conditions the + // operator can check for themselves, rather than asserting an outcome + // this code does not know. + `Could not release the lock on the secrets file at ${target} (${describeError(err)}). It may clear on its own — stale takeover reclaims a lock through the same directory removal, so it will not if whatever blocked this persists. If saves keep failing against this file and no other Inspector is running, remove ${lockPathOf(target)} by hand.`, ); } };