Skip to content

Commit 836cd5d

Browse files
committed
fix(exec+appserver): 同步 3.0.x 生产就绪加固——exit/参数原样传递/WS 缓冲上限/ws 明文告警(Jackson 2 适配)
1 parent 57a36b2 commit 836cd5d

7 files changed

Lines changed: 170 additions & 6 deletions

File tree

README.md

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -228,6 +228,13 @@ There is no configuration file of its own. Key fields:
228228

229229
### 7.1 `CodexAppServerConfig` (app-server WebSocket route)
230230

231+
> **Upgrade notes (3.0.x.x.20260630+)**: CLI-route arguments are now passed
232+
> to the child process raw — multi-word prompts no longer arrive at `codex`
233+
> wrapped in embedded literal quotes. Non-zero CLI exits now preserve the real
234+
> exit code and both captured streams instead of collapsing to `exitCode=-1`
235+
> with empty output. A bearer token over a plaintext `ws://` connection logs a
236+
> warning; prefer `wss://`.
237+
231238
Plain POJO (Spring `@ConfigurationProperties`-bindable). Field names mirror the
232239
commonly used `CodexEndpoint` binding:
233240

@@ -238,6 +245,8 @@ commonly used `CodexEndpoint` binding:
238245
| `connectTimeoutMillis` | int | `5000` | TCP/TLS + WebSocket handshake timeout |
239246
| `readTimeoutMillis` | int | `120000` | Upper bound for a whole turn (connect → `turn/completed`) |
240247
| `maxSessionMappings` | int | `1000` | Bound of the `sessionKey → threadId` LRU; evicted sessions start fresh threads |
248+
| `maxFrameChars` | int | `1048576` | Frame accumulation hard cap; oversized server frames fail the turn (`<= 0` = unbounded) |
249+
| `maxContentChars` | int | `1048576` | Per-turn agent-message content cap; excess is truncated with a warning (`<= 0` = unbounded) |
241250

242251
## 8. Core Usage / API
243252

README.zh-CN.md

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -222,6 +222,11 @@ public class CodexDemo {
222222

223223
### 7.1 `CodexAppServerConfig`(app-server WebSocket 路线)
224224

225+
> **升级注意(3.0.x.x.20260630+)**:CLI 路线的参数改为原样传给子进程——
226+
> 含空格的多词 prompt 不再被塞进字面双引号后发给 `codex`。CLI 非零退出现在
227+
> 保留真实退出码与两路输出,不再折叠为 `exitCode=-1` 加空输出。通过明文
228+
> `ws://` 携带 Bearer token 会打告警日志,生产环境请优先 `wss://`
229+
225230
纯 POJO(可绑定 Spring `@ConfigurationProperties`)。字段名与常用的
226231
`CodexEndpoint` 绑定保持一致:
227232

@@ -232,6 +237,8 @@ public class CodexDemo {
232237
| `connectTimeoutMillis` | int | `5000` | TCP/TLS + WebSocket 握手超时 |
233238
| `readTimeoutMillis` | int | `120000` | 单个 turn 全程上限(建连 → `turn/completed`|
234239
| `maxSessionMappings` | int | `1000` | `sessionKey → threadId` LRU 上限;被淘汰的会话退化为新建线程 |
240+
| `maxFrameChars` | int | `1048576` | 帧累积硬上限;超限的服务器帧使 turn 失败(`<= 0` = 不限) |
241+
| `maxContentChars` | int | `1048576` | 单 turn agent 消息内容上限;超出部分截断并告警(`<= 0` = 不限) |
235242

236243
## 8. 核心用法 / API
237244

src/main/java/io/github/easy4j/codex/appserver/CodexAppServerConfig.java

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -56,4 +56,19 @@ public class CodexAppServerConfig {
5656
* degrade to starting a fresh thread. Defaults to {@code 1000}.
5757
*/
5858
private int maxSessionMappings = 1000;
59+
60+
/**
61+
* Hard cap in characters for the frame accumulation buffer. A server frame
62+
* exceeding it fails the turn with {@link CodexAppServerException}. Values
63+
* {@code <= 0} mean unbounded. Defaults to 1&nbsp;MiB characters.
64+
*/
65+
private int maxFrameChars = 1_048_576;
66+
67+
/**
68+
* Hard cap in characters for the agent-message content accumulated per
69+
* turn; excess item text is truncated (with a warning) rather than failing
70+
* the turn. Values {@code <= 0} mean unbounded. Defaults to 1&nbsp;MiB
71+
* characters.
72+
*/
73+
private int maxContentChars = 1_048_576;
5974
}

src/main/java/io/github/easy4j/codex/appserver/CodexAppServerTurn.java

Lines changed: 35 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -101,12 +101,16 @@ class CodexAppServerTurn implements WebSocket.Listener {
101101
*/
102102
CompletableFuture<AppServerTurnResult> start() {
103103
Objects.requireNonNull(httpClient, "httpClient");
104+
String url = toWebSocketUrl(config.getBaseUrl());
105+
if (hasText(config.getToken()) && url.startsWith("ws://")) {
106+
log.warn("Codex app-server token is sent over an unencrypted ws:// connection; prefer wss:// in production");
107+
}
104108
WebSocket.Builder builder = httpClient.newWebSocketBuilder();
105109
builder.connectTimeout(Duration.ofMillis(config.getConnectTimeoutMillis()));
106110
if (hasText(config.getToken())) {
107111
builder.header("Authorization", "Bearer " + config.getToken().trim());
108112
}
109-
builder.buildAsync(URI.create(toWebSocketUrl(config.getBaseUrl())), this)
113+
builder.buildAsync(URI.create(url), this)
110114
.whenComplete((socket, error) -> {
111115
if (Objects.nonNull(error)) {
112116
completeError(new CodexAppServerException("Codex WebSocket connection failed", error));
@@ -158,6 +162,11 @@ void begin() {
158162

159163
@Override
160164
public CompletionStage<?> onText(WebSocket socket, CharSequence data, boolean last) {
165+
if (frameBuffer.length() + data.length() > effectiveMaxFrameChars()) {
166+
completeError(new CodexAppServerException(
167+
"Codex frame buffer exceeded maxFrameChars=" + config.getMaxFrameChars()));
168+
return null;
169+
}
161170
frameBuffer.append(data);
162171
if (last) {
163172
String frame = frameBuffer.toString();
@@ -233,10 +242,32 @@ private void onItemCompleted(JsonNode params) {
233242
if (!hasText(text)) {
234243
return;
235244
}
236-
content.append(text);
237-
if (Objects.nonNull(request.getOnDelta())) {
238-
request.getOnDelta().accept(text);
245+
String applied = truncateToContentCap(text);
246+
content.append(applied);
247+
if (!applied.isEmpty() && Objects.nonNull(request.getOnDelta())) {
248+
request.getOnDelta().accept(applied);
249+
}
250+
}
251+
252+
/** Applies the {@code maxContentChars} hard cap; excess text is dropped with a single warning. */
253+
private String truncateToContentCap(String text) {
254+
int cap = effectiveMaxContentChars();
255+
if (content.length() >= cap) {
256+
return "";
257+
}
258+
if (content.length() + text.length() > cap) {
259+
log.warn("Codex turn content truncated at maxContentChars={}", config.getMaxContentChars());
260+
return text.substring(0, cap - content.length());
239261
}
262+
return text;
263+
}
264+
265+
private int effectiveMaxFrameChars() {
266+
return config.getMaxFrameChars() <= 0 ? Integer.MAX_VALUE : config.getMaxFrameChars();
267+
}
268+
269+
private int effectiveMaxContentChars() {
270+
return config.getMaxContentChars() <= 0 ? Integer.MAX_VALUE : config.getMaxContentChars();
240271
}
241272

242273
private void onTurnCompleted(JsonNode params) {

src/main/java/io/github/easy4j/codex/cli/CodexCliExecutor.java

Lines changed: 28 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@
1818
import io.github.easy4j.codex.CodexClientConfig;
1919
import org.apache.commons.exec.CommandLine;
2020
import org.apache.commons.exec.DefaultExecutor;
21+
import org.apache.commons.exec.ExecuteException;
2122
import org.apache.commons.exec.ExecuteWatchdog;
2223
import org.slf4j.Logger;
2324
import org.slf4j.LoggerFactory;
@@ -79,6 +80,10 @@ public CodexCliExecutor(CodexClientConfig config) {
7980
* <li>Process timeout &mdash; {@link CodexCliResult#isTimeout()} returns
8081
* {@code true}; exit code is {@code -1}; stderr contains the timeout
8182
* notice.</li>
83+
* <li>Non-zero process exit &mdash; the real exit code is preserved in
84+
* {@link CodexCliResult#getExitCode()}, and both captured streams are
85+
* returned as-is ({@link CodexCliResult#isSuccess()} is simply
86+
* {@code exitCode == 0}).</li>
8287
* <li>IOException (missing executable, permission denied, etc.) &mdash;
8388
* the {@link IOException#getMessage()} is captured in
8489
* {@link CodexCliResult#getStderr()} and the exit code is {@code -1}.</li>
@@ -113,7 +118,11 @@ private CodexCliResult runProcess(String stdin, String... args) {
113118
CommandLine cmd = CommandLine.parse(config.getLocalExecutable());
114119
for (String arg : args) {
115120
if (arg != null) {
116-
cmd.addArgument(arg);
121+
// handleQuoting=false: the child is spawned via exec(argv), not
122+
// a shell — commons-exec's default quoting would embed literal
123+
// double quotes inside arguments containing spaces (prompts,
124+
// config overrides, paths), corrupting them on arrival.
125+
cmd.addArgument(arg, false);
117126
}
118127
}
119128

@@ -131,6 +140,7 @@ private CodexCliResult runProcess(String stdin, String... args) {
131140
ExecuteWatchdog watchdog = new ExecuteWatchdog(timeoutMs);
132141
executor.setWatchdog(watchdog);
133142

143+
long startNanos = System.nanoTime();
134144
try {
135145
int exitCode = executor.execute(cmd);
136146
String out = stdout.toString().trim();
@@ -140,6 +150,23 @@ private CodexCliResult runProcess(String stdin, String... args) {
140150
return new CodexCliResult(-1, out, "codex CLI timed out after " + timeoutMs + " ms\n" + err);
141151
}
142152
return new CodexCliResult(exitCode, out, err);
153+
} catch (ExecuteException e) {
154+
// commons-exec throws ExecuteException for EVERY non-zero exit
155+
// (and for watchdog kills). The stream pumps are joined before it
156+
// is thrown, so both buffers are complete — surface them together
157+
// with the real exit code instead of discarding the output. The
158+
// deadline check makes the timeout verdict race-free even when
159+
// {@code watchdog.killedProcess()} has not observed the kill yet.
160+
String out = stdout.toString().trim();
161+
String err = stderr.toString().trim();
162+
boolean timedOut = watchdog.killedProcess()
163+
|| System.nanoTime() - startNanos >= timeoutMs * 1_000_000L;
164+
if (timedOut) {
165+
return new CodexCliResult(-1, out, "codex CLI timed out after " + timeoutMs + " ms\n" + err);
166+
}
167+
log.debug("codex CLI failed: exitCode={}, stdout.len={}, stderr.len={}",
168+
e.getExitValue(), out.length(), err.length());
169+
return new CodexCliResult(e.getExitValue(), out, err);
143170
} catch (IOException e) {
144171
return new CodexCliResult(-1, "", e.getMessage());
145172
}

src/test/java/io/github/easy4j/codex/appserver/CodexAppServerTurnTest.java

Lines changed: 54 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -261,4 +261,58 @@ void shouldCompleteWithoutErrorAfterTurnCompletedWhenSocketClosesLate() throws E
261261
assertTrue(turn.future().isDone());
262262
assertEquals("stop", turn.future().get(1, TimeUnit.SECONDS).getFinishReason());
263263
}
264+
265+
@Test
266+
void shouldFailTurnWhenFrameExceedsCap() {
267+
CodexAppServerConfig config = new CodexAppServerConfig();
268+
config.setMaxFrameChars(8);
269+
CodexAppServerTurn turn = new CodexAppServerTurn(
270+
AppServerTurnRequest.builder().prompt("hi").build(), config, mapper, new ThreadMappingCache(10), null);
271+
272+
turn.onText(null, "way-too-long-frame-data", false);
273+
274+
assertTrue(turn.future().isCompletedExceptionally());
275+
CompletionException ex = assertThrows(CompletionException.class, () -> turn.future().join());
276+
CodexAppServerException cause = assertInstanceOf(CodexAppServerException.class, ex.getCause());
277+
assertTrue(cause.getMessage().contains("maxFrameChars"));
278+
}
279+
280+
@Test
281+
void shouldTruncateContentAtCapWithoutFailingTurn() {
282+
CodexAppServerConfig config = new CodexAppServerConfig();
283+
config.setMaxContentChars(10);
284+
List<String> deltas = new ArrayList<>();
285+
CodexAppServerTurn turn = new CodexAppServerTurn(
286+
AppServerTurnRequest.builder().prompt("hi").onDelta(deltas::add).build(),
287+
config, mapper, new ThreadMappingCache(10), null);
288+
289+
turn.begin();
290+
turn.handleFrame("{\"jsonrpc\":\"2.0\",\"id\":1,\"result\":{\"threadId\":\"th_1\"}}");
291+
turn.handleFrame("{\"method\":\"item/completed\",\"params\":{\"item\":{\"type\":\"agentMessage\",\"text\":\"12345\"}}}");
292+
turn.handleFrame("{\"method\":\"item/completed\",\"params\":{\"item\":{\"type\":\"agentMessage\",\"text\":\"67890\"}}}");
293+
turn.handleFrame("{\"method\":\"item/completed\",\"params\":{\"item\":{\"type\":\"agentMessage\",\"text\":\"ABCDE\"}}}");
294+
turn.handleFrame("{\"method\":\"turn/completed\",\"params\":{}}");
295+
296+
AppServerTurnResult result = turn.future().join();
297+
assertEquals("1234567890", result.getContent());
298+
assertEquals(List.of("12345", "67890"), deltas, "truncated-to-empty text must not emit a delta");
299+
assertEquals("stop", result.getFinishReason());
300+
}
301+
302+
@Test
303+
void shouldTreatNonPositiveCapsAsUnbounded() {
304+
CodexAppServerConfig config = new CodexAppServerConfig();
305+
config.setMaxFrameChars(0);
306+
config.setMaxContentChars(-1);
307+
CodexAppServerTurn turn = new CodexAppServerTurn(
308+
AppServerTurnRequest.builder().prompt("hi").build(), config, mapper, new ThreadMappingCache(10), null);
309+
310+
turn.begin();
311+
turn.handleFrame("{\"jsonrpc\":\"2.0\",\"id\":1,\"result\":{\"threadId\":\"th_1\"}}");
312+
turn.onText(null, "x".repeat(4096), false);
313+
turn.handleFrame("{\"method\":\"turn/completed\",\"params\":{}}");
314+
315+
assertTrue(turn.future().isDone());
316+
assertFalse(turn.future().isCompletedExceptionally());
317+
}
264318
}

src/test/java/io/github/easy4j/codex/cli/CodexCliExecutorTest.java

Lines changed: 22 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -64,10 +64,21 @@ void shouldCaptureExitCodeFromFailingProcess() {
6464

6565
CodexCliResult result = executor.execute("-c", "exit 7");
6666

67-
assertEquals(-1, result.getExitCode());
6867
assertFalse(result.isSuccess());
6968
}
7069

70+
@Test
71+
void shouldPreserveRealExitCodeAndStreamsOnNonZeroExit() {
72+
CodexCliExecutor executor = new CodexCliExecutor(configFor("/bin/sh"));
73+
74+
CodexCliResult result = executor.execute("-c", "echo out-marker; echo err-marker 1>&2; exit 7");
75+
76+
assertEquals(7, result.getExitCode());
77+
assertFalse(result.isSuccess());
78+
assertTrue(result.getStdout().contains("out-marker"), "stdout must survive a non-zero exit");
79+
assertTrue(result.getStderr().contains("err-marker"), "stderr must survive a non-zero exit");
80+
}
81+
7182
@Test
7283
void shouldReturnIoExceptionMessageWhenExecutableMissing() {
7384
CodexCliExecutor executor = new CodexCliExecutor(configFor("/nonexistent/path/to/codex"));
@@ -80,6 +91,16 @@ void shouldReturnIoExceptionMessageWhenExecutableMissing() {
8091
assertFalse(result.getStderr().isEmpty());
8192
}
8293

94+
@Test
95+
void shouldPassArgumentsRawWithoutEmbeddedQuotes() {
96+
CodexCliExecutor executor = new CodexCliExecutor(configFor("/bin/echo"));
97+
98+
CodexCliResult result = executor.execute("Write a failing test", "-c", "key=some value");
99+
100+
assertEquals("Write a failing test -c key=some value", result.getStdout(),
101+
"multi-word arguments must arrive without embedded literal quotes");
102+
}
103+
83104
@Test
84105
void shouldIgnoreNullArguments() {
85106
CodexCliExecutor executor = new CodexCliExecutor(configFor("/bin/echo"));

0 commit comments

Comments
 (0)