agentscope-ai / agentscope-ai/agentscope-java

[Bug]: SessionTree mirror NPEs when it outlives the sandbox — PinnedSandboxFilesystem cannot know the pinned sandbox is closed (regression from #2946, v2.0.3)

Abierto
#3,036 2 comentarios 1 reacción 0 asignados Ver en GitHub
area/extensions area/harness bug
Lenguaje dominante
Java
Estrellas
5.6k
Forks
1.3k
Merge medio
4 d 12 h
PR fusionados (30 d)
77

Descripción

**Versions:** `agentscope-harness:2.0.3`, `agentscope-extensions-sandbox-kubernetes:2.0.3`
**Regressed by:** #2946 (merged 2026-09-06, released in v2.0.3 on 2026-09-07)
**Same root cause as:** #2147 (still open) — different teardown point
**Context:** #2490 / #2675 (per-call sandbox binding), #2771 (per-call K8s pod termination), #2777 / #2935 (fire-and-forget flush)

### Summary

`SessionTree` mirrors the session log to the agent's filesystem fire-and-forget. When that filesystem is a sandbox, the queued mirror routinely runs *after* the sandbox has been shut down and dies with a `NullPointerException` rather than any meaningful error.

Before #2946 this same race degraded to a `No active sandbox` warning, because the async mirror resolved the sandbox lazily through the shared slot, which release had already cleared. #2946 pinned the `Sandbox` instance to fix *which* sandbox the mirror targets — correct in itself — but the pinned reference is now a hard one, so the mirror walks into a dead sandbox instead of failing soft.

### Observed

```
WARN session-tree-mirror i.a.h.a.filesystem.sandbox.SandboxBackedFilesystem
[sandbox-fs] native upload failed for path: agents//sessions/.log.jsonl
java.lang.NullPointerException: Cannot invoke
"io.agentscope.extensions.sandbox.kubernetes.client.CommandExecutor.run(String)"
because the return value of
"io.agentscope.extensions.sandbox.kubernetes.client.Sandbox.commands()" is null
at io.agentscope.extensions.sandbox.kubernetes.KubernetesSandbox.uploadFile(KubernetesSandbox.java:272)
at io.agentscope.harness.agent.filesystem.sandbox.SandboxBackedFilesystem.uploadFiles(...)
at io.agentscope.harness.agent.memory.session.SessionTree.mirrorToFilesystem(SessionTree.java:733)
at io.agentscope.harness.agent.memory.session.SessionTree.lambda$scheduleMirror$6(SessionTree.java:593)
at java.base/java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1144)
```

One run, three log lines 78 ms apart: the trace middleware logs POST_CALL, then `[sandbox-client] Connection to sandbox claim '' has been closed.`, then the mirror fails.

> The `SandboxBackedFilesystem` frame's line number is omitted because we shadow that class locally for an unrelated reason. The frame is the unmodified native-transfer branch of `uploadFiles` that calls `SandboxFileTransfer.uploadFile`.

### Root cause

1. `SessionTree.flush()` → `scheduleMirror()` wraps the filesystem via `pinIfSandbox(...)` and hands the work to the static single-threaded `MIRROR_EXECUTOR` (thread `session-tree-mirror`). `flush()` returns as soon as the task is queued.
2. `PinnedSandboxFilesystem` pins the concrete `Sandbox` in its constructor (`setSandbox(sandbox)`) **and overrides `clearSandboxIfCurrent` to a no-op**. The override is deliberate — it stops release from un-pinning the instance — but `clearSandboxIfCurrent` is also the only channel through which this object could learn the sandbox was released (#2675). It therefore holds a hard reference to a sandbox it has no way of knowing is dead.
3. `SandboxLifecycleMiddleware.releaseForCall` → `SandboxManager.release` calls `stop()` then `shutdown()` for every sandbox with `isSelfManaged() == true`, at the end of *every* call.
4. `KubernetesSandbox.shutdown()` reaches the SDK's `Sandbox.closeConnection()`, which nulls `commands` and `files` and sets `closed = true`.
5. The queued mirror then calls `KubernetesSandbox.uploadFile`, which does `sdkSandbox.commands().run("mkdir -p ...")` with no liveness check → NPE.

Steps 1 and 3 are not synchronised in any way.

### This is not limited to user-managed sandboxes

#2771 documents the same step 3 for the stock client: *"`SandboxLifecycleMiddleware` … releases the sandbox after **every** call: `SandboxManager.release()` (self-managed path) → `stop()` → `shutdown()`. `KubernetesSandbox.shutdown()` sees `claimOwned == true` and calls `terminate()`, deleting the claim and the pod — every call."* #2771 was closed on the acquire/resume side; `release` still shuts the sandbox down per call in 2.0.3. So a plain framework-managed Kubernetes sandbox hits this race too.

### Why the existing quiescence API doesn't cover it

`SessionTree.awaitMirrorQuiescence` exists, but across the whole harness jar it is referenced only by `SessionTree` itself and `HarnessAgent.close()` — i.e. container shutdown. Nothing on the sandbox lifecycle path drains the queue, and there is no way to wait for one session's mirror; the executor is static and global, so the only available barrier is "wait for every session".

That is consistent with how the API arrived: #2777 made the flush fire-and-forget on purpose, to unblock conversation completion, and #2935 describes `awaitMirrorQuiescence` as something `HarnessAgent#close()` already did, reusing it test-side to stop `@TempDir` teardown flakiness. It was never introduced as a lifecycle contract for resource owners.

**#2147 is the same root cause at a different teardown point** and has been open since 2026-07-12: there the late `session-tree-mirror` write lands on a closed Jedis pool (`Pool not open`) after `HarnessAgent.close()` returned. Its "Expected" list — stop accepting new mirror jobs on close, drain or cancel accepted ones, close the executor before returning — is exactly what the sandbox path needs as well. Filing separately because the fix location differs (pinned filesystem / sandbox release rather than agent close), but the two should probably be solved together.

### Secondary defects

- **`KubernetesSandbox.uploadFile` / `downloadFile` don't check liveness.** The SDK's `Sandbox` already exposes `isActive()`. A closed sandbox should produce a `SandboxException` naming the cause, not an NPE from dereferencing a nulled field.
- **`isRunning()` lies after shutdown.** `KubernetesSandbox.shutdown()` overrides the interface default and only calls `terminate()` / `closeConnection()`; it never clears `AbstractBaseSandbox`'s `running` flag, so `Sandbox.isRunning()` still returns `true` on a shut-down sandbox. Callers wanting to guard defensively have no reliable check on the `Sandbox` interface and must downcast to `KubernetesSandbox` for `getSdkSandbox().isActive()`.

### Suggested fixes, roughly in order of value

1. Have `PinnedSandboxFilesystem` (or `SessionTree.mirrorToFilesystem`) skip the write and log at debug when the pinned sandbox is no longer alive — the mirror is best-effort by design.
2. Make `KubernetesSandbox.uploadFile` / `downloadFile` fail with a `SandboxException` on a closed sandbox instead of an NPE.
3. Clear the `running` flag in `KubernetesSandbox.shutdown()` so `isRunning()` is trustworthy.
4. Drain the mirror queue in `SandboxManager.release` before `shutdown()` for self-managed sandboxes, or expose a per-session `awaitMirror(sessionId, timeout)` so lifecycle owners can order the two correctly. This would also address #2147.

`SessionTreeMirrorTest`, added by #2946, is the natural home for a regression test: schedule a mirror against a sandbox that is shut down before the executor picks the task up.

### Workaround

We own our sandbox lifecycle (`SandboxContext.externalSandbox`), so we call `SessionTree.awaitMirrorQuiescence(5, TimeUnit.SECONDS)` immediately before shutting the sandbox down. It covers the normal path deterministically — `TranscriptMiddleware` triggers the flush inside the middleware chain, which completes before the `Flux.using` cleanup that releases the sandbox — but it relies on internal ordering rather than a contract, it blocks on unrelated sessions queued on the shared executor, and it isn't available at all to anyone using a framework-managed sandbox, since that shutdown happens inside `SandboxManager.release`.

Guía de contribución

Abrir la guía de contribución

Evaluación

Este issue todavía no se ha evaluado.

Recibe los nuevos issues en tu correo

Un resumen breve de issues de GitHub para principiantes.