feat(acp): Add --debug flag for protocol message logging

[jpshackelford]

Summary

Add a --debug flag to the openhands acp command that enables logging of all ACP protocol messages (JSON-RPC) to a JSONL file for debugging purposes.

Closes #540

Implementation

The implementation leverages the existing StreamObserver pattern from the agent-client-protocol library to observe all incoming and outgoing JSON-RPC messages without modifying the core protocol handling.

Architecture

┌─────────────────────────────────────────────────────────────────────┐
│                          ACP Server                                  │
│  ┌───────────────┐    ┌──────────────────┐    ┌─────────────────┐   │
│  │  stdin/stdout │────▶  Connection      │────▶  Agent Handler  │   │
│  │   (stdio)     │◀────                  │◀────                 │   │
│  └───────────────┘    └────────┬─────────┘    └─────────────────┘   │
│                                │                                     │
│                       ┌────────▼─────────┐                           │
│                       │  DebugLogger     │ (StreamObserver)          │
│                       │  (if --debug)    │                           │
│                       └────────┬─────────┘                           │
│                                │                                     │
│                       ┌────────▼─────────┐                           │
│                       │  Log File        │                           │
│                       │  (JSONL format)  │                           │
│                       └──────────────────┘                           │
└─────────────────────────────────────────────────────────────────────┘

Files Changed

File	Change
openhands_cli/acp_impl/debug_logger.py	New file - DebugLogger class
openhands_cli/argparsers/acp_parser.py	Add --debug argument
openhands_cli/acp_impl/agent/launcher.py	Accept and wire up debug parameter
openhands_cli/entrypoint.py	Pass debug flag to run_acp_server
tests/acp/test_debug_logger.py	New file - Unit tests
tests/acp/test_launcher.py	Add tests for debug parameter

Log File Format

Location: ~/.openhands/acp-debug/YYYYMMDD_HHMMSS_acp.jsonl

Format (JSONL - one JSON object per line):

{"ts": "2024-02-25T00:50:00.123456Z", "dir": ">>>", "msg": {"jsonrpc": "2.0", "id": 1, "method": "initialize", "params": {...}}}
{"ts": "2024-02-25T00:50:00.234567Z", "dir": "<<<", "msg": {"jsonrpc": "2.0", "id": 1, "result": {...}}}

• >>> = incoming messages (from IDE to agent)
• <<< = outgoing messages (from agent to IDE)

Usage

# Basic usage
openhands acp --debug

# With other options
openhands acp --debug --llm-approve --cloud

Then check logs at:

ls -la ~/.openhands/acp-debug/
cat ~/.openhands/acp-debug/*_acp.jsonl | jq .

Testing

All 331 ACP tests pass, including 10 new tests for the debug logging functionality.

tests/acp/test_debug_logger.py - 7 tests
tests/acp/test_launcher.py - 3 new tests for debug parameter

---

🚀 Try this PR

uvx --python 3.12 git+https://github.com/OpenHands/OpenHands-CLI.git@add-acp-debug-logging

[github-actions]

Coverage Report •

File	Stmts	Miss	Cover	Missing
openhands_cli
entrypoint.py	123	45	63%	32, 52–53, 56, 59, 61–62, 64–65, 68, 70, 72, 76, 81, 83–84, 133, 135–136, 139–143, 146–147, 149, 151, 162, 164–166, 168, 170–172, 175, 177, 181, 190, 192–194, 211, 258
openhands_cli/acp_impl
debug_logger.py	49	0	100%
openhands_cli/acp_impl/agent
launcher.py	27	0	100%
openhands_cli/argparsers
acp_parser.py	13	0	100%
TOTAL	6666	933	86%

[all-hands-bot]

Taste Rating: 🟡 Acceptable

Core design is sound - using StreamObserver is the right pattern and keeps this non-invasive. Simple implementation that solves a real problem. Main issues are resource management and lack of error handling.

[enyst]

I think maybe the CLI should have a debug mode for the application. Can we do that, and when set, it will also apply to ACP server, or is there a reason not to?

[jpshackelford]

@enyst Thanks for taking a look! Not sure what you mean by debug mode for the application. Are you talking about the CLI or WebUI? In any case, seems like that would be a separate PR, no?

[enyst]

@enyst Thanks for taking a look! Not sure what you mean by debug mode for the application. Are you talking about the CLI or WebUI? In any case, seems like that would be a separate PR, no?

What I mean is: every repo has or should have a debug flag, which, when set, the application logs at log.debug level.

We log at debug level a lot of things. So I wonder, can’t we use that, for ACP debug logging, instead of adding a new ACP-specific flag?

[jpshackelford]

@enyst This flag is different from just using the log level setting in that it also writes logs to a file in a specific format for debugging acp. We could consider adding log files for other elements of the system, but that would be new scope and it isn't clear to me whether it is needed as urgently as acp debug logging where we need output written to file since stdout is used by the acp protocol itself.

[all-hands-bot]

Acceptable - Core implementation is solid. Using StreamObserver is the right pattern, error handling is in place, and tests cover the important cases. One minor cleanup suggestion below.

[all-hands-bot]

🟡 Suggestion: The atexit registration logic could be tighter. Currently you register cleanup even when the logger failed to initialize (enabled=False). While harmless since close() handles None gracefully, it's cleaner to only register when actually enabled:

Suggested change

	# Register cleanup for debug logger file handle
	# Setup debug observer if enabled
	observers = []
	if debug:
	debug_logger = DebugLogger()
	if debug_logger.enabled:
	observers.append(debug_logger)
	atexit.register(debug_logger.close)

This makes the lifecycle clearer: only set up cleanup for resources that were successfully initialized.