agentscope-ai / agentscope-ai/agentscope-java

fix(sandbox-e2b): E2bEnvdProcessClient drops the end event when exit_code is 0 - every successful command is reported as a failure

オープン
#2,793 コメント 0 件 リアクション 0 件 担当者 0 名 GitHub で見る
主要言語
Java
スター
5.6k
フォーク
1.3k
平均マージ
4日 12時間
マージ済み PR(30日)
77

説明

## Summary

`E2bEnvdProcessClient` (agentscope-extensions-sandbox-e2b) uses proto3 field-presence
APIs (`getAllFields()` / `hasField()`) to decide whether an `end` event exists and
whether `exit_code` was set. `exit_code` is a proto3 singular scalar **without
explicit presence**, so the value `0` is indistinguishable from "absent" through
those APIs. The result inverts the meaning of success:

- a command that exits with **0 (success)** → the `end` event is dropped / the exit
code is never read → the caller sees the initial sentinel `Integer.MIN_VALUE` →
`ExecException: Command exited with code -2147483648`;
- a command that exits with **non-zero (failure)** → everything works normally.

**Every successful command is reported as a failure.** Since sandbox/workspace
bootstrap itself runs commands that exit 0, the E2B backend cannot even hand out a
sandbox handle when the envd endpoint speaks JSON.

## Affected versions

- 2.0.1, 2.0.2, 2.0.2-subagent-bugfix — `E2bEnvdProcessClient.class` is byte-identical
across these three releases on Maven Central.
- Still present on `main` as of 2026-08-20 (checked at 643905e6).

## Environment where it bites

Any e2b-compatible envd that serves **JSON** frames. Concretely reproduced against
Alibaba Cloud FC's e2b-compatible service (`*.e2b.fc.aliyuncs.com`): FC's envd
rejects the PROTO codec with HTTP 400 and only speaks JSON, so the JSON parsing
path is mandatory there.

Server-side evidence that the endpoint is fine: the same request sent with `curl`
returns three complete frames —

```
{"event":{"start":{...}}}
{"event":{"data":{"stdout":"aGkK"}}}
{"event":{"end":{"exitCode":0}}}
```

The `end` frame is present with `exitCode: 0`; it is the client that discards it.

## Standalone reproduction (no network, unmodified Central artifact)

A minimal project with a single dependency on
`io.agentscope:agentscope-extensions-sandbox-e2b:2.0.1` feeds the exact envd JSON
frames above into `parseJsonStartResponse` via reflection. Output:

```
== Part 1: the proto3 fact the bug rests on (pure protobuf, no agentscope logic) ==
EndEvent builder after setField(exit_code, 0) -> getAllFields().isEmpty() = true
EndEvent builder after setField(exit_code, 1) -> getAllFields().isEmpty() = false

== Part 2: agentscope 2.0.1 parseJsonStartResponse on real envd JSON frames ==
envd frame: {"event":{"end":{"exitCode":0}}}
end event survives parsing : false <== END EVENT DROPPED BY THE CLIENT
exit code the caller sees : -2147483648 <== ExecException: Command exited with code -2147483648
envd frame: {"event":{"end":{"exitCode":1}}}
end event survives parsing : true
exit code the caller sees : 1
```

The `exitCode:1` control case shows the parser is otherwise fine — the failure is
specific to the success value `0`.

## Root cause — two spots, same mistake

**1. JSON → DynamicMessage assembly, `parseJsonStartResponse`:**

```java
JsonNode exitCodeNode = endNode.path("exitCode");
if (exitCodeNode.canConvertToInt()) {
endBuilder.setField(exitCodeField, exitCodeNode.intValue());
}
if (!endBuilder.getAllFields().isEmpty()) { // <-- bug
event.setField(processEventDesc.findFieldByName("end"), endBuilder.build());
}
```

`exit_code` is a proto3 singular int32 without explicit presence. After
`setField(exitCodeField, 0)`, `endBuilder.getAllFields()` is **empty** — that is
documented proto3 behavior, not a protobuf bug. So the presence of the `end` event
is judged from the wrong place, and the event is attached only when the exit code
is non-zero.

**2. Stream draining, `drainStartStream`:**

```java
if (pe.hasField(peEndF)) {
DynamicMessage end = (DynamicMessage) pe.getField(peEndF);
Descriptors.FieldDescriptor ec =
end.getDescriptorForType().findFieldByName("exit_code");
if (end.hasField(ec)) { // <-- bug
Object v = end.getField(ec);
exit = v instanceof Integer ? (Integer) v : ((Long) v).intValue();
}
}
```

Same presence trap on the read side: for `exit_code == 0`, `hasField()` is false and
`exit` keeps its `Integer.MIN_VALUE` initializer. Note this read-side guard is wrong
even for the binary PROTO path — proto3 wire format omits default-valued singular
scalars, so a server that legitimately encodes `exit_code = 0` also trips it.

## Suggested fix

**`parseJsonStartResponse`** — judge presence from the JSON, not from
`getAllFields()`:

```java
JsonNode exitCodeNode = endNode.path("exitCode");
boolean hasExitCode = exitCodeNode.canConvertToInt();
if (hasExitCode) {
endBuilder.setField(exitCodeField, exitCodeNode.intValue());
}
if (hasExitCode || !endBuilder.getAllFields().isEmpty()) {
event.setField(processEventDesc.findFieldByName("end"), endBuilder.build());
}
```

**`drainStartStream`** — read unconditionally; the proto3 default for an absent
field is `0`, which is exactly the right value here:

```java
Object v = end.getField(ec);
exit = v instanceof Integer ? (Integer) v : ((Long) v).intValue();
```

We have been running both changes as a local patch in production use against FC's
e2b service; happy to turn this into a PR if the approach looks right to you.

## Regression test suggestion

A unit test in the same package pins the behavior without any network:

```java
@Test
void anExitCodeOfZeroSurvivesJsonParsing() throws Exception {
E2bSandboxClientOptions options = new E2bSandboxClientOptions();
options.setApiKey("test-only-not-used");
E2bEnvdProcessClient client = new E2bEnvdProcessClient(options);

byte[] frame = "{\"event\":{\"end\":{\"exitCode\":0}}}"
.getBytes(StandardCharsets.UTF_8);
DynamicMessage response = /* invoke parseJsonStartResponse(frame) */;

Descriptors.FieldDescriptor eventField =
response.getDescriptorForType().findFieldByName("event");
assertThat(response.hasField(eventField)).isTrue();

DynamicMessage event = (DynamicMessage) response.getField(eventField);
Descriptors.FieldDescriptor endField =
event.getDescriptorForType().findFieldByName("end");
assertThat(event.hasField(endField)).isTrue(); // fails before the fix

DynamicMessage end = (DynamicMessage) event.getField(endField);
Object exitCode = end.getField(
end.getDescriptorForType().findFieldByName("exit_code"));
assertThat(((Number) exitCode).intValue()).isZero();
}
```

The negative control that documents the proto3 fact this bug rests on: build an
`EndEvent` via `DynamicMessage.newBuilder(endDesc)`, `setField(exit_code, 0)`, and
assert `builder.getAllFields()` is empty — while `setField(exit_code, 1)` makes it
non-empty. That is why the `getAllFields().isEmpty()` heuristic can never work for
this field.

コントリビューションガイド

コントリビューションガイドを開く

評価

この issue はまだ評価されていません。

新しい issue をメールで受け取る

初心者向けの GitHub issue を短くまとめたダイジェスト。