fix: filter non-JSON log messages from ACP agent output#159
fix: filter non-JSON log messages from ACP agent output#159gandli wants to merge 4 commits intoopenclaw:mainfrom
Conversation
- Add filterAcpOutputStream() to filter out non-JSON lines from agent stdout before ndJsonStream parsing - Add isValidAcpMessage() check in createTappedStream() as additional safety layer - Prevents '[iFlow ACP Agent] ...' style log messages from causing JSON parse errors in @agentclientprotocol/sdk Fixes integration with iflow CLI and other ACP agents that output log messages to stdout/stderr.
gandli
force-pushed
the
fix/iflow-json-parse-error
branch
from
f2e397a to
684d721
Compare
![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: f2e397ac65
ℹ️ 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".
| } catch { | ||
| // If it fails to parse, it's likely a log message - skip it silently | ||
| // This prevents "[iFlow ACP Agent] ..." messages from causing parse errors |
There was a problem hiding this comment.
If an adapter prints banner/log text to stdout and then never completes the ACP handshake, this catch now drops that line and leaves connection.initialize() pending forever for every non-Gemini agent, because the only startup timeout here is the Gemini-specific path at src/client.ts:1020-1036. Before this change the same misconfiguration failed immediately with a parse error, so silently skipping stdout noise turns a debuggable protocol violation into a hang.
Useful? React with 👍 / 👎.
| const { value, done } = await reader.read(); | ||
| if (done) { | ||
| break; |
There was a problem hiding this comment.
When the child closes stdout without a trailing newline on the last JSON-RPC frame, hitting done breaks out of the loop and the remaining buffer is never parsed or enqueued. That drops the agent's final ACP message, so one-shot adapters can hang waiting for initialize() or the last prompt result even though the response was already written before exit.
Useful? React with 👍 / 👎.
This hook automatically removes any Co-authored-by lines from commit messages, preventing AI assistants from adding their signature to commits.
- Remove unused 'parsed' variable in filterAcpOutputStream() - Keep JSON.parse() call for validation but don't assign to variable
- Add fail-fast mechanism: throw error after 10 consecutive non-JSON lines to detect adapters that don't support stdio ACP mode - Flush buffered content at EOF to handle final JSON-RPC frame without trailing newline - Reset non-JSON counter after successful JSON parse to allow occasional log lines Addresses: - P2: Fail fast on stdout protocol violations - P2: Flush the buffered final frame at EOF
1 participant
Problem
acpx fails to integrate with iflow CLI due to JSON parse errors when the agent outputs log messages like:
Root Cause
iflow CLI outputs initialization log messages to stdout/stderr during startup. These non-JSON lines are passed to
@agentclientprotocol/sdk'sndJsonStreamparser, which attempts to parse them as JSON and fails.Solution
Added two-layer filtering to prevent non-JSON log messages from breaking the ACP protocol stream:
filterAcpOutputStream()- New method that filters the output stream beforendJsonStreamparsing, silently skipping lines that cannot be parsed as JSONisValidAcpMessage()- Additional validation layer increateTappedStream()to ensure only valid ACP messages are processedTesting
acpx iflow sessions newnow works without JSON parse errorsImpact
Fixes integration with iflow CLI and other ACP agents that output initialization log messages to stdout/stderr during startup.