Skip to content

Commit 05ce059

Browse files
SteveSandersonMSCopilot
SteveSandersonMS
and
Copilot
committed
Fix CI: formatting, Go race, Node sqlite error handling, scenario go.mod
- Fix Python/Rust formatting issues caught by CI linters - Fix Go data race in SqliteQuery by holding mutex for full operation - Add try/catch to Node.js sessionFsProvider sqlite methods for error mapping - Update Go scenario go.mod files from go 1.24 to go 1.24.0 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
1 parent 24027e8 commit 05ce059

36 files changed

Lines changed: 117 additions & 96 deletions

File tree

‎go/internal/e2e/session_fs_sqlite_e2e_test.go‎

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,13 @@ func newInMemorySqliteProvider(sessionID string, calls *[]sqliteCall) *inMemoryS
4343
}
4444

4545
func (p *inMemorySqliteProvider) getOrCreateDB() (*sql.DB, error) {
46+
p.mu.Lock()
47+
defer p.mu.Unlock()
48+
return p.getOrCreateDBLocked()
49+
}
50+
51+
// getOrCreateDBLocked must be called while holding p.mu.
52+
func (p *inMemorySqliteProvider) getOrCreateDBLocked() (*sql.DB, error) {
4653
if p.db == nil {
4754
db, err := sql.Open("sqlite", ":memory:")
4855
if err != nil {
@@ -212,13 +219,14 @@ func (p *inMemorySqliteProvider) Rename(src string, dest string) error {
212219
}
213220

214221
func (p *inMemorySqliteProvider) SqliteQuery(sessionID string, query string, queryType rpc.SessionFsSqliteQueryType, params map[string]any) (*rpc.SessionFsSqliteQueryResult, error) {
222+
p.mu.Lock()
223+
defer p.mu.Unlock()
215224
*p.sqliteCalls = append(*p.sqliteCalls, sqliteCall{
216225
SessionID: sessionID,
217226
QueryType: string(queryType),
218227
Query: query,
219228
})
220-
221-
db, err := p.getOrCreateDB()
229+
db, err := p.getOrCreateDBLocked()
222230
if err != nil {
223231
return nil, err
224232
}
@@ -284,6 +292,8 @@ func (p *inMemorySqliteProvider) SqliteQuery(sessionID string, query string, que
284292
}
285293

286294
func (p *inMemorySqliteProvider) SqliteExists(sessionID string) (bool, error) {
295+
p.mu.Lock()
296+
defer p.mu.Unlock()
287297
return p.db != nil, nil
288298
}
289299

‎nodejs/src/sessionFsProvider.ts‎

Lines changed: 11 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -191,14 +191,22 @@ export function createSessionFsAdapter(provider: SessionFsProvider): SessionFsHa
191191
if (!provider.sqlite) {
192192
throw new Error("SQLite is not supported by this provider");
193193
}
194-
const result = await provider.sqlite.query(queryType, query, bindParams);
195-
return result ?? { rows: [], columns: [], rowsAffected: 0 };
194+
try {
195+
const result = await provider.sqlite.query(queryType, query, bindParams);
196+
return result ?? { rows: [], columns: [], rowsAffected: 0 };
197+
} catch (err) {
198+
return { error: toSessionFsError(err), rows: [], columns: [], rowsAffected: 0 };
199+
}
196200
},
197201
sqliteExists: async () => {
198202
if (!provider.sqlite) {
199203
throw new Error("SQLite is not supported by this provider");
200204
}
201-
return { exists: await provider.sqlite.exists() };
205+
try {
206+
return { exists: await provider.sqlite.exists() };
207+
} catch {
208+
return { exists: false };
209+
}
202210
},
203211
};
204212
}

‎python/e2e/test_session_fs_sqlite_e2e.py‎

Lines changed: 18 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -110,7 +110,7 @@ async def readdir(self, path: str) -> list[str]:
110110
names: set[str] = set()
111111
for p in list(self._files.keys()) + list(self._dirs):
112112
if p.startswith(prefix):
113-
rest = p[len(prefix):]
113+
rest = p[len(prefix) :]
114114
if rest:
115115
names.add(rest.split("/")[0])
116116
return sorted(names)
@@ -120,21 +120,18 @@ async def readdir_with_types(self, path: str) -> list[SessionFSReaddirWithTypesE
120120
entries: dict[str, SessionFSReaddirWithTypesEntryType] = {}
121121
for p in self._dirs:
122122
if p.startswith(prefix):
123-
rest = p[len(prefix):]
123+
rest = p[len(prefix) :]
124124
if rest:
125125
name = rest.split("/")[0]
126126
entries[name] = SessionFSReaddirWithTypesEntryType.DIRECTORY
127127
for p in self._files:
128128
if p.startswith(prefix):
129-
rest = p[len(prefix):]
129+
rest = p[len(prefix) :]
130130
if rest:
131131
name = rest.split("/")[0]
132132
if name not in entries:
133133
entries[name] = SessionFSReaddirWithTypesEntryType.FILE
134-
return [
135-
SessionFSReaddirWithTypesEntry(name=n, type=t)
136-
for n, t in sorted(entries.items())
137-
]
134+
return [SessionFSReaddirWithTypesEntry(name=n, type=t) for n, t in sorted(entries.items())]
138135

139136
async def rm(self, path: str, recursive: bool, force: bool) -> None:
140137
self._files.pop(path, None)
@@ -152,18 +149,18 @@ async def sqlite_query(
152149
query_type: SessionFSSqliteQueryType,
153150
params: dict[str, float | str | None] | None = None,
154151
) -> SessionFSSqliteQueryResult:
155-
self._sqlite_calls.append({
156-
"sessionId": session_id,
157-
"queryType": query_type.value,
158-
"query": query,
159-
})
152+
self._sqlite_calls.append(
153+
{
154+
"sessionId": session_id,
155+
"queryType": query_type.value,
156+
"query": query,
157+
}
158+
)
160159

161160
db = self._get_or_create_db()
162161
trimmed = query.strip()
163162
if not trimmed:
164-
return SessionFSSqliteQueryResult(
165-
columns=[], rows=[], rows_affected=0
166-
)
163+
return SessionFSSqliteQueryResult(columns=[], rows=[], rows_affected=0)
167164

168165
if query_type == SessionFSSqliteQueryType.EXEC:
169166
db.executescript(trimmed)
@@ -174,9 +171,7 @@ async def sqlite_query(
174171
cursor = db.execute(trimmed, params or {})
175172
columns = [desc[0] for desc in cursor.description] if cursor.description else []
176173
rows = [dict(zip(columns, row)) for row in cursor.fetchall()]
177-
return SessionFSSqliteQueryResult(
178-
columns=columns, rows=rows, rows_affected=0
179-
)
174+
return SessionFSSqliteQueryResult(columns=columns, rows=rows, rows_affected=0)
180175

181176
# run (INSERT/UPDATE/DELETE)
182177
cursor = db.execute(trimmed, params or {})
@@ -195,6 +190,7 @@ async def sqlite_exists(self, session_id: str) -> bool:
195190
def _create_sqlite_handler(sqlite_calls: list[dict]):
196191
def factory(session):
197192
return _InMemorySessionFsSqliteProvider(session.session_id, sqlite_calls)
193+
198194
return factory
199195

200196

@@ -284,8 +280,10 @@ def handler_factory(session):
284280
lines = [line for line in content.split("\n") if line.strip()]
285281
parsed = [json.loads(line) for line in lines]
286282
sql_tool_events = [
287-
e for e in parsed
288-
if e.get("type") == "tool.execution_start" and e.get("data", {}).get("toolName") == "sql"
283+
e
284+
for e in parsed
285+
if e.get("type") == "tool.execution_start"
286+
and e.get("data", {}).get("toolName") == "sql"
289287
]
290288
assert len(sql_tool_events) > 0
291289
assert all(e.get("agentId") for e in sql_tool_events)

‎rust/src/types.rs‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -16,8 +16,7 @@ use crate::handler::SessionHandler;
1616
use crate::hooks::SessionHooks;
1717
pub use crate::session_fs::{
1818
DirEntry, DirEntryKind, FileInfo, FsError, SessionFsCapabilities, SessionFsConfig,
19-
SessionFsConventions,
20-
SessionFsProvider, SessionFsSqliteQueryResult, SessionFsSqliteQueryType,
19+
SessionFsConventions, SessionFsProvider, SessionFsSqliteQueryResult, SessionFsSqliteQueryType,
2120
};
2221
pub use crate::trace_context::{TraceContext, TraceContextProvider};
2322
use crate::transforms::SystemMessageTransform;

‎rust/tests/e2e/session_fs_sqlite.rs‎

Lines changed: 44 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -51,8 +51,7 @@ impl InMemorySqliteProvider {
5151

5252
fn get_or_create_db(db: &mut Option<Connection>) -> Result<&mut Connection, FsError> {
5353
if db.is_none() {
54-
let conn =
55-
Connection::open_in_memory().map_err(|e| FsError::Other(e.to_string()))?;
54+
let conn = Connection::open_in_memory().map_err(|e| FsError::Other(e.to_string()))?;
5655
conn.execute_batch("PRAGMA busy_timeout = 5000;")
5756
.map_err(|e| FsError::Other(e.to_string()))?;
5857
*db = Some(conn);
@@ -111,24 +110,13 @@ impl SessionFsProvider for InMemorySqliteProvider {
111110
if dirs.contains(path) {
112111
Ok(FileInfo::new(false, true, 0, now, now))
113112
} else if let Some(content) = files.get(path) {
114-
Ok(FileInfo::new(
115-
true,
116-
false,
117-
content.len() as i64,
118-
now,
119-
now,
120-
))
113+
Ok(FileInfo::new(true, false, content.len() as i64, now, now))
121114
} else {
122115
Err(FsError::NotFound(path.to_string()))
123116
}
124117
}
125118

126-
async fn mkdir(
127-
&self,
128-
path: &str,
129-
recursive: bool,
130-
_mode: Option<i64>,
131-
) -> Result<(), FsError> {
119+
async fn mkdir(&self, path: &str, recursive: bool, _mode: Option<i64>) -> Result<(), FsError> {
132120
let mut dirs = self.dirs.lock().unwrap();
133121
if recursive {
134122
let parts: Vec<&str> = path.trim_end_matches('/').split('/').collect();
@@ -181,7 +169,9 @@ impl SessionFsProvider for InMemorySqliteProvider {
181169
if let Some(rest) = f.strip_prefix(&prefix) {
182170
if !rest.is_empty() {
183171
if let Some(name) = rest.split('/').next() {
184-
entries.entry(name.to_string()).or_insert(DirEntryKind::File);
172+
entries
173+
.entry(name.to_string())
174+
.or_insert(DirEntryKind::File);
185175
}
186176
}
187177
}
@@ -261,21 +251,19 @@ impl SessionFsProvider for InMemorySqliteProvider {
261251
.prepare(trimmed)
262252
.map_err(|e| FsError::Other(e.to_string()))?;
263253
let col_count = stmt.column_count();
264-
let columns: Vec<String> =
265-
(0..col_count).map(|i| stmt.column_name(i).unwrap().to_string()).collect();
254+
let columns: Vec<String> = (0..col_count)
255+
.map(|i| stmt.column_name(i).unwrap().to_string())
256+
.collect();
266257
let mut rows = vec![];
267-
let mut query_rows = stmt
268-
.query([])
269-
.map_err(|e| FsError::Other(e.to_string()))?;
258+
let mut query_rows = stmt.query([]).map_err(|e| FsError::Other(e.to_string()))?;
270259
while let Some(row) = query_rows
271260
.next()
272261
.map_err(|e| FsError::Other(e.to_string()))?
273262
{
274263
let mut map = HashMap::new();
275264
for (i, col) in columns.iter().enumerate() {
276-
let val: rusqlite::types::Value = row
277-
.get(i)
278-
.map_err(|e| FsError::Other(e.to_string()))?;
265+
let val: rusqlite::types::Value =
266+
row.get(i).map_err(|e| FsError::Other(e.to_string()))?;
279267
let json_val = match val {
280268
rusqlite::types::Value::Null => serde_json::Value::Null,
281269
rusqlite::types::Value::Integer(n) => {
@@ -285,9 +273,9 @@ impl SessionFsProvider for InMemorySqliteProvider {
285273
serde_json::Number::from_f64(f).unwrap_or(0.into()),
286274
),
287275
rusqlite::types::Value::Text(s) => serde_json::Value::String(s),
288-
rusqlite::types::Value::Blob(b) => serde_json::Value::String(
289-
String::from_utf8_lossy(&b).into_owned(),
290-
),
276+
rusqlite::types::Value::Blob(b) => {
277+
serde_json::Value::String(String::from_utf8_lossy(&b).into_owned())
278+
}
291279
};
292280
map.insert(col.clone(), json_val);
293281
}
@@ -342,14 +330,21 @@ fn session_state_path_sqlite() -> String {
342330
}
343331

344332
fn sqlite_session_fs_config() -> SessionFsConfig {
345-
SessionFsConfig::new("/", session_state_path_sqlite(), SessionFsConventions::Posix)
346-
.with_capabilities(SessionFsCapabilities::new().with_sqlite(true))
333+
SessionFsConfig::new(
334+
"/",
335+
session_state_path_sqlite(),
336+
SessionFsConventions::Posix,
337+
)
338+
.with_capabilities(SessionFsCapabilities::new().with_sqlite(true))
347339
}
348340

349341
async fn start_sqlite_client(ctx: &super::support::E2eContext) -> Client {
350-
Client::start(ctx.client_options().with_session_fs(sqlite_session_fs_config()))
351-
.await
352-
.expect("start sqlite client")
342+
Client::start(
343+
ctx.client_options()
344+
.with_session_fs(sqlite_session_fs_config()),
345+
)
346+
.await
347+
.expect("start sqlite client")
353348
}
354349

355350
fn sqlite_session_config(
@@ -370,7 +365,10 @@ async fn should_route_sql_queries_through_the_sessionfs_sqlite_handler() {
370365
ctx.set_default_copilot_user();
371366
let session_id = "00000000-0000-4000-8000-000000000201";
372367
let sqlite_calls = Arc::new(Mutex::new(Vec::new()));
373-
let provider = Arc::new(InMemorySqliteProvider::new(session_id, sqlite_calls.clone()));
368+
let provider = Arc::new(InMemorySqliteProvider::new(
369+
session_id,
370+
sqlite_calls.clone(),
371+
));
374372
let client = start_sqlite_client(ctx).await;
375373
let session = client
376374
.create_session(
@@ -395,19 +393,27 @@ async fn should_route_sql_queries_through_the_sessionfs_sqlite_handler() {
395393
);
396394

397395
let calls = sqlite_calls.lock().unwrap();
398-
let session_calls: Vec<&SqliteCall> =
399-
calls.iter().filter(|c| c.session_id == session_id).collect();
396+
let session_calls: Vec<&SqliteCall> = calls
397+
.iter()
398+
.filter(|c| c.session_id == session_id)
399+
.collect();
400400
assert!(!session_calls.is_empty(), "expected sqlite calls");
401401
assert!(
402-
session_calls.iter().any(|c| c.query.to_uppercase().contains("CREATE TABLE")),
402+
session_calls
403+
.iter()
404+
.any(|c| c.query.to_uppercase().contains("CREATE TABLE")),
403405
"expected CREATE TABLE"
404406
);
405407
assert!(
406-
session_calls.iter().any(|c| c.query.to_uppercase().contains("INSERT")),
408+
session_calls
409+
.iter()
410+
.any(|c| c.query.to_uppercase().contains("INSERT")),
407411
"expected INSERT"
408412
);
409413
assert!(
410-
session_calls.iter().any(|c| c.query.to_uppercase().contains("SELECT")),
414+
session_calls
415+
.iter()
416+
.any(|c| c.query.to_uppercase().contains("SELECT")),
411417
"expected SELECT"
412418
);
413419
assert!(

‎test/scenarios/auth/byok-anthropic/go/go.mod‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
module github.com/github/copilot-sdk/samples/auth/byok-anthropic/go
22

3-
go 1.24
3+
go 1.24.0
44

55
require github.com/github/copilot-sdk/go v0.0.0
66

‎test/scenarios/auth/byok-azure/go/go.mod‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
module github.com/github/copilot-sdk/samples/auth/byok-azure/go
22

3-
go 1.24
3+
go 1.24.0
44

55
require github.com/github/copilot-sdk/go v0.0.0
66

‎test/scenarios/auth/byok-ollama/go/go.mod‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
module github.com/github/copilot-sdk/samples/auth/byok-ollama/go
22

3-
go 1.24
3+
go 1.24.0
44

55
require github.com/github/copilot-sdk/go v0.0.0
66

‎test/scenarios/auth/byok-openai/go/go.mod‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
module github.com/github/copilot-sdk/samples/auth/byok-openai/go
22

3-
go 1.24
3+
go 1.24.0
44

55
require github.com/github/copilot-sdk/go v0.0.0
66

‎test/scenarios/auth/gh-app/go/go.mod‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
module github.com/github/copilot-sdk/samples/auth/gh-app/go
22

3-
go 1.24
3+
go 1.24.0
44

55
require github.com/github/copilot-sdk/go v0.0.0
66

0 commit comments

Comments
 (0)