Add closed-session guard to CopilotSession

[Copilot]

Implementation Complete: Closed-Session Guard for CopilotSession

Summary

Successfully implemented fail-fast protection for CopilotSession that prevents usage after close() has been called, providing clear error messages instead of cryptic RPC failures.

Changes Made

Core Implementation (CopilotSession.java)

• Added volatile boolean isTerminated field to track session lifecycle state
• Implemented ensureNotTerminated() validation method that throws IllegalStateException("Session is closed")
• Made close() idempotent using synchronized block to safely handle multiple invocations
• Added guards to all public methods that interact with session state:
  • send(String) and send(MessageOptions)
  • sendAndWait(String), sendAndWait(MessageOptions), sendAndWait(MessageOptions, long)
  • on(Consumer<AbstractSessionEvent>) and on(Class<T>, Consumer<T>)
  • getMessages()
  • abort()
  • setEventErrorHandler(EventErrorHandler)
  • setEventErrorPolicy(EventErrorPolicy)
• Preserved getter behavior: getSessionId() and getWorkspacePath() still work after close (field reads for logging)

Comprehensive Test Coverage (ClosedSessionGuardTest.java)

Created 15 new tests covering:

• All guarded methods throw IllegalStateException after termination
• Getters still work after close (as designed)
• close() is idempotent (safe to call multiple times)
• Try-with-resources double-close scenario works correctly
• Updated: Replaced Java assert keyword with JUnit assertions (assertTrue, assertEquals) to ensure tests run properly

Compatibility Fix (CopilotSessionTest.java)

• Updated existing test to handle new exception behavior
• Maintains backward compatibility by checking for both new IllegalStateException and legacy RPC errors

Benefits

• Fail-fast: Immediate, clear errors instead of hanging or timing out
• Standard Java pattern: Consistent with java.io streams, java.sql.Connection, etc.
• Thread-safe: Volatile field ensures visibility across threads
• Idempotent close: Safe for try-with-resources and explicit double-close scenarios
• Diagnostic-friendly: Clear "Session is closed" message for easier debugging

Test Results

✅ All 172 tests passing
✅ Code formatted with Spotless
✅ All test assertions use proper JUnit methods (no Java assert keyword)

Original prompt

---

This section details on the original issue you should resolve

<issue_title>Add closed-session guard to CopilotSession</issue_title>
<issue_description>## Problem

After close() is called on a CopilotSession, there is no guard preventing subsequent calls to send(), on(), sendAndWait(), abort(), getMessages(), or other public methods. These calls will likely fail with opaque RPC errors (e.g., timeouts, connection refused, or server-side "unknown session" responses), which are confusing and hard to diagnose.

Current Behavior

session.close(); // destroys session on server, clears handlers

// These all still "work" — no immediate error, just eventual RPC failures
session.send("hello");             // RPC error: session not found
session.on(msg -> { ... });        // Silently registers handler on a dead session
session.sendAndWait("hello");      // Hangs or times out after 60 seconds

Proposal

Add an AtomicBoolean closed field and check it at the top of every public method:

private final AtomicBoolean closed = new AtomicBoolean(false);

@Override
public void close() {
    if (!closed.compareAndSet(false, true)) {
        return; // already closed, idempotent
    }
    // existing cleanup...
}

public CompletableFuture<String> send(String prompt) {
    checkNotClosed();
    return send(new MessageOptions().setPrompt(prompt));
}

private void checkNotClosed() {
    if (closed.get()) {
        throw new IllegalStateException("Session is closed");
    }
}

Methods to guard

All public methods that interact with the session:

• send(String) / send(MessageOptions)
• sendAndWait(String) / sendAndWait(MessageOptions) / sendAndWait(MessageOptions, long)
• on(Consumer) / on(Class, Consumer)
• getMessages()
• abort()
• setEventErrorHandler(EventErrorHandler)
• setEventErrorPolicy(EventErrorPolicy) (once PR Add EventErrorPolicy enum to control error propagation behavior #37 merges)

Design decisions

Aspect	Behavior
close() idempotency	Calling close() multiple times is safe (no-op after first)
Exception type	IllegalStateException("Session is closed") — standard Java convention
Thread safety	AtomicBoolean ensures visibility across threads
Getters	getSessionId() and getWorkspacePath() should still work after close (they're just field reads, useful for logging)

Benefits

• Fail-fast: immediate, clear error instead of hanging or cryptic RPC failures
• Standard Java pattern: consistent with java.io streams, java.sql.Connection, etc.
• Idempotent close: safe for try-with-resources double-close scenarios</issue_description>

Comments on the Issue (you are @copilot in this section)

• Fixes Add closed-session guard to CopilotSession #39

---

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

[Copilot]

Pull request overview

This PR adds a fail-fast “closed session” guard to CopilotSession so that calling session APIs after close() throws an immediate IllegalStateException instead of triggering slow/opaque RPC failures.

Changes:

• Added an isTerminated lifecycle flag with ensureNotTerminated() checks across session-interacting public APIs.
• Made CopilotSession.close() idempotent (safe to call multiple times / via try-with-resources).
• Added a dedicated ClosedSessionGuardTest suite and updated an existing session test expectation.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.

File	Description
src/main/java/com/github/copilot/sdk/CopilotSession.java	Adds termination tracking, idempotent close(), and fail-fast guards on public session operations.
src/test/java/com/github/copilot/sdk/ClosedSessionGuardTest.java	New tests validating post-close behavior across guarded APIs and idempotent close semantics.
src/test/java/com/github/copilot/sdk/CopilotSessionTest.java	Updates expectations to align with new fail-fast IllegalStateException behavior.

[Copilot]

assert here is the Java assertion keyword, which is typically disabled in Maven test runs unless the JVM is started with -ea. That means this test may pass without actually verifying the exception message. Use JUnit assertions (e.g., assertTrue / assertEquals) instead (and apply the same fix to the other assert ... statements in this test class).

[brunoborges]

@copilot apply changes based on this feedback

[Copilot]

Fixed in commit fac2e20. Replaced all Java assert statements with JUnit assertions (assertTrue with descriptive messages, assertEquals for value comparisons) to ensure proper test execution without requiring -ea flag.

[Copilot]

This comparison uses == on Strings, which checks reference identity rather than value equality and can make the test flaky/incorrect. Use a JUnit value assertion (e.g., assertEquals) and explicitly handle the null case if getWorkspacePath() can be null.

[Copilot]

Fixed in commit fac2e20. Replaced == reference comparison with assertEquals() which properly handles both null and non-null String values using value equality.

[Copilot]

Just as a heads up, I was blocked by some firewall rules while working on your feedback. Expand below for details.

Warning

Firewall rules blocked me from connecting to one or more addresses (expand for details)

I tried to connect to the following addresses, but was blocked by firewall rules:

• https://api.github.com/copilot_internal/user
  • Triggering command: /home/REDACTED/work/copilot-sdk-java/copilot-sdk-java/target/copilot-sdk/test/harness/node_modules/@github/copilot-linux-x64/copilot /home/REDACTED/work/copilot-sdk-java/copilot-sdk-java/target/copilot-sdk/test/harness/node_modules/@github/copilot-linux-x64/copilot --server --log-level info --stdio --auth-token-env COPILOT_SDK_AUTH_TOKEN --no-auto-login (http block)
  • Triggering command: /home/REDACTED/work/copilot-sdk-java/copilot-sdk-java/target/copilot-sdk/test/harness/node_modules/@github/copilot-linux-x64/copilot /home/REDACTED/work/copilot-sdk-java/copilot-sdk-java/target/copilot-sdk/test/harness/node_modules/@github/copilot-linux-x64/copilot --server --log-level info --stdio --auth-token-env COPILOT_SDK_AUTH_TOKEN --no-auto-login jacoco.agent/0.8.14/org.jacoco.agent-0.8.14-runtime.jar=destfile=/home/REDACTED/work/copilot-sdk- (http block)
  • Triggering command: /home/REDACTED/work/copilot-sdk-java/copilot-sdk-java/target/copilot-sdk/test/harness/node_modules/@github/copilot-linux-x64/copilot /home/REDACTED/work/copilot-sdk-java/copilot-sdk-java/target/copilot-sdk/test/harness/node_modules/@github/copilot-linux-x64/copilot --server --log-level info --stdio --auth-token-env COPILOT_SDK_AUTH_TOKEN --no-auto-login 9d4c5e76 (http block)

If you need me to access, download, or install something from one of these locations, you can either:

• Configure Actions setup steps to set up my environment, which run before the firewall is enabled
• Add the appropriate URLs or hosts to the custom allowlist in this repository's Copilot coding agent settings (admins only)