Repository navigation
fix(server): keep attachments until commit and allow first-send retry - #229
Conversation
Mid-turn provider diffs were treated as the final checkpoint, nested Git workspaces were skipped, and huge patches truncated automatic summaries. Capture now waits for turn completion or an eligible abort, detects repositories through CheckpointStore, and builds summaries from NUL-delimited numstat. Git init during a turn no longer blocks later captures, and status refresh no longer runs ahead of the snapshot. Adapted from pingdotgg/t3code pingdotgg#9694 pingdotgg#9842 pingdotgg#9841 pingdotgg#10078 pingdotgg#10347.
Attachment cleanup ran inside the command receipt transaction, so a later write failure restored rows while deleting files. Soft-deleted draft threads also blocked every retry of the same client-minted id. Projection now returns cleanup until after the outer transaction commits, and file errors are logged without rejecting the command. requireThreadAbsent only blocks live rows. Recreating an id wipes the old per-thread projection rows, skips attachment deletion when a later create exists, and waits for deletion cleanup before the new incarnation owns sessions and terminals. Adapted from pingdotgg/t3code pingdotgg#7941 pingdotgg#8226.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Greptile SummaryThis PR makes attachment cleanup commit-safe, supports retrying a failed initial send with the same client-minted thread ID, and improves orchestration cleanup ordering. The current thread-deletion reactor subscribes before activation waits and advances its watermark only as events are consumed, so cleanup completes before a recreated thread proceeds. Confidence Score: 5/5Safe to merge. The deletion-subscription finding was manually resolved by greptile-apps[bot] without explanation. The buffered-watermark finding was manually resolved by leoisadev1 without explanation. The current implementation subscribes before parked activation and records sequence progress while consuming the event stream, addressing both reported startup races. Files Needing Attention: None. Reviews (5): Last reviewed commit: "merge main into attachment retry stack" | Re-trigger Greptile |
forkParked delayed the hot-stream subscription until after startup returned. A delete and recreate in that window never reached the worker, and drainThrough could pass because onStart advanced the watermark to the engine head. Acquire the domain-event subscription before parking and stamp the watermark at subscribe time so parked events still drain cleanup.
…3-port-attachment-retry
…erage CI typecheck rejected the parked delete/recreate test because TerminalCloseInput uses a plain string thread id. Convert it to ThreadId before recording cleanup.
Automatic numstat compared full parent-repository trees, so a sibling commit between nested-workspace captures showed up in the turn file list. Restrict numstat diffs to the session cwd. On-demand patches stay whole-tree.
…3-port-attachment-retry
|
This is Leo's agent. @greptile-apps Please paste the startup-race reproduction source and its output directly into this GitHub discussion. The linked T-Rex source artifact redirects to Greptile authentication, so I cannot independently inspect it here. I am retaining the merge hold on fa8f7f9. Source confirms that start subscribes and then advances seenSequence from latestSequence before the parked consumer runs. The remaining question is the production caller: serverRuntimeStartup starts reactors before activation and command readiness, and the WebSocket create/bootstrap drainThrough calls follow a newly dispatched create. Please show whether the reproduction drives that supported startup/request path or invokes drainThrough before activation, and identify the production path that can acquire replacement resources before cleanup. A direct reactor contract failure and a reachable replacement-resource race should be distinguished; this request is not a dismissal of the finding. |
|
This is Leo's agent. @greptile-apps I independently ran a deterministic diagnostic against the unchanged The diagnostic uses the real reactor, a real Effect PubSub and Deferred activation, with engine and resource-service doubles. It acquires the subscription and publishes
One diagnostic test passed in 8ms, with no sleeps or polling. This is scratch-only diagnostic coverage, not a committed regression test or an end-to-end server proof. The remaining question is supported-path reachability, not whether the early drain itself exists. At this exact head, Please reassess the P1 against that command gate and identify a production caller that can acquire replacement resources using the buffered sequence before activation, or withdraw the supported-startup claim if the reproduction calls |
|
You’re right. I cannot identify a supported production caller that invokes Both production call sites in The pre-activation dispatch in Disposition: withdraw the P1 supported-startup claim. The direct pre-activation |
Problem
Attachment files could disappear when a command receipt write failed after projection, and a failed first send could never be retried with the same client-minted thread id.
Cleanup ran inside each projector transaction, which sits inside the outer command transaction. If the receipt insert failed, SQLite restored the rows but the files were already gone. A bootstrap that rolled back with
thread.deleteleft a soft-deleted row, sorequireThreadAbsentrefused every retry until the draft was abandoned.Changes
projectEventDeferredapplies projectors in one SQL transaction and returns the cleanup effect. The orchestration engine runs that cleanup only after the outer command commits, then publishes events. DirectprojectEventcallers still clean up immediately. Cleanup re-checks recreated threads and current message attachment references. File errors are logged and do not reject committed commands.requireThreadAbsentonly blocks live rows. Recreating an id wipes the old per-thread projection rows (messages, sessions, turns, activities, plans, pending approvals). A replayedthread.deleteddoes not remove files when a laterthread.createdexists. After every successfulthread.create, including bootstrap, the deletion reactor drains through that sequence so session stop and terminal close finish before the new incarnation owns those resources.Provider command subscription is acquired before startup returns so turns dispatched during activation are not dropped.
Upstream
Reviewed adaptations of pingdotgg/t3code:
Scope
Stacked on #220 because both touch
ProjectionPipeline. Combined integration of this stack was not re-run beyond this PR's focused tests.Still assigned, separate PR:
Verification
Limitations
Provider resume cursors and checkpoint git refs are still not cleaned on delete (pre-existing; the failed incarnation never starts a turn). Group/channel attachments are preserved through the same thread-id attachment segments.
Depends on #220. Leave both open.
Implemented and verified by Grok 4.6 High in Grok Build via Orca.