Repository navigation
Conversation
… merges (G183(e), G21) POST /maintenance/dedup-sweep had no write-window guard, ran its model judge on the event loop, and left every merged page dirty for the next `git add -A` writer to sweep under its own author. Now: - 409 while sleep_cycle.is_writing() (a dry run too), and a window that opens mid-sweep stops the merging (`stoppedForSleep`); - each merge and its commit of exactly the merge's paths run under the page lock (never across the judge's call): `Dedup sweep <date>`, trigger maintenance/dedup-sweep, Cicada-Author: cicada, the judge's engine trailer; - a failed commit puts the merge's paths back from HEAD; - the sweep runs off the event loop. Co-Authored-By: Claude <[email protected]>
…7, G183(a))
PUT /entities/{id}/decay, PATCH /entities/{id}/repos and every inbox resolve
door (/inbox/{id}/resolve and the deprecated /nudges and /clarifications that
route through inbox_service.resolve) rewrote bank pages with no
sleep_cycle.is_writing() guard. They now answer 409 with a sentence inside the
window and go through between a drain's batches. The decay and repo rewrites
run under the page lock, and the repo PATCH commits only its page instead of
`git add -A` (it swept any dirty file under `Cicada-Author: user`).
MCP's cicada_resolve_inbox relays a 409's sentence instead of asking whether
the backend is running. /entities/{id}/read and /repos/observed write no page
and stay ungated.
Co-Authored-By: Claude <[email protected]>
…(G183(e)) The write-window check now runs before each pair's judge call too, not only before a merge: a model call whose merge could not land is not spent. Co-Authored-By: Claude <[email protected]>
…y there (G183(e)) Review finding 1 (attribution smear): `commit_paths_sync` commits whole files, so a merge over a dirty winner, loser, referencing page or graph committed that other writer's edit under `Cicada-Author: cicada`. Chosen fix: refuse rather than keep apart. Under the page lock the sweep reads the dirty set (`git_service.dirty_paths_sync`, new) and refuses a merge whose winner or loser is dirty before writing; otherwise it snapshots every page and the graph, merges, diffs, and when a changed path was dirty puts every byte back and refuses (`skipped_dirty`). Only the paths whose bytes really changed are committed. A kept-apart commit (the inbox's mechanism) was not used: it would have a background pass commit someone else's uncommitted edit as an unauthored commit and move HEAD twice inside one transaction; the sweep can simply wait for that writer and retry next run. `entity_merge.repoint_edges` no longer rewrites (or reports) the graph when no endpoint named the old id. Co-Authored-By: Claude <[email protected]>
…e locked transaction (G183(e)) Review finding 2: recovery restored from HEAD (dropping an edit already on a page), released the git write lock between the failed commit and each per-path checkout (a `git add -A` writer could commit the failed merge in between), and only logged a checkout failure. Now the page lock and the bank's git write lock are held from the dirty check through the commit or the recovery. Before merging, the sweep snapshots every page's and the graph's bytes, their stage-0 index entries and HEAD; a failed merge or commit gets those exact bytes and index entries back (an untracked or absent path stays out of the index), never HEAD's. A moved HEAD or a put-back error raises RecoveryFailed: the sweep stops and answers `recoveryFailed: true`. The response also carries `skippedDirty` and `failed`. Co-Authored-By: Claude <[email protected]>
…hrough commit (G183(a)) Review finding 3: both routes released the page lock before awaiting their commit, so another page writer (an agent's claim, a dedup merge) could rewrite the same page in the gap and commit the person's change under its own author, leaving the person's commit a no-op. Each route now runs read → modify → write → scoped synchronous commit as one page-lock section in a worker thread (`_rewrite_page_and_commit`); nothing is awaited inside it, so the thread-re-entrant lock never spans the event loop, and a held lock no longer stalls the loop. An edit already on the page is committed apart first, unauthored (`commit_touched_sync(before=…)`, the inbox's P1-3 mechanism), so the person's commit holds only their change. Tests: a barrier competing writer between the write and the commit waits for the person's commit; an existing edit is kept apart; the between-batch test now asserts a real user commit holding exactly the route's file. Co-Authored-By: Claude <[email protected]>
… the residual race is documented (G177, G183)
Review finding 4: the sweep, the decay/repo routes and the inbox checked
`is_writing()` only before waiting (on the page lock, or on the inbox's
awaited dirty snapshot), so a window that opened during the wait was missed.
- The sweep re-asks `may_write` once the page lock is held (`STOPPED`), and
its per-pair check now covers dry runs too (no judge call while Sleep holds
the pages).
- The decay/repo section re-asks `_page_guard()` under the lock.
- The inbox re-asks after its awaited snapshot, before any write.
Sleep enters its window without taking the page lock, so this narrows the race
and does not close it; storage.md ("Residual race"), capture-and-sleep.md,
api.md and features.md now say so instead of implying isolation. Barrier tests
open the window while each writer waits.
Co-Authored-By: Claude <[email protected]>
…d touches nothing outside it (G183(e)) Re-review findings 1–3: the whole-bank snapshot cleared a refused reference page's conflict stages, restored files as fresh 0600 regular files (a symlinked page's target stayed merged, an executable lost its mode), and its bank-wide diff took an unrelated concurrent write into the merge commit or erased it in recovery. Simpler instead of more machinery: before any write the transaction computes the merge's footprint (`entity_merge.merge_footprint`: winner, loser, the graph when an edge names the loser, every page naming the loser). The pages come from `_repoint_page`, the one matcher `repoint_references` now also writes with, so prediction and merge cannot drift. The pair is refused untouched when a footprint path has unmerged index stages, is a symlink or is not a regular file (`skippedUnsafe`), or is dirty (`skippedDirty`). Only the footprint is snapshotted (bytes, permission bits, stage-0 entry or absence); the merge's returned paths must lie inside it (else the footprint is put back and the sweep stops); only those paths whose bytes changed are committed; recovery restores only footprint paths. `may_write` is also re-asked once the git lock is held. Tests: the reviewer's reproductions (conflicted reference page refused with `ls-files -s -z` identical; symlinked winner refused, link and target intact; an executable loser comes back 0755 after a failed commit; an unguarded write to an unrelated page is neither committed nor reverted), a write outside the footprint, the git-lock window re-check, and the footprint agreeing with what the merge writes across related-by-id/name, wikilink styles, a source link, a claim object and an edge. Co-Authored-By: Claude <[email protected]>
…nbounded in time (G177, G183) Re-review finding 4: the residual-race text called the exposure "milliseconds" and named Sleep taking the page lock as the fix. The exposure is the rest of a writer's transaction after its last check, lock waits and retries included, with no time bound; the future fix is shared admission coordination and/or revision-safe read-modify-write — two disjoint lock acquisitions in Sleep would not protect a stale read between them. The sweep's footprint confinement is named as what limits the damage meanwhile. Co-Authored-By: Claude <[email protected]>
…p's window transition takes no lock (G183) Re-review nit: storage.md now says commits and rollback are limited to the footprint frozen at planning, a returned path outside it stops the sweep, and a write the merge makes outside the snapshot (a reference an unguarded writer added after planning) is left for recovery, reported as `recoveryFailed`. dedup_sweep.py and api.md say "Sleep's window transition takes neither lock" — its commits do take the git write lock. Co-Authored-By: Claude <[email protected]>
…write-window-guards
… under UTF-8 (G182) Co-Authored-By: Claude Sonnet 5.5 <[email protected]> Claude-Session: https://claude.ai/code/session_01KRTB5tMLtEqzg3qCkxyz3J
fix(api): writers refuse inside Sleep's write window; the dedup sweep commits only its own merges (G183 a, e; G21; G177)
fix(install): brace expansions before non-ASCII so the installer runs under UTF-8 (G182)
chore(release): 0.3.1
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Merging this PR releases Cicada 0.3.1 (G182, TODO ruling 19): CI on main tags v0.3.1 at the merge commit, builds, verifies and publishes the GitHub Release.
Merge it with Create a merge commit — a squash or rebase would give main commits dev does not have.