Skip to content

chat: a chat another process holds gets no sync drive from here - #2046

Merged
AbirAbbas merged 4 commits into
app/v3from
fix/nohost-quit-keeps-app-hold
Oct 29, 2026
Merged

AbirAbbas merged 4 commits into
app/v3from
fix/nohost-quit-keeps-app-hold

Conversation

@AbirAbbas

@AbirAbbas AbirAbbas commented Oct 29, 2026 •

Copy link
Copy Markdown
Collaborator

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.

  • The desktop app (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.
  • On the same home, 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.
  • That quit released the app's hold (relay: fence F, expires 0) and deleted the app's receipt. The app then showed "This chat is available to continue here." and refused its own next send (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):

  1. openV3Agent (cmd/codeaf/chatv3.go) seats the chat, and v3Seated → v3SeatWith → driveOf → newLiveDrive starts its sync drive at once, before open(restored) learns that the journal is locked.
  2. The drive's driving() (internal/syncsetup/drive.go) sees a lease that is this device's and unexpired, so resumeOwn heartbeats the app's fence F and recordSendLease rewrites 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.
  3. The open fails with ErrSessionLocked (→ *sessionHeldElsewhere), but the drive stays in syncDrives.
  4. The quit runs closeAll → Drive.Close → Batcher.Close → Release(F), then forgetSendLease({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/syncsetup is not touched.

  • A chat's sync drive starts only once this process holds the chat's journal. openV3Agent seats with the drive side parked (v3SeatedPending → pendingDriveOf). It calls claim(true) right after open(restored) succeeds. Every other return, including a panic, runs the deferred claim(false).
  • A parked drive looks like a startup in progress. Its ready channel already exists, so Gate, lateStore.readyStore and Close mean what they meant during a slow startup, and try() does nothing. Restored work in session.New still waits for the ownership decision instead of running ungated, and session.New never waits on it synchronously (checked).
  • A refused open leaves no trace. When every claimant of a still-parked drive gives up, it is abandoned: closed, its waiters released, never started, no relay call. It is also removed from the book. Close, closeAll, take, closeCell and takeCell abandon a parked drive at once, with no I/O and no wait.
  • Two openers of one chat in one process share the parked drive (claims are counted under the book mutex). It begins exactly once when one of them gets the journal, and is abandoned only when none did.
  • The fresh chat offered beside a held one opens on its own seat. v3TakeOverInstead used 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 through openV3Agent, the road /new takes, so it gets its own seat and its own drive.
  • The one-step forms (v3Seated, v3SeatWith, driveOf, newLiveDrive) keep "start at once" for their existing callers; no production caller of the one-step seat remains.
  • Every door that seats a synced chat goes through openV3Agent: the interactive launch, --once, /new, /resume and the picker, the session host's Boot and its Open, 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.New has 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 from agents.open to 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: resumeOwn still 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's lockChat/.publication locks 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):

Test What it proves Engine On app/v3 (×3 each) On this branch
TestAChatAnotherProcessHoldsIsNeitherRenewedNorReleasedFromHere P1 holds the journal flock, a real relay lease and its receipt through a real Sync.Drive. P2's refused openV3Agent and 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 still HoldsLease, Gates and ConfirmWorks. engine-free (celltest.Available), so Linux CI runs it FAIL: "P2 made 2 lease writes while opening and quitting a held chat; want 0". Also fails with furrow unstaged. PASS ×8, under -race, and with furrow unstaged
TestAQuitAtTheMoveQuestionLeavesTheOtherWindowsChatSendable The same, with P1 a real seated, spoken door chat. Afterwards P1's next command is admitted and seals under P1's device and fence. furrow FAIL: "lease expiry after P2 quit = 0; want unreleased" PASS ×8
TestTheFreshChatBesideAHeldOneWorksOnItsOwnSeat The fresh chat offered beside a held one runs a command on its own cell. P1's sealed turns, lease and receipt are unchanged, and P2 started no drive for P1's chat. furrow FAIL: "P1's sealed turns = 2; want unchanged count 1" (the fresh chat's turn landed in P1's cell) PASS ×8
TestAParkedChatDriveWaitsForItsJournalDecision A parked drive's waiters block until the decision; a success starts it once; refusals, Close, closeAll, take, closeCell and takeCell abandon it at once with no start; two openers share it in either order. engine-free n/a (new API) PASS ×8 and under -race

The app/v3 runs use go test -overlay with the five product files replaced by their bd98788 versions; nothing on disk changed.

Gates:

  • gofmt: clean.
  • go vet ./cmd/codeaf: passes for darwin and GOOS=linux.
  • GOOS=windows go vet ./cmd/codeaf fails identically on app/v3 and this branch, on existing darwin-only test fixtures (syscall.Mkfifo in appcheckout_test.go, then appOpenSeam). GOOS=windows go build ./cmd/codeaf passes. A Windows vet of the product package plus the new test file, with the other test files blanked by an overlay, passes.
  • Laws: TestEveryLockInTheGuardedTreeUnlocksFromADefer and TestSourceComments already fail on app/v3. Their lists of violations are identical on this branch. The first round of this branch added one bare Lock/Unlock pair; it now goes through isParked().
  • syncsetup: a window displaced by a newer hold on the same computer lets go at once #2034 still holds: the nine displacement tests (displaced_same_device_test.go, displaced_watch_test.go, including TestWatchDisplacementRetiresTheDriveHoldImmediately/higher_local_fence) pass ×2 with furrow staged. internal/syncsetup is not touched.
  • The touched package and its importers: cmd/codeaf is package 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.
    • 10 are the same names on both trees.
    • Only on the branch: TestAppChatsOlderCopyStaysLocalL1 and TestCellRewindRestoresTheWorkspaceTheSessionSealed. Both are TempDir cleanup races ("unlinkat … directory not empty") and pass ×5 on both trees.
    • Only on app/v3: TestAppPresenceP1ReconnectReconcilesMissedOffline and TestAppTakeModelT1T2T3N1NoUsableDefault. Both pass ×5 on both trees.
    • The branch run was cut off by a session restart 4 tests from its end. Those 4 (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:

  • TestAppTakeF1P2FenceAndPromptPublication and TestAppTakeProcessS6 failed only on the branch, 1 in 5. Timeline of a failure:

    1. A quits, releasing fence 1, then restarts.
    2. agents.open returns at .701.
    3. B takes with "seen fence 1" at .703.
    4. A's reopened drive acquires fence 2 at .715.
    5. B's acquire gets 409 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/startup failed 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 own GET /v1/dir/list and /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 work ConfirmWork does 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:

  • a relay on loopback, with two homes paired through it, and a fake model;
  • P1 = codeaf app --home A, the desktop's engine process, driven over its JSON-lines wire;
  • a terminal on the same home, driven on a pty;
  • the relay's request log, the receipt file's mtime, P1's wire events, and P1's next agents.send.

The fixed side ran on c1ec418, and the rows marked † also on f19c10b.

Case app/v3 this branch
--no-host terminal shows "Move this conversation here?", then ctrl-c ×2. A heartbeat at terminal start and P1's receipt rewritten. Then a release at the quit, the receipt deleted, P1 "This chat is available to continue here.", and P1's next send refused (held_elsewhere). One run's terminal needed SIGTERM after 17 s; the release came at the kill. ×4†. No lease write while open or at the quit, the receipt untouched (same mtime), P1 stays here, and its next send succeeds at fence 1. Quit 2.0–2.3 s.
The same, after "2 leave it there" ×2. One reproduced the bug; the other exited rc 1 without its cleanup. ×1. Nothing written, P1 sendable, rc 0.
Host mode, then codeaf engine --stop of the session host ×2. A heartbeat and the receipt rewritten at terminal start (by the host). Then a release when the host stops and P1 "available"; its next send was refused once and recovered at fence 2 once. A host left running held its drive for P1's chat. ×3†. No write at terminal start or at host stop, and P1 sendable at fence 1.
esc into the fresh chat, send a message there, then ctrl-c ×1. The bug reproduced: P1's fence released and P1 refused. ×1†. The fresh chat creates and publishes its own record and releases only its own lease. P1 untouched and sendable.
"1" + enter: move it here P1 releases and the terminal acquires fence 2, with an extra heartbeat of fence 1 at start and a double release at the move. P1 releases once, the terminal acquires fence 2 once, and P1 shows "moved to another window". The terminal releases its own fence 2 at its quit.

Not verified here:

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.

  • PR gate 40959635711:
    • Light gate: the same 5 failing tests as the base. The commentlaw and lock-defer law output is identical in content; only timings differ.
    • touched codeaf passes on Linux. This includes the engine-free TestAChatAnotherProcessHoldsIsNeitherRenewedNorReleasedFromHere and TestAParkedChatDriveWaitsForItsJournalDecision; the furrow-backed ones skip there. Its one failure, TestRecipeDoorWritesTheWorktreeWhenOnlyTheCheckoutIsHeld (no git identity on the runner), is marked "already failing on the base".
  • Full check 40959638295: 28 failing full tests against the base's 29.
    • New: 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.
    • Gone: internal/session TestTheSourceHoldsEveryRowTheAppLists and the known flake TestR7PersonalLeaderFallbackAndNoAutomaticRegain.
    • Cross builds: all six pass.
  • The earlier round on 145db19 (PR gate 40953662453, Full check 40953668433) matched in the same way. Its 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:

  • Round 1. No blocker or major. One minor: T3's sleeps could fail under load. Fixed: T3 now waits on the goroutine dump and bounds the close by driveCloseWithin/3.
  • Round 2.
    • Major, fixed: the fresh chat offered after a refused open inherited the held chat's seat (it already did on app/v3; this branch had turned that seat's drive into an abandoned one). It now opens through openV3Agent, with a new test that fails on app/v3.
    • Minors, fixed: the dump buffer grows until it is complete; the change entry no longer claims the overlap below is fixed.
  • Round 3. No blocker or major.
    • Take/C6 test changes: the reviewer found that neither hides a product defect. agents.open never promised a settled relay hold, and that ordering already failed on the hosted relay.
    • Minor, rebutted: the seat's warm capture starts its 30 s SealWithin deadline at seat time (cellstore/pipeline.go:140,155), and now waits out session.New as well as the drive's startup. A construction over 30 s could time it out before the drive begins. On app/v3 the same lateStore.Prepare already waited out the drive's startup inside that deadline, so the failure mode is unchanged; only session.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):

  • v3Migrated still 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.
  • The close/open overlap: Agent.Close releases 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

AbirAbbas and others added 4 commits October 29, 2026 16:47
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>
@AbirAbbas
AbirAbbas marked this pull request as ready for review October 29, 2026 23:15
@AbirAbbas
AbirAbbas merged commit 7e40a79 into app/v3 Oct 29, 2026
1 check passed
@AbirAbbas
AbirAbbas deleted the fix/nohost-quit-keeps-app-hold branch October 29, 2026 23:15
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