From 01313b3bf2484e0d7dc538827800fc727fdaeb54 Mon Sep 17 00:00:00 2001 From: phaedrus Date: Mon, 31 Aug 2026 09:52:42 -0500 Subject: [PATCH 1/2] fix: do not fail landed writes or captured reads on unlock Unlock after a captured HEAD or a landed put/register is not lock_failed. Callers would retry a write that already committed or skip a valid snapshot. Existing conflicts still record the unlock error; a successful put warns unlock_failed the same way dirty_marker_failed does. --- internal/kernel/put.go | 31 ++++++++++------ internal/kernel/read.go | 6 ++-- internal/kernel/unlock_test.go | 66 ++++++++++++++++++++++++++++++++++ modules.budget | 4 ++- 4 files changed, 91 insertions(+), 16 deletions(-) create mode 100644 internal/kernel/unlock_test.go diff --git a/internal/kernel/put.go b/internal/kernel/put.go index 3013fe1..5732240 100644 --- a/internal/kernel/put.go +++ b/internal/kernel/put.go @@ -46,6 +46,11 @@ type PutResult struct { // refresh and its push, so the push is genuinely rejected. var beforePushHook func() +// releaseHook, when non-nil, is joined into cortexLock.release then +// disarmed. Production leaves it nil; tests inject unlock failure +// after the critical section has already committed its result. +var releaseHook func() error + // Put runs the pinned pipeline: lock → refresh → pre-flight → CAS → // validate → no-op short-circuit → stamp → atomic write → VCS tail, // all inside one per-cortex critical section. @@ -59,7 +64,13 @@ func Put(ctx context.Context, cs []Cortex, in PutInput) (res *PutResult, conf *C return nil, bound } defer func() { - conf = attachUnlock(conf, lock.release(), op, rel) + rerr := lock.release() + conf = attachUnlock(conf, rerr, op, rel) + if rerr != nil && conf == nil && res != nil { + res.Warnings = append(res.Warnings, fm.Finding{ + Level: "warning", Rule: "unlock_failed", Message: rerr.Error(), + }) + } }() res = &PutResult{Operation: op, Cortex: c.Name, Path: rel} @@ -470,17 +481,18 @@ func acquireLock(name string) (*cortexLock, error) { type cortexLock struct{ f *os.File } func (l *cortexLock) release() error { - return errors.Join(syscall.Flock(int(l.f.Fd()), syscall.LOCK_UN), l.f.Close()) + err := errors.Join(syscall.Flock(int(l.f.Fd()), syscall.LOCK_UN), l.f.Close()) + if releaseHook != nil { + err = errors.Join(err, releaseHook()) + releaseHook = nil + } + return err } func attachUnlock(conf *Conflict, rerr error, operation, path string) *Conflict { - if rerr == nil { + if rerr == nil || conf == nil { return conf } - if conf == nil { - return conflict("lock_failed", operation, path, "fix lock-file access and retry", - map[string]any{"detail": rerr.Error()}) - } if conf.Detail == nil { conf.Detail = map[string]any{} } @@ -489,11 +501,8 @@ func attachUnlock(conf *Conflict, rerr error, operation, path string) *Conflict } func attachUnlockErr(err, rerr error, operation, path string) error { - if rerr == nil { - return err - } if err == nil { - return attachUnlock(nil, rerr, operation, path) + return nil } if conf, ok := err.(*Conflict); ok { return attachUnlock(conf, rerr, operation, path) diff --git a/internal/kernel/read.go b/internal/kernel/read.go index b412fc3..fbe7d47 100644 --- a/internal/kernel/read.go +++ b/internal/kernel/read.go @@ -343,10 +343,8 @@ func withReadSnapshot(c *Cortex, operation, path string, fn func(readSnapshot) * return fail(errors.New("publisher repository has no HEAD commit")) } snapshot := readSnapshot{repo: root, sha: head} - if rerr := lock.release(); rerr != nil { - return attachUnlock(nil, rerr, operation, path) - } - return fn(snapshot) + rerr := lock.release() + return attachUnlock(fn(snapshot), rerr, operation, path) } func snapshotUnavailable(operation, path string, err error) *Conflict { diff --git a/internal/kernel/unlock_test.go b/internal/kernel/unlock_test.go new file mode 100644 index 0000000..c441666 --- /dev/null +++ b/internal/kernel/unlock_test.go @@ -0,0 +1,66 @@ +package kernel + +import ( + "errors" + "strings" + "testing" +) + +func TestAttachUnlockPreservesSuccessAndJoinsFailure(t *testing.T) { + if got := attachUnlock(nil, errors.New("unlock"), "get", "notes/x.md"); got != nil { + t.Fatalf("success became %s", got.Code) + } + conf := conflict("exists", "create", "notes/x.md", "retry", nil) + got := attachUnlock(conf, errors.New("unlock"), "create", "notes/x.md") + if got.Code != "exists" { + t.Fatalf("primary failure replaced with %s", got.Code) + } + if got.Detail["unlock"] != "unlock" { + t.Fatalf("unlock detail = %v", got.Detail) + } + if err := attachUnlockErr(nil, errors.New("unlock"), "register", "box"); err != nil { + t.Fatalf("successful register became %v", err) + } +} + +func TestUnlockAfterLandedPutIsWarningNotConflict(t *testing.T) { + f := newFixture(t) + releaseHook = func() error { return errors.New("injected unlock") } + t.Cleanup(func() { releaseHook = nil }) + res, conf := f.put("hosta", "notes/unlock.md", mkNote("note", "landed")) + if conf != nil { + t.Fatalf("landed put became %s", conf.Code) + } + found := false + for _, w := range res.Warnings { + if w.Rule == "unlock_failed" { + found = true + } + } + if !found { + t.Fatalf("want unlock_failed warning, got %+v", res.Warnings) + } + got, gconf := Get(f.cs, "hosta", "notes/unlock.md") + if gconf != nil { + t.Fatal(gconf.Code) + } + if !strings.Contains(got.Content, "landed") { + t.Fatalf("get missed landed bytes: %q", got.Content) + } +} + +func TestUnlockAfterSnapshotDoesNotSkipGet(t *testing.T) { + f := newFixture(t) + if _, conf := f.put("hosta", "notes/visible.md", mkNote("note", "visible")); conf != nil { + t.Fatal(conf.Code) + } + releaseHook = func() error { return errors.New("injected unlock") } + t.Cleanup(func() { releaseHook = nil }) + got, conf := Get(f.cs, "hosta", "notes/visible.md") + if conf != nil { + t.Fatalf("get skipped snapshot: %s", conf.Code) + } + if !strings.Contains(got.Content, "visible") { + t.Fatalf("get missed snapshot bytes: %q", got.Content) + } +} diff --git a/modules.budget b/modules.budget index 12c6a6e..2778f33 100644 --- a/modules.budget +++ b/modules.budget @@ -7,7 +7,9 @@ # Complexity ratchet: split Lint/index/GetMany/sync helpers and takeaways # scanner so gocognit 16 and nestif 5 can fail closed. Unlock/cleanup # errors join the primary failure instead of blank assignment. -kernel internal/kernel 57700 +# Unlock-after-success: do not convert a landed write or captured snapshot +# into lock_failed; tests inject release failure. +kernel internal/kernel 58400 # FlagSet-driven splitArgs: VisitAll replaces per-command value-flag maps. cli internal/cli 16200 fm internal/fm 8100 From 2875c6cf4dab89561c7bd67164475841061c6696 Mon Sep 17 00:00:00 2001 From: phaedrus Date: Mon, 31 Aug 2026 09:54:13 -0500 Subject: [PATCH 2/2] fix: run captured snapshot read before attaching unlock Spell the order as statements so unlock failure cannot be read as skipping fn(snapshot). --- internal/kernel/read.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/internal/kernel/read.go b/internal/kernel/read.go index fbe7d47..5e8ff42 100644 --- a/internal/kernel/read.go +++ b/internal/kernel/read.go @@ -344,7 +344,8 @@ func withReadSnapshot(c *Cortex, operation, path string, fn func(readSnapshot) * } snapshot := readSnapshot{repo: root, sha: head} rerr := lock.release() - return attachUnlock(fn(snapshot), rerr, operation, path) + conf := fn(snapshot) + return attachUnlock(conf, rerr, operation, path) } func snapshotUnavailable(operation, path string, err error) *Conflict {