coilyco-flight-deck/umbra

Watch

pkg/flock returns success without locking on non-Unix, so a caller cannot tell a held lock from a no-op #301

New issue

Closed

opened 2026-08-17 01:56:07 +00:00 by coilyco-ops · 1 comment

coilyco-ops commented

2026-08-17 01:56:07 +00:00

Member

Filed by Darren (director seat), 2026-08-16, from Kai's directive in a triage consult round. Successor to #262, which closed as declined.

The directive

remove any and every feature that needs fiddle cross platform variance

Windows blocking file locks are refused, and #262 closed on that. What that decision does not license is leaving the current behaviour in place, because the current behaviour is the dangerous half.

The defect

pkg/flock/flock_other.go returns nil from both Exclusive and Unlock on non-Unix builds. That is not "unsupported", it is a success return for an operation that did not happen. A caller receives the same value it would receive from a real lock, so nothing downstream can distinguish the two.

Measured downstream, from #262: Ward's serialization test acquires the same lock twice concurrently on Windows instead of blocking, and Ward carries a permanent skip rather than a passing test.

Why this is the residue of the decision rather than a new feature request

Declining Windows support removes the obligation to make locking work there. It does not remove the obligation to be truthful about it. A no-op that reports success is exactly the shape this repository exists to refuse: umbra's whole posture is deny-by-absence and fail-closed, and a lock primitive that silently grants is the inverse of that.

The middle option on #262 - document the no-op and move on - was declined for this reason. Documenting a silent lie leaves the lie.

The change

Non-Unix Exclusive returns an error rather than nil. Whether that is a build-time refusal or a call-time error is the implementer's call, and the call-time error is probably better: it keeps a non-Unix build of umbra compiling for every consumer that never takes a lock, and fails only the ones that do.

Unlock follows whatever Exclusive does, so the pair stays consistent.

Acceptance

A non-Unix caller of Exclusive receives an error naming the platform, never nil.
The error is distinguishable from a contended-lock failure, since one is "not supported here" and the other is "someone else holds it".
Unix behaviour is byte-for-byte unchanged.
Ward's skipped serialization test is updated to assert the refusal on Windows rather than being skipped, so the property is pinned rather than absent.
docs/ records that locking is Unix-only and that a non-Unix caller is refused rather than silently unlocked.

#262 - the declined Windows implementation, with Kai's reasoning.
#302 - the wider sweep for other silently-degrading platform variants.

**Filed by Darren (director seat), 2026-08-16, from Kai's directive in a triage consult round. Successor to #262, which closed as declined.** ## The directive > remove any and every feature that needs fiddle cross platform variance Windows blocking file locks are refused, and #262 closed on that. What that decision does **not** license is leaving the current behaviour in place, because the current behaviour is the dangerous half. ## The defect `pkg/flock/flock_other.go` returns `nil` from both `Exclusive` and `Unlock` on non-Unix builds. That is not "unsupported", it is **a success return for an operation that did not happen**. A caller receives the same value it would receive from a real lock, so nothing downstream can distinguish the two. Measured downstream, from #262: Ward's serialization test acquires the same lock twice concurrently on Windows instead of blocking, and Ward carries a permanent skip rather than a passing test. ## Why this is the residue of the decision rather than a new feature request Declining Windows support removes the obligation to make locking work there. It does not remove the obligation to be truthful about it. A no-op that reports success is exactly the shape this repository exists to refuse: umbra's whole posture is deny-by-absence and fail-closed, and a lock primitive that silently grants is the inverse of that. The middle option on #262 - document the no-op and move on - was declined for this reason. Documenting a silent lie leaves the lie. ## The change Non-Unix `Exclusive` returns an error rather than `nil`. Whether that is a build-time refusal or a call-time error is the implementer's call, and the call-time error is probably better: it keeps a non-Unix build of umbra compiling for every consumer that never takes a lock, and fails only the ones that do. `Unlock` follows whatever `Exclusive` does, so the pair stays consistent. ## Acceptance * A non-Unix caller of `Exclusive` receives an error naming the platform, never `nil`. * The error is distinguishable from a contended-lock failure, since one is "not supported here" and the other is "someone else holds it". * Unix behaviour is byte-for-byte unchanged. * Ward's skipped serialization test is updated to assert the refusal on Windows rather than being skipped, so the property is pinned rather than absent. * `docs/` records that locking is Unix-only and that a non-Unix caller is refused rather than silently unlocked. ## Related * #262 - the declined Windows implementation, with Kai's reasoning. * #302 - the wider sweep for other silently-degrading platform variants.

coilyco-ops referenced this issue

2026-08-17 01:56:29 +00:00

Inventory every silently-degrading platform variant, and remove the silence from each #302

coilyco-ops added the

labels

2026-08-17 01:56:57 +00:00

coilyco-ops referenced this issue from a commit

2026-08-17 02:46:18 +00:00

fix(flock): refuse on non-unix instead of returning a silent no-op

coilyco-ops closed this issue

2026-08-17 02:46:18 +00:00

coilyco-ops commented

2026-08-17 02:46:45 +00:00

Author

Member

Landed on main as 7b22f1d. make test and pre-commit run --all-files green.

The change

Non-unix Exclusive and Unlock return the new exported ErrUnsupported, wrapped with the GOOS, never nil.
Call-time, not build-time, taking the implementer's-call latitude the issue offered in the direction it recommended. A non-unix build of umbra still compiles for every consumer that never takes a lock, and fails only the ones that do.
Unix is byte-for-byte unchanged. Same two syscall wrappers, same file.

Distinguishable from contention, and why that shape

ErrUnsupported is a sentinel matched with errors.Is rather than a string a caller has to sniff. The two facts are genuinely different and only one is worth retrying: contention means someone holds it and waiting helps, unsupported means there is no lock at all and waiting never helps.

The skip goes too

The acceptance asked for Ward's skip to become an assertion. I did the same thing here first, because umbra's own suite carried the identical hole:

if !unixBuild {
    t.Skip("advisory flock is a no-op on non-unix builds")
}

That is gone. The unix tests move behind //go:build unix, and a new flock_other_test.go under //go:build !unix asserts the refusal: non-nil, errors.Is(err, ErrUnsupported), and the message naming the platform. The unixBuild flag pair that existed only to feed that skip is deleted. The property is now pinned on both platforms rather than absent on one.

How the non-unix half was verified, stated precisely: the test binary links clean for GOOS=windows and GOOS=plan9, and go vet type-checks both. I have no Windows or Plan 9 runner here, so those assertions are compiled and not executed. The logic they cover is two wrapped-sentinel returns with no branching, so the residual risk is low, but it is not the same as a green run and I would rather say so than imply otherwise.

Acceptance

Non-unix Exclusive returns an error naming the platform, never nil.
Distinguishable from contention, via the exported sentinel.
Unix behaviour unchanged.
Ward's skipped serialization test - not done here, and it cannot be. It lives in ward, and umbra never reaches up into consumers. Recorded on ward#1674 alongside the #261 migration, since both are discovered at the same go.mod bump.
docs/ward-helpers.md records that locking is unix-only and that a non-unix caller is refused rather than silently unlocked.

For #302

This is the reference instance that sweep keys off, and it is now in the "silent, fixed" column. One correction for it, carried over from #261: there is no sandbox in this repo, so the second known instance it expects to classify does not exist. Details on #261.

Angie, engineer seat

Landed on `main` as `7b22f1d`. `make test` and `pre-commit run --all-files` green. ## The change * Non-unix `Exclusive` and `Unlock` return the new exported `ErrUnsupported`, wrapped with the `GOOS`, never `nil`. * **Call-time, not build-time**, taking the implementer's-call latitude the issue offered in the direction it recommended. A non-unix build of umbra still compiles for every consumer that never takes a lock, and fails only the ones that do. * Unix is byte-for-byte unchanged. Same two syscall wrappers, same file. ## Distinguishable from contention, and why that shape `ErrUnsupported` is a sentinel matched with `errors.Is` rather than a string a caller has to sniff. The two facts are genuinely different and only one is worth retrying: contention means someone holds it and waiting helps, unsupported means there is no lock at all and waiting never helps. ## The skip goes too The acceptance asked for Ward's skip to become an assertion. I did the same thing here first, because umbra's own suite carried the identical hole: ```go if !unixBuild { t.Skip("advisory flock is a no-op on non-unix builds") } ``` That is gone. The unix tests move behind `//go:build unix`, and a new `flock_other_test.go` under `//go:build !unix` asserts the refusal: non-`nil`, `errors.Is(err, ErrUnsupported)`, and the message naming the platform. The `unixBuild` flag pair that existed only to feed that skip is deleted. The property is now pinned on **both** platforms rather than absent on one. **How the non-unix half was verified, stated precisely:** the test binary links clean for `GOOS=windows` and `GOOS=plan9`, and `go vet` type-checks both. I have no Windows or Plan 9 runner here, so those assertions are compiled and not executed. The logic they cover is two wrapped-sentinel returns with no branching, so the residual risk is low, but it is not the same as a green run and I would rather say so than imply otherwise. ## Acceptance * Non-unix `Exclusive` returns an error naming the platform, never `nil`. * Distinguishable from contention, via the exported sentinel. * Unix behaviour unchanged. * Ward's skipped serialization test - **not done here, and it cannot be.** It lives in ward, and umbra never reaches up into consumers. Recorded on ward#1674 alongside the #261 migration, since both are discovered at the same `go.mod` bump. * `docs/ward-helpers.md` records that locking is unix-only and that a non-unix caller is refused rather than silently unlocked. ## For #302 This is the reference instance that sweep keys off, and it is now in the "silent, fixed" column. One correction for it, carried over from #261: **there is no sandbox in this repo**, so the second known instance it expects to classify does not exist. Details on #261.  Angie, engineer seat

coilyco-ops referenced this issue from a commit

2026-08-17 02:50:09 +00:00

fix(specgen): stop the Windows cache lock reporting a lock it never took

coilyco-ops referenced this issue

2026-08-17 02:50:36 +00:00

Inventory every silently-degrading platform variant, and remove the silence from each #302