fix: support fs.watch on non-recursive platforms and make better-sqlite3 optional#21
fix: support fs.watch on non-recursive platforms and make better-sqlite3 optional#21httpsVishu wants to merge 1 commit intovtemian:mainfrom
Conversation
There was a problem hiding this comment.
3 issues found across 6 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="src/providers/shared/watch.ts">
<violation number="1" location="src/providers/shared/watch.ts:36">
P2: Platform allowlist incorrectly disables recursive fs.watch on Linux runtimes that support it, causing potential missed nested file change events.</violation>
</file>
<file name="src/providers/opencode/provider.ts">
<violation number="1" location="src/providers/opencode/provider.ts:30">
P2: Using top-level CommonJS `require("better-sqlite3")` in an ESM-distributed package can fail in ESM runtime paths and silently disable the OpenCode provider.</violation>
</file>
<file name="tests/opencode-fixtures.ts">
<violation number="1" location="tests/opencode-fixtures.ts:6">
P2: Catching all errors when requiring `better-sqlite3` can hide real runtime/dependency failures by silently skipping sqlite tests.</violation>
</file>
Since this is your first cubic review, here's how it works:
- cubic automatically reviews your code and comments on bugs and improvements
- Teach cubic by replying to its comments. cubic learns from your replies and gets better over time
- Add one-off context when rerunning by tagging
@cubic-dev-aiwith guidance or docs links (includingllms.txt) - Ask questions if you need clarification on any suggestion
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
| } | ||
| onEvent(); | ||
| }); | ||
| const isRecursiveSupported = process.platform === "darwin" || process.platform === "win32"; |
There was a problem hiding this comment.
P2: Platform allowlist incorrectly disables recursive fs.watch on Linux runtimes that support it, causing potential missed nested file change events.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src/providers/shared/watch.ts, line 36:
<comment>Platform allowlist incorrectly disables recursive fs.watch on Linux runtimes that support it, causing potential missed nested file change events.</comment>
<file context>
@@ -33,12 +33,18 @@ export function createProviderWatch(
- }
- onEvent();
- });
+ const isRecursiveSupported = process.platform === "darwin" || process.platform === "win32";
+
+ watcher = fsWatch(
</file context>
| let betterSqlite3: any; | ||
|
|
||
| try { | ||
| betterSqlite3 = require("better-sqlite3"); |
There was a problem hiding this comment.
P2: Using top-level CommonJS require("better-sqlite3") in an ESM-distributed package can fail in ESM runtime paths and silently disable the OpenCode provider.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src/providers/opencode/provider.ts, line 30:
<comment>Using top-level CommonJS `require("better-sqlite3")` in an ESM-distributed package can fail in ESM runtime paths and silently disable the OpenCode provider.</comment>
<file context>
@@ -25,17 +24,25 @@ import { createOpenCodeDatabase, type OpenCodeDatabase, type SessionStats } from
+let betterSqlite3: any;
+
+try {
+ betterSqlite3 = require("better-sqlite3");
+} catch {
+ // Optional dependency not available
</file context>
|
|
||
| try { | ||
| Database = require("better-sqlite3"); | ||
| } catch { |
There was a problem hiding this comment.
P2: Catching all errors when requiring better-sqlite3 can hide real runtime/dependency failures by silently skipping sqlite tests.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At tests/opencode-fixtures.ts, line 6:
<comment>Catching all errors when requiring `better-sqlite3` can hide real runtime/dependency failures by silently skipping sqlite tests.</comment>
<file context>
@@ -1,5 +1,13 @@
+
+try {
+ Database = require("better-sqlite3");
+} catch {
+ // optional dependency not available
+}
</file context>
What
Fixes cross-platform issues with
fs.watchand improves handling of optional dependencies.Why
While setting up and running the project on a Linux/WSL environment,
fs.watchwith recursive support caused runtime errors, andbetter-sqlite3being treated as required also broke tests when not installed. This PR ensures smoother setup and execution across platforms.Results
fs.watchoptionsChecklist
npm run checkpassesSummary by cubic
Fix cross-platform file watching by using non-recursive mode on Linux/WSL, and make
better-sqlite3optional to prevent setup failures when not installed. Tests now skip SQLite-only suites if the module is missing and skip recursive watch checks on unsupported platforms.fs.watchrecursion only ondarwinandwin32; avoids runtime errors on Linux/WSL.better-sqlite3via saferequire; provider/test types updated to accept an injected db.hasSqlite; skip recursive watch test where not supported.Written for commit 93e3ca3. Summary will update on new commits.