Skip to content

Document ACP session initialization delay - #1070

Merged
tbrandenburg merged 3 commits into
mainfrom
copilot/add-session-initialization-comment
Feb 3, 2026
Merged

Document ACP session initialization delay#1070
tbrandenburg merged 3 commits into
mainfrom
copilot/add-session-initialization-comment

Conversation

Copilot AI commented Feb 2, 2026

Copy link
Copy Markdown
Contributor

The initializeSession method specifies a 30-second timeout without explaining why such a long timeout is needed. PoC documentation already notes session creation takes 5-7 seconds on first run, but this knowledge wasn't captured in the implementation.

Changes

Added documentation to src/core/target-handlers/acp-handler.ts:

private async initializeSession(
  process: ChildProcess,
  config: ACPTargetConfig
): Promise<string> {
  // NOTE: Session creation typically takes 5-7 seconds on first run
  // as the ACP client bootstraps the environment. Subsequent sessions
  // with persisted sessionId are much faster.

  // Initialize protocol (fast, < 1s)
  await this.sendRequest(process, 'initialize', {...}, config.timeout || 30);

  // Create session (slow, 5-7s on first run)
  const sessionResult = await this.sendRequest(
    process, 'session/new', {...}, config.timeout || 30
  );

  return sessionResult.sessionId;
}

Clarifies performance characteristics and sets expectations for developers using this method.

Original prompt

Add a documentation comment to explain the delay in ACP session initialization during the first run. In the Proof of Concept (PoC), documentation in dev/poc-opencode-server/README.md:199-205 and dev/poc-opencode-server/complete-acp-test.js:125-132 clarifies that the session creation takes 5-7 seconds on the first run due to the ACP client bootstrapping the user's environment. However, this delay is not explained in our current implementation, even though a 30-second timeout for initialization is specified.

Proposed solution:

Include the following note in the initializeSession method (src/core/target-handlers/acp-handler.ts:165-196):

private async initializeSession(
  process: ChildProcess,
  config: ACPTargetConfig
): Promise<string> {
  // NOTE: Session creation typically takes 5-7 seconds on first run
  // as the ACP client bootstraps the environment. Subsequent sessions
  // with persisted sessionId are much faster.

  // Initialize protocol (fast, < 1s)
  await this.sendRequest(process, 'initialize', {...}, config.timeout || 30);

  // Create session (slow, 5-7s on first run)
  const sessionResult = await this.sendRequest(
    process, 'session/new', {...}, config.timeout || 30
  );

  return sessionResult.sessionId;
}

The above comment should be added to describe that the delay is due to the ACP client bootstrapping the environment during the session's first run. This ensures developers using the method have accurate expectations and the implementation aligns with PoC documentation.

This pull request was created from Copilot chat.


✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.

Co-authored-by: tbrandenburg <40181002+tbrandenburg@users.noreply.github.com>
Co-authored-by: tbrandenburg <40181002+tbrandenburg@users.noreply.github.com>
Copilot AI changed the title [WIP] Add documentation comment for session initialization delay Document ACP session initialization delay Feb 3, 2026
Copilot AI requested a review from tbrandenburg February 3, 2026 07:51
@tbrandenburg
tbrandenburg merged commit 54a3d3c into main Feb 3, 2026
4 checks passed
@tbrandenburg
tbrandenburg deleted the copilot/add-session-initialization-comment branch February 3, 2026 09:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Documentation: Document ACP prompt format choice Documentation: Add comment explaining ACP session initialization timing

2 participants