fix: do not fail landed writes or captured reads on unlock

[moomooskycow]

Intent

Unlock failure after a captured publisher HEAD or a landed put/register must not become lock_failed / cortex_unavailable. Callers would skip a valid snapshot or retry a write that already committed.

Decisions

• withReadSnapshot: capture HEAD, release, run fn(snapshot), then attachUnlock the callback result. Do not skip the read.
• attachUnlock(nil, rerr) stays nil. Unlock joins only an existing conflict (Detail.unlock).
• Landed Put records unlock_failed as a warning, same class as dirty_marker_failed.
• Register success stays (cortex, nil) so a retry cannot create duplicate_cortex.
• Failures before the snapshot still join unlock into cortex_unavailable.

Checks

• TestAttachUnlockPreservesSuccessAndJoinsFailure
• TestUnlockAfterLandedPutIsWarningNotConflict (injected releaseHook; get sees landed bytes)
• TestUnlockAfterSnapshotDoesNotSkipGet
• golangci-lint run ./... 0 issues
• go test -race -count=1 ./... pass
• ./scripts/smoke.sh pass

Production impact

Runtime lock/unlock path only. No CLI/MCP schema change except optional warnings: [{rule: unlock_failed}] on an otherwise successful put. No migration.

Rollback

Revert the merge. Prior behavior treated post-success unlock as lock_failed.

Residual risk

• Sync success with unlock failure is now success with no warning field (SyncResult has none). Follow-up, not this slice.
• Get success with unlock failure has no warning channel.

Base: 4d141c45 (origin/master). Head: 2875c6cf4dab89561c7bd67164475841061c6696 (fix: run captured snapshot read before attaching unlock).

Summary by CodeRabbit

• Bug Fixes
  • Successful writes and snapshots are now preserved even when lock release encounters an error.
  • Lock-release failures are reported as warnings instead of incorrectly appearing as operation conflicts.
  • Snapshot callbacks continue to run after lock release issues, preventing skipped reads.
  • Existing conflict details now retain their original status while including relevant release-error information.

Delivery verification (2026-09-01, final head 2875c6c)

• go test -race -count=1 -run 'TestAttachUnlockPreservesSuccessAndJoinsFailure|TestUnlockAfterLandedPutIsWarningNotConflict|TestUnlockAfterSnapshotDoesNotSkipGet' — pass (injected release failure; get sees landed bytes; snapshot read not skipped).
• go test -race -count=1 ./... — all 9 packages pass.
• PATH=/tmp/gclv2:$PATH ./scripts/check.sh (clone-to-green: lint fuse + report, gitleaks, govulncheck, module token budgets — kernel 58351/58400, go test -race, CLI smoke) — gate passed.
• Review reconciliation: CodeRabbit — no actionable comments (docstring-coverage warning non-blocking; repo keeps comments for non-obvious "why" only). Codex — review completed at 01313b3, no findings. Residual risks above (Sync/Get warning channels) remain follow-ups.

[chatgpt-codex-connector]

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review	Status	Commit	Review trigger
📝 Code Review	✅ Completed 2026-08-31T14:56:38.580272Z	01313b3	PR opened

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 25295a6f-437a-4463-a6ef-08bc26368a72

📥 Commits

Reviewing files that changed from the base of the PR and between 4d141c4 and 2875c6c.

📒 Files selected for processing (4)

• internal/kernel/put.go
• internal/kernel/read.go
• internal/kernel/unlock_test.go
• modules.budget

Included review availability: Your plan provides up to 1 included review per hour; 0 remain after this review.

---

📝 Walkthrough

Walkthrough

The kernel now preserves successful writes and snapshots when lock release fails. Release errors are joined with conflicts or reported as unlock_failed warnings. Tests cover Put, snapshot, and helper behavior.

Changes

Unlock error handling

Layer / File(s)	Summary
Release and Put error handling internal/kernel/put.go, internal/kernel/unlock_test.go, modules.budget	Lock release combines unlock, close, and hook errors. Put preserves successful results and records unlock_failed warnings. Existing conflicts retain their primary code and include the unlock error. Tests cover these behaviors.
Snapshot callback after release errors internal/kernel/read.go, internal/kernel/unlock_test.go	The snapshot callback runs after lock release, even when release returns an error. The callback result and release error are combined. Tests confirm that a subsequent get still returns the snapshot bytes.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Merge Risk: 🔵 Low · up to 2875c

Writes and captured reads now remain successful when cleanup reports an error, preventing unsafe retries and skipped valid snapshots. A bounded follow-up risk remains because a real cleanup failure could leave lock state unresolved and is not surfaced for successful reads or registrations, affecting contention recovery and diagnosis.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 12.50% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 8 functions across 3 files. (1 skipped: 1…	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly and concisely describes the main change: successful landed writes and captured reads no longer fail when unlock fails.

Full details: Docstring Coverage

Explanation

Docstring coverage is 12.50% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 8 functions across 3 files. (1 skipped: 1 unsupported.)

• Fix all pre-merge checks with AI

✨ Finishing Touches 💡 1

📝 Generate docstrings 💡

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch fix/unlock-after-success

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}