Skip to content

ACP mode: session/prompt panics with "advisor config not initialized" (advisor init skipped by early return) #242

Description

@teamgbg

Summary

In --acp mode, every session/prompt panics the agent process:

thread 'main' panicked at src/extras/advisor/mod.rs:68:30:
advisor config not initialized

initialize and session/new both succeed, so it presents as a mid-session transport failure (an ACP client sees incoming transport closed) rather than a missing initialisation.

Cause

src/main.rs returns from the ACP branch before feature init:

// ACP mode: serve and exit before feature init
#[cfg(feature = "acp")]
if startup.cli.acp_enabled {
    return extras::acp::serve(startup.cli, startup.cfg, startup.context).await;
}

startup.init_features().await?;   // <- never reached in ACP mode

The advisor's one-time init lives inside that skipped call, at src/startup.rs:491:

crate::extras::advisor::init_config(config);

advisor::with_config then does .expect("advisor config not initialized") (src/extras/advisor/mod.rs:68), so the first prompt that reaches the advisor tool aborts the process.

Reproduction

Built --release --features acp,advisor (v1.7.2). Reproduces on both transports, which is expected since the skipped init is transport-agnostic:

stdio

zerostack --acp < <(
  printf '%s\n' '{"jsonrpc":"2.0","id":1,"method":"initialize","params":{"protocolVersion":1}}'
  printf '%s\n' '{"jsonrpc":"2.0","id":2,"method":"session/new","params":{"cwd":"/tmp","mcpServers":[]}}'
  sleep 4
  printf '%s\n' '{"jsonrpc":"2.0","id":3,"method":"session/prompt","params":{"sessionId":"<id>","prompt":[{"type":"text","text":"hello"}]}}'
  sleep 20
)

TCP — same result with --acp --acp-host 127.0.0.1 --acp-port 7301.

Expected: the prompt completes. Actual: process panics; the client sees the connection close mid-request.

Suggested fix

Initialise the advisor on the ACP path before serving. A disabled config is sufficient and seems the honest default, since an ACP client drives the session itself and does not use the advisor hand-off:

#[cfg(feature = "acp")]
if startup.cli.acp_enabled {
    #[cfg(feature = "advisor")]
    extras::advisor::init_config(extras::advisor::AdvisorToolConfig {
        client: None,
        advisor_model: String::new(),
        human_handoff: false,
        max_uses: None,
        handoff_tx: None,
        enabled: false,
        kilobytes_limit: 0,
    });
    return extras::acp::serve(startup.cli, startup.cfg, startup.context).await;
}

The #[cfg(feature = "advisor")] guard matters because advisor is not a default feature — without it the module does not exist and the panic cannot occur either.

Verified against a locally patched build:

"sessionUpdate":"agent_thought_chunk"  x13
"sessionUpdate":"agent_message_chunk"   -> expected reply
"stopReason":"end_turn"

No panic; the turn completes normally.

Two smaller notes from the same session

  • session/new requires mcpServers; omitting it returns missing field 'mcpServers'. Harmless, but the field is not obviously required from the docs.
  • In TCP mode the listener does a single accept() and serves that one connection, so a second client cannot attach. Worth documenting if intentional.

Happy to open a PR with the fix above if useful.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions