Rename repo to sqlite-web and add workflows #2
Conversation
WalkthroughAdds three Nix-based GitHub Actions workflows (npm-release, test-ui, test-wasm); renames workspace members, crates, docs, configs, and generated assets from sqlite-worker* → sqlite-web*; adds flake task build-submodules; and rewrites the local bundling script and wasm glue to build and embed sqlite-web artifacts. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant Dev as Developer
participant GH as GitHub Actions
participant Nix as Nix Environment
participant Registry as npm Registry
participant GHRel as GitHub Releases
Dev->>GH: Push to repo (main or branch)
GH->>GH: Select workflows (test-ui, test-wasm, npm-release on main)
GH->>Nix: Install Nix, restore cache, setup Node.js
GH->>Nix: Run build & tests via `nix develop`
Nix-->>GH: Produce package tarball and NEW_HASH
GH->>Registry: npm view dist.shasum -> OLD_HASH (or none)
alt OLD_HASH != NEW_HASH
GH->>GH: Compute NEW_VERSION (prerelease) and update pkg metadata
GH->>GH: Commit pkg change, tag npm-v<NEW_VERSION>, push
GH->>Nix: npm pack -> tarball, rename artifact
GH->>Registry: npm publish tarball (public) using NPM_TOKEN
GH->>GHRel: Create GitHub Release and attach tarball (GITHUB_TOKEN)
GH-->>Dev: Output NEW_VERSION
else
GH-->>Dev: Skip release (hash unchanged)
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes ✨ Finishing Touches
🧪 Generate unit tests
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 37
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (24)
Cargo.toml (1)
12-12: Update homepage URL to reflect repo rename.The homepage still points to rainlanguage/sqlite-wasm-rs-test. If the repository is now rainlanguage/sqlite-web, update this for accuracy and tooling that reads Cargo metadata.
-homepage = "https://github.com/rainlanguage/sqlite-wasm-rs-test" +homepage = "https://github.com/rainlanguage/sqlite-web"svelte-test/tests/fixtures/test-helpers.ts (3)
18-21: Avoid fixed sleep for worker readiness; poll the DB until it responds.A hardcoded 1s delay is flaky under CI load. Poll a trivial query until success or timeout.
- // Wait for worker to be ready - await new Promise(resolve => setTimeout(resolve, 1000)); + // Wait for worker/DB to be ready by polling a trivial query + await waitForDbReady(db);Add this helper in the same file (outside the function):
+async function waitForDbReady(db: SQLiteWasmDatabase, timeoutMs = 5000, intervalMs = 50): Promise<void> { + const start = performance.now(); + // eslint-disable-next-line no-constant-condition + while (true) { + try { + await db.query('SELECT 1;'); + return; + } catch { + if (performance.now() - start > timeoutMs) { + throw new Error(`SQLite worker did not become ready within ${timeoutMs}ms`); + } + await new Promise(r => setTimeout(r, intervalMs)); + } + } +}
29-33: Naive SQL splitting by semicolons will break on statements containing strings or triggers.Tests will intermittently fail when a semicolon appears inside string literals. Use a minimal tokenizer to ignore semicolons inside quotes.
- const statements = sqlContent - .split(';') - .map(stmt => stmt.trim()) - .filter(stmt => stmt.length > 0); + const statements = splitSqlStatements(sqlContent);Add this helper (simple quote-aware splitter):
+function splitSqlStatements(sql: string): string[] { + const out: string[] = []; + let buf = ''; + let inSingle = false, inDouble = false; + for (let i = 0; i < sql.length; i++) { + const ch = sql[i]; + const prev = sql[i - 1]; + if (ch === "'" && prev !== '\\' && !inDouble) inSingle = !inSingle; + if (ch === '"' && prev !== '\\' && !inSingle) inDouble = !inDouble; + if (ch === ';' && !inSingle && !inDouble) { + const s = buf.trim(); + if (s) out.push(s); + buf = ''; + } else { + buf += ch; + } + } + const tail = buf.trim(); + if (tail) out.push(tail); + return out; +}
11-17: Nit: Avoid non-null assertion when value is proved by control flow.Following the error check, value should be present; prefer explicit narrowing for clarity.
- const db = result.value! + const db = result.value as SQLiteWasmDatabasesvelte-test/src/routes/sql/+page.svelte (3)
95-96: Svelte event binding uses incorrect attribute; replaceonclickwithon:click.Svelte requires the
on:eventdirective;onclick={...}won’t attach handlers reliably.-<button class="sample-btn" onclick={insertSampleData}>Insert Sample Data</button> +<button class="sample-btn" on:click={insertSampleData}>Insert Sample Data</button> -<button class="clear-btn" onclick={clearQuery}>Clear</button> +<button class="clear-btn" on:click={clearQuery}>Clear</button> -<button +<button class="execute-btn" - onclick={executeQuery} + on:click={executeQuery} disabled={isExecuting || !sqlQuery.trim()} > {isExecuting ? 'Executing...' : 'Execute Query'} </button> -<button onclick={() => sqlQuery = 'SELECT * FROM users ORDER BY created_at DESC;'}> +<button on:click={() => sqlQuery = 'SELECT * FROM users ORDER BY created_at DESC;'}> Select All Users </button> -<button onclick={() => sqlQuery = 'SELECT COUNT(*) as total_users FROM users;'}> +<button on:click={() => sqlQuery = 'SELECT COUNT(*) as total_users FROM users;'}> Count Users </button> -<button onclick={() => sqlQuery = 'SELECT name, email FROM users WHERE email LIKE \'%@example.com\';'}> +<button on:click={() => sqlQuery = 'SELECT name, email FROM users WHERE email LIKE \'%@example.com\';'}> Filter by Email Domain </button> -<button onclick={() => sqlQuery = 'DROP TABLE IF EXISTS test_table;\nCREATE TABLE test_table (id INTEGER, data TEXT);'}> +<button on:click={() => sqlQuery = 'DROP TABLE IF EXISTS test_table;\nCREATE TABLE test_table (id INTEGER, data TEXT);'}> Create Test Table </button> -<button onclick={() => window.location.reload()}>Reload Page</button> +<button on:click={() => window.location.reload()}>Reload Page</button>Also applies to: 107-113, 167-176, 186-186
29-31: Replace fixed sleep with a readiness probe to avoid flaky startup.Same rationale as tests: poll a trivial query until success or timeout.
- status = 'Waiting for worker to be ready...'; - await new Promise(resolve => setTimeout(resolve, 1000)); + status = 'Waiting for worker to be ready...'; + await waitForDbReady(db);Add helper (same file, e.g., after script’s functions):
+async function waitForDbReady(db: SQLiteWasmDatabase, timeoutMs = 5000, intervalMs = 50) { + const start = performance.now(); + while (true) { + try { await db.query('SELECT 1;'); return; } + catch { if (performance.now() - start > timeoutMs) throw new Error(`Worker not ready in ${timeoutMs}ms`); } + await new Promise(r => setTimeout(r, intervalMs)); + } +}
17-17: Minor copy: consider updating status text to match rename.“Loading SQLite Worker...” could be “Loading sqlite-web...” to avoid confusion during the rename period.
- status = 'Loading SQLite Worker...'; + status = 'Loading sqlite-web...';svelte-test/tests/integration/opfs-persistence.test.ts (2)
194-195: Replace fixed sleep with readiness polling to avoid flakiness.Waiting 200ms after creating a table in another instance is racy on OPFS. Prefer polling until sqlite_master reflects the table in db2.
Apply this diff:
- // Wait for table to be available - await new Promise(resolve => setTimeout(resolve, 200)); + // Wait for table to be visible to the second connection + await waitFor(async () => { + const r = await db2.query(` + SELECT name FROM sqlite_master WHERE type='table' AND name='concurrent_test' + `); + const rows = JSON.parse(r.value || '[]'); + return rows.length === 1; + }, 3000);
275-301: Make the storage-limit test assertions explicit.Right now we warn and break if a limit is hit. Consider asserting that either all inserts succeed or that a specific size triggers a failure to avoid silent passes.
Example pattern:
- } catch (error) { - // If we hit storage limits, that's acceptable - console.warn(`Storage limit reached at size ${size}:`, error); - break; - } + } catch (error) { + expect(error).toBeDefined(); + // Stop further inserts—limit hit is an expected outcome. + break; + }svelte-test/package.json (1)
10-12: Align @typescript-eslint versions to avoid toolchain conflicts.You have parser/plugin at 7.18.0 and the metapackage at 8.14.0. These major versions should match.
Apply one of:
- Upgrade parser/plugin to 8.14.0 to match typescript-eslint 8.14.0, or
- Remove the standalone parser/plugin and just keep the metapackage at 8.x, or
- Downgrade typescript-eslint to 7.18.0.
Happy to propose exact diffs once you pick a direction.
Also applies to: 22-22
svelte-test/src/routes/+page.svelte (4)
31-31: Avoid fixed 1s sleep; wait on a readiness signal instead.A static delay can cause flaky init on slower machines. Prefer an explicit readiness or a quick “ping” query loop.
Example:
- // Wait for worker to be ready - await new Promise(resolve => setTimeout(resolve, 1000)); + // Wait for worker/DB readiness with a fast query loop + for (let i = 0; i < 20; i++) { + try { + await db.query('SELECT 1'); + break; + } catch { + await new Promise(r => setTimeout(r, 100)); + } + }
91-96: Use placeholders for DELETE statements too.IDs are numbers, but parameterization prevents accidental injection and improves plan caching.
- await db.query(`DELETE FROM users WHERE id = ${id}`); + await db.query("DELETE FROM users WHERE id = ?1", [id]);- await db.query('DELETE FROM users'); + await db.query('DELETE FROM users'); // OK as-is; no user inputAlso applies to: 105-111
140-145: Use Svelte’s on:click instead of onclick for idiomatic event binding.Keeps to Svelte conventions and avoids assigning DOM properties.
- <button - onclick={addUser} + <button + on:click={addUser} disabled={isLoading || !newUserName.trim() || !newUserEmail.trim()} >- <button class="refresh-btn" onclick={loadUsers} disabled={isLoading}> + <button class="refresh-btn" on:click={loadUsers} disabled={isLoading}>- <button class="clear-btn" onclick={clearAll} disabled={isLoading}> + <button class="clear-btn" on:click={clearAll} disabled={isLoading}>- onclick={() => deleteUser(user.id)} + on:click={() => deleteUser(user.id)}- <button onclick={() => window.location.reload()}>Reload Page</button> + <button on:click={() => window.location.reload()}>Reload Page</button>Also applies to: 152-154, 156-159, 176-181, 195-196
120-121: Update heading to reflect rename (sqlite-web vs worker).Minor UX consistency nit.
- <h1>SQLite Worker Demo</h1> + <h1>sqlite-web Demo</h1>svelte-test/README.md (2)
71-81: API naming in docs likely outdated; align with actual export.Your Svelte page uses
SQLiteWasmDatabase.new()while README showsnew DatabaseConnection(). For consistency, update the sample to match the code.- import init, { DatabaseConnection } from 'sqlite-web'; + import init, { SQLiteWasmDatabase } from 'sqlite-web'; // Initialize WASM module await init(); - // Create database connection - const db = new DatabaseConnection(); + // Create database connection + const res = SQLiteWasmDatabase.new(); + if (res.error) throw new Error(String(res.error)); + const db = res.value; // Execute queries - const result = await db.query("SELECT * FROM users"); + const result = await db.query("SELECT * FROM users");
1-1: Rename “SQLite Worker” to “sqlite-web” across the README.The project has been renamed; reflect this in the title, features, and usage section.
-# SQLite Worker - Svelte Integration Example +# sqlite-web - Svelte Integration Example ... -- **SQLite Worker Integration** - Uses the parent project's WASM SQLite implementation +- **sqlite-web Integration** - Uses the parent project's WASM SQLite implementation ... -## SQLite Worker Usage +## sqlite-web UsageAlso applies to: 7-11, 66-68
CLAUDE.md (1)
34-45: Add blank lines around subheadings in Core Components.-#### 1. `packages/sqlite-web-core/` +#### 1. `packages/sqlite-web-core/` + ... -#### 2. `packages/sqlite-web/` +#### 2. `packages/sqlite-web/` +svelte-test/vitest.config.js (1)
15-21: Set content-type via extension map to cover .map and other assets.- if (fs.existsSync(fullPath)) { - if (fullPath.endsWith('.wasm')) { - res.setHeader('Content-Type', 'application/wasm'); - } else if (fullPath.endsWith('.js')) { - res.setHeader('Content-Type', 'application/javascript'); - } + if (fs.existsSync(fullPath) && fs.statSync(fullPath).isFile()) { + if (fullPath.endsWith('.wasm')) res.setHeader('Content-Type', 'application/wasm'); + else if (fullPath.endsWith('.js')) res.setHeader('Content-Type', 'application/javascript'); + else if (fullPath.endsWith('.map')) res.setHeader('Content-Type', 'application/json');svelte-test/vite.config.ts (2)
10-26: Harden middleware against path traversal and tighten content serving.Current logic joins req.url directly, allowing "../../" to escape node_modules/sqlite-web and read arbitrary files. Also consider stripping querystrings before path resolution.
Apply this diff to sanitize and confine paths:
{ - name: 'sqlite-web-serve', + name: 'sqlite-web-serve', configureServer(server) { - server.middlewares.use('/pkg', (req, res, next) => { - const filePath = req.url?.substring(1); // remove leading slash - const fullPath = path.join(process.cwd(), 'node_modules/sqlite-web', filePath || ''); + const baseDir = path.resolve(process.cwd(), 'node_modules/sqlite-web'); + server.middlewares.use('/pkg', (req, res, next) => { + // drop query/hash, normalize, and resolve within baseDir + const urlPath = (req.url || '/').split('?')[0].split('#')[0]; + const fullPath = path.resolve(baseDir, '.' + urlPath); + + // forbid path escape + if (!fullPath.startsWith(baseDir + path.sep)) { + res.statusCode = 403; + res.end('Forbidden'); + return; + } if (fs.existsSync(fullPath)) { if (fullPath.endsWith('.wasm')) { res.setHeader('Content-Type', 'application/wasm'); } else if (fullPath.endsWith('.js')) { res.setHeader('Content-Type', 'application/javascript'); } + // dev: avoid caching stale artifacts + res.setHeader('Cache-Control', 'no-store'); fs.createReadStream(fullPath).pipe(res); } else { next(); } }); } }
31-34: Restrict Vite fs.allow to only what’s needed.allow: ['..'] is overly permissive. Limit to the sqlite-web package assets and local pkg dir.
Apply this diff:
server: { fs: { - allow: ['..'] + allow: [ + path.resolve(process.cwd(), 'node_modules/sqlite-web'), + path.resolve(process.cwd(), 'pkg') + ] } },scripts/local-bundle.sh (4)
2-2: Use strict shell options and error trap for safer execution.Strengthen the script to fail fast on unset vars and failed pipelines and to report where it failed.
Apply this diff:
-set -e +set -Eeuo pipefail +trap 'echo "❌ Error on line $LINENO"; exit 1' ERR
31-59: Fetch interceptor: broaden match and avoid accidental overrides.Use stricter equality for the basename and preserve Request objects. Also handle absolute URLs ending with the WASM filename.
Apply this diff:
self.fetch = (function(originalFetch) { return function(resource, init) { try { - const resourceStr = typeof resource === 'string' ? resource : resource.toString(); - if (resourceStr.includes('sqlite_web_core_bg.wasm') || resourceStr === './sqlite_web_core_bg.wasm') { + const resourceStr = typeof resource === 'string' + ? resource + : (resource && typeof resource.url === 'string' ? resource.url : String(resource)); + const isCoreWasm = + resourceStr === './sqlite_web_core_bg.wasm' || + resourceStr.endsWith('/sqlite_web_core_bg.wasm') || + resourceStr.endsWith('sqlite_web_core_bg.wasm'); + if (isCoreWasm) { const bytes = self.__b64ToU8(self.__WASM_B64_MAP['sqlite_web_core_bg.wasm']); return Promise.resolve(new Response(bytes, { headers: { 'Content-Type': 'application/wasm' } })); } } catch (e) { console.warn('Fetch interceptor error:', e); } return originalFetch.call(this, resource, init); }; })(self.fetch || (() => Promise.reject(new Error('fetch not available'))));
4-4: Minor wording nit: update banner to match rename.“Building SQLite Worker…” could be “Building sqlite-web…” to stay consistent with the new naming.
Apply this diff:
-echo "🔨 Building SQLite Worker with workspace architecture..." +echo "🔨 Building sqlite-web with workspace architecture..."
70-76: Worker init: consider explicit ready/error event types and a timeout.Looks good; optionally add a startup timeout to surface hangs.
Example (outside diff scope):
const startupTimeout = setTimeout(() => { self.postMessage({ type: 'worker-error', error: 'startup-timeout' }); }, 10000); // On success: clearTimeout(startupTimeout); self.postMessage({ type: 'worker-ready' });
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
⛔ Files ignored due to path filters (3)
Cargo.lockis excluded by!**/*.locksvelte-test/bun.lockis excluded by!**/*.locksvelte-test/package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (22)
.github/workflows/npm-release.yaml(1 hunks).github/workflows/test-ui.yaml(1 hunks).github/workflows/test-wasm.yaml(1 hunks).gitignore(1 hunks).vscode/settings.json(1 hunks)CLAUDE.md(4 hunks)Cargo.toml(1 hunks)flake.nix(3 hunks)packages/sqlite-web-core/Cargo.toml(1 hunks)packages/sqlite-web/Cargo.toml(1 hunks)scripts/local-bundle.sh(5 hunks)svelte-test/README.md(2 hunks)svelte-test/package.json(1 hunks)svelte-test/src/routes/+page.svelte(1 hunks)svelte-test/src/routes/sql/+page.svelte(1 hunks)svelte-test/tests/fixtures/test-helpers.ts(1 hunks)svelte-test/tests/integration/database-basic.test.ts(1 hunks)svelte-test/tests/integration/error-handling.test.ts(1 hunks)svelte-test/tests/integration/opfs-persistence.test.ts(1 hunks)svelte-test/tests/integration/worker-communication.test.ts(1 hunks)svelte-test/vite.config.ts(2 hunks)svelte-test/vitest.config.js(2 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
svelte-test/**/*.{js,ts,svelte}
📄 CodeRabbit inference engine (CLAUDE.md)
Use an async/Promise-based API when interacting with the SQLite worker (await/Promises for query results)
Files:
svelte-test/src/routes/+page.sveltesvelte-test/vitest.config.jssvelte-test/tests/fixtures/test-helpers.tssvelte-test/tests/integration/error-handling.test.tssvelte-test/tests/integration/worker-communication.test.tssvelte-test/src/routes/sql/+page.sveltesvelte-test/tests/integration/database-basic.test.tssvelte-test/tests/integration/opfs-persistence.test.tssvelte-test/vite.config.ts
🪛 LanguageTool
CLAUDE.md
[grammar] ~9-~9: Use correct spacing
Context: ...hat: 1. Builds sqlite-web-core with wasm-pack build --target web 2. Embeds WASM into JavaScript worker templ...
(QB_NEW_EN_OTHER_ERROR_IDS_5)
[grammar] ~10-~10: There might be a mistake here.
Context: ...eds WASM into JavaScript worker template 3. Builds sqlite-web package with embedde...
(QB_NEW_EN)
[grammar] ~11-~11: There might be a mistake here.
Context: ... sqlite-web package with embedded core 4. Packages with npm pack and updates Sve...
(QB_NEW_EN)
[grammar] ~12-~12: Use correct spacing
Context: ...ackand updates Svelte test integration ### Individual Package Builds -cd packages...
(QB_NEW_EN_OTHER_ERROR_IDS_5)
[grammar] ~14-~14: Use correct spacing
Context: ...tegration ### Individual Package Builds - cd packages/sqlite-web-core && wasm-pack build --target web --out-dir ../../pkg - cd packages/sqlite-web && wasm-pack build --target web --out-dir ../../pkg ### Testing - ./test.sh - Run all Rust WAS...
(QB_NEW_EN_OTHER_ERROR_IDS_5)
[grammar] ~18-~18: Use correct spacing
Context: ...et web --out-dir ../../pkg ### Testing -./test.sh` - Run all Rust WASM tests (both packages...
(QB_NEW_EN_OTHER_ERROR_IDS_5)
[grammar] ~19-~19: Use hyphens correctly
Context: ... Run all Rust WASM tests (both packages) - cd packages/sqlite-web-core && wasm-pack test --headless --chrome - Test core package only - `cd packages/...
(QB_NEW_EN_OTHER_ERROR_IDS_29)
[grammar] ~20-~20: Use hyphens correctly
Context: ...dless --chrome- Test core package only -cd packages/sqlite-web && wasm-pack test --headless --chrome` - Test worker package only ### Svelte T...
(QB_NEW_EN_OTHER_ERROR_IDS_29)
[grammar] ~21-~21: There might be a mistake here.
Context: ...ess --chrome- Test worker package only ### Svelte Test App -cd svelte-test && bun...
(QB_NEW_EN_OTHER)
[grammar] ~34-~34: Use correct spacing
Context: ...lication. ### Core Components #### 1. packages/sqlite-web-core/ - Purpose: Core SQLite functionality and...
(QB_NEW_EN_OTHER_ERROR_IDS_5)
[grammar] ~45-~45: Use correct spacing
Context: ... WASM module with JS glue code #### 2. packages/sqlite-web/ - Purpose: Public API that creates self-...
(QB_NEW_EN_OTHER_ERROR_IDS_5)
[grammar] ~62-~62: Use correct spacing
Context: ... sqlite-web package from local tarball ### Build Process Flow 1. Core Build: `...
(QB_NEW_EN_OTHER_ERROR_IDS_5)
[grammar] ~64-~64: Use correct spacing
Context: ...om local tarball ### Build Process Flow 1. Core Build: sqlite-web-core compiled...
(QB_NEW_EN_OTHER_ERROR_IDS_5)
[grammar] ~66-~66: There might be a mistake here.
Context: ...ite-web-core` compiled to WASM + JS glue 2. Embedding: WASM converted to base64 an...
(QB_NEW_EN)
[grammar] ~67-~67: There might be a mistake here.
Context: ...64 and embedded into JavaScript template 3. Wrapper Build: sqlite-web compiled w...
(QB_NEW_EN)
[grammar] ~68-~68: There might be a mistake here.
Context: ... compiled with embedded worker generator 4. Packaging: NPM package created and int...
(QB_NEW_EN)
svelte-test/README.md
[grammar] ~85-~85: Use correct spacing
Context: ...sqlite-web* - Local WASM package from ../pkg/sqlite-web-0.1.0.tgz - @sveltejs/kit - SvelteKit framework - ...
(QB_NEW_EN_OTHER_ERROR_IDS_5)
🪛 markdownlint-cli2 (0.17.2)
CLAUDE.md
14-14: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
15-15: Lists should be surrounded by blank lines
(MD032, blanks-around-lists)
18-18: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
19-19: Lists should be surrounded by blank lines
(MD032, blanks-around-lists)
34-34: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
45-45: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
🪛 actionlint (1.7.7)
.github/workflows/test-wasm.yaml
17-17: the runner of "actions/checkout@v2" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
.github/workflows/npm-release.yaml
52-52: shellcheck reported issue in this script: SC2086:info:2:30: Double quote to prevent globbing and word splitting
(shellcheck)
59-59: shellcheck reported issue in this script: SC2086:info:3:30: Double quote to prevent globbing and word splitting
(shellcheck)
71-71: shellcheck reported issue in this script: SC2086:info:3:36: Double quote to prevent globbing and word splitting
(shellcheck)
96-96: shellcheck reported issue in this script: SC2086:info:2:44: Double quote to prevent globbing and word splitting
(shellcheck)
.github/workflows/test-ui.yaml
17-17: the runner of "actions/checkout@v2" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
🪛 YAMLlint (1.37.1)
.github/workflows/test-wasm.yaml
[warning] 2-2: truthy value should be one of [false, true]
(truthy)
[error] 43-43: no new line character at the end of file
(new-line-at-end-of-file)
.github/workflows/npm-release.yaml
[warning] 2-2: truthy value should be one of [false, true]
(truthy)
[error] 124-124: no new line character at the end of file
(new-line-at-end-of-file)
.github/workflows/test-ui.yaml
[warning] 2-2: truthy value should be one of [false, true]
(truthy)
[error] 44-44: no new line character at the end of file
(new-line-at-end-of-file)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: test
- GitHub Check: test
🔇 Additional comments (27)
packages/sqlite-web-core/Cargo.toml (1)
2-2: Confirmed rename and updated referencesAll searches across manifests, Rust sources, and scripts reveal no lingering
sqlite-worker-corementions. Workspace members include onlypackages/sqlite-web-core, and scripts/local-bundle.sh (as well as Svelte test configs) referencesqlite_web_core_bg.wasmandsqlite_web_core.js—the expected new stems. No further changes are required.svelte-test/tests/integration/worker-communication.test.ts (1)
9-9: Confirm SQLiteWasmDatabase export in sqlite-web
I’ve verified that:
- No stale
sqlite-workerimports remain in thesvelte-testdirectorysvelte-test/package.jsonnow depends on"sqlite-web"and no longer references"sqlite-worker"However, I wasn’t able to inspect the packaged types directly. Please manually confirm that the
SQLiteWasmDatabasesymbol is exported from thesqlite-webpackage’s entrypoint (e.g.index.d.ts), so that type-only imports continue to resolve correctly.svelte-test/tests/integration/error-handling.test.ts (1)
9-9: All SQLiteWasmDatabase imports correctly point to ‘sqlite-web’Verified no remaining imports from
sqlite-worker, and every occurrence ofSQLiteWasmDatabasenow imports from'sqlite-web'across both routes and all integration tests/fixtures. Changes are consistent with the migration and incur no bundling impact due to type‐only imports. Approving.svelte-test/tests/integration/database-basic.test.ts (1)
10-10: All references to sqlite-worker removed; type-only import updated to sqlite-web
- Verified no
import … from 'sqlite-worker'remains in thesvelte-testsuite.- Confirmed
package.jsonlists only"sqlite-web"(v0.1.0) and nosqlite-workerentries.- Tests already leverage
async/awaitfor DB operations, satisfying the svelte-test guidelines.Cargo.toml (2)
3-4: Workspace rename to sqlite-web-{core,} looks correct.Members point to the new package paths and align with the broader rename. No functional issues spotted here.
3-4: No stale “sqlite-worker” references found
I ran a repo-wide scan for “sqlite-worker” (excluding build artifacts) and checked for any old package paths underpackages/. There were zero hits, so all references have been successfully removed. No further action needed.svelte-test/tests/fixtures/test-helpers.ts (2)
1-1: Import path rename is consistent with the repo-wide change.No issues with the switch to 'sqlite-web'.
178-195: Dropping tables via template strings is fine for fixed test tables; keep inputs non-user controlled.Given the fixed allowlist, risk is low. If this ever takes dynamic input, quote identifiers or build a safe allowlist.
packages/sqlite-web/Cargo.toml (1)
2-2: Crate rename to sqlite-web is correct; library type is appropriate for WASM.cdylib + wasm-bindgen workspace dependencies align with the build.
.vscode/settings.json (1)
8-9: Updated rust-analyzer linkedProjects match the new crate paths.This will keep IDE features working with the renamed packages.
svelte-test/src/routes/sql/+page.svelte (1)
4-4: Import path rename is consistent; no API changes flagged.Matches the workspace and package rename to sqlite-web.
svelte-test/tests/integration/opfs-persistence.test.ts (2)
9-9: Import rename to sqlite-web looks correct.Type-only import aligns with the repo rename. No behavior change and satisfies the svelte-test guideline to keep async/Promise-based DB interactions.
38-43: Confirm parameterized query support or extend the helperI ran a search through our integration tests and didn’t find any existing calls to
db.querywith a secondparamsargument—everything is string‐interpolated. That suggests ourqueryhelper likely only accepts a single SQL‐string argument.Please verify whether
query(sql, params?)is supported in the underlying helper.
• If it is supported, we should switch these tests over to use placeholders instead of string interpolation.
• If it is not, consider extending the helper to accept a secondparamsarray (and pass those through to SQLite) so we can avoid brittle interpolation in tests going forward.svelte-test/package.json (1)
43-43: Verify build output path or switch to directory/workspace linkingI ran a search and did not find any
pkgdirectory orsqlite-web-*.tgzfile in the repository. This suggests that either the bundle step hasn’t been executed yet, or the output path differs from../pkg. Please verify and update accordingly:
- Confirm where your build step (e.g.,
wasm-pack build) emits thesqlite-web-0.1.0.tgztarball.- If you’re outputting into a folder (e.g.,
pkg/sqlite-web), you can simplify the dependency:- "sqlite-web": "file:../pkg/sqlite-web-0.1.0.tgz" + "sqlite-web": "file:../pkg/sqlite-web"- If you’re using npm workspaces, you can avoid paths altogether:
"sqlite-web": "workspace:*"Please confirm that the tarball or directory exists at the specified path, or adjust to the correct location/workspace reference.
svelte-test/src/routes/+page.svelte (2)
4-4: Import rename to sqlite-web is correct.Matches the project rename and keeps the async usage pattern intact.
72-76: Verify parameter binding support in sqlite-webI wasn’t able to find documentation or type definitions for
SQLiteWasmDatabase.queryaccepting a parameters array in thesqlite-webpackage. Please confirm whether the API supports a call signature like:await db.query( "INSERT OR IGNORE INTO users (name, email) VALUES (?1, ?2)", [newUserName.trim(), newUserEmail.trim()] );If it does, switching to this placeholder-based form will both prevent SQL injection and handle embedded quotes correctly. If not, consider adding a small escaping helper, for example:
function escapeSql(str: string) { return str.replace(/'/g, "''"); } await db.query(` INSERT OR IGNORE INTO users (name, email) VALUES ('${escapeSql(newUserName.trim())}', '${escapeSql(newUserEmail.trim())}') `);Either approach will guard against injection and brittle edge cases.
svelte-test/README.md (1)
85-85: Fix bullet list spacing/format in Dependencies.Minor formatting issue flagged by grammar tooling; ensure each item is a separate bullet and spacing is consistent.
- - **sqlite-web** - Local WASM package from `../pkg/sqlite-web-0.1.0.tgz` + - **sqlite-web** — Local WASM package from `../pkg/sqlite-web-0.1.0.tgz`Likely an incorrect or invalid review comment.
flake.nix (2)
18-24: Headless Chrome dependency for wasm tests.
wasm-pack test --headless --chromerequires a Chromium binary in PATH. If Rainix’s devShell doesn’t provide it, addpkgs.chromium(orpkgs.google-chrome) to the dev shell or switch to--firefoxif that’s available.Suggested tweak (only if needed):
- packages = [ packages.test-wasm packages.build-wasm packages.local-bundle packages.test-ui packages.test-full-integration pkgs.wasm-pack ]; + packages = [ packages.test-wasm packages.build-wasm packages.local-bundle packages.test-ui packages.test-full-integration pkgs.wasm-pack pkgs.chromium ];
71-71: Nice: adding pkgs.wasm-pack to the dev shell.This eliminates local tool drift when running tasks.
CLAUDE.md (1)
20-21: Hyphenate “headless” flags consistently and add list spacing in “Testing.”-### Testing -- `./test.sh` - Run all Rust WASM tests (both packages) -- `cd packages/sqlite-web-core && wasm-pack test --headless --chrome` - Test core package only -- `cd packages/sqlite-web && wasm-pack test --headless --chrome` - Test worker package only +### Testing + +- `./test.sh` — Run all Rust WASM tests (both packages). +- `cd packages/sqlite-web-core && wasm-pack test --headless --chrome` — Test core package only. +- `cd packages/sqlite-web && wasm-pack test --headless --chrome` — Test worker package only. +Likely an incorrect or invalid review comment.
svelte-test/vitest.config.js (2)
8-9: LGTM: Plugin rename aligns with package rename.
79-80: LGTM: Excluding sqlite-web from optimizeDeps is correct for WASM workers.svelte-test/vite.config.ts (1)
36-37: Renaming and dependency optimization exclusions look good.Excluding sqlite-web from optimizeDeps is appropriate to prevent pre-bundling of the worker package during dev.
scripts/local-bundle.sh (4)
27-27: BSD/GNU base64 compatibility looks good.Using tr -d '\n' ensures single-line output across platforms. No change required.
118-121: Confirm npm is acting on the right package.Running npm pack inside pkg relies on the last wasm-pack build having written a package.json for sqlite-web there; that’s fine given Step 2 rewrites pkg. No change required.
133-135: Messaging is clear; reflects the new split.Nice clarity on core vs public API after the rename.
31-31: Atob/Buffer fallback is adequate.Worker has atob; Buffer fallback is harmless if reused in Node-like environments.
| - name: Commit And Tag | ||
| if: ${{ env.OLD_HASH != env.NEW_HASH }} | ||
| run: | | ||
| git add "pkg/package.json" | ||
| git commit -m "NPM Package Release v${{ env.NEW_VERSION }}" | ||
| git tag npm-v${{ env.NEW_VERSION }} | ||
There was a problem hiding this comment.
this is fine as we use this workflow in other repos
There was a problem hiding this comment.
@findolor, understood! Thanks for the clarification - I'll note that this workflow pattern works well for your organization's established practices.
✏️ Learnings added
Learnt from: findolor
PR: rainlanguage/sqlite-web#2
File: .github/workflows/npm-release.yaml:79-85
Timestamp: 2025-08-21T12:24:24.582Z
Learning: The rainlanguage organization uses workflows that commit pkg/package.json as part of their NPM release process across multiple repositories, and this pattern works fine for their setup.
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
| - name: Commit And Tag | ||
| if: ${{ env.OLD_HASH != env.NEW_HASH }} | ||
| run: | | ||
| git add "pkg/package.json" | ||
| git commit -m "NPM Package Release v${{ env.NEW_VERSION }}" | ||
| git tag npm-v${{ env.NEW_VERSION }} | ||
There was a problem hiding this comment.
this is fine as we use this workflow in other repos
There was a problem hiding this comment.
Actionable comments posted: 6
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
Cargo.toml (1)
15-15: Update homepage to reflect repo rename.Points to sqlite-wasm-rs-test. Recommend updating to sqlite-web repository URL.
-homepage = "https://github.com/rainlanguage/sqlite-wasm-rs-test" +homepage = "https://github.com/rainlanguage/sqlite-web"
♻️ Duplicate comments (20)
.github/workflows/test-wasm.yaml (6)
2-2: Quote “on” and expand event mapping to satisfy YAML 1.1 linters.-on: [push] +"on": + push:
10-12: Drop unnecessary id-token permission (least privilege).test: permissions: - id-token: write contents: read
14-16: Remove or use unused env COMMIT_SHA.- env: - COMMIT_SHA: ${{ github.sha }} + # env (unused) + # COMMIT_SHA: ${{ github.sha }}
17-17: Update checkout to v4 (v2 is EOL on new runners).- - uses: actions/checkout@v2 + - uses: actions/checkout@v4
28-37: Increase Nix store cache size to reduce churn on Rust+WASM builds.- gc-max-store-size-linux: 1G + gc-max-store-size-linux: 5G
43-43: Add trailing newline at EOF.- - run: nix develop -c test-wasm + - run: nix develop -c test-wasm +.github/workflows/test-ui.yaml (6)
2-2: Quote “on” and expand to mapping form.-on: [push] +"on": + push:
10-12: Reduce permissions: remove id-token.test: permissions: - id-token: write contents: read
14-16: Remove or use unused env COMMIT_SHA.- env: - COMMIT_SHA: ${{ github.sha }} + # env (unused) + # COMMIT_SHA: ${{ github.sha }}
17-17: Upgrade checkout to v4.- - uses: actions/checkout@v2 + - uses: actions/checkout@v4
43-46: Set up Node and install deps before running npm scripts.- - name: Frontend linting and formatting - run: | - cd svelte-test - npm run lint-format-check + - uses: actions/setup-node@v4 + with: + node-version: 22 + cache: "npm" + cache-dependency-path: "svelte-test/package-lock.json" + - name: Install deps and run lint/format + run: | + cd svelte-test + npm ci + npm run lint-format-check
46-46: Add trailing newline at EOF.- npm run lint-format-check + npm run lint-format-check +.github/workflows/npm-release.yaml (8)
11-13: Remove id-token permission (OIDC not used).permissions: - id-token: write contents: write
15-16: Expose version via step outputs instead of env.- outputs: - version: ${{ env.NEW_VERSION }} + outputs: + version: ${{ steps.set_version.outputs.version }}
52-58: Harden shell: quote vars and set strict mode.- OLD_HASH=$(npm view @rainlanguage/sqlite-web@latest dist.shasum 2>/dev/null || echo "none") - echo "OLD_HASH=$OLD_HASH" >> $GITHUB_ENV - echo "old hash: $OLD_HASH" + set -euo pipefail + OLD_HASH="$(npm view @rainlanguage/sqlite-web@latest dist.shasum 2>/dev/null || echo "none")" + echo "OLD_HASH=$OLD_HASH" >> "$GITHUB_ENV" + echo "old hash: $OLD_HASH"
61-67: Harden build/hash step; specify shasum algorithm and quote.- nix develop -c local-bundle - NEW_HASH=$(cd pkg && npm pack --silent | xargs shasum | cut -c1-40) - echo "NEW_HASH=$NEW_HASH" >> $GITHUB_ENV - echo "new hash: $NEW_HASH" - rm -f pkg/*.tgz + set -euo pipefail + nix develop -c local-bundle + NEW_HASH="$(cd pkg && npm pack --silent | xargs -I{} shasum -a 1 "{}" | cut -c1-40)" + echo "NEW_HASH=$NEW_HASH" >> "$GITHUB_ENV" + echo "new hash: $NEW_HASH" + rm -f pkg/*.tgz
71-77: Set version: write to env and step outputs; strip leading “v”.- - name: Set Version + - name: Set Version if: ${{ env.OLD_HASH != env.NEW_HASH }} - run: | - cd pkg - NEW_VERSION=$(npm version prerelease --preid alpha --no-git-tag-version) - echo "NEW_VERSION=$NEW_VERSION" >> $GITHUB_ENV + id: set_version + run: | + set -euo pipefail + cd pkg + npm version prerelease --preid alpha --no-git-tag-version >/dev/null + NEW_VERSION=$(node -p "require('./package.json').version") + echo "NEW_VERSION=$NEW_VERSION" >> "$GITHUB_ENV" + echo "version=$NEW_VERSION" >> "$GITHUB_OUTPUT"
88-92: Bug: wrong condition when pushing; compare hashes, not version.- - name: Push Changes To Remote - if: ${{ env.OLD_HASH != env.NEW_VERSION }} + - name: Push Changes To Remote + if: ${{ env.OLD_HASH != env.NEW_HASH }} run: | git push origin - git push -u origin npm-v${{ env.NEW_VERSION }} + git push -u origin "npm-v${NEW_VERSION}"Note: GITHUB_TOKEN env is unused for git push over SSH; safe to drop the env block here.
95-105: Quote env when creating/renaming tarball.- name: Create sqlite-web NPM Package Tarball if: ${{ env.OLD_HASH != env.NEW_HASH }} run: | cd pkg - echo "NPM_PACKAGE=$(npm pack --silent)" >> $GITHUB_ENV + echo "NPM_PACKAGE=$(npm pack --silent)" >> "$GITHUB_ENV" - name: Rename sqlite-web NPM Package Tarball if: ${{ env.OLD_HASH != env.NEW_HASH }} - run: mv pkg/${{ env.NPM_PACKAGE }} sqlite_web_npm_package_${{ env.NEW_VERSION }}.tgz + run: mv "pkg/${NPM_PACKAGE}" "sqlite_web_npm_package_${NEW_VERSION}.tgz"
126-126: Add trailing newline at EOF.- GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} + GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} +
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (4)
.github/workflows/npm-release.yaml(1 hunks).github/workflows/test-ui.yaml(1 hunks).github/workflows/test-wasm.yaml(1 hunks)Cargo.toml(1 hunks)
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2025-08-21T12:24:20.624Z
Learnt from: findolor
PR: rainlanguage/sqlite-web#2
File: .github/workflows/npm-release.yaml:73-77
Timestamp: 2025-08-21T12:24:20.624Z
Learning: The findolor user prefers to maintain consistency with existing npm-release workflows across repositories, even when technical improvements are available. They use a standardized GitHub Actions workflow pattern for NPM package releases.
Applied to files:
.github/workflows/npm-release.yaml
📚 Learning: 2025-08-21T12:24:24.582Z
Learnt from: findolor
PR: rainlanguage/sqlite-web#2
File: .github/workflows/npm-release.yaml:79-85
Timestamp: 2025-08-21T12:24:24.582Z
Learning: The rainlanguage organization uses workflows that commit pkg/package.json as part of their NPM release process across multiple repositories, and this pattern works fine for their setup.
Applied to files:
.github/workflows/npm-release.yaml
🪛 actionlint (1.7.7)
.github/workflows/test-wasm.yaml
17-17: the runner of "actions/checkout@v2" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
.github/workflows/test-ui.yaml
17-17: the runner of "actions/checkout@v2" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
🪛 YAMLlint (1.37.1)
.github/workflows/test-wasm.yaml
[warning] 2-2: truthy value should be one of [false, true]
(truthy)
[error] 43-43: no new line character at the end of file
(new-line-at-end-of-file)
.github/workflows/npm-release.yaml
[warning] 2-2: truthy value should be one of [false, true]
(truthy)
[error] 126-126: no new line character at the end of file
(new-line-at-end-of-file)
.github/workflows/test-ui.yaml
[warning] 2-2: truthy value should be one of [false, true]
(truthy)
[error] 46-46: no new line character at the end of file
(new-line-at-end-of-file)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: test
- GitHub Check: test
🔇 Additional comments (1)
Cargo.toml (1)
41-41: Confirm intentional exact pin of sqlite-wasm-rs (=0.3.0).Exact pins can cause friction for downstream consumers and make dependabot updates harder. If you want patch/minor updates, prefer a caret or tilde; if reproducibility is required, keep exact.
-sqlite-wasm-rs = { version = "=0.3.0", default-features = false, features = ["precompiled"] } +sqlite-wasm-rs = { version = "0.3", default-features = false, features = ["precompiled"] }
There was a problem hiding this comment.
Actionable comments posted: 11
♻️ Duplicate comments (17)
.github/workflows/test-ui.yaml (6)
2-2: Quote “on” and use mapping form; optionally add pull_request/workflow_dispatch triggers.YAML 1.1 treats
onas a truthy token; quoting it avoids linter warnings. Mapping form is clearer and extensible.-on: [push] +"on": + push: + # Optional: run CI on PRs and allow manual runs + # pull_request: + # workflow_dispatch:
10-13: Least privilege: drop unnecessary id-token permission.No step uses OIDC; keep only contents: read.
test: - permissions: - id-token: write - contents: read + permissions: + contents: read
14-16: Remove or actually use COMMIT_SHA.Defined but unused; remove to reduce noise, or wire it into a step.
- env: - COMMIT_SHA: ${{ github.sha }} + # env: (unused) remove or use in a step + # COMMIT_SHA: ${{ github.sha }}Run to confirm it's unused:
#!/bin/bash rg -n --hidden -S '\bCOMMIT_SHA\b' -g '!**/node_modules/**' -g '!**/dist/**'
17-20: Update checkout to v4 (runner compatibility) and consider pinning by commit SHA.actions/checkout@v2 is too old for current runners. Also consider pinning to a commit digest for supply-chain hardening.
- - uses: actions/checkout@v2 + - uses: actions/checkout@v4 # consider pinning to a commit SHA with: submodules: recursive fetch-depth: 0
43-46: Set up Node and install deps before lint/format (use working-directory).Without setup-node and npm ci, this will likely fail. Use cache and WD instead of cd.
- - name: Frontend linting and formatting - run: | - cd svelte-test - npm run lint-format-check + - name: Setup Node + uses: actions/setup-node@v4 # consider pinning to a commit SHA + with: + node-version: 22 + cache: npm + cache-dependency-path: svelte-test/package-lock.json + - name: Install dependencies + run: npm ci + working-directory: svelte-test + - name: Frontend linting and formatting + run: npm run lint-format-check + working-directory: svelte-testVerify the script exists and Node engine expectations:
#!/bin/bash set -euo pipefail test -f svelte-test/package.json || { echo "missing svelte-test/package.json"; exit 1; } jq -r '.engines // {}' svelte-test/package.json jq -r '.scripts["lint-format-check"]' svelte-test/package.json
46-46: Add a trailing newline at EOF.- npm run lint-format-check + npm run lint-format-check +.github/workflows/npm-release.yaml (5)
86-96: Quote env/path when writing env and renaming tarball.This was noted previously; keeping for traceability only.
- echo "NPM_PACKAGE=$(npm pack --silent)" >> $GITHUB_ENV + echo "NPM_PACKAGE=$(npm pack --silent)" >> "$GITHUB_ENV" @@ - run: mv pkg/${{ env.NPM_PACKAGE }} sqlite_web_npm_package_${{ env.NEW_VERSION }}.tgz + run: mv "pkg/${{ env.NPM_PACKAGE }}" "sqlite_web_npm_package_${{ env.NEW_VERSION }}.tgz"
2-2: Quote the “on” key for YAML 1.1 compatibility.Repeating the prior lint suggestion for completeness.
-on: +"on":
15-15: Job output depends on env var that may be unset when no release occurs.Prefer step outputs from the Set Version step (if you decide to keep outputs). Previously suggested; keeping as a reference.
117-117: Add trailing newline to satisfy linters.YAMLlint flags missing newline at EOF.
GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} +
63-68: Fix: versioning runs in the wrong directory and produces “vv…” tags.
- npm version is executed from repo root (no package.json) → step will fail.
- npm version returns a leading “v”, and later you tag npm-v${NEW_VERSION} → results in npm-vvX.Y.Z… tags.
Run in pkg, then strip the leading “v” before exporting NEW_VERSION.
- - name: Set Version + - name: Set Version if: ${{ env.OLD_HASH != env.NEW_HASH }} run: | - NEW_VERSION=$(npm version prerelease --preid alpha --no-git-tag-version) - echo "NEW_VERSION=$NEW_VERSION" >> $GITHUB_ENV + cd pkg + NEW_VERSION="$(npm version prerelease --preid alpha --no-git-tag-version)" + # drop leading "v" from npm output + NEW_VERSION="${NEW_VERSION#v}" + echo "NEW_VERSION=$NEW_VERSION" >> "$GITHUB_ENV".github/workflows/test-wasm.yaml (6)
2-2: Quote the on key and use mapping form.This was already raised; agree and recommend applying it to satisfy YAML 1.1 linters and improve clarity.
10-13: Drop id-token: write — not used.No step uses OIDC; least-privilege is contents: read only.
14-16: Remove unused env COMMIT_SHA or use it.It’s defined but not referenced; delete to keep the job tidy.
17-17: Upgrade checkout to v4.v2 is EOL on new runners; move to v4.
28-37: Increase Nix store cache size to reduce churn on Rust+WASM builds.1G is commonly too small; 5–10G tends to avoid frequent evictions.
- gc-max-store-size-linux: 1G + gc-max-store-size-linux: 5G
43-43: Add a trailing newline.Fixes the “no newline at end of file” linter error.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (3)
.github/workflows/npm-release.yaml(1 hunks).github/workflows/test-ui.yaml(1 hunks).github/workflows/test-wasm.yaml(1 hunks)
🧰 Additional context used
🧠 Learnings (4)
📚 Learning: 2025-08-21T12:24:20.624Z
Learnt from: findolor
PR: rainlanguage/sqlite-web#2
File: .github/workflows/npm-release.yaml:73-77
Timestamp: 2025-08-21T12:24:20.624Z
Learning: The findolor user prefers to maintain consistency with existing npm-release workflows across repositories, even when technical improvements are available. They use a standardized GitHub Actions workflow pattern for NPM package releases.
Applied to files:
.github/workflows/npm-release.yaml
📚 Learning: 2025-08-21T12:24:24.582Z
Learnt from: findolor
PR: rainlanguage/sqlite-web#2
File: .github/workflows/npm-release.yaml:79-85
Timestamp: 2025-08-21T12:24:24.582Z
Learning: The rainlanguage organization uses workflows that commit pkg/package.json as part of their NPM release process across multiple repositories, and this pattern works fine for their setup.
Applied to files:
.github/workflows/npm-release.yaml
📚 Learning: 2025-08-21T12:24:09.141Z
Learnt from: findolor
PR: rainlanguage/sqlite-web#2
File: .github/workflows/npm-release.yaml:54-58
Timestamp: 2025-08-21T12:24:09.141Z
Learning: The rainlanguage organization prefers to maintain consistency in GitHub Actions workflows across repositories, even when shellcheck suggests improvements. They prioritize organizational standards over individual script hardening suggestions.
Applied to files:
.github/workflows/npm-release.yaml
📚 Learning: 2025-08-21T12:24:03.443Z
Learnt from: findolor
PR: rainlanguage/sqlite-web#2
File: .github/workflows/npm-release.yaml:11-13
Timestamp: 2025-08-21T12:24:03.443Z
Learning: User findolor prefers to keep GitHub Actions workflow permissions consistent across repositories, even if some permissions are unused in specific workflows, to maintain template standardization.
Applied to files:
.github/workflows/npm-release.yaml
🪛 actionlint (1.7.7)
.github/workflows/test-ui.yaml
17-17: the runner of "actions/checkout@v2" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
.github/workflows/test-wasm.yaml
17-17: the runner of "actions/checkout@v2" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
🪛 YAMLlint (1.37.1)
.github/workflows/test-ui.yaml
[warning] 2-2: truthy value should be one of [false, true]
(truthy)
[error] 46-46: no new line character at the end of file
(new-line-at-end-of-file)
.github/workflows/npm-release.yaml
[warning] 2-2: truthy value should be one of [false, true]
(truthy)
[error] 117-117: no new line character at the end of file
(new-line-at-end-of-file)
.github/workflows/test-wasm.yaml
[warning] 2-2: truthy value should be one of [false, true]
(truthy)
[error] 43-43: no new line character at the end of file
(new-line-at-end-of-file)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: test
- GitHub Check: test
🔇 Additional comments (2)
.github/workflows/test-wasm.yaml (2)
22-27: Pin nix-quick-install-action to immutable commit SHA for v30
To harden the supply chain, replace the floating tag with the exact commit SHA corresponding to v30.Updated snippet:
- - uses: nixbuild/nix-quick-install-action@v30 + - uses: nixbuild/nix-quick-install-action@5bb6a3b3abe66fd09bbf250dce8ada94f856a703 # v30
17-20: Harden the WASM workflow’s checkout stepThe
test-wasmjob doesn’t push or otherwise require full Git history, so it’s safe to switch to a shallow clone and disable credentials persistence:• File:
.github/workflows/test-wasm.yaml
• Job:test, Step:uses: actions/checkoutSuggested diff:
- - uses: actions/checkout@v2 - with: - submodules: recursive - fetch-depth: 0 + - uses: actions/checkout@v4 + with: + submodules: recursive + fetch-depth: 1 + persist-credentials: falseNo other steps in this workflow push commits or rely on deeper history, so these settings will improve security and performance.
There was a problem hiding this comment.
Actionable comments posted: 4
♻️ Duplicate comments (9)
.github/workflows/test-wasm.yaml (9)
2-3: Quote "on", expand to mapping, and add pull_request + workflow_dispatch triggers.Prevents YAML 1.1 “truthy” lint warnings and ensures CI runs on PRs and manual dispatches.
-name: Test WASM -on: [push] +name: Test WASM +"on": + push: + branches: + - "**" + pull_request: + branches: + - "**" + workflow_dispatch: {}
4-6: Scope concurrency group to the workflow to avoid cross-workflow cancellations.concurrency: - group: ${{ github.ref }}-wasm + group: ${{ github.workflow }}-${{ github.ref }}-wasm cancel-in-progress: ${{ github.ref != 'refs/heads/main' }}
9-16: Add a job timeout to prevent hung CI on long Nix builds.jobs: test: + timeout-minutes: 30 permissions: id-token: write contents: read
13-13: Pin the runner for reproducibility.- runs-on: ubuntu-latest + runs-on: ubuntu-24.04
14-16: Remove or use unused env COMMIT_SHA.It’s defined but never referenced; drop it or reference ${{ env.COMMIT_SHA }} somewhere.
- env: - COMMIT_SHA: ${{ github.sha }} + # env: (unused) + # COMMIT_SHA: ${{ github.sha }}
17-17: Update actions/checkout to v4 (v2 is EOL on new runners).- - uses: actions/checkout@v2 + - uses: actions/checkout@v4
28-37: Pin cache-nix-action to a commit SHA.Same rationale as above; replace @v6 with the v6 commit SHA.
- name: Restore and save Nix store - uses: nix-community/cache-nix-action@v6 + uses: nix-community/cache-nix-action@<commit-sha> # v6
39-45: Name the run steps for clearer logs.Improves readability in the Actions UI.
- - run: nix develop -c rainix-sol-prelude + - name: Prelude checks + run: nix develop -c rainix-sol-prelude @@ - - run: echo "// Generated file placeholder" > packages/sqlite-web/src/embedded_worker.js + - name: Generate placeholder embedded worker + run: echo "// Generated file placeholder" > packages/sqlite-web/src/embedded_worker.js @@ - - run: nix develop -c rainix-rs-static + - name: Build Rust static artifacts + run: nix develop -c rainix-rs-static @@ - - run: nix develop -c test-wasm + - name: Run WASM tests + run: nix develop -c test-wasm
45-45: Add a trailing newline.Fixes the “no newline at end of file” linter error.
- - name: Run WASM tests - run: nix develop -c test-wasm + - name: Run WASM tests + run: nix develop -c test-wasm +
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
.github/workflows/test-wasm.yaml(1 hunks)
🧰 Additional context used
🪛 actionlint (1.7.7)
.github/workflows/test-wasm.yaml
17-17: the runner of "actions/checkout@v2" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
🪛 YAMLlint (1.37.1)
.github/workflows/test-wasm.yaml
[warning] 2-2: truthy value should be one of [false, true]
(truthy)
[error] 45-45: no new line character at the end of file
(new-line-at-end-of-file)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: test
- GitHub Check: test
There was a problem hiding this comment.
Actionable comments posted: 6
♻️ Duplicate comments (30)
.github/workflows/npm-release.yaml (10)
46-50: Shell quoting and hashing nits noted previously; acknowledging org preference.Unquoted
$GITHUB_ENVwrites and unspecifiedshasumalgo were flagged earlier (SC2086). Given your documented preference for consistency across repos, I’m not re‑requesting changes. Keeping as-is is acceptable for your org.Also applies to: 53-59, 91-92, 95-95
11-13: Permissions note: leaving id-token per org standard.Prior nit suggested dropping
id-token. Respecting your standardization choice to keep it.
14-15: Bug: job.outputs cannot read env values written during steps. Use a step output.
jobs.release.outputs.version: ${{ env.NEW_VERSION }}won’t work because$GITHUB_ENVwrites aren’t available in theoutputscontext. Expose the version via a step output and reference that.Apply:
- outputs: - version: ${{ env.NEW_VERSION }} + outputs: + version: ${{ steps.set_version.outputs.version }}
63-68: Fix “Set Version”: run in pkg/, avoid leading “v”, and emit a step output.Currently runs from repo root (likely no package.json), and
npm versionreturns a value prefixed with “v” (leading to “vv…” in tags/messages). Also no step output is set. Make it robust:- - name: Set Version + - name: Set Version if: ${{ env.OLD_HASH != env.NEW_HASH }} - run: | - NEW_VERSION=$(npm version prerelease --preid alpha --no-git-tag-version) - echo "NEW_VERSION=$NEW_VERSION" >> $GITHUB_ENV + id: set_version + run: | + set -euo pipefail + cd pkg + # Bump prerelease without creating a git tag + npm version prerelease --preid alpha --no-git-tag-version >/dev/null + # Read the raw semver (no leading "v") from package.json + NEW_VERSION="$(node -p "require('./package.json').version")" + echo "NEW_VERSION=$NEW_VERSION" >> "$GITHUB_ENV" + echo "version=$NEW_VERSION" >> "$GITHUB_OUTPUT"
77-84: Nit: remove “-u” when pushing a tag.
-uis ignored for tag refs; a plain push is clearer.git push origin - git push -u origin npm-v${{ env.NEW_VERSION }} + git push origin npm-v${{ env.NEW_VERSION }}
23-27: Pin third‑party actions (avoid @main) to a tag or SHA.For supply‑chain stability, pin these to a release tag or commit SHA.
- - uses: DeterminateSystems/nix-installer-action@main + - uses: DeterminateSystems/nix-installer-action@<TAG_OR_SHA> @@ - - uses: DeterminateSystems/flakehub-cache-action@main + - uses: DeterminateSystems/flakehub-cache-action@<TAG_OR_SHA>
2-2: YAML 1.1 nit: quote “on”.Some linters warn on unquoted
on. Optional.-on: +"on":
86-96: Quote env expansions in paths/metadata for safety.Quoting avoids accidental word-splitting. Optional polish.
- echo "NPM_PACKAGE=$(npm pack --silent)" >> $GITHUB_ENV + echo "NPM_PACKAGE=$(npm pack --silent)" >> "$GITHUB_ENV" @@ - run: mv pkg/${{ env.NPM_PACKAGE }} sqlite_web_npm_package_${{ env.NEW_VERSION }}.tgz + run: mv "pkg/${{ env.NPM_PACKAGE }}" "sqlite_web_npm_package_${{ env.NEW_VERSION }}.tgz" @@ - tag_name: npm-v${{ env.NEW_VERSION }} - name: NPM Package Release v${{ env.NEW_VERSION }} + tag_name: npm-v${{ env.NEW_VERSION }} + name: NPM Package Release v${{ env.NEW_VERSION }} files: | - sqlite_web_npm_package_${{ env.NEW_VERSION }}.tgz + sqlite_web_npm_package_${{ env.NEW_VERSION }}.tgzAlso applies to: 112-116
117-117: Add trailing newline at EOF (linter).Minor formatting fix to satisfy YAMLlint.
- GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} + GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} +
1-6: Optional: add workflow concurrency to prevent parallel releases.Prevents two overlapping main pushes from racing.
name: NPM Packages Release +"concurrency": + group: release-${{ github.ref }} + cancel-in-progress: false "on": push: branches: - mainflake.nix (1)
31-36: Fix out-dir collision in wasm-pack builds (artifacts clobber each other).Both crates write to ../../pkg, so the second build overwrites the first. Use distinct subdirectories.
- cd packages/sqlite-web-core - wasm-pack build --target web --out-dir ../../pkg + cd packages/sqlite-web-core + wasm-pack build --target web --out-dir ../../pkg/sqlite-web-core cd ../.. - cd packages/sqlite-web - wasm-pack build --target web --out-dir ../../pkg + cd packages/sqlite-web + wasm-pack build --target web --out-dir ../../pkg/sqlite-web cd ../..Follow-up: ensure scripts/local-bundle.sh reads from the new subdirs or copies/merges them into the final bundle.
.github/workflows/test-wasm.yaml (10)
2-3: Quote "on" and expand triggers to include PRs and manual runs.Improves lint compatibility and ensures CI runs on pull requests.
-on: [push] +"on": + push: + branches: + - "**" + pull_request: + branches: + - "**" + workflow_dispatch: {}
4-6: Scope concurrency group to workflow to avoid cross-workflow cancellations.concurrency: - group: ${{ github.ref }}-wasm + group: ${{ github.workflow }}-${{ github.ref }}-wasm cancel-in-progress: ${{ github.ref != 'refs/heads/main' }}
10-12: Least-privilege: drop id-token unless required.No OIDC usage is visible; remove id-token: write.
permissions: - id-token: write contents: read + # id-token: write # Uncomment only if a later step needs OIDC
13-13: Pin the runner and add a job timeout to prevent hangs.- runs-on: ubuntu-latest + runs-on: ubuntu-24.04 + timeout-minutes: 30
14-16: Remove unused env COMMIT_SHA.- env: - COMMIT_SHA: ${{ github.sha }} + # env: (unused) + # COMMIT_SHA: ${{ github.sha }}
17-21: Update checkout to v4 and prefer a shallow clone.- - uses: actions/checkout@v2 + - uses: actions/checkout@v4 with: submodules: recursive - fetch-depth: 0 + fetch-depth: 1
28-37: Pin cache-nix-action to a full commit SHA (security best practice).- name: Restore and save Nix store - uses: nix-community/cache-nix-action@v6 + uses: nix-community/cache-nix-action@135667ec418502fa5a3598af6fb9eb733888ce6a # v6 with: # restore and save a cache using this key primary-key: nix-${{ runner.os }}-${{ hashFiles('**/*.nix', '**/flake.lock') }} # if there's no cache hit, restore a cache by this prefix restore-prefixes-first-match: nix-${{ runner.os }}- # collect garbage until the Nix store size (in bytes) is at most this number # before trying to save a new cache # 1G = 1073741824 gc-max-store-size-linux: 1G
41-41: Ensure target directory exists before writing placeholder file.Handled by the named step diff above (mkdir -p + heredoc). No further action if you apply that diff.
45-45: Add a trailing newline at EOF.- - run: nix develop -c test-wasm + - run: nix develop -c test-wasm +
39-45: Name the run steps for clearer logs.- - run: nix develop -c build-submodules + - name: Build submodules + run: nix develop -c build-submodules @@ - - run: echo "// Generated file placeholder" > packages/sqlite-web/src/embedded_worker.js + - name: Generate placeholder embedded worker + run: | + set -euo pipefail + mkdir -p packages/sqlite-web/src + cat > packages/sqlite-web/src/embedded_worker.js <<'EOF' +// Generated file placeholder +EOF @@ - - run: nix develop -c rainix-rs-static + - name: Build Rust static artifacts + run: nix develop -c rainix-rs-static @@ - - run: nix develop -c test-wasm + - name: Run WASM tests + run: nix develop -c test-wasm.github/workflows/test-ui.yaml (9)
2-3: Quote "on" and add pull_request + manual triggers.Enables CI on PRs and satisfies YAML 1.1 linters.
-on: [push] +"on": + push: + branches: + - "**" + pull_request: + branches: + - "**" + workflow_dispatch: {}
10-13: Reduce permissions and pin the runner; add a timeout.test: - permissions: - id-token: write - contents: read + permissions: + contents: read - runs-on: ubuntu-latest + runs-on: ubuntu-24.04 + timeout-minutes: 30
14-16: Remove unused env COMMIT_SHA.- env: - COMMIT_SHA: ${{ github.sha }} + # env: (unused) + # COMMIT_SHA: ${{ github.sha }}
17-21: Update checkout to v4 and prefer shallow clone.- - uses: actions/checkout@v2 + - uses: actions/checkout@v4 with: submodules: recursive - fetch-depth: 0 + fetch-depth: 1
22-27: Nix setup: explicitly enable flake features and consider pinning the action.- - uses: nixbuild/nix-quick-install-action@v30 + - uses: nixbuild/nix-quick-install-action@v30 # consider pinning to a commit SHA with: nix_conf: | keep-env-derivations = true keep-outputs = true + experimental-features = nix-command flakes
46-46: Add a trailing newline at EOF.- npm run lint-format-check + npm run lint-format-check +
39-41: Name your steps for clearer logs.- - run: nix develop -c build-submodules + - name: Build submodules + run: nix develop -c build-submodules @@ - - run: nix develop -c test-ui + - name: UI tests + run: nix develop -c test-ui
43-46: Set up Node and install deps before running lint/format.Without setup-node and npm ci, this step is brittle on clean runners.
- - name: Frontend linting and formatting - run: | - cd svelte-test - npm run lint-format-check + - name: Setup Node + uses: actions/setup-node@v4 + with: + node-version: 22 + cache: "npm" + cache-dependency-path: "svelte-test/package-lock.json" + - name: Install deps and run lint/format + run: | + cd svelte-test + npm ci + npm run lint-format-check
28-37: Pin cache-nix-action to a commit for supply-chain safety.- name: Restore and save Nix store - uses: nix-community/cache-nix-action@v6 + uses: nix-community/cache-nix-action@135667ec418502fa5a3598af6fb9eb733888ce6a # v6
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (4)
.github/workflows/npm-release.yaml(1 hunks).github/workflows/test-ui.yaml(1 hunks).github/workflows/test-wasm.yaml(1 hunks)flake.nix(4 hunks)
🧰 Additional context used
🧠 Learnings (4)
📚 Learning: 2025-08-21T12:24:20.624Z
Learnt from: findolor
PR: rainlanguage/sqlite-web#2
File: .github/workflows/npm-release.yaml:73-77
Timestamp: 2025-08-21T12:24:20.624Z
Learning: The findolor user prefers to maintain consistency with existing npm-release workflows across repositories, even when technical improvements are available. They use a standardized GitHub Actions workflow pattern for NPM package releases.
Applied to files:
.github/workflows/npm-release.yaml
📚 Learning: 2025-08-21T12:24:24.582Z
Learnt from: findolor
PR: rainlanguage/sqlite-web#2
File: .github/workflows/npm-release.yaml:79-85
Timestamp: 2025-08-21T12:24:24.582Z
Learning: The rainlanguage organization uses workflows that commit pkg/package.json as part of their NPM release process across multiple repositories, and this pattern works fine for their setup.
Applied to files:
.github/workflows/npm-release.yaml
📚 Learning: 2025-08-21T12:24:09.141Z
Learnt from: findolor
PR: rainlanguage/sqlite-web#2
File: .github/workflows/npm-release.yaml:54-58
Timestamp: 2025-08-21T12:24:09.141Z
Learning: The rainlanguage organization prefers to maintain consistency in GitHub Actions workflows across repositories, even when shellcheck suggests improvements. They prioritize organizational standards over individual script hardening suggestions.
Applied to files:
.github/workflows/npm-release.yaml
📚 Learning: 2025-08-21T12:24:03.443Z
Learnt from: findolor
PR: rainlanguage/sqlite-web#2
File: .github/workflows/npm-release.yaml:11-13
Timestamp: 2025-08-21T12:24:03.443Z
Learning: User findolor prefers to keep GitHub Actions workflow permissions consistent across repositories, even if some permissions are unused in specific workflows, to maintain template standardization.
Applied to files:
.github/workflows/npm-release.yaml
🪛 actionlint (1.7.7)
.github/workflows/test-wasm.yaml
17-17: the runner of "actions/checkout@v2" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
.github/workflows/test-ui.yaml
17-17: the runner of "actions/checkout@v2" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
🪛 YAMLlint (1.37.1)
.github/workflows/test-wasm.yaml
[warning] 2-2: truthy value should be one of [false, true]
(truthy)
[error] 45-45: no new line character at the end of file
(new-line-at-end-of-file)
.github/workflows/test-ui.yaml
[warning] 2-2: truthy value should be one of [false, true]
(truthy)
[error] 46-46: no new line character at the end of file
(new-line-at-end-of-file)
.github/workflows/npm-release.yaml
[warning] 2-2: truthy value should be one of [false, true]
(truthy)
[error] 117-117: no new line character at the end of file
(new-line-at-end-of-file)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: test
- GitHub Check: test
There was a problem hiding this comment.
Actionable comments posted: 4
♻️ Duplicate comments (28)
.github/workflows/test-ui.yaml (11)
2-2: Quote "on" and use mapping form; also add pull_request and workflow_dispatch triggers.Prevents YAML 1.1 truthiness lint warnings and enables PR/manual runs.
-on: [push] +"on": + push: + pull_request: + workflow_dispatch:
4-6: Scope concurrency group to workflow to avoid cross-workflow cancellations.Including the workflow name removes collisions across different workflows on the same ref.
concurrency: - group: ${{ github.ref }}-ui + group: ${{ github.workflow }}-${{ github.ref }}-ui cancel-in-progress: ${{ github.ref != 'refs/heads/main' }}
10-13: Drop unnecessary id-token permission (least privilege).OIDC is not used here; keep only contents: read.
test: permissions: - id-token: write contents: read
14-16: Remove unused COMMIT_SHA env or wire it up.Currently unused; delete to reduce noise.
- env: - COMMIT_SHA: ${{ github.sha }}
22-27: Nix: explicitly enable flakes and nix-command.Avoids surprises if runner defaults change.
- uses: nixbuild/nix-quick-install-action@v30 with: nix_conf: | keep-env-derivations = true keep-outputs = true + experimental-features = nix-command flakes
28-29: Consider pinning cache action to a commit SHA.Improves supply-chain posture; configuration looks correct otherwise.
- - name: Restore and save Nix store - uses: nix-community/cache-nix-action@v6 + - name: Restore and save Nix store + uses: nix-community/cache-nix-action@v6 # consider pinning to a commit SHA
13-13: Add a conservative job timeout.Prevents runaway jobs.
runs-on: ubuntu-latest + timeout-minutes: 30
39-43: Name the Nix steps for easier troubleshooting.Makes logs clearer.
- - run: nix develop -c build-submodules + - name: Build submodules + run: nix develop -c build-submodules @@ - - run: nix develop -c local-bundle + - name: Bundle UI locally + run: nix develop -c local-bundle @@ - - run: nix develop -c test-ui + - name: UI tests + run: nix develop -c test-ui
45-48: Set up Node and install deps before running lint/format.Without Node setup and npm ci this step will likely fail.
- - name: Frontend linting and formatting - run: | - cd svelte-test - npm run lint-format-check + - name: Setup Node + uses: actions/setup-node@v4 + with: + node-version: 22 + cache: npm + cache-dependency-path: svelte-test/package-lock.json + - name: Install deps and run lint/format + working-directory: svelte-test + run: | + npm ci + npm run lint-format-check +
48-48: Add trailing newline at EOF.Satisfies linters and editors.
- npm run lint-format-check + npm run lint-format-check +
17-17: Update checkout to v4 (v2 is too old and flagged by actionlint).Prevents runtime failure on modern runners.
- - uses: actions/checkout@v2 + - uses: actions/checkout@v4.github/workflows/test-wasm.yaml (10)
2-3: Quote "on" and expand triggers; also run on PRs and allow manual dispatch.Quoting fixes YAML 1.1 truthy warning; mapping form is clearer. Adding pull_request and workflow_dispatch ensures CI runs on PRs and can be invoked manually.
-name: Test WASM -on: [push] +name: Test WASM +"on": + push: + branches: + - "**" + pull_request: + branches: + - "**" + workflow_dispatch: {}
4-6: Scope the concurrency group per workflow to avoid cross-workflow cancellations.Include the workflow name so different workflows on the same ref don’t cancel each other. Behavior of cancel-in-progress remains intact.
concurrency: - group: ${{ github.ref }}-wasm + group: ${{ github.workflow }}-${{ github.ref }}-wasm cancel-in-progress: ${{ github.ref != 'refs/heads/main' }}
8-16: Add a reasonable job timeout and drop the unused COMMIT_SHA env.Timeout caps hung builds; COMMIT_SHA isn’t referenced—remove to keep the job tidy.
jobs: test: + timeout-minutes: 30 permissions: id-token: write contents: read runs-on: ubuntu-latest - env: - COMMIT_SHA: ${{ github.sha }}
10-13: Least-privilege: remove id-token: write unless you really need OIDC.Keep contents: read; grant id-token only if a later step uses OIDC.
permissions: - id-token: write contents: read + # id-token: write # Uncomment only if OIDC is required by a later step
13-13: Pin the runner for reproducibility.Avoid ubuntu-latest drift; pin to a specific image (24.04 shown).
- runs-on: ubuntu-latest + runs-on: ubuntu-24.04
17-20: Upgrade checkout to v4 and prefer a shallow clone.actions/checkout@v2 is EOL on newer runners; v4 resolves actionlint’s complaint. Use fetch-depth: 1 unless full history is required.
- - uses: actions/checkout@v2 + - uses: actions/checkout@v4 with: submodules: recursive - fetch-depth: 0 + fetch-depth: 1
22-27: Pin nix-quick-install-action to an immutable commit and enable flakes explicitly.Pinning improves supply-chain security; enabling flake features avoids relying on runner defaults. Replace with the commit for v30.
- - uses: nixbuild/nix-quick-install-action@v30 + - uses: nixbuild/nix-quick-install-action@<commit-sha> # v30 with: nix_conf: | keep-env-derivations = true keep-outputs = true + experimental-features = nix-command flakes
28-37: Pin cache-nix-action to a full commit SHA for immutability.Use the v6 commit to prevent tag drift.
- name: Restore and save Nix store - uses: nix-community/cache-nix-action@v6 + uses: nix-community/cache-nix-action@135667ec418502fa5a3598af6fb9eb733888ce6a # v6 with: # restore and save a cache using this key primary-key: nix-${{ runner.os }}-${{ hashFiles('**/*.nix', '**/flake.lock') }} # if there's no cache hit, restore a cache by this prefix restore-prefixes-first-match: nix-${{ runner.os }}- # collect garbage until the Nix store size (in bytes) is at most this number # before trying to save a new cache # 1G = 1073741824 gc-max-store-size-linux: 1G
39-45: Name the run steps for clearer logs and add a default strict shell.Step names improve Actions UI readability. Setting a strict default shell helps catch errors early.
name: Test WASM +"on": + # ... +defaults: + run: + shell: bash -euo pipefail {0} jobs: test: steps: - - run: nix develop -c build-submodules + - name: Build submodules + run: nix develop -c build-submodules - - - run: nix develop -c local-bundle + - name: Local bundle + run: nix develop -c local-bundle - - - run: nix develop -c rainix-rs-static + - name: Build Rust static artifacts + run: nix develop -c rainix-rs-static - - - run: nix develop -c test-wasm + - name: Run WASM tests + run: nix develop -c test-wasm
45-45: Add a trailing newline.Fixes the “no newline at end of file” linter error.
- - run: nix develop -c test-wasm + - run: nix develop -c test-wasm +.github/workflows/npm-release.yaml (7)
11-13: Permissions: keeping id-token is acceptable per your org template.Acknowledging your preference to standardize permissions across repos; no change requested here.
2-2: Quote "on" for YAML 1.1 compatibility.Keeps parsers happy and silences yamllint truthy warnings.
-on: +"on":
23-27: Pin third-party actions to immutable versions (avoid @main).Supply-chain hardening. Use a release tag or commit SHA for both DeterminateSystems actions.
- - uses: DeterminateSystems/nix-installer-action@main + - uses: DeterminateSystems/nix-installer-action@<COMMIT_SHA_OR_TAG> @@ - - uses: DeterminateSystems/flakehub-cache-action@main + - uses: DeterminateSystems/flakehub-cache-action@<COMMIT_SHA_OR_TAG>If helpful, I can propose the latest stable SHAs.
75-84: Git tagging/push nits: annotated tag; drop -u for tag push.
- Create an annotated tag (better metadata).
- -u has no effect for tags; remove it.
git add "pkg/package.json" git commit -m "NPM Package Release v${{ env.NEW_VERSION }}" - git tag npm-v${{ env.NEW_VERSION }} + git tag -a "npm-v${{ env.NEW_VERSION }}" -m "NPM Package Release v${{ env.NEW_VERSION }}" @@ - git push origin - git push -u origin npm-v${{ env.NEW_VERSION }} + git push origin + git push origin "npm-v${{ env.NEW_VERSION }}"
93-97: Quote env and paths when capturing/renaming the tarball.Prevents word-splitting/globbing issues.
- echo "NPM_PACKAGE=$(npm pack --silent)" >> $GITHUB_ENV + echo "NPM_PACKAGE=$(npm pack --silent)" >> "$GITHUB_ENV" @@ - run: mv pkg/${{ env.NPM_PACKAGE }} sqlite_web_npm_package_${{ env.NEW_VERSION }}.tgz + run: mv "pkg/${{ env.NPM_PACKAGE }}" "sqlite_web_npm_package_${{ env.NEW_VERSION }}.tgz"
108-119: Minor: quote release inputs; add workflow concurrency; ensure trailing newline.
- Quote interpolations for consistency.
- Add concurrency to avoid parallel releases on main.
- File is missing a trailing newline.
+concurrency: + group: release-${{ github.ref }} + cancel-in-progress: false @@ - tag_name: npm-v${{ env.NEW_VERSION }} - name: NPM Package Release v${{ env.NEW_VERSION }} + tag_name: npm-v${{ env.NEW_VERSION }} + name: NPM Package Release v${{ env.NEW_VERSION }} files: | sqlite_web_npm_package_${{ env.NEW_VERSION }}.tgz env: GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} +
65-70: Prevent “vv” version/tag: normalize NEW_VERSION after npm version.npm version prints a leading “v”. Strip it once so commit/tag strings aren’t double-prefixed while preserving your existing pattern.
- run: | - NEW_VERSION=$(npm version prerelease --preid alpha --no-git-tag-version) - echo "NEW_VERSION=$NEW_VERSION" >> $GITHUB_ENV + run: | + set -euo pipefail + NEW_VERSION="$(npm version prerelease --preid alpha --no-git-tag-version)" + NEW_VERSION="${NEW_VERSION#v}" + echo "NEW_VERSION=$NEW_VERSION" >> "$GITHUB_ENV"
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (3)
.github/workflows/npm-release.yaml(1 hunks).github/workflows/test-ui.yaml(1 hunks).github/workflows/test-wasm.yaml(1 hunks)
🧰 Additional context used
🧠 Learnings (4)
📚 Learning: 2025-08-21T12:24:20.624Z
Learnt from: findolor
PR: rainlanguage/sqlite-web#2
File: .github/workflows/npm-release.yaml:73-77
Timestamp: 2025-08-21T12:24:20.624Z
Learning: The findolor user prefers to maintain consistency with existing npm-release workflows across repositories, even when technical improvements are available. They use a standardized GitHub Actions workflow pattern for NPM package releases.
Applied to files:
.github/workflows/npm-release.yaml
📚 Learning: 2025-08-21T12:24:24.582Z
Learnt from: findolor
PR: rainlanguage/sqlite-web#2
File: .github/workflows/npm-release.yaml:79-85
Timestamp: 2025-08-21T12:24:24.582Z
Learning: The rainlanguage organization uses workflows that commit pkg/package.json as part of their NPM release process across multiple repositories, and this pattern works fine for their setup.
Applied to files:
.github/workflows/npm-release.yaml
📚 Learning: 2025-08-21T12:24:09.141Z
Learnt from: findolor
PR: rainlanguage/sqlite-web#2
File: .github/workflows/npm-release.yaml:54-58
Timestamp: 2025-08-21T12:24:09.141Z
Learning: The rainlanguage organization prefers to maintain consistency in GitHub Actions workflows across repositories, even when shellcheck suggests improvements. They prioritize organizational standards over individual script hardening suggestions.
Applied to files:
.github/workflows/npm-release.yaml
📚 Learning: 2025-08-21T12:24:03.443Z
Learnt from: findolor
PR: rainlanguage/sqlite-web#2
File: .github/workflows/npm-release.yaml:11-13
Timestamp: 2025-08-21T12:24:03.443Z
Learning: User findolor prefers to keep GitHub Actions workflow permissions consistent across repositories, even if some permissions are unused in specific workflows, to maintain template standardization.
Applied to files:
.github/workflows/npm-release.yaml
🪛 actionlint (1.7.7)
.github/workflows/test-wasm.yaml
17-17: the runner of "actions/checkout@v2" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
.github/workflows/test-ui.yaml
17-17: the runner of "actions/checkout@v2" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
🪛 YAMLlint (1.37.1)
.github/workflows/test-wasm.yaml
[warning] 2-2: truthy value should be one of [false, true]
(truthy)
[error] 45-45: no new line character at the end of file
(new-line-at-end-of-file)
.github/workflows/test-ui.yaml
[warning] 2-2: truthy value should be one of [false, true]
(truthy)
[error] 48-48: no new line character at the end of file
(new-line-at-end-of-file)
.github/workflows/npm-release.yaml
[warning] 2-2: truthy value should be one of [false, true]
(truthy)
[error] 35-35: trailing spaces
(trailing-spaces)
[error] 119-119: no new line character at the end of file
(new-line-at-end-of-file)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: test
- GitHub Check: test
🔇 Additional comments (1)
.github/workflows/test-wasm.yaml (1)
39-42: Nix task definitions exist inflake.nixand are included in your devShellAll of the steps you invoke in
.github/workflows/test-wasm.yamlare defined—and consumable vianix develop -c—in yourflake.nix:
- In
flake.nix, underpackages.${system} // { … }, you have tasks namedbuild-submodules,local-bundle,test-wasm,build-wasm,test-ui, andtest-full-integration(see lines 14–26, 39–46, 59–68 offlake.nix).- Your
devShells.defaultincludespackages.build-submodulesandpackages.local-bundle(among the others), so bothnix develop -c build-submodulesandnix develop -c local-bundlewill work as expected.- No stray
COMMIT_SHAreferences remain.Since the CI steps you’ve added all map to valid flake tasks and are exposed in the default devShell, there’s nothing left to change here.
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
scripts/local-bundle.sh (1)
2-2: Harden the script with pipefail and nounset.Catches errors in pipelines and unset vars; safer for CI.
-set -e +set -Eeuo pipefail +IFS=$'\n\t'
♻️ Duplicate comments (7)
scripts/local-bundle.sh (7)
12-12: Ensure target directory exists and truncate atomically.Prevents failures if src/ is missing and avoids subshell/echo quirks.
-echo "" > packages/sqlite-web/src/embedded_worker.js +mkdir -p packages/sqlite-web/src +: > packages/sqlite-web/src/embedded_worker.js
15-17: Build the core in release mode for optimized WASM.Smaller, faster output; runs wasm-opt when available.
-cd packages/sqlite-web-core -wasm-pack build --target web --out-dir ../../pkg +cd packages/sqlite-web-core +wasm-pack build --release --target web --out-dir ../../pkg cd ../..
20-23: Improve core build failure diagnostics (name missing file(s) and list pkg/).Faster debugging in CI and locally.
-if [ ! -f "pkg/sqlite_web_core_bg.wasm" ] || [ ! -f "pkg/sqlite_web_core.js" ]; then - echo "❌ Core build failed - missing generated files" - exit 1 -fi +missing=() +[ -f "pkg/sqlite_web_core_bg.wasm" ] || missing+=("pkg/sqlite_web_core_bg.wasm") +[ -f "pkg/sqlite_web_core.js" ] || missing+=("pkg/sqlite_web_core.js") +if [ "${#missing[@]}" -gt 0 ]; then + echo "❌ Core build failed - missing file(s): ${missing[*]}" + echo "Contents of pkg/:" + ls -la pkg || true + exit 1 +fi
94-101: Make the ESM→classic glue transform resilient across wasm-bindgen variants.The sed rules can miss default function exports and const exports, leading to SyntaxError in workers.
- # Add the JS glue code (convert exports to regular variables for worker context) - sed 's/^export function /function /; s/^export class /class /; s/^export { initSync };/self.initSync = initSync;/; s/^export default __wbg_init;/self.wasm_bindgen = __wbg_init;/; s/import\.meta\.url/self.location.href/g' pkg/sqlite_web_core.js + # Add the JS glue code (convert ESM to classic for worker context) + if command -v node >/dev/null 2>&1; then + node - <<'NODE' +const fs = require('fs'); +let src = fs.readFileSync('pkg/sqlite_web_core.js', 'utf8'); +src = src + .replace(/^export\s+function\s+/mg, 'function ') + .replace(/^export\s+class\s+/mg, 'class ') + .replace(/^export\s+const\s+/mg, 'const ') + .replace(/^export\s+\{\s*initSync\s*\};?/m, 'self.initSync = initSync;') + .replace(/^export\s+default\s+function\s+([A-Za-z0-9_$]+)\s*\(/m, 'function $1(') + .replace(/^export\s+default\s+([A-Za-z0-9_$]+);/m, 'self.wasm_bindgen = $1;') + .replace(/import\.meta\.url/g, 'self.location.href'); +if (!/self\.wasm_bindgen\s*=/.test(src)) { + src += '\nself.wasm_bindgen = (typeof __wbg_init !== "undefined") ? __wbg_init : (typeof init !== "undefined" ? init : undefined);\n'; +} +process.stdout.write(src); +NODE + else + sed -E 's/^export function /function /; s/^export class /class /; s/^export const /const /; s/^export[[:space:]]+\{[[:space:]]*initSync[[:space:]]*\};/self.initSync = initSync;/; s/^export default function ([A-Za-z0-9_]+)\(/function \1(/; s/^export default __wbg_init;/self.wasm_bindgen = __wbg_init;/; s/import\.meta\.url/self.location.href/g' pkg/sqlite_web_core.js + fi
101-101: Fail fast if embedded WASM base64 looks suspiciously small.Catches truncated/empty artifacts before shipping.
-} | awk 'BEGIN{getline b64<"pkg/sqlite_web_core_bg.wasm.b64"} {gsub(/__WASM_B64_CORE__/, b64)}1' > packages/sqlite-web/src/embedded_worker.js.final +} | awk 'BEGIN{getline b64<"pkg/sqlite_web_core_bg.wasm.b64"} {gsub(/__WASM_B64_CORE__/, b64)}1' > packages/sqlite-web/src/embedded_worker.js.final +test $(wc -c < pkg/sqlite_web_core_bg.wasm.b64) -gt 1024 || { echo "❌ Embedded WASM seems too small"; exit 1; }
112-115: Build the main package in release mode as well.Consistency with core; smaller artifacts in pkg/.
cd packages/sqlite-web -wasm-pack build --target web --out-dir ../../pkg +wasm-pack build --release --target web --out-dir ../../pkg cd ../..
125-129: Preserve dependencies and remove duplicate uninstall in svelte-test.Current sequence deletes node_modules and repeats removal; use npm add to install the tarball cleanly.
cd svelte-test -npm remove sqlite-web -npm remove sqlite-web -rm -rf node_modules package-lock.json -npm install ../pkg/sqlite-web-*.tgz -npm install +npm remove sqlite-web || true +npm add ../pkg/sqlite-web-*.tgz +npm install cd ..
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
scripts/local-bundle.sh(5 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: test
- GitHub Check: test
🔇 Additional comments (5)
scripts/local-bundle.sh (5)
27-27: LGTM: cross-platform base64 generation.Using tr -d '\n' avoids GNU/BSD base64 -w differences.
32-87: Template generation looks solid.Good worker-scoped utilities, safe fetch fallback, and clear error reporting.
62-62: LGTM: single-key WASM map is clear and explicit.Easy to extend if more WASM artifacts are embedded later.
106-108: Useful build telemetry.Size and line counts are helpful for spotting anomalies.
70-70: The grep didn’t find anyembedded_worker.js—it may not have been built yet or lives under a different path. Please:
- Run the full local-bundle build (so the worker gets emitted).
- Locate the generated worker file (e.g.
dist/embedded_worker.js,scripts/embedded_worker*.js, etc.).- Re-run these checks against that file to ensure there are no
exportstatements and thatwasm_bindgen(is referenced:file=$(find . -type f -name "embedded_worker*.js" | head -n1) echo "Checking $file" rg -n '^export\s' "$file" && { echo "ESM exports remain"; exit 1; } || echo "No ESM exports found" rg -n 'wasm_bindgen\s*\(' "$file" && echo "Found wasm_bindgen references" || echo "No wasm_bindgen references found"If you still can’t locate the worker bundle or the checks fail, please verify manually.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (4)
flake.nix (4)
18-24: Use pushd/popd for safer dir changes in test-wasm.Stack-based directory changes reduce path mistakes and simplify future edits. This was flagged previously; re-applying here.
- cd packages/sqlite-web-core + pushd packages/sqlite-web-core wasm-pack test --headless --chrome - cd ../.. - cd packages/sqlite-web + popd + pushd packages/sqlite-web wasm-pack test --headless --chrome - cd ../.. + popd
31-37: Out-dir collision: both crates write to ../../pkg (artifacts clobber). Also switch to pushd/popd.Building sqlite-web-core and sqlite-web into the same pkg directory overwrites outputs. Write to distinct subdirs and (optionally) build with --release. This was raised earlier and remains unresolved.
- cd packages/sqlite-web-core - wasm-pack build --target web --out-dir ../../pkg - cd ../.. - cd packages/sqlite-web - wasm-pack build --target web --out-dir ../../pkg - cd ../.. + pushd packages/sqlite-web-core + wasm-pack build --target web --out-dir ../../pkg/sqlite-web-core --release + popd + pushd packages/sqlite-web + wasm-pack build --target web --out-dir ../../pkg/sqlite-web --release + popdFollow-up: if scripts/local-bundle.sh consumes ../../pkg, update it to read from ../../pkg/sqlite-web-core and ../../pkg/sqlite-web (or merge/copy their contents as needed). I can provide a patch if you share that script.
60-69: build-submodules may fail without Foundry project; add guard and safer dir changes.Prior verification indicated lib/rain.math.float/foundry.toml was missing, which would make
forge buildfail. Add checks and use pushd/popd. Also ensureforgeis on PATH afterrainix-sol-prelude.- build-submodules = rainix.mkTask.${system} { + build-submodules = rainix.mkTask.${system} { name = "build-submodules"; body = '' set -euxo pipefail rainix-sol-prelude - cd lib/rain.math.float - forge build - cd ../.. + command -v forge >/dev/null || { echo "forge not found on PATH after rainix-sol-prelude"; exit 1; } + pushd lib/rain.math.float + test -f foundry.toml || { echo "Missing foundry.toml in lib/rain.math.float; is the submodule initialized?"; exit 1; } + forge build + popd ''; };Run this quick repo check to verify the assumptions:
#!/usr/bin/env bash set -euo pipefail # Confirm submodule and Foundry project exist and forge is available test -d lib/rain.math.float || { echo "Missing lib/rain.math.float"; exit 1; } if ! command -v forge >/dev/null; then echo "forge not on PATH; ensure rainix-sol-prelude is invoked before build or add Foundry to devShells.default" exit 1 fi if [ ! -f lib/rain.math.float/foundry.toml ]; then echo "foundry.toml missing under lib/rain.math.float" exit 1 fi echo "Submodule and Foundry config present."
83-83: Add Chromium to dev shell; wasm-pack test uses --chrome.Without a Chrome binary in nix develop, test-wasm will fail locally/CI. Include pkgs.chromium. This was suggested earlier and is still necessary.
- packages = [ packages.test-wasm packages.build-wasm packages.local-bundle packages.test-ui packages.build-submodules packages.test-full-integration pkgs.wasm-pack ]; + packages = [ packages.test-wasm packages.build-wasm packages.local-bundle packages.test-ui packages.build-submodules packages.test-full-integration pkgs.wasm-pack pkgs.chromium ];
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
flake.nix(4 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: test
- GitHub Check: test
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (4)
svelte-test/src/routes/sql/+page.svelte (1)
29-31: Avoid fixed 1s sleep; use an explicit readiness probe.A hard sleep is brittle on slow/fast devices. Prefer a trivial query or an API-provided readiness signal.
Apply this diff:
- status = 'Waiting for worker to be ready...'; - await new Promise(resolve => setTimeout(resolve, 1000)); + status = 'Probing worker readiness...'; + // If the worker isn't ready yet, this will naturally await until it is. + await db.query('SELECT 1');svelte-test/src/routes/+page.svelte (3)
22-27: Surface the actual constructor error instead of throwing a generic one.You lose the concrete error detail from
res.error.msg, which complicates triage and violates the now-stricterno-consolelogging policy (since you can’t log it). Prefer setting status with the real message and returning.Apply this diff:
- let res = SQLiteWasmDatabase.new(); - if (res.error) { - throw new Error('Failed to create database connection'); - } - db = res.value; + let res = SQLiteWasmDatabase.new(); + if (res.error) { + status = `Failed to create database connection: ${res.error.msg}`; + return; + } + db = res.value;
29-31: Replace fixed delay with a readiness ping.Same rationale as the SQL console page.
Apply this diff:
- status = 'Waiting for worker to be ready...'; - // Wait for worker to be ready - await new Promise(resolve => setTimeout(resolve, 1000)); + status = 'Probing worker readiness...'; + await db.query('SELECT 1');
52-52: Nit: extra space in function declaration.Tiny formatting glitch.
Apply this diff:
- async function loadUsers() { + async function loadUsers() {
♻️ Duplicate comments (1)
svelte-test/src/routes/sql/+page.svelte (1)
2-4: Dispose the database/worker on teardown.Matches a prior review on this file; still missing cleanup. Prevent resource leaks during navigation.
Apply this diff:
- import { onMount } from 'svelte'; + import { onMount, onDestroy } from 'svelte'; @@ let db: SQLiteWasmDatabase | undefined; @@ onMount(async () => { // ... }); + + onDestroy(() => { + // Adjust method based on actual API: close/terminate/free + (db as any)?.close?.(); + (db as any)?.terminate?.(); + });
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (3)
svelte-test/eslint.config.js(1 hunks)svelte-test/src/routes/+page.svelte(2 hunks)svelte-test/src/routes/sql/+page.svelte(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
svelte-test/**/*.{js,ts,svelte}
📄 CodeRabbit inference engine (CLAUDE.md)
Use an async/Promise-based API when interacting with the SQLite worker (await/Promises for query results)
Files:
svelte-test/eslint.config.jssvelte-test/src/routes/sql/+page.sveltesvelte-test/src/routes/+page.svelte
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: test
- GitHub Check: test
🔇 Additional comments (5)
svelte-test/src/routes/sql/+page.svelte (2)
4-4: Import path rename tosqlite-web: looks consistent with the repo-wide migration.API surface (init, SQLiteWasmDatabase) appears unchanged here. Good to go.
4-4: No lingeringsqlite-workerreferences found
All searches—including imports (static, dynamic, and CommonJS), filenames, lock and manifest files, and documentation—returned no occurrences of “sqlite-worker.” The migration tosqlite-webis complete.svelte-test/src/routes/+page.svelte (3)
4-4: Import path updated tosqlite-web: OK.The symbols and usage remain compatible with the rest of the code in this file.
108-108: Status-only error reporting for Clear All: good alignment withno-console.The switch from console logging to status messaging keeps errors visible without violating lint rules.
71-73: Parameter binding isn’t currently supported bydb.query– sanitize inputs insteadThe
db.querymethod exposed by the WASM binding only accepts a single SQL string (see its definition inpackages/sqlite-web/src/lib.rs), so you can’t pass parameters directly to it. To avoid SQL-injection and quoting bugs, you should properly escape any interpolated values before embedding them in your query.• File:
svelte-test/src/routes/+page.svelte, lines 71–73 and 92–93
• Replace direct interpolation ofnewUserName/newUserEmailandidwith an escaping helper, for example:function sqlEscape(val: string) { return val.replace(/'/g, "''"); } // … await db.query(` INSERT OR IGNORE INTO users (name, email) VALUES ('${sqlEscape(newUserName.trim())}', '${sqlEscape(newUserEmail.trim())}') `); … await db.query(` DELETE FROM users WHERE id = ${Number(id)} `);• Optionally, extract and reuse a helper (e.g.
formatInsert(table, columns, values)) to centralize escaping.If you’d prefer true parameter binding in the future, you’ll need to extend the WASM API (adding a
query(sql: string, params: any[])overload and handling prepared statements on the Rust side).Likely an incorrect or invalid review comment.
| # Create sqlite-web npm package tarball | ||
| - name: Create sqlite-web NPM Package Tarball | ||
| if: ${{ env.OLD_HASH != env.NEW_HASH }} | ||
| run: | | ||
| cd pkg | ||
| echo "NPM_PACKAGE=$(npm pack --silent)" >> $GITHUB_ENV | ||
| - name: Rename sqlite-web NPM Package Tarball | ||
| if: ${{ env.OLD_HASH != env.NEW_HASH }} | ||
| run: mv pkg/${{ env.NPM_PACKAGE }} sqlite_web_npm_package_${{ env.NEW_VERSION }}.tgz | ||
|
|
There was a problem hiding this comment.
nitpick: ShellCheck would also be complaining about lack of quotes
There was a problem hiding this comment.
nitpick: wouldn't it be more consistent with the structure of other repos to put crates under crates/ and only leave TS code under packages/?
There was a problem hiding this comment.
this is a good idea so i am going to create an issue to track this
Motivation
sqlite-webSolution
Checks
By submitting this for review, I'm confirming I've done the following:
Summary by CodeRabbit
Refactor
New Features
Build
Tests
Chores