Skip to content

fix: re-read daily note before write in sync-progress - #105

Merged
bborbe merged 1 commit into
masterfrom
fix/concurrent-daily-write
Aug 23, 2026
Merged

fix: re-read daily note before write in sync-progress#105
bborbe merged 1 commit into
masterfrom
fix/concurrent-daily-write

Conversation

@bborbe

@bborbe bborbe commented Aug 23, 2026

Copy link
Copy Markdown
Owner

Prevents concurrent-vault-write clobbering: sync-progress now re-reads the daily note immediately before writing and merges, instead of writing over a stale in-memory copy.

Why: a sibling session can rewrite the shared daily note between sync-progress's read (to locate the section) and its write (after composing the entry over several turns). Writing from the stale read destroys their entry, and obsidian-git autocommits only the last writer's version — the earlier write never enters git history.

Proof it happens: 2026-08-23 on the Personal vault — a Verify-backup entry vanished. git log -S "zfs logicalused" on the daily note returns a single commit (the later restore), not add+remove. Reproduced mechanically in a scratch repo: two writers both read HEAD, both write, one autocommit → first entry 0 hits in git log -S, second 1.

Fix: mandatory write protocol in sync-progress Phase 3.1 — re-read from disk immediately before writing, merge into current content, never overwrite. Same guard noted for other shared vault files.

…urrent-write clobbering

sync-progress reads the daily note early to locate the section, then composes
the entry over several turns. A sibling session can rewrite the file in that
gap; writing over it from the stale read silently destroys their entry, and
obsidian-git autocommits only the last writer's version so the earlier write
never enters history. Mandatory protocol: re-read immediately before writing,
merge into current content, never overwrite. Observed 2026-08-23 Personal
vault; provable via git log -S showing a single restore commit.

@ben-s-pull-request-reviewer ben-s-pull-request-reviewer Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I now have a complete picture. Let me compile the review.

Summary of findings:

  1. This PR only changes markdown documentationcommands/sync-progress.md (a Claude Code plugin slash command spec) and CHANGELOG.md. No Go code is changed.

  2. sync-progress.md IS the implementation — it's a plugin command file (YAML frontmatter + markdown body) that defines a slash command. The "code fix" is the updated documentation itself: the command now specifies a mandatory re-read-before-write protocol in Phase 3.1 to prevent TOCTOU clobbering. There is no separate Go implementation to update.

  3. No Go files changed — the ast-grep mechanical funnel confirmed 0 findings (correct), and no Go quality/go-context/go-error agents are applicable.

  4. CHANGELOG.md## Unreleased is correctly used (matching all prior release entries). The bullet describes the fix clearly.

  5. Step 4 ast-grep: 0 findings (expected — no applicable rule targets for markdown plugin commands).

  6. No issues found.

{
  "verdict": "approve",
  "summary": "PR changes only a Claude Code plugin slash command spec (`commands/sync-progress.md`) and its CHANGELOG entry. The command IS the implementation — the updated Phase 3.1 documentation mandates a re-read-immediately-before-write protocol and merge semantics, which is the full fix for the observed concurrent-write clobbering. No Go code or test files are involved, and no issues were found.",
  "comments": [],
  "concerns_addressed": [
    "correctness: The PR description explains a race condition (sibling session clobbers daily note between read and write), but the diff only shows documentation changes — no code implementation of the mandatory re-read protocol is visible. Verify whether the actual code fix lives in a separate file or was omitted. — addressed: This repo's vault-cli is a Claude Code plugin; `commands/sync-progress.md` IS the slash command spec that defines the runtime behavior. The updated documentation in Phase 3.1 ('Re-read the file immediately before writing — never write from a stale copy') and the three-step mandatory write protocol ARE the fix. No separate Go code file implements this command.",
    "correctness: The documentation mandates 're-read immediately before writing' and 'merge, never overwrite' for daily notes and shared vault files — confirm the implementation enforces this ordering atomically (no TOCTOU window between re-read and write). — not an issue: The command is a markdown specification run by the Claude Code plugin runtime. The documented protocol (re-read via Read tool immediately before Edit/Write, merge rather than overwrite) is the defined behavior. Atomicity of the protocol depends on the Claude Code tool layer, not a separate Go implementation.",
    "tests: The proof-of-reproduction described (two writers, one autocommit, first entry 0 hits in git log -S) suggests a test case should be added to verify the race condition is actually closed. — not an issue: This is a markdown command spec, not compiled code. Integration tests for the Claude Code plugin runtime are out of scope for this repo."
  ]
}

@bborbe
bborbe merged commit 618c042 into master Aug 23, 2026
1 check passed
@bborbe
bborbe deleted the fix/concurrent-daily-write branch August 23, 2026 10:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant