Skip to content

Draft: Support providing a custom session store#917

Draft
SteveSandersonMS wants to merge 8 commits intomainfrom
sdk-session-store-abstraction
Draft

Draft: Support providing a custom session store#917
SteveSandersonMS wants to merge 8 commits intomainfrom
sdk-session-store-abstraction

Conversation

@SteveSandersonMS
Copy link
Copy Markdown
Contributor

Not yet ready.

@SteveSandersonMS SteveSandersonMS force-pushed the sdk-session-store-abstraction branch 2 times, most recently from 5b7cfff to 152e970 Compare March 25, 2026 15:38
@github-actions
Copy link
Copy Markdown
Contributor

Cross-SDK Consistency Review

I've reviewed this PR for consistency with the other SDK implementations (Python, Go, and .NET). This PR introduces a custom session data store feature to the Node.js/TypeScript SDK only.

Summary of Changes

This PR adds:

  • SessionDataStoreConfig interface with a descriptor field and handler methods (load, append, truncate, list, delete)
  • New sessionDataStore option in CopilotClientOptions
  • Client-side RPC handlers via registerClientApiHandlers() that delegate storage operations to user-provided callbacks
  • Automatic registration of the data store when sessionDataStore is provided during connection
  • Test coverage in nodejs/test/e2e/session_store.test.ts

Consistency Gap Identified

Finding: This feature is not yet implemented in Python, Go, or .NET SDKs.

The equivalent configuration options in other SDKs are:

  • Python: SubprocessConfig and ExternalServerConfig (in python/copilot/client.py)
  • Go: ClientOptions struct (in go/types.go)
  • .NET: CopilotClientOptions class (in dotnet/src/Types.cs)

None of these currently have a sessionDataStore / session_data_store / SessionDataStore option.

Recommendation

Since this PR is marked as Draft and the body says "Not yet ready," I suggest:

  1. Consider this a phased rollout: If the goal is to validate the API design in Node.js first before implementing in other languages, that's a reasonable approach. Just ensure there's a plan to bring feature parity to other SDKs.

  2. For a consistent multi-language release, you may want to:

    • Add equivalent session_data_store support to Python (SubprocessConfig / client constructor)
    • Add SessionDataStore field to Go's ClientOptions struct
    • Add SessionDataStore property to .NET's CopilotClientOptions class
    • Ensure the handler interface (load, append, truncate, list, delete) is similarly structured across languages (accounting for naming conventions)
  3. API naming patterns to maintain:

    • Node.js: sessionDataStore (camelCase) ✅
    • Python: session_data_store (snake_case)
    • Go: SessionDataStore (PascalCase for public fields)
    • .NET: SessionDataStore (PascalCase)

Conclusion

No immediate action required since this is a draft PR. However, before marking this feature as generally available, consider implementing equivalent functionality in Python, Go, and .NET to maintain feature parity across the SDK suite. If this is intended as a Node.js-exclusive feature, that should be clearly documented with rationale.

Great work on the test coverage and RPC handler design! The approach is clean and extensible. 👍

Generated by SDK Consistency Review Agent for issue #917 ·

@github-actions
Copy link
Copy Markdown
Contributor

Cross-SDK Consistency Review

I've reviewed this PR for consistency across the four SDK implementations (Node.js/TypeScript, Python, Go, and .NET).

Summary

This PR introduces a custom session data store feature to the Node.js/TypeScript SDK only. This creates a feature parity gap with the other SDK implementations.

What This PR Adds

The PR adds a sessionDataStore option to CopilotClientOptions that allows applications to provide custom storage backends for session event persistence. Key aspects:

  1. New client option: sessionDataStore?: SessionDataStoreConfig in CopilotClientOptions
  2. Handler interface: SessionDataStoreHandler with methods: load(), append(), truncate(), list(), delete()
  3. Client API RPC registration: New registerClientApiHandlers() function that sets up bi-directional RPC handlers
  4. Automatic registration: When configured, the client calls sessionDataStore.setDataStore on connection and registers RPC handlers for the CLI to invoke

Cross-SDK Status

SDK Custom Session Store Support
Node.js/TypeScript ✅ Added in this PR
Python ❌ Not implemented
Go ❌ Not implemented
.NET ❌ Not implemented

Recommendation

To maintain feature parity, this capability should be added to the other three SDKs. Here's how it would map to each language's conventions:

Python (python/copilot/client.py)

  • Add session_data_store?: SessionDataStoreConfig to connection config (matching snake_case convention)
  • Create SessionDataStoreHandler protocol/interface with async methods
  • Example:
`@dataclass`
class SessionDataStoreConfig:
    descriptor: str
    load: Callable[[SessionDataStoreLoadParams], Awaitable[SessionDataStoreLoadResult]]
    append: Callable[[SessionDataStoreAppendParams], Awaitable[None]]
    truncate: Callable[[SessionDataStoreTruncateParams], Awaitable[SessionDataStoreTruncateResult]]
    list: Callable[[], Awaitable[SessionDataStoreListResult]]
    delete: Callable[[SessionDataStoreDeleteParams], Awaitable[None]]

Go (go/client.go, go/types.go)

  • Add SessionDataStore *SessionDataStoreConfig to ClientOptions (matching Go naming)
  • Create SessionDataStoreHandler interface
  • Example:
type SessionDataStoreHandler interface {
    Load(ctx context.Context, params SessionDataStoreLoadParams) (*SessionDataStoreLoadResult, error)
    Append(ctx context.Context, params SessionDataStoreAppendParams) error
    Truncate(ctx context.Context, params SessionDataStoreTruncateParams) (*SessionDataStoreTruncateResult, error)
    List(ctx context.Context) (*SessionDataStoreListResult, error)
    Delete(ctx context.Context, params SessionDataStoreDeleteParams) error
}

type SessionDataStoreConfig struct {
    Descriptor string
    Handler    SessionDataStoreHandler
}

.NET (dotnet/src/Types.cs, dotnet/src/Client.cs)

  • Add SessionDataStore property to CopilotClientOptions (PascalCase)
  • Create ISessionDataStoreHandler interface or callback delegates
  • Example:
public interface ISessionDataStoreHandler
{
    Task(SessionDataStoreLoadResult) LoadAsync(SessionDataStoreLoadParams parameters, CancellationToken cancellationToken = default);
    Task AppendAsync(SessionDataStoreAppendParams parameters, CancellationToken cancellationToken = default);
    Task(SessionDataStoreTruncateResult) TruncateAsync(SessionDataStoreTruncateParams parameters, CancellationToken cancellationToken = default);
    Task(SessionDataStoreListResult) ListAsync(CancellationToken cancellationToken = default);
    Task DeleteAsync(SessionDataStoreDeleteParams parameters, CancellationToken cancellationToken = default);
}

public sealed class SessionDataStoreConfig
{
    public required string Descriptor { get; init; }
    public required ISessionDataStoreHandler Handler { get; init; }
}

Notes

  • This is marked as a draft PR with "Not yet ready" in the description, so it may still be in development
  • The feature is valuable for applications that need custom storage (e.g., Redis, PostgreSQL, cloud storage)
  • Given that this is a draft, consider whether implementation should happen across all SDKs before merging, or if language-specific rollout is acceptable
  • The E2E tests added (nodejs/test/e2e/session_store.test.ts) demonstrate the feature thoroughly, which will be helpful when implementing in other languages

Would you like me to create tracking issues for the other SDK implementations, or is this intentionally a Node.js-only feature for now?

Generated by SDK Consistency Review Agent for issue #917 ·

- Add sessionDataStore option to CopilotClientOptions
- Extend codegen to generate client API handler types (SessionDataStoreHandler)
- Register as session data storage provider on connection via sessionDataStore.setDataStore RPC
- Add E2E tests for persist, resume, list, delete, and reject scenarios

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@SteveSandersonMS SteveSandersonMS force-pushed the sdk-session-store-abstraction branch from 152e970 to 3d11cff Compare March 25, 2026 15:45
@github-actions
Copy link
Copy Markdown
Contributor

Cross-SDK Consistency Review

This PR introduces a significant new feature: custom session data storage backend support via the sessionDataStore option. However, this feature is currently only implemented in the Node.js/TypeScript SDK.

What This PR Adds (Node.js/TypeScript only)

  1. New client option: sessionDataStore in CopilotClientOptions that allows providing custom callbacks for:

    • load(sessionId) - Load session events from storage
    • append(sessionId, events) - Append new events to storage
    • truncate(sessionId, upToEventId) - Truncate old events (compaction)
    • list() - List all sessions in storage
    • delete(sessionId) - Delete a session from storage
  2. Generated RPC types: New SessionDataStore* types in generated/rpc.ts including handler interfaces and registration helpers

  3. Integration: The client automatically calls sessionDataStore.setDataStore on connection and registers RPC handlers to route storage operations to the provided callbacks

  4. E2E tests: Comprehensive test suite demonstrating the feature with an in-memory store implementation

Missing From Other SDKs

Python (python/copilot/)

  • ❌ No session_data_store option in client configuration
  • ❌ No equivalent handler interface or callbacks
  • ❌ No tests for this feature

Go (go/)

  • ❌ No SessionDataStore field in ClientOptions
  • ❌ No equivalent handler interface or callbacks
  • ❌ No tests for this feature

.NET (dotnet/src/)

  • ❌ No SessionDataStore property in CopilotClientOptions
  • ❌ No equivalent handler interface or callbacks
  • ❌ No tests for this feature

Recommendation

This is a valuable feature for users who want to:

  • Store session state in Redis, databases, or other persistence layers
  • Implement custom retention/archival policies
  • Share session state across distributed systems

To maintain SDK consistency, this feature should be implemented across all four SDKs with equivalent APIs (accounting for language conventions):

  • Python: session_data_store parameter with Protocol or ABC defining required methods
  • Go: SessionDataStore field in ClientOptions with interface type defining required methods
  • .NET: SessionDataStore property in CopilotClientOptions with interface defining required methods

Each SDK should follow the same pattern established here:

  1. Client option to provide storage handlers
  2. Automatic registration on connection via sessionDataStore.setDataStore RPC
  3. Handler callbacks invoked by the CLI server for persistence operations
  4. E2E tests with an in-memory store implementation

Note: This PR is marked as draft and states "Not yet ready", so there may be time to implement this feature across all SDKs before merging. If the intent is to ship this feature incrementally (Node.js first), please add tracking issues for the other SDKs and update their respective roadmaps/documentation to note the feature gap.

Generated by SDK Consistency Review Agent for issue #917 ·

Migrate the TypeScript SDK from the event-level sessionDataStore
abstraction to the general-purpose SessionFs virtual filesystem,
matching the runtime's new design (copilot-agent-runtime#5432).

Key changes:
- Regenerate RPC types from runtime schema with sessionFs.* methods
- Replace SessionDataStoreConfig with SessionFsConfig (initialCwd,
  sessionStatePath, conventions + 9 filesystem handler callbacks)
- Client calls sessionFs.setProvider on connect (was setDataStore)
- Client registers sessionFs.* RPC handlers (readFile, writeFile,
  appendFile, exists, stat, mkdir, readdir, rm, rename)
- New E2E tests with InMemorySessionFs (filesystem-level, not events)
- Remove old session_store tests and snapshots
@github-actions
Copy link
Copy Markdown
Contributor

Cross-SDK Consistency Review: Session Filesystem Provider

This PR introduces a new session filesystem provider feature to the Node.js/TypeScript SDK only. This represents a consistency gap across the SDK implementations.

Feature Summary

The PR adds the ability to provide a custom storage backend for session state via the sessionFs option in CopilotClientOptions. This allows applications to:

  • Intercept all file I/O operations for session state (events, workspace, checkpoints)
  • Implement custom storage backends (e.g., cloud storage, databases, virtual filesystems)
  • Control where and how session data is persisted

Added APIs:

  • CopilotClientOptions.sessionFs?: SessionFsConfig - Configuration property
  • SessionFsConfig interface extending SessionFsHandler with initialCwd, sessionStatePath, and conventions
  • SessionFsHandler interface with methods: readFile, writeFile, appendFile, exists, stat, mkdir, readdir, rm, rename
  • Client-side RPC handler registration via registerClientApiHandlers

Cross-SDK Consistency Analysis

Current state:

  • Node.js/TypeScript: Full implementation (this PR)
  • Python: No equivalent feature
  • Go: No equivalent feature
  • .NET: No equivalent feature

Recommendation

This is a useful extensibility feature that would benefit all SDK implementations. Consider whether:

  1. This feature should be added to other SDKs to maintain feature parity, especially for:

    • Applications needing cloud-based session storage
    • Multi-tenant scenarios with isolated storage
    • Testing scenarios with mock filesystems (note: the test uses @platformatic/vfs)
  2. Language-specific alternatives might be more idiomatic:

    • Python: Could use protocol classes or abstract base classes
    • Go: Could use interface types (already idiomatic)
    • .NET: Could use interface types with async methods
  3. Documentation should clarify this is currently Node.js-only if other SDKs won't implement it soon

Implementation Notes (if extending to other SDKs)

The feature requires:

  • Client-side JSON-RPC request handlers (reverse RPC from server → client)
  • Async file operation interfaces matching the 9 methods in SessionFsHandler
  • Registration call (sessionFs.setProvider) sent after connection establishment
  • Path convention awareness (Windows vs. Linux)

Python equivalent naming:

class SessionFsHandler(Protocol):
    async def read_file(self, params: SessionFsReadFileParams) -> SessionFsReadFileResult: ...
    async def write_file(self, params: SessionFsWriteFileParams) -> None: ...
    # ... etc

Go equivalent naming:

type SessionFsHandler interface {
    ReadFile(ctx context.Context, params SessionFsReadFileParams) (SessionFsReadFileResult, error)
    WriteFile(ctx context.Context, params SessionFsWriteFileParams) error
    // ... etc
}

.NET equivalent naming:

public interface ISessionFsHandler {
    Task(SessionFsReadFileResult) ReadFileAsync(SessionFsReadFileParams params, CancellationToken cancellationToken = default);
    Task WriteFileAsync(SessionFsWriteFileParams params, CancellationToken cancellationToken = default);
    // ... etc
}

Status: This PR is architecturally sound for the Node.js SDK. The consistency gap is flagged for awareness and future planning, not as a blocker for this PR.

Generated by SDK Consistency Review Agent for issue #917 ·

@paulpach
Copy link
Copy Markdown

paulpach commented Mar 29, 2026

I am very interested in this feature. We would love to use something like redis to store the session data so that I can use this in a multi instance deployment.

Providing a virtual file system abstraction instead of session data storage abstraction (as originally coded) is a bit strange to me. In the commit, copilot-agent-runtime#5432 is mentioned, but there is no link, could you provide a link to that discussion? is that public?

@github-actions
Copy link
Copy Markdown
Contributor

Cross-SDK Consistency Review

This PR continues to add session filesystem provider support exclusively to the Node.js/TypeScript SDK.

Previous reviews have thoroughly documented this consistency gap (see March 27 review). The feature remains unimplemented in Python, Go, and .NET.

Latest Changes (March 30)

The most recent commit ("Expand API surface slightly") made minor additions to the generated RPC types and TypeScript types—no cross-SDK implications.

Key Observations

  1. User interest: A community member expressed interest in using this for distributed deployments with Redis backing.

  2. Feature scope: This provides filesystem-level abstraction (9 methods: readFile, writeFile, appendFile, exists, stat, mkdir, readdir, readdirWithTypes, rm, rename) rather than higher-level session storage.

  3. Draft status: PR remains marked as draft, allowing time to plan cross-SDK rollout strategy.

Recommendation

Given external user interest and the value for distributed/cloud scenarios, consider the rollout strategy:

  • Option A: Ship Node.js first to validate the API design, then implement in other SDKs
  • Option B: Implement across all SDKs before merging (maintaining strict feature parity)

If choosing Option A, document the feature gap in other SDK READMEs and create tracking issues for Python/Go/.NET implementations.


No new consistency issues introduced since the last review. The core consistency gap (Node.js-only feature) remains.

Generated by SDK Consistency Review Agent for issue #917 ·

@github-actions
Copy link
Copy Markdown
Contributor

Cross-SDK Consistency Review: Session Filesystem Provider

This PR introduces a custom session filesystem provider feature that allows Node.js SDK users to provide their own storage implementation for session-scoped files, routing all file I/O through custom callbacks instead of the server's default local filesystem.

✅ Current Implementation (Node.js only)

The PR adds:

  • SessionFsConfig interface with:
    • initialCwd: Initial working directory
    • sessionStatePath: Path for session-scoped files
    • conventions: "windows" | "linux" path conventions
    • createHandler: Factory function to create a SessionFsHandler per session
  • SessionFsHandler interface with methods like readFile, writeFile, mkdir, readdir, etc.
  • RPC integration via sessionFs.setProvider and client-side handlers for sessionFs.* RPC calls
  • Registration in CopilotClient constructor via sessionFs option

⚠️ Cross-SDK Consistency Gap

This feature does NOT exist in Python, Go, or .NET SDKs. All three SDKs:

  • Support infinite sessions with workspace persistence (via InfiniteSessionConfig)
  • Expose workspacePath / WorkspacePath property on sessions
  • Provide session.workspace.* RPC methods (listFiles, readFile, createFile)
  • Do NOT support custom storage providers - storage is always managed by the CLI server

🤔 Assessment

This appears to be an intentional Node.js-specific enhancement rather than an oversight, because:

  1. Language ecosystem alignment: Node.js has a rich ecosystem of virtual filesystem libraries (e.g., @platformatic/vfs added as a devDependency in this PR), making custom storage providers natural in JavaScript/TypeScript environments

  2. Platform requirements: Some Node.js hosting environments (serverless, edge functions) may not have reliable local filesystem access, making custom providers essential

  3. Implementation complexity: This feature requires:

    • Bidirectional RPC routing (server calls back into client)
    • Per-session handler lifecycle management
    • Protocol negotiation for provider registration

    Adding this to all SDKs is non-trivial

📋 Recommendation

If this feature is intended for Node.js only:

  • ✅ Document this as a Node.js-specific capability in the README
  • ✅ Consider whether Python, Go, and .NET will need this in the future (e.g., for cloud/serverless scenarios)

If this should be available in all SDKs:

  • ❌ This PR should not be merged until equivalent implementations exist in Python, Go, and .NET
  • ❌ Coordinate with maintainers to plan implementation timeline for other languages

Suggested documentation addition (if Node.js-only is intentional):

### Custom Session Filesystem Provider (Node.js only)

For environments without reliable local filesystem access, you can provide a custom storage backend:

```typescript
const client = new CopilotClient({
  sessionFs: {
    initialCwd: '/virtual/workspace',
    sessionStatePath: '.copilot/sessions',
    conventions: 'linux',
    createHandler: (session) => ({
      readFile: async ({ path }) => { /* ... */ },
      writeFile: async ({ path, content }) => { /* ... */ },
      // ... other methods
    })
  }
});
```

cc: @SteveSandersonMS - Is this intended as a Node.js-specific feature, or should we coordinate adding it to Python/Go/.NET as well?

Generated by SDK Consistency Review Agent for issue #917 ·

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.

2 participants