fix(auth): make the stuck-lock guidance conditional, not an instruction (#2082)

The one genuinely new finding from Copilot round 9; the other four were
re-reports of rounds 7 and 8 against an earlier state of the files, verified
already fixed in e956d498 / edddc6c9.

This message has now been wrong in both directions. Round 5 replaced "it
expires on its own after 10s" — false whenever the same `rmdir` that blocked
us also blocks stale takeover, which reclaims through that identical call —
with "remove it by hand". That is false the other way: `proper-lockfile`
forwards whatever the filesystem returned, so a transient failure can clear,
another Inspector can legitimately acquire the path afterwards, and an
operator following the advice then deletes a *live* holder's lock. Same class
of hazard as the `ERELEASED` case in round 7: guidance that destroys the
exclusion of a process that did nothing wrong.

Nothing here can tell a permanent refusal from a passing one, so it no longer
claims to. It reports what happened, says the lock may or may not clear and
why, and states the two conditions the operator can check for themselves —
saves still failing, and no other Inspector running — rather than asserting
an outcome this code does not know.

Behavioural only in what it tells a human; the locking path is unchanged.
The test asserts both conditions are present and that the expiry promise is
absent, so neither blanket version can come back.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01JTHVxSu8AUgHRLvo1ntZ8H
Signed-off-by: cliffhall <cliff@futurescale.com>
This commit is contained in:
cliffhall 2026-08-23 12:49:06 -04:00
commit 31feed970c

View file

@ -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 () => {