ADR 0036: run the agent loop as Temporal workflows, behind AGENT_ENGINE - #194
Merged
Conversation
Records the decision and, more usefully, the case for it: what the ADRs since 0002 have had to build to keep a non-durable loop correct. Four state stores and a hold protocol standing in for one property, each a good local decision. ADR 0033 is the sharpest example and the ADR quotes its own numbers — eleven rollouts in fourteen hours, a reply discarded because core NATS has no subscriber, and a closing line saying durability retires the mechanism. States plainly what the change does NOT touch: with AGENT_ENGINE=langgraph, the default, every one of those ADRs remains exactly as it is. Nothing is retracted. Two things I want a reviewer to push back on if they disagree, so both are stated as decisions rather than details: - Credentials must not enter workflow state. Doing only what the LangGraph path does would be WEAKER here, not equal, because anything a workflow holds is written to event history durably and in the clear. Hence the Secret-name indirection and the test that serializes a verdict the way Temporal would. - The engine's own ADR 0001 dropped NATS, and this narrows that: right for agents written for it, wrong for claude-code-swe-agent, which is in production. Bridged execution runs it unmodified. The differences a reviewer should accept or reject INDIVIDUALLY are enumerated rather than smoothed over — including one that looks like a small bug on the existing path (the identity gate is skipped for sub-agent tool calls) and is worth fixing there too. The acceptance test is named as the existing e2e suite, with the one deliberate exception: resilience and rollout-recovery encode losses that stop occurring, so they need re-baselining, and that re-baselining is the evidence rather than a workaround. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018Uj1SJ41DJJ8woZM7fd3DQ
This was referenced Aug 4, 2026
This was referenced Aug 6, 2026
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.
Proposes running the agent loop as Temporal workflows, selected by
AGENT_ENGINE=langgraph|temporal.This PR is the ADR only — docs, no code. It's deliberately first and deliberately standalone so the design can be accepted or rejected before anyone reads ~17k lines of Go. If the answer is "we don't want a second agent loop", this is the only PR that needed to exist.
The argument, briefly
ADR 0002 chose LangGraph because the flow is "stateful, branching, resumable, waiting on asynchronous Job completion mid-turn" — which is accurate, and is also a description of a durable execution engine's product.
Look at what we've had to build since to keep that flow correct while the loop itself isn't durable:
Map— that ADR documents the restart/scale-out loss itself.--save ""on anemptyDir.release.ymldeploys on every push, and that incident recorded eleven rollouts in fourteen hours. Core NATS has no durability, so areplypublished while no orchestrator is subscribed is discarded. The fix — hold the concluding message and re-offer every 10s until acked — is careful and correct and entirely about compensating for a wait that can't survive a deploy. Its own closing line says durability retires it.Each is a good local decision. Together they're four state stores and a hold protocol standing in for one property.
What this does not touch
With
AGENT_ENGINE=langgraph— the default — every one of those ADRs stays exactly as it is. Nothing is retracted.agent-orchestratorkeeps the OpenAI facade,/invoke, identity and RBAC resolution, the authorization pre-flight (0030/0031), the credential store (0034), both launchers, and session pages. The flag chooses only whether a turn runsbuildAgentGraph()or is forwarded to the engine.AgentGraphLikeis already the Server's dependency, so it's a second implementation of an existing interface rather than a refactor.That split is the whole reviewability argument: the claim is about the loop, and every layer above it is where the last six weeks of work landed.
Three things I'd like you to push back on if you disagree
Pod agents keep their NATS channel. The engine's own ADR dropped NATS. That's right for agents written for it and wrong for
claude-code-swe-agent, which is in production — 0026's tunnel, 0028's tool calls and 0033's hold all live on that channel. So a bridged style drives an unmodifiedAgentRunover the existing protocol: same image, same CR, same subjects, with a workflow holding the durable half. Nothing in production gets rewritten as a precondition.Credentials must not enter workflow state. Doing only what the LangGraph path does would be weaker here, not equal — anything a workflow holds is written to Temporal event history, durably and in the clear. So the engine's pre-flight writes values into a Secret and returns only its name.
Enabling it is two steps, so the engine can be deployed, watched and rolled back before a single turn depends on it.
Also worth knowing
Temporal is already running on the target platform, so the subchart takes an address and bundles no server. That was the strongest argument against this whole thing and it doesn't apply.
The ADR enumerates the behavioural differences individually rather than smoothing them over — including one that looks like a small bug on the current path: the ADR 0032 §5 identity gate covers the planner's
runToolbutdispatchResolvedToolhas no equivalent, so a Tool meant to act as a specific human runs with whatever static token its template carries. Worth fixing here regardless of this proposal.Sequence, if you want it
git subtree, history intact), CI + release wiringAGENT_ENGINEswitch, defaultlanggraphenabled: falsePR 5 is not written yet, on purpose: its precondition is the
e2e/suite run whole underAGENT_ENGINE=temporal, and writing it before that evidence exists would assert a result nobody has. Two expectations there, judged differently — everything must pass unchanged, exceptresilienceandrollout-recovery, which encode losses that stop occurring and so need re-baselining. That re-baselining is the evidence, not a workaround.Happy to close this and stop here if the answer is no.
🤖 Generated with Claude Code
https://claude.ai/code/session_018Uj1SJ41DJJ8woZM7fd3DQ