Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
47 commits
Select commit Hold shift + click to select a range
1243200
Add SessionFs sqlite support for runtime sqlite routing
SteveSandersonMS May 14, 2026
9746832
SessionFs: make sqlite required, remove handleSqlite flag
SteveSandersonMS May 15, 2026
375b38a
SDK: Update sqlite contract to flat sqliteQuery/sqliteExists
SteveSandersonMS May 15, 2026
5489ea9
Fix e2e test recording
SteveSandersonMS May 15, 2026
fe58b83
Update to make sessionFs.sqlite support optional
SteveSandersonMS May 15, 2026
340fea9
E2E test showing it works for subagents too
SteveSandersonMS May 15, 2026
aece0f0
.NET SDK: Add SessionFs SQLite support with ISessionFsSqliteProvider
SteveSandersonMS May 15, 2026
76a1885
Regenerate all generated files after rebase on main
SteveSandersonMS May 19, 2026
992d863
Remove api.schema.json - not part of this repo
SteveSandersonMS May 19, 2026
7bd8d0a
Fix rebase: remove stale abstract sqlite methods from SessionFsProvider
SteveSandersonMS May 19, 2026
5b7d12f
Simplify SQLite E2E tests to use in-memory FS instead of disk
SteveSandersonMS May 19, 2026
ee5d33a
Extract InMemorySessionFsSqliteHandler into reusable file
SteveSandersonMS May 19, 2026
9c6d4c0
Change SessionFsSqliteResult.Rows to IList<object?[]> and LastInsertR…
SteveSandersonMS May 19, 2026
6169962
Remove unused variable msg in subagent test
SteveSandersonMS May 19, 2026
8723765
Fix CI: update existing tests to use sqlite provider interface
SteveSandersonMS May 19, 2026
132d70b
Fix Node.js formatting (prettier)
SteveSandersonMS May 19, 2026
48a810d
Add error handling to sqliteQuery/sqliteExists adapter methods
SteveSandersonMS May 19, 2026
b37d69e
Revert "Add error handling to sqliteQuery/sqliteExists adapter methods"
SteveSandersonMS May 19, 2026
24027e8
Add SessionFs SQLite E2E tests for Python, Go, and Rust
SteveSandersonMS May 19, 2026
05ce059
Fix CI: formatting, Go race, Node sqlite error handling, scenario go.mod
SteveSandersonMS May 19, 2026
90c53d0
Remove try/catch from sqlite adapter methods, update tests to expect …
SteveSandersonMS May 19, 2026
80b4291
Fix Go and Rust CI: remove unused func, fix clippy warnings
SteveSandersonMS May 19, 2026
5387c54
Fix Rust clippy: collapse nested if-let and scope MutexGuard before a…
SteveSandersonMS May 19, 2026
220d29d
Fix Go SQLite E2E: force single connection for in-memory DB
SteveSandersonMS May 19, 2026
8d34029
Add comment explaining why sqlite methods skip toSessionFsError wrapping
SteveSandersonMS May 19, 2026
b5030f4
Replace real SQLite with stub in Go E2E tests, revert go.mod changes
SteveSandersonMS May 19, 2026
d853a4d
Rust SQLite E2E: use stub responses instead of real rusqlite
SteveSandersonMS May 19, 2026
aa324a6
.NET SQLite E2E: use stub responses instead of real Microsoft.Data.Sq…
SteveSandersonMS May 19, 2026
ec88807
Restore real SQLite in .NET and Rust E2E tests, fix Go stub comments
SteveSandersonMS May 19, 2026
b41e979
Make SessionFs SQLite optional in Go, Python, and Rust
SteveSandersonMS May 19, 2026
1ae46d6
Fix Python ruff errors: missing imports and line length
SteveSandersonMS May 19, 2026
2950a1a
Remove sqlite methods from non-sqlite Rust E2E test provider
SteveSandersonMS May 19, 2026
0a5f123
Fix ruff format: client.py and session_fs_provider.py
SteveSandersonMS May 19, 2026
6ab9e8e
Align Go/Python/Rust SQLite provider APIs with Node.js/.NET design
SteveSandersonMS May 19, 2026
a222c29
Reorder sqlite_query params: query_type before query (match Node.js/.…
SteveSandersonMS May 19, 2026
5533304
Fix Go formatting alignment in session_fs_provider.go
SteveSandersonMS May 19, 2026
d5a3b91
Fix Python type checking: rename shadowed variable, remove stale type…
SteveSandersonMS May 19, 2026
aaec17b
Fix E2E sqlite test session_id recording and Windows flaky test
SteveSandersonMS May 19, 2026
eaf0994
Align .NET sqlite provider Rows type with wire format
SteveSandersonMS May 19, 2026
e2efe84
Fix Rust nightly fmt formatting
SteveSandersonMS May 19, 2026
424f184
Rust formatting
SteveSandersonMS May 19, 2026
39ba0fd
E2E fixes
SteveSandersonMS May 19, 2026
01eb131
Another e2e fix
SteveSandersonMS May 19, 2026
209e35f
Address PR feedback: integer LastInsertRowid, optional query return, …
SteveSandersonMS May 19, 2026
fe4bbd3
Fix Python ruff line-length lint error
SteveSandersonMS May 19, 2026
b9f2cf4
Fix Python ruff format
SteveSandersonMS May 19, 2026
63eb8d1
Fix Go fmt alignment
SteveSandersonMS May 19, 2026
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions dotnet/Directory.Packages.props
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
<ItemGroup>
<PackageVersion Include="coverlet.collector" Version="6.0.4" />
<PackageVersion Include="Microsoft.Bcl.AsyncInterfaces" Version="10.0.2" />
<PackageVersion Include="Microsoft.Data.Sqlite" Version="9.0.6" />
<PackageVersion Include="Microsoft.Extensions.AI.Abstractions" Version="10.2.0" />
<PackageVersion Include="Microsoft.Extensions.Logging.Abstractions" Version="10.0.2" />
<PackageVersion Include="Microsoft.NET.Test.Sdk" Version="18.3.0" />
Expand Down
11 changes: 10 additions & 1 deletion dotnet/src/Client.cs
Original file line number Diff line number Diff line change
Expand Up @@ -1329,6 +1329,7 @@ await Rpc.SessionFs.SetProviderAsync(
_options.SessionFs.InitialCwd,
_options.SessionFs.SessionStatePath,
_options.SessionFs.Conventions,
_options.SessionFs.Capabilities,
cancellationToken: cancellationToken);
}

Expand All @@ -1345,8 +1346,16 @@ private void ConfigureSessionFsHandlers(CopilotSession session, Func<CopilotSess
"CreateSessionFsHandler is required in the session config when CopilotClientOptions.SessionFs is configured.");
}

session.ClientSessionApis.SessionFs = createSessionFsHandler(session)
var provider = createSessionFsHandler(session)
?? throw new InvalidOperationException("CreateSessionFsHandler returned null.");

if (_options.SessionFs.Capabilities?.Sqlite == true && provider is not ISessionFsSqliteProvider)
{
throw new InvalidOperationException(
"SessionFsConfig declares capabilities.sqlite but the provider does not implement ISessionFsSqliteProvider.");
}

session.ClientSessionApis.SessionFs = provider;
}

private async Task VerifyProtocolVersionAsync(Connection connection, CancellationToken cancellationToken)
Expand Down
105 changes: 72 additions & 33 deletions dotnet/src/SessionFsProvider.cs
Original file line number Diff line number Diff line change
Expand Up @@ -6,11 +6,60 @@

namespace GitHub.Copilot.SDK;

/// <summary>
/// Result of a SQLite query execution via <see cref="ISessionFsSqliteProvider"/>.
/// Same shape as <see cref="SessionFsSqliteQueryResult"/> but without the <c>Error</c> field,
/// since providers signal errors by throwing.
/// </summary>
public class SessionFsSqliteResult
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cross-SDK naming inconsistency: This class is named SessionFsSqliteResult, but all other SDKs name the equivalent type SessionFsSqliteQueryResult:

  • Go: SessionFsSqliteQueryResult (go/session_fs_provider.go)
  • Node.js: SessionFsSqliteQueryResult (nodejs/src/sessionFsProvider.ts)
  • Python: SessionFsSqliteQueryResult (python/copilot/session_fs_provider.py)
  • Rust: SessionFsSqliteQueryResult (rust/src/session_fs.rs)
  • .NET: SessionFsSqliteResult ← missing "Query"

Suggestion: Rename to SessionFsSqliteQueryResult to align with the other SDKs. This is a public API type and the name divergence could confuse developers working across languages.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cross-SDK naming inconsistency: SessionFsSqliteResult vs SessionFsSqliteQueryResult

All other SDK implementations name the provider-facing query result type SessionFsSqliteQueryResult:

  • Node.js: SessionFsSqliteQueryResult (sessionFsProvider.ts)
  • Python: SessionFsSqliteQueryResult (session_fs_provider.py)
  • Go: SessionFsSqliteQueryResult (session_fs_provider.go)
  • Rust: SessionFsSqliteQueryResult (session_fs.rs)

The .NET type here is named SessionFsSqliteResult, dropping the Query segment. Suggest renaming to SessionFsSqliteQueryResult to keep API naming consistent across the SDK family (and match the doc comment on line 11 which already references SessionFsSqliteQueryResult).

// Suggested rename:
public class SessionFsSqliteQueryResult { ... }

{
/// <summary>Column names from the result set.</summary>
public IList<string> Columns { get; set; } = [];

/// <summary>For SELECT: rows as column-keyed dictionaries. For others: empty.</summary>
public IList<IDictionary<string, object>> Rows { get; set; } = [];

/// <summary>Number of rows affected (for INSERT/UPDATE/DELETE).</summary>
public long RowsAffected { get; set; }

/// <summary>Last inserted row ID (for INSERT).</summary>
public long? LastInsertRowid { get; set; }
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cross-SDK type inconsistency: LastInsertRowid is typed as long? here, but all other SDKs use a floating-point type for this field:

  • Go: *float64
  • Rust: Option<f64>
  • Python: float | None
  • Node.js: number | undefined (via Omit<GeneratedSqliteQueryResult, "error">)
  • The generated wire-format RPC type in .NET itself also uses double? (see SessionFsSqliteQueryResult in Rpc.cs)

Using long? is semantically reasonable since SQLite rowids are always integers, but it diverges from the rest of the SDK surface. It also means there's an implicit long → double widening in the adapter (result?.LastInsertRowid at line 297), which could lose precision for rowids > 2^53 on paper (unlikely in practice).

Suggestion: Use double? to match the wire type and other SDKs. If you prefer integer semantics, consider adding a // SQLite rowids are always integers; cast is safe comment in the adapter.

}

/// <summary>
/// Optional interface for <see cref="SessionFsProvider"/> subclasses that support
/// per-session SQLite databases. Implement this interface on your provider to enable
/// the runtime's SQL tool to route queries through your SessionFs implementation.
/// </summary>
public interface ISessionFsSqliteProvider
{
/// <summary>
/// Executes a SQLite query against the per-session database.
/// </summary>
/// <param name="queryType">How to execute: <c>"exec"</c> for DDL/multi-statement, <c>"query"</c> for SELECT, <c>"run"</c> for INSERT/UPDATE/DELETE.</param>
/// <param name="query">SQL query to execute.</param>
/// <param name="bindParams">Optional named bind parameters.</param>
/// <param name="cancellationToken">Cancellation token.</param>
/// <returns>The query result, or <c>null</c> for exec-type queries.</returns>
Task<SessionFsSqliteResult?> QueryAsync(
SessionFsSqliteQueryType queryType,
string query,
IDictionary<string, object>? bindParams,
CancellationToken cancellationToken);

/// <summary>
/// Checks whether the per-session SQLite database already exists, without creating it.
/// </summary>
/// <param name="cancellationToken">Cancellation token.</param>
Task<bool> ExistsAsync(CancellationToken cancellationToken);
}

/// <summary>
/// Base class for session filesystem providers. Subclasses override the
/// virtual methods and use normal C# patterns (return values, throw exceptions).
/// The base class catches exceptions and converts them to <see cref="SessionFsError"/>
/// results expected by the runtime.
/// To add SQLite support, also implement <see cref="ISessionFsSqliteProvider"/>.
/// </summary>
public abstract class SessionFsProvider : ISessionFsHandler
{
Expand Down Expand Up @@ -75,24 +124,6 @@ public abstract class SessionFsProvider : ISessionFsHandler
/// <param name="cancellationToken">Cancellation token.</param>
protected abstract Task RenameAsync(string src, string dest, CancellationToken cancellationToken);

/// <summary>Executes a SQLite query against the per-session database.</summary>
/// <param name="sessionId">Target session identifier.</param>
/// <param name="query">SQL query to execute.</param>
/// <param name="queryType">How to execute the query.</param>
/// <param name="parameters">Optional named bind parameters.</param>
/// <param name="cancellationToken">Cancellation token.</param>
protected abstract Task<SessionFsSqliteQueryResult> SqliteQueryAsync(
string sessionId,
string query,
SessionFsSqliteQueryType queryType,
IDictionary<string, object>? parameters,
CancellationToken cancellationToken);

/// <summary>Checks whether the per-session SQLite database already exists.</summary>
/// <param name="sessionId">Target session identifier.</param>
/// <param name="cancellationToken">Cancellation token.</param>
protected abstract Task<bool> SqliteExistsAsync(string sessionId, CancellationToken cancellationToken);

// ---- ISessionFsHandler implementation (private, handles error mapping) ----

async Task<SessionFsReadFileResult> ISessionFsHandler.ReadFileAsync(SessionFsReadFileRequest request, CancellationToken cancellationToken)
Expand Down Expand Up @@ -246,42 +277,50 @@ async Task<SessionFsReaddirWithTypesResult> ISessionFsHandler.ReaddirWithTypesAs

async Task<SessionFsSqliteQueryResult> ISessionFsHandler.SqliteQueryAsync(SessionFsSqliteQueryRequest request, CancellationToken cancellationToken)
{
ArgumentNullException.ThrowIfNull(request);
if (this is not ISessionFsSqliteProvider sqliteProvider)
{
return new SessionFsSqliteQueryResult
{
Error = new SessionFsError { Code = SessionFsErrorCode.UNKNOWN, Message = "SQLite is not supported by this provider." },
};
}

try
{
return await SqliteQueryAsync(request.SessionId, request.Query, request.QueryType, request.Params, cancellationToken).ConfigureAwait(false);
var result = await sqliteProvider.QueryAsync(request.QueryType, request.Query, request.Params, cancellationToken).ConfigureAwait(false);

return new SessionFsSqliteQueryResult
{
Rows = result?.Rows ?? [],
Columns = result?.Columns ?? [],
RowsAffected = result?.RowsAffected ?? 0,
LastInsertRowid = result?.LastInsertRowid,
};
}
catch (Exception ex) when (!IsCriticalException(ex))
catch (Exception ex)
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Behavioral inconsistency with Node.js and Python for SQL execution errors (same issue as go/session_fs_provider.go):

  • Node.js: SQL errors propagate to the JSON-RPC layer (no try/catch)
  • Python: SQL errors propagate to the JSON-RPC layer (no try/except)
  • .NET (here): SQL errors are caught and returned as result-level SessionFsError

If the intent is for SQL errors to propagate (preserving original error detail in the JSON-RPC response), this catch block should be removed. If result-level error wrapping is preferred, Node.js and Python should add equivalent catch blocks.

Note: the previous implementation had catch (Exception ex) when (!IsCriticalException(ex)) — the new bare catch (Exception ex) will now also swallow critical exceptions like OutOfMemoryException. This may be intentional given the redesign, but worth double-checking.

{
return new SessionFsSqliteQueryResult { Error = ToSessionFsError(ex) };
}
Comment thread
github-code-quality[bot] marked this conversation as resolved.
Fixed
}

async Task<SessionFsSqliteExistsResult> ISessionFsHandler.SqliteExistsAsync(SessionFsSqliteExistsRequest request, CancellationToken cancellationToken)
{
ArgumentNullException.ThrowIfNull(request);
if (this is not ISessionFsSqliteProvider sqliteProvider)
{
return new SessionFsSqliteExistsResult { Exists = false };
}

try
{
var exists = await SqliteExistsAsync(request.SessionId, cancellationToken).ConfigureAwait(false);
var exists = await sqliteProvider.ExistsAsync(cancellationToken).ConfigureAwait(false);
return new SessionFsSqliteExistsResult { Exists = exists };
}
catch (Exception ex) when (!IsCriticalException(ex))
catch
{
return new SessionFsSqliteExistsResult { Exists = false };
}
Comment thread
github-code-quality[bot] marked this conversation as resolved.
Fixed
}

private static bool IsCriticalException(Exception ex) =>
ex is OperationCanceledException
or OutOfMemoryException
or StackOverflowException
or AccessViolationException
or AppDomainUnloadedException
or BadImageFormatException
or CannotUnloadAppDomainException
or InvalidProgramException;

private static SessionFsError ToSessionFsError(Exception ex)
{
Expand Down
7 changes: 7 additions & 0 deletions dotnet/src/Types.cs
Original file line number Diff line number Diff line change
Expand Up @@ -323,6 +323,13 @@ public sealed class SessionFsConfig
/// Path conventions used by this filesystem provider.
/// </summary>
public required SessionFsSetProviderConventions Conventions { get; init; }

/// <summary>
/// Optional capabilities that this filesystem provider supports.
/// When <see cref="SessionFsSetProviderCapabilities.Sqlite"/> is <c>true</c>,
/// the runtime routes SQLite queries through the provider instead of using a local database file.
/// </summary>
public SessionFsSetProviderCapabilities? Capabilities { get; init; }
}

/// <summary>
Expand Down
Loading
Loading