Skip to content

Commit 5c38a91

Browse files
committed
Address more PR review comments
1 parent 5186ae9 commit 5c38a91

File tree

5 files changed

+35
-11
lines changed

5 files changed

+35
-11
lines changed

extensions/vscode/src/bridge/database-copier.ts

Lines changed: 17 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,14 @@ export class DatabaseCopier {
3131
* are ready for use (absolute paths).
3232
*/
3333
async syncAll(sourceDirs: string[]): Promise<string[]> {
34-
await mkdir(this.destinationBase, { recursive: true });
34+
try {
35+
await mkdir(this.destinationBase, { recursive: true });
36+
} catch (err) {
37+
this.logger.error(
38+
`Failed to create managed database directory ${this.destinationBase}: ${err instanceof Error ? err.message : String(err)}`,
39+
);
40+
return [];
41+
}
3542

3643
const copied: string[] = [];
3744

@@ -76,8 +83,15 @@ export class DatabaseCopier {
7683
this.logger.info(`Copying database ${src}${dest}`);
7784
try {
7885
// Remove stale destination if present
79-
if (existsSync(dest)) {
80-
await rm(dest, { recursive: true, force: true });
86+
try {
87+
if (existsSync(dest)) {
88+
await rm(dest, { recursive: true, force: true });
89+
}
90+
} catch (rmErr) {
91+
this.logger.error(
92+
`Failed to remove stale destination ${dest}: ${rmErr instanceof Error ? rmErr.message : String(rmErr)}`,
93+
);
94+
return;
8195
}
8296

8397
await cp(src, dest, { recursive: true });

extensions/vscode/src/bridge/environment-builder.ts

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -104,8 +104,15 @@ export class EnvironmentBuilder extends DisposableObject {
104104
if (copyEnabled) {
105105
const managedDir = this.storagePaths.getManagedDatabaseStoragePath();
106106
const copier = this.copierFactory(managedDir, this.logger);
107-
await copier.syncAll(sourceDirs);
108-
dbDirs = [managedDir, ...userDbDirs];
107+
try {
108+
await copier.syncAll(sourceDirs);
109+
dbDirs = [managedDir, ...userDbDirs];
110+
} catch (err) {
111+
this.logger.error(
112+
`Database copy failed, falling back to source dirs: ${err instanceof Error ? err.message : String(err)}`,
113+
);
114+
dbDirs = [...sourceDirs, ...userDbDirs];
115+
}
109116
} else {
110117
dbDirs = [...sourceDirs, ...userDbDirs];
111118
}

extensions/vscode/test/suite/workspace-scenario.integration.test.ts

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -109,8 +109,11 @@ suite('Workspace Scenario Tests', () => {
109109
// With copyDatabases enabled (default), CODEQL_DATABASES_BASE_DIRS points
110110
// to a managed databases/ directory under our globalStorage instead of the
111111
// original workspaceStorage paths. Accept either layout.
112-
const hasManagedDir = dirs.includes('/databases');
113-
const hasWorkspaceStorage = dirs.includes('workspaceStorage');
112+
const dirList = String(dirs)
113+
.split(path.delimiter)
114+
.filter((p: string) => p);
115+
const hasManagedDir = dirList.some((p: string) => path.basename(p) === 'databases');
116+
const hasWorkspaceStorage = dirList.some((p: string) => p.includes('workspaceStorage'));
114117
assert.ok(
115118
hasManagedDir || hasWorkspaceStorage,
116119
`With workspace open, CODEQL_DATABASES_BASE_DIRS should include managed databases dir or workspaceStorage: ${dirs}`,

server/dist/codeql-development-mcp-server.js.map

Lines changed: 1 addition & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

server/src/lib/discovery-config.ts

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,7 @@ function parsePathList(envValue: string | undefined): string[] {
3333
* Each directory is expected to contain one or more CodeQL database directories
3434
* (each with a `codeql-database.yml` file).
3535
*
36-
* Set via `CODEQL_DATABASES_BASE_DIRS` (colon-separated).
36+
* Set via `CODEQL_DATABASES_BASE_DIRS` (platform-delimited).
3737
*/
3838
export function getDatabaseBaseDirs(): string[] {
3939
return parsePathList(process.env.CODEQL_DATABASES_BASE_DIRS);
@@ -47,7 +47,7 @@ export function getDatabaseBaseDirs(): string[] {
4747
* subdirectories with `repo_task.json`, `results/results.sarif`, and
4848
* `results/results.bqrs`.
4949
*
50-
* Set via `CODEQL_MRVA_RUN_RESULTS_DIRS` (colon-separated).
50+
* Set via `CODEQL_MRVA_RUN_RESULTS_DIRS` (platform-delimited).
5151
*/
5252
export function getMrvaRunResultsDirs(): string[] {
5353
return parsePathList(process.env.CODEQL_MRVA_RUN_RESULTS_DIRS);
@@ -60,7 +60,7 @@ export function getMrvaRunResultsDirs(): string[] {
6060
* `<QueryName>.ql-<nanoid>/`, each holding artifacts such as
6161
* `evaluator-log.jsonl`, `results.bqrs`, and `results-interpreted.sarif`.
6262
*
63-
* Set via `CODEQL_QUERY_RUN_RESULTS_DIRS` (colon-separated).
63+
* Set via `CODEQL_QUERY_RUN_RESULTS_DIRS` (platform-delimited).
6464
*/
6565
export function getQueryRunResultsDirs(): string[] {
6666
return parsePathList(process.env.CODEQL_QUERY_RUN_RESULTS_DIRS);

0 commit comments

Comments
 (0)