Skip to content

fix(ci): handle non-JSON output in docker smoke test ACP handshake#353

Merged
thepagent merged 2 commits intoopenabdev:mainfrom
chaodu-agent:fix/docker-smoke-test-acp-handshake
Apr 15, 2026
Merged

fix(ci): handle non-JSON output in docker smoke test ACP handshake#353
thepagent merged 2 commits intoopenabdev:mainfrom
chaodu-agent:fix/docker-smoke-test-acp-handshake

Conversation

@chaodu-agent
Copy link
Copy Markdown
Collaborator

Problem

The ACP initialize handshake step in docker-smoke-test.yml uses head -1 to capture the agent's response. Some agents emit non-JSON lines to stdout before the JSON-RPC response:

  • gemini: outputs Ignore file not found: /home/node/.geminiignore before the JSON response
  • kiro-cli: may output empty or non-JSON content when no config/API key is present

head -1 captures these lines instead of the actual JSON-RPC response, causing the handshake check to always fail for these variants.

This has been failing on every PR since the workflow was introduced.

Solution

  • Replace head -1 with grep -m1 '^{' to extract the first line that looks like JSON
  • Add || true to the docker run command so timeout exit codes don't kill the step before output is captured
  • Log raw output for easier debugging of future failures

Note

This fixes the gemini variant. The kiro-cli variant may still fail if it requires API credentials to respond to ACP initialize — that would need a separate investigation.

Agents like gemini emit non-JSON lines (e.g. 'Ignore file not found')
to stdout before the JSON-RPC response. The previous 'head -1' approach
captured these lines instead of the actual response.

Changes:
- Replace 'head -1' with 'grep -m1 ^{' to extract first JSON line
- Add '|| true' to docker run to prevent timeout exit code from
  killing the step before output is captured
- Log raw output for easier debugging of future failures
@chaodu-agent chaodu-agent requested a review from thepagent as a code owner April 15, 2026 01:11
@github-actions
Copy link
Copy Markdown

⚠️ This PR is missing a Discord Discussion URL in the body.

All PRs must reference a prior Discord discussion to ensure community alignment before implementation.

Please edit the PR description to include a link like:

Discord Discussion URL: https://discord.com/channels/...

This PR will be automatically closed in 3 days if the link is not added.

@github-actions github-actions bot added the closing-soon PR missing Discord Discussion URL — will auto-close in 3 days label Apr 15, 2026
@chaodu-agent chaodu-agent force-pushed the fix/docker-smoke-test-acp-handshake branch 2 times, most recently from 5754871 to 3d81b1f Compare April 15, 2026 01:22
@chaodu-agent
Copy link
Copy Markdown
Collaborator Author

Known limitation: kiro-cli ACP handshake in CI

The kiro-cli and codex-acp variants are marked with continue-on-error: true because their ACP handshake produces empty output in CI — both stdout and stderr are completely silent.

This is likely because kiro-cli acp --trust-all-tools requires authentication credentials (AWS credentials or Kiro auth token) that are not available in the CI environment. Without them, the process exits immediately without responding to the JSON-RPC initialize request.

To verify locally (requires Docker):

echo '{"jsonrpc":"2.0","id":1,"method":"initialize","params":{"protocolVersion":1,"clientCapabilities":{},"clientInfo":{"name":"test","version":"0.0.1"}}}' | \
  docker run --rm -i --entrypoint kiro-cli openab-test acp --trust-all-tools 2>&1

If/when CI secrets are configured for these agents, the acp_optional flag can be removed to make their handshake checks required.

@thepagent
Copy link
Copy Markdown
Collaborator

Verified locally with OrbStack + Docker. Here are the ACP handshake results for all 5 variants (no auth configured):

Agent ACP Handshake Output
kiro-cli ❌ Silent exit not logged in on stderr, no JSON-RPC
claude-agent-acp ✅ Works Returns agentInfo without auth
gemini ✅ Works Non-JSON first line (Ignore file not found: .geminiignore), then valid JSON-RPC
copilot ❌ Silent exit Empty stdout
codex-acp ❌ Silent exit Empty stdout

Two findings:

  1. copilot is missing acp_optional: true — it also silently exits without auth, same as kiro-cli and codex-acp. The CI will fail on the copilot variant without this flag.

  2. The grep -m1 '^{' fix is confirmed necessary — gemini emits a non-JSON warning line before the JSON-RPC response.

Only claude-agent-acp responds to initialize without auth. The other three (kiro, codex, copilot) all require auth just to handshake, which is arguably a bug in those CLIs — initialize should be unauthenticated per the ACP spec.

@thepagent
Copy link
Copy Markdown
Collaborator

Suggestion: instead of continue-on-error + acp_optional, use a fallback approach so every variant gets a real pass/fail:

- name: Verify agent responds
  run: |
    INIT='{"jsonrpc":"2.0","id":1,"method":"initialize","params":{"protocolVersion":1,"clientCapabilities":{},"clientInfo":{"name":"ci-test","version":"0.0.1"}}}'

    RAW=$(echo "$INIT" | timeout 30 docker run --rm -i \
      --entrypoint ${{ matrix.variant.agent }} \
      openab-test${{ matrix.variant.suffix }} \
      ${{ matrix.variant.agent_args }} 2>/dev/null || true)

    RESPONSE=$(echo "$RAW" | grep -m1 '^{' || true)

    if [ -n "$RESPONSE" ] && echo "$RESPONSE" | jq -e '.result.agentInfo.name' >/dev/null 2>&1; then
      AGENT_NAME=$(echo "$RESPONSE" | jq -r '.result.agentInfo.name')
      echo "✅ ACP handshake ok — agent=$AGENT_NAME"
    else
      echo "⚠️ ACP handshake returned no response — falling back to CLI check"
      docker run --rm --entrypoint ${{ matrix.variant.agent }} \
        openab-test${{ matrix.variant.suffix }} --help >/dev/null 2>&1
      echo "✅ Agent CLI responds (ACP skipped — likely needs auth)"
    fi

This way:

  • No continue-on-error needed — every variant gets a definitive pass/fail
  • No acp_optional matrix flag needed
  • Agents that support unauthenticated handshake (claude, gemini) get the full ACP test
  • Agents that need auth (kiro, codex, copilot) still get verified via --help

Also note: copilot also needs acp_optional if you keep the current approach — it silently exits without auth too (verified locally).

@chaodu-agent chaodu-agent force-pushed the fix/docker-smoke-test-acp-handshake branch from 3d81b1f to 185f713 Compare April 15, 2026 01:37
@thepagent thepagent removed the closing-soon PR missing Discord Discussion URL — will auto-close in 3 days label Apr 15, 2026
@chaodu-agent
Copy link
Copy Markdown
Collaborator Author

Adopted your fallback approach — much cleaner than continue-on-error. Updated in 185f713:

  • Removed acp_optional flag and continue-on-error
  • ACP handshake success → report agent name
  • ACP handshake fail → fallback to --help check, still a real pass/fail
  • Added Co-authored-by credit

The only thing is this push won't trigger the smoke test since we reverted the Dockerfile.gemini touch. Need a Dockerfile change in the PR to trigger it, or a maintainer re-run.

Adopt thepagent's suggestion: instead of continue-on-error + acp_optional,
use a fallback approach so every variant gets a real pass/fail.

- Replace 'head -1' with 'grep -m1 ^{' to skip non-JSON lines
  (fixes gemini 'Ignore file not found' issue)
- If ACP handshake succeeds, report agent name
- If ACP handshake fails (no auth in CI), fall back to '--help' check
  so the agent CLI is still verified
- Remove acp_optional flag and continue-on-error — all variants now
  get a definitive pass/fail

Co-authored-by: thepagent <thepagent@users.noreply.github.com>
@chaodu-agent chaodu-agent force-pushed the fix/docker-smoke-test-acp-handshake branch from 185f713 to 0d6b037 Compare April 15, 2026 01:38
@github-actions github-actions bot added the closing-soon PR missing Discord Discussion URL — will auto-close in 3 days label Apr 15, 2026
@thepagent thepagent merged commit 9ce771b into openabdev:main Apr 15, 2026
7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

closing-soon PR missing Discord Discussion URL — will auto-close in 3 days

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants