Repository navigation
chat: a chat another process holds gets no sync drive from here - #2046
Merged
Merged
Conversation
openV3Agent seated a chat, and so started its sync drive, before the session open learned that another process holds the chat's journal. On the same computer the relay lease is this device's, so the drive resumed the other process's fence (a heartbeat and a rewritten receipt), and this process's quit then released that fence and deleted the receipt. The desktop app's chat flipped to "available to continue here" and refused its next send. A --no-host terminal left at "Move this conversation here?" did it on quit; a session host that booted the held chat did it when it stopped or retired. The drive side openV3Agent seats now waits, parked, until open() holds the journal, and is abandoned without any relay call when it does not. A parked drive looks to gates, seals and Close like a startup in progress, so restored work still waits for the ownership decision. Openers of one chat in one process share the parked drive. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…lock The waiters are asserted parked when the goroutine dump shows them blocked inside Gate and readyStore, rather than after a 20 ms sleep, and closing a parked drive is bounded by a third of driveCloseWithin rather than 250 ms, so a loaded machine cannot fail correct code. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
v3TakeOverInstead pointed the refused launch's config at a fresh folder and opened it directly, so the fresh chat kept the held chat's seat, seals and renewals: its commands ran and sealed on the held chat's cell. It now opens through openV3Agent, the road /new takes, so the fresh chat gets its own seat and its own drive, begun once its journal opens. Tests follow the drive starting once the journal is held: - the take round trip waits for the reopened chat's new hold before the other computer reads its row, instead of racing that acquire; - C6's startup boundary does not count the device and chat listings, which the drive's startup and the app's inventory send in either order; a cell read by admission is still counted; - the held-chat lease check allows the holder's own renewal while it still refuses a release, and the parked waiters' dump grows until it is complete. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
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
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.
Why
A chat open in the desktop app could be knocked out of the app by a second window on the same computer that never took it.
codeaf app) holds a synced chat: the relay lease is this computer's at fence F, its send receipt (v3/sync/holds/<hash>) says{chat, F}, and its session holds the chat's journal lock.codeaf chat --no-host --session <that chat>opens the same chat. Its journal is locked, so it comes up on home with "Move this conversation here?". It is quit with ctrl-c without an answer.conflict,held_elsewhere), although nobody else held the chat.Root cause (confirmed by a read-only Codex root-cause run with file:line evidence, then measured):
openV3Agent(cmd/codeaf/chatv3.go) seats the chat, andv3Seated→v3SeatWith→driveOf→newLiveDrivestarts its sync drive at once, beforeopen(restored)learns that the journal is locked.driving()(internal/syncsetup/drive.go) sees a lease that is this device's and unexpired, soresumeOwnheartbeats the app's fence F andrecordSendLeaserewrites the same receipt{chat, F}. Measured: a relay heartbeat 0.1–0.3 s after the terminal starts, and the receipt file replaced 0.29 s after it starts.ErrSessionLocked(→*sessionHeldElsewhere), but the drive stays insyncDrives.closeAll→Drive.Close→Batcher.Close→Release(F), thenforgetSendLease({chat, F}). The receipt is identical to the app's, so the newer-owner guard cannot tell them apart and deletes it.Host mode has the same defect, only later. The host-mode terminal attaches to the workspace's session host, whose Boot reaches the same
openV3Agent(cmd/codeaf/engine.go). The host heartbeats the app's fence and rewrites its receipt the same way, then keeps the drive. When the host is stopped (codeaf engine --stop) or retires idle, it releases the app's hold and deletes the receipt. That is why host mode looked safe in a terminal-only test.What changed
All in
cmd/codeaf;internal/syncsetupis not touched.openV3Agentseats with the drive side parked (v3SeatedPending→pendingDriveOf). It callsclaim(true)right afteropen(restored)succeeds. Every other return, including a panic, runs the deferredclaim(false).readychannel already exists, soGate,lateStore.readyStoreandClosemean what they meant during a slow startup, andtry()does nothing. Restored work insession.Newstill waits for the ownership decision instead of running ungated, andsession.Newnever waits on it synchronously (checked).Close,closeAll,take,closeCellandtakeCellabandon a parked drive at once, with no I/O and no wait.v3TakeOverInsteadused to point the refused launch's config at a fresh folder and open it directly, so the fresh chat kept the held chat's seat, seals and renewals: its commands ran and sealed on the held chat's cell. It now opens throughopenV3Agent, the road/newtakes, so it gets its own seat and its own drive.v3Seated,v3SeatWith,driveOf,newLiveDrive) keep "start at once" for their existing callers; no production caller of the one-step seat remains.openV3Agent: the interactive launch,--once,/new,/resumeand the picker, the session host's Boot and itsOpen, the desktop app's open (assembleEngine), and the factory stages. So all of them now leave a chat that another process holds alone.One cost, measured: the drive's startup now begins once
session.Newhas opened the journal, not during it, so a chat's hold settles about one session construction later. In real app processes on a loopback relay, the time fromagents.opento the app's "here at the new fence" event was 171–472 ms on this branch and 384–648 ms on app/v3 (3 runs each), so it is not visibly slower. On the hosted relay the old ordering never held anyway. Three app tests had relied on that loopback ordering; see below.Unchanged on purpose:
resumeOwnstill resumes this device's unexpired hold, so an app restart or a crash still continues its own chat. #2034's same-computer displacement retirement and #2031'slockChat/.publicationlocks are not touched.How it was checked (this Mac)
Every result below is from this Mac (macOS 15.6, Go 1.26.5). app/v3 is bd98788, this branch is f19c10b (the head, 65389d2, only renumbers the change entry) unless a line says otherwise, and furrow is staged unless a line says otherwise.
The contract tests (
cmd/codeaf/heldchat_drive_test.go):TestAChatAnotherProcessHoldsIsNeitherRenewedNorReleasedFromHereSync.Drive. P2's refusedopenV3Agentand its quit make zero lease writes; the lease, fence and head are unchanged and unreleased; the receipt is byte-identical and the same file; P1 stillHoldsLease,Gates andConfirmWorks.celltest.Available), so Linux CI runs it-race, and with furrow unstagedTestAQuitAtTheMoveQuestionLeavesTheOtherWindowsChatSendableTestTheFreshChatBesideAHeldOneWorksOnItsOwnSeatTestAParkedChatDriveWaitsForItsJournalDecisionClose,closeAll,take,closeCellandtakeCellabandon it at once with no start; two openers share it in either order.-raceThe app/v3 runs use
go test -overlaywith the five product files replaced by their bd98788 versions; nothing on disk changed.Gates:
gofmt: clean.go vet ./cmd/codeaf: passes for darwin andGOOS=linux.GOOS=windows go vet ./cmd/codeaffails identically on app/v3 and this branch, on existing darwin-only test fixtures (syscall.Mkfifoinappcheckout_test.go, thenappOpenSeam).GOOS=windows go build ./cmd/codeafpasses. A Windows vet of the product package plus the new test file, with the other test files blanked by an overlay, passes.TestEveryLockInTheGuardedTreeUnlocksFromADeferandTestSourceCommentsalready fail on app/v3. Their lists of violations are identical on this branch. The first round of this branch added one bareLock/Unlockpair; it now goes throughisParked().displaced_same_device_test.go,displaced_watch_test.go, includingTestWatchDisplacementRetiresTheDriveHoldImmediately/higher_local_fence) pass ×2 with furrow staged.internal/syncsetupis not touched.cmd/codeafispackage main, so nothing imports it. Its tests ran on both trees, compared by test name: app/v3 has 12 failing tests on this Mac, and so does this branch at f19c10b.TestAppChatsOlderCopyStaysLocalL1andTestCellRewindRestoresTheWorkspaceTheSessionSealed. Both are TempDir cleanup races ("unlinkat … directory not empty") and pass ×5 on both trees.TestAppPresenceP1ReconnectReconcilesMissedOfflineandTestAppTakeModelT1T2T3N1NoUsableDefault. Both pass ×5 on both trees.TestC1…,TestC3…,TestC4AStartupFaultReleasesWaitingTools,TestC4SeatConstructionDoesNotWaitForTheRelay, the last of which takes 30 s by design on both trees) were rerun on the branch and pass.Three existing app tests had relied on the old start order. All three were reproduced ×5 on both trees and instrumented through an overlay:
TestAppTakeF1P2FenceAndPromptPublicationandTestAppTakeProcessS6failed only on the branch, 1 in 5. Timeline of a failure:agents.openreturns at .701.lease_held.The test now waits for A's reopened hold to reach its new fence before B reads its row. The F1, P2 and S6 assertions are unchanged.
TestC6AppSendDoesNotWaitForSyncStartupOrCapture/startupfailed only on the branch, 2 in 5. The relay holds every request. A goroutine dump showed the drive's startup held at its first request, a device listing (PutOwnDevice→GET /v1/dir/list), which arrives at the same moment as the app's ownGET /v1/dir/listand/v1/dir/watch. Whichever arrived after the window opened was counted. Those two listings are no longer counted in that boundary. Every other request is counted, including a cell read (the only relay workConfirmWorkdoes before it names a foreign holder), and the failure now names the paths it adds. The capture boundary is unchanged.Each now passes ×8 on the branch.
Real processes, app/v3 vs this branch. Both sides are built with the shipped sidecar recipe (
make app-sidecar: embed, staged furrow, garble). The rig:codeaf app --home A, the desktop's engine process, driven over its JSON-lines wire;agents.send.The fixed side ran on c1ec418, and the rows marked † also on f19c10b.
--no-hostterminal shows "Move this conversation here?", then ctrl-cheld_elsewhere). One run's terminal needed SIGTERM after 17 s; the release came at the kill.codeaf engine --stopof the session hostNot verified here:
closeEngineProcess→closeAll).CI: dispatched on f19c10b (65389d2 on top only renumbers the change entry), compared by test name with app/v3 bd98788's PR gate 40946826723 and Full check 40946831047.
touched codeafpasses on Linux. This includes the engine-freeTestAChatAnotherProcessHoldsIsNeitherRenewedNorReleasedFromHereandTestAParkedChatDriveWaitsForItsJournalDecision; the furrow-backed ones skip there. Its one failure,TestRecipeDoorWritesTheWorktreeWhenOnlyTheCheckoutIsHeld(no git identity on the runner), is marked "already failing on the base".internal/session TestARefusedRunRowNoteStoresNoKey, in a package this branch does not touch. On this Mac it passes ×5 on the branch and fails 1 in 5 on app/v3.internal/session TestTheSourceHoldsEveryRowTheAppListsand the known flakeTestR7PersonalLeaderFallbackAndNoAutomaticRegain.cross build (linux/amd64)failed on a module-proxy stream error (proxy.golang.org … INTERNAL_ERROR) and passed on rerun. Its one new full test,internal/session TestARequestOlderThanTheRunningTurnWaitsForIt, passes ×5 on both trees here.Codex, read-only. The root-cause run confirmed c4's five-step reading with file:line evidence and found the host-mode door. Three adversarial rounds followed:
driveCloseWithin/3.openV3Agent, with a new test that fails on app/v3.agents.opennever promised a settled relay hold, and that ordering already failed on the hosted relay.SealWithindeadline at seat time (cellstore/pipeline.go:140,155), and now waits outsession.Newas well as the drive's startup. A construction over 30 s could time it out before the drive begins. On app/v3 the samelateStore.Preparealready waited out the drive's startup inside that deadline, so the failure mode is unchanged; onlysession.New's time is added to the window. Scheduling warm capture at the claim would change seat construction (cellstore options) for every door, so it belongs in its own change.Left as they are (other doors with the same root, a different defect):
v3Migratedstill writes into the held chat before its journal is known to be free (AdoptArtifacts,budgetOnOpen,SealMemories,cellindex.RebuildAt), and the refused seat's warm capture screens its files.Agent.Closereleases the journal before the process closes the chat's drive (closeCell/closeAll). A new opener in another process can then resume the same fence and lose it when the first process's drive closes, and a later refused open in the first process can still inspect that retained drive (superseded()→HoldsLease). This change does not widen it.🤖 Generated with Claude Code