Conversation
be15949 to
743e458
Compare
ac85715 to
3648824
Compare
743e458 to
74cce23
Compare
129165f to
2ec7d5f
Compare
74cce23 to
816334f
Compare
|
@cpacker Is it safe for a repair worker launched from listener turn cleanup to derive |
2ec7d5f to
bb1bfb1
Compare
816334f to
713705a
Compare
713705a to
9be690d
Compare
bb1bfb1 to
8deb8fb
Compare
9be690d to
065640a
Compare
8deb8fb to
8004d1a
Compare
065640a to
b132e34
Compare
8004d1a to
d9bf6b6
Compare
b132e34 to
61bb67e
Compare
d9bf6b6 to
34f91f9
Compare
34f91f9 to
2da15e6
Compare
61bb67e to
09a1105
Compare
2da15e6 to
26a6808
Compare
09a1105 to
1f9441b
Compare
26a6808 to
91a1ada
Compare
1f9441b to
7946e81
Compare
| ((repair) => | ||
| startMemoryConflictRepair(repair, spawnBackgroundSubagentTask)) | ||
| )({ ...params, result })); | ||
| const reminder = repairLaunched |
There was a problem hiding this comment.
Post-turn sync returns as soon as this worker is spawned, then releases the checkout lease, so the next primary turn can edit memory while the repair mutates the same checkout. Primary memory tools do not take this lease, which means a concurrent edit can be swept into the repair commit or overwritten; the repair must serialize primary writes or avoid the primary checkout.
There was a problem hiding this comment.
Addressed in e58bf61 the same way the rest of this stack coordinates the primary: repair has to work on the checkout itself (the unfinished operation lives there), and holding the post-turn lease for the worker's whole run would block the next turn, so post-turn sync now queues a MEMORY REPAIR IN PROGRESS reminder telling the primary not to edit memory files or run Git there until the worker's task log ends. Making the primary's own file tools take the lease was cut from this stack deliberately (see #4626).
7946e81 to
be21410
Compare
When the primary's post-turn sync finds an unfinished merge or rebase, launch a repair-only memory worker instead of asking the agent to fix it by hand. The worker skips the transcript export, re-checks the conflict under the checkout lease so a queued duplicate becomes a no-op, and reports the outcome to its task log. A worker whose own sync ends in a conflict triggers the same repair. Each conflict handed to a worker is recorded (HEAD plus the unfinished operation's heads) in the repository's Git directory, so a conflict the worker could not resolve is reported to the agent with the MEMORY GIT CONFLICT reminder instead of launching another worker every turn; new commits make it eligible again, and a repair cancelled at exit is forgotten so the next session retries. Dirty and push-failed outcomes of the primary's own sync keep their reminders.
A worker whose sync leaves a conflict now awaits the repair launch, so the repair task is registered before the worker completes and a one-shot drain picks it up instead of exiting first. startMemoryConflictRepair catches claim and launch failures (capacity, unreadable checkout) and reports false, so they are logged rather than surfacing as unhandled rejections.
Also point memory-conflict-repair at the git-dir helper's new home and let the fixture initialize a repository at a caller-chosen path.
…ancelled launches claimMemoryConflictRepair now returns a release handle bound to the attempt it recorded (by nonce), so releasing a stale or cancelled attempt can never delete a newer attempt recorded by another process. A launch that fails after the claim (task capacity, unreadable checkout) releases the marker so the next turn retries instead of treating the conflict as already attempted; a repair cancelled at exit releases through the same handle. A queued repair that finds no conflict now reports a dirty or unpushed checkout as a failure instead of "No memory conflict remains". startMemoryConflictRepair moves next to the rest of the memory task lifecycle; callers pass the spawn function, which also frees task.ts from the size ceiling it was sitting on.
…kout lease Attempt markers are written under the lease, so removing one is done under it too: a release that runs after a launch failure reuses the lease the caller holds, and a release for a cancelled repair takes a non-blocking claim and leaves the marker when the checkout is busy, which costs one extra "already attempted" report rather than deleting another process's newer attempt.
be21410 to
7afadd1
Compare
…er the lease The marker that keeps one conflict from relaunching repair every turn was claimed in one place and released from several others, some outside the checkout lease, so a cancelled or failed launch could leave it behind forever. It is now a small state machine written only under the lease: claimed as "launching" with the recording process's identity, cleared by the repair worker if it never ran (launch failure, cancellation), and marked "done" once it has run. A later claim skips a done attempt or one still in progress in a running process, and retries one whose process is gone. Post-turn sync also tells the primary when a repair worker is editing the checkout in place, so it leaves memory files and Git state alone until the worker's task log ends. The update and repair workers share one settle step for sync, refresh and recompile.
…fails A repair whose initial sync threw, or that was cancelled while still waiting for the checkout lease, never reached its cleanup and left a "launching" marker naming this live process, which suppressed retries until the process exited. The marker is now cleared when the whole leased run fails, limited to an attempt this process recorded so a caller without the lease cannot touch another process's attempt.
| // Nothing ran: a failure anywhere in the leased run, or cancellation while | ||
| // still waiting for the lease. Forget this process's attempt so the next | ||
| // turn retries instead of waiting for this process to exit. | ||
| await clearMemoryConflictRepair(params.memoryDir, { ownOnly: true }); |
There was a problem hiding this comment.
withMemoryOperation() releases the lease before this catch runs, and pid plus process start time identifies every attempt from this host process, not this specific one. A concurrent turn can write a newer marker from the same process before this remove, then have that marker deleted; cleanup needs a per-attempt token or must remain under the lease.
There was a problem hiding this comment.
Fixed in 16f989b with a per-attempt token. claimMemoryConflictRepair records a random token in the marker and returns it; ensureMemoryConflictRepair passes it to the worker as memoryRepairToken (replacing the memoryRepairOnly boolean), and every transition — completeMemoryConflictRepair, clearMemoryConflictRepair — applies only when the marker still holds that token. A worker cancelled after the lease is gone can therefore only remove its own record, never a newer claim from the same process. Test: "a late worker cannot forget a newer attempt for the same checkout" (stale worker waiting on the lease, conflict changes, new claim, stale worker cancelled → marker still in_progress).
There was a problem hiding this comment.
The token narrows ownership, but it does not make readAttempt() plus rm() atomic: after this catch releases the lease, a new claimant can write its token between those calls and still have its marker removed. Keep cleanup under the lease or use an atomic compare-and-delete.
There was a problem hiding this comment.
Fair — the token narrowed ownership but read-then-remove was still two steps. b330f40 serializes every marker transition (claim, mark done, forget) under a small lock on the marker file itself (letta-memory-repair.json.lock, via withFileLock), independent of the checkout lease. The compare and the write/removal are now one critical section, so a claim made under the lease cannot interleave with a late worker's cleanup. Test: "marker transitions wait for each other rather than racing" holds the marker lock and checks the clear does not proceed until it is released.
… repairs Post-turn sync could not tell a repair still running from one that had run and failed, so a second turn during a repair told the primary to resolve the conflict by hand while the worker was still editing the checkout. The claim now reports claimed, in_progress or attempted, and the primary keeps the MEMORY REPAIR IN PROGRESS reminder until a worker has actually run. The attempt record carries a token that the launched worker receives, so a worker forgetting its attempt (cancelled while waiting for the lease, failed sync, nothing left to repair) can only remove its own record, never one a newer claim wrote from the same process. A repair that finds no conflict remaining forgets the attempt too, so the same merge aborted and retried is repaired again instead of being suppressed while the process lives.
Replace the Parameters<typeof runMemoryWorker>[n] references with named parameter, execute and dependency types, bind the worker's dependencies once instead of threading the raw deps object, and share the failed-result and no-worker result shapes that update and repair built inline. Resolve the post-turn repair dependency alongside the other sync dependencies, and read the memory Git directory once per repair claim.
A worker forgetting its attempt after the checkout lease is released did a read-then-remove that could race a claim made under the lease in between, dropping the newer record. Every transition on the marker (claim, done, forget) now runs under a small lock on the marker itself, so the token compare and the write or removal are one step.
This branch has not been deployed
2 participants
Summary
Split 4/5 of #4539. When the primary's post-turn sync finds an unfinished merge or rebase, launch a repair-only memory worker instead of asking the agent to fix it by hand.
MEMORY REPAIR IN PROGRESSreminder; once it has run without resolving the conflict, the agent gets theMEMORY GIT CONFLICTreminder instead of another worker every turn. New commits make it eligible again, and an attempt that never ran (cancelled, failed sync, nothing left to repair) is forgotten so the next turn retries; the token keeps a late worker from forgetting a newer attempt.dirtyandpush_failedoutcomes of the primary's own sync keep theirMEMORY COMMIT NEEDED/MEMORY SYNC FAILEDreminders.Stack: #4625 fork helpers → #4626 checkout lock → #4627 background worker → #4628 (this) → #4634 prompt policy.
Verification
bun run check: 12/12bun testmemory-conflict-repair, memory-worker (real Git conflict repair, duplicate repair no-op, conflict after update triggers repair, queued repair notifies, no-conflict identity), memory-handoff (repair skips export), memory-git-sync (repair launch, already-attempted reminder, dirty/push_failed reminders), memory-task, memory-task-lifecycle, memory-operation: 47 pass; isolated task-computer-routing 11, listener e2e 5headless-memory-worker.test.tsthrough the built Node bundle (one-shot and bidirectional repair workers; one-shot and bidirectional primaries launching repair): 4 passAI Disclosure
AI Tool(s) Used
OpenAI Codex (original implementation in #4539); Claude Code (review fixes, lock simplification, and splitting #4539 into this stack).
Human Verification
Pending.