Align Go/Python/Rust SQLite provider APIs with Node.js/.NET design

SteveSandersonMS · Copilot · SteveSandersonMS · commit 6ab9e8e770dd · 2026-05-19T14:55:55.000+01:00
- Remove session_id parameter from sqlite_query/sqlite_exists (providers
  are already session-scoped)
- Add clean SessionFsSqliteQueryResult types without error field (errors
  signaled by raising/returning errors, not via result field)
- Update adapters to convert clean result types to generated RPC types
- Update all tests to use new signatures and clean result types

Co-authored-by: Copilot &lt;223556219+Copilot@users.noreply.github.com&gt;
diff --git a/go/internal/e2e/session_fs_sqlite_e2e_test.go b/go/internal/e2e/session_fs_sqlite_e2e_test.go
@@ -195,12 +195,12 @@ func (p *inMemorySqliteProvider) Rename(src string, dest string) error {
 	return nil
 }
 
-func (p *inMemorySqliteProvider) SqliteQuery(sessionID string, query string, queryType rpc.SessionFsSqliteQueryType, params map[string]any) (*rpc.SessionFsSqliteQueryResult, error) {
+func (p *inMemorySqliteProvider) SqliteQuery(query string, queryType rpc.SessionFsSqliteQueryType, params map[string]any) (*copilot.SessionFsSqliteQueryResult, error) {
 	p.mu.Lock()
 	defer p.mu.Unlock()
 	p.hadQuery = true
 	*p.sqliteCalls = append(*p.sqliteCalls, sqliteCall{
-		SessionID: sessionID,
+		SessionID: "stub",
 		QueryType: string(queryType),
 		Query:     query,
 	})
@@ -212,28 +212,28 @@ func (p *inMemorySqliteProvider) SqliteQuery(sessionID string, query string, que
 	upper := strings.ToUpper(strings.TrimSpace(query))
 	switch queryType {
 	case rpc.SessionFsSqliteQueryTypeExec:
-		return &rpc.SessionFsSqliteQueryResult{Columns: []string{}, Rows: []map[string]any{}}, nil
+		return &copilot.SessionFsSqliteQueryResult{Columns: []string{}, Rows: []map[string]any{}}, nil
 	case rpc.SessionFsSqliteQueryTypeRun:
 		lastID := float64(1)
-		return &rpc.SessionFsSqliteQueryResult{
+		return &copilot.SessionFsSqliteQueryResult{
 			Columns:         []string{},
 			Rows:            []map[string]any{},
 			RowsAffected:    1,
 			LastInsertRowid: &lastID,
 		}, nil
 	case rpc.SessionFsSqliteQueryTypeQuery:
 		if strings.Contains(upper, "SELECT") {
-			return &rpc.SessionFsSqliteQueryResult{
+			return &copilot.SessionFsSqliteQueryResult{
 				Columns: []string{"id", "name"},
 				Rows:    []map[string]any{{"id": "a1", "name": "Widget"}},
 			}, nil
 		}
-		return &rpc.SessionFsSqliteQueryResult{Columns: []string{}, Rows: []map[string]any{}}, nil
+		return &copilot.SessionFsSqliteQueryResult{Columns: []string{}, Rows: []map[string]any{}}, nil
 	}
-	return &rpc.SessionFsSqliteQueryResult{Columns: []string{}, Rows: []map[string]any{}}, nil
+	return &copilot.SessionFsSqliteQueryResult{Columns: []string{}, Rows: []map[string]any{}}, nil
 }
 
-func (p *inMemorySqliteProvider) SqliteExists(sessionID string) (bool, error) {
+func (p *inMemorySqliteProvider) SqliteExists() (bool, error) {
 	p.mu.Lock()
 	defer p.mu.Unlock()
 	return p.hadQuery, nil
diff --git a/go/session_fs_provider.go b/go/session_fs_provider.go
@@ -52,11 +52,24 @@ type SessionFsProvider interface {
 // may also implement to support per-session SQLite databases. The adapter
 // checks for this interface at runtime using a type assertion. If the
 // provider does not implement it, SQLite requests return an "unsupported" error.
+//
+// Providers are already session-scoped (created per session by the factory),
+// so these methods do not take a session ID parameter.
 type SessionFsSqliteProvider interface {
 	// SqliteQuery executes a SQLite query against the provider's per-session database.
-	SqliteQuery(sessionID string, query string, queryType rpc.SessionFsSqliteQueryType, params map[string]any) (*rpc.SessionFsSqliteQueryResult, error)
+	SqliteQuery(query string, queryType rpc.SessionFsSqliteQueryType, params map[string]any) (*SessionFsSqliteQueryResult, error)
 	// SqliteExists checks whether the provider has a SQLite database for the session.
-	SqliteExists(sessionID string) (bool, error)
+	SqliteExists() (bool, error)
+}
+
+// SessionFsSqliteQueryResult holds the result of a SQLite query execution.
+// Same shape as the generated RPC type but without the Error field,
+// since providers signal errors by returning a Go error.
+type SessionFsSqliteQueryResult struct {
+	Columns        []string         `json:"columns"`
+	Rows           []map[string]any `json:"rows"`
+	RowsAffected   int64            `json:"rowsAffected"`
+	LastInsertRowid *float64        `json:"lastInsertRowid,omitempty"`
 }
 
 // SessionFsFileInfo holds file metadata returned by SessionFsProvider.Stat.
@@ -188,7 +201,7 @@ func (a *sessionFsAdapter) SqliteQuery(request *rpc.SessionFsSqliteQueryRequest)
 			Error:        &rpc.SessionFsError{Code: rpc.SessionFsErrorCodeUNKNOWN, Message: &msg},
 		}, nil
 	}
-	result, err := sp.SqliteQuery(request.SessionID, request.Query, request.QueryType, request.Params)
+	result, err := sp.SqliteQuery(request.Query, request.QueryType, request.Params)
 	if err != nil {
 		return &rpc.SessionFsSqliteQueryResult{
 			Columns:      []string{},
@@ -197,15 +210,20 @@ func (a *sessionFsAdapter) SqliteQuery(request *rpc.SessionFsSqliteQueryRequest)
 			Error:        toSessionFsError(err),
 		}, nil
 	}
-	return result, nil
+	return &rpc.SessionFsSqliteQueryResult{
+		Columns:        result.Columns,
+		Rows:           result.Rows,
+		RowsAffected:   result.RowsAffected,
+		LastInsertRowid: result.LastInsertRowid,
+	}, nil
 }
 
 func (a *sessionFsAdapter) SqliteExists(request *rpc.SessionFsSqliteExistsRequest) (*rpc.SessionFsSqliteExistsResult, error) {
 	sp, ok := a.provider.(SessionFsSqliteProvider)
 	if !ok {
 		return &rpc.SessionFsSqliteExistsResult{Exists: false}, nil
 	}
-	exists, err := sp.SqliteExists(request.SessionID)
+	exists, err := sp.SqliteExists()
 	if err != nil {
 		return &rpc.SessionFsSqliteExistsResult{Exists: false}, nil
 	}
diff --git a/python/copilot/__init__.py b/python/copilot/__init__.py
@@ -43,6 +43,7 @@
     SessionFsFileInfo,
     SessionFsProvider,
     SessionFsSqliteProvider,
+    SessionFsSqliteQueryResult,
     create_session_fs_adapter,
 )
 from .tools import (
@@ -88,6 +89,7 @@
     "SessionFsFileInfo",
     "SessionFsProvider",
     "SessionFsSqliteProvider",
+    "SessionFsSqliteQueryResult",
     "create_session_fs_adapter",
     "SessionUiApi",
     "SessionUiCapabilities",
diff --git a/python/copilot/session_fs_provider.py b/python/copilot/session_fs_provider.py
@@ -33,10 +33,12 @@
     SessionFSReaddirWithTypesResult,
     SessionFSReadFileResult,
     SessionFSSqliteExistsResult,
-    SessionFSSqliteQueryResult,
     SessionFSSqliteQueryType,
     SessionFSStatResult,
 )
+from .generated.rpc import (
+    SessionFSSqliteQueryResult as _GeneratedSqliteQueryResult,
+)
 
 
 @dataclass
@@ -110,21 +112,37 @@ class MyProvider(SessionFsProvider, SessionFsSqliteProvider): ...
 
     The adapter checks ``isinstance(provider, SessionFsSqliteProvider)`` at
     runtime to decide whether SQLite calls should be dispatched.
+
+    Providers are already session-scoped (created per session by the factory),
+    so these methods do not take a ``session_id`` parameter.
     """
 
     @abc.abstractmethod
     async def sqlite_query(
         self,
-        session_id: str,
         query: str,
         query_type: SessionFSSqliteQueryType,
         params: dict[str, float | str | None] | None = None,
-    ) -> SessionFSSqliteQueryResult:
+    ) -> SessionFsSqliteQueryResult:
         """Execute a SQLite query against the provider's per-session database."""
 
     @abc.abstractmethod
-    async def sqlite_exists(self, session_id: str) -> bool:
-        """Return whether the provider has a SQLite database for *session_id*."""
+    async def sqlite_exists(self) -> bool:
+        """Return whether the provider has a SQLite database for this session."""
+
+
+@dataclass
+class SessionFsSqliteQueryResult:
+    """Result of a SQLite query execution.
+
+    Same shape as the generated RPC type but without the ``error`` field,
+    since providers signal errors by raising exceptions.
+    """
+
+    columns: list[str]
+    rows: list[dict[str, Any]]
+    rows_affected: int
+    last_insert_rowid: float | None = None
 
 
 def create_session_fs_adapter(provider: SessionFsProvider) -> SessionFsHandler:
@@ -240,12 +258,12 @@ async def rename(self, params: Any) -> SessionFSError | None:
         except Exception as exc:
             return _to_session_fs_error(exc)
 
-    async def sqlite_query(self, params: Any) -> SessionFSSqliteQueryResult:
+    async def sqlite_query(self, params: Any) -> _GeneratedSqliteQueryResult:
         # SQLite methods intentionally skip toSessionFsError wrapping — FS errno
         # mapping (ENOENT) isn't meaningful for SQL errors and the JSON-RPC layer
         # already handles uncaught exceptions.
         if not isinstance(self._p, SessionFsSqliteProvider):
-            return SessionFSSqliteQueryResult(
+            return _GeneratedSqliteQueryResult(
                 columns=[],
                 rows=[],
                 rows_affected=0,
@@ -254,18 +272,23 @@ async def sqlite_query(self, params: Any) -> SessionFSSqliteQueryResult:
                     message="SQLite is not supported by this SessionFs provider",
                 ),
             )
-        return await self._p.sqlite_query(
-            params.session_id,
+        result = await self._p.sqlite_query(
             params.query,
             params.query_type,
             getattr(params, "params", None),
         )
+        return _GeneratedSqliteQueryResult(
+            columns=result.columns,
+            rows=result.rows,
+            rows_affected=result.rows_affected,
+            last_insert_rowid=result.last_insert_rowid,
+        )
 
     async def sqlite_exists(self, params: Any) -> SessionFSSqliteExistsResult:
         if not isinstance(self._p, SessionFsSqliteProvider):
             return SessionFSSqliteExistsResult.from_dict({"exists": False})
         try:
-            result = await self._p.sqlite_exists(params.session_id)  # type: ignore[attr-defined]
+            result = await self._p.sqlite_exists()
             return SessionFSSqliteExistsResult.from_dict({"exists": result})
         except Exception:
             return SessionFSSqliteExistsResult.from_dict({"exists": False})
diff --git a/python/e2e/test_session_fs_sqlite_e2e.py b/python/e2e/test_session_fs_sqlite_e2e.py
@@ -17,14 +17,14 @@
 from copilot.generated.rpc import (
     SessionFSReaddirWithTypesEntry,
     SessionFSReaddirWithTypesEntryType,
-    SessionFSSqliteQueryResult,
     SessionFSSqliteQueryType,
 )
 from copilot.session import PermissionHandler
 from copilot.session_fs_provider import (
     SessionFsFileInfo,
     SessionFsProvider,
     SessionFsSqliteProvider,
+    SessionFsSqliteQueryResult,
 )
 
 from .testharness import DEFAULT_GITHUB_TOKEN, E2ETestContext
@@ -148,14 +148,12 @@ async def rename(self, src: str, dest: str) -> None:
 
     async def sqlite_query(
         self,
-        session_id: str,
         query: str,
         query_type: SessionFSSqliteQueryType,
         params: dict[str, float | str | None] | None = None,
-    ) -> SessionFSSqliteQueryResult:
+    ) -> SessionFsSqliteQueryResult:
         self._sqlite_calls.append(
             {
-                "sessionId": session_id,
                 "queryType": query_type.value,
                 "query": query,
             }
@@ -164,30 +162,30 @@ async def sqlite_query(
         db = self._get_or_create_db()
         trimmed = query.strip()
         if not trimmed:
-            return SessionFSSqliteQueryResult(columns=[], rows=[], rows_affected=0)
+            return SessionFsSqliteQueryResult(columns=[], rows=[], rows_affected=0)
 
         if query_type == SessionFSSqliteQueryType.EXEC:
             db.executescript(trimmed)
             db.commit()
-            return SessionFSSqliteQueryResult(columns=[], rows=[], rows_affected=0)
+            return SessionFsSqliteQueryResult(columns=[], rows=[], rows_affected=0)
 
         if query_type == SessionFSSqliteQueryType.QUERY:
             cursor = db.execute(trimmed, params or {})
             columns = [desc[0] for desc in cursor.description] if cursor.description else []
             rows = [dict(zip(columns, row)) for row in cursor.fetchall()]
-            return SessionFSSqliteQueryResult(columns=columns, rows=rows, rows_affected=0)
+            return SessionFsSqliteQueryResult(columns=columns, rows=rows, rows_affected=0)
 
         # run (INSERT/UPDATE/DELETE)
         cursor = db.execute(trimmed, params or {})
         db.commit()
-        return SessionFSSqliteQueryResult(
+        return SessionFsSqliteQueryResult(
             columns=[],
             rows=[],
             rows_affected=cursor.rowcount,
             last_insert_rowid=float(cursor.lastrowid) if cursor.lastrowid else None,
         )
 
-    async def sqlite_exists(self, session_id: str) -> bool:
+    async def sqlite_exists(self) -> bool:
         return self._db is not None
 
 
diff --git a/rust/src/session_fs.rs b/rust/src/session_fs.rs
@@ -48,7 +48,7 @@ use crate::generated::api_types::{
     SessionFsError, SessionFsErrorCode, SessionFsReaddirWithTypesEntry,
     SessionFsReaddirWithTypesEntryType, SessionFsSetProviderConventions, SessionFsStatResult,
 };
-pub use crate::generated::api_types::{SessionFsSqliteQueryResult, SessionFsSqliteQueryType};
+pub use crate::generated::api_types::SessionFsSqliteQueryType;
 
 /// Optional capabilities declared by a session filesystem provider.
 #[non_exhaustive]
@@ -388,6 +388,9 @@ pub trait SessionFsProvider: Send + Sync + 'static {
 
 /// Optional trait for providers that support SQLite operations.
 ///
+/// Providers are already session-scoped (created per session by the factory),
+/// so these methods do not take a `session_id` parameter.
+///
 /// To opt in, implement this trait on your provider and override
 /// [`SessionFsProvider::sqlite`] to return `Some(self)`:
 ///
@@ -407,14 +410,29 @@ pub trait SessionFsSqliteProvider: Send + Sync {
     /// Execute a SQLite query against the provider's per-session database.
     async fn sqlite_query(
         &self,
-        session_id: &str,
         query: &str,
         query_type: SessionFsSqliteQueryType,
         params: Option<&HashMap<String, serde_json::Value>>,
     ) -> Result<SessionFsSqliteQueryResult, FsError>;
 
-    /// Check whether the provider has a SQLite database for the session.
-    async fn sqlite_exists(&self, session_id: &str) -> Result<bool, FsError>;
+    /// Check whether the provider has a SQLite database for this session.
+    async fn sqlite_exists(&self) -> Result<bool, FsError>;
+}
+
+/// Result of a SQLite query execution via [`SessionFsSqliteProvider::sqlite_query`].
+///
+/// Same shape as the generated RPC type but without the `error` field,
+/// since providers signal errors by returning `Err`.
+#[derive(Debug, Clone, Default)]
+pub struct SessionFsSqliteQueryResult {
+    /// Column names from the result set.
+    pub columns: Vec<String>,
+    /// For SELECT: array of row objects. For others: empty array.
+    pub rows: Vec<HashMap<String, serde_json::Value>>,
+    /// Number of rows affected (for INSERT/UPDATE/DELETE).
+    pub rows_affected: i64,
+    /// Last inserted row ID (for INSERT).
+    pub last_insert_rowid: Option<f64>,
 }
 
 #[cfg(test)]
diff --git a/rust/src/session_fs_dispatch.rs b/rust/src/session_fs_dispatch.rs
diff --git a/rust/tests/e2e/session_fs_sqlite.rs b/rust/tests/e2e/session_fs_sqlite.rs
diff --git a/rust/tests/session_test.rs b/rust/tests/session_test.rs
