Skip to content

feat(runner): session keep-alive across turn boundaries (flag off by default)#5156

Merged
mmabrouk merged 1 commit into
big-agentsfrom
feat/session-keepalive-pool
Jul 8, 2026
Merged

feat(runner): session keep-alive across turn boundaries (flag off by default)#5156
mmabrouk merged 1 commit into
big-agentsfrom
feat/session-keepalive-pool

Conversation

@mmabrouk

@mmabrouk mmabrouk commented Jul 8, 2026

Copy link
Copy Markdown
Member

Context

On 2026-07-07 two production conversations failed the same way. In one, the user approved a curl command three times before it ran, because each approval round rebuilt the command with slightly different bytes. In the other, the agent restarted its task five times and executed zero commits. The root cause is shared: the runner destroys the harness session at the end of every turn, so each turn cold-starts a fresh agent that only sees a lossy text summary of its own history. Full analysis: cold-replay-failure-report.md. Design and review trail: #5153.

This PR is slice 1 of the fix: keep the harness session alive for a short TTL after a turn ends. If the next message in the same conversation arrives inside the window and nothing changed, the runner continues the live session and sends only the new user text. The harness keeps its full native memory because the process never died. Anything else falls back to today's cold replay. Approval parking is slice 2 (stacked on this branch).

What changed

  • New services/runner/src/engines/sandbox_agent/session-pool.ts: the session pool, the config and history fingerprints, the credential epoch, and the keep-alive config.
  • runSandboxAgent split into acquireEnvironment (session-scoped; builds one idempotent destroy() incrementally, so a half-built environment cannot leak) and runTurn (per-turn). With the flag off, runSandboxAgent composes them and behaves exactly as before.
  • Listeners attach once per session and route events into the active turn's sink. A per-turn detach/attach model would drop events and auto-cancel permission requests in the gap between turns.
  • services/runner/src/server.ts: the continue-vs-cold dispatch, plus pool drains on POST /kill and on shutdown (the existing shutdown path only destroys sandboxes; the pool drain runs the full cleanup).
  • mount.ts surfaces the mount's project_id so the pool key is project-scoped. No project scope means no parking.
  • Rebased on current big-agents. The time-based run-limits controller (big-agents 8e99171aff) is integrated at the runTurn level: a tripped total/idle/TTFB/per-tool-call limit ends the turn as an error so the caller's teardown reclaims the sandbox; a human pause retires the deadlines. The base branch's own run-limits tests pass against this placement.
  • The session event demux was refactored for readability after review: acquireEnvironment names the environment explicitly and routes events through routeSessionEventToActiveTurn / routePermissionRequestToActiveTurn.

Scope and risk

  • Flag off by default (AGENTA_RUNNER_SESSION_KEEPALIVE). With the flag off, behavior is byte-identical to today: the pool is never consulted and teardown runs in the same place, in the same order. All 620 pre-existing tests pass unchanged.
  • Local sandbox only. Daytona requests never park (an idle remote sandbox costs money; that is slice 3, deferred).
  • Runner only. No wire change, no SDK change, no frontend change, no storage change.
  • Not covered here: approval parking (slice 2), memory across runner restarts (the session-resume design), multi-replica routing (a pool miss just runs cold).
  • A validation failure can never fail a turn. Every mismatch (config, history, credentials, busy, dead session) degrades to cold replay, which is today's path.
  • Secret hygiene: the credential epoch hashes resolved secret values process-locally; the hash and the values never reach logs, events, or errors.

How to QA

Prerequisites: a local dev stack with the runner, and a playground conversation on a local-sandbox agent. Confirm mount signing works first: without mount credentials keep-alive silently runs all-cold by design (no project scope means never park), so a broken mounts table (for example the oss000000006 meta-column drift on stale dev DBs) masks the feature. A [keepalive] park line in the runner log proves signing works.

  1. Leave AGENTA_RUNNER_SESSION_KEEPALIVE unset. Run a two-turn conversation. Behavior and logs match today; no [keepalive] lines appear.
  2. Set AGENTA_RUNNER_SESSION_KEEPALIVE=true on the runner and restart it.
  3. Run turn 1 of a conversation. After the turn ends, the runner logs [keepalive] park ....
  4. Send turn 2 within 60 seconds. The runner logs [keepalive] hit-continue ... and the same live session answers (no new sandbox start in the logs). Ask the agent what it did in turn 1; it answers from native memory. This works after tool-using turns too. Measured on the dev box: turn 1 cold 25.61 s, turn 2 hit-continue 3.12 s.
  5. Wait past 60 seconds (AGENTA_RUNNER_SESSION_TTL_MS) and send another message. The runner logs [keepalive] expire ... then [keepalive] evict ... reason=expire and runs cold, exactly as today (measured: 22.6 s, answer still correct).
  6. Edit an earlier message and resend: [keepalive] mismatch (history), then cold.
  7. Change the model or tools between turns: [keepalive] mismatch (config), then cold.
  8. POST /kill drains the pool: [keepalive] destroy ... for every parked session.

Test command: cd services/runner && pnpm test (683 tests at this PR's head: 620 pre-existing unchanged, 56 new keep-alive tests, plus the run-limits tests that landed on big-agents) and pnpm run typecheck.

Edge cases covered by tests: TTL expiry, LRU eviction at pool-max (never evicts a busy session), a second request racing a busy session (supersede, identity-checked so cleanups cannot destroy a successor's session), fingerprint mismatches, credential rotation and expiry, no-mount-no-park, client disconnect never parks, a paused turn never parks in this slice, mid-acquire failure cleanup, double destroy, exactly-once mount signing, /kill and shutdown drains, and flag-off identity.

https://claude.ai/code/session_01AumZJ9xRd4XYNHThqy4rTv

@vercel

vercel Bot commented Jul 8, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
agenta-documentation Ready Ready Preview, Comment Jul 8, 2026 3:31pm

Request Review

@coderabbitai

coderabbitai Bot commented Jul 8, 2026

Copy link
Copy Markdown
🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Title check ✅ Passed The title clearly summarizes the main change: runner session keep-alive across turn boundaries with default-off rollout.
Description check ✅ Passed The description is directly about the session keep-alive runner changes and rollout scope described in the diff.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feat/session-keepalive-pool

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

// The one complete, idempotent teardown — the same steps the old per-run `finally` ran, in the
// same order. Every resource is null-checked, so it is safe after a partial acquire and safe to
// call twice (the guard returns on a second call). It must never throw.
env2.destroy = async () => {

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is the teardown-deferral seam the whole feature hangs on. The old per-run finally block became this one closure, built up incrementally during acquire, so a keep-alive park just defers calling it instead of skipping steps. Reviewers: compare it step by step against the old finally; the order and the null-check conditions are intentionally identical.

// (the sandbox-agent registries are plain Sets; a thrown handler would corrupt the stream).
// Deliberate divergence from the old inline handler: a handler throw is swallowed + logged
// here instead of propagating, because the listener now outlives any single turn.
env2.session.onEvent((event: any) => {

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Listeners attach once here, for the life of the session, and demux into env.currentTurn. Per-turn detach/attach would be wrong for this package: the listener registries are plain Sets, an event with no listener is silently dropped, and a permission request with no listener is auto-cancelled, so any detach window is a drop/cancel window.

// after the prompt; stop is safe to repeat, matching the old finally). Null it afterwards so
// a later `destroy()` — possibly after the dispatch cleared the sink — cannot double-stop or
// orphan it.
await activeTurn?.toolRelay?.stop().catch(() => {});

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The turn owns its relay on every exit path, including the ok:false catch return. Without this, the dispatch clearing the sink after a failed turn would orphan the relay so destroy() could no longer reach it. Found in review; pinned by a test.

* pruned but contributes neither text nor ids — the prediction is deterministic either way
* (assistant text is not hashed). An id divergence still trips a mismatch and falls to cold.
*/
export function expectedNextHistoryFingerprint(

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Park-time prediction of the NEXT request's history fingerprint: fold in the user text just consumed and the tool call ids the turn just emitted. Tool parts count as answer parts in the frontend's pruning, so a tool-calling turn always survives pruning and its ids will be in the next request. This is what lets tool-using conversations (the feature's main audience) continue live instead of falling cold.

mountExpiresAtMs?: number;
}

export function computeCredentialEpoch(

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Credential epoch: a parked session must not outlive its baked credentials. The wire carries raw secret values and no version identity, so this hashes the values process-locally (never logged, persisted, or emitted) plus the mount expiry; a same-slug rotation changes the hash and evicts to cold.

* null when there is no session id or no mount project id — such a request MUST NOT park (there
* is no safe key that separates callers), and the dispatch runs it fully cold.
*/
export function poolKeyFor(

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The pool key is project-scoped via the mount-sign response, not the caller-supplied session_id alone, so two projects can never share a live harness process. No mount project id means no safe scope, so the session is never parked at all.


const runAgent: RunAgent = (request, emit, signal, options) => {
const config = readKeepaliveConfig();
if (!config.enabled) return runSandboxAgent(request, emit, signal);

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The flag gate: off (the default) returns to runSandboxAgent directly, so the pool is never consulted and teardown runs in-turn exactly as before the refactor. Byte-identity is proven by the 620 pre-existing tests passing unchanged.

clientGone: (() => boolean) | undefined,
): boolean {
if (signal?.aborted) return false; // aborted run: destroy, do not park
if (clientGone?.()) return false; // client disconnected mid-turn: destroy, do not park

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Session-owned streams intentionally survive a client disconnect without aborting the run, so the park decision cannot use the abort signal; this separate clientGone flag is what enforces the plan's rule that a disconnected client's session is destroyed, never parked.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1


ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 518a738e-feec-42d7-9a77-0ca4652fb020

📥 Commits

Reviewing files that changed from the base of the PR and between aeb6135 and 8dd2ee5.

📒 Files selected for processing (8)
  • services/runner/src/engines/sandbox_agent.ts
  • services/runner/src/engines/sandbox_agent/mount.ts
  • services/runner/src/engines/sandbox_agent/session-pool.ts
  • services/runner/src/server.ts
  • services/runner/tests/unit/server.test.ts
  • services/runner/tests/unit/session-keepalive-dispatch.test.ts
  • services/runner/tests/unit/session-keepalive-engine.test.ts
  • services/runner/tests/unit/session-pool.test.ts

Comment on lines +506 to +508
plan.isPi && !plan.isDaytona ? `${plan.relayDir}.otlp-auth` : undefined;
const otlpAuthorization =
request.telemetry?.exporters?.otlp?.headers?.authorization;

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Are OTLP headers normalized to lowercase before reaching the runner?
rg -nP --type=ts -C3 'otlp\?\.headers|exporters\?\.otlp|headers\?\.[Aa]uthorization' services/runner/src

Repository: Agenta-AI/agenta

Length of output: 4366


🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Inspect the relevant helpers and request shape to see whether OTLP headers are normalized
# before they reach sandbox_agent.ts.
sed -n '120,180p' services/runner/src/engines/sandbox_agent.ts
printf '\n---\n'
sed -n '500,520p' services/runner/src/engines/sandbox_agent.ts
printf '\n---\n'
sed -n '940,956p' services/runner/src/engines/sandbox_agent.ts
printf '\n---\n'
rg -n --type=ts 'headers\.authorization|headers\.Authorization|otlpAuthFilePath|AGENTA_AGENT_OTLP_AUTH_FILE|exporters\.otlp\.headers' services/runner/src

Repository: Agenta-AI/agenta

Length of output: 5982


🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Find the AgentRunRequest type and any code that may normalize telemetry headers.
rg -n --type=ts 'interface AgentRunRequest|type AgentRunRequest|telemetry.*headers|headers.*normalize|authorization ?? headers\.Authorization|Authorization' services/runner/src

Repository: Agenta-AI/agenta

Length of output: 3062


🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Inspect the helper and surrounding code paths that consume OTLP headers.
sed -n '132,146p' services/runner/src/engines/sandbox_agent.ts
printf '\n---\n'
sed -n '500,512p' services/runner/src/engines/sandbox_agent.ts
printf '\n---\n'
sed -n '946,954p' services/runner/src/engines/sandbox_agent.ts
printf '\n---\n'
rg -n --type=ts 'headers\.authorization|headers\.Authorization|exporters\.otlp\.headers|AgentRunRequest' services/runner/src

Repository: Agenta-AI/agenta

Length of output: 6870


🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Inspect the request type and server-side normalization, if any.
sed -n '1,260p' services/runner/src/server.ts
printf '\n---\n'
rg -n --type=ts 'telemetry.*headers|headers.*Authorization|headers.*authorization|normalize.*header|lowercase' services/runner/src/server.ts services/runner/src/engines

Repository: Agenta-AI/agenta

Length of output: 11911


Mirror the dual-case OTLP header lookup here. runCredential already accepts both authorization and Authorization; this guard only reads lowercase, so a capitalized header skips writing the Pi OTLP auth file.

mmabrouk added a commit that referenced this pull request Jul 8, 2026
Adds AGENTA_RUNNER_SESSION_KEEPALIVE, _SESSION_TTL_MS,
_SESSION_APPROVAL_TTL_MS, and _SESSION_POOL_MAX to the canonical agent
env-var list. The flags land across the two keep-alive slices (#5156,
#5158); this stacked lane carries the doc so both are in by the time it
applies.

Claude-Session: https://claude.ai/code/session_01AumZJ9xRd4XYNHThqy4rTv
if (existing) {
this.clearTimer(existing);
this.sessions.delete(input.key);
void this.safeDestroy(existing);

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[P2] Await replaced same-key session teardown before parking

When two cold turns for the same <projectId>:<sessionId> both miss and finish near each other, the second park() replaces the first entry but starts its destroy fire-and-forget. Both environments use the same durable cwd/mount, so the old session can unmount/delete it after the new session has been parked, breaking the parked successor; make this replacement path awaitable like evict() before taking the slot.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Verified and fixed. park() is now async and awaits the replaced same-key session's teardown before taking the slot, exactly like evict(). The old fire-and-forget destroy could unmount the shared durable cwd/mount out from under the freshly parked successor.

The replace branch now deletes the old entry, awaits its safeDestroy, then does the cap check and insert. Updated the two server.ts callers (parkFreshOrDestroy) to await, plus the four return-asserting session-pool tests. Added a test that gates the replaced session's destroy behind a manual resolver and asserts the successor is NOT discoverable in the pool until that teardown completes, then takes the slot once it does.

Landed on feat/session-keepalive-approvals (commit f099be1) alongside the other two fixes; lands with the stack bottom-up into big-agents.

@mmabrouk

mmabrouk commented Jul 8, 2026

Copy link
Copy Markdown
Member Author

Review findings from the runner keep-alive refactor:

  • [P1] Restore run-limit enforcement in the split turn path.
  • [P2] Do not retry after streaming failed continuation events.
  • [P2] Await replaced same-key session teardown before parking.

I also added each finding as an inline PR comment on the relevant diff line.

klog(`evict (continuation-failed) key=${key}; retry cold`);
live.environment.clearTurn();
await pool.evictIfCurrent(live, "continuation-failed");
return coldAndPark();

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[P2] Do not retry after streaming failed continuation events

On a keep-alive hit with streaming emit, runTurn can already have emitted chunks or an error event before returning {ok:false}. This branch then retries cold on the same response, so clients and persistence can receive an error or partial output from the failed live session followed by a successful cold answer; retry only before anything is emitted, or buffer/suppress the first attempt.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Verified and fixed. A failed keep-alive hit now retries cold ONLY when nothing has reached the client yet.

runWithKeepalive wraps the client emit in a tracker (trackedEmit) that flips a flag the first time any event is emitted. On a continuation or resume that fails (throw or ok:false):

  • if the failed live turn already streamed (a partial answer or an error event), the dispatch evicts the broken session and returns the failure as-is, so no cold turn re-emits a second, contradictory answer to the client and persistence
  • if nothing was emitted yet, it retries cold as before

In buffered mode (emit undefined) nothing is ever streamed, so the resilient cold retry is preserved unchanged. All four paths are gated (continuation-failed/threw, resume-failed/threw). New test in session-keepalive-dispatch.test.ts pins the streaming case (exactly one delta reaches the client, no cold retry); the existing buffered-mode retry test still passes.

Landed on feat/session-keepalive-approvals (commit f099be1) since this edits the slice-2 resume branch; lands with the stack bottom-up.

if (!acquired.ok) return { ok: false, error: acquired.error };
const env = acquired.env;
try {
return await runTurn(env, request, emit, signal);

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[P1] Restore run-limit enforcement in the split turn path

For every run, including the default flag-off path, runTurn now receives emit directly and the split removed the createRunLimits controller, wrapped emitter, and merged deadline abort signal that used to surround the prompt. That means configured total/idle/TTFB/per-tool-call limits no longer abort silent or hung harnesses, so those requests can run until the process is killed instead of being cleaned up.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Verified and fixed. The run-limits controller (run-limits.ts / createRunLimits) was added to big-agents in commit 8e99171 AFTER this lane branched from e8e8237, so the acquire/runTurn split never carried it. That is why the split path has no deadline: it is a missing merge integration, not a deliberate removal.

I re-integrated the controller into runTurn (the per-turn unit):

  • wraps the emit sink for idle/TTFB progress
  • tracks per-tool-call deadlines in handleUpdate (before the pause-suppress check, as on big-agents)
  • retires on a human pause via notePaused (a HITL wait is not a wedge)
  • disposes on every exit path

A tripped limit resolves the prompt race with a sentinel and ends the turn as an error, so the caller's teardown (runSandboxAgent's finally, or the dispatch's evict-on-failure) reclaims the wedged sandbox exactly as any other failure does. run-limits.ts and its unit test came over byte-identical from big-agents so they merge cleanly; two integration tests in sandbox-agent-orchestration.test.ts pin the split wiring (a wedged never-responding turn is reclaimed as an error; a paused turn is exempt even past every window).

Landed on the stacked lane feat/session-keepalive-approvals (commit f099be1) because a sibling fix in the same commit edits slice-2 code; it lands when the stack merges bottom-up into big-agents. 723 runner tests green, typecheck clean.

@mmabrouk

mmabrouk commented Jul 8, 2026

Copy link
Copy Markdown
Member Author

Thanks for the review. All three addressed:

  • P1 (run limits): re-integrated the time-based run-limits controller into the acquire/runTurn split. It was added to big-agents in 8e99171 AFTER this lane branched, so the split never carried it (a missing merge integration). A tripped total/idle/TTFB/per-tool-call limit now ends the turn as an error and the caller's teardown reclaims the wedged sandbox.
  • P2 (streaming retry): a failed live continuation/resume no longer retries cold once anything has streamed to the client; it returns the failure instead. Buffered mode keeps the cold retry.
  • P2 (same-key teardown): park() now awaits the replaced same-key session's teardown before taking the slot, like evict().

All landed as one commit f099be1 on the stacked lane feat/session-keepalive-approvals (#5158), not the pool lane, because the streaming-retry fix edits slice-2's approval-resume branch. It lands with the stack bottom-up into big-agents, so PR #5158's diff carries these fixes rather than #5156's. 723 runner tests green, typecheck clean, flag still off by default, no wire/protocol change.

* id today), so session keep-alive keys its pool on `<projectId>:<sessionId>`. Absent when the
* response omitted the mount object; keep-alive then refuses to park (no safe key source).
*/
projectId?: string;

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

as mentioned previously does not seem to be the right place for this unless i am missing something

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You are right that this mixes concerns: the mount is a storage credential, and here it doubles as the project authorization scope. That is a real smell, and we settled it in the #5153 review (see Decision 1 in architecture-notes.md).

It lives here for now because of a wire fact: no project id rides the live /run request today. The nominal field is never populated by the SDK and never read by the runner. So the mount-sign response (POST /sessions/mounts/sign, answered by the API) is the only project scope the runner can verify on this request. That is why the pool keys on <projectId>:<sessionId> from the mount, with the hard rule that no mount project id means no parking at all (never a cross-project reuse).

The clean long-term home is a server-stamped project id in runContext, written by the service and never caller-supplied. That is recorded as follow-up 5 in docs/design/agent-workflows/projects/session-keepalive/ and is deliberately out of scope for slice 1. Slice 1 keeps the mount-derived scope so the safety property (never share a live harness across projects) ships now. Full reasoning is in #5153 Decision 1.

const doFetch = deps.fetchImpl ?? fetch;
const api =
deps.ngrokApi ?? process.env.AGENTA_MOUNTS_TUNNEL_API ?? "http://ngrok:4040";
deps.ngrokApi ??

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why are we using ngrok here (I realize it is from before). please show the blame and reasoning

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Blame: this function is Juan Pablo's, from commit 98a6c5c (2026-06-30), part of the M9 remote-mount work. This PR only carried it through the services/agent to services/runner rename; the ngrok default is not introduced here.

Reasoning: a remote sandbox (Daytona/E2B) runs off-host and cannot reach seaweedfs:8333 on the compose network, so geesefs inside it needs a public URL for the store. The ngrok service (compose profile remote) tunnels seaweedfs and lists its active tunnels at http://ngrok:4040/api/tunnels, which this reads. When no tunnel is up it returns null and the remote mount is skipped, not fatal.

For this slice it is dormant: session keep-alive is local-sandbox only, and local runs mount geesefs on-host and never call discoverTunnelEndpoint. The endpoint is overridable via AGENTA_MOUNTS_TUNNEL_API. Replacing ngrok with a first-party tunnel is a fair cleanup, but it belongs with the remote-mount path, not this PR.

version: workflow.revision.version ?? null,
}
: null,
isDraft: workflow?.is_draft ?? null,

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why would care for this

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is inside configFingerprint, the hash that decides whether the next turn may continue on the parked live session or must fall back to cold. It has to change whenever anything that would make the harness behave differently changes.

workflow.revision (id plus version) identifies which variant config the session was built for: model, prompt, tools, and so on. If the user switches to a different revision, or bumps the same revision to a new version between turns, that is a config change, so the old live session must not answer. Including the version means a revision bump invalidates reuse and we cold-start with the new config, instead of replying from a session built for the previous one. On any mismatch we log mismatch (config) and run cold, which is today's path, so the worst case is no worse than before the feature.

@mmabrouk
mmabrouk marked this pull request as ready for review July 8, 2026 13:04
@dosubot dosubot Bot added size:XXL This PR changes 1000+ lines, ignoring generated files. Backend Feature Request New feature or request tests labels Jul 8, 2026
mmabrouk added a commit that referenced this pull request Jul 8, 2026
…ct helpers)

Readability-only, no behavior change. Renames the SessionEnvironment `env2` to
`environment` and extracts the two session-lifetime listeners into documented
`routeSessionEventToActiveTurn` / `routePermissionRequestToActiveTurn` helpers, so
`acquireEnvironment` reads as clear steps and the demux data flow is obvious.
Addresses Mahmoud's #5156 review note that the event handling was hard to read.
723 tests unchanged, typecheck clean, reviewer confirmed no semantic drift.

Lands on the stacked approvals lane (top of the keep-alive stack) because the working
tree is the stack's applied state and GitButler attributes the edit to the top lane;
it rides into big-agents with the stack.

Claude-Session: https://claude.ai/code/session_01AumZJ9xRd4XYNHThqy4rTv
@mmabrouk

mmabrouk commented Jul 8, 2026

Copy link
Copy Markdown
Member Author

Addressed your readability note on the event handling here.

What changed. The confusing part was the env2 name plus the two inline listener bodies in acquireEnvironment (the onEvent / onPermissionRequest demux, a ~50-line inline dump). I renamed env2 to environment (it is the SessionEnvironment) and extracted the two listeners into small documented helpers, routeSessionEventToActiveTurn and routePermissionRequestToActiveTurn, so the wiring reads as two clear one-liners and the data flow is obvious: harness event -> demux -> the active turn's sink, with explicit between-turns handling. Readability only, no behavior change. 723 tests unchanged, typecheck clean, and a separate reviewer pass confirmed no semantic drift.

Where it landed. On the stacked approvals lane (#5158, commit 7551a87), not this PR. The working tree is the applied state of the whole keep-alive stack, and GitButler attributes an edit to the stack's top lane; pushing it down to this lower lane cleanly would need an unapply/reapply of the approvals lane, which is risky in the current shared, dirty workspace. The refactored demux is the same code in both PRs and rides into big-agents with the stack. If you would rather see it isolated on this PR, say the word and I will do the lane surgery on a quiet workspace.

On the merge. This PR conflicts with big-agents because Juan Pablo's run-limits work (8e99171) landed after this lane branched, and this lane restructured the same function (the acquire/runTurn split). Resolving it cleanly means advancing the lane's base onto the current big-agents tip, which is a workspace-wide but pull. The GitButler workspace is shared and hot right now (other sessions mid-commit, dirty worktree, target still pinned at the old big-agents tip), so I have not run it. The run-limits integration itself is already done and you approved it on #5158 (f099be1). Recommended: advance the base on a quiet workspace, resolve the single sandbox_agent.ts conflict by keeping the runTurn run-limits integration from #5158, then merge bottom-up.

@mmabrouk
mmabrouk force-pushed the feat/session-keepalive-pool branch from 8dd2ee5 to 2084078 Compare July 8, 2026 15:23
mmabrouk added a commit that referenced this pull request Jul 8, 2026
Adds AGENTA_RUNNER_SESSION_KEEPALIVE, _SESSION_TTL_MS,
_SESSION_APPROVAL_TTL_MS, and _SESSION_POOL_MAX to the canonical agent
env-var list. The flags land across the two keep-alive slices (#5156,
#5158); this stacked lane carries the doc so both are in by the time it
applies.

Claude-Session: https://claude.ai/code/session_01AumZJ9xRd4XYNHThqy4rTv
mmabrouk added a commit that referenced this pull request Jul 8, 2026
…ct helpers)

Readability-only, no behavior change. Renames the SessionEnvironment `env2` to
`environment` and extracts the two session-lifetime listeners into documented
`routeSessionEventToActiveTurn` / `routePermissionRequestToActiveTurn` helpers, so
`acquireEnvironment` reads as clear steps and the demux data flow is obvious.
Addresses Mahmoud's #5156 review note that the event handling was hard to read.
723 tests unchanged, typecheck clean, reviewer confirmed no semantic drift.

Lands on the stacked approvals lane (top of the keep-alive stack) because the working
tree is the stack's applied state and GitButler attributes the edit to the top lane;
it rides into big-agents with the stack.

Claude-Session: https://claude.ai/code/session_01AumZJ9xRd4XYNHThqy4rTv
…flag off by default)

Slice 1 of the session-keepalive plan (#5153). When
AGENTA_RUNNER_SESSION_KEEPALIVE is on and the sandbox is local, the runner
parks the live harness session for a TTL after a turn ends and continues
it on the next matching message, so the harness keeps its native memory.

- new session-pool.ts: project-scoped pool (mount.project_id : session_id),
  config + history fingerprints (pruned-array contract), credential epoch
  (process-local value hash + mount expiry), TTL/LRU, complete idempotent
  destroy per session, drained on /kill and shutdown
- sandbox_agent.ts: acquireEnvironment/runTurn split with incremental
  finalizers; session-lifetime listeners demux into the current turn's sink
- server.ts: continue-vs-cold dispatch; any mismatch degrades to cold;
  disconnect/pause/Daytona/no-mount never park; busy race supersedes
- flag off (default) is byte-identical: pool never consulted, teardown
  unchanged; all 620 pre-existing tests pass unchanged; 56 new tests

Claude-Session: https://claude.ai/code/session_01AumZJ9xRd4XYNHThqy4rTv
@mmabrouk
mmabrouk force-pushed the feat/session-keepalive-pool branch from 2084078 to 8d774de Compare July 8, 2026 15:30
mmabrouk added a commit that referenced this pull request Jul 8, 2026
Adds AGENTA_RUNNER_SESSION_KEEPALIVE, _SESSION_TTL_MS,
_SESSION_APPROVAL_TTL_MS, and _SESSION_POOL_MAX to the canonical agent
env-var list. The flags land across the two keep-alive slices (#5156,
#5158); this stacked lane carries the doc so both are in by the time it
applies.

Claude-Session: https://claude.ai/code/session_01AumZJ9xRd4XYNHThqy4rTv
mmabrouk added a commit that referenced this pull request Jul 8, 2026
…ct helpers)

Readability-only, no behavior change. Renames the SessionEnvironment `env2` to
`environment` and extracts the two session-lifetime listeners into documented
`routeSessionEventToActiveTurn` / `routePermissionRequestToActiveTurn` helpers, so
`acquireEnvironment` reads as clear steps and the demux data flow is obvious.
Addresses Mahmoud's #5156 review note that the event handling was hard to read.
723 tests unchanged, typecheck clean, reviewer confirmed no semantic drift.

Lands on the stacked approvals lane (top of the keep-alive stack) because the working
tree is the stack's applied state and GitButler attributes the edit to the top lane;
it rides into big-agents with the stack.

Claude-Session: https://claude.ai/code/session_01AumZJ9xRd4XYNHThqy4rTv
@mmabrouk
mmabrouk merged commit c01722e into big-agents Jul 8, 2026
14 of 19 checks passed
@mmabrouk mmabrouk mentioned this pull request Jul 8, 2026
12 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Backend Feature Request New feature or request size:XXL This PR changes 1000+ lines, ignoring generated files. tests

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant