chore: improve live codex harness diagnostics #14
Conversation
Ensure codex in-flight ends within 2.5s of completion, pick active JSONL per process, and make opencode message completion authoritative. Remove wheel passive listener warnings and align docs/tests.
Start the server on the next available port during dev and ignore stale OpenCode status events when determining in-flight state.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6b1f2f5ca6
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| const assistantCreatedAt = latestAssistant?.info?.time?.created; | ||
| const inFlightSignalAt = | ||
| !hasCompleted && typeof assistantCreatedAt === "number" | ||
| ? assistantCreatedAt | ||
| : hasPendingTool && typeof latestPartStart === "number" | ||
| ? latestPartStart | ||
| : undefined; |
There was a problem hiding this comment.
When CONSENSUS_OPENCODE_INFLIGHT_STALE_MS is set, the stale cutoff uses inFlightSignalAt, which for non-tool responses is based on the assistant message created timestamp. For long-running assistant generations (no tool parts, no completion yet), created can be much older than the most recent activity, so the stale check later in this function can force inFlight to false while the response is still actively streaming. This only shows up when stale cutoff is enabled and the response duration exceeds the cutoff; consider using the most recent activity time (e.g., latestActivityAt) or updating inFlightSignalAt with newer timestamps to avoid premature idle transitions.
Useful? React with 👍 / 👎.
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
| async function waitForExit(child: ReturnType<typeof spawn>, timeoutMs: number): Promise<boolean> { | ||
| try { | ||
| await Promise.race([once(child, "exit"), delay(timeoutMs)]); | ||
| return true; | ||
| } catch { | ||
| return false; | ||
| } |
There was a problem hiding this comment.
🔴 waitForExit() always returns true because Promise.race never rejects on timeout
waitForExit uses Promise.race([once(child, "exit"), delay(timeoutMs)]) inside a try/catch and returns true whenever the race resolves. Since delay(timeoutMs) resolves (it does not reject), the race always resolves even when the process has not exited.
Actual vs expected:
- Expected: return
trueonly if the child emittedexitwithintimeoutMs, elsefalse. - Actual: returns
trueeven on timeout, so callers think the process exited.
Impact:
startConsensusServer().stop()relies onwaitForExitto decide whether to SIGKILL (if (!exited) ...). With the current implementation, SIGKILL will never be sent, potentially leaving straynodeserver processes running (port conflicts, resource leaks) in live tests.
Where
async function waitForExit(child, timeoutMs): Promise<boolean> {
try {
await Promise.race([once(child, "exit"), delay(timeoutMs)]);
return true;
} catch {
return false;
}
}Recommendation: Change the timeout branch to reject (or resolve a sentinel) so the function can distinguish between an exit event and a timeout, e.g. const exited = await Promise.race([once(...).then(()=>true), delay(timeoutMs).then(()=>false)]); return exited;.
Was this helpful? React with 👍 or 👎 to provide feedback.
|
Superseded by cherry-picked Codex tail fixes in fix/activity-state. |
1 participant
What
Why
How tested
User impact
Docs
Risk
Rollback