feat(temporal): Made a session outlive the client that started it. - #8
Draft
moedash wants to merge 24 commits into
Draft
feat(temporal): Made a session outlive the client that started it.#8moedash wants to merge 24 commits into
moedash wants to merge 24 commits into
Conversation
A durable session outlives the process that started it, but nothing on the command line could start one that way, ask what is still running, or follow one from a machine that never had it. These are thin HTTP clients because any serve in the deployment can answer for any session.
Kills the serve that started a turn while a tool is still running, then asks a second serve that never saw the session to report it and replay it. The claim only shows up across processes, so it needs processes.
A fresh host has no tip note, so the check that protects somebody's working copy also refused to build a tree that was never there. The path being present is not the same as the project being present, and a mounted empty directory is the ordinary shape of a machine that has never seen this session.
Each worker is a container with its own filesystem, so a session that moves has to bring its worktree with it out of the shared store. That is the half a single host cannot exercise: there the tree is already on the disk the other process reads.
Two attempts of one activity can be alive at once and they do not arrive in order. A paused attempt 1 that resumed after attempt 2 had claimed took the log back, and every publish from attempt 2's tool activities then died on the fence for a step that was going fine.
A host the store had moved past packed its older files, became the newest by time, and every other host then checked that out over the work they were shipped to carry. It refuses now, ahead of the note and outside the packing, which swallows its own failures on purpose. The read side stopped warning and returning, since running against files the store has moved past tells the model a stale tree is the project. Rollover also reads the server's own suggestion: one drain is a whole turn, so a drain count crosses the history limit late.
Packs were ordered by `time_created`, which is whichever host wrote the row, so a worker with a slow clock made its older tree the newest one and every other host checked that out. They chain onto each other already, and that order no host can get wrong. A tool's writes only reached the store if the seal happened to land on the same host. Each tool ships from the host that ran it now, and a step's tools run one at a time wherever the store is shared, because two on two hosts each publish a tree without the other's work. Also: the deferred call no longer carries its arguments, which were crossing history twice and could pass the payload limit on a big step; an interrupt in the tool phase closes its step instead of publishing nothing for a follower to see; `watch` reconnects rather than reporting a live turn as done when its serve restarts; and a transient store failure retries instead of failing the step for good.
The README described a guard that was not there and an incremental ingest that was not incremental, and it framed affinity as what keeps a step's tools on one tree when it is keyed by the directory every container shares.
Claiming the event log read and wrote as two statements, so two attempts of one activity could both pass the check and the loser land last. It is conditional on what was read now. It also only ordered attempts of the same activity, so a model call paused before it claimed came back after three steps had completed and fenced out the one that was running: activity ids order the units of work within a run. `ensure` gated moving a tree on a marker only a rebuild writes, so a host that seeded the session from its own checkout died on every activity once anyone else shipped, non-retryably. The note is the rule: a host that agreed to a state may be moved off it, and a checkout nobody agreed to has no note. `push` wrote its note before the insert, and the insert's failure is swallowed, so one bad write left the host naming a tree the store never saw and every later ship from it died. The note goes last. Also: an interrupted step was sealed with the model's own finish reason, which for a step that asked for tools reads as "another step follows"; the rollover flag never fired while a queue kept the drain in flight; and the chain walk was one stack frame per capture.
Taking them off was wrong. A deferred call is recorded as pending with an empty input, because the streaming path leaves `Tool.Called` to whoever dispatches it: that publish is the dispatch record the no-double-run rule reads, so it cannot happen before a dispatch. Reading the arguments from the log therefore handed every tool an empty string, and the model spent a turn reporting that its input was not an object. The payload cost is real and stays open. Fixing it means recording the arguments at stream time without recording a dispatch, which is a change to the pending state rather than to this shape. `detached-session-check.sh` passes end to end, including `watch` stopping when the turn does.
The re-check inside the lock exists because the reading before it can be stale, and the cleanup then used the stale one. A drain that materialized the tree while this one waited made it a full checkout, and a failed refresh took the whole directory with the files git ignores in it.
The stream has no replay, so a turn whose last step lands while the client is reconnecting publishes into a gap: nothing arrives afterwards, and a settling flag that starts again per connection means nothing ends the wait. The state now outlives the connection, and a periodic ask of the running set answers the case the wire cannot. Without `--wait` the stream's end is the command's end.
The reason recorded with the revert was wrong: the log does hold them, because
the streaming path ends the input fragment before it defers. What broke was the
type. The pending part holds the raw JSON string, and recording a non-object
back wraps it as `{ value }`, so every tool got a string where it wanted an
object.
It has been wrong in both directions, so the rule and the reason each half of it cannot answer alone are worth stating where the command is described.
The test beside it passes with the condition on the write removed, because two claims started together in one process run one after the other. This one puts the seam at the database instead: both read the current owner before either writes, which is what two attempts over a network store do.
They crossed the boundary twice, once as the model call's result and once as the tool call's input, so a step with large `write` bodies wrote them into history twice. The dispatcher reads them off the recorded call and parses them, which is what the first attempt at this missed: the record holds the provider's raw JSON string. A provider that delivers a call whole streams no input deltas, so the recorded text is seeded from the call itself.
Its tools write the tree that worker is standing in, so sending them back to it lets them see each other through the filesystem rather than by shipping the tree to each other, and they can run together again. Each worker polls a queue of its own, keyed by host as well as directory, because two containers serve the same path and share none of it. A pin nobody answers times out on schedule-to-start, which means the activity never started, so the work moves to the shared queue and what is left of the step goes one at a time from there. Capture and push take a lock per directory, since two tools of one step now end at once.
The cleanup asked a reading taken before the lock, where the re-check inside it exists because that reading can be stale, so a drain that filled the directory while this one waited had its work removed. The check reproduces that with the wait injected and asserts what survives, which is the file no pack carries.
The settings that have to agree were independent variables with no way to ask whether they did, and each one fails as something else: a store only one process can see reads as a worker that never picks anything up. A profile sets them together, a preflight refuses what a fleet cannot be talked out of, and `session doctor` says what a process resolved. Reaching a real cluster is a key or a certificate pair, read from a file, built once for the client and the worker.
A step ending is not a turn ending: a steer or a queued prompt continues the same turn through another step, so everything watching from outside inferred the difference from a finish reason and then a silence. The one place that decides whether the turn continues says so now, for both modes, and `watch` stops on it instead of waiting out a grace window. Live-only: it adds a boundary, not a record, and a stopped or failed turn still ends a follower the other ways.
A session is a row in the store before it is work, and a workflow cannot write one, so there was no schedule or webhook path into a deployment where nothing runs but workers. A firing admits the prompt through an activity and starts the session's own supervisor as an abandoned child, or wakes it when it is already running. The container check waits for a firing to answer, with no client in it.
An address that is not loopback with no credentials is a private network in most deployments and a mistake in some, and nothing here can tell which. Only what cannot work refuses now; the rest is said and got on with.
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.
This PR makes a coding session belong to the deployment instead of to whoever started it.
The durable executor already had every piece needed for this, which is the interesting part. A session is a workflow rather than a process, the running set comes from Temporal visibility, the store is shared, and a live tail re-reads so a subscriber sees work another process is doing. Put together, that means a session can outlive its client. Nothing said so, and nothing on the command line could use it.
So this is a client surface, not new machinery. It adds no dependency on Temporal: the commands are plain HTTP against any serve in the deployment, because the serves are interchangeable.
What is new
opencode session start "<prompt>"hands a prompt over and returns the session id. It holds no terminal, so a turn can be started by something that is not a person at a keyboard.opencode session runninglists what the deployment is executing right now. It reads the executor's own answer, so it survives a restart of whichever process happens to serve the call.opencode session watch <id>follows a session from a machine that has never seen it, and stops when the turn stops. It ends on the model's own finish reason. The running-session set cannot answer that question, because a session stays in it while its supervisor waits out the idle timeout with nothing left to do.All three take
--attach <url>(or$OPENCODE_SERVER). For an interactive terminal rather than a follower,opencode attach <url> --session <id>already existed.What is not here
A turn started from a schedule or a webhook still needs its own entry point;
session startis a command, so something has to run it. And the deployment is still a set of environment variables rather than a supported mode, so defaults, migration-on-deploy and credential distribution remain the operator's problem. Both are called out in the README.Issue for this PR
Issues are disabled on this fork, so there is no number to link. This continues the durable-executor work in #2 and #7.
Type of change
What does this PR do?
Adds
session start,session runningandsession watchunder the existingsessioncommand, so the detached lifecycle the durable executor already supports can be driven from a command line. Addspackages/temporal/scripts/detached-session-check.sh, which proves the claim against real processes. Documents both in the temporal README, along with the deployment shape and what is still missing.The commands are thin HTTP clients on purpose. In a durable deployment any serve reads the shared store and signals the same workflows, so a client needs an endpoint and a session id, never a particular host.
One fix fell out of writing the check: machine-readable output was going to stderr with everything else, so
--jsonwas not pipeable. It goes to stdout now.How did you verify your code works?
detached-session-check.shruns the whole story against real processes, with a Temporal dev server, one standalone worker, two serve processes and one shared store. Serve A starts a turn and is killed with a tool still running. The turn finishes on the worker. Serve B, which never saw the session, reports it running and replays the transcript including the work done while no client existed. Thensession startreturns without waiting,session runninglists it, andsession watchfollows it live and exits when the turn ends. All assertions pass from a clean run.The assertions were mutation-checked rather than trusted. Giving serve B its own
OPENCODE_DBmakes exactly the cross-process ones fail (activereturns{}, the replay is empty, the follower hangs) while the serve-A-and-worker ones still pass, which is what shows the shared store is load-bearing and that the check is testing it. Thewatchexit condition was found the same way: an earlier version asked the running-session set instead of reading the finish reason, and it hung, because a session stays running while its supervisor idles.Typecheck is clean for the changed files. Pre-existing errors in
packages/tui/src/component/dialog-move-session.tsxare untouched by this branch.Checklist
Across two machines (added after the first pass)
packages/temporal/scripts/cross-host-check.shruns the claim against containers: each worker has its own filesystem and hostname, and the store is a real libSQL server. A session writes a file on worker A, worker A's host is killed, and worker B, whose project volume is empty, continues the same session and reads that file back.That found a bug a single host cannot show, and it is the reason this PR now touches
packages/core.WorktreeMaterializer.ensuretreated any existing directory as somebody's working copy. A fresh host has no tip note, sobehindreturned false and the tree was never built. The tools then ran in an empty directory and the model was told a wrong answer, which is worse than a failure. Turn 1 wrote/project/note.txtand read it back; turn 2 of the same session on the other host gotcat: /project/note.txt: No such file or directory, with the packs sitting in the store the whole time.On one host the case never appears. Worker B either already has the project, or has no directory at all, and an absent directory materializes fine, which is exactly what the existing unit test covers. A mounted directory that exists and is empty is the ordinary shape of a machine that has never seen the session.
The fix is to treat an existing but empty directory as absent, which keeps the protection for a real checkout: an empty directory has no work to lose.
packages/core/test/worktree-materialize.test.tsgains that case so CI catches it rather than only the container check.Verified. With the fix, the container check passes end to end and the worker logs
materialized worktree from snapshot packs. Reverting the fix fails it with the original symptom. The unit test goes red on the revert and green with it.bun test test/worktree-materialize.test.tsis 4/4.Two things I got wrong first, both caught by mutation rather than by reading: the first version of the fix changed only the outer guard, and the re-check inside the lock still bailed on an empty directory, so nothing materialized. And the check's own completion poll used
history?limit=400, which the endpoint rejects at 100, so a rejected request looked exactly like a turn that never finished, and its transcript assertion matched turn 1's output for turn 2's result. Both are fixed; the poll now counts step endings rather than matching one, and the outcome is read off worker B's own disk rather than the shared transcript.Still open. A turn started from a schedule or a webhook needs an entry point of its own. The deployment is a set of environment variables rather than a supported mode. And this is one libSQL server, so it shows a shared store over a network rather than one that survives losing a node.
Since this description was written
Three more reviews went over the fixes above, and most of what came back was in them.
A failed rebuild removed a directory another drain had filled. The re-check inside the lock exists because the reading before it can be stale, and the cleanup then asked the stale one. What that costs is not the rebuild, which retries, but the files git ignores in what it removed. The check reproduces it with the wait injected and asserts what survives.
watchwas wrong in both directions, and now has an event to read. It first exited when its stream ended, which is what a serve restart looks like. Gating the exit on the running set then meant never exiting, because that set holds a session for its whole idle period. What ends a turn is now published: a step ending is not a turn ending, since a steer or a queued prompt continues the same turn, so the one place that decides publishessession.next.turn.ended. The settling state also survives a reconnect, and the running set is a periodic backstop for a turn that ended inside a gap.The claim's compare and set has a test that fails without it. The concurrent test beside it passes with the condition removed, because two claims started together in one process run one after the other. The new one puts the seam at the database, so both read the same owner before either writes.
The tool arguments are off the hand-off, correctly this time. They crossed the boundary twice, once as the model call's result and once as the tool call's input. The first attempt at this was reverted because the dispatcher read the recorded input as an object when the log holds the provider's raw JSON string. It parses it now, and a provider that delivers a call whole has its arguments seeded into the record. The test is a probe whose schema wants a field: every other probe takes an empty object, which accepts a wrong input silently.
A step stays on the worker that ran its model call. Its tools write the tree that worker is standing in, so they see each other through the filesystem rather than by shipping the tree, and they run together again. Each worker polls a queue of its own, keyed by host and directory, because two containers serve the same path and share none of it. A pin nobody answers times out on schedule-to-start, so the work moves to the shared queue with nothing run twice, and what is left of that step goes one at a time from there. Capture and push take a lock per directory, since two tools of one step now end at once.
A turn nobody starts.
opencode session schedulecreates a Temporal schedule whose workflow admits the prompt itself and starts the session's supervisor, so a firing needs no client and no serve process.Deploying is a profile rather than a pile of variables.
OPENCODE_TEMPORAL_PROFILE=fleetsets what has to agree, a preflight refuses what a fleet cannot be talked out of, andopencode session doctorsays what a process resolved. Temporal Cloud and mTLS are reachable now, from a key or certificate file.